Re: [Xen-devel] [PATCH v8 46/50] x86emul: support GFNI insns

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.06.19 at 16:20, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 21/06/2019 15:00, Jan Beulich wrote:
>>> On 15/03/2019 11:06, Jan Beulich wrote:
>>> (On a tangent, AVX512_VP2INTERSECT now exists in the extensions doc.)
>> And I have it implemented, but no way to test until sde supports it.
> 
> Fair enough.
> 
>>>> @@ -138,6 +141,26 @@ static bool simd_check_avx512vbmi_vl(voi
>>>>      return cpu_has_avx512_vbmi && cpu_has_avx512vl;
>>>>  }
>>>>  
>>>> +static bool simd_check_sse2_gf(void)
>>>> +{
>>>> +    return cpu_has_gfni && cpu_has_sse2;
>>> This dependency doesn't match the manual.  The legacy encoding needs
>>> GFNI alone.
>>>
>>> gen-cpuid.py is trying to reduce the ability to create totally
>>> implausible configurations via levelling, but for software checks, we
>>> should follow the manual to the letter.
>> This is test harness code - I'd rather be a little more strict here than
>> having to needlessly spend time fixing an issue in there. Furthermore
>> this matches how gcc enforces dependencies.
>>>> +}
>>>> +
>>>> +static bool simd_check_avx2_gf(void)
>>>> +{
>>>> +    return cpu_has_gfni && cpu_has_avx2;
>>> Here, the dependency is only on AVX, which I think is probably trying to
>>> express a dependency on xcr0.ymm
>> Mostly as per above, except that here gcc (imo wrongly) enables just
>> AVX.
>>
>>>> +}
>>>> +
>>>> +static bool simd_check_avx512bw_gf(void)
>>>> +{
>>>> +    return cpu_has_gfni && cpu_has_avx512bw;
>>> I don't see any BW interaction anywhere (in the manual), despite the
>>> fact it operates on a datatype of int8.
>> But by operating on vectors of bytes, it requires 64 bits wide mask
>> registers, which is the connection to BW. Again gcc also does so.
> 
> To be honest, it doesn't matter what GCC does.
> 
> What matter is the expectation of arbitrary library/application code,
> written in adherence to the Intel manual, when running with a levelled
> CPUID policy, because *that* is the set of corner cases where things
> might end up exploding.

I agree, and I would agree making the changes you've asked for if
it was code in the main emulator. But as said - you've commented
on test harness qualification functions.

> I see your point about needing a full width mask register, which to me
> suggests that the extension manual is documenting the dependency
> incorrectly.
> 
> It also means that I need to change how we do feature dependency
> derivation, because this is the first example of a conditional
> dependency.  I.e. AVX512F but not AVX512BW implies no GFNI even if
> hardware has it, but on the same hardware when levelling AVX512F out,
> GFNI could be used via its legacy or VEX version.

Right, the way GFNI is specified isn't overly helpful. Otoh - why the
former? The legacy and VEX versions are usable in that case, too.

Jan



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