>>> On 06.03.17 at 14:30, <andrew.coop...@citrix.com> wrote: > On 06/03/17 13:10, Jan Beulich wrote: >>>>> On 06.03.17 at 11:30, <andrew.coop...@citrix.com> wrote: >>> The mov to/from cr/dr behave as if they were encoded with Mod = 3. When >>> encoded with Mod != 3, no displacement or SIB bytes are fetched. >> Would mind letting us know how you became aware of this oddity? >> It's clearly both unexpected > > Most definitely. > >> and undocumented, > > Actually, the contrary. It is explicitly called out in both Intel and > AMDs instruction manual, which is why I noticed it.
Ah, well hidden in the middle of text. Using standard opcode representation for non-standard encodings is far from helpful - that's what I look at mainly (and the text becomes important only when there _are_ oddities there). > I have confirmed that hardware doesn't fetch disp or SIB bytes on my > oldest and newest Intel and AMD boxes. So did I. >> and I wonder whether there are any other opcodes behaving the same. > > Not that I have spotted. Neither me, but there must be something in the decoder special casing those insns, and it's easily imaginable for such special cases to also apply elsewhere. >>> --- a/tools/tests/x86_emulator/test_x86_emulator.c >>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c >>> @@ -894,6 +894,27 @@ int main(int argc, char **argv) >>> } >>> printf("okay\n"); >>> >>> + printf("%-40s", "Testing mov %%cr4,%%esi (bad ModRM)..."); >>> + /* >>> + * Mod = 1, Reg = 4, R/M = 6 would normally encode a memory reference >>> of >>> + * disp8(%esi), but mov to/from cr/dr are special and behave as if they >>> + * were encoded with Mod == 3. >>> + */ >>> + instr[0] = 0x0f; instr[1] = 0x20, instr[2] = 0146; >> I can guess why you've done it this way, but I'd prefer if we didn't >> start using octal numbers. I for one am very used to reading ModRM >> bytes in their hex representation. > > The argument ought to be for which representation is easier. I'd accept > an argument for consistency with the rest of the code, but are you > seriously saying you think that hex is easier to read/manipulate than > octal for ModRM/SIB bytes? Yes, and very seriously in fact. Nevertheless I admit that this is a matter of what one is used to. The only alternative to hex I'd freely accept would be to use bit fields to fill the pieces individually. >>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >>> @@ -2086,7 +2086,8 @@ x86_decode_twobyte( >>> } >>> /* fall through */ >>> case 0x21: case 0x23: /* mov to/from dr */ >>> - generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD); >>> + ASSERT(ea.type == OP_REG); /* Early operand adjustment ensures >>> this. */ >>> + generate_exception_if(lock_prefix, EXC_UD); >>> op_bytes = mode_64bit() ? 8 : 4; >>> break; >>> >>> @@ -2427,6 +2428,23 @@ x86_decode( >>> break; >>> } >>> } >>> + else if ( ext == ext_0f ) >>> + { >>> + switch ( b ) >>> + { >>> + case 0x20: /* mov cr,reg */ >>> + case 0x21: /* mov dr,reg */ >>> + case 0x22: /* mov reg,cr */ >>> + case 0x23: /* mov reg,dr */ >>> + /* >>> + * Mov to/from cr/dr ignore the encoding of Mod, and >>> behave as >>> + * if they were encoded as reg/reg instructions. No futher >>> + * disp/SIB bytes are fetched. >>> + */ >>> + modrm_mod = 3; >>> + break; >>> + } >>> + } >> Just like done by >> https://lists.xenproject.org/archives/html/xen-devel/2017-03/msg00588.html >> this calls for the outer if/else to be morphed into a switch(). > > I did that first, but it was rather complicated to read. I can switch > back to that if you'd prefer. Well, if you don't here then that other patch will in any case. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel