Re: [Xen-devel] [PATCH v8 10/50] x86emul: support AVX512{F, BW, _VBMI} full permute insns

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.05.19 at 18:50, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 15/03/2019 10:41, Jan Beulich wrote:
>> Take the liberty and also correct the (public interface) name of the
>> AVX512_VBMI feature flag, on the assumption that no external consumer
>> has actually been using that flag so far.
> 
> I've been giving this some thought, and I think putting these in the
> public interface was a mistake.
> 
> They are a representation of other peoples stable ABI, and are only used
> in tools interfaces as far as Xen is concerned (SYSCTL_get_featureset,
> DOMCTL_get_cpu_policy)
> 
> The only external representations are xen_cpuid_leaf_t/xen_msr_entry_t
> which don't need constants to go with them.

While I don't really mind removing them from the public interface
again, I think you're missing to discuss one point of why they've
been put there originally: We wanted them to also serve as
documentation of the forward compatible nature of the A/S/H
annotations, i.e. such that we wouldn't lightly remove something
that was previously exposed.

>>  Furthermore make it have
>> AVX512BW instead of AVX512F as a prerequisite, for requiring full
>> 64-bit mask registers (the upper 48 bits of which can't be accessed
>> other than through XSAVE/XRSTOR without AVX512BW support).
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> As for the rest, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> but
> we perhaps want to sort out the position in the public interface before
> changing AVX512VBMI.

Thanks; I don't think the order of events matters much, but if this is
something to happen soon, I don't mind waiting (and the re-basing).

There's an important point here though that you don't say any word
on, despite me having mentioned it to you on irc already after you
had given your ack to "x86/CPUID: support leaf 7 subleaf 1 /
AVX512_BF16" making a similar dependency as the patch here upon
AVX512BW rather than AVX512F. Recall that I had to split off the
change below from another patch of mine, because of our
disagreement on the intended dependencies. If we make the sub-
features requiring wider than 16-bit mask registers dependent
upon AVX512BW (as suggested by the patch here), then I think
we also want to change the SSEn dependencies as done/suggested
in the patch below (albeit of course there's no dependency on
changed register width there, just one on permitted element types,
so I could also see a basis to accept the dependency here as is, but
view things differently for SSEn).

Seeing your further acks (thanks!), just as a side note: Are you
aware that you've skipped patch 9 in the series, without which the
ones you've acked can't go in anyway (i.e. regardless of whether
I'm to wait for the public interface adjustment above)?

Jan

x86/cpuid: correct dependencies of post-SSE ISA extensions

Move AESNI, PCLMULQDQ, and SHA to SSE2, as all of them act on vectors of
integers, whereas plain SSE supports vectors of single precision floats
only. This is in line with how e.g. binutils and gcc treat them.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v?: Re-base over split out PCLMULQDQ addition.
---
TBD: On the same basis, SSE3, SSSE3 and SSE4A should probably also
depend on SSE2 rather than SSE. In fact making this a chain SSE -> SSE2
-> SSE3 -> { SSSE3, SSE4A } would probably be best, and get us in line
with both binutils and gcc. But I think I did suggest so when the
dependencies were introduced, and this wasn't liked for a reason I
forgot.

--- unstable.orig/xen/tools/gen-cpuid.py
+++ unstable/xen/tools/gen-cpuid.py
@@ -196,11 +196,12 @@ def crunch_numbers(state):
         # instructions.  Several futher instruction sets are built on core
         # %XMM support, without specific inter-dependencies.  Additionally
         # AMD has a special mis-alignment sub-mode.
-        SSE: [SSE2, SSE3, SSSE3, SSE4A, MISALIGNSSE,
-              AESNI, PCLMULQDQ, SHA],
+        SSE: [SSE2, SSE3, SSSE3, SSE4A, MISALIGNSSE],
 
-        # SSE2 was re-specified as core instructions for 64bit.
-        SSE2: [LM],
+        # SSE2 was re-specified as core instructions for 64bit.  Also ISA
+        # extensions dealing with vectors of integers are added here rather
+        # than to SSE.
+        SSE2: [LM, AESNI, PCLMULQDQ, SHA],
 
         # SSE4.1 explicitly depends on SSE3 and SSSE3
         SSE3: [SSE4_1],


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