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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.06.19 at 15:19, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 15/03/2019 11:06, Jan Beulich wrote:
>> Note that the ISA extensions document revision 035 is ambiguous
>> regarding fault suppression for VGF2P8MULB: Text says it's supported,
>> while the exception specification listed is E4NF. Given the wording here
>> and for the other two insns I'm inclined to trust the text more than the
>> exception reference, which was also confirmed informally.
> 
> Version 037 has the exception reference as E4 rather than E4NF, so I
> think this entire paragraph is stale now and can be dropped.

Oh, indeed, they've corrected that.

> (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.

>> As to the feature dependency adjustment, while strictly speaking SSE is
>> a sufficient prereq (to have XMM registers), vectors of bytes and qwords
>> have got introduced only with SSE2. gcc, for example, uses a similar
>> connection in its respective intrinsics header.
> 
> This is stale now that you've moved the other integer dependences to SSE2.

Hmm, it's redundant with that other change, but not really stale.
I can drop it if you want.

>> @@ -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.

Jan



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