[PATCH v2 0/4] Add support for Zhaoxin Yongfeng CPU model and other improvements

[PATCH v2 0/4] Add support for Zhaoxin Yongfeng CPU model and other improvements

[EwanHai]

### Summary of changes

EwanHai (4):
  target/i386: Add support for Zhaoxin CPU vendor identification
  target/i386: Add CPUID leaf 0xC000_0001 EDX definitions
  target/i386: Introduce Zhaoxin Yongfeng CPU model
  target/i386: Update CMPLegacy handling for Zhaoxin CPUs

 target/i386/cpu.c | 128 ++++++++++++++++++++++++++++++++++++++++++++--
 target/i386/cpu.h |  41 ++++++++++++++-
 2 files changed, 165 insertions(+), 4 deletions(-)

### Changes since v1
1. Removed VIA-related information from the patch description to avoid
misunderstanding.
2. Replaced CPUID_VENDOR_VIA with CPUID_VENDOR_ZHAOXIN1 because the
"Centaurhauls" vendor ID now belongs to Zhaoxin.The previous CPUID_VENDOR_VIA
macro was only defined but never used in QEMU, making this change
straightforward.

v1 link: https://lore.kernel.org/qemu-devel/20240625091905.1325205-1-ewanhai-
oc@zhaoxin.com/

### Known Issues
1. Issue with VMX Preemption Timer Rate on Yongfeng CPU:
   - Description: On Yongfeng CPUs, the VMX preemption timer rate is 128,
     meaning that bits 4:0 of MSR_IA32_VMX_MISC_CTLS should be set to 7.
     However, due to Intel's rate being 5, the Linux kernel has hardcoded
     this value as 5: `#define VMX_MISC_EMULATED_PREEMPTION_TIMER_RATE 5`.
   - Impact: This discrepancy can cause incorrect behavior in the VMX
     preemption timer on Yongfeng CPUs.
   - Workaround: A patch to correct this issue in the Linux kernel is
     currently being prepared and will be submitted soon.
-- 
2.34.1

[PATCH v2 1/4] target/i386: Add support for Zhaoxin CPU vendor identification

[EwanHai]

Zhaoxin currently uses two vendors: "Shanghai" and "Centaurhauls".
It is important to note that the latter now belongs to Zhaoxin. Therefore,
this patch replaces CPUID_VENDOR_VIA with CPUID_VENDOR_ZHAOXIN1.

The previous CPUID_VENDOR_VIA macro was only defined but never used in
QEMU, making this change straightforward.

Additionally, the IS_ZHAOXIN_CPU macro has been added to simplify the
checks for Zhaoxin CPUs.

Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
---
 target/i386/cpu.h | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c64ef0c1a2..07e8353f36 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1001,7 +1001,16 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
 #define CPUID_VENDOR_AMD_3   0x444d4163 /* "cAMD" */
 #define CPUID_VENDOR_AMD   "AuthenticAMD"
 
-#define CPUID_VENDOR_VIA   "CentaurHauls"
+#define CPUID_VENDOR_ZHAOXIN1_1 0x746E6543 /* "Cent" */
+#define CPUID_VENDOR_ZHAOXIN1_2 0x48727561 /* "aurH" */
+#define CPUID_VENDOR_ZHAOXIN1_3 0x736C7561 /* "auls" */
+
+#define CPUID_VENDOR_ZHAOXIN2_1 0x68532020 /* "  Sh" */
+#define CPUID_VENDOR_ZHAOXIN2_2 0x68676E61 /* "angh" */
+#define CPUID_VENDOR_ZHAOXIN2_3 0x20206961 /* "ai  " */
+
+#define CPUID_VENDOR_ZHAOXIN1   "CentaurHauls"
+#define CPUID_VENDOR_ZHAOXIN2   "  Shanghai  "
 
 #define CPUID_VENDOR_HYGON    "HygonGenuine"
 
