Re: [Xen-devel] [PATCH] x86/emul: Correct the decoding of mov to/from cr/dr

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.03.17 at 11:30, <andrew.cooper3@xxxxxxxxxx> 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 and undocumented, and I wonder
whether there are any other opcodes behaving the same. In any
even I think we want to make at least an attempt at having HW
vendors confirm this (and to address the other-opcodes concern),
so I'm extending the Cc list.

> --- 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.

> --- 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().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel