On 06/11/2018 13:45, Jan Beulich wrote:
> Now that there's an almost unconditional CR4 read right at the beginning
> of x86_emulate(), centralize its reading there and use result and value
> everywhere else without further invoking the hook.
>
> Subsequently we may want to consider having the callers provide
> whichever value they deem appropriate in their contexts, to avoid
> invoking the hook altogether for this purpose.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>

I'm afraid that I am still unconvinced that cr4_rc is a clever move.

It would be far more simple to require callers to provide it in
x86_emulate_ctxt.

> @@ -4000,13 +4000,10 @@ x86_emulate(
>          if ( (_regs.eflags & X86_EFLAGS_VM) &&
>               MASK_EXTR(_regs.eflags, X86_EFLAGS_IOPL) != 3 )
>          {
> -            cr4 = 0;
> -            if ( op_bytes == 2 && ops->read_cr )
> -            {
> -                rc = ops->read_cr(4, &cr4, ctxt);
> -                if ( rc != X86EMUL_OKAY )
> -                    goto done;
> -            }
> +            if ( op_bytes == 2 )
> +                check_cr4();
> +            else
> +                cr4 = 0;

This clobbers cr4, which is a latent bug if any of the retire logic
wants to start using the value.

This code is only like this because you've overloaded the value in CR4
to include an implicit "opsize == 16", and results in exceedingly
complicated logic to follow.

~Andrew

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

Reply via email to