@@ -1011,6 +1020,15 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
 #define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
                          (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
                          (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
+#define IS_ZHAOXIN1_CPU(env) \
+    ((env)->cpuid_vendor1 == CPUID_VENDOR_ZHAOXIN1_1 && \
+     (env)->cpuid_vendor2 == CPUID_VENDOR_ZHAOXIN1_2 && \
+     (env)->cpuid_vendor3 == CPUID_VENDOR_ZHAOXIN1_3)
+#define IS_ZHAOXIN2_CPU(env) \
+    ((env)->cpuid_vendor1 == CPUID_VENDOR_ZHAOXIN2_1 && \
+     (env)->cpuid_vendor2 == CPUID_VENDOR_ZHAOXIN2_2 && \
+     (env)->cpuid_vendor3 == CPUID_VENDOR_ZHAOXIN2_3)
+#define IS_ZHAOXIN_CPU(env) (IS_ZHAOXIN1_CPU(env) || IS_ZHAOXIN2_CPU(env))
 
 #define CPUID_MWAIT_IBE     (1U << 1) /* Interrupts can exit capability */
 #define CPUID_MWAIT_EMX     (1U << 0) /* enumeration supported */
-- 
2.34.1

Re: [PATCH v2 1/4] target/i386: Add support for Zhaoxin CPU vendor identification

[Zhao Liu]

On Thu, Jul 04, 2024 at 07:25:08AM -0400, EwanHai wrote:
> Date: Thu, 4 Jul 2024 07:25:08 -0400
> From: EwanHai <ewanhai-oc@zhaoxin.com>
> Subject: [PATCH v2 1/4] target/i386: Add support for Zhaoxin CPU vendor
>  identification
> X-Mailer: git-send-email 2.34.1
> 
> Zhaoxin currently uses two vendors: "Shanghai" and "Centaurhauls".
> It is important to note that the latter now belongs to Zhaoxin. Therefore,
> this patch replaces CPUID_VENDOR_VIA with CPUID_VENDOR_ZHAOXIN1.
> 
> The previous CPUID_VENDOR_VIA macro was only defined but never used in
> QEMU, making this change straightforward.
> 
> Additionally, the IS_ZHAOXIN_CPU macro has been added to simplify the
> checks for Zhaoxin CPUs.
> 
> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
> ---
>  target/i386/cpu.h | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)

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

[PATCH v2 2/4] target/i386: Add CPUID leaf 0xC000_0001 EDX definitions

[EwanHai]

Add new CPUID feature flags for various Zhaoxin PadLock extensions.
These definitions will be used for Zhaoxin CPU models.

Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
---
 target/i386/cpu.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 07e8353f36..935bf96451 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -989,6 +989,27 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
 /* CPUID[0x80000007].EDX flags: */
 #define CPUID_APM_INVTSC       (1U << 8)
 
+/* "rng" RNG present (xstore) */
+#define CPUID_C000_0001_EDX_XSTORE             (1U << 2)
+/* "rng_en" RNG enabled */
+#define CPUID_C000_0001_EDX_XSTORE_EN          (1U << 3)
+/* "ace" on-CPU crypto (xcrypt) */
+#define CPUID_C000_0001_EDX_XCRYPT             (1U << 6)
+/* "ace_en" on-CPU crypto enabled */
+#define CPUID_C000_0001_EDX_XCRYPT_EN          (1U << 7)
+/* Advanced Cryptography Engine v2 */
+#define CPUID_C000_0001_EDX_ACE2               (1U << 8)
+/* ACE v2 enabled */
+#define CPUID_C000_0001_EDX_ACE2_EN            (1U << 9)
+/* PadLock Hash Engine */
+#define CPUID_C000_0001_EDX_PHE                (1U << 10)
+/* PHE enabled */
+#define CPUID_C000_0001_EDX_PHE_EN             (1U << 11)
+/* PadLock Montgomery Multiplier */
+#define CPUID_C000_0001_EDX_PMM                (1U << 12)
+/* PMM enabled */
+#define CPUID_C000_0001_EDX_PMM_EN             (1U << 13)
+
 #define CPUID_VENDOR_SZ      12
 
 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
-- 
2.34.1

Re: [PATCH v2 2/4] target/i386: Add CPUID leaf 0xC000_0001 EDX definitions

[Zhao Liu]

On Thu, Jul 04, 2024 at 07:25:09AM -0400, EwanHai wrote:
> Date: Thu, 4 Jul 2024 07:25:09 -0400
> From: EwanHai <ewanhai-oc@zhaoxin.com>
> Subject: [PATCH v2 2/4] target/i386: Add CPUID leaf 0xC000_0001 EDX
>  definitions
> X-Mailer: git-send-email 2.34.1
> 
> Add new CPUID feature flags for various Zhaoxin PadLock extensions.
> These definitions will be used for Zhaoxin CPU models.
> 
> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
> ---
>  target/i386/cpu.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

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

[PATCH v2 3/4] target/i386: Introduce Zhaoxin Yongfeng CPU model

[EwanHai]

Introduce support for the Zhaoxin Yongfeng CPU model.
The Zhaoxin Yongfeng CPU is Zhaoxin's latest server CPU.

This new cpu model ensure that QEMU can correctly emulate the Zhaoxin
Yongfeng CPU, providing accurate functionality and performance characteristics.

Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
---
 target/i386/cpu.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 914bef442c..a3747fc487 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5211,6 +5211,130 @@ static const X86CPUDefinition builtin_x86_defs[] = {
         .model_id = "AMD EPYC-Genoa Processor",
         .cache_info = &epyc_genoa_cache_info,
     },
+    {
+        .name = "YongFeng",
+        .level = 0x1F,
+        .vendor = CPUID_VENDOR_ZHAOXIN1,
+        .family = 7,
+        .model = 11,
+        .stepping = 3,
+        /* missing: CPUID_HT, CPUID_TM, CPUID_PBE */
+        .features[FEAT_1_EDX] =
+            CPUID_SS | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+            CPUID_ACPI | CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV |
+            CPUID_MCA | CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC |
+            CPUID_CX8 | CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC |
+            CPUID_PSE | CPUID_DE | CPUID_VME | CPUID_FP87,
+        /*
+         * missing: CPUID_EXT_OSXSAVE, CPUID_EXT_XTPR, CPUID_EXT_TM2,
+         * CPUID_EXT_EST, CPUID_EXT_SMX, CPUID_EXT_VMX
+         */
+        .features[FEAT_1_ECX] =
+            CPUID_EXT_RDRAND | CPUID_EXT_F16C | CPUID_EXT_AVX |
+            CPUID_EXT_XSAVE | CPUID_EXT_AES | CPUID_EXT_TSC_DEADLINE_TIMER |
+            CPUID_EXT_POPCNT | CPUID_EXT_MOVBE | CPUID_EXT_X2APIC |
+            CPUID_EXT_SSE42 | CPUID_EXT_SSE41 | CPUID_EXT_PCID |
+            CPUID_EXT_CX16 | CPUID_EXT_FMA | CPUID_EXT_SSSE3 |
+            CPUID_EXT_MONITOR | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
+        .features[FEAT_7_0_EBX] =
+            CPUID_7_0_EBX_SHA_NI | CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_ADX |
+            CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_INVPCID | CPUID_7_0_EBX_BMI2 |
+            CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_BMI1 |
+            CPUID_7_0_EBX_FSGSBASE,
+        /* missing: CPUID_7_0_ECX_OSPKE */
+        .features[FEAT_7_0_ECX] =
+            CPUID_7_0_ECX_RDPID | CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_UMIP,
+        .features[FEAT_7_0_EDX] =
+            CPUID_7_0_EDX_ARCH_CAPABILITIES | CPUID_7_0_EDX_SPEC_CTRL,
+        .features[FEAT_8000_0001_EDX] =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_PDPE1GB |
+            CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
+        .features[FEAT_8000_0001_ECX] =
+            CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM,
+        .features[FEAT_8000_0007_EDX] = CPUID_APM_INVTSC,
+        /*
+         * TODO: When the Linux kernel introduces other existing definitions
+         * for this leaf, remember to update the definitions here.
+         */
+        .features[FEAT_C000_0001_EDX] =
+            CPUID_C000_0001_EDX_PMM_EN | CPUID_C000_0001_EDX_PMM |
+            CPUID_C000_0001_EDX_PHE_EN | CPUID_C000_0001_EDX_PHE |
+            CPUID_C000_0001_EDX_ACE2 |
+            CPUID_C000_0001_EDX_XCRYPT_EN | CPUID_C000_0001_EDX_XCRYPT |
+            CPUID_C000_0001_EDX_XSTORE_EN | CPUID_C000_0001_EDX_XSTORE,
+        .features[FEAT_XSAVE] =
+            CPUID_XSAVE_XSAVEOPT,
+        .features[FEAT_ARCH_CAPABILITIES] =
+            MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY |
+            MSR_ARCH_CAP_MDS_NO | MSR_ARCH_CAP_PSCHANGE_MC_NO |
+            MSR_ARCH_CAP_SSB_NO,
+        .features[FEAT_VMX_PROCBASED_CTLS] =
+            VMX_CPU_BASED_VIRTUAL_INTR_PENDING | VMX_CPU_BASED_HLT_EXITING |
+            VMX_CPU_BASED_USE_TSC_OFFSETING | VMX_CPU_BASED_INVLPG_EXITING |
+            VMX_CPU_BASED_MWAIT_EXITING | VMX_CPU_BASED_RDPMC_EXITING |
+            VMX_CPU_BASED_RDTSC_EXITING | VMX_CPU_BASED_CR3_LOAD_EXITING |
+            VMX_CPU_BASED_CR3_STORE_EXITING | VMX_CPU_BASED_CR8_LOAD_EXITING |
+            VMX_CPU_BASED_CR8_STORE_EXITING | VMX_CPU_BASED_TPR_SHADOW |
+            VMX_CPU_BASED_VIRTUAL_NMI_PENDING | VMX_CPU_BASED_MOV_DR_EXITING |
+            VMX_CPU_BASED_UNCOND_IO_EXITING | VMX_CPU_BASED_USE_IO_BITMAPS |
+            VMX_CPU_BASED_MONITOR_TRAP_FLAG | VMX_CPU_BASED_USE_MSR_BITMAPS |
+            VMX_CPU_BASED_MONITOR_EXITING | VMX_CPU_BASED_PAUSE_EXITING |
+            VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS,
+        /*
+         * missing: VMX_SECONDARY_EXEC_PAUSE_LOOP_EXITING,
+         * VMX_SECONDARY_EXEC_TSC_SCALING
+         */
+        .features[FEAT_VMX_SECONDARY_CTLS] =
+            VMX_SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
+            VMX_SECONDARY_EXEC_ENABLE_EPT | VMX_SECONDARY_EXEC_DESC |
+            VMX_SECONDARY_EXEC_RDTSCP | VMX_SECONDARY_EXEC_ENABLE_VPID |
+            VMX_SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
+            VMX_SECONDARY_EXEC_WBINVD_EXITING |
+            VMX_SECONDARY_EXEC_UNRESTRICTED_GUEST |
+            VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT |
+            VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
+            VMX_SECONDARY_EXEC_RDRAND_EXITING |
+            VMX_SECONDARY_EXEC_ENABLE_INVPCID |
+            VMX_SECONDARY_EXEC_ENABLE_VMFUNC |
+            VMX_SECONDARY_EXEC_SHADOW_VMCS |
+            VMX_SECONDARY_EXEC_ENABLE_PML,
+        .features[FEAT_VMX_PINBASED_CTLS] =
+            VMX_PIN_BASED_EXT_INTR_MASK | VMX_PIN_BASED_NMI_EXITING |
+            VMX_PIN_BASED_VIRTUAL_NMIS | VMX_PIN_BASED_VMX_PREEMPTION_TIMER |
+            VMX_PIN_BASED_POSTED_INTR,
+        .features[FEAT_VMX_EXIT_CTLS] =
+            VMX_VM_EXIT_SAVE_DEBUG_CONTROLS | VMX_VM_EXIT_HOST_ADDR_SPACE_SIZE |
+            VMX_VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |
+            VMX_VM_EXIT_ACK_INTR_ON_EXIT | VMX_VM_EXIT_SAVE_IA32_PAT |
+            VMX_VM_EXIT_LOAD_IA32_PAT | VMX_VM_EXIT_SAVE_IA32_EFER |
+            VMX_VM_EXIT_LOAD_IA32_EFER | VMX_VM_EXIT_SAVE_VMX_PREEMPTION_TIMER,
+        /* missing: VMX_VM_ENTRY_SMM, VMX_VM_ENTRY_DEACT_DUAL_MONITOR */
+        .features[FEAT_VMX_ENTRY_CTLS] =
+            VMX_VM_ENTRY_LOAD_DEBUG_CONTROLS | VMX_VM_ENTRY_IA32E_MODE |
+            VMX_VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL |
+            VMX_VM_ENTRY_LOAD_IA32_PAT | VMX_VM_ENTRY_LOAD_IA32_EFER,
+        /*
+         * missing: MSR_VMX_MISC_ACTIVITY_SHUTDOWN,
+         * MSR_VMX_MISC_ACTIVITY_WAIT_SIPI
+         */
+        .features[FEAT_VMX_MISC] =
+            MSR_VMX_MISC_STORE_LMA | MSR_VMX_MISC_ACTIVITY_HLT |
+            MSR_VMX_MISC_VMWRITE_VMEXIT,
+        /* missing: MSR_VMX_EPT_UC */
+        .features[FEAT_VMX_EPT_VPID_CAPS] =
+            MSR_VMX_EPT_EXECONLY | MSR_VMX_EPT_PAGE_WALK_LENGTH_4 |
+            MSR_VMX_EPT_WB | MSR_VMX_EPT_2MB | MSR_VMX_EPT_1GB |
+            MSR_VMX_EPT_INVEPT | MSR_VMX_EPT_AD_BITS |
+            MSR_VMX_EPT_INVEPT_SINGLE_CONTEXT | MSR_VMX_EPT_INVEPT_ALL_CONTEXT |
+            MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT | MSR_VMX_EPT_INVVPID |
+            MSR_VMX_EPT_INVVPID_ALL_CONTEXT | MSR_VMX_EPT_INVVPID_SINGLE_ADDR |
+            MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT_NOGLOBALS,
+        .features[FEAT_VMX_BASIC] =
+            MSR_VMX_BASIC_INS_OUTS | MSR_VMX_BASIC_TRUE_CTLS,
+        .features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
+        .xlevel = 0x80000008,
+        .model_id = "Zhaoxin YongFeng Processor",
+    },
 };
 
 /*
-- 
2.34.1

Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Yongfeng CPU model

[Zhao Liu]

On Thu, Jul 04, 2024 at 07:25:10AM -0400, EwanHai wrote:
> Date: Thu, 4 Jul 2024 07:25:10 -0400
> From: EwanHai <ewanhai-oc@zhaoxin.com>
> Subject: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Yongfeng CPU model
> X-Mailer: git-send-email 2.34.1
> 
> Introduce support for the Zhaoxin Yongfeng CPU model.
> The Zhaoxin Yongfeng CPU is Zhaoxin's latest server CPU.
> 
> This new cpu model ensure that QEMU can correctly emulate the Zhaoxin
> Yongfeng CPU, providing accurate functionality and performance characteristics.
> 
> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
> ---
>  target/i386/cpu.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
 
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin CPUs

[EwanHai]

Zhaoxin CPUs handle the CMPLegacy bit in the same way
as Intel CPUs. This patch simplifies the existing logic by
using the IS_XXX_CPU macro and includes checks for Zhaoxin
vendor to align their behavior with Intel.

Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
---
 target/i386/cpu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a3747fc487..c52a4cf3ba 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6945,9 +6945,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
          * So don't set it here for Intel to make Linux guests happy.
          */
         if (threads_per_pkg > 1) {
-            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
-                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
-                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
+            if (!IS_INTEL_CPU(env) && !IS_ZHAOXIN_CPU(env)) {
                 *ecx |= 1 << 1;    /* CmpLegacy bit */
             }
         }
