From: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
To: "Zhao Liu" <zhao1.liu@intel.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Marcelo Tosatti" <mtosatti@redhat.com>,
"Michael S . Tsirkin" <mst@redhat.com>,
"Daniel P . Berrangé" <berrange@redhat.com>,
"Igor Mammedov" <imammedo@redhat.com>,
"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Eduardo Habkost" <eduardo@habkost.net>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Babu Moger" <babu.moger@amd.com>,
"Ewan Hai" <ewanhai-oc@zhaoxin.com>, "Pu Wen" <puwen@hygon.cn>,
"Tao Su" <tao1.su@intel.com>, "Yi Lai" <yi1.lai@intel.com>,
"Dapeng Mi" <dapeng1.mi@intel.com>,
qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [PATCH 16/16] i386/cpu: Use a unified cache_info in X86CPUState
Date: Thu, 3 Jul 2025 16:53:50 +0800 [thread overview]
Message-ID: <3d1f5698-1936-4fc0-af04-db900f0d1b9e@linux.intel.com> (raw)
In-Reply-To: <20250620092734.1576677-17-zhao1.liu@intel.com>
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>
next prev parent reply other threads:[~2025-07-03 8:53 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-20 9:27 [PATCH 00/16] i386/cpu: Unify the cache model in X86CPUState Zhao Liu
2025-06-20 9:27 ` [PATCH 01/16] i386/cpu: Refine comment of CPUID2CacheDescriptorInfo Zhao Liu
2025-07-02 8:48 ` Mi, Dapeng
2025-07-03 7:38 ` Zhao Liu
2025-06-20 9:27 ` [PATCH 02/16] i386/cpu: Add descriptor 0x49 for CPUID 0x2 encoding Zhao Liu
2025-07-02 9:04 ` Mi, Dapeng
2025-07-03 7:39 ` Zhao Liu
2025-06-20 9:27 ` [PATCH 03/16] i386/cpu: Add default cache model for Intel CPUs with level < 4 Zhao Liu
2025-07-02 9:53 ` Mi, Dapeng
2025-07-03 7:47 ` Zhao Liu
2025-06-20 9:27 ` [PATCH 04/16] i386/cpu: Present same cache model in CPUID 0x2 & 0x4 Zhao Liu
2025-07-03 4:14 ` Mi, Dapeng
2025-07-03 6:35 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 05/16] i386/cpu: Consolidate CPUID 0x4 leaf Zhao Liu
2025-06-26 12:10 ` Ewan Hai
2025-06-27 2:44 ` Zhao Liu
2025-07-03 6:41 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 06/16] i386/cpu: Drop CPUID 0x2 specific cache info in X86CPUState Zhao Liu
2025-07-03 7:03 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 07/16] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel Zhao Liu
2025-07-03 7:07 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 08/16] i386/cpu: Fix CPUID[0x80000006] for Intel CPU Zhao Liu
2025-07-03 7:09 ` Mi, Dapeng
2025-07-03 7:52 ` Zhao Liu
2025-06-20 9:27 ` [PATCH 09/16] i386/cpu: Add legacy_intel_cache_info cache model Zhao Liu
2025-07-03 7:15 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 10/16] i386/cpu: Add legacy_amd_cache_info " Zhao Liu
2025-07-03 7:18 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 11/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x2 Zhao Liu
2025-07-03 8:47 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 12/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x4 Zhao Liu
2025-07-03 8:49 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 13/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x80000005 Zhao Liu
2025-07-03 8:52 ` Mi, Dapeng
2025-06-20 9:27 ` [PATCH 14/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x80000006 Zhao Liu
2025-06-20 9:27 ` [PATCH 15/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x8000001D Zhao Liu
2025-06-20 9:27 ` [PATCH 16/16] i386/cpu: Use a unified cache_info in X86CPUState Zhao Liu
2025-07-03 8:53 ` Mi, Dapeng [this message]
2025-07-03 9:50 ` Zhao Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3d1f5698-1936-4fc0-af04-db900f0d1b9e@linux.intel.com \
--to=dapeng1.mi@linux.intel.com \
--cc=babu.moger@amd.com \
--cc=berrange@redhat.com \
--cc=dapeng1.mi@intel.com \
--cc=eduardo@habkost.net \
--cc=ewanhai-oc@zhaoxin.com \
--cc=imammedo@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=marcel.apfelbaum@gmail.com \
--cc=mst@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@linaro.org \
--cc=puwen@hygon.cn \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=tao1.su@intel.com \
--cc=yi1.lai@intel.com \
--cc=zhao1.liu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.