Hi Colton, On Tue, Dec 09, 2025 at 08:51:07PM +0000, Colton Lewis wrote: > In order to gain the best performance benefit from partitioning the > PMU, utilize fine grain traps (FEAT_FGT and FEAT_FGT2) to avoid > trapping common PMU register accesses by the guest to remove that > overhead. > > Untrapped: > * PMCR_EL0 > * PMUSERENR_EL0 > * PMSELR_EL0 > * PMCCNTR_EL0 > * PMCNTEN_EL0 > * PMINTEN_EL1 > * PMEVCNTRn_EL0 > > These are safe to untrap because writing MDCR_EL2.HPMN as this series > will do limits the effect of writes to any of these registers to the > partition of counters 0..HPMN-1. Reads from these registers will not > leak information from between guests as all these registers are > context swapped by a later patch in this series. Reads from these > registers also do not leak any information about the host's hardware > beyond what is promised by PMUv3. > > Trapped: > * PMOVS_EL0 > * PMEVTYPERn_EL0 > * PMCCFILTR_EL0 > * PMICNTR_EL0 > * PMICFILTR_EL0 > * PMCEIDn_EL0 > * PMMIR_EL1 > > PMOVS remains trapped so KVM can track overflow IRQs that will need to > be injected into the guest. > > PMICNTR and PMIFILTR remain trapped because KVM is not handling them > yet. > > PMEVTYPERn remains trapped so KVM can limit which events guests can > count, such as disallowing counting at EL2. PMCCFILTR and PMCIFILTR > are special cases of the same. > > PMCEIDn and PMMIR remain trapped because they can leak information > specific to the host hardware implementation. > > NOTE: This patch temporarily forces kvm_vcpu_pmu_is_partitioned() to > be false to prevent partial feature activation for easier debugging. > > Signed-off-by: Colton Lewis <[email protected]> > --- > arch/arm64/include/asm/kvm_pmu.h | 33 ++++++++++++++++++++++ > arch/arm64/kvm/config.c | 34 ++++++++++++++++++++-- > arch/arm64/kvm/pmu-direct.c | 48 ++++++++++++++++++++++++++++++++ > 3 files changed, 112 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_pmu.h > b/arch/arm64/include/asm/kvm_pmu.h > index 8887f39c25e60..7297a697a4a62 100644 > --- a/arch/arm64/include/asm/kvm_pmu.h > +++ b/arch/arm64/include/asm/kvm_pmu.h > @@ -96,6 +96,23 @@ u64 kvm_pmu_guest_counter_mask(struct arm_pmu *pmu); > void kvm_pmu_host_counters_enable(void); > void kvm_pmu_host_counters_disable(void); > > +#if !defined(__KVM_NVHE_HYPERVISOR__) > +bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu); > +bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu); > +#else > +static inline bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu) > +{ > + return false; > +} > + > +static inline bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu) > +{ > + return false; > +} > +#endif > +u64 kvm_pmu_fgt_bits(void); > +u64 kvm_pmu_fgt2_bits(void); > + > /* > * Updates the vcpu's view of the pmu events for this cpu. > * Must be called before every vcpu run after disabling interrupts, to ensure > @@ -135,6 +152,22 @@ static inline u64 kvm_pmu_get_counter_value(struct > kvm_vcpu *vcpu, > { > return 0; > } > +static inline bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu) > +{ > + return false; > +} > +static inline bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu) > +{ > + return false; > +} > +static inline u64 kvm_pmu_fgt_bits(void) > +{ > + return 0; > +} > +static inline u64 kvm_pmu_fgt2_bits(void) > +{ > + return 0; > +} > static inline void kvm_pmu_set_counter_value(struct kvm_vcpu *vcpu, > u64 select_idx, u64 val) {} > static inline void kvm_pmu_set_counter_value_user(struct kvm_vcpu *vcpu, > diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c > index 24bb3f36e9d59..064dc6aa06f76 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_pmu.h> > #include <asm/kvm_nested.h> > #include <asm/sysreg.h> > > @@ -1489,12 +1490,39 @@ 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 (kvm_vcpu_pmu_use_fgt(vcpu)) > + *vcpu_fgt(vcpu, HDFGRTR_EL2) |= kvm_pmu_fgt_bits();
Couple of suggestions. I'd rather see this conditioned on kvm_vcpu_pmu_is_partitioned() and get rid of the FGT predicate. After all, kvm_vcpu_load_fgt() already checks for the presence of FEAT_FGT first. Additionally, I'd prefer that the trap configuration is inline instead of done in a helper in some other file. Centralizing the FGT configuration here was very much intentional. The other reason for doing this is kvm_pmu_fgt_bits() assumes a 'positive' trap polarity, even though there are several cases where FGTs have a 'negative' priority (i.e. 0 => trap). Thanks, Oliver