-- 
2.34.1

Re: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin CPUs

[Zhao Liu]

Hi EwanHai,

On Thu, Jul 04, 2024 at 07:25:11AM -0400, EwanHai wrote:
> Date: Thu, 4 Jul 2024 07:25:11 -0400
> From: EwanHai <ewanhai-oc@zhaoxin.com>
> Subject: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin
>  CPUs
> X-Mailer: git-send-email 2.34.1
> 
> Zhaoxin CPUs handle the CMPLegacy bit in the same way
> as Intel CPUs. This patch simplifies the existing logic by
> using the IS_XXX_CPU macro and includes checks for Zhaoxin
> vendor to align their behavior with Intel.
> 
> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
> ---
>  target/i386/cpu.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a3747fc487..c52a4cf3ba 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6945,9 +6945,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>           * So don't set it here for Intel to make Linux guests happy.
>           */
>          if (threads_per_pkg > 1) {
> -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
> -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
> -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
> +            if (!IS_INTEL_CPU(env) && !IS_ZHAOXIN_CPU(env)) {

This change implicitly changes the behavior of existing VIA CPU.

Is this a bug for the original VIA? If so, I suggest a separate patch to
fix it and explain the effect on the VIA (Zhaoxin1) CPU.

Regards,
Zhao

>                  *ecx |= 1 << 1;    /* CmpLegacy bit */
>              }
>          }
> -- 
> 2.34.1
> 

