[PATCH 00/16] i386/cpu: Unify the cache model in X86CPUState

[PATCH 00/16] i386/cpu: Unify the cache model in X86CPUState

[Zhao Liu]

Hi,

This series tries to unify the three cache models currently in
X86CPUState: cache_info_cpuid2, cache_info_cpuid4 and cache_info_amd,
into a single cache_info.

Fix, clean up, and simplify the current x86 CPU cache model support.
Especially, make the cache infomation in CPUID aligns with the vendor's
specifications.

QEMU x86 supports four vendors, and the impact of this series is as
follows:
  * AMD: No change.

  * Hygon (mostly follows AMD): No change.
    - However, I suspect that Hygon should skip the 0x2 and 0x4 leaves
      just like AMD. But since this cannot be confirmed for me, I just
      leave everything unchanged. If necessary, we can fix it. 

  * Intel:
    - Clarify the use of legacy_l2_cache_cpuid2. And for very older
      named CPUs ("486", "pentium", "pentium2" and "pentium3") that do
      not support CPUID 0x4, use the cache model like cache_info_cpuid2.
    - For other CPUs, use the cache model like cache_info_cpuid4.
    - CPUID 0x2, 0x4 and 0x80000006 use the consistent cache model.
    - CPUID 0x80000005 is marked reserved as SDM requires.

  * Zhaoxin (mostly follows Intel): mostly consistent with Intel's
    changes, except for CPUID 0x80000005, which follows AMD behavior but
    can correctly use the cache model consistent with CPUID 0x4.

Please note that one significant reason Intel requires so many fixes
(which also implies such confusion) is that Intel's named CPUs currently
do not have specific cache models and instead use the default legacy
cache models. This reflects the importance of adding cache models [1]
for named CPUs.

Philippe already has the patch [2] to remove "legacy-cache" compat
property. I initially intended to base upon his work (which could get
some simplification). However, I found that this series and [2] can be
well decoupled, making it easier to review and apply, so this series now
is based on the master branch at 6e1571533fd9.

(Next, I will detail the thought process behind the solution. You can
 skip to the end of cover letter for a concise "Patch Summary")

Thanks for your patience and feedback!


Background
==========

First of all, this the typical CPUIDs (cache related) from an Intel Guest:

CPU 0:
   ...
   0x00000002 0x00: eax=0x00000001 ebx=0x00000000 ecx=0x0000004d edx=0x002c307d

   * X86CPUState.cache_info_cpuid2:

            L1 data cache:  32K,  8-way, 64 byte lines
     L1 instruction cache:  32K,  8-way, 64 byte lines
                 L2 cache:   2M,  8-way, 64 byte lines  <--- legacy_l2_cache_cpuid2
                 L3 cache:  16M, 16-way, 64 byte lines)

   ...
   0x00000004 0x00: eax=0x00000121 ebx=0x01c0003f ecx=0x0000003f edx=0x00000001
   0x00000004 0x01: eax=0x00000122 ebx=0x01c0003f ecx=0x0000003f edx=0x00000001
   0x00000004 0x02: eax=0x00000143 ebx=0x03c0003f ecx=0x00000fff edx=0x00000001
   0x00000004 0x03: eax=0x00000163 ebx=0x03c0003f ecx=0x00003fff edx=0x00000006
   0x00000004 0x04: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000

   * X86CPUState.cache_info_cpuid4:

            L1 data cache:  32K,  8-way, 64 byte lines
     L1 instruction cache:  32K,  8-way, 64 byte lines
                 L2 cache:   4M, 16-way, 64 byte lines  <--- legacy_l2_cache_cpuid4
                 L3 cache:  16M, 16-way, 64 byte lines)

   ...
   0x80000006 0x00: eax=0x00000000 ebx=0x42004200 ecx=0x02008140 edx=0x00808140
   0x80000007 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000

   * X86CPUState.cache_info_amd:

            L1 data cache:  64K,  2-way, 64 byte lines  <--- legacy_l1d_cache_amd 
     L1 instruction cache:  64K,  2-way, 64 byte lines  <--- legacy_l1i_cache_amd
                 L2 cache: 512K, 16-way, 64 byte lines  <--- legacy_l2_cache_amd
                 L3 cache:  16M, 16-way, 64 byte lines

    Note: L1 & L3 fields should be reserved for Intel in these 2 leaves.


It's quite surprising that an Intel Guest CPU actually includes three
different cache models!

The reason, as I mentioned at the beginning, is that Intel named CPUs
lack the built-in "named" cache model and can only use the legacy cache
model. The issues above are caused by having three legacy cache models.
Of course, host/max CPUs will also have these issues.

Despite the confusion, fortunately, software that follows the SDM will
prefer CPUID 0x4. So, no related bug reports have been observed.

But this issue has already been noticed for quite some time, like the
many "FIXME" notes left by Eduardo:

/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
/*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */


Solution
========

The most challenging thing to fix this issue, is how to handle
compatibility!

Among the legacy cache models, the oldest, legacy_l2_cache_cpuid2, was
introduced during the Pentium era (2007, for more details, please refer
to the commit message of patch 4).

Moreover, after then, QEMU has continuously introduced various compat
properties, making any change likely to have widespread effects. But
eventually, I realized that the most crucial compat property is
"x-vendor-cpuid-only".

And, the entire cleanup process can be divided into two steps:


1. Merge cache_info_cpuid2 and cache_info_cpuid4
------------------------------------------------

These 2 cache models are both used for Intel, but one is used in CPUID
0x2 and another is for 0x4.

I introduced the x-consistent-cache compat property and, according to
the SDM, reworked the encoding of 0x2, marking 0x2 as unavailiable for
cache info. This way, only cache_info_cpuid4 is needed.

For the older CPUs without 0x4 ("486", "pentium", "pentium2" and
"pentium3"), I add a "named" cache model (based on cache_info_cpuid2)
and build it into the definition structures of these old CPU models.


2. Merge cache_info_cpuid4 and cache_info_cpuid_amd
---------------------------------------------------

Merging these two cache models requires consideration of the following
issues:
 
 1) The final unified cache model is based on the vendor.

 2) Compatibility with older machines is needed:
    - x-vendor-cpuid-only=false for PC v6.0 and older.
    - x-vendor-cpuid-only=true for PC v6.0 to PC v10.0 - and newer).

Therefore, I have the following table to reflect the behavior of
historical machines:

[Table 1: Cache models used in CPUID leaves for different versioned
 machines]

Diagram: C4 = cache_info_cpuid4, CA = cache_info_cpuid_amd

* Intel CPU:

           | x-vendor-cpuid-only=false |  x-vendor-cpuid-only=true  || ideal (x-vendor-cpuid-only-2=true)
           |    (PC v6.0 and older)    |    (PC v6.0 to PC v10.0)   ||          (PC v10.1 ~)
---------------------------------------------------------------------------------------------------------
       0x2 |           C4              |             C4             ||               C4
           |                           |                            ||
---------------------------------------------------------------------------------------------------------
       0x4 |           C4              |             C4             ||               C4
           |                           |                            ||
---------------------------------------------------------------------------------------------------------
0x80000005 |           CA              |             CA             ||               0 (Reserved)
           |                           |                            ||   [Note: "0" <==> "C4"]
---------------------------------------------------------------------------------------------------------
0x80000006 |           CA              |             CA             ||               C4 (eax=ebx=edx=0)
           |                           |                            ||   [Note: "0" <==> "C4"]
---------------------------------------------------------------------------------------------------------
0x8000001D |           - (Unreached)   |             - (Unreached)  ||               - (Unreached)
           |  [Note: "-" <==> "CA"]    |    [Note: "-" <==> "CA"]   ||   [Note: "0" <==> "C4"]


* AMD CPU:

           | x-vendor-cpuid-only=false |  x-vendor-cpuid-only=true  || ideal (x-vendor-cpuid-only-2=true)
           |    (PC v6.0 and older)    |    (PC v6.0 to PC v10.0)   ||         (PC v10.1 ~)
----------------------------------------------------------------------------------------------------------
       0x2 |           C4              |             0 (Reserved)   ||               CA
           |                           | [Note: "0" <==> "C4"]      ||
----------------------------------------------------------------------------------------------------------
       0x4 |           C4              |             0 (Reserved)   ||               CA
           |                           | [Note: "0" <==> "C4"]      ||
----------------------------------------------------------------------------------------------------------
0x80000005 |           CA              |             CA             ||               CA
           |                           |                            ||
----------------------------------------------------------------------------------------------------------
0x80000006 |           CA              |             CA             ||               CA
           |                           |                            ||
----------------------------------------------------------------------------------------------------------
0x8000001D |           CA              |             CA             ||               CA
           |                           |                            ||

Our final goal is to select between legacy AMD cache model and legacy
Intel cache model based on the vendor.

At first glance, this table appears very chaotic, seemingly consisting
of various unrelated cases, like a somewhat unsightly monster composed
of "different vendors", "different CPUID leaves", "different versioned
machines", as well as reserved "0" and unreached "-".

But brain teaser!
 * Reserved: If a leaf is reserved, which means whatever the cache
   models it selects, it always have all-0 registers! Thus, we can

   It's valid to consider this leaf as choosing either the Intel cache
   model or the AMD cache model, because the specific values will be
   ignored.

 * Unreached: In practice, it's similar to being reserved, although the
   spec doesn't explicitly state it as reserved. Similarly, choosing any
   cache model doesn't affect the encoding of the "Unreached" leaf.

With this consideration, (and by combining the "Note" in square brackets
within the table,) we can replace the "reserved" and "unreached" cases
with the specific cache models noted in the annotations. This reveals
the underlying pattern:


[Table 2: "Refined" cache models used in CPUID leaves for different
 versioned machines]

* Intel CPU:

           | x-vendor-cpuid-only=false |  x-vendor-cpuid-only=true  || ideal (x-vendor-cpuid-only-2=true)
           |    (PC v6.0 and older)    |    (PC v6.0 to PC v10.0)   ||          (PC v10.1 ~)
---------------------------------------------------------------------------------------------------------
       0x2 |           C4              |             C4             ||               C4
           |                           |                            ||
---------------------------------------------------------------------------------------------------------
       0x4 |           C4              |             C4             ||               C4
           |                           |                            ||
---------------------------------------------------------------------------------------------------------
0x80000005 |           CA              |             CA             ||              "C4"
           |                           |                            ||
---------------------------------------------------------------------------------------------------------
0x80000006 |           CA              |             CA             ||              "C4"
           |                           |                            ||
---------------------------------------------------------------------------------------------------------
0x8000001D |          "CA"             |            "CA"            ||              "C4"
           |                           |                            ||

* AMD CPU:

           | x-vendor-cpuid-only=false |  x-vendor-cpuid-only=true  || ideal (x-vendor-cpuid-only-2=true)
           |    (PC v6.0 and older)    |    (PC v6.0 to PC v10.0)   ||         (PC v10.1 ~)
----------------------------------------------------------------------------------------------------------
       0x2 |           C4              |            "C4"            ||               CA
           |                           |                            ||
----------------------------------------------------------------------------------------------------------
       0x4 |           C4              |            "C4"            ||               CA
           |                           |                            ||
----------------------------------------------------------------------------------------------------------
0x80000005 |           CA              |             CA             ||               CA
           |                           |                            ||
----------------------------------------------------------------------------------------------------------
0x80000006 |           CA              |             CA             ||               CA
           |                           |                            ||
----------------------------------------------------------------------------------------------------------
0x8000001D |           CA              |             CA             ||               CA
           |                           |                            ||

Based on Table 2, where the "reserved"/"unreached" fields have been
equivalently replaced, we can see that although x-vendor-cpuid-only
(since v6.1) affects the specific CPUID leaf encoding, its essence can
be regarded as not changing the underlying cache model choice
(cache_info_amd vs. cache_info_cpuid4).

Therefore, we can confidently propose this solution:

 * For v10.1 and future, select legacy cache model based Guest CPU's
   vendor.
   - Then we can merge cache_info_cpuid4 and cache_info_amd into a
     single cache_info, but just initialize cache_info based on vendor.

 * For v10.0 and older:
   - Use legacy Intel cache model (original cache_info_cpuid4) by
     default in CPUID 0x2 and 0x4 leaves.
   - Use legacy AMD cache model (original cache_info_amd) by default
     in CPUID 0x80000005, 0x80000006 and 0x8000001D.


Patch Summary
=============

Patch 01-06: Merge cache_info_cpuid2 and cache_info_cpuid4
Patch 07-16: Merge cache_info_cpuid4 and cache_info_amd

Note: patch 11-15 they each provide more specific evidence that
selecting a legacy cache model based on the Guest vendor in CPUID 0x2,
0x4, 0x80000005, 0x80000006, and 0x8000001D leaves is both valid and
safe, and doesn't break compatibility.


Reference
=========

[1]: https://lore.kernel.org/qemu-devel/20250423114702.1529340-1-zhao1.liu@intel.com/
[2]: https://lore.kernel.org/qemu-devel/20250501223522.99772-9-philmd@linaro.org/


Thanks and Best Regards,
Zhao

---
Zhao Liu (16):
  i386/cpu: Refine comment of CPUID2CacheDescriptorInfo
  i386/cpu: Add descriptor 0x49 for CPUID 0x2 encoding
  i386/cpu: Add default cache model for Intel CPUs with level < 4
  i386/cpu: Present same cache model in CPUID 0x2 & 0x4
  i386/cpu: Consolidate CPUID 0x4 leaf
  i386/cpu: Drop CPUID 0x2 specific cache info in X86CPUState
  i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
  i386/cpu: Fix CPUID[0x80000006] for Intel CPU
  i386/cpu: Add legacy_intel_cache_info cache model
  i386/cpu: Add legacy_amd_cache_info cache model
  i386/cpu: Select legacy cache model based on vendor in CPUID 0x2
  i386/cpu: Select legacy cache model based on vendor in CPUID 0x4
  i386/cpu: Select legacy cache model based on vendor in CPUID
    0x80000005
  i386/cpu: Select legacy cache model based on vendor in CPUID
    0x80000006
  i386/cpu: Select legacy cache model based on vendor in CPUID
    0x8000001D
  i386/cpu: Use a unified cache_info in X86CPUState

 hw/i386/pc.c      |   5 +-
 target/i386/cpu.c | 543 +++++++++++++++++++++++++++++-----------------
 target/i386/cpu.h |  25 ++-
 3 files changed, 372 insertions(+), 201 deletions(-)

-- 
2.34.1

[PATCH 01/16] i386/cpu: Refine comment of CPUID2CacheDescriptorInfo

[Zhao Liu]

Refer to SDM vol.3 table 1-21, add the notes about the missing
descriptor, and fix the typo and comment format.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 40aefb38f6da..e398868a3f8d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -66,6 +66,7 @@ struct CPUID2CacheDescriptorInfo {
 
 /*
  * Known CPUID 2 cache descriptors.
+ * TLB, prefetch and sectored cache related descriptors are not included.
  * From Intel SDM Volume 2A, CPUID instruction
  */
 struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
@@ -87,18 +88,29 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
                .associativity = 2,  .line_size = 64, },
     [0x21] = { .level = 2, .type = UNIFIED_CACHE,     .size = 256 * KiB,
                .associativity = 8,  .line_size = 64, },
-    /* lines per sector is not supported cpuid2_cache_descriptor(),
-    * so descriptors 0x22, 0x23 are not included
-    */
+    /*
+     * lines per sector is not supported cpuid2_cache_descriptor(),
+     * so descriptors 0x22, 0x23 are not included
+     */
     [0x24] = { .level = 2, .type = UNIFIED_CACHE,     .size =   1 * MiB,
                .associativity = 16, .line_size = 64, },
-    /* lines per sector is not supported cpuid2_cache_descriptor(),
-    * so descriptors 0x25, 0x20 are not included
-    */
+    /*
+     * lines per sector is not supported cpuid2_cache_descriptor(),
+     * so descriptors 0x25, 0x29 are not included
+     */
     [0x2C] = { .level = 1, .type = DATA_CACHE,        .size =  32 * KiB,
                .associativity = 8,  .line_size = 64, },
     [0x30] = { .level = 1, .type = INSTRUCTION_CACHE, .size =  32 * KiB,
                .associativity = 8,  .line_size = 64, },
+    /*
+     * Newer Intel CPUs (having the cores without L3, e.g., Intel MTL, ARL)
+     * use CPUID 0x4 leaf to describe cache topology, by encoding CPUID 0x2
+     * leaf with 0xFF. For older CPUs (without 0x4 leaf), it's also valid
+     * to just ignore l3's code if there's no l3.
+     *
+     * This already covers all the cases in QEMU, so code 0x40 is not
+     * included.
+     */
     [0x41] = { .level = 2, .type = UNIFIED_CACHE,     .size = 128 * KiB,
                .associativity = 4,  .line_size = 32, },
     [0x42] = { .level = 2, .type = UNIFIED_CACHE,     .size = 256 * KiB,
@@ -136,9 +148,10 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
                .associativity = 4,  .line_size = 64, },
     [0x78] = { .level = 2, .type = UNIFIED_CACHE,     .size =   1 * MiB,
                .associativity = 4,  .line_size = 64, },
-    /* lines per sector is not supported cpuid2_cache_descriptor(),
-    * so descriptors 0x79, 0x7A, 0x7B, 0x7C are not included.
-    */
+    /*
+     * lines per sector is not supported cpuid2_cache_descriptor(),
+     * so descriptors 0x79, 0x7A, 0x7B, 0x7C are not included.
+     */
     [0x7D] = { .level = 2, .type = UNIFIED_CACHE,     .size =   2 * MiB,
                .associativity = 8,  .line_size = 64, },
     [0x7F] = { .level = 2, .type = UNIFIED_CACHE,     .size = 512 * KiB,
-- 
2.34.1

[PATCH 02/16] i386/cpu: Add descriptor 0x49 for CPUID 0x2 encoding

[Zhao Liu]

The legacy_l2_cache (2nd-level cache: 4 MByte, 16-way set associative,
64 byte line size) corresponds to descriptor 0x49, but at present
cpuid2_cache_descriptors doesn't support descriptor 0x49 because it has
multiple meanings.

The 0x49 is necessary when CPUID 0x2 and 0x4 leaves have the consistent
cache model, and use legacy_l2_cache as the default l2 cache.

Therefore, add descriptor 0x49 to represent general l2 cache.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e398868a3f8d..995766c9d74c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -127,7 +127,18 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
                .associativity = 8,  .line_size = 64, },
     [0x48] = { .level = 2, .type = UNIFIED_CACHE,     .size =   3 * MiB,
                .associativity = 12, .line_size = 64, },
