Re: [PATCH 05/33] KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping

[Marc Zyngier <maz@kernel.org>]

On Tue, 04 Nov 2025 08:50:26 +0000,
Yao Yuan <yaoyuan@linux.alibaba.com> wrote:
> 
> On Mon, Nov 03, 2025 at 04:54:49PM +0800, Marc Zyngier wrote:
> > A long time ago, an unsuspecting architect forgot to add a trap
> > bit for ICV_DIR_EL1 in ICH_HCR_EL2. Which was unfortunate, but
> > what's a bit of spec between friends? Thankfully, this was fixed
> > in a later revision, and ARM "deprecates" the lack of trapping
> > ability.
> >
> > Unfortuantely, a few (billion) CPUs went out with that defect,
> > anything ARMv8.0 from ARM, give or take. And on these CPUs,
> > you can't trap DIR on its own, full stop.
> >
> > As the next best thing, we can trap everything in the common group,
> > which is a tad expensive, but hey ho, that's what you get. You can
> > otherwise recycle the HW in the neaby bin.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/virt.h  |  7 ++++++-
> >  arch/arm64/kernel/cpufeature.c | 34 ++++++++++++++++++++++++++++++++++
> >  arch/arm64/kernel/hyp-stub.S   |  5 +++++
> >  arch/arm64/kvm/vgic/vgic-v3.c  |  3 +++
> >  arch/arm64/tools/cpucaps       |  1 +
> >  5 files changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> > index aa280f356b96a..8eb63d3294974 100644
> > --- a/arch/arm64/include/asm/virt.h
> > +++ b/arch/arm64/include/asm/virt.h
> > @@ -40,8 +40,13 @@
> >   */
> >  #define HVC_FINALISE_EL2	3
> >
> > +/*
> > + * HVC_GET_ICH_VTR_EL2 - Retrieve the ICH_VTR_EL2 value
> > + */
> > +#define HVC_GET_ICH_VTR_EL2	4
> > +
> >  /* Max number of HYP stub hypercalls */
> > -#define HVC_STUB_HCALL_NR 4
> > +#define HVC_STUB_HCALL_NR 5
> >
> >  /* Error returned when an invalid stub number is passed into x0 */
> >  #define HVC_STUB_ERR	0xbadca11
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index 5ed401ff79e3e..44103ad98805d 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -2303,6 +2303,31 @@ static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry
> >  }
> >  #endif
> >
> > +static bool can_trap_icv_dir_el1(const struct arm64_cpu_capabilities *entry,
> > +				 int scope)
> > +{
> > +	struct arm_smccc_res res = {};
> > +
> > +	BUILD_BUG_ON(ARM64_HAS_ICH_HCR_EL2_TDS <= ARM64_HAS_GICV3_CPUIF);
> > +	BUILD_BUG_ON(ARM64_HAS_ICH_HCR_EL2_TDS <= ARM64_HAS_GICV5_LEGACY);
> > +	if (!cpus_have_cap(ARM64_HAS_GICV3_CPUIF) ||
> > +	    !cpus_have_cap(ARM64_HAS_GICV3_CPUIF))
> 
> Duplicated checking ?

Yup, cut'n'paste, and lack of GICv5 testing... This should really look
like the hack below, since GICv5 legacy feature is guaranteed to have
TDIR:

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 44103ad98805d..3f2d4b033966d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2310,13 +2310,15 @@ static bool can_trap_icv_dir_el1(const struct arm64_cpu_capabilities *entry,
 
 	BUILD_BUG_ON(ARM64_HAS_ICH_HCR_EL2_TDS <= ARM64_HAS_GICV3_CPUIF);
 	BUILD_BUG_ON(ARM64_HAS_ICH_HCR_EL2_TDS <= ARM64_HAS_GICV5_LEGACY);
-	if (!cpus_have_cap(ARM64_HAS_GICV3_CPUIF) ||
-	    !cpus_have_cap(ARM64_HAS_GICV3_CPUIF))
+	if (!cpus_have_cap(ARM64_HAS_GICV3_CPUIF))
 		return false;
 
 	if (!is_hyp_mode_available())
 		return false;
 
+	if (cpus_have_cap(ARM64_HAS_GICV5_LEGACY))
+		return true;
+
 	if (is_kernel_in_hyp_mode())
 		res.a1 = read_sysreg_s(SYS_ICH_VTR_EL2);
 	else


Thanks for the heads-up!

	M.

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