Re: [Xen-devel] [PATCH][XTF] add FPU/SIMD register state test

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.03.17 at 12:36, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 13/03/17 11:07, Jan Beulich wrote:
>> Add tests to verify that
>> - FPU insns leave correct (guest) values in FIP/FDP/FOP/FCS/FDS,
>> - FPU insns writing memory don't update FPU register state when the
>>   write faults (at the example of FISTP),
>> - VCVTPS2PH (once implemented in the emulator) doesn't update MXCSR
>>   if its write faults (VCVTPS2PH currently is the only SIMD insn
>>   writing to memory and updating register state).
>>
>> Note that the AMD-specific code in fpu_fxrstor() and xrstor() causes
>> the FISTP part of this test to always fail. I don't really have any
>> idea what to do about this (other than perhaps skipping the test on AMD
>> altogether).
>>
>> Note further that the FCS part of 64-bit variant of the FSTP test also
>> always fails on AMD, since, other than on Intel, {F,}XRSTOR with REX.W
>> set do not appear to clear FCS/FDS (I can't find any statement either
>> way in the PM).
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> TBD: It is not clear to me how to deal with the native execution
>>      failure of VCVTPS2PH on at least some Intel models.
> 
> As a general comment, tests dealing with quirks like this should
> tolerate real hardware behaviour, without causing the test to fail. 
> There is nothing useful from having tests failing in cases we cant/wont
> do anything to fix.
> 
> As for the Intel behaviour, Is this documented, or does it have an
> erratum reference?

Neither - I am waiting for them to confirm whether this is an erratum,
or whether (for some strange reason) this is intended behavior.

> For the AMD behaviour, some more though is going to be required.

One option I've been considering was to dynamically select between
xtf_failure() and xtf_warning() for the problematic checks.

>> +void probe_fstp(bool force)
>> +{
>> +    const uint8_t *fstp_offs;
>> +    uint32_t flt;
>> +    struct x87_env fenv;
>> +
>> +    fenv.cw = 0x35f; /* unmask PE */
>> +    asm volatile ( "fninit;"
>> +                   "fldcw %[cw];"
>> +                   "fldpi;"
>> +                   "mov $1f, %[offs];"
> 
> You can have the compiler do all of this by using a named label, and using
> 
> extern char fstp_offs[] asm("$label");

Is there any benefit to this? To me this looks uglier than what I
have now.

Jan


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