Re: [RFC PATCH v6 19/35] KVM: arm64: Trap PMBIDR_EL1 and PMSIDR_EL1

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi James,

On Mon, Jan 12, 2026 at 11:54:05AM +0000, James Clark wrote:
> 
> 
> On 12/01/2026 11:28 am, Alexandru Elisei wrote:
> > Hi James,
> > 
> > On Fri, Jan 09, 2026 at 04:29:37PM +0000, James Clark wrote:
> > > 
> > > 
> > > On 14/11/2025 4:07 pm, Alexandru Elisei wrote:
> > > > PMBIDR_EL1 and PMSIDR_EL1 are read-only registers that describe the SPE
> > > > implementation. Trap reads to allow KVM to control how SPE is described to
> > > > a virtual machine. In particular, this is needed to:
> > > > 
> > > > - Advertise the maximum buffer size set by userspace in
> > > >     PMBIDR_EL1.MaxBuffSize.
> > > > - Hide in PMSIDR_EL1, if necessary, the presence of FEAT_SPE_FDS and
> > > >     FEAT_SPE_FnE, both of which add new registers which are already
> > > >     trapped via the FGU mechanism.
> > > > 
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > ---
> > > >    arch/arm64/include/asm/kvm_spe.h |  4 +-
> > > >    arch/arm64/kvm/config.c          | 13 ++++-
> > > >    arch/arm64/kvm/spe.c             | 82 +++++++++++++++++++++++++++++++-
> > > >    arch/arm64/kvm/sys_regs.c        | 13 +++--
> > > >    4 files changed, 103 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/include/asm/kvm_spe.h b/arch/arm64/include/asm/kvm_spe.h
> > > > index 3506d8c4c661..47b94794cc5f 100644
> > > > --- a/arch/arm64/include/asm/kvm_spe.h
> > > > +++ b/arch/arm64/include/asm/kvm_spe.h
> > > > @@ -40,7 +40,7 @@ int kvm_spe_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> > > >    int kvm_spe_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> > > >    bool kvm_spe_write_sysreg(struct kvm_vcpu *vcpu, int reg, u64 val);
> > > > -u64 kvm_spe_read_sysreg(struct kvm_vcpu *vcpu, int reg);
> > > > +u64 kvm_spe_read_sysreg(struct kvm_vcpu *vcpu, int reg, u32 encoding);
> > > >    #else
> > > >    struct kvm_spe {
> > > >    };
> > > > @@ -78,7 +78,7 @@ static inline bool kvm_spe_write_sysreg(struct kvm_vcpu *vcpu, int reg, u64 val)
> > > >    {
> > > >    	return true;
> > > >    }
> > > > -static inline u64 kvm_spe_read_sysreg(struct kvm_vcpu *vcpu, int reg)
> > > > +static inline u64 kvm_spe_read_sysreg(struct kvm_vcpu *vcpu, int reg, u32 encoding)
> > > >    {
> > > >    	return 0;
> > > >    }
> > > > diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
> > > > index 24bb3f36e9d5..ed6b167b7aa8 100644
> > > > --- a/arch/arm64/kvm/config.c
> > > > +++ b/arch/arm64/kvm/config.c
> > > > @@ -6,6 +6,7 @@
> > > >    #include <linux/kvm_host.h>
> > > >    #include <asm/kvm_emulate.h>
> > > > +#include <asm/kvm_spe.h>
> > > >    #include <asm/kvm_nested.h>
> > > >    #include <asm/sysreg.h>
> > > > @@ -1489,6 +1490,16 @@ static void __compute_hfgwtr(struct kvm_vcpu *vcpu)
> > > >    		*vcpu_fgt(vcpu, HFGWTR_EL2) |= HFGWTR_EL2_TCR_EL1;
> > > >    }
> > > > +static void __compute_hdfgrtr(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +	__compute_fgt(vcpu, HDFGRTR_EL2);
> > > > +
> > > > +	if (vcpu_has_spe(vcpu)) {
> > > > +		*vcpu_fgt(vcpu, HDFGRTR_EL2) |= HDFGRTR_EL2_PMBIDR_EL1;
> > > > +		*vcpu_fgt(vcpu, HDFGRTR_EL2) |= HDFGRTR_EL2_PMSIDR_EL1;
> > > > +	}
> > > > +}
> > > > +
> > > >    static void __compute_hdfgwtr(struct kvm_vcpu *vcpu)
> > > >    {
> > > >    	__compute_fgt(vcpu, HDFGWTR_EL2);
> > > > @@ -1505,7 +1516,7 @@ void kvm_vcpu_load_fgt(struct kvm_vcpu *vcpu)
> > > >    	__compute_fgt(vcpu, HFGRTR_EL2);
> > > >    	__compute_hfgwtr(vcpu);
> > > >    	__compute_fgt(vcpu, HFGITR_EL2);
> > > > -	__compute_fgt(vcpu, HDFGRTR_EL2);
> > > > +	__compute_hdfgrtr(vcpu);
> > > >    	__compute_hdfgwtr(vcpu);
> > > >    	__compute_fgt(vcpu, HAFGRTR_EL2);
> > > > diff --git a/arch/arm64/kvm/spe.c b/arch/arm64/kvm/spe.c
> > > > index 5b3dc622cf82..92eb46276c71 100644
> > > > --- a/arch/arm64/kvm/spe.c
> > > > +++ b/arch/arm64/kvm/spe.c
> > > > @@ -22,10 +22,16 @@ struct arm_spu_entry {
> > > >    	struct arm_spe_pmu *arm_spu;
> > > >    };
> > > > +static u64 max_buffer_size_to_pmbidr_el1(u64 size);
> > > > +
> > > >    void kvm_host_spe_init(struct arm_spe_pmu *arm_spu)
> > > >    {
> > > >    	struct arm_spu_entry *entry;
> > > > +	/* PMBIDR_EL1 cannot be trapped without FEAT_FGT. */
> > > > +	if (!cpus_have_final_cap(ARM64_HAS_FGT))
> > > > +		return;
> > > > +
> > > 
> > > Isn't this only required if you want to report a different PMBIDR value? If
> > > the value in hardware is already what you want to give to guests then it's
> > > not strictly required. Maybe it's something we can do as an addition later
> > > to make it usable in more places.
> > 
> > The maximum buffer size is advertised in PMBIDR_EL1.
> > 
> > Thanks,
> > Alex
> > 
> 
> I know, but 0 "unlimited" is a valid value, and that's what's already in
> PMBIDR. You only need to trap if the value doesn't match what's already
> there.
> 
> You can also "probe" the maximum buffer size without this by attempting to
> set the limit pointer and looking at the buffer management error code.
> 
> The first thing I did when I went to use this was comment this out because
> my hardware doesn't support FGT. But the rest of it works fine without,
> maybe other people could benefit in the future too.

I could add a new capability for setting the maximum buffer size (or to set a
buffer size different than what the hardware advertises), enabled when FEAT_FGT
is implement by the hardware. But that means not allowing KVM SPE on Neoverse
because of erratum 3023823 [1]. So I don't think you would be able to use KVM
SPE out of the box on your hardware anyway (if I understood correctly that
you've been using a Neoverse CPU to test the series).

[1] https://developer.arm.com/documentation/SDEN-885747/34-0

Thanks,
Alex