* [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented
@ 2026-09-11 10:47 Fuad Tabba
2026-09-11 13:27 ` Ben Horgan
2026-09-13 10:00 ` Marc Zyngier
0 siblings, 2 replies; 3+ messages in thread
From: Fuad Tabba @ 2026-09-11 10:47 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon
Cc: James Morse, Ben Horgan, Xi Ruoyao, Mark Rutland, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Steffen Eiden, Gavin Shan,
Yuan Yao, Fuad Tabba, linux-arm-kernel, kvmarm, linux-kernel
finalise_el2_state() clears the EL2 MPAM traps whenever the ID
registers advertise MPAM, while KVM sets them only under ARM64_MPAM,
which also requires MPAMEN. Without EL3 nothing sets that enable, so a
guest reaches the MPAM registers while ID_AA64PFR0_EL1.MPAM reads 0
for it.
Gate on what finalise_el2_state() tests instead: this CPU's ID
registers with the arm64.nompam override applied, and its
MPAMIDR_EL1.HAS_HCR before writing MPAMHCR_EL2, which is UNDEFINED
without it. MPAMEN isn't a term in any MPAM accessor, so the traps
take effect without it.
Fixes: 31ff96c38ea3 ("KVM: arm64: Fix missing traps of guest accesses to the MPAM registers")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
Notes:
Changes since v2:
- A per-CPU flag in kvm_host_data, set at KVM's CPU init from the ID
registers with the override applied, instead of a third cpucap
(Will). The probe checks presence only, since MPAM3_EL3.TRAPLOWER
can't be read at EL2.
- The MPAMHCR_EL2 branch reads this CPU's MPAMIDR_EL1 rather than the
sanitised ARM64_MPAM_HCR, which would otherwise have been the one
system-wide test left in a per-CPU function.
- Fixes: names the KVM commit that left this case open, rather than
the head.S commit that cleared the traps.
- Yuan's Reviewed-by dropped, since the mechanism changed.
v2: https://lore.kernel.org/all/20260908145651.2828597-1-fuad.tabba@linux.dev/
v1: https://lore.kernel.org/all/20260903160819.831518-1-fuad.tabba@linux.dev/
arch/arm64/include/asm/kvm_host.h | 1 +
arch/arm64/kvm/arm.c | 7 +++++++
arch/arm64/kvm/hyp/include/hyp/switch.h | 13 ++++++++-----
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 27fe0cd5b2d7a..98ca2d9b9e18d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -755,6 +755,7 @@ struct kvm_host_data {
#define KVM_HOST_DATA_FLAG_VCPU_IN_HYP_CONTEXT 4
#define KVM_HOST_DATA_FLAG_L1_VNCR_MAPPED 5
#define KVM_HOST_DATA_FLAG_HAS_BRBE 6
+#define KVM_HOST_DATA_FLAG_HAS_MPAM 7
unsigned long flags;
struct kvm_cpu_context host_ctxt;
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8b080804bc90b..fde75a63cf045 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -2281,9 +2281,16 @@ static void cpu_set_hyp_vector(void)
static void cpu_hyp_init_context(void)
{
+ u64 pfr0 = __read_sysreg_by_encoding(SYS_ID_AA64PFR0_EL1);
+ u64 pfr1 = __read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1);
+
kvm_init_host_cpu_context(host_data_ptr(host_ctxt));
kvm_init_host_debug_data();
+ /* The traps take effect without MPAMEN, which ARM64_MPAM requires. */
+ if (id_aa64pfr0_mpam(pfr0) || id_aa64pfr1_mpamfrac(pfr1))
+ host_data_set_flag(HAS_MPAM);
+
if (!is_kernel_in_hyp_mode())
cpu_init_hyp_mode();
}
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 1ce7130e25490..2cb611e2bd69b 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -298,14 +298,17 @@ static inline void __activate_traps_mpam(struct kvm_vcpu *vcpu)
u64 clr = MPAM2_EL2_EnMPAMSM;
u64 set = MPAM2_EL2_TRAPMPAM0EL1 | MPAM2_EL2_TRAPMPAM1EL1;
- if (!system_supports_mpam())
+ if (!host_data_test_flag(HAS_MPAM))
return;
/* trap guest access to MPAMIDR_EL1 */
- if (system_supports_mpam_hcr()) {
+ if (read_sysreg_s(SYS_MPAMIDR_EL1) & MPAMIDR_EL1_HAS_HCR) {
write_sysreg_s(MPAMHCR_EL2_TRAP_MPAMIDR_EL1, SYS_MPAMHCR_EL2);
} else {
- /* From v1.1 TIDR can trap MPAMIDR, set it unconditionally */
+ /*
+ * TIDR is RES0 without MPAMIDR_EL1.HAS_TIDR, which MPAM v1.0
+ * prohibits: such a PE without HAS_HCR can't trap MPAMIDR_EL1.
+ */
set |= MPAM2_EL2_TIDR;
}
@@ -317,12 +320,12 @@ static inline void __deactivate_traps_mpam(void)
u64 clr = MPAM2_EL2_TRAPMPAM0EL1 | MPAM2_EL2_TRAPMPAM1EL1 | MPAM2_EL2_TIDR;
u64 set = MPAM2_EL2_EnMPAMSM;
- if (!system_supports_mpam())
+ if (!host_data_test_flag(HAS_MPAM))
return;
sysreg_clear_set_s(SYS_MPAM2_EL2, clr, set);
- if (system_supports_mpam_hcr())
+ if (read_sysreg_s(SYS_MPAMIDR_EL1) & MPAMIDR_EL1_HAS_HCR)
write_sysreg_s(MPAMHCR_HOST_FLAGS, SYS_MPAMHCR_EL2);
}
--
2.39.5
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented
2026-09-11 10:47 [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented Fuad Tabba
@ 2026-09-11 13:27 ` Ben Horgan
2026-09-13 10:00 ` Marc Zyngier
1 sibling, 0 replies; 3+ messages in thread
From: Ben Horgan @ 2026-09-11 13:27 UTC (permalink / raw)
To: Fuad Tabba, Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon
Cc: James Morse, Xi Ruoyao, Mark Rutland, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Steffen Eiden, Gavin Shan,
Yuan Yao, Fuad Tabba, linux-arm-kernel, kvmarm, linux-kernel
Hi Fuad,
On 11/09/2026 11:47, Fuad Tabba wrote:
> finalise_el2_state() clears the EL2 MPAM traps whenever the ID
> registers advertise MPAM, while KVM sets them only under ARM64_MPAM,
> which also requires MPAMEN. Without EL3 nothing sets that enable, so a
This "Without EL3..." part reads oddly to me. I guess what you are getting at is that the MPAMEN is
read only apart from at the highest implemented exception level.
> guest reaches the MPAM registers while ID_AA64PFR0_EL1.MPAM reads 0
> for it.
>
> Gate on what finalise_el2_state() tests instead: this CPU's ID
> registers with the arm64.nompam override applied, and its
> MPAMIDR_EL1.HAS_HCR before writing MPAMHCR_EL2, which is UNDEFINED
> without it. MPAMEN isn't a term in any MPAM accessor, so the traps
> take effect without it.
>
> Fixes: 31ff96c38ea3 ("KVM: arm64: Fix missing traps of guest accesses to the MPAM registers")
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
This looks good to me. Now that you're determining if the MPAM registers are present on a per CPU
basis and using that via a host flag, the traps will be set on all CPUs that have MPAM registers
unless overridden on the command line. IIUC this covers the mismatched CPU case as well and so all
cases that the register can be safely accessed (arm64.nompam not required) are covered.
> ---
>
> Notes:
> Changes since v2:
> - A per-CPU flag in kvm_host_data, set at KVM's CPU init from the ID
> registers with the override applied, instead of a third cpucap
> (Will). The probe checks presence only, since MPAM3_EL3.TRAPLOWER
> can't be read at EL2.
> - The MPAMHCR_EL2 branch reads this CPU's MPAMIDR_EL1 rather than the
> sanitised ARM64_MPAM_HCR, which would otherwise have been the one
> system-wide test left in a per-CPU function.
> - Fixes: names the KVM commit that left this case open, rather than
> the head.S commit that cleared the traps.
> - Yuan's Reviewed-by dropped, since the mechanism changed.
>
> v2: https://lore.kernel.org/all/20260908145651.2828597-1-fuad.tabba@linux.dev/
> v1: https://lore.kernel.org/all/20260903160819.831518-1-fuad.tabba@linux.dev/
>
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/arm.c | 7 +++++++
> arch/arm64/kvm/hyp/include/hyp/switch.h | 13 ++++++++-----
> 3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27fe0cd5b2d7a..98ca2d9b9e18d 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -755,6 +755,7 @@ struct kvm_host_data {
> #define KVM_HOST_DATA_FLAG_VCPU_IN_HYP_CONTEXT 4
> #define KVM_HOST_DATA_FLAG_L1_VNCR_MAPPED 5
> #define KVM_HOST_DATA_FLAG_HAS_BRBE 6
> +#define KVM_HOST_DATA_FLAG_HAS_MPAM 7
> unsigned long flags;
>
> struct kvm_cpu_context host_ctxt;
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90b..fde75a63cf045 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -2281,9 +2281,16 @@ static void cpu_set_hyp_vector(void)
>
> static void cpu_hyp_init_context(void)
> {
> + u64 pfr0 = __read_sysreg_by_encoding(SYS_ID_AA64PFR0_EL1);
> + u64 pfr1 = __read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1);
> +
> kvm_init_host_cpu_context(host_data_ptr(host_ctxt));
> kvm_init_host_debug_data();
>
> + /* The traps take effect without MPAMEN, which ARM64_MPAM requires. */
> + if (id_aa64pfr0_mpam(pfr0) || id_aa64pfr1_mpamfrac(pfr1))
> + host_data_set_flag(HAS_MPAM);
> +
> if (!is_kernel_in_hyp_mode())
> cpu_init_hyp_mode();
> }
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 1ce7130e25490..2cb611e2bd69b 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -298,14 +298,17 @@ static inline void __activate_traps_mpam(struct kvm_vcpu *vcpu)
> u64 clr = MPAM2_EL2_EnMPAMSM;
> u64 set = MPAM2_EL2_TRAPMPAM0EL1 | MPAM2_EL2_TRAPMPAM1EL1;
>
> - if (!system_supports_mpam())
> + if (!host_data_test_flag(HAS_MPAM))
> return;
>
> /* trap guest access to MPAMIDR_EL1 */
> - if (system_supports_mpam_hcr()) {
> + if (read_sysreg_s(SYS_MPAMIDR_EL1) & MPAMIDR_EL1_HAS_HCR) {
> write_sysreg_s(MPAMHCR_EL2_TRAP_MPAMIDR_EL1, SYS_MPAMHCR_EL2);
> } else {
> - /* From v1.1 TIDR can trap MPAMIDR, set it unconditionally */
> + /*
> + * TIDR is RES0 without MPAMIDR_EL1.HAS_TIDR, which MPAM v1.0
> + * prohibits: such a PE without HAS_HCR can't trap MPAMIDR_EL1.
> + */
The lack of a trap too bad as all the fields of MPAMIDR_EL1 are read only.
Reviewed-by: Ben Horgan <ben.horgan@arm.com>
Thanks,
Ben
> set |= MPAM2_EL2_TIDR;
> }
>
> @@ -317,12 +320,12 @@ static inline void __deactivate_traps_mpam(void)
> u64 clr = MPAM2_EL2_TRAPMPAM0EL1 | MPAM2_EL2_TRAPMPAM1EL1 | MPAM2_EL2_TIDR;
> u64 set = MPAM2_EL2_EnMPAMSM;
>
> - if (!system_supports_mpam())
> + if (!host_data_test_flag(HAS_MPAM))
> return;
>
> sysreg_clear_set_s(SYS_MPAM2_EL2, clr, set);
>
> - if (system_supports_mpam_hcr())
> + if (read_sysreg_s(SYS_MPAMIDR_EL1) & MPAMIDR_EL1_HAS_HCR)
> write_sysreg_s(MPAMHCR_HOST_FLAGS, SYS_MPAMHCR_EL2);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented
2026-09-11 10:47 [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented Fuad Tabba
2026-09-11 13:27 ` Ben Horgan
@ 2026-09-13 10:00 ` Marc Zyngier
1 sibling, 0 replies; 3+ messages in thread
From: Marc Zyngier @ 2026-09-13 10:00 UTC (permalink / raw)
To: Fuad Tabba
Cc: Oliver Upton, Catalin Marinas, Will Deacon, James Morse,
Ben Horgan, Xi Ruoyao, Mark Rutland, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Steffen Eiden, Gavin Shan,
Yuan Yao, Fuad Tabba, linux-arm-kernel, kvmarm, linux-kernel
On Fri, 11 Sep 2026 11:47:15 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
>
> finalise_el2_state() clears the EL2 MPAM traps whenever the ID
> registers advertise MPAM, while KVM sets them only under ARM64_MPAM,
> which also requires MPAMEN. Without EL3 nothing sets that enable, so a
> guest reaches the MPAM registers while ID_AA64PFR0_EL1.MPAM reads 0
> for it.
>
> Gate on what finalise_el2_state() tests instead: this CPU's ID
> registers with the arm64.nompam override applied, and its
> MPAMIDR_EL1.HAS_HCR before writing MPAMHCR_EL2, which is UNDEFINED
> without it. MPAMEN isn't a term in any MPAM accessor, so the traps
> take effect without it.
>
> Fixes: 31ff96c38ea3 ("KVM: arm64: Fix missing traps of guest accesses to the MPAM registers")
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>
> Notes:
> Changes since v2:
> - A per-CPU flag in kvm_host_data, set at KVM's CPU init from the ID
> registers with the override applied, instead of a third cpucap
> (Will). The probe checks presence only, since MPAM3_EL3.TRAPLOWER
> can't be read at EL2.
> - The MPAMHCR_EL2 branch reads this CPU's MPAMIDR_EL1 rather than the
> sanitised ARM64_MPAM_HCR, which would otherwise have been the one
> system-wide test left in a per-CPU function.
> - Fixes: names the KVM commit that left this case open, rather than
> the head.S commit that cleared the traps.
> - Yuan's Reviewed-by dropped, since the mechanism changed.
>
> v2: https://lore.kernel.org/all/20260908145651.2828597-1-fuad.tabba@linux.dev/
> v1: https://lore.kernel.org/all/20260903160819.831518-1-fuad.tabba@linux.dev/
>
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/arm.c | 7 +++++++
> arch/arm64/kvm/hyp/include/hyp/switch.h | 13 ++++++++-----
> 3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27fe0cd5b2d7a..98ca2d9b9e18d 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -755,6 +755,7 @@ struct kvm_host_data {
> #define KVM_HOST_DATA_FLAG_VCPU_IN_HYP_CONTEXT 4
> #define KVM_HOST_DATA_FLAG_L1_VNCR_MAPPED 5
> #define KVM_HOST_DATA_FLAG_HAS_BRBE 6
> +#define KVM_HOST_DATA_FLAG_HAS_MPAM 7
> unsigned long flags;
>
> struct kvm_cpu_context host_ctxt;
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90b..fde75a63cf045 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -2281,9 +2281,16 @@ static void cpu_set_hyp_vector(void)
>
> static void cpu_hyp_init_context(void)
> {
> + u64 pfr0 = __read_sysreg_by_encoding(SYS_ID_AA64PFR0_EL1);
> + u64 pfr1 = __read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1);
> +
Why not directly read_sysreg(id_aa64pfr0_el1) and co?
__read_sysreg_by_encoding() is useful when the encoding comes from a
variable, but it looks odd in the case of a literal sysreg.
> kvm_init_host_cpu_context(host_data_ptr(host_ctxt));
> kvm_init_host_debug_data();
>
> + /* The traps take effect without MPAMEN, which ARM64_MPAM requires. */
> + if (id_aa64pfr0_mpam(pfr0) || id_aa64pfr1_mpamfrac(pfr1))
> + host_data_set_flag(HAS_MPAM);
> +
> if (!is_kernel_in_hyp_mode())
> cpu_init_hyp_mode();
> }
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 1ce7130e25490..2cb611e2bd69b 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -298,14 +298,17 @@ static inline void __activate_traps_mpam(struct kvm_vcpu *vcpu)
> u64 clr = MPAM2_EL2_EnMPAMSM;
> u64 set = MPAM2_EL2_TRAPMPAM0EL1 | MPAM2_EL2_TRAPMPAM1EL1;
>
> - if (!system_supports_mpam())
> + if (!host_data_test_flag(HAS_MPAM))
> return;
>
> /* trap guest access to MPAMIDR_EL1 */
> - if (system_supports_mpam_hcr()) {
> + if (read_sysreg_s(SYS_MPAMIDR_EL1) & MPAMIDR_EL1_HAS_HCR) {
This is going to suck under NV. The host hypervisor is of course going
to set MPAMHCR_EL2.TRAP_MPAMIDR_EL1, and we're in for a recursive trap
on the hottest possible path in KVM. Which is silly as the actual
write to MPAMHCR_EL2 is free (it lands in NVMem[]).
This really should be replaced by a flag called HAS_MPAM_HCR, just
like you have HAS_MPAM.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-13 10:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-11 10:47 [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented Fuad Tabba
2026-09-11 13:27 ` Ben Horgan
2026-09-13 10:00 ` Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®