[PATCH 1/2] i386/cpu: Drop incorrect comment for CPUID 0x1D

Zhao Liu posted 2 patches 2 days, 4 hours ago
[PATCH 1/2] i386/cpu: Drop incorrect comment for CPUID 0x1D
Posted by Zhao Liu 2 days, 4 hours ago
The information in CPUID 0x1D.0x1 is for tile palette 1, and is not
SPR-specific.

This is to say, these "hardcoded" values won't change in future. If
the palette needs to be extended, a new tile palette (maybe in a new
subleaf) will be introduced instead of changing current information of
tile palette 1.

Furthermore, the previous attempt [*] to make the 0x1D.0x1 fields
user-configurable is incorrect and unnecessary.

Therefore, drop the incorrect and misleading comment.

[*]: https://lore.kernel.org/qemu-devel/20230106083826.5384-2-lei4.wang@intel.com/

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f703b1478d71..41d224330d05 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8381,7 +8381,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *edx = 0; /* EDX is reserved. */
         break;
     case 0x1D: {
-        /* AMX TILE, for now hardcoded for Sapphire Rapids*/
+        /* AMX TILE */
         *eax = 0;
         *ebx = 0;
         *ecx = 0;
@@ -8394,6 +8394,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             /* Highest numbered palette subleaf */
             *eax = INTEL_AMX_TILE_MAX_SUBLEAF;
         } else if (count == 1) {
+            /* Tile palette 1 */
             *eax = INTEL_AMX_TOTAL_TILE_BYTES |
                    (INTEL_AMX_BYTES_PER_TILE << 16);
             *ebx = INTEL_AMX_BYTES_PER_ROW | (INTEL_AMX_TILE_MAX_NAMES << 16);
-- 
2.34.1
Re: [PATCH 1/2] i386/cpu: Drop incorrect comment for CPUID 0x1D
Posted by Xiaoyao Li 2 days, 3 hours ago
On 11/18/2025 4:08 PM, Zhao Liu wrote:
> The information in CPUID 0x1D.0x1 is for tile palette 1, and is not
> SPR-specific.
> 
> This is to say, these "hardcoded" values won't change in future. If
> the palette needs to be extended, a new tile palette (maybe in a new
> subleaf) will be introduced instead of changing current information of
> tile palette 1.
> 
> Furthermore, the previous attempt [*] to make the 0x1D.0x1 fields
> user-configurable is incorrect and unnecessary.
> 
> Therefore, drop the incorrect and misleading comment.
> 
> [*]: https://lore.kernel.org/qemu-devel/20230106083826.5384-2-lei4.wang@intel.com/
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   target/i386/cpu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f703b1478d71..41d224330d05 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8381,7 +8381,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>           *edx = 0; /* EDX is reserved. */
>           break;
>       case 0x1D: {
> -        /* AMX TILE, for now hardcoded for Sapphire Rapids*/
> +        /* AMX TILE */
>           *eax = 0;
>           *ebx = 0;
>           *ecx = 0;
> @@ -8394,6 +8394,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>               /* Highest numbered palette subleaf */
>               *eax = INTEL_AMX_TILE_MAX_SUBLEAF;
>           } else if (count == 1) {
> +            /* Tile palette 1 */

how about
	/* Tile palette 1, the values are architecturally defiend */

>               *eax = INTEL_AMX_TOTAL_TILE_BYTES |
>                      (INTEL_AMX_BYTES_PER_TILE << 16);
>               *ebx = INTEL_AMX_BYTES_PER_ROW | (INTEL_AMX_TILE_MAX_NAMES << 16);