Re: [Xen-devel] [PATCH 09/10] tools/x86emul: Advertise more CPUID features for testing purposes

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/03/17 13:13, Jan Beulich wrote:
>>>> On 27.03.17 at 13:20, <george.dunlap@xxxxxxxxxx> wrote:
>> On 27/03/17 10:56, Andrew Cooper wrote:
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>>> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
>>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>>> ---
>>>  tools/tests/x86_emulator/x86_emulate.c | 41 
>>> ++++++++++++++++++++++++----------
>>>  1 file changed, 29 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/tools/tests/x86_emulator/x86_emulate.c 
>> b/tools/tests/x86_emulator/x86_emulate.c
>>> index cea0595..2c49954 100644
>>> --- a/tools/tests/x86_emulator/x86_emulate.c
>>> +++ b/tools/tests/x86_emulator/x86_emulate.c
>>> @@ -73,20 +73,37 @@ int emul_test_cpuid(
>>>           : "a" (leaf), "c" (subleaf));
>>>  Oh, s
>>>      /*
>>> -     * The emulator doesn't itself use MOVBE, so we can always run the
>>> -     * respective tests.
>>> +     * Some instructions and features can be emulated without specific
>>> +     * hardware support.  These features are unconditionally reported here,
>>> +     * for testing and fuzzing-coverage purposes.
>> But similarly to my question in patch 10 -- is there any chance that the
>> emulator will ever be called with a cpuid callback that returns 'false"
>> for these?  If so, isn't there therefore a chance that there will be
>> some sort of bug which only triggers if these bits are set to 'false'?
> I think I've suggested before that the cpuid hook should actually
> return void, as it can't possibly fail (now that CPUID faulting is
> being handled in generic code).

I've been considering this quite a lot recently.  One the one hand, the
introspection hook for CPUID really ought to be using X86EMUL_RETRY.

On the other, we really are (ab)using the existing cpuid() hook for two
different purposes.  There really is a conceptual difference between
issuing a cpuid() call as part of emulating a CPUID instruction, and
using it to find out whether other instructions are permitted.  The
latter is synonymous to having or not having the requisite piece of
silicon, and isn't something which can fail.

In light of the new struct cpuid_policy, I was considering exposing that
to the emulator, so the feature checks are straight bit tests.  This
would separate the two different purposes we have, and should reduce the
size of x86_emulate() a little  (At the moment, it is 1/4 MB compiled,
or about 1/6th of the entire hypervisor.)

Furthermore, the soon-to-appear struct msr_policy would help resolve our
speculative MSR read issues in a similar way.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel