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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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

~Andrew

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