[PATCH 0/2] i386/cpu: Correct comments for CPUID 0x1D and 0x1E

[PATCH 0/2] i386/cpu: Correct comments for CPUID 0x1D and 0x1E

[Zhao Liu]

Hi,

CPUID 0x1D and 0x1E are marked as hardcoded for Sapphire Rapids and
there's the previous attempt [*] to make the fields in 0x1D.0x1 and
0x1E.0x0.EBX user-configurable.

In fact, the "hardcoded" values are defined for architecture, not
the SPR-specific thing, so that it's incorrect and unnecessary to make
them user-configurable.

Therefore, drop the incorrect and misleading comments.

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

Thanks and Best Regards,
Zhao
---
Zhao Liu (2):
  i386/cpu: Drop incorrect comment for CPUID 0x1D
  i386/cpu: Drop incorrect comment for CPUID 0x1E

 target/i386/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.34.1

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

[Zhao Liu]

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

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

[Zhao Liu]

The information (tmul_maxk and tmul_maxn) in CPUID 0x1E.0x0.EBX is
defined for architecture, not for SPR.

This is to say, these "hardcoded" values won't change in future. If
the TMUL component needs to be extended for new palettes, there'll
likely be the new TMUL instructions, or new types of AMX instructions
that are _parallel_ to TMUL that operate in particular palettes,
instead of changing current tmul_maxk and tmul_maxn fields in CPUID
0x1E.0x0.EBX.

Furthermore, the previous attempt [*] to make the 0x1E.0x0.EBX 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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 41d224330d05..0c954202cea8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8403,7 +8403,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     }
     case 0x1E: {
-        /* AMX TMUL, for now hardcoded for Sapphire Rapids */
+        /* AMX TMUL */
         *eax = 0;
         *ebx = 0;
         *ecx = 0;
-- 
2.34.1

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

[Xiaoyao Li]

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);

Re: [PATCH 2/2] i386/cpu: Drop incorrect comment for CPUID 0x1E

[Xiaoyao Li]

On 11/18/2025 4:08 PM, Zhao Liu wrote:
> The information (tmul_maxk and tmul_maxn) in CPUID 0x1E.0x0.EBX is
> defined for architecture, not for SPR.
> 
> This is to say, these "hardcoded" values won't change in future. If
> the TMUL component needs to be extended for new palettes, there'll
> likely be the new TMUL instructions, or new types of AMX instructions
> that are _parallel_ to TMUL that operate in particular palettes,
> instead of changing current tmul_maxk and tmul_maxn fields in CPUID
> 0x1E.0x0.EBX.
> 
> Furthermore, the previous attempt [*] to make the 0x1E.0x0.EBX 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>

The intent was that when new palette introduced in the future, AMX TMUL 
might be adjusted for better performance. But given that SDM now defines 
it clearly as constant:

   EBX[7:0]  TMUL_MAXK tmul_maxk (rows or columns). Value = 16.
   EBX[23:8] TMUL_MAXN tmul_maxn (column bytes).    Value = 64.

so, it cannot change any more.

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

> ---
>   target/i386/cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 41d224330d05..0c954202cea8 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8403,7 +8403,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>           break;
>       }
>       case 0x1E: {
> -        /* AMX TMUL, for now hardcoded for Sapphire Rapids */
> +        /* AMX TMUL */
>           *eax = 0;
>           *ebx = 0;
>           *ecx = 0;