* [PATCH v2] xen/arm: Hide PMU registers from the guest, when the vPMU feature is disabled.
@ 2026-08-13 3:29 Hirokazu Takahashi
2026-08-17 7:00 ` Orzel, Michal
0 siblings, 1 reply; 4+ messages in thread
From: Hirokazu Takahashi @ 2026-08-13 3:29 UTC (permalink / raw)
To: xen-devel
Cc: Hirokazu Takahashi, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
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 <taka@valinux.co.jp>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] xen/arm: Hide PMU registers from the guest, when the vPMU feature is disabled.
2026-08-13 3:29 [PATCH v2] xen/arm: Hide PMU registers from the guest, when the vPMU feature is disabled Hirokazu Takahashi
@ 2026-08-17 7:00 ` Orzel, Michal
2026-08-18 8:50 ` Hirokazu Takahashi
0 siblings, 1 reply; 4+ messages in thread
From: Orzel, Michal @ 2026-08-17 7:00 UTC (permalink / raw)
To: Hirokazu Takahashi, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk
On 13-Aug-26 05:29, Hirokazu Takahashi wrote:
> 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:
Please use the imperative mood
>
> - 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
When vPMU is enabled, there is no trap/emulation
> 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"
Fixes commit title needs to be in brackets ()
> Signed-off-by: Hirokazu Takahashi <taka@valinux.co.jp>
> ---
> 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:
What about AArch32 PMMIR?
> /*
> * 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:
You only cover AArch64. What about AArch32 DFR0?
> + {
> + struct domain *d = current->domain;
Use v->domain instead like the surrounding code
> + 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)
What about DFR1 fields like PMICNTR? They suffer from the same problem.
> 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:
Please adhere to the order in which the cases were originally placed
> + {
> + struct domain *d = current->domain;
> + union cpuinfo_dbg64 info_dbg64 = domain_cpuinfo.dbg64;
> +
> + if ( !(d->options & XEN_DOMCTL_CDF_vpmu) )
To avoid duplication, please introduce is_vpmu_domain
> + {
> + 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;
Where did you take this field from? I can't see it in the latest Arm ARM:
https://support.arm.com/documentation/ddi0487/mc/-Part-D-The-AArch64-System-Level-Architecture/-Chapter-D24-AArch64-System-Register-Descriptions/-D24-2-General-system-control-registers/-D24-2-79-ID-AA64DFR0-EL1--AArch64-Debug-Feature-Register-0?lang=en
> 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 */
~Michal
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] xen/arm: Hide PMU registers from the guest, when the vPMU feature is disabled.
2026-08-17 7:00 ` Orzel, Michal
@ 2026-08-18 8:50 ` Hirokazu Takahashi
2026-08-18 9:18 ` Orzel, Michal
0 siblings, 1 reply; 4+ messages in thread
From: Hirokazu Takahashi @ 2026-08-18 8:50 UTC (permalink / raw)
To: Orzel, Michal, xen-devel@lists.xenproject.org
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk
Hello,
Thanks for the comments.
> > 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:
> Please use the imperative mood
Okay.
> > - 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
> When vPMU is enabled, there is no trap/emulation
Understood.
> > 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"
> Fixes commit title needs to be in brackets ()
Okay.
> > --- 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:
> What about AArch32 PMMIR?
Okay, I will add a trap handler entry for AArch32 PMMIR register accesses.
> > @@ -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:
> You only cover AArch64. What about AArch32 DFR0?
Okay, I will also add a trap handler entry for it.
> > + {
> > + struct domain *d = current->domain;
> Use v->domain instead like the surrounding code
Okay.
> > + 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)
> What about DFR1 fields like PMICNTR? They suffer from the same problem.
Okay, I will fix it.
> > 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:
> Please adhere to the order in which the cases were originally placed
Okay.
> > + {
> > + struct domain *d = current->domain;
> > + union cpuinfo_dbg64 info_dbg64 = domain_cpuinfo.dbg64;
> > +
> > + if ( !(d->options & XEN_DOMCTL_CDF_vpmu) )
> To avoid duplication, please introduce is_vpmu_domain
Okay, I will.
> > + {
> > + 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]);
> > + }
> > +
> > @@ -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;
> Where did you take this field from? I can't see it in the latest Arm ARM:
> https://support.arm.com/documentation/ddi0487/mc/-Part-D-The-AArch64
> -System-Level-Architecture/-Chapter-D24-AArch64-System-Register-Descri
> ptions/-D24-2-General-system-control-registers/-D24-2-79-ID-AA64DFR0-
> EL1--AArch64-Debug-Feature-Register-0?lang=en
In a slightly older Architecture Reference Manual, there was a SEBEP field
in ID_AA64DFR0_EL1. Has it been removed from the spec?
https://support.arm.com/documentation/111180/2025-09_ASL1/AArch64-Registers/ID-AA64DFR0-EL1--AArch64-Debug-Feature-Register-0?lang=en
Thank you,
Hirokazu Takahashi.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] xen/arm: Hide PMU registers from the guest, when the vPMU feature is disabled.
2026-08-18 8:50 ` Hirokazu Takahashi
@ 2026-08-18 9:18 ` Orzel, Michal
0 siblings, 0 replies; 4+ messages in thread
From: Orzel, Michal @ 2026-08-18 9:18 UTC (permalink / raw)
To: Hirokazu Takahashi, xen-devel@lists.xenproject.org
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk
On 18-Aug-26 10:50, Hirokazu Takahashi wrote:
> Hello,
>
> Thanks for the comments.
>
>>> 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:
>> Please use the imperative mood
>
> Okay.
>
>>> - 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
>> When vPMU is enabled, there is no trap/emulation
>
> Understood.
>
>>> 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"
>> Fixes commit title needs to be in brackets ()
>
> Okay.
>
>>> --- 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:
>> What about AArch32 PMMIR?
>
> Okay, I will add a trap handler entry for AArch32 PMMIR register accesses.
>
>>> @@ -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:
>> You only cover AArch64. What about AArch32 DFR0?
>
> Okay, I will also add a trap handler entry for it.
>
>>> + {
>>> + struct domain *d = current->domain;
>> Use v->domain instead like the surrounding code
>
> Okay.
>
>>> + 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)
>> What about DFR1 fields like PMICNTR? They suffer from the same problem.
>
> Okay, I will fix it.
>
>>> 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:
>> Please adhere to the order in which the cases were originally placed
>
> Okay.
>
>>> + {
>>> + struct domain *d = current->domain;
>>> + union cpuinfo_dbg64 info_dbg64 = domain_cpuinfo.dbg64;
>>> +
>>> + if ( !(d->options & XEN_DOMCTL_CDF_vpmu) )
>> To avoid duplication, please introduce is_vpmu_domain
>
> Okay, I will.
>
>>> + {
>>> + 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]);
>>> + }
>>> +
>
>>> @@ -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;
>> Where did you take this field from? I can't see it in the latest Arm ARM:
>> https://support.arm.com/documentation/ddi0487/mc/-Part-D-The-AArch64
>> -System-Level-Architecture/-Chapter-D24-AArch64-System-Register-Descri
>> ptions/-D24-2-General-system-control-registers/-D24-2-79-ID-AA64DFR0-
>> EL1--AArch64-Debug-Feature-Register-0?lang=en
>
> In a slightly older Architecture Reference Manual, there was a SEBEP field
> in ID_AA64DFR0_EL1. Has it been removed from the spec?
I can see it's been added since Armv9.3. I'd recommend leaving it as RES given
that you do not use it anyway in this patch. We usually do the update looking at
the latest Armv8-A spec.
~Michal
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-18 9:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 3:29 [PATCH v2] xen/arm: Hide PMU registers from the guest, when the vPMU feature is disabled Hirokazu Takahashi
2026-08-17 7:00 ` Orzel, Michal
2026-08-18 8:50 ` Hirokazu Takahashi
2026-08-18 9:18 ` Orzel, Michal
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.