We have support for determining a set of fine grained traps to enable for
the guest which is tied to the support for injecting UNDEFs for undefined
features. This means that we can't use the mechanism for system registers
which should be present but need emulation, such as SMPRI_EL1 which should
be accessible when SME is present but if SME priority support is absent
SMPRI_EL1.Priority should be RAZ.

Add an additional set of fine grained traps fgt, mirroring the existing fgu
array. We use the same format where we always set the bit for the trap in
the array as for FGU. This makes it clear what is being explicitly managed
and keeps the code consistent.

We do not convert the handling of ARM_WORKAROUND_AMPERE_ACO3_CPU_38 to this
mechanism since this only enables a write trap and when implementing the
existing UNDEF that we would share the read and write trap enablement (this
being the overwhelmingly common case).

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/kvm_host.h       | 6 ++++++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 7 ++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 2f2394cce24e..b501c2880ba2 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -302,6 +302,12 @@ struct kvm_arch {
 	 */
 	u64 fgu[__NR_FGT_GROUP_IDS__];
 
+	/*
+	 * Additional FGTs to enable for the guests, eg. for emulated
+	 * registers,
+	 */
+	u64 fgt[__NR_FGT_GROUP_IDS__];
+
 	/*
 	 * Stage 2 paging state for VMs with nested S2 using a virtual
 	 * VMID.
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 84ec4e100fbb..d5620e02787f 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -283,9 +283,9 @@ static inline void __deactivate_cptr_traps(struct kvm_vcpu *vcpu)
 		id;							\
 	})
 
-#define compute_undef_clr_set(vcpu, kvm, reg, clr, set)			\
+#define compute_trap_clr_set(vcpu, kvm, trap, reg, clr, set)		\
 	do {								\
-		u64 hfg = kvm->arch.fgu[reg_to_fgt_group_id(reg)];	\
+		u64 hfg = kvm->arch.trap[reg_to_fgt_group_id(reg)];	\
 		struct fgt_masks *m = reg_to_fgt_masks(reg);		\
 		set |= hfg & m->mask;					\
 		clr |= hfg & m->nmask;					\
@@ -301,7 +301,8 @@ static inline void __deactivate_cptr_traps(struct kvm_vcpu *vcpu)
 		if (is_nested_ctxt(vcpu))				\
 			compute_clr_set(vcpu, reg, c, s);		\
 									\
-		compute_undef_clr_set(vcpu, kvm, reg, c, s);		\
+		compute_trap_clr_set(vcpu, kvm, fgu, reg, c, s);	\
+		compute_trap_clr_set(vcpu, kvm, fgt, reg, c, s);	\
 									\
 		val = m->nmask;						\
 		val |= s;						\

-- 
2.39.5

On Tue, 02 Sep 2025 12:36:09 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> We have support for determining a set of fine grained traps to enable for
> the guest which is tied to the support for injecting UNDEFs for undefined
> features. This means that we can't use the mechanism for system registers
> which should be present but need emulation, such as SMPRI_EL1 which should
> be accessible when SME is present but if SME priority support is absent
> SMPRI_EL1.Priority should be RAZ.
> 
> Add an additional set of fine grained traps fgt, mirroring the existing fgu
> array. We use the same format where we always set the bit for the trap in
> the array as for FGU. This makes it clear what is being explicitly managed
> and keeps the code consistent.
> 
> We do not convert the handling of ARM_WORKAROUND_AMPERE_ACO3_CPU_38 to this
> mechanism since this only enables a write trap and when implementing the
> existing UNDEF that we would share the read and write trap enablement (this
> being the overwhelmingly common case).
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_host.h       | 6 ++++++
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 7 ++++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 2f2394cce24e..b501c2880ba2 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -302,6 +302,12 @@ struct kvm_arch {
>  	 */
>  	u64 fgu[__NR_FGT_GROUP_IDS__];
>  
> +	/*
> +	 * Additional FGTs to enable for the guests, eg. for emulated
> +	 * registers,
> +	 */
> +	u64 fgt[__NR_FGT_GROUP_IDS__];
> +

Conceptually, this serves the same role as the existing control
registers (HCR_EL2, HCRX_EL2, MDCR_EL2), which are obviously
per-vcpu. So having this on a per-VM basis doesn't really work,
because we definitely don't expect this to be uniform (see
20250917203125.283116-3-oliver.upton@linux.dev for an example of why
this is not the case).

FGUs are uniform, because when something doesn't exist on a vcpu, it
doesn't exist on *any* vcpu. Non-FGU use of FGTs, however, has to be
more flexible because that's part of the emulation, and is actually
pretty rare that we want to trap something at all times, on all vcpus.

For the same reason, conflating the R and W registers doesn't work
either. For the above example, I want to be able to trap write
accesses to MDSCR_EL1, and not reads, just like the Ampere
brain-damage.

So please make this per-vcpu, decouple R and W FGTs, and convert the
Ampere horror to this scheme.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.

On Fri, Sep 19, 2025 at 04:14:49PM +0100, Marc Zyngier wrote:

> FGUs are uniform, because when something doesn't exist on a vcpu, it
> doesn't exist on *any* vcpu. Non-FGU use of FGTs, however, has to be
> more flexible because that's part of the emulation, and is actually
> pretty rare that we want to trap something at all times, on all vcpus.

> For the same reason, conflating the R and W registers doesn't work
> either. For the above example, I want to be able to trap write
> accesses to MDSCR_EL1, and not reads, just like the Ampere
> brain-damage.

> So please make this per-vcpu, decouple R and W FGTs, and convert the
> Ampere horror to this scheme.

OK, that makes more sense - it was a bit confusing that all the FGT
handling was done per VM not per vCPU without even any provision for
per-vCPU or distinct R/W stuff, it seemed strange.  Since the SME
requirement also ends up being per VM anyway I just tried to fit in with
what was there but if we don't want that behaviour it's certainly more
obvious to make it per-vCPU and to split read and write.