Re: [Xen-devel] [PATCH v5 03/14] x86emul: abstract out XCRn accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.18 at 16:41, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 15/03/18 13:04, Jan Beulich wrote:
>> --- a/xen/arch/x86/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate.c
>> @@ -42,3 +42,50 @@
>>  })
>>  
>>  #include "x86_emulate/x86_emulate.c"
>> +
>> +int x86emul_read_xcr(unsigned int reg, uint64_t *val,
>> +                     struct x86_emulate_ctxt *ctxt)
>> +{
>> +    switch ( reg )
>> +    {
>> +    case 0:
>> +        *val = current->arch.xcr0;
>> +        return X86EMUL_OKAY;
>> +
>> +    case 1:
>> +        if ( cpu_has_xgetbv1 && current->domain->arch.cpuid->xstate.xgetbv1 
>> )
>> +            break;
>> +        /* fall through */
>> +    default:
>> +        x86_emul_hw_exception(TRAP_gp_fault, 0, ctxt);
>> +        return X86EMUL_EXCEPTION;
>> +    }
>> +
>> +    *val = xgetbv(reg);
>> +
>> +    return X86EMUL_OKAY;
>> +}
>> +
>> +int x86emul_write_xcr(unsigned int reg, uint64_t val,
>> +                      struct x86_emulate_ctxt *ctxt)
>> +{
>> +    switch ( reg )
>> +    {
>> +    case 0:
>> +        break;
>> +
>> +    case 1:
>> +        if ( cpu_has_xgetbv1 && current->domain->arch.cpuid->xstate.xgetbv1 
>> )
>> +            break;
>> +        /* fall through */
> 
> Actually, this is wrong.  cpu_has_xgetbv1 applies only to the read
> side.  xsetbv[1] is still strictly reserved and yields #GP.  (Given the
> way other bits in xcr0 work, I wouldn't be surprised if xsetbv[1] is
> reserved forever more.)
> 
> I'd just drop this case block and let 1 fall into the default case,
> rather than relying on the sanity check in handle_xsetbv()

Oh, indeed, this was rather silly of me.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel