[PATCH v2] xen/arm: Hide PMU registers from the guest, when the vPMU feature is disabled.

Hirokazu Takahashi <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <[email protected]>
On ARMv8.4-A and newer platforms, booting Dom0 Linux with ACPI enabled
causes the domain to probe advanced PMU feature based on system ID
register ID_AA64DFR0_EL1.During this probe, Linux accesses PMMIR_EL1,
which causes unhandled register traps and crashes the domain.

To address this issue, I implement the following:

- Hide PMU registers from a guest domain when its vPMU feature is
  disabled.
- Proactively make SPE, TRBE, BRBE, and Trace Extensions inaccessible
  to guest domains, as they could potentially cause similar issues.
- Add emulation for PMMIR_EL1 register accesses performed by a guest
  domain when vPMU is enabled. However, similar to reads from other
  PMU registers, the read value returns zero (note that this is a
  temporary implementation).
- Emulation for PMSS (PMU Snapshot) register accesses is not yet
  implemented, because PMSS support is not available in
  qemu-system-aarch64 and could not be verified.

Fixes: 07b9acea116e "xen/arm: Add handler for ID registers on arm64"
Fixes: 3669a1cb9598 "xen/arm: create a cpuinfo structure for guest"
Signed-off-by: Hirokazu Takahashi <[email protected]>
---
Changes in v2:
 * Instead of unconditionally hiding the PMU feature from guests,
   we now determine whether to expose PMU to a guest domain based on
   its configuration.

 xen/arch/arm/arm64/vsysreg.c          | 31 +++++++++++++++++++++++++--
 xen/arch/arm/cpufeature.c             |  8 +++++++
 xen/arch/arm/include/asm/arm64/hsr.h  |  1 +
 xen/arch/arm/include/asm/cpufeature.h | 14 ++++++------
 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index d14258290f..520faa02ca 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -229,6 +229,7 @@ void do_sysreg(struct cpu_user_regs *regs,
      */
     case HSR_SYSREG_PMINTENSET_EL1:
     case HSR_SYSREG_PMINTENCLR_EL1:
+    case HSR_SYSREG_PMMIR_EL1:
         /*
          * Accessible from EL1 only, but if EL0 trap happens handle as
          * undef.
@@ -306,7 +307,6 @@ void do_sysreg(struct cpu_user_regs *regs,
     GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
     GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
     GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
-    GENERATE_TID3_INFO(ID_DFR0_EL1, dbg32, 0)
     GENERATE_TID3_INFO(ID_DFR1_EL1, dbg32, 1)
     GENERATE_TID3_INFO(ID_AFR0_EL1, aux32, 0)
     GENERATE_TID3_INFO(ID_MMFR0_EL1, mm32, 0)
@@ -326,6 +326,18 @@ void do_sysreg(struct cpu_user_regs *regs,
     GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1)
     GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2)
 
+    case HSR_SYSREG_ID_DFR0_EL1:
+    {
+        struct domain *d = current->domain;
+        union cpuinfo_dbg32 info_dbg32 = domain_cpuinfo.dbg32;
+
+        if ( !(d->options & XEN_DOMCTL_CDF_vpmu) )
+            info_dbg32.perfmon = 0;
+
+        return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
+                                  info_dbg32.bits[0]);
+    }
+
     case HSR_SYSREG_ID_AA64PFR0_EL1:
     {
         register_t guest_reg_value = domain_cpuinfo.pfr64.bits[0];
@@ -348,7 +360,6 @@ void do_sysreg(struct cpu_user_regs *regs,
     }
 
     GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1)
-    GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0)
     GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1)
     GENERATE_TID3_INFO(ID_AA64ISAR0_EL1, isa64, 0)
     GENERATE_TID3_INFO(ID_AA64ISAR1_EL1, isa64, 1)
@@ -358,6 +369,22 @@ void do_sysreg(struct cpu_user_regs *regs,
     GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0)
     GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1)
 
+    case HSR_SYSREG_ID_AA64DFR0_EL1:
+    {
+        struct domain *d = current->domain;
+        union cpuinfo_dbg64 info_dbg64 = domain_cpuinfo.dbg64;
+
+        if ( !(d->options & XEN_DOMCTL_CDF_vpmu) )
+        {
+            info_dbg64.pmu_ver = 0;
+            info_dbg64.mtpmu = 0;
+            info_dbg64.pmss = 0;
+        }
+
+        return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
+                                  info_dbg64.bits[0]);
+    }
+
     case HSR_SYSREG_ID_AA64ZFR0_EL1:
     {
         /*
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 94d14fb6a9..0e9bf15ca5 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -219,6 +219,14 @@ static int __init create_domain_cpuinfo(void)
     domain_cpuinfo.isa64.api = 0;
     domain_cpuinfo.isa64.gpa = 0;
     domain_cpuinfo.isa64.gpi = 0;
+
+    /* Hide SPE, TRBE, BRBE, and Trace Extensions */
+    domain_cpuinfo.dbg64.pms_ver = 0;
+    domain_cpuinfo.dbg64.trace_ver = 0;
+    domain_cpuinfo.dbg64.trace_filt = 0;
+    domain_cpuinfo.dbg64.trace_buffer = 0;
+    domain_cpuinfo.dbg64.ext_trc_buff = 0;
+    domain_cpuinfo.dbg64.brbe = 0;
 #endif
 
     /* Hide AMU support */
diff --git a/xen/arch/arm/include/asm/arm64/hsr.h b/xen/arch/arm/include/asm/arm64/hsr.h
index 1495ccddea..ed18184cc7 100644
--- a/xen/arch/arm/include/asm/arm64/hsr.h
+++ b/xen/arch/arm/include/asm/arm64/hsr.h
@@ -84,6 +84,7 @@
 #define HSR_SYSREG_FAR_EL1        HSR_SYSREG(3,0,c6, c0,0)
 #define HSR_SYSREG_PMINTENSET_EL1 HSR_SYSREG(3,0,c9,c14,1)
 #define HSR_SYSREG_PMINTENCLR_EL1 HSR_SYSREG(3,0,c9,c14,2)
+#define HSR_SYSREG_PMMIR_EL1      HSR_SYSREG(3,0,c9,c14,6)
 #define HSR_SYSREG_MAIR_EL1       HSR_SYSREG(3,0,c10,c2,0)
 #define HSR_SYSREG_AMAIR_EL1      HSR_SYSREG(3,0,c10,c3,0)
 #define HSR_SYSREG_ICC_SGI1R_EL1  HSR_SYSREG(3,0,c12,c11,5)
diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
index bf902a3970..ce8b58458f 100644
--- a/xen/arch/arm/include/asm/cpufeature.h
+++ b/xen/arch/arm/include/asm/cpufeature.h
@@ -208,7 +208,7 @@ struct cpuinfo_arm {
         };
     } pfr64;
 
-    union {
+    union cpuinfo_dbg64 {
         register_t bits[2];
         struct {
             /* DFR0 */
@@ -216,16 +216,18 @@ struct cpuinfo_arm {
             unsigned long trace_ver:4;
             unsigned long pmu_ver:4;
             unsigned long brps:4;
-            unsigned long __res0:4;
+            unsigned long pmss:4;
             unsigned long wrps:4;
-            unsigned long __res1:4;
+            unsigned long sebep:4;
             unsigned long ctx_cmps:4;
             unsigned long pms_ver:4;
             unsigned long double_lock:4;
             unsigned long trace_filt:4;
-            unsigned long __res2:4;
+            unsigned long trace_buffer:4;
             unsigned long mtpmu:4;
-            unsigned long __res3:12;
+            unsigned long brbe:4;
+            unsigned long ext_trc_buff:4;
+            unsigned long hpmn0:4;
 
             /* DFR1 */
             unsigned long __res4:64;
@@ -408,7 +410,7 @@ struct cpuinfo_arm {
         };
     } pfr32;
 
-    union {
+    union cpuinfo_dbg32 {
         register_t bits[2];
         struct {
             /* DFR0 */
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.