-    /* Descriptor 0x49 depends on CPU family/model, so it is not included */
+    /*
+     * Descriptor 0x49 has 2 cases:
+     *  - 2nd-level cache: 4 MByte, 16-way set associative, 64 byte line size.
+     *  - 3rd-level cache: 4MB, 16-way set associative, 64-byte line size
+     *    (Intel Xeon processor MP, Family 0FH, Model 06H).
+     *
+     * When it represents l3, then it depends on CPU family/model. Fortunately,
+     * the legacy cache/CPU models don't have such special l3. So, just add it
+     * to represent the general l2 case.
+     */
+    [0x49] = { .level = 2, .type = UNIFIED_CACHE,     .size =   4 * MiB,
+               .associativity = 16, .line_size = 64, },
     [0x4A] = { .level = 3, .type = UNIFIED_CACHE,     .size =   6 * MiB,
                .associativity = 12, .line_size = 64, },
     [0x4B] = { .level = 3, .type = UNIFIED_CACHE,     .size =   8 * MiB,
-- 
2.34.1

[PATCH 03/16] i386/cpu: Add default cache model for Intel CPUs with level < 4

[Zhao Liu]

Old Intel CPUs with CPUID level < 4, use CPUID 0x2 leaf (if available)
to encode cache information.

Introduce a cache model "legacy_intel_cpuid2_cache_info" for the CPUs
with CPUID level < 4, based on legacy_l1d_cache, legacy_l1i_cache,
legacy_l2_cache_cpuid2 and legacy_l3_cache. But for L2 cache, this
cache model completes self_init, sets, partitions, no_invd_sharing and
share_level fields, referring legacy_l2_cache, to avoid someone
increases CPUID level manually and meets assert() error. But the cache
information present in CPUID 0x2 leaf doesn't change.

This new cache model makes it possible to remove legacy_l2_cache_cpuid2
in X86CPUState and help to clarify historical cache inconsistency issue.

Furthermore, apply this legacy cache model to all Intel CPUs with CPUID
level < 4. This includes not only "pentium2" and "pentium3" (which have
0x2 leaf), but also "486" and "pentium" (which only have 0x1 leaf, and
cache model won't be presented, just for simplicity).

A legacy_intel_cpuid2_cache_info cache model doesn't change the cache
information of the above CPUs, because they just depend on 0x2 leaf.

Only when someone adjusts the min-level to >=4 will the cache
information in CPUID leaf 4 differ from before: previously, the L2
cache information in CPUID leaf 0x2 and 0x4 was different, but now with
legacy_intel_cpuid2_cache_info, the information they present will be
consistent. This case almost never happens, emulating a CPUID that is
not supported by the "ancient" hardware is itself meaningless behavior.

Therefore, even though there's the above difference (for really rare
case) and considering these old CPUs ("486", "pentium", "pentium2" and
"pentium3") won't be used for migration, there's no need to add new
versioned CPU models

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 995766c9d74c..0a2c32214cc3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -710,6 +710,67 @@ static CPUCacheInfo legacy_l3_cache = {
     .share_level = CPU_TOPOLOGY_LEVEL_DIE,
 };
 
+/*
+ * Only used for the CPU models with CPUID level < 4.
+ * These CPUs (CPUID level < 4) only use CPUID leaf 2 to present
+ * cache information.
+ *
+ * Note: This cache model is just a default one, and is not
+ *       guaranteed to match real hardwares.
+ */
+static const CPUCaches legacy_intel_cpuid2_cache_info = {
+    .l1d_cache = &(CPUCacheInfo) {
+        .type = DATA_CACHE,
+        .level = 1,
+        .size = 32 * KiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 8,
+        .sets = 64,
+        .partitions = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l1i_cache = &(CPUCacheInfo) {
+        .type = INSTRUCTION_CACHE,
+        .level = 1,
+        .size = 32 * KiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 8,
+        .sets = 64,
+        .partitions = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l2_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 2,
+        .size = 2 * MiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 8,
+        .sets = 4096,
+        .partitions = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l3_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 3,
+        .size = 16 * MiB,
+        .line_size = 64,
+        .associativity = 16,
+        .sets = 16384,
+        .partitions = 1,
+        .lines_per_tag = 1,
+        .self_init = true,
+        .inclusive = true,
+        .complex_indexing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
+    },
+};
+
 /* TLB definitions: */
 
 #define L1_DTLB_2M_ASSOC       1
@@ -3043,6 +3104,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
             I486_FEATURES,
         .xlevel = 0,
         .model_id = "",
+        .cache_info = &legacy_intel_cpuid2_cache_info,
     },
     {
         .name = "pentium",
@@ -3055,6 +3117,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
             PENTIUM_FEATURES,
         .xlevel = 0,
         .model_id = "",
+        .cache_info = &legacy_intel_cpuid2_cache_info,
     },
     {
         .name = "pentium2",
@@ -3067,6 +3130,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
             PENTIUM2_FEATURES,
         .xlevel = 0,
         .model_id = "",
+        .cache_info = &legacy_intel_cpuid2_cache_info,
     },
     {
         .name = "pentium3",
@@ -3079,6 +3143,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
             PENTIUM3_FEATURES,
         .xlevel = 0,
         .model_id = "",
+        .cache_info = &legacy_intel_cpuid2_cache_info,
     },
     {
         .name = "athlon",
-- 
2.34.1

[PATCH 04/16] i386/cpu: Present same cache model in CPUID 0x2 & 0x4

[Zhao Liu]

For a long time, the default cache models used in CPUID 0x2 and
0x4 were inconsistent and had a FIXME note from Eduardo at commit
5e891bf8fd50 ("target-i386: Use #defines instead of magic numbers for
CPUID cache info"):

"/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */".

This difference is wrong, in principle, both 0x2 and 0x4 are used for
Intel's cache description. 0x2 leaf is used for ancient machines while
0x4 leaf is a subsequent addition, and both should be based on the same
cache model. Furthermore, on real hardware, 0x4 leaf should be used in
preference to 0x2 when it is available.

Revisiting the git history, that difference occurred much earlier.

Current legacy_l2_cache_cpuid2 (hardcode: "0x2c307d"), which is used for
CPUID 0x2 leaf, is introduced in commit d8134d91d9b7 ("Intel cache info,
by Filip Navara."). Its commit message didn't said anything, but its
patch [1] mentioned the cache model chosen is "closest to the ones
reported in the AMD registers". Now it is not possible to check which
AMD generation this cache model is based on (unfortunately, AMD does not
use 0x2 leaf), but at least it is close to the Pentium 4.

In fact, the patch description of commit d8134d91d9b7 is also a bit
wrong, the original cache model in leaf 2 is from Pentium Pro, and its
cache descriptor had specified the cache line size ad 32 byte by default,
while the updated cache model in commit d8134d91d9b7 has 64 byte line
size. But after so many years, such judgments are no longer meaningful.

On the other hand, for legacy_l2_cache, which is used in CPUID 0x4 leaf,
is based on Intel Core Duo (patch [2]) and Core2 Duo (commit e737b32a3688
("Core 2 Duo specification (Alexander Graf).")

The patches of Core Duo and Core 2 Duo add the cache model for CPUID
0x4, but did not update CPUID 0x2 encoding. This is the reason that
Intel Guests use two cache models in 0x2 and 0x4 all the time.

Of course, while no Core Duo or Core 2 Duo machines have been found for
double checking, this still makes no sense to encode different cache
models on a single machine.

Referring to the SDM and the real hardware available, 0x2 leaf can be
directly encoded 0xFF to instruct software to go to 0x4 leaf to get the
cache information, when 0x4 is available.

Therefore, it's time to clean up Intel's default cache models. As the
first step, add "x-consistent-cache" compat option to allow newer
machines (v10.1 and newer) to have the consistent cache model in CPUID
0x2 and 0x4 leaves.

This doesn't affect the CPU models with CPUID level < 4 ("486",
"pentium", "pentium2" and "pentium3"), because they have already had the
special default cache model - legacy_intel_cpuid2_cache_info.

[1]: https://lore.kernel.org/qemu-devel/5b31733c0709081227w3e5f1036odbc649edfdc8c79b@mail.gmail.com/
[2]: https://lore.kernel.org/qemu-devel/478B65C8.2080602@csgraf.de/

Cc: Alexander Graf <agraf@csgraf.de>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/pc.c      | 4 +++-
 target/i386/cpu.c | 7 ++++++-
 target/i386/cpu.h | 7 +++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b2116335752d..ad2d6495ebde 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -81,7 +81,9 @@
     { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
     { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
 
-GlobalProperty pc_compat_10_0[] = {};
+GlobalProperty pc_compat_10_0[] = {
+    { TYPE_X86_CPU, "x-consistent-cache", "false" },
+};
 const size_t pc_compat_10_0_len = G_N_ELEMENTS(pc_compat_10_0);
 
 GlobalProperty pc_compat_9_2[] = {};
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0a2c32214cc3..2f895bf13523 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8931,7 +8931,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         /* Build legacy cache information */
         env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
         env->cache_info_cpuid2.l1i_cache = &legacy_l1i_cache;
-        env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
+        if (!cpu->consistent_cache) {
+            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
+        } else {
+            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache;
+        }
         env->cache_info_cpuid2.l3_cache = &legacy_l3_cache;
 
         env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
@@ -9457,6 +9461,7 @@ static const Property x86_cpu_properties[] = {
      * own cache information (see x86_cpu_load_def()).
      */
     DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true),
+    DEFINE_PROP_BOOL("x-consistent-cache", X86CPU, consistent_cache, true),
     DEFINE_PROP_BOOL("legacy-multi-node", X86CPU, legacy_multi_node, false),
     DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false),
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5910dcf74d42..3c7e59ffb12a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2259,6 +2259,13 @@ struct ArchCPU {
      */
     bool legacy_cache;
 
+    /*
+     * Compatibility bits for old machine types.
+     * If true, use the same cache model in CPUID leaf 0x2
+     * and 0x4.
+     */
+    bool consistent_cache;
+
     /* Compatibility bits for old machine types.
      * If true decode the CPUID Function 0x8000001E_ECX to support multiple
      * nodes per processor
-- 
2.34.1

[PATCH 05/16] i386/cpu: Consolidate CPUID 0x4 leaf

[Zhao Liu]

Modern Intel CPUs use CPUID 0x4 leaf to describe cache information
and leave space in 0x2 for prefetch and TLBs (even TLB has its own leaf
CPUID 0x18).

And 0x2 leaf provides a descriptor 0xFF to instruct software to check
cache information in 0x4 leaf instead.

Therefore, follow this behavior to encode 0xFF when Intel CPU has 0x4
leaf with "x-consistent-cache=true" for compatibility.

In addition, for older CPUs without 0x4 leaf, still enumerate the cache
descriptor in 0x2 leaf, except the case that there's no descriptor
matching the cache model, then directly encode 0xFF in 0x2 leaf. This
makes sense, as in the 0x2 leaf era, all supported caches should have
the corresponding descriptor.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2f895bf13523..a06aa1d629dc 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -223,7 +223,7 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
  * Return a CPUID 2 cache descriptor for a given cache.
  * If no known descriptor is found, return CACHE_DESCRIPTOR_UNAVAILABLE
  */
-static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache)
+static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache, bool *unmacthed)
 {
     int i;
 
@@ -240,9 +240,44 @@ static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache)
             }
     }
 
+    *unmacthed |= true;
     return CACHE_DESCRIPTOR_UNAVAILABLE;
 }
 
+/* Encode cache info for CPUID[4] */
+static void encode_cache_cpuid2(X86CPU *cpu,
+                                uint32_t *eax, uint32_t *ebx,
+                                uint32_t *ecx, uint32_t *edx)
+{
+    CPUX86State *env = &cpu->env;
+    CPUCaches *caches = &env->cache_info_cpuid2;
+    int l1d, l1i, l2, l3;
+    bool unmatched = false;
+
+    *eax = 1; /* Number of CPUID[EAX=2] calls required */
+    *ebx = *ecx = *edx = 0;
+
+    l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
+    l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
+    l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
+    l3 = cpuid2_cache_descriptor(caches->l3_cache, &unmatched);
+
+    if (!cpu->consistent_cache ||
+        (env->cpuid_min_level < 0x4 && !unmatched)) {
+        /*
+         * Though SDM defines code 0x40 for cases with no L2 or L3. It's
+         * also valid to just ignore l3's code if there's no l2.
+         */
+        if (cpu->enable_l3_cache) {
+            *ecx = l3;
+        }
+        *edx = (l1d << 16) | (l1i <<  8) | l2;
+    } else {
+        *ecx = 0;
+        *edx = CACHE_DESCRIPTOR_UNAVAILABLE;
+    }
+}
+
 /* CPUID Leaf 4 constants: */
 
 /* EAX: */
@@ -7451,16 +7486,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *eax = *ebx = *ecx = *edx = 0;
             break;
         }
-        *eax = 1; /* Number of CPUID[EAX=2] calls required */
-        *ebx = 0;
-        if (!cpu->enable_l3_cache) {
-            *ecx = 0;
-        } else {
-            *ecx = cpuid2_cache_descriptor(env->cache_info_cpuid2.l3_cache);
-        }
-        *edx = (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1d_cache) << 16) |
-               (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1i_cache) <<  8) |
-               (cpuid2_cache_descriptor(env->cache_info_cpuid2.l2_cache));
+        encode_cache_cpuid2(cpu, eax, ebx, ecx, edx);
         break;
     case 4:
         /* cache info: needed for Core compatibility */
-- 
2.34.1

[PATCH 06/16] i386/cpu: Drop CPUID 0x2 specific cache info in X86CPUState

[Zhao Liu]

With the pre-defined cache model legacy_intel_cpuid2_cache_info,
for X86CPUState there's no need to cache special cache information
for CPUID 0x2 leaf.

Drop the cache_info_cpuid2 field of X86CPUState and use the
legacy_intel_cpuid2_cache_info directly.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 31 +++++++++++--------------------
 target/i386/cpu.h |  3 ++-
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a06aa1d629dc..8f174fb971b6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -244,19 +244,27 @@ static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache, bool *unmacthed)
     return CACHE_DESCRIPTOR_UNAVAILABLE;
 }
 
+static const CPUCaches legacy_intel_cpuid2_cache_info;
+
 /* Encode cache info for CPUID[4] */
 static void encode_cache_cpuid2(X86CPU *cpu,
                                 uint32_t *eax, uint32_t *ebx,
                                 uint32_t *ecx, uint32_t *edx)
 {
     CPUX86State *env = &cpu->env;
-    CPUCaches *caches = &env->cache_info_cpuid2;
+    const CPUCaches *caches;
     int l1d, l1i, l2, l3;
     bool unmatched = false;
 
     *eax = 1; /* Number of CPUID[EAX=2] calls required */
     *ebx = *ecx = *edx = 0;
 
+    if (env->enable_legacy_cpuid2_cache) {
+        caches = &legacy_intel_cpuid2_cache_info;
+    } else {
+        caches = &env->cache_info_cpuid4;
+    }
+
     l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
     l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
     l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
@@ -705,17 +713,6 @@ static CPUCacheInfo legacy_l2_cache = {
     .share_level = CPU_TOPOLOGY_LEVEL_CORE,
 };
 
-/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
-static CPUCacheInfo legacy_l2_cache_cpuid2 = {
-    .type = UNIFIED_CACHE,
-    .level = 2,
-    .size = 2 * MiB,
-    .line_size = 64,
-    .associativity = 8,
-    .share_level = CPU_TOPOLOGY_LEVEL_INVALID,
-};
-
-
 /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
 static CPUCacheInfo legacy_l2_cache_amd = {
     .type = UNIFIED_CACHE,
@@ -8951,18 +8948,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
                        "CPU model '%s' doesn't support legacy-cache=off", name);
             return;
         }
