Re: [Xen-devel] [PATCH v4 08/20] x86emul: abstract out XCRn accesses

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/02/18 13:03, Jan Beulich wrote:
> @@ -5178,18 +5202,33 @@ x86_emulate(
>                  _regs.eflags |= X86_EFLAGS_AC;
>              break;
>  
> -#ifdef __XEN__
> -        case 0xd1: /* xsetbv */
> +        case 0xd0: /* xgetbv */
>              generate_exception_if(vex.pfx, EXC_UD);
> -            if ( !ops->read_cr || ops->read_cr(4, &cr4, ctxt) != 
> X86EMUL_OKAY )
> +            if ( !ops->read_cr || !ops->read_xcr ||
> +                 ops->read_cr(4, &cr4, ctxt) != X86EMUL_OKAY )
>                  cr4 = 0;
>              generate_exception_if(!(cr4 & X86_CR4_OSXSAVE), EXC_UD);
> -            generate_exception_if(!mode_ring0() ||
> -                                  handle_xsetbv(_regs.ecx,
> -                                                _regs.eax | (_regs.rdx << 
> 32)),
> +            generate_exception_if(_regs.ecx > (vcpu_has_xgetbv1() ? 1 : 0),
>                                    EXC_GP, 0);

I'm still opposed to this change.  It is inconsistent with all other
handling in the emulator, because we do not do input register validation
for any of the CR/DR/MSR hooks.

The {read,write}_xcr() hooks should be required to deal with any
arbitrary register, just like the {read,write}_{cr,dr,msr}() hooks are
currently expected to do.

Everything else is fine (subject to the adjustments required to change
this property).

~Andrew

> +            rc = ops->read_xcr(_regs.ecx, &msr_val, ctxt);
> +            if ( rc != X86EMUL_OKAY )
> +                goto done;
> +            _regs.r(ax) = (uint32_t)msr_val;
> +            _regs.r(dx) = msr_val >> 32;
> +            break;
> +
> +        case 0xd1: /* xsetbv */
> +            generate_exception_if(vex.pfx, EXC_UD);
> +            if ( !ops->read_cr || !ops->write_xcr ||
> +                 ops->read_cr(4, &cr4, ctxt) != X86EMUL_OKAY )
> +                cr4 = 0;
> +            generate_exception_if(!(cr4 & X86_CR4_OSXSAVE), EXC_UD);
> +            generate_exception_if(!mode_ring0() || _regs.ecx, EXC_GP, 0);
> +            rc = ops->write_xcr(_regs.ecx,
> +                                _regs.eax | ((uint64_t)_regs.edx << 32), 
> ctxt);
> +            if ( rc != X86EMUL_OKAY )
> +                goto done;
>              break;
> -#endif
>  
>          case 0xd4: /* vmfunc */
>              generate_exception_if(vex.pfx, EXC_UD);
>


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