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>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Wei Liu <wei.liu2@citrix.com>, Juergen Gross <jgross@suse.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v2] x86emul: don't read mask register on AVX512F-incapable platforms
Date: Mon, 1 Apr 2019 15:14:17 +0100	[thread overview]
Message-ID: <4a2b8058-1f17-cdb0-005e-a6be2af61120@citrix.com> (raw)
In-Reply-To: <5C9DF9CA0200007800222C2E@prv1-mh.provo.novell.com>

On 29/03/2019 10:56, Jan Beulich wrote:
>>>> On 29.03.19 at 11:02, <andrew.cooper3@citrix.com> wrote:
>> On 29/03/2019 09:36, Jan Beulich wrote:
>>> I'd like to put up the other option then: Rather than using
>>> _get_fpu() (and in particular the read_xcr() and read_cr() hooks)
>>> we could read the real XCR0 here. After all we issue the KMOV not
>>> because the guest has specified it, but because we need the value
>>> of the register for correct fault suppression emulation.
>> True, and that would be rather smaller and less invasive than
>> deliberately squashing the other side effects of get_fpu()
> Hmm, I've tried to do this, but this is more complicated: CR0.TS
> may be set, in which case we need to invoke the get_fpu() hook
> to get it cleared with appropriate bookkeeping. I don't think it's
> worth further complicating the code by invoking the hook _only_
> in that case. So I guess we better stick to v2.

Oh ok.  That does complicated things.  Lets just use the existing
infrastructure, even if it is rather heavyweight.

>
> Which makes me come back to your request to drop the
> cpu_has_avx512f part of the condition: Right now the fuzzer
> uses emul_test_read_xcr() instead of actually fuzzing the
> value. Once it does, would we expect it to never set any bits
> in the returned value that aren't set in hardware, but could
> in principle be set based on (real) CPUID output? In that case
> I could agree to remove the extra condition.

I don't see how we could ever emulate with a (v)xcr0 different to a
legitimate value in hardware, as the stubs would #UD.

I also don't see how the userspace tools could ever test with a value
other than what it can see in xgetbv, because only the kernel gets to
choose %xcr0.  Even with faking up a smaller xcr0, you'd end up with
instructions which should fault but don't.

~Andrew

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