-        env->cache_info_cpuid2 = env->cache_info_cpuid4 = env->cache_info_amd =
-            *cache_info;
+        env->cache_info_cpuid4 = env->cache_info_amd = *cache_info;
     } else {
         /* Build legacy cache information */
-        env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
-        env->cache_info_cpuid2.l1i_cache = &legacy_l1i_cache;
         if (!cpu->consistent_cache) {
-            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
-        } else {
-            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache;
+            env->enable_legacy_cpuid2_cache = true;
         }
-        env->cache_info_cpuid2.l3_cache = &legacy_l3_cache;
 
         env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
         env->cache_info_cpuid4.l1i_cache = &legacy_l1i_cache;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 3c7e59ffb12a..8d3ce8a2b678 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2076,7 +2076,8 @@ typedef struct CPUArchState {
      * on each CPUID leaf will be different, because we keep compatibility
      * with old QEMU versions.
      */
-    CPUCaches cache_info_cpuid2, cache_info_cpuid4, cache_info_amd;
+    CPUCaches cache_info_cpuid4, cache_info_amd;
+    bool enable_legacy_cpuid2_cache;
 
     /* MTRRs */
     uint64_t mtrr_fixed[11];
-- 
2.34.1

[PATCH 07/16] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel

[Zhao Liu]

Per SDM, 0x80000005 leaf is reserved for Intel CPU, and its current
"assert" check blocks adding new cache model for non-AMD CPUs.

And please note, although Zhaoxin mostly follows Intel behavior,
this leaf is an exception [1].

So, add a compat property "x-vendor-cpuid-only-v2" (for PC machine v10.0
and older) to keep the original behavior. For the machine since v10.1,
check the vendor and encode this leaf as all-0 only for Intel CPU.

This fix also resolves 2 FIXMEs of legacy_l1d_cache_amd and
legacy_l1i_cache_amd:

/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */

In addition, per AMD's APM, update the comment of CPUID[0x80000005].

[1]: https://lore.kernel.org/qemu-devel/fa16f7a8-4917-4731-9d9f-7d4c10977168@zhaoxin.com/
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * Only set all-0 for Intel CPU.
 * Add x-vendor-cpuid-only-v2.
---
 hw/i386/pc.c      |  1 +
 target/i386/cpu.c | 11 ++++++++---
 target/i386/cpu.h | 11 ++++++++++-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ad2d6495ebde..9ec3f4db31f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -83,6 +83,7 @@
 
 GlobalProperty pc_compat_10_0[] = {
     { TYPE_X86_CPU, "x-consistent-cache", "false" },
+    { TYPE_X86_CPU, "x-vendor-cpuid-only-v2", "false" },
 };
 const size_t pc_compat_10_0_len = G_N_ELEMENTS(pc_compat_10_0);
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8f174fb971b6..df40d1362566 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -655,7 +655,6 @@ static CPUCacheInfo legacy_l1d_cache = {
     .share_level = CPU_TOPOLOGY_LEVEL_CORE,
 };
 
-/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
 static CPUCacheInfo legacy_l1d_cache_amd = {
     .type = DATA_CACHE,
     .level = 1,
@@ -684,7 +683,6 @@ static CPUCacheInfo legacy_l1i_cache = {
     .share_level = CPU_TOPOLOGY_LEVEL_CORE,
 };
 
-/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
 static CPUCacheInfo legacy_l1i_cache_amd = {
     .type = INSTRUCTION_CACHE,
     .level = 1,
@@ -7889,11 +7887,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3];
         break;
     case 0x80000005:
-        /* cache info (L1 cache) */
+        /* cache info (L1 cache/TLB Associativity Field) */
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
             break;
         }
+
+        if (cpu->vendor_cpuid_only_v2 && IS_INTEL_CPU(env)) {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        }
+
         *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
                (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
         *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
@@ -9464,6 +9468,7 @@ static const Property x86_cpu_properties[] = {
     DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
     DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
     DEFINE_PROP_BOOL("x-vendor-cpuid-only", X86CPU, vendor_cpuid_only, true),
+    DEFINE_PROP_BOOL("x-vendor-cpuid-only-v2", X86CPU, vendor_cpuid_only_v2, true),
     DEFINE_PROP_BOOL("x-amd-topoext-features-only", X86CPU, amd_topoext_features_only, true),
     DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
     DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 8d3ce8a2b678..02cda176798f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2282,9 +2282,18 @@ struct ArchCPU {
     /* Enable auto level-increase for all CPUID leaves */
     bool full_cpuid_auto_level;
 
-    /* Only advertise CPUID leaves defined by the vendor */
+    /*
+     * Compatibility bits for old machine types (PC machine v6.0 and older).
+     * Only advertise CPUID leaves defined by the vendor.
+     */
     bool vendor_cpuid_only;
 
+    /*
+     * Compatibility bits for old machine types (PC machine v10.0 and older).
+     * Only advertise CPUID leaves defined by the vendor.
+     */
+    bool vendor_cpuid_only_v2;
+
     /* Only advertise TOPOEXT features that AMD defines */
     bool amd_topoext_features_only;
 
-- 
2.34.1

[PATCH 08/16] i386/cpu: Fix CPUID[0x80000006] for Intel CPU

[Zhao Liu]

Per SDM, Intel supports CPUID[0x80000006]. But only L2 information is
encoded in ECX (note that L2 associativity field encodings rules
consistent with AMD are used), all other fields are reserved.

Therefore, make the following changes to CPUID[0x80000006]:
 * Rename AMD_ENC_ASSOC to X86_ENC_ASSOC since Intel also uses the same
   rules. (While there are some slight differences between the rules in
   AMD APM v4.07 no.40332 and those in the current QEMU, generally they
   are consistent.)
 * Check the vendor in CPUID[0x80000006] and just encode L2 to ECX for
   Intel.
 * Assert L2's lines_per_tag is not 0 for AMD, and assert it is 0 for
   Intel.
 * Apply the encoding change of Intel for Zhaoxin as well [1].

This fix also resolves the FIXME of legacy_l2_cache_amd:

/*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */

In addition, per AMD's APM, update the comment of CPUID[0x80000006].

[1]: https://lore.kernel.org/qemu-devel/c522ebb5-04d5-49c6-9ad8-d755b8998988@zhaoxin.com/
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * Check vendor_cpuid_only_v2 instead of vendor_cpuid_only.
 * Move lines_per_tag assert check into encode_cache_cpuid80000006().
---
 target/i386/cpu.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index df40d1362566..0b292aa2e07b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -506,8 +506,8 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
 
 #define ASSOC_FULL 0xFF
 
-/* AMD associativity encoding used on CPUID Leaf 0x80000006: */
-#define AMD_ENC_ASSOC(a) (a <=   1 ? a   : \
+/* x86 associativity encoding used on CPUID Leaf 0x80000006: */
+#define X86_ENC_ASSOC(a) (a <=   1 ? a   : \
                           a ==   2 ? 0x2 : \
                           a ==   4 ? 0x4 : \
                           a ==   8 ? 0x6 : \
@@ -526,23 +526,26 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
  */
 static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
                                        CPUCacheInfo *l3,
-                                       uint32_t *ecx, uint32_t *edx)
+                                       uint32_t *ecx, uint32_t *edx,
+                                       bool lines_per_tag_supported)
 {
     assert(l2->size % 1024 == 0);
     assert(l2->associativity > 0);
-    assert(l2->lines_per_tag > 0);
-    assert(l2->line_size > 0);
+    assert(lines_per_tag_supported ?
+           l2->lines_per_tag > 0 : l2->lines_per_tag == 0);
     *ecx = ((l2->size / 1024) << 16) |
-           (AMD_ENC_ASSOC(l2->associativity) << 12) |
+           (X86_ENC_ASSOC(l2->associativity) << 12) |
            (l2->lines_per_tag << 8) | (l2->line_size);
 
+    /* For Intel, EDX is reserved. */
     if (l3) {
         assert(l3->size % (512 * 1024) == 0);
         assert(l3->associativity > 0);
-        assert(l3->lines_per_tag > 0);
+        assert(lines_per_tag_supported ?
+               l3->lines_per_tag > 0 : l3->lines_per_tag == 0);
         assert(l3->line_size > 0);
         *edx = ((l3->size / (512 * 1024)) << 18) |
-               (AMD_ENC_ASSOC(l3->associativity) << 12) |
+               (X86_ENC_ASSOC(l3->associativity) << 12) |
                (l3->lines_per_tag << 8) | (l3->line_size);
     } else {
         *edx = 0;
@@ -711,7 +714,6 @@ static CPUCacheInfo legacy_l2_cache = {
     .share_level = CPU_TOPOLOGY_LEVEL_CORE,
 };
 
-/*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
 static CPUCacheInfo legacy_l2_cache_amd = {
     .type = UNIFIED_CACHE,
     .level = 2,
@@ -7906,23 +7908,33 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
         break;
     case 0x80000006:
-        /* cache info (L2 cache) */
+        /* cache info (L2 cache/TLB/L3 cache) */
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
             break;
         }
-        *eax = (AMD_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) |
+
+        if (cpu->vendor_cpuid_only_v2 &&
+            (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
+            *eax = *ebx = 0;
+            encode_cache_cpuid80000006(env->cache_info_cpuid4.l2_cache,
+                                       NULL, ecx, edx, false);
+            break;
+        }
+
+        *eax = (X86_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) |
                (L2_DTLB_2M_ENTRIES << 16) |
-               (AMD_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) |
+               (X86_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) |
                (L2_ITLB_2M_ENTRIES);
-        *ebx = (AMD_ENC_ASSOC(L2_DTLB_4K_ASSOC) << 28) |
+        *ebx = (X86_ENC_ASSOC(L2_DTLB_4K_ASSOC) << 28) |
                (L2_DTLB_4K_ENTRIES << 16) |
-               (AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) |
+               (X86_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) |
                (L2_ITLB_4K_ENTRIES);
+
         encode_cache_cpuid80000006(env->cache_info_amd.l2_cache,
                                    cpu->enable_l3_cache ?
                                    env->cache_info_amd.l3_cache : NULL,
-                                   ecx, edx);
+                                   ecx, edx, true);
         break;
     case 0x80000007:
         *eax = 0;
-- 
2.34.1

[PATCH 09/16] i386/cpu: Add legacy_intel_cache_info cache model

[Zhao Liu]

Based on legacy_l1d_cache, legacy_l1i_cache, legacy_l2_cache and
legacy_l3_cache, build a complete legacy intel cache model, which can
clarify the purpose of these trivial legacy cache models, simplify the
initialization of cache info in X86CPUState, and make it easier to
handle compatibility later.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0b292aa2e07b..ec229830c532 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -643,21 +643,6 @@ static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
  * These are legacy cache values. If there is a need to change any
  * of these values please use builtin_x86_defs
  */
-
-/* L1 data cache: */
-static CPUCacheInfo legacy_l1d_cache = {
-    .type = DATA_CACHE,
-    .level = 1,
-    .size = 32 * KiB,
-    .self_init = 1,
-    .line_size = 64,
-    .associativity = 8,
-    .sets = 64,
-    .partitions = 1,
-    .no_invd_sharing = true,
-    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
-};
-
 static CPUCacheInfo legacy_l1d_cache_amd = {
     .type = DATA_CACHE,
     .level = 1,
@@ -672,20 +657,6 @@ static CPUCacheInfo legacy_l1d_cache_amd = {
     .share_level = CPU_TOPOLOGY_LEVEL_CORE,
 };
 
-/* L1 instruction cache: */
-static CPUCacheInfo legacy_l1i_cache = {
-    .type = INSTRUCTION_CACHE,
-    .level = 1,
-    .size = 32 * KiB,
-    .self_init = 1,
-    .line_size = 64,
-    .associativity = 8,
-    .sets = 64,
-    .partitions = 1,
-    .no_invd_sharing = true,
-    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
-};
-
 static CPUCacheInfo legacy_l1i_cache_amd = {
     .type = INSTRUCTION_CACHE,
     .level = 1,
@@ -700,20 +671,6 @@ static CPUCacheInfo legacy_l1i_cache_amd = {
     .share_level = CPU_TOPOLOGY_LEVEL_CORE,
 };
 
-/* Level 2 unified cache: */
-static CPUCacheInfo legacy_l2_cache = {
-    .type = UNIFIED_CACHE,
-    .level = 2,
-    .size = 4 * MiB,
-    .self_init = 1,
-    .line_size = 64,
-    .associativity = 16,
-    .sets = 4096,
-    .partitions = 1,
-    .no_invd_sharing = true,
-    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
-};
-
 static CPUCacheInfo legacy_l2_cache_amd = {
     .type = UNIFIED_CACHE,
     .level = 2,
@@ -803,6 +760,59 @@ static const CPUCaches legacy_intel_cpuid2_cache_info = {
     },
 };
 
+static const CPUCaches legacy_intel_cache_info = {
+    .l1d_cache = &(CPUCacheInfo) {
+        .type = DATA_CACHE,
+        .level = 1,
+        .size = 32 * KiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 8,
+        .sets = 64,
+        .partitions = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l1i_cache = &(CPUCacheInfo) {
+        .type = INSTRUCTION_CACHE,
+        .level = 1,
+        .size = 32 * KiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 8,
+        .sets = 64,
+        .partitions = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l2_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 2,
+        .size = 4 * MiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 16,
+        .sets = 4096,
+        .partitions = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l3_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 3,
+        .size = 16 * MiB,
+        .line_size = 64,
+        .associativity = 16,
+        .sets = 16384,
+        .partitions = 1,
+        .lines_per_tag = 1,
+        .self_init = true,
+        .inclusive = true,
+        .complex_indexing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
+    },
+};
+
 /* TLB definitions: */
 
 #define L1_DTLB_2M_ASSOC       1
@@ -8971,10 +8981,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
             env->enable_legacy_cpuid2_cache = true;
         }
 
-        env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
-        env->cache_info_cpuid4.l1i_cache = &legacy_l1i_cache;
-        env->cache_info_cpuid4.l2_cache = &legacy_l2_cache;
-        env->cache_info_cpuid4.l3_cache = &legacy_l3_cache;
+        env->cache_info_cpuid4 = legacy_intel_cache_info;
 
         env->cache_info_amd.l1d_cache = &legacy_l1d_cache_amd;
         env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd;
-- 
2.34.1

[PATCH 10/16] i386/cpu: Add legacy_amd_cache_info cache model

[Zhao Liu]

Based on legacy_l1d_cachei_amd, legacy_l1i_cache_amd, legacy_l2_cache_amd
and legacy_l3_cache, build a complete legacy AMD cache model, which can
clarify the purpose of these trivial legacy cache models, simplify the
initialization of cache info in X86CPUState, and make it easier to
handle compatibility later.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ec229830c532..bf8d7a19c88d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -643,60 +643,58 @@ static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
  * These are legacy cache values. If there is a need to change any
  * of these values please use builtin_x86_defs
  */
-static CPUCacheInfo legacy_l1d_cache_amd = {
-    .type = DATA_CACHE,
-    .level = 1,
-    .size = 64 * KiB,
-    .self_init = 1,
-    .line_size = 64,
-    .associativity = 2,
-    .sets = 512,
-    .partitions = 1,
-    .lines_per_tag = 1,
-    .no_invd_sharing = true,
-    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
-};
-
-static CPUCacheInfo legacy_l1i_cache_amd = {
-    .type = INSTRUCTION_CACHE,
-    .level = 1,
-    .size = 64 * KiB,
-    .self_init = 1,
-    .line_size = 64,
-    .associativity = 2,
-    .sets = 512,
-    .partitions = 1,
-    .lines_per_tag = 1,
-    .no_invd_sharing = true,
-    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
-};
-
-static CPUCacheInfo legacy_l2_cache_amd = {
-    .type = UNIFIED_CACHE,
-    .level = 2,
-    .size = 512 * KiB,
-    .line_size = 64,
-    .lines_per_tag = 1,
-    .associativity = 16,
-    .sets = 512,
-    .partitions = 1,
-    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
-};
-
-/* Level 3 unified cache: */
-static CPUCacheInfo legacy_l3_cache = {
-    .type = UNIFIED_CACHE,
-    .level = 3,
-    .size = 16 * MiB,
-    .line_size = 64,
-    .associativity = 16,
-    .sets = 16384,
-    .partitions = 1,
-    .lines_per_tag = 1,
-    .self_init = true,
-    .inclusive = true,
-    .complex_indexing = true,
-    .share_level = CPU_TOPOLOGY_LEVEL_DIE,
+static const CPUCaches legacy_amd_cache_info = {
+    .l1d_cache = &(CPUCacheInfo) {
+        .type = DATA_CACHE,
+        .level = 1,
+        .size = 64 * KiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 2,
+        .sets = 512,
+        .partitions = 1,
+        .lines_per_tag = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l1i_cache = &(CPUCacheInfo) {
+        .type = INSTRUCTION_CACHE,
+        .level = 1,
+        .size = 64 * KiB,
+        .self_init = 1,
+        .line_size = 64,
+        .associativity = 2,
+        .sets = 512,
+        .partitions = 1,
+        .lines_per_tag = 1,
+        .no_invd_sharing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l2_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 2,
+        .size = 512 * KiB,
+        .line_size = 64,
+        .lines_per_tag = 1,
+        .associativity = 16,
+        .sets = 512,
+        .partitions = 1,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l3_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 3,
+        .size = 16 * MiB,
+        .line_size = 64,
+        .associativity = 16,
+        .sets = 16384,
+        .partitions = 1,
+        .lines_per_tag = 1,
+        .self_init = true,
+        .inclusive = true,
+        .complex_indexing = true,
+        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
+    },
 };
 
 /*
@@ -8982,11 +8980,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         }
 
         env->cache_info_cpuid4 = legacy_intel_cache_info;
-
-        env->cache_info_amd.l1d_cache = &legacy_l1d_cache_amd;
-        env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd;
-        env->cache_info_amd.l2_cache = &legacy_l2_cache_amd;
-        env->cache_info_amd.l3_cache = &legacy_l3_cache;
+        env->cache_info_amd = legacy_amd_cache_info;
     }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.34.1

[PATCH 11/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x2

[Zhao Liu]

As preparation for merging cache_info_cpuid4 and cache_info_amd in
X86CPUState, set legacy cache model based on vendor in the CPUID 0x2
leaf. For AMD CPU, select legacy AMD cache model (in cache_info_amd) as
the default cache model, otherwise, select legacy Intel cache model (in
cache_info_cpuid4) as before.

To ensure compatibility is not broken, add an enable_legacy_vendor_cache
flag based on x-vendor-only-v2 to indicate cases where the legacy cache
model should be used regardless of the vendor. For CPUID 0x2 leaf,
enable_legacy_vendor_cache flag indicates to pick legacy Intel cache
model, which is for compatibility with the behavior of PC machine v10.0
and older.

The following explains how current vendor-based default legacy cache
model ensures correctness without breaking compatibility.

* For the PC machine v6.0 and older, vendor_cpuid_only=false, and
  vendor_cpuid_only_v2=false.

  - If the named CPU model has its own cache model, and doesn't use
    legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
    cache_info_amd are same, so 0x2 leaf uses its own cache model
    regardless of the vendor.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is true, they will use legacy Intel cache
    model just like their previous behavior.

* For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
  and vendor_cpuid_only_v2=false.

  - If the named CPU model has its own cache model (legacy_cache=false),
    then cache_info_cpuid4 & cache_info_amd both equal to its own cache
    model, so it uses its own cache model in 0x2 leaf regardless of the
    vendor. Only AMD CPUs have all-0 leaf due to vendor_cpuid_only=true,
    and this is exactly the behavior of these old machines.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is true, they will use legacy Intel cache
    model. Similarly, only AMD CPUs have all-0 leaf, and this is exactly
    the behavior of these old machines.

* For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
  vendor_cpuid_only_v2=true.

  - If the named CPU model has its own cache model (legacy_cache=false),
    then cache_info_cpuid4 & cache_info_amd both equal to its own cache
    model, so it uses its own cache model in 0x2 leaf regardless of the
    vendor. And AMD CPUs have all-0 leaf. Nothing will change.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is false, the legacy cache model is
    selected based on vendor.

    For AMD CPU, it will use legacy AMD cache but still get all-0 leaf
    due to vendor_cpuid_only=true.

    For non-AMD (Intel/Zhaoxin) CPU, it will use legacy Intel cache as
    expected.

    Here, selecting the legacy cache model based on the vendor does not
    change the previous (before the change)  behavior.

Therefore, the above analysis proves that, with the help of the flag
enable_legacy_vendor_cache, it is acceptable to select the default
legacy cache model based on the vendor.

For the CPUID 0x2 leaf, in X86CPUState, a unified cache_info is enough.
It only needs to be initialized and configured with the corresponding
legacy cache model based on the vendor.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++----------
 target/i386/cpu.h |  1 +
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index bf8d7a19c88d..524d39de9ace 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -248,23 +248,17 @@ static const CPUCaches legacy_intel_cpuid2_cache_info;
 
 /* Encode cache info for CPUID[4] */
 static void encode_cache_cpuid2(X86CPU *cpu,
+                                const CPUCaches *caches,
                                 uint32_t *eax, uint32_t *ebx,
                                 uint32_t *ecx, uint32_t *edx)
 {
     CPUX86State *env = &cpu->env;
-    const CPUCaches *caches;
     int l1d, l1i, l2, l3;
     bool unmatched = false;
 
     *eax = 1; /* Number of CPUID[EAX=2] calls required */
     *ebx = *ecx = *edx = 0;
 
-    if (env->enable_legacy_cpuid2_cache) {
-        caches = &legacy_intel_cpuid2_cache_info;
-    } else {
-        caches = &env->cache_info_cpuid4;
-    }
-
     l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
     l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
     l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
@@ -7482,8 +7476,37 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ecx &= ~CPUID_EXT_PDCM;
         }
         break;
-    case 2:
-        /* cache info: needed for Pentium Pro compatibility */
+    case 2: { /* cache info: needed for Pentium Pro compatibility */
+        const CPUCaches *caches;
+
+        if (env->enable_legacy_cpuid2_cache) {
+            caches = &legacy_intel_cpuid2_cache_info;
+        } else if (env->enable_legacy_vendor_cache) {
+            caches = &legacy_intel_cache_info;
+        } else {
+            /*
+             * FIXME: Temporarily select cache info model here based on
+             * vendor, and merge these 2 cache info models later.
+             *
+             * This condition covers the following cases (with
+             * enable_legacy_vendor_cache=false):
+             *  - When CPU model has its own cache model and doesn't use legacy
+             *    cache model (legacy_model=off). Then cache_info_amd and
+             *    cache_info_cpuid4 are the same.
+             *
+             *  - For v10.1 and newer machines, when CPU model uses legacy cache
+             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
+             *    CPU will use cache_info_amd. But this doesn't matter for AMD
+             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
+             *    model is.
+             */
+            if (IS_AMD_CPU(env)) {
+                caches = &env->cache_info_amd;
+            } else {
+                caches = &env->cache_info_cpuid4;
+            }
+        }
+
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
             break;
@@ -7491,8 +7514,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *eax = *ebx = *ecx = *edx = 0;
             break;
         }
-        encode_cache_cpuid2(cpu, eax, ebx, ecx, edx);
+        encode_cache_cpuid2(cpu, caches, eax, ebx, ecx, edx);
         break;
+    }
     case 4:
         /* cache info: needed for Core compatibility */
         if (cpu->cache_info_passthrough) {
@@ -8979,6 +9003,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
             env->enable_legacy_cpuid2_cache = true;
         }
 
+        if (!cpu->vendor_cpuid_only_v2) {
+            env->enable_legacy_vendor_cache = true;
+        }
         env->cache_info_cpuid4 = legacy_intel_cache_info;
         env->cache_info_amd = legacy_amd_cache_info;
     }
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 02cda176798f..243383efd602 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2078,6 +2078,7 @@ typedef struct CPUArchState {
      */
     CPUCaches cache_info_cpuid4, cache_info_amd;
     bool enable_legacy_cpuid2_cache;
+    bool enable_legacy_vendor_cache;
 
     /* MTRRs */
     uint64_t mtrr_fixed[11];
-- 
2.34.1

[PATCH 12/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x4

[Zhao Liu]

As preparation for merging cache_info_cpuid4 and cache_info_amd in
X86CPUState, set legacy cache model based on vendor in the CPUID 0x4
leaf. For AMD CPU, select legacy AMD cache model (in cache_info_amd) as
the default cache model, otherwise, select legacy Intel cache model (in
cache_info_cpuid4) as before.

To ensure compatibility is not broken, add an enable_legacy_vendor_cache
flag based on x-vendor-only-v2 to indicate cases where the legacy cache
model should be used regardless of the vendor. For CPUID 0x4 leaf,
enable_legacy_vendor_cache flag indicates to pick legacy Intel cache
model, which is for compatibility with the behavior of PC machine v10.0
and older.

The following explains how current vendor-based default legacy cache
model ensures correctness without breaking compatibility.

* For the PC machine v6.0 and older, vendor_cpuid_only=false, and
  vendor_cpuid_only_v2=false.

  - If the named CPU model has its own cache model, and doesn't use
    legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
    cache_info_amd are same, so 0x4 leaf uses its own cache model
    regardless of the vendor.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is true, they will use legacy Intel cache
    model just like their previous behavior.

* For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
  and vendor_cpuid_only_v2=false.

  - If the named CPU model has its own cache model (legacy_cache=false),
    then cache_info_cpuid4 & cache_info_amd both equal to its own cache
    model, so it uses its own cache model in 0x4 leaf regardless of the
    vendor. Only AMD CPUs have all-0 leaf due to vendor_cpuid_only=true,
    and this is exactly the behavior of these old machines.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is true, they will use legacy Intel cache
    model. Similarly, only AMD CPUs have all-0 leaf, and this is exactly
    the behavior of these old machines.

* For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
  vendor_cpuid_only_v2=true.

  - If the named CPU model has its own cache model (legacy_cache=false),
    then cache_info_cpuid4 & cache_info_amd both equal to its own cache
    model, so it uses its own cache model in 0x4 leaf regardless of the
    vendor. And AMD CPUs have all-0 leaf. Nothing will change.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is false, the legacy cache model is
    selected based on vendor.

    For AMD CPU, it will use legacy AMD cache but still get all-0 leaf
    due to vendor_cpuid_only=true.

    For non-AMD (Intel/Zhaoxin) CPU, it will use legacy Intel cache as
    expected.

    Here, selecting the legacy cache model based on the vendor does not
    change the previous (before the change) behavior.

Therefore, the above analysis proves that, with the help of the flag
enable_legacy_vendor_cache, it is acceptable to select the default
legacy cache model based on the vendor.

For the CPUID 0x4 leaf, in X86CPUState, a unified cache_info is enough.
It only needs to be initialized and configured with the corresponding
legacy cache model based on the vendor.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 524d39de9ace..afbf11569ab4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7517,7 +7517,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         encode_cache_cpuid2(cpu, caches, eax, ebx, ecx, edx);
         break;
     }
-    case 4:
+    case 4: {
+        const CPUCaches *caches;
+
+        if (env->enable_legacy_vendor_cache) {
+            caches = &legacy_intel_cache_info;
+        } else {
+            /*
+             * FIXME: Temporarily select cache info model here based on
+             * vendor, and merge these 2 cache info models later.
+             *
+             * This condition covers the following cases (with
+             * enable_legacy_vendor_cache=false):
+             *  - When CPU model has its own cache model and doesn't use legacy
+             *    cache model (legacy_model=off). Then cache_info_amd and
+             *    cache_info_cpuid4 are the same.
+             *
+             *  - For v10.1 and newer machines, when CPU model uses legacy cache
+             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
+             *    CPU will use cache_info_amd. But this doesn't matter for AMD
+             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
+             *    model is.
+             */
+            if (IS_AMD_CPU(env)) {
+                caches = &env->cache_info_amd;
+            } else {
+                caches = &env->cache_info_cpuid4;
+            }
+        }
+
         /* cache info: needed for Core compatibility */
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, count, eax, ebx, ecx, edx);
@@ -7545,30 +7573,26 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
 
             switch (count) {
             case 0: /* L1 dcache info */
-                encode_cache_cpuid4(env->cache_info_cpuid4.l1d_cache,
-                                    topo_info,
+                encode_cache_cpuid4(caches->l1d_cache, topo_info,
                                     eax, ebx, ecx, edx);
                 if (!cpu->l1_cache_per_core) {
                     *eax &= ~MAKE_64BIT_MASK(14, 12);
                 }
                 break;
             case 1: /* L1 icache info */
-                encode_cache_cpuid4(env->cache_info_cpuid4.l1i_cache,
-                                    topo_info,
+                encode_cache_cpuid4(caches->l1i_cache, topo_info,
                                     eax, ebx, ecx, edx);
                 if (!cpu->l1_cache_per_core) {
                     *eax &= ~MAKE_64BIT_MASK(14, 12);
                 }
                 break;
             case 2: /* L2 cache info */
-                encode_cache_cpuid4(env->cache_info_cpuid4.l2_cache,
-                                    topo_info,
+                encode_cache_cpuid4(caches->l2_cache, topo_info,
                                     eax, ebx, ecx, edx);
                 break;
             case 3: /* L3 cache info */
                 if (cpu->enable_l3_cache) {
-                    encode_cache_cpuid4(env->cache_info_cpuid4.l3_cache,
-                                        topo_info,
+                    encode_cache_cpuid4(caches->l3_cache, topo_info,
                                         eax, ebx, ecx, edx);
                     break;
                 }
@@ -7579,6 +7603,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             }
         }
         break;
+    }
     case 5:
         /* MONITOR/MWAIT Leaf */
         *eax = cpu->mwait.eax; /* Smallest monitor-line size in bytes */
-- 
2.34.1

[PATCH 13/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x80000005

[Zhao Liu]

As preparation for merging cache_info_cpuid4 and cache_info_amd in
X86CPUState, set legacy cache model based on vendor in the CPUID
0x80000005 leaf. For AMD CPU, select legacy AMD cache model (in
cache_info_amd) as the default cache model like before, otherwise,
select legacy Intel cache model (in cache_info_cpuid4).

To ensure compatibility is not broken, add an enable_legacy_vendor_cache
flag based on x-vendor-only-v2 to indicate cases where the legacy cache
model should be used regardless of the vendor. For CPUID 0x80000005
leaf, enable_legacy_vendor_cache flag indicates to pick legacy AMD cache
model, which is for compatibility with the behavior of PC machine v10.0
and older.

The following explains how current vendor-based default legacy cache
model ensures correctness without breaking compatibility.

* For the PC machine v6.0 and older, vendor_cpuid_only=false, and
  vendor_cpuid_only_v2=false.

  - If the named CPU model has its own cache model, and doesn't use
    legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
    cache_info_amd are same, so 0x80000005 leaf uses its own cache model
    regardless of the vendor.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is true, they will use legacy AMD cache
    model just like their previous behavior.

* For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
  and vendor_cpuid_only_v2=false.

  - No change, since this leaf doesn't aware vendor_cpuid_only.

* For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
  vendor_cpuid_only_v2=true.

  - If the named CPU model has its own cache model (legacy_cache=false),
    then cache_info_cpuid4 & cache_info_amd both equal to its own cache
    model, so it uses its own cache model in 0x80000005 leaf regardless
    of the vendor. Only Intel CPUs have all-0 leaf due to
    vendor_cpuid_only_2=true, and this is exactly the expected behavior.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is false, the legacy cache model is
    selected based on vendor.

    For AMD CPU, it will use legacy AMD cache as expected.

    For Intel CPU, it will use legacy Intel cache but still get all-0
    leaf due to vendor_cpuid_only_2=true as expected.

    (Note) And for Zhaoxin CPU, it will use legacy Intel cache model
    instead of AMD's. This is the difference brought by this change! But
    it's correct since then Zhaoxin could have the consistent cache info
    in CPUID 0x2, 0x4 and 0x80000005 leaves.

    Here, except Zhaoxin, selecting the legacy cache model based on the
    vendor does not change the previous (before the change) behavior.
    And the change for Zhaoxin is also a good improvement.

Therefore, the above analysis proves that, with the help of the flag
enable_legacy_vendor_cache, it is acceptable to select the default
legacy cache model based on the vendor.

For the CPUID 0x80000005 leaf, in X86CPUState, a unified cache_info is
enough. It only needs to be initialized and configured with the
corresponding legacy cache model based on the vendor.

Cc: EwanHai <ewanhai-oc@zhaoxin.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Note, side effect of this patch: fix the inconsistency cache info for
Zhaoxin. For more details, see the commit message above.
---
 target/i386/cpu.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index afbf11569ab4..16b4ecb76113 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7945,8 +7945,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ecx = env->cpuid_model[(index - 0x80000002) * 4 + 2];
         *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3];
         break;
-    case 0x80000005:
-        /* cache info (L1 cache/TLB Associativity Field) */
+    case 0x80000005: { /* cache info (L1 cache/TLB Associativity Field) */
+        const CPUCaches *caches;
+
+        if (env->enable_legacy_vendor_cache) {
+            caches = &legacy_amd_cache_info;
+        } else {
+            /*
+             * FIXME: Temporarily select cache info model here based on
+             * vendor, and merge these 2 cache info models later.
+             *
+             * This condition covers the following cases (with
+             * enable_legacy_vendor_cache=false):
+             *  - When CPU model has its own cache model and doesn't uses legacy
+             *    cache model (legacy_model=off). Then cache_info_amd and
+             *    cache_info_cpuid4 are the same.
+             *
+             *  - For v10.1 and newer machines, when CPU model uses legacy cache
+             *    model. AMD CPUs use cache_info_amd like before and non-AMD
+             *    CPU will use cache_info_cpuid4. But this doesn't matter,
+             *    because for Intel CPU, it will get all-0 leaf, and Zhaoxin CPU
+             *    will get correct cache info. Both are expected.
+             */
+            if (IS_AMD_CPU(env)) {
+                caches = &env->cache_info_amd;
+            } else {
+                caches = &env->cache_info_cpuid4;
+            }
+        }
+
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
             break;
@@ -7961,9 +7988,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
         *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
                (L1_ITLB_4K_ASSOC <<  8) | (L1_ITLB_4K_ENTRIES);
-        *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache);
-        *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
+        *ecx = encode_cache_cpuid80000005(caches->l1d_cache);
+        *edx = encode_cache_cpuid80000005(caches->l1i_cache);
         break;
+    }
     case 0x80000006:
         /* cache info (L2 cache/TLB/L3 cache) */
         if (cpu->cache_info_passthrough) {
-- 
2.34.1

[PATCH 14/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x80000006

[Zhao Liu]

As preparation for merging cache_info_cpuid4 and cache_info_amd in
X86CPUState, set legacy cache model based on vendor in the CPUID
0x80000006 leaf. For AMD CPU, select legacy AMD cache model (in
cache_info_amd) as the default cache model like before, otherwise,
select legacy Intel cache model (in cache_info_cpuid4).

To ensure compatibility is not broken, add an enable_legacy_vendor_cache
flag based on x-vendor-only-v2 to indicate cases where the legacy cache
model should be used regardless of the vendor. For CPUID 0x80000006 leaf,
enable_legacy_vendor_cache flag indicates to pick legacy Intel cache
model, which is for compatibility with the behavior of PC machine v10.0
and older.

The following explains how current vendor-based default legacy cache
model ensures correctness without breaking compatibility.

* For the PC machine v6.0 and older, vendor_cpuid_only=false, and
  vendor_cpuid_only_v2=false.

  - If the named CPU model has its own cache model, and doesn't use
    legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
    cache_info_amd are same, so 0x80000006 leaf uses its own cache model
    regardless of the vendor.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is true, they will use legacy AMD cache
    model just like their previous behavior.

* For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
  and vendor_cpuid_only_v2=false.

  - No change, since this leaf doesn't aware vendor_cpuid_only.

* For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
  vendor_cpuid_only_v2=true.

  - If the named CPU model has its own cache model (legacy_cache=false),
    then cache_info_cpuid4 & cache_info_amd both equal to its own cache
    model, so it uses its own cache model in 0x80000006 leaf regardless
    of the vendor. Intel and Zhaoxin CPUs have their special encoding
    based on SDM, which is the expected behavior and no different from
    before.

  - For max/host/named CPU (without its own cache model), then the flag
    enable_legacy_vendor_cache is false, the legacy cache model is
    selected based on vendor.

    For AMD CPU, it will use legacy AMD cache as before.

    For non-AMD (Intel/Zhaoxin) CPU, it will use legacy Intel cache and
    be encoded based on SDM as expected.

    Here, selecting the legacy cache model based on the vendor does not
    change the previous (before the change) behavior.

Therefore, the above analysis proves that, with the help of the flag
enable_legacy_vendor_cache, it is acceptable to select the default
legacy cache model based on the vendor.

For the CPUID 0x80000006 leaf, in X86CPUState, a unified cache_info is
enough. It only needs to be initialized and configured with the
corresponding legacy cache model based on the vendor.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 16b4ecb76113..4fa5907027a0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7992,8 +7992,33 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *edx = encode_cache_cpuid80000005(caches->l1i_cache);
         break;
     }
-    case 0x80000006:
-        /* cache info (L2 cache/TLB/L3 cache) */
+    case 0x80000006: { /* cache info (L2 cache/TLB/L3 cache) */
+        const CPUCaches *caches;
+
+        if (env->enable_legacy_vendor_cache) {
+            caches = &legacy_amd_cache_info;
+        } else {
+            /*
+             * FIXME: Temporarily select cache info model here based on
+             * vendor, and merge these 2 cache info models later.
+             *
+             * This condition covers the following cases (with
+             * enable_legacy_vendor_cache=false):
+             *  - When CPU model has its own cache model and doesn't uses legacy
+             *    cache model (legacy_model=off). Then cache_info_amd and
+             *    cache_info_cpuid4 are the same.
+             *
+             *  - For v10.1 and newer machines, when CPU model uses legacy cache
+             *    model. AMD CPUs use cache_info_amd like before and non-AMD
+             *    CPU (Intel & Zhaoxin) will use cache_info_cpuid4 as expected.
+             */
+            if (IS_AMD_CPU(env)) {
+                caches = &env->cache_info_amd;
+            } else {
+                caches = &env->cache_info_cpuid4;
+            }
+        }
+
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
             break;
@@ -8002,7 +8027,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         if (cpu->vendor_cpuid_only_v2 &&
             (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
             *eax = *ebx = 0;
-            encode_cache_cpuid80000006(env->cache_info_cpuid4.l2_cache,
+            encode_cache_cpuid80000006(caches->l2_cache,
                                        NULL, ecx, edx, false);
             break;
         }
@@ -8016,11 +8041,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                (X86_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) |
                (L2_ITLB_4K_ENTRIES);
 
-        encode_cache_cpuid80000006(env->cache_info_amd.l2_cache,
+        encode_cache_cpuid80000006(caches->l2_cache,
                                    cpu->enable_l3_cache ?
-                                   env->cache_info_amd.l3_cache : NULL,
+                                   caches->l3_cache : NULL,
                                    ecx, edx, true);
         break;
+    }
     case 0x80000007:
         *eax = 0;
         *ebx = env->features[FEAT_8000_0007_EBX];
-- 
2.34.1

[PATCH 15/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x8000001D

[Zhao Liu]

As preparation for merging cache_info_cpuid4 and cache_info_amd in
X86CPUState, set legacy cache model based on vendor in the CPUID
0x8000001D leaf. For AMD CPU, select legacy AMD cache model (in
cache_info_amd) as the default cache model like before, otherwise,
select legacy Intel cache model (in cache_info_cpuid4).

In fact, for Intel (and Zhaoxin) CPU, this change is safe because the
extended CPUID level supported by Intel is up to 0x80000008. So Intel
Guest doesn't have this 0x8000001D leaf.

Although someone could bump "xlevel" up to 0x8000001D for Intel Guest,
it's meaningless and this is undefined behavior. This leaf should be
considered reserved, but the SDM does not explicitly state this. So,
there's no need to specifically use vendor_cpuid_only_v2 to fix
anything, as it doesn't even qualify as a fix since nothing is
currently broken.

Therefore, it is acceptable to select the default legacy cache model
based on the vendor.

For the CPUID 0x8000001D leaf, in X86CPUState, a unified cache_info is
enough. It only needs to be initialized and configured with the
corresponding legacy cache model based on the vendor.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4fa5907027a0..4e9ac37850c0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8089,7 +8089,22 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *edx = 0;
         }
         break;
-    case 0x8000001D:
+    case 0x8000001D: {
+        const CPUCaches *caches;
+
+        /*
+         * FIXME: Temporarily select cache info model here based on
+         * vendor, and merge these 2 cache info models later.
+         *
+         * Intel doesn't support this leaf so that Intel Guests don't
+         * have this leaf. This change is harmless to Intel CPUs.
+         */
+        if (IS_AMD_CPU(env)) {
+            caches = &env->cache_info_amd;
+        } else {
+            caches = &env->cache_info_cpuid4;
+        }
+
         *eax = 0;
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, count, eax, ebx, ecx, edx);
@@ -8097,19 +8112,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         switch (count) {
         case 0: /* L1 dcache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache,
+            encode_cache_cpuid8000001d(caches->l1d_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         case 1: /* L1 icache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache,
+            encode_cache_cpuid8000001d(caches->l1i_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         case 2: /* L2 cache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache,
+            encode_cache_cpuid8000001d(caches->l2_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         case 3: /* L3 cache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache,
+            encode_cache_cpuid8000001d(caches->l3_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         default: /* end of info */
@@ -8120,6 +8135,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *edx &= CACHE_NO_INVD_SHARING | CACHE_INCLUSIVE;
         }
         break;
+    }
     case 0x8000001E:
         if (cpu->core_id <= 255) {
             encode_topo_cpuid8000001e(cpu, topo_info, eax, ebx, ecx, edx);
-- 
2.34.1

[PATCH 16/16] i386/cpu: Use a unified cache_info in X86CPUState

[Zhao Liu]

At present, all cases using the cache model (CPUID 0x2, 0x4, 0x80000005,
0x80000006 and 0x8000001D leaves) have been verified to be able to
select either cache_info_intel or cache_info_amd based on the vendor.

Therefore, further merge cache_info_intel and cache_info_amd into a
unified cache_info in X86CPUState, and during its initialization, set
different legacy cache models based on the vendor.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 150 ++++++++--------------------------------------
 target/i386/cpu.h |   5 +-
 2 files changed, 27 insertions(+), 128 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4e9ac37850c0..c1d1289ee9de 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7484,27 +7484,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         } else if (env->enable_legacy_vendor_cache) {
             caches = &legacy_intel_cache_info;
         } else {
-            /*
-             * FIXME: Temporarily select cache info model here based on
-             * vendor, and merge these 2 cache info models later.
-             *
-             * This condition covers the following cases (with
-             * enable_legacy_vendor_cache=false):
-             *  - When CPU model has its own cache model and doesn't use legacy
-             *    cache model (legacy_model=off). Then cache_info_amd and
-             *    cache_info_cpuid4 are the same.
-             *
-             *  - For v10.1 and newer machines, when CPU model uses legacy cache
-             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
-             *    CPU will use cache_info_amd. But this doesn't matter for AMD
-             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
-             *    model is.
-             */
-            if (IS_AMD_CPU(env)) {
-                caches = &env->cache_info_amd;
-            } else {
-                caches = &env->cache_info_cpuid4;
-            }
+            caches = &env->cache_info;
         }
 
         if (cpu->cache_info_passthrough) {
@@ -7523,27 +7503,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         if (env->enable_legacy_vendor_cache) {
             caches = &legacy_intel_cache_info;
         } else {
-            /*
-             * FIXME: Temporarily select cache info model here based on
-             * vendor, and merge these 2 cache info models later.
-             *
-             * This condition covers the following cases (with
-             * enable_legacy_vendor_cache=false):
-             *  - When CPU model has its own cache model and doesn't use legacy
-             *    cache model (legacy_model=off). Then cache_info_amd and
-             *    cache_info_cpuid4 are the same.
-             *
-             *  - For v10.1 and newer machines, when CPU model uses legacy cache
-             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
-             *    CPU will use cache_info_amd. But this doesn't matter for AMD
-             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
-             *    model is.
-             */
-            if (IS_AMD_CPU(env)) {
-                caches = &env->cache_info_amd;
-            } else {
-                caches = &env->cache_info_cpuid4;
-            }
+            caches = &env->cache_info;
         }
 
         /* cache info: needed for Core compatibility */
@@ -7951,27 +7911,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         if (env->enable_legacy_vendor_cache) {
             caches = &legacy_amd_cache_info;
         } else {
-            /*
-             * FIXME: Temporarily select cache info model here based on
-             * vendor, and merge these 2 cache info models later.
-             *
-             * This condition covers the following cases (with
-             * enable_legacy_vendor_cache=false):
-             *  - When CPU model has its own cache model and doesn't uses legacy
-             *    cache model (legacy_model=off). Then cache_info_amd and
-             *    cache_info_cpuid4 are the same.
-             *
-             *  - For v10.1 and newer machines, when CPU model uses legacy cache
-             *    model. AMD CPUs use cache_info_amd like before and non-AMD
-             *    CPU will use cache_info_cpuid4. But this doesn't matter,
-             *    because for Intel CPU, it will get all-0 leaf, and Zhaoxin CPU
-             *    will get correct cache info. Both are expected.
-             */
-            if (IS_AMD_CPU(env)) {
-                caches = &env->cache_info_amd;
-            } else {
-                caches = &env->cache_info_cpuid4;
-            }
+            caches = &env->cache_info;
         }
 
         if (cpu->cache_info_passthrough) {
@@ -7998,25 +7938,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         if (env->enable_legacy_vendor_cache) {
             caches = &legacy_amd_cache_info;
         } else {
-            /*
-             * FIXME: Temporarily select cache info model here based on
-             * vendor, and merge these 2 cache info models later.
-             *
-             * This condition covers the following cases (with
-             * enable_legacy_vendor_cache=false):
-             *  - When CPU model has its own cache model and doesn't uses legacy
-             *    cache model (legacy_model=off). Then cache_info_amd and
-             *    cache_info_cpuid4 are the same.
-             *
-             *  - For v10.1 and newer machines, when CPU model uses legacy cache
-             *    model. AMD CPUs use cache_info_amd like before and non-AMD
-             *    CPU (Intel & Zhaoxin) will use cache_info_cpuid4 as expected.
-             */
-            if (IS_AMD_CPU(env)) {
-                caches = &env->cache_info_amd;
-            } else {
-                caches = &env->cache_info_cpuid4;
-            }
+            caches = &env->cache_info;
         }
 
         if (cpu->cache_info_passthrough) {
@@ -8089,22 +8011,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *edx = 0;
         }
         break;
-    case 0x8000001D: {
-        const CPUCaches *caches;
-
-        /*
-         * FIXME: Temporarily select cache info model here based on
-         * vendor, and merge these 2 cache info models later.
-         *
-         * Intel doesn't support this leaf so that Intel Guests don't
-         * have this leaf. This change is harmless to Intel CPUs.
-         */
-        if (IS_AMD_CPU(env)) {
-            caches = &env->cache_info_amd;
-        } else {
-            caches = &env->cache_info_cpuid4;
-        }
-
+    case 0x8000001D:
         *eax = 0;
         if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, count, eax, ebx, ecx, edx);
@@ -8112,19 +8019,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         switch (count) {
         case 0: /* L1 dcache info */
-            encode_cache_cpuid8000001d(caches->l1d_cache,
+            encode_cache_cpuid8000001d(env->cache_info.l1d_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         case 1: /* L1 icache info */
-            encode_cache_cpuid8000001d(caches->l1i_cache,
+            encode_cache_cpuid8000001d(env->cache_info.l1i_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         case 2: /* L2 cache info */
-            encode_cache_cpuid8000001d(caches->l2_cache,
+            encode_cache_cpuid8000001d(env->cache_info.l2_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         case 3: /* L3 cache info */
-            encode_cache_cpuid8000001d(caches->l3_cache,
+            encode_cache_cpuid8000001d(env->cache_info.l3_cache,
                                        topo_info, eax, ebx, ecx, edx);
             break;
         default: /* end of info */
@@ -8135,7 +8042,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *edx &= CACHE_NO_INVD_SHARING | CACHE_INCLUSIVE;
         }
         break;
-    }
     case 0x8000001E:
         if (cpu->core_id <= 255) {
             encode_topo_cpuid8000001e(cpu, topo_info, eax, ebx, ecx, edx);
@@ -8825,46 +8731,34 @@ static bool x86_cpu_update_smp_cache_topo(MachineState *ms, X86CPU *cpu,
 
     level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1D);
     if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
-        env->cache_info_cpuid4.l1d_cache->share_level = level;
-        env->cache_info_amd.l1d_cache->share_level = level;
+        env->cache_info.l1d_cache->share_level = level;
     } else {
         machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1D,
-            env->cache_info_cpuid4.l1d_cache->share_level);
-        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1D,
-            env->cache_info_amd.l1d_cache->share_level);
+            env->cache_info.l1d_cache->share_level);
     }
 
     level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1I);
     if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
-        env->cache_info_cpuid4.l1i_cache->share_level = level;
-        env->cache_info_amd.l1i_cache->share_level = level;
+        env->cache_info.l1i_cache->share_level = level;
     } else {
         machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1I,
-            env->cache_info_cpuid4.l1i_cache->share_level);
-        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1I,
-            env->cache_info_amd.l1i_cache->share_level);
+            env->cache_info.l1i_cache->share_level);
     }
 
     level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L2);
     if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
-        env->cache_info_cpuid4.l2_cache->share_level = level;
-        env->cache_info_amd.l2_cache->share_level = level;
+        env->cache_info.l2_cache->share_level = level;
     } else {
         machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L2,
-            env->cache_info_cpuid4.l2_cache->share_level);
-        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L2,
-            env->cache_info_amd.l2_cache->share_level);
+            env->cache_info.l2_cache->share_level);
     }
 
     level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L3);
     if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
-        env->cache_info_cpuid4.l3_cache->share_level = level;
-        env->cache_info_amd.l3_cache->share_level = level;
+        env->cache_info.l3_cache->share_level = level;
     } else {
         machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L3,
-            env->cache_info_cpuid4.l3_cache->share_level);
-        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L3,
-            env->cache_info_amd.l3_cache->share_level);
+            env->cache_info.l3_cache->share_level);
     }
 
     if (!machine_check_smp_cache(ms, errp)) {
@@ -9091,7 +8985,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
                        "CPU model '%s' doesn't support legacy-cache=off", name);
             return;
         }
