An FC4/i386 install inside VMX on an x86_64 system fails because byte size is not handled by __set_reg_value. This patch adds that support. This patch also reindents Chengyuan Li cmpb patch so that is aligns with the rest of the code in that block. Signed-Off-By: Leendert van Doorn <leendert@watson.ibm.com> diff -r 3f2751c6e721 xen/arch/x86/vmx_io.c --- a/xen/arch/x86/vmx_io.c Sat Sep 10 14:44:31 2005 +++ b/xen/arch/x86/vmx_io.c Sat Sep 10 14:23:12 2005 @@ -99,7 +99,6 @@ printk("Error: size:%x, index:%x are invalid!\n", size, index); domain_crash_synchronous(); break; - } break; case WORD: @@ -199,6 +198,7 @@ static inline void __set_reg_value(unsigned long *reg, int size, long value) { switch (size) { + case BYTE: case BYTE_64: *reg &= ~0xFF; *reg |= (value & 0xFF); @@ -215,7 +215,7 @@ *reg = value; break; default: - printk("Error: <__set_reg_value> : Unknown size for register\n"); + printk("Error: <__set_reg_value>: size:%x is invalid\n", size); domain_crash_synchronous(); } } diff -r 3f2751c6e721 xen/arch/x86/vmx_platform.c --- a/xen/arch/x86/vmx_platform.c Sat Sep 10 14:44:31 2005 +++ b/xen/arch/x86/vmx_platform.c Sat Sep 10 14:23:12 2005 @@ -55,6 +55,7 @@ static inline long __get_reg_value(unsigned long reg, int size) { switch(size) { + case BYTE: case BYTE_64: return (char)(reg & 0xFF); case WORD: @@ -430,10 +431,10 @@ if (((opcode[1] >> 3) & 7) == 7) { /* cmp $imm, m32/16 */ instr->instr = INSTR_CMP; - if (opcode[0] == 0x80) - GET_OP_SIZE_FOR_BYTE(instr->op_size); - else - GET_OP_SIZE_FOR_NONEBYTE(instr->op_size); + if (opcode[0] == 0x80) + GET_OP_SIZE_FOR_BYTE(instr->op_size); + else + GET_OP_SIZE_FOR_NONEBYTE(instr->op_size); instr->operand[0] = mk_operand(instr->op_size, 0, 0, IMMEDIATE); instr->immediate = get_immediate(vm86, opcode+1, BYTE); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
# The byte size is handled in set_reg_value(), but not in __set_reg_value(), # and it''s true that there is a bug in set_reg_value(). # I''ve send a patch for this bug: Yes, that fixes the problem, our patches crossed. I saw Keir already integrated my patch and while it is harmless, it is redundant and here is the diff to take it out. The patch also addresses some indent problems and aligns instructions up with the rest of the block. <ON SOAPBOX> While on this topic, and I know I''m starting a religious war, could we get some agreement over the prefered indent/coding style for Xen? It is currently a hotchpotch of styles, sometimes even within the same function. I don''t have a strong preference for the style (the Linux kernel style seems appropriate given the amount of Linux code that''s incorporated), as long as it is consistent. <OFF SOAPBOX> Leendert Signed-Off-By: Leendert van Doorn <leendert@watson.ibm.com> diff -r 21cbdb20ff4c xen/arch/x86/vmx_io.c --- a/xen/arch/x86/vmx_io.c Sun Sep 11 09:28:21 2005 +++ b/xen/arch/x86/vmx_io.c Sun Sep 11 11:23:38 2005 @@ -198,7 +198,6 @@ static inline void __set_reg_value(unsigned long *reg, int size, long value) { switch (size) { - case BYTE: case BYTE_64: *reg &= ~0xFF; *reg |= (value & 0xFF); diff -r 21cbdb20ff4c xen/arch/x86/vmx_platform.c --- a/xen/arch/x86/vmx_platform.c Sun Sep 11 09:28:21 2005 +++ b/xen/arch/x86/vmx_platform.c Sun Sep 11 11:23:38 2005 @@ -55,7 +55,6 @@ static inline long __get_reg_value(unsigned long reg, int size) { switch(size) { - case BYTE: case BYTE_64: return (char)(reg & 0xFF); case WORD: @@ -90,10 +89,11 @@ return (char)((regs->rdx & 0xFF00) >> 8); case 7: /* %bh */ return (char)((regs->rbx & 0xFF00) >> 8); - default: + default: printf("Error: (get_reg_value) Invalid index value\n"); - domain_crash_synchronous(); + domain_crash_synchronous(); } + /* NOTREACHED */ } switch (index) { @@ -114,8 +114,8 @@ case 14: return __get_reg_value(regs->r14, size); case 15: return __get_reg_value(regs->r15, size); default: - printf("Error: (get_reg_value) Invalid index value\n"); - domain_crash_synchronous(); + printf("Error: (get_reg_value) Invalid index value\n"); + domain_crash_synchronous(); } } #elif defined (__i386__) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Leendert, The byte size is handled in set_reg_value(), but not in __set_reg_value(), and it''s true that there is a bug in set_reg_value(). I''ve send a patch for this bug: diff -r c8e840ac3019 xen/arch/x86/vmx_io.c --- a/xen/arch/x86/vmx_io.c Tue Sep 6 10:11:20 2005 +++ b/xen/arch/x86/vmx_io.c Fri Sep 9 22:28:25 2005 @@ -261,7 +261,7 @@ domain_crash_synchronous(); break; } - + return ; } switch (index) { leendert@watson.ibm.com scribbled on 2005年9月10日 21:36:> An FC4/i386 install inside VMX on an x86_64 system fails because byte > size is not handled by __set_reg_value. This patch adds that support. > > This patch also reindents Chengyuan Li cmpb patch so that is aligns > with the rest of the code in that block. > > Signed-Off-By: Leendert van Doorn <leendert@watson.ibm.com> > > diff -r 3f2751c6e721 xen/arch/x86/vmx_io.c > --- a/xen/arch/x86/vmx_io.c Sat Sep 10 14:44:31 2005 > +++ b/xen/arch/x86/vmx_io.c Sat Sep 10 14:23:12 2005 > @@ -99,7 +99,6 @@ > printk("Error: size:%x, index:%x are invalid!\n", size, > index); domain_crash_synchronous(); > break; > - > } > break; > case WORD: > @@ -199,6 +198,7 @@ > static inline void __set_reg_value(unsigned long *reg, int size, > long value) { > switch (size) { > + case BYTE: > case BYTE_64: > *reg &= ~0xFF; > *reg |= (value & 0xFF); > @@ -215,7 +215,7 @@ > *reg = value; > break; > default: > - printk("Error: <__set_reg_value> : Unknown size for > register\n"); + printk("Error: <__set_reg_value>: size:%x is > invalid\n", size); domain_crash_synchronous(); > } > } > diff -r 3f2751c6e721 xen/arch/x86/vmx_platform.c > --- a/xen/arch/x86/vmx_platform.c Sat Sep 10 14:44:31 2005 > +++ b/xen/arch/x86/vmx_platform.c Sat Sep 10 14:23:12 2005 > @@ -55,6 +55,7 @@ > static inline long __get_reg_value(unsigned long reg, int size) > { > switch(size) { > + case BYTE: > case BYTE_64: > return (char)(reg & 0xFF); > case WORD: > @@ -430,10 +431,10 @@ > if (((opcode[1] >> 3) & 7) == 7) { /* cmp $imm, m32/16 */ > instr->instr = INSTR_CMP; > > - if (opcode[0] == 0x80) > - GET_OP_SIZE_FOR_BYTE(instr->op_size); > - else > - GET_OP_SIZE_FOR_NONEBYTE(instr->op_size); > + if (opcode[0] == 0x80) > + GET_OP_SIZE_FOR_BYTE(instr->op_size); > + else > + GET_OP_SIZE_FOR_NONEBYTE(instr->op_size); > > instr->operand[0] = mk_operand(instr->op_size, 0, 0, IMMEDIATE); > instr->immediate = get_immediate(vm86, opcode+1, BYTE);Thanks, Chengyuan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> <ON SOAPBOX> > While on this topic, and I know I''m starting a religious war, > could we get some agreement over the prefered indent/coding > style for Xen? It is currently a hotchpotch of styles, > sometimes even within the same function. I don''t have a > strong preference for the style (the Linux kernel style seems > appropriate given the amount of Linux code that''s > incorporated), as long as it is consistent. > <OFF SOAPBOX>This is policy we''ve been operating: For files that belong in guest operating systems (the sparse trees) we use the style of that particular OS. For Xen itself (and the tools) we use bsd style with soft tabs of 4 spaces. The only exceptions to this are the small handful of files that are very closely related to Linux couterparts (apic.c etc). Keeping them in Linux style makes it easier to track changes. For python we use whatever is the default emacs python mode uses. However, some hard tabs have crept in and we could seriously do with running all the python code through a python-specific lint. I think the best way to enforce the above is probably to make sure we have the appropriate magic at the bottom of every file to put common editors into the right mode. Some people are likely to take exception with the style we use for Xen not being Linux style. I find 4 space tabs much easier on the eye than the Linux style and see no reason to change. The tree could do with a bit of tyidying up to make sure every file is at least internally consistent and to add some editor mode info (everyone uses emacs or vi, right?). To cause minimum pain we should do this with plenty of warning so that people can avoid having too many local patches to merge. Right after 3.0 would probably be good. Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel