Re: [PATCH v2] x86emul: don't read mask register on AVX512F-incapable platforms

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>,
	Juergen Gross <jgross@suse.com>, Wei Liu <wei.liu2@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v2] x86emul: don't read mask register on AVX512F-incapable platforms
Date: Fri, 29 Mar 2019 09:19:56 +0000	[thread overview]
Message-ID: <0eaf98eb-3e30-0136-1c75-1deb7c8a3bbc@citrix.com> (raw)
In-Reply-To: <5C9DCC4A0200007800222AEA@prv1-mh.provo.novell.com>

On 29/03/2019 07:42, Jan Beulich wrote:
> Nor when register state isn't sufficiently enabled.
>
> Reported-by: George Dunlap <george.dunlap@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> This is surely a stable tree candidate, unless it could still make it
> into 4.12 before the release.
> ---
> v2: Add XCR0 check.
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -3511,7 +3511,8 @@ x86_emulate(
>      }
>  
>      /* With a memory operand, fetch the mask register in use (if any). */
> -    if ( ea.type == OP_MEM && evex.opmsk )
> +    if ( ea.type == OP_MEM && cpu_has_avx512f && evex.opmsk &&
> +         _get_fpu(fpu_type = X86EMUL_FPU_opmask, ctxt, ops) == X86EMUL_OKAY )

The cpu_has_avx512f check is now redundant.  It is fully encapsulated by
_get_fpu() being happy with X86EMUL_FPU_opmask.

Preferably with it dropped, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>

>      {
>          uint8_t *stb = get_stub(stub);
>  
> @@ -3532,6 +3533,14 @@ x86_emulate(
>          fault_suppression = true;
>      }
>  
> +    if ( fpu_type == X86EMUL_FPU_opmask )
> +    {
> +        /* Squash (side) effects of the _get_fpu() above. */
> +        x86_emul_reset_event(ctxt);
> +        put_fpu(X86EMUL_FPU_opmask, false, state, ctxt, ops);
> +        fpu_type = X86EMUL_FPU_none;
> +    }
> +
>      /* Decode (but don't fetch) the destination operand: register or memory. */
>      switch ( d & DstMask )
>      {
>
>
>
>

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