-        env->cache_info_cpuid4 = env->cache_info_amd = *cache_info;
+        env->cache_info = *cache_info;
     } else {
         /* Build legacy cache information */
         if (!cpu->consistent_cache) {
@@ -9101,8 +8995,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         if (!cpu->vendor_cpuid_only_v2) {
             env->enable_legacy_vendor_cache = true;
         }
-        env->cache_info_cpuid4 = legacy_intel_cache_info;
-        env->cache_info_amd = legacy_amd_cache_info;
+
+        if (IS_AMD_CPU(env)) {
+            env->cache_info = legacy_amd_cache_info;
+        } else {
+            env->cache_info = legacy_intel_cache_info;
+        }
     }
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 243383efd602..3f79db679888 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2072,11 +2072,12 @@ typedef struct CPUArchState {
     /* Features that were explicitly enabled/disabled */
     FeatureWordArray user_features;
     uint32_t cpuid_model[12];
-    /* Cache information for CPUID.  When legacy-cache=on, the cache data
+    /*
+     * Cache information for CPUID.  When legacy-cache=on, the cache data
      * on each CPUID leaf will be different, because we keep compatibility
      * with old QEMU versions.
      */
-    CPUCaches cache_info_cpuid4, cache_info_amd;
+    CPUCaches cache_info;
     bool enable_legacy_cpuid2_cache;
     bool enable_legacy_vendor_cache;
 
-- 
2.34.1

Re: [PATCH 05/16] i386/cpu: Consolidate CPUID 0x4 leaf

[Ewan Hai]

On 6/20/25 5:27 PM, Zhao Liu wrote:
> 
> 
> Modern Intel CPUs use CPUID 0x4 leaf to describe cache information
> and leave space in 0x2 for prefetch and TLBs (even TLB has its own leaf
> CPUID 0x18).
> 
> And 0x2 leaf provides a descriptor 0xFF to instruct software to check
> cache information in 0x4 leaf instead.
> 
> Therefore, follow this behavior to encode 0xFF when Intel CPU has 0x4
> leaf with "x-consistent-cache=true" for compatibility.
> 
> In addition, for older CPUs without 0x4 leaf, still enumerate the cache
> descriptor in 0x2 leaf, except the case that there's no descriptor
> matching the cache model, then directly encode 0xFF in 0x2 leaf. This
> makes sense, as in the 0x2 leaf era, all supported caches should have
> the corresponding descriptor.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/i386/cpu.c | 48 ++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 2f895bf13523..a06aa1d629dc 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -223,7 +223,7 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
>    * Return a CPUID 2 cache descriptor for a given cache.
>    * If no known descriptor is found, return CACHE_DESCRIPTOR_UNAVAILABLE
>    */
> -static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache)
> +static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache, bool *unmacthed)
>   {
>       int i;
> 
> @@ -240,9 +240,44 @@ static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache)
>               }
>       }
> 
> +    *unmacthed |= true;
>       return CACHE_DESCRIPTOR_UNAVAILABLE;
>   }
> 
> +/* Encode cache info for CPUID[4] */

Maybe this should be /* Encode cache info for CPUID[2] */ ?
I'm not sure.

> +static void encode_cache_cpuid2(X86CPU *cpu,
> +                                uint32_t *eax, uint32_t *ebx,
> +                                uint32_t *ecx, uint32_t *edx)
> +{
> +    CPUX86State *env = &cpu->env;
> +    CPUCaches *caches = &env->cache_info_cpuid2;
> +    int l1d, l1i, l2, l3;
> +    bool unmatched = false;
> +
> +    *eax = 1; /* Number of CPUID[EAX=2] calls required */
> +    *ebx = *ecx = *edx = 0;
> +
> +    l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
> +    l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
> +    l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
> +    l3 = cpuid2_cache_descriptor(caches->l3_cache, &unmatched);
> +
> +    if (!cpu->consistent_cache ||
> +        (env->cpuid_min_level < 0x4 && !unmatched)) {
> +        /*
> +         * Though SDM defines code 0x40 for cases with no L2 or L3. It's
> +         * also valid to just ignore l3's code if there's no l2.
> +         */
> +        if (cpu->enable_l3_cache) {
> +            *ecx = l3;
> +        }
> +        *edx = (l1d << 16) | (l1i <<  8) | l2;
> +    } else {
> +        *ecx = 0;
> +        *edx = CACHE_DESCRIPTOR_UNAVAILABLE;
> +    }
> +}
> +
>   /* CPUID Leaf 4 constants: */
> 
>   /* EAX: */
> @@ -7451,16 +7486,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>               *eax = *ebx = *ecx = *edx = 0;
>               break;
>           }
> -        *eax = 1; /* Number of CPUID[EAX=2] calls required */
> -        *ebx = 0;
> -        if (!cpu->enable_l3_cache) {
> -            *ecx = 0;
> -        } else {
> -            *ecx = cpuid2_cache_descriptor(env->cache_info_cpuid2.l3_cache);
> -        }
> -        *edx = (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1d_cache) << 16) |
> -               (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1i_cache) <<  8) |
> -               (cpuid2_cache_descriptor(env->cache_info_cpuid2.l2_cache));
> +        encode_cache_cpuid2(cpu, eax, ebx, ecx, edx);
>           break;
>       case 4:
>           /* cache info: needed for Core compatibility */
> --
> 2.34.1
> 

Re: [PATCH 05/16] i386/cpu: Consolidate CPUID 0x4 leaf

[Zhao Liu]

> > +/* Encode cache info for CPUID[4] */
> 
> Maybe this should be /* Encode cache info for CPUID[2] */ ?
> I'm not sure.

Yep, you're right! The following function is used to encode CPUID[2]
as its name indicates.