Re: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin CPUs

[Ewan Hai]

Hi Zhao Liu,

Thank you for your feedback.

On 8/8/24 06:30, Zhao Liu wrote:
> Hi EwanHai,
>
> On Thu, Jul 04, 2024 at 07:25:11AM -0400, EwanHai wrote:
>> Date: Thu, 4 Jul 2024 07:25:11 -0400
>> From: EwanHai <ewanhai-oc@zhaoxin.com>
>> Subject: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin
>>   CPUs
>> X-Mailer: git-send-email 2.34.1
>>
>> Zhaoxin CPUs handle the CMPLegacy bit in the same way
>> as Intel CPUs. This patch simplifies the existing logic by
>> using the IS_XXX_CPU macro and includes checks for Zhaoxin
>> vendor to align their behavior with Intel.
>>
>> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
>> ---
>>   target/i386/cpu.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index a3747fc487..c52a4cf3ba 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -6945,9 +6945,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>>            * So don't set it here for Intel to make Linux guests happy.
>>            */
>>           if (threads_per_pkg > 1) {
>> -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
>> -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
>> -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
>> +            if (!IS_INTEL_CPU(env) && !IS_ZHAOXIN_CPU(env)) {
> This change implicitly changes the behavior of existing VIA CPU.
>
> Is this a bug for the original VIA? If so, I suggest a separate patch to
> fix it and explain the effect on the VIA (Zhaoxin1) CPU.
>
> Regards,
> Zhao
The reason for this change is not due to a discovered bug, but rather
because both Centaurhauls and Shanghai CPUs follow Intel’s behavior
regarding the CMPLegacy bit. Specifically, AMD CPUs enumerate the
threads per package information in the CPUID leaf 0x80000001 output
ECX register, while Intel (and **other processors following Intel’s
behavior**) do not. Therefore, this modification is simply intended to
logically supplement the existing code.

Given this, do you think it would be appropriate for me to submit
a separate patch to explain this behavior and its effect on
VIA (Zhaoxin1) CPUs? If so, I will submmit this change in a separate
patch.
>>                   *ecx |= 1 << 1;    /* CmpLegacy bit */
>>               }
>>           }
>> --
>> 2.34.1
>>

