Re: [PATCH V3 12/13] target/i386: Clean up Intel Debug Store feature dependencies

[Xiaoyao Li]

On 3/16/2026 11:21 AM, Chenyi Qiang wrote:


On 3/5/2026 2:07 AM, Zide Chen wrote:
- 64-bit DS Area (CPUID.01H:ECX[2]) depends on DS (CPUID.01H:EDX[21]).
- When PMU is disabled, Debug Store must not be exposed to the guest,
   which implicitly disables legacy DS-based PEBS.

Signed-off-by: Zide Chen <zide.c...@intel.com>
---
V3:
- Update title to be more accurate.
- Make DTES64 depend on DS.
- Mark MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL in previous patch.
- Clean up the commit message.

V2: New patch.
---
  target/i386/cpu.c | 5 +++++
  1 file changed, 5 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2e1dea65d708..3ff9f76cf7da 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1899,6 +1899,10 @@ static FeatureDep feature_dependencies[] = {
          .from = { FEAT_1_ECX,             CPUID_EXT_PDCM },
          .to = { FEAT_PERF_CAPABILITIES,       ~0ull },
      },
+    {
+        .from = { FEAT_1_EDX,               CPUID_DTS},
+        .to = { FEAT_1_ECX,                 CPUID_EXT_DTES64},
+    },
      {
          .from = { FEAT_1_ECX,               CPUID_EXT_VMX },
          .to = { FEAT_VMX_PROCBASED_CTLS,    ~0ull },
@@ -9471,6 +9475,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
              env->features[FEAT_1_ECX] &= ~CPUID_EXT_PDCM;
          }
  
+        env->features[FEAT_1_EDX] &= ~CPUID_DTS;
          env->features[FEAT_7_0_EDX] &= ~CPUID_7_0_EDX_ARCH_LBR;

This change, along with the original CPUID_7_0_EDX_ARCH_LBR clear, will also 
affect the configuration for TD VMs.
For a TD VM, enable_pmu controls TDX_TD_ATTRIBUTES_PERFMON, CPUID_DTS is fixed 
to 1, and arch_lbr is controlled by XFAM[15].
These features' configuration do not have dependencies on each other. So how 
about skipping the TD VM case like:

I think the dependency between enable_pmu and ARCH_LBR still applies to 
TDX. This dependency is defined by QEMU that enable_pmu controls all the 
PMU features, including ARCH_LBR. So we can enforce the rule of 
"XFAM[15] cannot be 1 when enable_pmu == 0"

For CPUID_DTS, it seems OK to expose it even when PMU is disabled?
I sort of disagree with the statement in the changelog:

  - When PMU is disabled, Debug Store must not be exposed to the guest,
    which implicitly disables legacy DS-based PEBS

If I read SDM correctly, the availability of legacy PEBS can be 
enumerated by MSR_IA32_MISC_ENABLE.PEBS_UNAVAILABLE bit. And from the 
linux code, it also proves that DTS can be 1 while PEBS is 0:

        if (boot_cpu_has(X86_FEATURE_DS)) {
                unsigned int l1, l2;

                rdmsr(MSR_IA32_MISC_ENABLE, l1, l2);
                if (!(l1 & MSR_IA32_MISC_ENABLE_BTS_UNAVAIL))
                        set_cpu_cap(c, X86_FEATURE_BTS);
                if (!(l1 & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL))
                        set_cpu_cap(c, X86_FEATURE_PEBS);
        }

Since it will need nasty handling for TDX case, I would vote not 
clearing CPUID_DTS here when !PMU, unless a strong reason is provided.

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 98e95d0842..458bfb07b9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -9736,8 +9736,10 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
              env->features[FEAT_1_ECX] &= ~CPUID_EXT_PDCM;
          }

-        env->features[FEAT_1_EDX] &= ~CPUID_DTS;
-        env->features[FEAT_7_0_EDX] &= ~CPUID_7_0_EDX_ARCH_LBR;
+        if (!is_tdx_vm()) {
+            env->features[FEAT_1_EDX] &= ~CPUID_DTS;
+            env->features[FEAT_7_0_EDX] &= ~CPUID_7_0_EDX_ARCH_LBR;
+        }
      }

      for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {



      }