Re: [PATCH v4 03/29] KVM: arm64: Merge guest's HCRX_EL2 using NV_HCRX_GUEST_EXCLUDE

Marc Zyngier <[email protected]> Thu, 30 Jul 2026 09:01:33 +0100
Newsgroups dev.linux.lists.sashiko-reviews,dev.linux.lists.kvmarm,org.kernel.vger.kvm
Message-ID <[email protected]>
On Thu, 30 Jul 2026 08:29:29 +0100,
[email protected] wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The NV_HCRX_GUEST_EXCLUDE macro fails to exclude HCRX_EL2_MCE2, allowing a nested hypervisor to break KVM's transparent MOPS migration handling.
> --
> 
> --- Patch [3]: [PATCH v4 03/29] KVM: arm64: Merge guest's HCRX_EL2 using NV_HCRX_GUEST_EXCLUDE ---
> commit 32a9437b5c31bcb57be7ba071a094800d7980587
> Author: Marc Zyngier <[email protected]>
> 
> KVM: arm64: Merge guest's HCRX_EL2 using NV_HCRX_GUEST_EXCLUDE
> 
> This commit refactors HCRX_EL2 handling by introducing a macro that
> explicitly lists the bits excluded from the merge, preventing the nested
> guest from overriding them.
> 
> > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > index 18131e395e24c..0903e08f4b6a0 100644
> > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> 
> [ ... ]
> 
> > @@ -325,6 +325,24 @@ static inline void __deactivate_traps_mpam(void)
> >  		write_sysreg_s(MPAMHCR_HOST_FLAGS, SYS_MPAMHCR_EL2);
> >  }
> >  
> > +/*
> > + * Just like for HCR_EL2, we can't let the guest mess with some of the
> > + * basics we rely on in HCRX_EL2. However, the major difference is that
> > + * HCRX_EL2 only affects EL1, and never EL2 (sudden outburst of sanity, I
> > + * guess). So it is always the guest inflicting it on its own guestx.
> > + *
> > + * Things we don't want to let the guest control are:
> > + *
> > + * - TMEA: That's for us to decide how an SEA is routed, not the guest.
> > + *
> > + * - PTTWI: Similarly, it is for us to decide whether Reduced Coherency for
> > + *   the PTW is a thing. It really isn't.
> > + *
> > + * - EnIDCP128: We don't allow IMPDEF sysregs -- full stop.
> > + */
> > +#define NV_HCRX_GUEST_EXCLUDE	(HCRX_EL2_TMEA	    | HCRX_EL2_PTTWI | \
> > +				 HCRX_EL2_EnIDCP128)
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but since NV_HCRX_GUEST_EXCLUDE
> enumerates the bits the nested guest is not allowed to override, should
> HCRX_EL2_MCE2 be excluded here as well?
> 
> KVM (L0) relies on intercepting MOPS exceptions (via HCRX_EL2.MCE2=1) to
> handle vCPU physical migrations transparently. If L0 merges a value from the
> L1 hypervisor that sets MCE2 to 0 (which happens naturally if L1 does not
> support MOPS),

If L1 doesn't support MOPS, then HCRX_EL2.MCEn==0, and the
instructions UNDEF.

> and L0 then migrates the physical CPU while L2 is executing a
> MOPS instruction, the MOPS exception could trap to EL1 (L2) instead of
> EL2 (L0).

No, the instruction will have UNDEF in L2.

Furthermore, trapping non-matching CPUs is a guest decision, and L0 is
not in charge of deciding this. However, what is missing is the
redirection of the exception to L1 when it fires.

Something else to fix at some point.

	M.

-- 
Without deviation from the norm, progress is not possible.