> > +static void encode_cache_cpuid2(X86CPU *cpu,
> > +                                uint32_t *eax, uint32_t *ebx,
> > +                                uint32_t *ecx, uint32_t *edx)
> > +{

Thanks,
Zhao

Re: [PATCH 01/16] i386/cpu: Refine comment of CPUID2CacheDescriptorInfo

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Refer to SDM vol.3 table 1-21, add the notes about the missing
> descriptor, and fix the typo and comment format.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 40aefb38f6da..e398868a3f8d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -66,6 +66,7 @@ struct CPUID2CacheDescriptorInfo {
>  
>  /*
>   * Known CPUID 2 cache descriptors.
> + * TLB, prefetch and sectored cache related descriptors are not included.
>   * From Intel SDM Volume 2A, CPUID instruction
>   */
>  struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
> @@ -87,18 +88,29 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
>                 .associativity = 2,  .line_size = 64, },
>      [0x21] = { .level = 2, .type = UNIFIED_CACHE,     .size = 256 * KiB,
>                 .associativity = 8,  .line_size = 64, },
> -    /* lines per sector is not supported cpuid2_cache_descriptor(),
> -    * so descriptors 0x22, 0x23 are not included
> -    */
> +    /*
> +     * lines per sector is not supported cpuid2_cache_descriptor(),
> +     * so descriptors 0x22, 0x23 are not included
> +     */
>      [0x24] = { .level = 2, .type = UNIFIED_CACHE,     .size =   1 * MiB,
>                 .associativity = 16, .line_size = 64, },
> -    /* lines per sector is not supported cpuid2_cache_descriptor(),
> -    * so descriptors 0x25, 0x20 are not included
> -    */
> +    /*
> +     * lines per sector is not supported cpuid2_cache_descriptor(),
> +     * so descriptors 0x25, 0x29 are not included
> +     */
>      [0x2C] = { .level = 1, .type = DATA_CACHE,        .size =  32 * KiB,
>                 .associativity = 8,  .line_size = 64, },
>      [0x30] = { .level = 1, .type = INSTRUCTION_CACHE, .size =  32 * KiB,
>                 .associativity = 8,  .line_size = 64, },
> +    /*
> +     * Newer Intel CPUs (having the cores without L3, e.g., Intel MTL, ARL)
> +     * use CPUID 0x4 leaf to describe cache topology, by encoding CPUID 0x2
> +     * leaf with 0xFF. For older CPUs (without 0x4 leaf), it's also valid
> +     * to just ignore l3's code if there's no l3.

s/l3/L3/g

Others look good to me. 

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>


> +     *
> +     * This already covers all the cases in QEMU, so code 0x40 is not
> +     * included.
> +     */
>      [0x41] = { .level = 2, .type = UNIFIED_CACHE,     .size = 128 * KiB,
>                 .associativity = 4,  .line_size = 32, },
>      [0x42] = { .level = 2, .type = UNIFIED_CACHE,     .size = 256 * KiB,
> @@ -136,9 +148,10 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
>                 .associativity = 4,  .line_size = 64, },
>      [0x78] = { .level = 2, .type = UNIFIED_CACHE,     .size =   1 * MiB,
>                 .associativity = 4,  .line_size = 64, },
> -    /* lines per sector is not supported cpuid2_cache_descriptor(),
> -    * so descriptors 0x79, 0x7A, 0x7B, 0x7C are not included.
> -    */
> +    /*
> +     * lines per sector is not supported cpuid2_cache_descriptor(),
> +     * so descriptors 0x79, 0x7A, 0x7B, 0x7C are not included.
> +     */
>      [0x7D] = { .level = 2, .type = UNIFIED_CACHE,     .size =   2 * MiB,
>                 .associativity = 8,  .line_size = 64, },
>      [0x7F] = { .level = 2, .type = UNIFIED_CACHE,     .size = 512 * KiB,

Re: [PATCH 02/16] i386/cpu: Add descriptor 0x49 for CPUID 0x2 encoding

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> The legacy_l2_cache (2nd-level cache: 4 MByte, 16-way set associative,
> 64 byte line size) corresponds to descriptor 0x49, but at present
> cpuid2_cache_descriptors doesn't support descriptor 0x49 because it has
> multiple meanings.
>
> The 0x49 is necessary when CPUID 0x2 and 0x4 leaves have the consistent
> cache model, and use legacy_l2_cache as the default l2 cache.
>
> Therefore, add descriptor 0x49 to represent general l2 cache.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index e398868a3f8d..995766c9d74c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -127,7 +127,18 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
>                 .associativity = 8,  .line_size = 64, },
>      [0x48] = { .level = 2, .type = UNIFIED_CACHE,     .size =   3 * MiB,
>                 .associativity = 12, .line_size = 64, },
> -    /* Descriptor 0x49 depends on CPU family/model, so it is not included */
> +    /*
> +     * Descriptor 0x49 has 2 cases:
> +     *  - 2nd-level cache: 4 MByte, 16-way set associative, 64 byte line size.
> +     *  - 3rd-level cache: 4MB, 16-way set associative, 64-byte line size
> +     *    (Intel Xeon processor MP, Family 0FH, Model 06H).
> +     *
> +     * When it represents l3, then it depends on CPU family/model. Fortunately,
> +     * the legacy cache/CPU models don't have such special l3. So, just add it
> +     * to represent the general l2 case.

For comments and commit message, we'd better use the capital character
"L2/L3" to represent the 2nd/3rd level cache which is more conventional. 

Others look good to me.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>


> +     */
> +    [0x49] = { .level = 2, .type = UNIFIED_CACHE,     .size =   4 * MiB,
> +               .associativity = 16, .line_size = 64, },
>      [0x4A] = { .level = 3, .type = UNIFIED_CACHE,     .size =   6 * MiB,
>                 .associativity = 12, .line_size = 64, },
>      [0x4B] = { .level = 3, .type = UNIFIED_CACHE,     .size =   8 * MiB,

Re: [PATCH 03/16] i386/cpu: Add default cache model for Intel CPUs with level < 4

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Old Intel CPUs with CPUID level < 4, use CPUID 0x2 leaf (if available)
> to encode cache information.
>
> Introduce a cache model "legacy_intel_cpuid2_cache_info" for the CPUs
> with CPUID level < 4, based on legacy_l1d_cache, legacy_l1i_cache,
> legacy_l2_cache_cpuid2 and legacy_l3_cache. But for L2 cache, this
> cache model completes self_init, sets, partitions, no_invd_sharing and
> share_level fields, referring legacy_l2_cache, to avoid someone
> increases CPUID level manually and meets assert() error. But the cache
> information present in CPUID 0x2 leaf doesn't change.
>
> This new cache model makes it possible to remove legacy_l2_cache_cpuid2
> in X86CPUState and help to clarify historical cache inconsistency issue.
>
> Furthermore, apply this legacy cache model to all Intel CPUs with CPUID
> level < 4. This includes not only "pentium2" and "pentium3" (which have
> 0x2 leaf), but also "486" and "pentium" (which only have 0x1 leaf, and
> cache model won't be presented, just for simplicity).
>
> A legacy_intel_cpuid2_cache_info cache model doesn't change the cache
> information of the above CPUs, because they just depend on 0x2 leaf.
>
> Only when someone adjusts the min-level to >=4 will the cache
> information in CPUID leaf 4 differ from before: previously, the L2
> cache information in CPUID leaf 0x2 and 0x4 was different, but now with
> legacy_intel_cpuid2_cache_info, the information they present will be
> consistent. This case almost never happens, emulating a CPUID that is
> not supported by the "ancient" hardware is itself meaningless behavior.
>
> Therefore, even though there's the above difference (for really rare
> case) and considering these old CPUs ("486", "pentium", "pentium2" and
> "pentium3") won't be used for migration, there's no need to add new
> versioned CPU models
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 995766c9d74c..0a2c32214cc3 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -710,6 +710,67 @@ static CPUCacheInfo legacy_l3_cache = {
>      .share_level = CPU_TOPOLOGY_LEVEL_DIE,
>  };
>  
> +/*
> + * Only used for the CPU models with CPUID level < 4.
> + * These CPUs (CPUID level < 4) only use CPUID leaf 2 to present
> + * cache information.
> + *
> + * Note: This cache model is just a default one, and is not
> + *       guaranteed to match real hardwares.
> + */
> +static const CPUCaches legacy_intel_cpuid2_cache_info = {
> +    .l1d_cache = &(CPUCacheInfo) {
> +        .type = DATA_CACHE,
> +        .level = 1,
> +        .size = 32 * KiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .sets = 64,
> +        .partitions = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l1i_cache = &(CPUCacheInfo) {
> +        .type = INSTRUCTION_CACHE,
> +        .level = 1,
> +        .size = 32 * KiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .sets = 64,
> +        .partitions = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l2_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 2,
> +        .size = 2 * MiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .sets = 4096,
> +        .partitions = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l3_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 3,
> +        .size = 16 * MiB,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .sets = 16384,
> +        .partitions = 1,
> +        .lines_per_tag = 1,
> +        .self_init = true,
> +        .inclusive = true,
> +        .complex_indexing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
> +    },

Does this cache information match the real legacy HW or just an emulation
of Qemu?


> +};
> +
>  /* TLB definitions: */
>  
>  #define L1_DTLB_2M_ASSOC       1
> @@ -3043,6 +3104,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>              I486_FEATURES,
>          .xlevel = 0,
>          .model_id = "",
> +        .cache_info = &legacy_intel_cpuid2_cache_info,
>      },
>      {
>          .name = "pentium",
> @@ -3055,6 +3117,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>              PENTIUM_FEATURES,
>          .xlevel = 0,
>          .model_id = "",
> +        .cache_info = &legacy_intel_cpuid2_cache_info,
>      },
>      {
>          .name = "pentium2",
> @@ -3067,6 +3130,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>              PENTIUM2_FEATURES,
>          .xlevel = 0,
>          .model_id = "",
> +        .cache_info = &legacy_intel_cpuid2_cache_info,
>      },
>      {
>          .name = "pentium3",
> @@ -3079,6 +3143,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>              PENTIUM3_FEATURES,
>          .xlevel = 0,
>          .model_id = "",
> +        .cache_info = &legacy_intel_cpuid2_cache_info,
>      },
>      {
>          .name = "athlon",

Re: [PATCH 04/16] i386/cpu: Present same cache model in CPUID 0x2 & 0x4

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> For a long time, the default cache models used in CPUID 0x2 and
> 0x4 were inconsistent and had a FIXME note from Eduardo at commit
> 5e891bf8fd50 ("target-i386: Use #defines instead of magic numbers for
> CPUID cache info"):
>
> "/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */".
>
> This difference is wrong, in principle, both 0x2 and 0x4 are used for
> Intel's cache description. 0x2 leaf is used for ancient machines while
> 0x4 leaf is a subsequent addition, and both should be based on the same
> cache model. Furthermore, on real hardware, 0x4 leaf should be used in
> preference to 0x2 when it is available.
>
> Revisiting the git history, that difference occurred much earlier.
>
> Current legacy_l2_cache_cpuid2 (hardcode: "0x2c307d"), which is used for
> CPUID 0x2 leaf, is introduced in commit d8134d91d9b7 ("Intel cache info,
> by Filip Navara."). Its commit message didn't said anything, but its
> patch [1] mentioned the cache model chosen is "closest to the ones
> reported in the AMD registers". Now it is not possible to check which
> AMD generation this cache model is based on (unfortunately, AMD does not
> use 0x2 leaf), but at least it is close to the Pentium 4.
>
> In fact, the patch description of commit d8134d91d9b7 is also a bit
> wrong, the original cache model in leaf 2 is from Pentium Pro, and its
> cache descriptor had specified the cache line size ad 32 byte by default,
> while the updated cache model in commit d8134d91d9b7 has 64 byte line
> size. But after so many years, such judgments are no longer meaningful.
>
> On the other hand, for legacy_l2_cache, which is used in CPUID 0x4 leaf,
> is based on Intel Core Duo (patch [2]) and Core2 Duo (commit e737b32a3688
> ("Core 2 Duo specification (Alexander Graf).")
>
> The patches of Core Duo and Core 2 Duo add the cache model for CPUID
> 0x4, but did not update CPUID 0x2 encoding. This is the reason that
> Intel Guests use two cache models in 0x2 and 0x4 all the time.
>
> Of course, while no Core Duo or Core 2 Duo machines have been found for
> double checking, this still makes no sense to encode different cache
> models on a single machine.
>
> Referring to the SDM and the real hardware available, 0x2 leaf can be
> directly encoded 0xFF to instruct software to go to 0x4 leaf to get the
> cache information, when 0x4 is available.
>
> Therefore, it's time to clean up Intel's default cache models. As the
> first step, add "x-consistent-cache" compat option to allow newer
> machines (v10.1 and newer) to have the consistent cache model in CPUID
> 0x2 and 0x4 leaves.
>
> This doesn't affect the CPU models with CPUID level < 4 ("486",
> "pentium", "pentium2" and "pentium3"), because they have already had the
> special default cache model - legacy_intel_cpuid2_cache_info.
>
> [1]: https://lore.kernel.org/qemu-devel/5b31733c0709081227w3e5f1036odbc649edfdc8c79b@mail.gmail.com/
> [2]: https://lore.kernel.org/qemu-devel/478B65C8.2080602@csgraf.de/
>
> Cc: Alexander Graf <agraf@csgraf.de>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  hw/i386/pc.c      | 4 +++-
>  target/i386/cpu.c | 7 ++++++-
>  target/i386/cpu.h | 7 +++++++
>  3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b2116335752d..ad2d6495ebde 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -81,7 +81,9 @@
>      { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
>      { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
>  
> -GlobalProperty pc_compat_10_0[] = {};
> +GlobalProperty pc_compat_10_0[] = {
> +    { TYPE_X86_CPU, "x-consistent-cache", "false" },
> +};
>  const size_t pc_compat_10_0_len = G_N_ELEMENTS(pc_compat_10_0);
>  
>  GlobalProperty pc_compat_9_2[] = {};
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 0a2c32214cc3..2f895bf13523 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8931,7 +8931,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          /* Build legacy cache information */
>          env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
>          env->cache_info_cpuid2.l1i_cache = &legacy_l1i_cache;
> -        env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
> +        if (!cpu->consistent_cache) {
> +            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
> +        } else {
> +            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache;
> +        }

This would encode the valid L1 and L3 cache descriptors and "0xff"
descriptor into CPUID leaf 0x2 when there is CPUID leaf 0x4. It seems a
little bit of ambiguous to mix "0xff" descriptor with other valid
descriptors and it isn't identical with real HW. Do we consider to make it
identical with real HW? Thanks.


>          env->cache_info_cpuid2.l3_cache = &legacy_l3_cache;
>  
>          env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
> @@ -9457,6 +9461,7 @@ static const Property x86_cpu_properties[] = {
>       * own cache information (see x86_cpu_load_def()).
>       */
>      DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true),
> +    DEFINE_PROP_BOOL("x-consistent-cache", X86CPU, consistent_cache, true),
>      DEFINE_PROP_BOOL("legacy-multi-node", X86CPU, legacy_multi_node, false),
>      DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false),
>  
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 5910dcf74d42..3c7e59ffb12a 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2259,6 +2259,13 @@ struct ArchCPU {
>       */
>      bool legacy_cache;
>  
> +    /*
> +     * Compatibility bits for old machine types.
> +     * If true, use the same cache model in CPUID leaf 0x2
> +     * and 0x4.
> +     */
> +    bool consistent_cache;
> +
>      /* Compatibility bits for old machine types.
>       * If true decode the CPUID Function 0x8000001E_ECX to support multiple
>       * nodes per processor

Re: [PATCH 04/16] i386/cpu: Present same cache model in CPUID 0x2 & 0x4

[Mi, Dapeng]

On 7/3/2025 12:14 PM, Mi, Dapeng wrote:
> On 6/20/2025 5:27 PM, Zhao Liu wrote:
>> For a long time, the default cache models used in CPUID 0x2 and
>> 0x4 were inconsistent and had a FIXME note from Eduardo at commit
>> 5e891bf8fd50 ("target-i386: Use #defines instead of magic numbers for
>> CPUID cache info"):
>>
>> "/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */".
>>
>> This difference is wrong, in principle, both 0x2 and 0x4 are used for
>> Intel's cache description. 0x2 leaf is used for ancient machines while
>> 0x4 leaf is a subsequent addition, and both should be based on the same
>> cache model. Furthermore, on real hardware, 0x4 leaf should be used in
>> preference to 0x2 when it is available.
>>
>> Revisiting the git history, that difference occurred much earlier.
>>
>> Current legacy_l2_cache_cpuid2 (hardcode: "0x2c307d"), which is used for
>> CPUID 0x2 leaf, is introduced in commit d8134d91d9b7 ("Intel cache info,
>> by Filip Navara."). Its commit message didn't said anything, but its
>> patch [1] mentioned the cache model chosen is "closest to the ones
>> reported in the AMD registers". Now it is not possible to check which
>> AMD generation this cache model is based on (unfortunately, AMD does not
>> use 0x2 leaf), but at least it is close to the Pentium 4.
>>
>> In fact, the patch description of commit d8134d91d9b7 is also a bit
>> wrong, the original cache model in leaf 2 is from Pentium Pro, and its
>> cache descriptor had specified the cache line size ad 32 byte by default,
>> while the updated cache model in commit d8134d91d9b7 has 64 byte line
>> size. But after so many years, such judgments are no longer meaningful.
>>
>> On the other hand, for legacy_l2_cache, which is used in CPUID 0x4 leaf,
>> is based on Intel Core Duo (patch [2]) and Core2 Duo (commit e737b32a3688
>> ("Core 2 Duo specification (Alexander Graf).")
>>
>> The patches of Core Duo and Core 2 Duo add the cache model for CPUID
>> 0x4, but did not update CPUID 0x2 encoding. This is the reason that
>> Intel Guests use two cache models in 0x2 and 0x4 all the time.
>>
>> Of course, while no Core Duo or Core 2 Duo machines have been found for
>> double checking, this still makes no sense to encode different cache
>> models on a single machine.
>>
>> Referring to the SDM and the real hardware available, 0x2 leaf can be
>> directly encoded 0xFF to instruct software to go to 0x4 leaf to get the
>> cache information, when 0x4 is available.
>>
>> Therefore, it's time to clean up Intel's default cache models. As the
>> first step, add "x-consistent-cache" compat option to allow newer
>> machines (v10.1 and newer) to have the consistent cache model in CPUID
>> 0x2 and 0x4 leaves.
>>
>> This doesn't affect the CPU models with CPUID level < 4 ("486",
>> "pentium", "pentium2" and "pentium3"), because they have already had the
>> special default cache model - legacy_intel_cpuid2_cache_info.
>>
>> [1]: https://lore.kernel.org/qemu-devel/5b31733c0709081227w3e5f1036odbc649edfdc8c79b@mail.gmail.com/
>> [2]: https://lore.kernel.org/qemu-devel/478B65C8.2080602@csgraf.de/
>>
>> Cc: Alexander Graf <agraf@csgraf.de>
>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
>> ---
>>  hw/i386/pc.c      | 4 +++-
>>  target/i386/cpu.c | 7 ++++++-
>>  target/i386/cpu.h | 7 +++++++
>>  3 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index b2116335752d..ad2d6495ebde 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -81,7 +81,9 @@
>>      { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
>>      { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
>>  
>> -GlobalProperty pc_compat_10_0[] = {};
>> +GlobalProperty pc_compat_10_0[] = {
>> +    { TYPE_X86_CPU, "x-consistent-cache", "false" },
>> +};
>>  const size_t pc_compat_10_0_len = G_N_ELEMENTS(pc_compat_10_0);
>>  
>>  GlobalProperty pc_compat_9_2[] = {};
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 0a2c32214cc3..2f895bf13523 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -8931,7 +8931,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>>          /* Build legacy cache information */
>>          env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
>>          env->cache_info_cpuid2.l1i_cache = &legacy_l1i_cache;
>> -        env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
>> +        if (!cpu->consistent_cache) {
>> +            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
>> +        } else {
>> +            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache;
>> +        }
> This would encode the valid L1 and L3 cache descriptors and "0xff"
> descriptor into CPUID leaf 0x2 when there is CPUID leaf 0x4. It seems a
> little bit of ambiguous to mix "0xff" descriptor with other valid
> descriptors and it isn't identical with real HW. Do we consider to make it
> identical with real HW? Thanks.

Just found the subsequent patch 05/16 has addressed this concern. Please
ignore this comment.


>
>
>>          env->cache_info_cpuid2.l3_cache = &legacy_l3_cache;
>>  
>>          env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
>> @@ -9457,6 +9461,7 @@ static const Property x86_cpu_properties[] = {
>>       * own cache information (see x86_cpu_load_def()).
>>       */
>>      DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true),
>> +    DEFINE_PROP_BOOL("x-consistent-cache", X86CPU, consistent_cache, true),
>>      DEFINE_PROP_BOOL("legacy-multi-node", X86CPU, legacy_multi_node, false),
>>      DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false),
>>  
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index 5910dcf74d42..3c7e59ffb12a 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -2259,6 +2259,13 @@ struct ArchCPU {
>>       */
>>      bool legacy_cache;
>>  
>> +    /*
>> +     * Compatibility bits for old machine types.
>> +     * If true, use the same cache model in CPUID leaf 0x2
>> +     * and 0x4.
>> +     */
>> +    bool consistent_cache;
>> +
>>      /* Compatibility bits for old machine types.
>>       * If true decode the CPUID Function 0x8000001E_ECX to support multiple
>>       * nodes per processor

Re: [PATCH 05/16] i386/cpu: Consolidate CPUID 0x4 leaf

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Modern Intel CPUs use CPUID 0x4 leaf to describe cache information
> and leave space in 0x2 for prefetch and TLBs (even TLB has its own leaf
> CPUID 0x18).
>
> And 0x2 leaf provides a descriptor 0xFF to instruct software to check
> cache information in 0x4 leaf instead.
>
> Therefore, follow this behavior to encode 0xFF when Intel CPU has 0x4
> leaf with "x-consistent-cache=true" for compatibility.
>
> In addition, for older CPUs without 0x4 leaf, still enumerate the cache
> descriptor in 0x2 leaf, except the case that there's no descriptor
> matching the cache model, then directly encode 0xFF in 0x2 leaf. This
> makes sense, as in the 0x2 leaf era, all supported caches should have
> the corresponding descriptor.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 48 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 2f895bf13523..a06aa1d629dc 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -223,7 +223,7 @@ struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
>   * Return a CPUID 2 cache descriptor for a given cache.
>   * If no known descriptor is found, return CACHE_DESCRIPTOR_UNAVAILABLE
>   */
> -static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache)
> +static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache, bool *unmacthed)
>  {
>      int i;
>  
> @@ -240,9 +240,44 @@ static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache)
>              }
>      }
>  
> +    *unmacthed |= true;
>      return CACHE_DESCRIPTOR_UNAVAILABLE;
>  }
>  
> +/* Encode cache info for CPUID[4] */
> +static void encode_cache_cpuid2(X86CPU *cpu,
> +                                uint32_t *eax, uint32_t *ebx,
> +                                uint32_t *ecx, uint32_t *edx)
> +{
> +    CPUX86State *env = &cpu->env;
> +    CPUCaches *caches = &env->cache_info_cpuid2;
> +    int l1d, l1i, l2, l3;
> +    bool unmatched = false;
> +
> +    *eax = 1; /* Number of CPUID[EAX=2] calls required */
> +    *ebx = *ecx = *edx = 0;
> +
> +    l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
> +    l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
> +    l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
> +    l3 = cpuid2_cache_descriptor(caches->l3_cache, &unmatched);
> +
> +    if (!cpu->consistent_cache ||
> +        (env->cpuid_min_level < 0x4 && !unmatched)) {
> +        /*
> +         * Though SDM defines code 0x40 for cases with no L2 or L3. It's
> +         * also valid to just ignore l3's code if there's no l2.
> +         */
> +        if (cpu->enable_l3_cache) {
> +            *ecx = l3;
> +        }
> +        *edx = (l1d << 16) | (l1i <<  8) | l2;
> +    } else {
> +        *ecx = 0;
> +        *edx = CACHE_DESCRIPTOR_UNAVAILABLE;
> +    }
> +}
> +
>  /* CPUID Leaf 4 constants: */
>  
>  /* EAX: */
> @@ -7451,16 +7486,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              *eax = *ebx = *ecx = *edx = 0;
>              break;
>          }
> -        *eax = 1; /* Number of CPUID[EAX=2] calls required */
> -        *ebx = 0;
> -        if (!cpu->enable_l3_cache) {
> -            *ecx = 0;
> -        } else {
> -            *ecx = cpuid2_cache_descriptor(env->cache_info_cpuid2.l3_cache);
> -        }
> -        *edx = (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1d_cache) << 16) |
> -               (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1i_cache) <<  8) |
> -               (cpuid2_cache_descriptor(env->cache_info_cpuid2.l2_cache));
> +        encode_cache_cpuid2(cpu, eax, ebx, ecx, edx);
>          break;
>      case 4:
>          /* cache info: needed for Core compatibility */

LGTM.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Re: [PATCH 06/16] i386/cpu: Drop CPUID 0x2 specific cache info in X86CPUState

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> With the pre-defined cache model legacy_intel_cpuid2_cache_info,
> for X86CPUState there's no need to cache special cache information
> for CPUID 0x2 leaf.
>
> Drop the cache_info_cpuid2 field of X86CPUState and use the
> legacy_intel_cpuid2_cache_info directly.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 31 +++++++++++--------------------
>  target/i386/cpu.h |  3 ++-
>  2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a06aa1d629dc..8f174fb971b6 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -244,19 +244,27 @@ static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache, bool *unmacthed)
>      return CACHE_DESCRIPTOR_UNAVAILABLE;
>  }
>  
> +static const CPUCaches legacy_intel_cpuid2_cache_info;
> +
>  /* Encode cache info for CPUID[4] */
>  static void encode_cache_cpuid2(X86CPU *cpu,
>                                  uint32_t *eax, uint32_t *ebx,
>                                  uint32_t *ecx, uint32_t *edx)
>  {
>      CPUX86State *env = &cpu->env;
> -    CPUCaches *caches = &env->cache_info_cpuid2;
> +    const CPUCaches *caches;
>      int l1d, l1i, l2, l3;
>      bool unmatched = false;
>  
>      *eax = 1; /* Number of CPUID[EAX=2] calls required */
>      *ebx = *ecx = *edx = 0;
>  
> +    if (env->enable_legacy_cpuid2_cache) {
> +        caches = &legacy_intel_cpuid2_cache_info;
> +    } else {
> +        caches = &env->cache_info_cpuid4;
> +    }
> +
>      l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
>      l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
>      l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
> @@ -705,17 +713,6 @@ static CPUCacheInfo legacy_l2_cache = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
> -static CPUCacheInfo legacy_l2_cache_cpuid2 = {
> -    .type = UNIFIED_CACHE,
> -    .level = 2,
> -    .size = 2 * MiB,
> -    .line_size = 64,
> -    .associativity = 8,
> -    .share_level = CPU_TOPOLOGY_LEVEL_INVALID,
> -};
> -
> -
>  /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
>  static CPUCacheInfo legacy_l2_cache_amd = {
>      .type = UNIFIED_CACHE,
> @@ -8951,18 +8948,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>                         "CPU model '%s' doesn't support legacy-cache=off", name);
>              return;
>          }
> -        env->cache_info_cpuid2 = env->cache_info_cpuid4 = env->cache_info_amd =
> -            *cache_info;
> +        env->cache_info_cpuid4 = env->cache_info_amd = *cache_info;
>      } else {
>          /* Build legacy cache information */
> -        env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
> -        env->cache_info_cpuid2.l1i_cache = &legacy_l1i_cache;
>          if (!cpu->consistent_cache) {
> -            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
> -        } else {
> -            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache;
> +            env->enable_legacy_cpuid2_cache = true;
>          }
> -        env->cache_info_cpuid2.l3_cache = &legacy_l3_cache;
>  
>          env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
>          env->cache_info_cpuid4.l1i_cache = &legacy_l1i_cache;
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 3c7e59ffb12a..8d3ce8a2b678 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2076,7 +2076,8 @@ typedef struct CPUArchState {
>       * on each CPUID leaf will be different, because we keep compatibility
>       * with old QEMU versions.
>       */
> -    CPUCaches cache_info_cpuid2, cache_info_cpuid4, cache_info_amd;
> +    CPUCaches cache_info_cpuid4, cache_info_amd;
> +    bool enable_legacy_cpuid2_cache;
>  
>      /* MTRRs */
>      uint64_t mtrr_fixed[11];

LGTM.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Re: [PATCH 07/16] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Per SDM, 0x80000005 leaf is reserved for Intel CPU, and its current
> "assert" check blocks adding new cache model for non-AMD CPUs.
>
> And please note, although Zhaoxin mostly follows Intel behavior,
> this leaf is an exception [1].
>
> So, add a compat property "x-vendor-cpuid-only-v2" (for PC machine v10.0
> and older) to keep the original behavior. For the machine since v10.1,
> check the vendor and encode this leaf as all-0 only for Intel CPU.
>
> This fix also resolves 2 FIXMEs of legacy_l1d_cache_amd and
> legacy_l1i_cache_amd:
>
> /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
>
> In addition, per AMD's APM, update the comment of CPUID[0x80000005].
>
> [1]: https://lore.kernel.org/qemu-devel/fa16f7a8-4917-4731-9d9f-7d4c10977168@zhaoxin.com/
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes since RFC:
>  * Only set all-0 for Intel CPU.
>  * Add x-vendor-cpuid-only-v2.
> ---
>  hw/i386/pc.c      |  1 +
>  target/i386/cpu.c | 11 ++++++++---
>  target/i386/cpu.h | 11 ++++++++++-
>  3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index ad2d6495ebde..9ec3f4db31f3 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -83,6 +83,7 @@
>  
>  GlobalProperty pc_compat_10_0[] = {
>      { TYPE_X86_CPU, "x-consistent-cache", "false" },
> +    { TYPE_X86_CPU, "x-vendor-cpuid-only-v2", "false" },
>  };
>  const size_t pc_compat_10_0_len = G_N_ELEMENTS(pc_compat_10_0);
>  
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8f174fb971b6..df40d1362566 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -655,7 +655,6 @@ static CPUCacheInfo legacy_l1d_cache = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
>  static CPUCacheInfo legacy_l1d_cache_amd = {
>      .type = DATA_CACHE,
>      .level = 1,
> @@ -684,7 +683,6 @@ static CPUCacheInfo legacy_l1i_cache = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
>  static CPUCacheInfo legacy_l1i_cache_amd = {
>      .type = INSTRUCTION_CACHE,
>      .level = 1,
> @@ -7889,11 +7887,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3];
>          break;
>      case 0x80000005:
> -        /* cache info (L1 cache) */
> +        /* cache info (L1 cache/TLB Associativity Field) */
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>              break;
>          }
> +
> +        if (cpu->vendor_cpuid_only_v2 && IS_INTEL_CPU(env)) {
> +            *eax = *ebx = *ecx = *edx = 0;
> +            break;
> +        }
> +
>          *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
>                 (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>          *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
> @@ -9464,6 +9468,7 @@ static const Property x86_cpu_properties[] = {
>      DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
>      DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
>      DEFINE_PROP_BOOL("x-vendor-cpuid-only", X86CPU, vendor_cpuid_only, true),
> +    DEFINE_PROP_BOOL("x-vendor-cpuid-only-v2", X86CPU, vendor_cpuid_only_v2, true),
>      DEFINE_PROP_BOOL("x-amd-topoext-features-only", X86CPU, amd_topoext_features_only, true),
>      DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
>      DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 8d3ce8a2b678..02cda176798f 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2282,9 +2282,18 @@ struct ArchCPU {
>      /* Enable auto level-increase for all CPUID leaves */
>      bool full_cpuid_auto_level;
>  
> -    /* Only advertise CPUID leaves defined by the vendor */
> +    /*
> +     * Compatibility bits for old machine types (PC machine v6.0 and older).
> +     * Only advertise CPUID leaves defined by the vendor.
> +     */
>      bool vendor_cpuid_only;
>  
> +    /*
> +     * Compatibility bits for old machine types (PC machine v10.0 and older).
> +     * Only advertise CPUID leaves defined by the vendor.
> +     */
> +    bool vendor_cpuid_only_v2;
> +
>      /* Only advertise TOPOEXT features that AMD defines */
>      bool amd_topoext_features_only;
>  

The Intel related part looks good to me. (Not quite familiar with AMD's
Spec, so no reviewed-by tag)

Re: [PATCH 08/16] i386/cpu: Fix CPUID[0x80000006] for Intel CPU

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Per SDM, Intel supports CPUID[0x80000006]. But only L2 information is
> encoded in ECX (note that L2 associativity field encodings rules
> consistent with AMD are used), all other fields are reserved.
>
> Therefore, make the following changes to CPUID[0x80000006]:
>  * Rename AMD_ENC_ASSOC to X86_ENC_ASSOC since Intel also uses the same
>    rules. (While there are some slight differences between the rules in
>    AMD APM v4.07 no.40332 and those in the current QEMU, generally they
>    are consistent.)
>  * Check the vendor in CPUID[0x80000006] and just encode L2 to ECX for
>    Intel.
>  * Assert L2's lines_per_tag is not 0 for AMD, and assert it is 0 for
>    Intel.
>  * Apply the encoding change of Intel for Zhaoxin as well [1].
>
> This fix also resolves the FIXME of legacy_l2_cache_amd:
>
> /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
>
> In addition, per AMD's APM, update the comment of CPUID[0x80000006].
>
> [1]: https://lore.kernel.org/qemu-devel/c522ebb5-04d5-49c6-9ad8-d755b8998988@zhaoxin.com/
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes since RFC:
>  * Check vendor_cpuid_only_v2 instead of vendor_cpuid_only.
>  * Move lines_per_tag assert check into encode_cache_cpuid80000006().
> ---
>  target/i386/cpu.c | 42 +++++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index df40d1362566..0b292aa2e07b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -506,8 +506,8 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
>  
>  #define ASSOC_FULL 0xFF
>  
> -/* AMD associativity encoding used on CPUID Leaf 0x80000006: */
> -#define AMD_ENC_ASSOC(a) (a <=   1 ? a   : \
> +/* x86 associativity encoding used on CPUID Leaf 0x80000006: */
> +#define X86_ENC_ASSOC(a) (a <=   1 ? a   : \
>                            a ==   2 ? 0x2 : \
>                            a ==   4 ? 0x4 : \
>                            a ==   8 ? 0x6 : \
> @@ -526,23 +526,26 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
>   */
>  static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
>                                         CPUCacheInfo *l3,
> -                                       uint32_t *ecx, uint32_t *edx)
> +                                       uint32_t *ecx, uint32_t *edx,
> +                                       bool lines_per_tag_supported)
>  {
>      assert(l2->size % 1024 == 0);
>      assert(l2->associativity > 0);
> -    assert(l2->lines_per_tag > 0);
> -    assert(l2->line_size > 0);

why remove the assert for l2->line_size?


> +    assert(lines_per_tag_supported ?
> +           l2->lines_per_tag > 0 : l2->lines_per_tag == 0);
>      *ecx = ((l2->size / 1024) << 16) |
> -           (AMD_ENC_ASSOC(l2->associativity) << 12) |
> +           (X86_ENC_ASSOC(l2->associativity) << 12) |
>             (l2->lines_per_tag << 8) | (l2->line_size);
>  
> +    /* For Intel, EDX is reserved. */
>      if (l3) {
>          assert(l3->size % (512 * 1024) == 0);
>          assert(l3->associativity > 0);
> -        assert(l3->lines_per_tag > 0);
> +        assert(lines_per_tag_supported ?
> +               l3->lines_per_tag > 0 : l3->lines_per_tag == 0);
>          assert(l3->line_size > 0);
>          *edx = ((l3->size / (512 * 1024)) << 18) |
> -               (AMD_ENC_ASSOC(l3->associativity) << 12) |
> +               (X86_ENC_ASSOC(l3->associativity) << 12) |
>                 (l3->lines_per_tag << 8) | (l3->line_size);
>      } else {
>          *edx = 0;
> @@ -711,7 +714,6 @@ static CPUCacheInfo legacy_l2_cache = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
>  static CPUCacheInfo legacy_l2_cache_amd = {
>      .type = UNIFIED_CACHE,
>      .level = 2,
> @@ -7906,23 +7908,33 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
>          break;
>      case 0x80000006:
> -        /* cache info (L2 cache) */
> +        /* cache info (L2 cache/TLB/L3 cache) */
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>              break;
>          }
> -        *eax = (AMD_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) |
> +
> +        if (cpu->vendor_cpuid_only_v2 &&
> +            (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
> +            *eax = *ebx = 0;
> +            encode_cache_cpuid80000006(env->cache_info_cpuid4.l2_cache,
> +                                       NULL, ecx, edx, false);
> +            break;
> +        }
> +
> +        *eax = (X86_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) |
>                 (L2_DTLB_2M_ENTRIES << 16) |
> -               (AMD_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) |
> +               (X86_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) |
>                 (L2_ITLB_2M_ENTRIES);
> -        *ebx = (AMD_ENC_ASSOC(L2_DTLB_4K_ASSOC) << 28) |
> +        *ebx = (X86_ENC_ASSOC(L2_DTLB_4K_ASSOC) << 28) |
>                 (L2_DTLB_4K_ENTRIES << 16) |
> -               (AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) |
> +               (X86_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) |
>                 (L2_ITLB_4K_ENTRIES);
> +
>          encode_cache_cpuid80000006(env->cache_info_amd.l2_cache,
>                                     cpu->enable_l3_cache ?
>                                     env->cache_info_amd.l3_cache : NULL,
> -                                   ecx, edx);
> +                                   ecx, edx, true);
>          break;
>      case 0x80000007:
>          *eax = 0;

Re: [PATCH 09/16] i386/cpu: Add legacy_intel_cache_info cache model

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Based on legacy_l1d_cache, legacy_l1i_cache, legacy_l2_cache and
> legacy_l3_cache, build a complete legacy intel cache model, which can
> clarify the purpose of these trivial legacy cache models, simplify the
> initialization of cache info in X86CPUState, and make it easier to
> handle compatibility later.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 101 +++++++++++++++++++++++++---------------------
>  1 file changed, 54 insertions(+), 47 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 0b292aa2e07b..ec229830c532 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -643,21 +643,6 @@ static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
>   * These are legacy cache values. If there is a need to change any
>   * of these values please use builtin_x86_defs
>   */
> -
> -/* L1 data cache: */
> -static CPUCacheInfo legacy_l1d_cache = {
> -    .type = DATA_CACHE,
> -    .level = 1,
> -    .size = 32 * KiB,
> -    .self_init = 1,
> -    .line_size = 64,
> -    .associativity = 8,
> -    .sets = 64,
> -    .partitions = 1,
> -    .no_invd_sharing = true,
> -    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> -};
> -
>  static CPUCacheInfo legacy_l1d_cache_amd = {
>      .type = DATA_CACHE,
>      .level = 1,
> @@ -672,20 +657,6 @@ static CPUCacheInfo legacy_l1d_cache_amd = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/* L1 instruction cache: */
> -static CPUCacheInfo legacy_l1i_cache = {
> -    .type = INSTRUCTION_CACHE,
> -    .level = 1,
> -    .size = 32 * KiB,
> -    .self_init = 1,
> -    .line_size = 64,
> -    .associativity = 8,
> -    .sets = 64,
> -    .partitions = 1,
> -    .no_invd_sharing = true,
> -    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> -};
> -
>  static CPUCacheInfo legacy_l1i_cache_amd = {
>      .type = INSTRUCTION_CACHE,
>      .level = 1,
> @@ -700,20 +671,6 @@ static CPUCacheInfo legacy_l1i_cache_amd = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/* Level 2 unified cache: */
> -static CPUCacheInfo legacy_l2_cache = {
> -    .type = UNIFIED_CACHE,
> -    .level = 2,
> -    .size = 4 * MiB,
> -    .self_init = 1,
> -    .line_size = 64,
> -    .associativity = 16,
> -    .sets = 4096,
> -    .partitions = 1,
> -    .no_invd_sharing = true,
> -    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> -};
> -
>  static CPUCacheInfo legacy_l2_cache_amd = {
>      .type = UNIFIED_CACHE,
>      .level = 2,
> @@ -803,6 +760,59 @@ static const CPUCaches legacy_intel_cpuid2_cache_info = {
>      },
>  };
>  
> +static const CPUCaches legacy_intel_cache_info = {
> +    .l1d_cache = &(CPUCacheInfo) {
> +        .type = DATA_CACHE,
> +        .level = 1,
> +        .size = 32 * KiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .sets = 64,
> +        .partitions = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l1i_cache = &(CPUCacheInfo) {
> +        .type = INSTRUCTION_CACHE,
> +        .level = 1,
> +        .size = 32 * KiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .sets = 64,
> +        .partitions = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l2_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 2,
> +        .size = 4 * MiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .sets = 4096,
> +        .partitions = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l3_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 3,
> +        .size = 16 * MiB,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .sets = 16384,
> +        .partitions = 1,
> +        .lines_per_tag = 1,
> +        .self_init = true,
> +        .inclusive = true,
> +        .complex_indexing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
> +    },
> +};
> +
>  /* TLB definitions: */
>  
>  #define L1_DTLB_2M_ASSOC       1
> @@ -8971,10 +8981,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>              env->enable_legacy_cpuid2_cache = true;
>          }
>  
> -        env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
> -        env->cache_info_cpuid4.l1i_cache = &legacy_l1i_cache;
> -        env->cache_info_cpuid4.l2_cache = &legacy_l2_cache;
> -        env->cache_info_cpuid4.l3_cache = &legacy_l3_cache;
> +        env->cache_info_cpuid4 = legacy_intel_cache_info;
>  
>          env->cache_info_amd.l1d_cache = &legacy_l1d_cache_amd;
>          env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd;

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Re: [PATCH 10/16] i386/cpu: Add legacy_amd_cache_info cache model

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Based on legacy_l1d_cachei_amd, legacy_l1i_cache_amd, legacy_l2_cache_amd
> and legacy_l3_cache, build a complete legacy AMD cache model, which can
> clarify the purpose of these trivial legacy cache models, simplify the
> initialization of cache info in X86CPUState, and make it easier to
> handle compatibility later.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 112 ++++++++++++++++++++++------------------------
>  1 file changed, 53 insertions(+), 59 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ec229830c532..bf8d7a19c88d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -643,60 +643,58 @@ static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
>   * These are legacy cache values. If there is a need to change any
>   * of these values please use builtin_x86_defs
>   */
> -static CPUCacheInfo legacy_l1d_cache_amd = {
> -    .type = DATA_CACHE,
> -    .level = 1,
> -    .size = 64 * KiB,
> -    .self_init = 1,
> -    .line_size = 64,
> -    .associativity = 2,
> -    .sets = 512,
> -    .partitions = 1,
> -    .lines_per_tag = 1,
> -    .no_invd_sharing = true,
> -    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> -};
> -
> -static CPUCacheInfo legacy_l1i_cache_amd = {
> -    .type = INSTRUCTION_CACHE,
> -    .level = 1,
> -    .size = 64 * KiB,
> -    .self_init = 1,
> -    .line_size = 64,
> -    .associativity = 2,
> -    .sets = 512,
> -    .partitions = 1,
> -    .lines_per_tag = 1,
> -    .no_invd_sharing = true,
> -    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> -};
> -
> -static CPUCacheInfo legacy_l2_cache_amd = {
> -    .type = UNIFIED_CACHE,
> -    .level = 2,
> -    .size = 512 * KiB,
> -    .line_size = 64,
> -    .lines_per_tag = 1,
> -    .associativity = 16,
> -    .sets = 512,
> -    .partitions = 1,
> -    .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> -};
> -
> -/* Level 3 unified cache: */
> -static CPUCacheInfo legacy_l3_cache = {
> -    .type = UNIFIED_CACHE,
> -    .level = 3,
> -    .size = 16 * MiB,
> -    .line_size = 64,
> -    .associativity = 16,
> -    .sets = 16384,
> -    .partitions = 1,
> -    .lines_per_tag = 1,
> -    .self_init = true,
> -    .inclusive = true,
> -    .complex_indexing = true,
> -    .share_level = CPU_TOPOLOGY_LEVEL_DIE,
> +static const CPUCaches legacy_amd_cache_info = {
> +    .l1d_cache = &(CPUCacheInfo) {
> +        .type = DATA_CACHE,
> +        .level = 1,
> +        .size = 64 * KiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 2,
> +        .sets = 512,
> +        .partitions = 1,
> +        .lines_per_tag = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l1i_cache = &(CPUCacheInfo) {
> +        .type = INSTRUCTION_CACHE,
> +        .level = 1,
> +        .size = 64 * KiB,
> +        .self_init = 1,
> +        .line_size = 64,
> +        .associativity = 2,
> +        .sets = 512,
> +        .partitions = 1,
> +        .lines_per_tag = 1,
> +        .no_invd_sharing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l2_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 2,
> +        .size = 512 * KiB,
> +        .line_size = 64,
> +        .lines_per_tag = 1,
> +        .associativity = 16,
> +        .sets = 512,
> +        .partitions = 1,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l3_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 3,
> +        .size = 16 * MiB,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .sets = 16384,
> +        .partitions = 1,
> +        .lines_per_tag = 1,
> +        .self_init = true,
> +        .inclusive = true,
> +        .complex_indexing = true,
> +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
> +    },
>  };
>  
>  /*
> @@ -8982,11 +8980,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          }
>  
>          env->cache_info_cpuid4 = legacy_intel_cache_info;
> -
> -        env->cache_info_amd.l1d_cache = &legacy_l1d_cache_amd;
> -        env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd;
> -        env->cache_info_amd.l2_cache = &legacy_l2_cache_amd;
> -        env->cache_info_amd.l3_cache = &legacy_l3_cache;
> +        env->cache_info_amd = legacy_amd_cache_info;
>      }
>  
>  #ifndef CONFIG_USER_ONLY

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Re: [PATCH 01/16] i386/cpu: Refine comment of CPUID2CacheDescriptorInfo

[Zhao Liu]

> > +    /*
> > +     * Newer Intel CPUs (having the cores without L3, e.g., Intel MTL, ARL)
> > +     * use CPUID 0x4 leaf to describe cache topology, by encoding CPUID 0x2
> > +     * leaf with 0xFF. For older CPUs (without 0x4 leaf), it's also valid
> > +     * to just ignore l3's code if there's no l3.
> 
> s/l3/L3/g

Sure!

> Others look good to me. 
> 
> Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
 
Thanks!

Re: [PATCH 02/16] i386/cpu: Add descriptor 0x49 for CPUID 0x2 encoding

[Zhao Liu]

> > -    /* Descriptor 0x49 depends on CPU family/model, so it is not included */
> > +    /*
> > +     * Descriptor 0x49 has 2 cases:
> > +     *  - 2nd-level cache: 4 MByte, 16-way set associative, 64 byte line size.
> > +     *  - 3rd-level cache: 4MB, 16-way set associative, 64-byte line size
> > +     *    (Intel Xeon processor MP, Family 0FH, Model 06H).
> > +     *
> > +     * When it represents l3, then it depends on CPU family/model. Fortunately,
> > +     * the legacy cache/CPU models don't have such special l3. So, just add it
> > +     * to represent the general l2 case.
> 
> For comments and commit message, we'd better use the capital character
> "L2/L3" to represent the 2nd/3rd level cache which is more conventional. 

Sure.

> Others look good to me.
> 
> Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Thanks!

Re: [PATCH 03/16] i386/cpu: Add default cache model for Intel CPUs with level < 4

[Zhao Liu]

> > +/*
> > + * Only used for the CPU models with CPUID level < 4.
> > + * These CPUs (CPUID level < 4) only use CPUID leaf 2 to present
> > + * cache information.
> > + *
> > + * Note: This cache model is just a default one, and is not
> > + *       guaranteed to match real hardwares.
> > + */
> > +static const CPUCaches legacy_intel_cpuid2_cache_info = {
> > +    .l1d_cache = &(CPUCacheInfo) {
> > +        .type = DATA_CACHE,
> > +        .level = 1,
> > +        .size = 32 * KiB,
> > +        .self_init = 1,
> > +        .line_size = 64,
> > +        .associativity = 8,
> > +        .sets = 64,
> > +        .partitions = 1,
> > +        .no_invd_sharing = true,
> > +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> > +    },
> > +    .l1i_cache = &(CPUCacheInfo) {
> > +        .type = INSTRUCTION_CACHE,
> > +        .level = 1,
> > +        .size = 32 * KiB,
> > +        .self_init = 1,
> > +        .line_size = 64,
> > +        .associativity = 8,
> > +        .sets = 64,
> > +        .partitions = 1,
> > +        .no_invd_sharing = true,
> > +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> > +    },
> > +    .l2_cache = &(CPUCacheInfo) {
> > +        .type = UNIFIED_CACHE,
> > +        .level = 2,
> > +        .size = 2 * MiB,
> > +        .self_init = 1,
> > +        .line_size = 64,
> > +        .associativity = 8,
> > +        .sets = 4096,
> > +        .partitions = 1,
> > +        .no_invd_sharing = true,
> > +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> > +    },
> > +    .l3_cache = &(CPUCacheInfo) {
> > +        .type = UNIFIED_CACHE,
> > +        .level = 3,
> > +        .size = 16 * MiB,
> > +        .line_size = 64,
> > +        .associativity = 16,
> > +        .sets = 16384,
> > +        .partitions = 1,
> > +        .lines_per_tag = 1,
> > +        .self_init = true,
> > +        .inclusive = true,
> > +        .complex_indexing = true,
> > +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
> > +    },
> 
> Does this cache information match the real legacy HW or just an emulation
> of Qemu?

This is the pure emulation and it doesn't macth any HW :-(, and is a
"hybrid" result of continuously modifying and adding new cache features
(like the virtual L3). But for compatibility reasons, I abstracte it
into this special cache model, used only for older CPUs.

This way, at least modern CPUs won't be burdened by old historical
issues.

Thanks,
Zhao

Re: [PATCH 08/16] i386/cpu: Fix CPUID[0x80000006] for Intel CPU

[Zhao Liu]

> >  static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
> >                                         CPUCacheInfo *l3,
> > -                                       uint32_t *ecx, uint32_t *edx)
> > +                                       uint32_t *ecx, uint32_t *edx,
> > +                                       bool lines_per_tag_supported)
> >  {
> >      assert(l2->size % 1024 == 0);
> >      assert(l2->associativity > 0);
> > -    assert(l2->lines_per_tag > 0);
> > -    assert(l2->line_size > 0);
> 
> why remove the assert for l2->line_size?

Good catch! My bad...

> > +    assert(lines_per_tag_supported ?
> > +           l2->lines_per_tag > 0 : l2->lines_per_tag == 0);
> >      *ecx = ((l2->size / 1024) << 16) |
> > -           (AMD_ENC_ASSOC(l2->associativity) << 12) |
> > +           (X86_ENC_ASSOC(l2->associativity) << 12) |
> >             (l2->lines_per_tag << 8) | (l2->line_size);
> >  

Re: [PATCH 11/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x2

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> As preparation for merging cache_info_cpuid4 and cache_info_amd in
> X86CPUState, set legacy cache model based on vendor in the CPUID 0x2
> leaf. For AMD CPU, select legacy AMD cache model (in cache_info_amd) as
> the default cache model, otherwise, select legacy Intel cache model (in
> cache_info_cpuid4) as before.
>
> To ensure compatibility is not broken, add an enable_legacy_vendor_cache
> flag based on x-vendor-only-v2 to indicate cases where the legacy cache
> model should be used regardless of the vendor. For CPUID 0x2 leaf,
> enable_legacy_vendor_cache flag indicates to pick legacy Intel cache
> model, which is for compatibility with the behavior of PC machine v10.0
> and older.
>
> The following explains how current vendor-based default legacy cache
> model ensures correctness without breaking compatibility.
>
> * For the PC machine v6.0 and older, vendor_cpuid_only=false, and
>   vendor_cpuid_only_v2=false.
>
>   - If the named CPU model has its own cache model, and doesn't use
>     legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
>     cache_info_amd are same, so 0x2 leaf uses its own cache model
>     regardless of the vendor.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is true, they will use legacy Intel cache
>     model just like their previous behavior.
>
> * For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
>   and vendor_cpuid_only_v2=false.
>
>   - If the named CPU model has its own cache model (legacy_cache=false),
>     then cache_info_cpuid4 & cache_info_amd both equal to its own cache
>     model, so it uses its own cache model in 0x2 leaf regardless of the
>     vendor. Only AMD CPUs have all-0 leaf due to vendor_cpuid_only=true,
>     and this is exactly the behavior of these old machines.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is true, they will use legacy Intel cache
>     model. Similarly, only AMD CPUs have all-0 leaf, and this is exactly
>     the behavior of these old machines.
>
> * For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
>   vendor_cpuid_only_v2=true.
>
>   - If the named CPU model has its own cache model (legacy_cache=false),
>     then cache_info_cpuid4 & cache_info_amd both equal to its own cache
>     model, so it uses its own cache model in 0x2 leaf regardless of the
>     vendor. And AMD CPUs have all-0 leaf. Nothing will change.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is false, the legacy cache model is
>     selected based on vendor.
>
>     For AMD CPU, it will use legacy AMD cache but still get all-0 leaf
>     due to vendor_cpuid_only=true.
>
>     For non-AMD (Intel/Zhaoxin) CPU, it will use legacy Intel cache as
>     expected.
>
>     Here, selecting the legacy cache model based on the vendor does not
>     change the previous (before the change)  behavior.
>
> Therefore, the above analysis proves that, with the help of the flag
> enable_legacy_vendor_cache, it is acceptable to select the default
> legacy cache model based on the vendor.
>
> For the CPUID 0x2 leaf, in X86CPUState, a unified cache_info is enough.
> It only needs to be initialized and configured with the corresponding
> legacy cache model based on the vendor.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++----------
>  target/i386/cpu.h |  1 +
>  2 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index bf8d7a19c88d..524d39de9ace 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -248,23 +248,17 @@ static const CPUCaches legacy_intel_cpuid2_cache_info;
>  
>  /* Encode cache info for CPUID[4] */
>  static void encode_cache_cpuid2(X86CPU *cpu,
> +                                const CPUCaches *caches,
>                                  uint32_t *eax, uint32_t *ebx,
>                                  uint32_t *ecx, uint32_t *edx)
>  {
>      CPUX86State *env = &cpu->env;
> -    const CPUCaches *caches;
>      int l1d, l1i, l2, l3;
>      bool unmatched = false;
>  
>      *eax = 1; /* Number of CPUID[EAX=2] calls required */
>      *ebx = *ecx = *edx = 0;
>  
> -    if (env->enable_legacy_cpuid2_cache) {
> -        caches = &legacy_intel_cpuid2_cache_info;
> -    } else {
> -        caches = &env->cache_info_cpuid4;
> -    }
> -
>      l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
>      l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
>      l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
> @@ -7482,8 +7476,37 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              *ecx &= ~CPUID_EXT_PDCM;
>          }
>          break;
> -    case 2:
> -        /* cache info: needed for Pentium Pro compatibility */
> +    case 2: { /* cache info: needed for Pentium Pro compatibility */
> +        const CPUCaches *caches;
> +
> +        if (env->enable_legacy_cpuid2_cache) {
> +            caches = &legacy_intel_cpuid2_cache_info;
> +        } else if (env->enable_legacy_vendor_cache) {
> +            caches = &legacy_intel_cache_info;
> +        } else {
> +            /*
> +             * FIXME: Temporarily select cache info model here based on
> +             * vendor, and merge these 2 cache info models later.
> +             *
> +             * This condition covers the following cases (with
> +             * enable_legacy_vendor_cache=false):
> +             *  - When CPU model has its own cache model and doesn't use legacy
> +             *    cache model (legacy_model=off). Then cache_info_amd and
> +             *    cache_info_cpuid4 are the same.
> +             *
> +             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> +             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
> +             *    CPU will use cache_info_amd. But this doesn't matter for AMD
> +             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
> +             *    model is.
> +             */
> +            if (IS_AMD_CPU(env)) {
> +                caches = &env->cache_info_amd;
> +            } else {
> +                caches = &env->cache_info_cpuid4;
> +            }
> +        }
> +
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>              break;
> @@ -7491,8 +7514,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              *eax = *ebx = *ecx = *edx = 0;
>              break;
>          }
> -        encode_cache_cpuid2(cpu, eax, ebx, ecx, edx);
> +        encode_cache_cpuid2(cpu, caches, eax, ebx, ecx, edx);
>          break;
> +    }
>      case 4:
>          /* cache info: needed for Core compatibility */
>          if (cpu->cache_info_passthrough) {
> @@ -8979,6 +9003,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>              env->enable_legacy_cpuid2_cache = true;
>          }
>  
> +        if (!cpu->vendor_cpuid_only_v2) {
> +            env->enable_legacy_vendor_cache = true;
> +        }
>          env->cache_info_cpuid4 = legacy_intel_cache_info;
>          env->cache_info_amd = legacy_amd_cache_info;
>      }
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 02cda176798f..243383efd602 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2078,6 +2078,7 @@ typedef struct CPUArchState {
>       */
>      CPUCaches cache_info_cpuid4, cache_info_amd;
>      bool enable_legacy_cpuid2_cache;
> +    bool enable_legacy_vendor_cache;
>  
>      /* MTRRs */
>      uint64_t mtrr_fixed[11];

LGTM. Thanks.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Re: [PATCH 12/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x4

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> As preparation for merging cache_info_cpuid4 and cache_info_amd in
> X86CPUState, set legacy cache model based on vendor in the CPUID 0x4
> leaf. For AMD CPU, select legacy AMD cache model (in cache_info_amd) as
> the default cache model, otherwise, select legacy Intel cache model (in
> cache_info_cpuid4) as before.
>
> To ensure compatibility is not broken, add an enable_legacy_vendor_cache
> flag based on x-vendor-only-v2 to indicate cases where the legacy cache
> model should be used regardless of the vendor. For CPUID 0x4 leaf,
> enable_legacy_vendor_cache flag indicates to pick legacy Intel cache
> model, which is for compatibility with the behavior of PC machine v10.0
> and older.
>
> The following explains how current vendor-based default legacy cache
> model ensures correctness without breaking compatibility.
>
> * For the PC machine v6.0 and older, vendor_cpuid_only=false, and
>   vendor_cpuid_only_v2=false.
>
>   - If the named CPU model has its own cache model, and doesn't use
>     legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
>     cache_info_amd are same, so 0x4 leaf uses its own cache model
>     regardless of the vendor.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is true, they will use legacy Intel cache
>     model just like their previous behavior.
>
> * For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
>   and vendor_cpuid_only_v2=false.
>
>   - If the named CPU model has its own cache model (legacy_cache=false),
>     then cache_info_cpuid4 & cache_info_amd both equal to its own cache
>     model, so it uses its own cache model in 0x4 leaf regardless of the
>     vendor. Only AMD CPUs have all-0 leaf due to vendor_cpuid_only=true,
>     and this is exactly the behavior of these old machines.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is true, they will use legacy Intel cache
>     model. Similarly, only AMD CPUs have all-0 leaf, and this is exactly
>     the behavior of these old machines.
>
> * For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
>   vendor_cpuid_only_v2=true.
>
>   - If the named CPU model has its own cache model (legacy_cache=false),
>     then cache_info_cpuid4 & cache_info_amd both equal to its own cache
>     model, so it uses its own cache model in 0x4 leaf regardless of the
>     vendor. And AMD CPUs have all-0 leaf. Nothing will change.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is false, the legacy cache model is
>     selected based on vendor.
>
>     For AMD CPU, it will use legacy AMD cache but still get all-0 leaf
>     due to vendor_cpuid_only=true.
>
>     For non-AMD (Intel/Zhaoxin) CPU, it will use legacy Intel cache as
>     expected.
>
>     Here, selecting the legacy cache model based on the vendor does not
>     change the previous (before the change) behavior.
>
> Therefore, the above analysis proves that, with the help of the flag
> enable_legacy_vendor_cache, it is acceptable to select the default
> legacy cache model based on the vendor.
>
> For the CPUID 0x4 leaf, in X86CPUState, a unified cache_info is enough.
> It only needs to be initialized and configured with the corresponding
> legacy cache model based on the vendor.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 43 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 524d39de9ace..afbf11569ab4 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7517,7 +7517,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          encode_cache_cpuid2(cpu, caches, eax, ebx, ecx, edx);
>          break;
>      }
> -    case 4:
> +    case 4: {
> +        const CPUCaches *caches;
> +
> +        if (env->enable_legacy_vendor_cache) {
> +            caches = &legacy_intel_cache_info;
> +        } else {
> +            /*
> +             * FIXME: Temporarily select cache info model here based on
> +             * vendor, and merge these 2 cache info models later.
> +             *
> +             * This condition covers the following cases (with
> +             * enable_legacy_vendor_cache=false):
> +             *  - When CPU model has its own cache model and doesn't use legacy
> +             *    cache model (legacy_model=off). Then cache_info_amd and
> +             *    cache_info_cpuid4 are the same.
> +             *
> +             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> +             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
> +             *    CPU will use cache_info_amd. But this doesn't matter for AMD
> +             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
> +             *    model is.
> +             */
> +            if (IS_AMD_CPU(env)) {
> +                caches = &env->cache_info_amd;
> +            } else {
> +                caches = &env->cache_info_cpuid4;
> +            }
> +        }
> +
>          /* cache info: needed for Core compatibility */
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, count, eax, ebx, ecx, edx);
> @@ -7545,30 +7573,26 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>  
>              switch (count) {
>              case 0: /* L1 dcache info */
> -                encode_cache_cpuid4(env->cache_info_cpuid4.l1d_cache,
> -                                    topo_info,
> +                encode_cache_cpuid4(caches->l1d_cache, topo_info,
>                                      eax, ebx, ecx, edx);
>                  if (!cpu->l1_cache_per_core) {
>                      *eax &= ~MAKE_64BIT_MASK(14, 12);
>                  }
>                  break;
>              case 1: /* L1 icache info */
> -                encode_cache_cpuid4(env->cache_info_cpuid4.l1i_cache,
> -                                    topo_info,
> +                encode_cache_cpuid4(caches->l1i_cache, topo_info,
>                                      eax, ebx, ecx, edx);
>                  if (!cpu->l1_cache_per_core) {
>                      *eax &= ~MAKE_64BIT_MASK(14, 12);
>                  }
>                  break;
>              case 2: /* L2 cache info */
> -                encode_cache_cpuid4(env->cache_info_cpuid4.l2_cache,
> -                                    topo_info,
> +                encode_cache_cpuid4(caches->l2_cache, topo_info,
>                                      eax, ebx, ecx, edx);
>                  break;
>              case 3: /* L3 cache info */
>                  if (cpu->enable_l3_cache) {
> -                    encode_cache_cpuid4(env->cache_info_cpuid4.l3_cache,
> -                                        topo_info,
> +                    encode_cache_cpuid4(caches->l3_cache, topo_info,
>                                          eax, ebx, ecx, edx);
>                      break;
>                  }
> @@ -7579,6 +7603,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              }
>          }
>          break;
> +    }
>      case 5:
>          /* MONITOR/MWAIT Leaf */
>          *eax = cpu->mwait.eax; /* Smallest monitor-line size in bytes */

LGTM. Thanks.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Re: [PATCH 13/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x80000005

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> As preparation for merging cache_info_cpuid4 and cache_info_amd in
> X86CPUState, set legacy cache model based on vendor in the CPUID
> 0x80000005 leaf. For AMD CPU, select legacy AMD cache model (in
> cache_info_amd) as the default cache model like before, otherwise,
> select legacy Intel cache model (in cache_info_cpuid4).
>
> To ensure compatibility is not broken, add an enable_legacy_vendor_cache
> flag based on x-vendor-only-v2 to indicate cases where the legacy cache
> model should be used regardless of the vendor. For CPUID 0x80000005
> leaf, enable_legacy_vendor_cache flag indicates to pick legacy AMD cache
> model, which is for compatibility with the behavior of PC machine v10.0
> and older.
>
> The following explains how current vendor-based default legacy cache
> model ensures correctness without breaking compatibility.
>
> * For the PC machine v6.0 and older, vendor_cpuid_only=false, and
>   vendor_cpuid_only_v2=false.
>
>   - If the named CPU model has its own cache model, and doesn't use
>     legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
>     cache_info_amd are same, so 0x80000005 leaf uses its own cache model
>     regardless of the vendor.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is true, they will use legacy AMD cache
>     model just like their previous behavior.
>
> * For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
>   and vendor_cpuid_only_v2=false.
>
>   - No change, since this leaf doesn't aware vendor_cpuid_only.
>
> * For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
>   vendor_cpuid_only_v2=true.
>
>   - If the named CPU model has its own cache model (legacy_cache=false),
>     then cache_info_cpuid4 & cache_info_amd both equal to its own cache
>     model, so it uses its own cache model in 0x80000005 leaf regardless
>     of the vendor. Only Intel CPUs have all-0 leaf due to
>     vendor_cpuid_only_2=true, and this is exactly the expected behavior.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is false, the legacy cache model is
>     selected based on vendor.
>
>     For AMD CPU, it will use legacy AMD cache as expected.
>
>     For Intel CPU, it will use legacy Intel cache but still get all-0
>     leaf due to vendor_cpuid_only_2=true as expected.
>
>     (Note) And for Zhaoxin CPU, it will use legacy Intel cache model
>     instead of AMD's. This is the difference brought by this change! But
>     it's correct since then Zhaoxin could have the consistent cache info
>     in CPUID 0x2, 0x4 and 0x80000005 leaves.
>
>     Here, except Zhaoxin, selecting the legacy cache model based on the
>     vendor does not change the previous (before the change) behavior.
>     And the change for Zhaoxin is also a good improvement.
>
> Therefore, the above analysis proves that, with the help of the flag
> enable_legacy_vendor_cache, it is acceptable to select the default
> legacy cache model based on the vendor.
>
> For the CPUID 0x80000005 leaf, in X86CPUState, a unified cache_info is
> enough. It only needs to be initialized and configured with the
> corresponding legacy cache model based on the vendor.
>
> Cc: EwanHai <ewanhai-oc@zhaoxin.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Note, side effect of this patch: fix the inconsistency cache info for
> Zhaoxin. For more details, see the commit message above.
> ---
>  target/i386/cpu.c | 36 ++++++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index afbf11569ab4..16b4ecb76113 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7945,8 +7945,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *ecx = env->cpuid_model[(index - 0x80000002) * 4 + 2];
>          *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3];
>          break;
> -    case 0x80000005:
> -        /* cache info (L1 cache/TLB Associativity Field) */
> +    case 0x80000005: { /* cache info (L1 cache/TLB Associativity Field) */
Better put the comment into an independent line.

> +        const CPUCaches *caches;
> +
> +        if (env->enable_legacy_vendor_cache) {
> +            caches = &legacy_amd_cache_info;
> +        } else {
> +            /*
> +             * FIXME: Temporarily select cache info model here based on
> +             * vendor, and merge these 2 cache info models later.
> +             *
> +             * This condition covers the following cases (with
> +             * enable_legacy_vendor_cache=false):
> +             *  - When CPU model has its own cache model and doesn't uses legacy
> +             *    cache model (legacy_model=off). Then cache_info_amd and
> +             *    cache_info_cpuid4 are the same.
> +             *
> +             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> +             *    model. AMD CPUs use cache_info_amd like before and non-AMD
> +             *    CPU will use cache_info_cpuid4. But this doesn't matter,
> +             *    because for Intel CPU, it will get all-0 leaf, and Zhaoxin CPU
> +             *    will get correct cache info. Both are expected.
> +             */
> +            if (IS_AMD_CPU(env)) {
> +                caches = &env->cache_info_amd;
> +            } else {
> +                caches = &env->cache_info_cpuid4;
> +            }
> +        }
> +
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>              break;
> @@ -7961,9 +7988,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                 (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>          *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
>                 (L1_ITLB_4K_ASSOC <<  8) | (L1_ITLB_4K_ENTRIES);
> -        *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache);
> -        *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
> +        *ecx = encode_cache_cpuid80000005(caches->l1d_cache);
> +        *edx = encode_cache_cpuid80000005(caches->l1i_cache);
>          break;
> +    }
>      case 0x80000006:
>          /* cache info (L2 cache/TLB/L3 cache) */
>          if (cpu->cache_info_passthrough) {

Re: [PATCH 16/16] i386/cpu: Use a unified cache_info in X86CPUState

[Mi, Dapeng]

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> At present, all cases using the cache model (CPUID 0x2, 0x4, 0x80000005,
> 0x80000006 and 0x8000001D leaves) have been verified to be able to
> select either cache_info_intel or cache_info_amd based on the vendor.
>
> Therefore, further merge cache_info_intel and cache_info_amd into a
> unified cache_info in X86CPUState, and during its initialization, set
> different legacy cache models based on the vendor.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 150 ++++++++--------------------------------------
>  target/i386/cpu.h |   5 +-
>  2 files changed, 27 insertions(+), 128 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 4e9ac37850c0..c1d1289ee9de 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7484,27 +7484,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          } else if (env->enable_legacy_vendor_cache) {
>              caches = &legacy_intel_cache_info;
>          } else {
> -            /*
> -             * FIXME: Temporarily select cache info model here based on
> -             * vendor, and merge these 2 cache info models later.
> -             *
> -             * This condition covers the following cases (with
> -             * enable_legacy_vendor_cache=false):
> -             *  - When CPU model has its own cache model and doesn't use legacy
> -             *    cache model (legacy_model=off). Then cache_info_amd and
> -             *    cache_info_cpuid4 are the same.
> -             *
> -             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> -             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
> -             *    CPU will use cache_info_amd. But this doesn't matter for AMD
> -             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
> -             *    model is.
> -             */
> -            if (IS_AMD_CPU(env)) {
> -                caches = &env->cache_info_amd;
> -            } else {
> -                caches = &env->cache_info_cpuid4;
> -            }
> +            caches = &env->cache_info;
>          }
>  
>          if (cpu->cache_info_passthrough) {
> @@ -7523,27 +7503,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          if (env->enable_legacy_vendor_cache) {
>              caches = &legacy_intel_cache_info;
>          } else {
> -            /*
> -             * FIXME: Temporarily select cache info model here based on
> -             * vendor, and merge these 2 cache info models later.
> -             *
> -             * This condition covers the following cases (with
> -             * enable_legacy_vendor_cache=false):
> -             *  - When CPU model has its own cache model and doesn't use legacy
> -             *    cache model (legacy_model=off). Then cache_info_amd and
> -             *    cache_info_cpuid4 are the same.
> -             *
> -             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> -             *    model. Non-AMD CPUs use cache_info_cpuid4 like before and AMD
> -             *    CPU will use cache_info_amd. But this doesn't matter for AMD
> -             *    CPU, because this leaf encodes all-0 for AMD whatever its cache
> -             *    model is.
> -             */
> -            if (IS_AMD_CPU(env)) {
> -                caches = &env->cache_info_amd;
> -            } else {
> -                caches = &env->cache_info_cpuid4;
> -            }
> +            caches = &env->cache_info;
>          }
>  
>          /* cache info: needed for Core compatibility */
> @@ -7951,27 +7911,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          if (env->enable_legacy_vendor_cache) {
>              caches = &legacy_amd_cache_info;
>          } else {
> -            /*
> -             * FIXME: Temporarily select cache info model here based on
> -             * vendor, and merge these 2 cache info models later.
> -             *
> -             * This condition covers the following cases (with
> -             * enable_legacy_vendor_cache=false):
> -             *  - When CPU model has its own cache model and doesn't uses legacy
> -             *    cache model (legacy_model=off). Then cache_info_amd and
> -             *    cache_info_cpuid4 are the same.
> -             *
> -             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> -             *    model. AMD CPUs use cache_info_amd like before and non-AMD
> -             *    CPU will use cache_info_cpuid4. But this doesn't matter,
> -             *    because for Intel CPU, it will get all-0 leaf, and Zhaoxin CPU
> -             *    will get correct cache info. Both are expected.
> -             */
> -            if (IS_AMD_CPU(env)) {
> -                caches = &env->cache_info_amd;
> -            } else {
> -                caches = &env->cache_info_cpuid4;
> -            }
> +            caches = &env->cache_info;
>          }
>  
>          if (cpu->cache_info_passthrough) {
> @@ -7998,25 +7938,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          if (env->enable_legacy_vendor_cache) {
>              caches = &legacy_amd_cache_info;
>          } else {
> -            /*
> -             * FIXME: Temporarily select cache info model here based on
> -             * vendor, and merge these 2 cache info models later.
> -             *
> -             * This condition covers the following cases (with
> -             * enable_legacy_vendor_cache=false):
> -             *  - When CPU model has its own cache model and doesn't uses legacy
> -             *    cache model (legacy_model=off). Then cache_info_amd and
> -             *    cache_info_cpuid4 are the same.
> -             *
> -             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> -             *    model. AMD CPUs use cache_info_amd like before and non-AMD
> -             *    CPU (Intel & Zhaoxin) will use cache_info_cpuid4 as expected.
> -             */
> -            if (IS_AMD_CPU(env)) {
> -                caches = &env->cache_info_amd;
> -            } else {
> -                caches = &env->cache_info_cpuid4;
> -            }
> +            caches = &env->cache_info;
>          }
>  
>          if (cpu->cache_info_passthrough) {
> @@ -8089,22 +8011,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              *edx = 0;
>          }
>          break;
> -    case 0x8000001D: {
> -        const CPUCaches *caches;
> -
> -        /*
> -         * FIXME: Temporarily select cache info model here based on
> -         * vendor, and merge these 2 cache info models later.
> -         *
> -         * Intel doesn't support this leaf so that Intel Guests don't
> -         * have this leaf. This change is harmless to Intel CPUs.
> -         */
> -        if (IS_AMD_CPU(env)) {
> -            caches = &env->cache_info_amd;
> -        } else {
> -            caches = &env->cache_info_cpuid4;
> -        }
> -
> +    case 0x8000001D:
>          *eax = 0;
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, count, eax, ebx, ecx, edx);
> @@ -8112,19 +8019,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          }
>          switch (count) {
>          case 0: /* L1 dcache info */
> -            encode_cache_cpuid8000001d(caches->l1d_cache,
> +            encode_cache_cpuid8000001d(env->cache_info.l1d_cache,
>                                         topo_info, eax, ebx, ecx, edx);
>              break;
>          case 1: /* L1 icache info */
> -            encode_cache_cpuid8000001d(caches->l1i_cache,
> +            encode_cache_cpuid8000001d(env->cache_info.l1i_cache,
>                                         topo_info, eax, ebx, ecx, edx);
>              break;
>          case 2: /* L2 cache info */
> -            encode_cache_cpuid8000001d(caches->l2_cache,
> +            encode_cache_cpuid8000001d(env->cache_info.l2_cache,
>                                         topo_info, eax, ebx, ecx, edx);
>              break;
>          case 3: /* L3 cache info */
> -            encode_cache_cpuid8000001d(caches->l3_cache,
> +            encode_cache_cpuid8000001d(env->cache_info.l3_cache,
>                                         topo_info, eax, ebx, ecx, edx);
>              break;
>          default: /* end of info */
> @@ -8135,7 +8042,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              *edx &= CACHE_NO_INVD_SHARING | CACHE_INCLUSIVE;
>          }
>          break;
> -    }
>      case 0x8000001E:
>          if (cpu->core_id <= 255) {
>              encode_topo_cpuid8000001e(cpu, topo_info, eax, ebx, ecx, edx);
> @@ -8825,46 +8731,34 @@ static bool x86_cpu_update_smp_cache_topo(MachineState *ms, X86CPU *cpu,
>  
>      level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1D);
>      if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> -        env->cache_info_cpuid4.l1d_cache->share_level = level;
> -        env->cache_info_amd.l1d_cache->share_level = level;
> +        env->cache_info.l1d_cache->share_level = level;
>      } else {
>          machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1D,
> -            env->cache_info_cpuid4.l1d_cache->share_level);
> -        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1D,
> -            env->cache_info_amd.l1d_cache->share_level);
> +            env->cache_info.l1d_cache->share_level);
>      }
>  
>      level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1I);
>      if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> -        env->cache_info_cpuid4.l1i_cache->share_level = level;
> -        env->cache_info_amd.l1i_cache->share_level = level;
> +        env->cache_info.l1i_cache->share_level = level;
>      } else {
>          machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1I,
> -            env->cache_info_cpuid4.l1i_cache->share_level);
> -        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L1I,
> -            env->cache_info_amd.l1i_cache->share_level);
> +            env->cache_info.l1i_cache->share_level);
>      }
>  
>      level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L2);
>      if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> -        env->cache_info_cpuid4.l2_cache->share_level = level;
> -        env->cache_info_amd.l2_cache->share_level = level;
> +        env->cache_info.l2_cache->share_level = level;
>      } else {
>          machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L2,
> -            env->cache_info_cpuid4.l2_cache->share_level);
> -        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L2,
> -            env->cache_info_amd.l2_cache->share_level);
> +            env->cache_info.l2_cache->share_level);
>      }
>  
>      level = machine_get_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L3);
>      if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> -        env->cache_info_cpuid4.l3_cache->share_level = level;
> -        env->cache_info_amd.l3_cache->share_level = level;
> +        env->cache_info.l3_cache->share_level = level;
>      } else {
>          machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L3,
> -            env->cache_info_cpuid4.l3_cache->share_level);
> -        machine_set_cache_topo_level(ms, CACHE_LEVEL_AND_TYPE_L3,
> -            env->cache_info_amd.l3_cache->share_level);
> +            env->cache_info.l3_cache->share_level);
>      }
>  
>      if (!machine_check_smp_cache(ms, errp)) {
> @@ -9091,7 +8985,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>                         "CPU model '%s' doesn't support legacy-cache=off", name);
>              return;
>          }
> -        env->cache_info_cpuid4 = env->cache_info_amd = *cache_info;
> +        env->cache_info = *cache_info;
>      } else {
>          /* Build legacy cache information */
>          if (!cpu->consistent_cache) {
> @@ -9101,8 +8995,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          if (!cpu->vendor_cpuid_only_v2) {
>              env->enable_legacy_vendor_cache = true;
>          }
> -        env->cache_info_cpuid4 = legacy_intel_cache_info;
> -        env->cache_info_amd = legacy_amd_cache_info;
> +
> +        if (IS_AMD_CPU(env)) {
> +            env->cache_info = legacy_amd_cache_info;
> +        } else {
> +            env->cache_info = legacy_intel_cache_info;
> +        }
>      }
>  
>  #ifndef CONFIG_USER_ONLY
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 243383efd602..3f79db679888 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2072,11 +2072,12 @@ typedef struct CPUArchState {
>      /* Features that were explicitly enabled/disabled */
>      FeatureWordArray user_features;
>      uint32_t cpuid_model[12];
> -    /* Cache information for CPUID.  When legacy-cache=on, the cache data
> +    /*
> +     * Cache information for CPUID.  When legacy-cache=on, the cache data
>       * on each CPUID leaf will be different, because we keep compatibility
>       * with old QEMU versions.
>       */
> -    CPUCaches cache_info_cpuid4, cache_info_amd;
> +    CPUCaches cache_info;
>      bool enable_legacy_cpuid2_cache;
>      bool enable_legacy_vendor_cache;
>  

Nice clean-up patch series. Thanks.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Re: [PATCH 16/16] i386/cpu: Use a unified cache_info in X86CPUState

[Zhao Liu]

> Nice clean-up patch series. Thanks.
> 
> Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> 

Thanks for your review and effort!

Regards,
Zhao