Re: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin CPUs

[Zhao Liu]

On Thu, Aug 08, 2024 at 09:44:18PM -0400, Ewan Hai wrote:
> Date: Thu, 8 Aug 2024 21:44:18 -0400
> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
> Subject: Re: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for
>  Zhaoxin CPUs
> 
> 
> Hi Zhao Liu,
> 
> Thank you for your feedback.
> 
> On 8/8/24 06:30, Zhao Liu wrote:
> > Hi EwanHai,
> > 
> > On Thu, Jul 04, 2024 at 07:25:11AM -0400, EwanHai wrote:
> > > Date: Thu, 4 Jul 2024 07:25:11 -0400
> > > From: EwanHai <ewanhai-oc@zhaoxin.com>
> > > Subject: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin
> > >   CPUs
> > > X-Mailer: git-send-email 2.34.1
> > > 
> > > Zhaoxin CPUs handle the CMPLegacy bit in the same way
> > > as Intel CPUs.

Here it could be clearer to say "Don't set up CMPLegacy bit in
CPUID[0x80000001].ecx for VIA/Zhaoxin CPUs".

> This patch simplifies the existing logic by
> > > using the IS_XXX_CPU macro and includes checks for Zhaoxin
> > > vendor to align their behavior with Intel.
> > > 
> > > Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
> > > ---
> > >   target/i386/cpu.c | 4 +---
> > >   1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index a3747fc487..c52a4cf3ba 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -6945,9 +6945,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > >            * So don't set it here for Intel to make Linux guests happy.
> > >            */
> > >           if (threads_per_pkg > 1) {
> > > -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
> > > -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
> > > -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
> > > +            if (!IS_INTEL_CPU(env) && !IS_ZHAOXIN_CPU(env)) {
> > This change implicitly changes the behavior of existing VIA CPU.
> > 
> > Is this a bug for the original VIA? If so, I suggest a separate patch to
> > fix it and explain the effect on the VIA (Zhaoxin1) CPU.
> > 
> > Regards,
> > Zhao
>
> The reason for this change is not due to a discovered bug, but rather
> because both Centaurhauls and Shanghai CPUs follow Intel’s behavior
> regarding the CMPLegacy bit. Specifically, AMD CPUs enumerate the
> threads per package information in the CPUID leaf 0x80000001 output
> ECX register, while Intel (and **other processors following Intel’s
> behavior**) do not. Therefore, this modification is simply intended to
> logically supplement the existing code.

I see, thanks.

> Given this, do you think it would be appropriate for me to submit
> a separate patch to explain this behavior and its effect on
> VIA (Zhaoxin1) CPUs? If so, I will submmit this change in a separate
> patch.

I think there's no need to split this.

However, I think it's necessary to state the effect of the change in
the changelog/commit message. It's also worth stating if it won't have
any effect on the OS/software. Afterall, the comment of this bit said
it affects Linux kernel.

Also, changes to the old VIA behavior are worth stating in the commit
message, i.e., this patch's changes to Zhaoxin CPUs include the previous
VIA CPUs.

Additionally, considering this change is to fix the CPUID which doesn't
match the bare metal, then what about changing the subject to

"target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ecx for Zhaoxin/VIA
CPUs"?

Thanks,
Zhao

> > >                   *ecx |= 1 << 1;    /* CmpLegacy bit */
> > >               }
> > >           }
> > > --
> > > 2.34.1
> > > 
> 

