Re: [PATCH 4/6] target/i386: Add feature dependencies for AVX10

[Zhao Liu <zhao1.liu@intel.com>]

Hi Tao,

On Mon, Oct 28, 2024 at 10:45:10AM +0800, Tao Su wrote:
> Date: Mon, 28 Oct 2024 10:45:10 +0800
> From: Tao Su <tao1.su@linux.intel.com>
> Subject: [PATCH 4/6] target/i386: Add feature dependencies for AVX10
> X-Mailer: git-send-email 2.34.1
> 
> Since the highest supported vector length for a processor implies that
> all lesser vector lengths are also supported, add the dependencies of
> the supported vector lengths. If all vector lengths aren't supported,
> clear AVX10 enable bit as well.
> 
> Note that the order of AVX10 related dependencies should be kept as:
>         CPUID_24_0_EBX_AVX10_128     -> CPUID_24_0_EBX_AVX10_256,
>         CPUID_24_0_EBX_AVX10_256     -> CPUID_24_0_EBX_AVX10_512,
>         CPUID_24_0_EBX_AVX10_VL_MASK -> CPUID_7_1_EDX_AVX10,
>         CPUID_7_1_EDX_AVX10          -> CPUID_24_0_EBX,
> so that prevent user from setting weird CPUID combinations, e.g. 256-bits
> and 512-bits are supported but 128-bits is not, no vector lengths are
> supported but AVX10 enable bit is still set.
> 
> Since AVX10_128 will be reserved as 1,

Does this means AVX10_128 bit is reserved and it is always 1?

From the spec you linked in cover letter (Table 1-1. CPUID Enumeration
of Intel® AVX10), it seems AVX10_128 bit is marked as reserved.

It's worth describing its behavior.

> adding these dependencies has the
> bonus that when user sets -cpu host,-avx10-128, CPUID_7_1_EDX_AVX10 and
> CPUID_24_0_EBX will be disabled automatically.
> 
> Tested-by: Xuelian Guo <xuelian.guo@intel.com>
> Signed-off-by: Tao Su <tao1.su@linux.intel.com>
> ---
>  target/i386/cpu.c | 16 ++++++++++++++++
>  target/i386/cpu.h |  4 ++++
>  2 files changed, 20 insertions(+)

Otherwise,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>