Re: [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented

[Marc Zyngier]

On Fri, 11 Sep 2026 11:47:15 +0100,
Fuad Tabba <fuad.tabba@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 
> 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@xxxxxxxxx/
>     v1: https://lore.kernel.org/all/20260903160819.831518-1-fuad.tabba@xxxxxxxxx/
> 
>  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.