Re: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin CPUs

[Ewan Hai]

On 8/8/24 23:22, Zhao Liu wrote:
>>> Hi EwanHai,
>>>
>>> On Thu, Jul 04, 2024 at 07:25:11AM -0400, EwanHai wrote:
>>>> Date: Thu, 4 Jul 2024 07:25:11 -0400
>>>> From: EwanHai <ewanhai-oc@zhaoxin.com>
>>>> Subject: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin
>>>>    CPUs
>>>> X-Mailer: git-send-email 2.34.1
>>>>
>>>> Zhaoxin CPUs handle the CMPLegacy bit in the same way
>>>> as Intel CPUs.
> Here it could be clearer to say "Don't set up CMPLegacy bit in
> CPUID[0x80000001].ecx for VIA/Zhaoxin CPUs".
Ok, I will change this statement.
>> This patch simplifies the existing logic by
>>>> using the IS_XXX_CPU macro and includes checks for Zhaoxin
>>>> vendor to align their behavior with Intel.
>>>>
>>>> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
>>>> ---
>>>>    target/i386/cpu.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>>> index a3747fc487..c52a4cf3ba 100644
>>>> --- a/target/i386/cpu.c
>>>> +++ b/target/i386/cpu.c
>>>> @@ -6945,9 +6945,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>>>>             * So don't set it here for Intel to make Linux guests happy.
>>>>             */
>>>>            if (threads_per_pkg > 1) {
>>>> -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
>>>> -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
>>>> -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
>>>> +            if (!IS_INTEL_CPU(env) && !IS_ZHAOXIN_CPU(env)) {
>>> This change implicitly changes the behavior of existing VIA CPU.
>>>
>>> Is this a bug for the original VIA? If so, I suggest a separate patch to
>>> fix it and explain the effect on the VIA (Zhaoxin1) CPU.
>>>
>>> Regards,
>>> Zhao
>> The reason for this change is not due to a discovered bug, but rather
>> because both Centaurhauls and Shanghai CPUs follow Intel’s behavior
>> regarding the CMPLegacy bit. Specifically, AMD CPUs enumerate the
>> threads per package information in the CPUID leaf 0x80000001 output
>> ECX register, while Intel (and **other processors following Intel’s
>> behavior**) do not. Therefore, this modification is simply intended to
>> logically supplement the existing code.
> I see, thanks.
>
>> Given this, do you think it would be appropriate for me to submit
>> a separate patch to explain this behavior and its effect on
>> VIA (Zhaoxin1) CPUs? If so, I will submmit this change in a separate
>> patch.
> I think there's no need to split this.
>
> However, I think it's necessary to state the effect of the change in
> the changelog/commit message. It's also worth stating if it won't have
> any effect on the OS/software. Afterall, the comment of this bit said
> it affects Linux kernel.
>
> Also, changes to the old VIA behavior are worth stating in the commit
> message, i.e., this patch's changes to Zhaoxin CPUs include the previous
> VIA CPUs.
>
> Additionally, considering this change is to fix the CPUID which doesn't
> match the bare metal, then what about changing the subject to
>
> "target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ecx for Zhaoxin/VIA
> CPUs"?
>
> Thanks,
> Zhao
Thank you for your suggestion; the changes will indeed make it clearer.
I have a question: since you’ve already added your reviewed-by tag to
the first three patches, if I want to modify these descriptions, should
I submit a v3 patchset containing all four patches, or should I only send a
new patch titled "target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ecx
for Zhaoxin/Centaur CPUs"?
>>>>                    *ecx |= 1 << 1;    /* CmpLegacy bit */
>>>>                }
>>>>            }
>>>> --
>>>> 2.34.1
>>>>

