Re: [PATCH v3] x86/emul: Adjust handling of CR8_LEGACY

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/07/2026 10:29 am, Teddy Astie wrote:
> Le 07/07/2026 à 21:49, Andrew Cooper a écrit :
>> The APM description of the AltMovCR8 feature bit is:
>>
>>    "LOCK MOV CR0 means MOV CR8"
>>
>> Adjust the decode logic to behave like this.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Teddy Astie <teddy.astie@xxxxxxxxxx>
>>
>> v3:
>>   * Change yet again.  A contact with information on the matter
>> confirmed that
>>     it is a special case for Reg = 0, and not a general modifier to
>> all Reg
>>     values.
>> ---
>>   xen/arch/x86/x86_emulate/decode.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/x86_emulate/decode.c
>> b/xen/arch/x86/x86_emulate/decode.c
>> index 2c13356c4d17..57f6baffb066 100644
>> --- a/xen/arch/x86/x86_emulate/decode.c
>> +++ b/xen/arch/x86/x86_emulate/decode.c
>> @@ -780,12 +780,12 @@ decode_twobyte(struct x86_emulate_state *s,
>>           break;
>>         case 0x20: case 0x22: /* mov to/from cr */
>> -        if ( s->lock_prefix && vcpu_has_cr8_legacy() )
>> +        if ( s->lock_prefix && vcpu_has_cr8_legacy() && s->modrm_reg
>> == 0 )
>>           {
>> -            s->modrm_reg += 8;
>> +            s->modrm_reg = 8;
>>               s->lock_prefix = false;
>>           }
>> -        /* fall through */
>> +        fallthrough;
>>       case 0x21: case 0x23: /* mov to/from dr */
>>           ASSERT(s->ea.type == OP_REG); /* Early operand adjustment
>> ensures this. */
>>           generate_exception_if(s->lock_prefix, X86_EXC_UD);
>
> I'm fine with it; though we probably want to make sure we always
> advertise the AltMovCr8 feature bit as APM suggest users to always
> check for this before relying on this behavior.
>
> Interestingly, I see this feature bit set on Intel hardware.

Are you mixing up the basic and extended feature leaves?  Intel does not
have this behaviour and doesn't (to my knowledge) advertise the CPUID bit.

> Though, it's a reserved bit in SDM, and lock mov cr0 is defined as
> being #UD.
>
> Which CPU model introduced this feature bit ? And does Intel behavior
> matches AMD's spec ?

That was in the v2 commit message, but I took it out here.

AMD K8 Rev F.  Prior K8 revisions didn't have it.

~Andrew