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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 27.03.17 at 15:03, <andrew.cooper3@citrix.com> wrote:
> On 27/03/17 13:56, George Dunlap wrote:
>> On 27/03/17 13:13, Jan Beulich wrote:
>>>>>> On 27.03.17 at 13:20, <george.dunlap@citrix.com> wrote:
>>>> On 27/03/17 10:56, Andrew Cooper wrote:
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> ---
>>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>>> CC: George Dunlap <george.dunlap@eu.citrix.com>
>>>>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>>> ---
>>>>>  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).
>> This isn't about failing so much as it is about reporting the presence /
>> absence of hardware features.  With this patch, cpuid unconditionally
>> advertises the presence of a number of features (MOVBE, rtm, ADCX/ADOX,
>> &c) because the emulation will work even if the features aren't actually
>> present in hardware.  I'm suggesting that we may want to make sure that
>> we test *both* the "feature is present" path, *and* the "feature is
>> missing" path.
> 
> I have some plans to make this happen, but it isn't easy with the
> existing infrastructure.  In the meantime, It is more important to get
> better coverage.

Can't we simply grab enough bits of input data to cover the ones
of interest here, store them away, and use that instead of the
hard coded 1s here?

Jan


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