Re: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin CPUs

[Zhao Liu]

On Thu, Aug 08, 2024 at 11:25:45PM -0400, Ewan Hai wrote:

[snip]

> Thank you for your suggestion; the changes will indeed make it clearer.
> I have a question: since you’ve already added your reviewed-by tag to
> the first three patches, if I want to modify these descriptions, should
> I submit a v3 patchset containing all four patches, or should I only send a
> new patch titled "target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ecx
> for Zhaoxin/Centaur CPUs"?

The v3 should contain all 4 patches, and you can add my R/b tag in the
first three patches.

Re: [PATCH v2 4/4] target/i386: Update CMPLegacy handling for Zhaoxin CPUs

[Ewan Hai]

On 8/8/24 23:47, Zhao Liu wrote:
> On Thu, Aug 08, 2024 at 11:25:45PM -0400, Ewan Hai wrote:
>
> [snip]
>
>> Thank you for your suggestion; the changes will indeed make it clearer.
>> I have a question: since you’ve already added your reviewed-by tag to
>> the first three patches, if I want to modify these descriptions, should
>> I submit a v3 patchset containing all four patches, or should I only send a
>> new patch titled "target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ecx
>> for Zhaoxin/Centaur CPUs"?
> The v3 should contain all 4 patches, and you can add my R/b tag in the
> first three patches.
>
Thanks! See you in v3!

Re: [PATCH v2 0/4] Add support for Zhaoxin Yongfeng CPU model and other improvements

[Ewan Hai]

Dear Maintainers,

     Could you please review the current patchset and let me know if you 
have any concerns?


On 7/4/24 07:25, EwanHai wrote:
> ### Summary of changes
>
> EwanHai (4):
>    target/i386: Add support for Zhaoxin CPU vendor identification
>    target/i386: Add CPUID leaf 0xC000_0001 EDX definitions
>    target/i386: Introduce Zhaoxin Yongfeng CPU model
>    target/i386: Update CMPLegacy handling for Zhaoxin CPUs
>
>   target/i386/cpu.c | 128 ++++++++++++++++++++++++++++++++++++++++++++--
>   target/i386/cpu.h |  41 ++++++++++++++-
>   2 files changed, 165 insertions(+), 4 deletions(-)
>
> ### Changes since v1
> 1. Removed VIA-related information from the patch description to avoid
> misunderstanding.
> 2. Replaced CPUID_VENDOR_VIA with CPUID_VENDOR_ZHAOXIN1 because the
> "Centaurhauls" vendor ID now belongs to Zhaoxin.The previous CPUID_VENDOR_VIA
> macro was only defined but never used in QEMU, making this change
> straightforward.
>
> v1 link: https://lore.kernel.org/qemu-devel/20240625091905.1325205-1-ewanhai-
> oc@zhaoxin.com/
>
> ### Known Issues
> 1. Issue with VMX Preemption Timer Rate on Yongfeng CPU:
>     - Description: On Yongfeng CPUs, the VMX preemption timer rate is 128,
>       meaning that bits 4:0 of MSR_IA32_VMX_MISC_CTLS should be set to 7.
>       However, due to Intel's rate being 5, the Linux kernel has hardcoded
>       this value as 5: `#define VMX_MISC_EMULATED_PREEMPTION_TIMER_RATE 5`.
>     - Impact: This discrepancy can cause incorrect behavior in the VMX
>       preemption timer on Yongfeng CPUs.
>     - Workaround: A patch to correct this issue in the Linux kernel is
>       currently being prepared and will be submitted soon.