| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <gsntbjq89am2.fsf@coltonlewis-kvm.c.googlers.com>
Date: Fri, 27 Jun 2025 20:45:57 +0000
From: Colton Lewis <coltonlewis@...gle.com>
To: Marc Zyngier <maz@...nel.org>
Cc: kvm@...r.kernel.org, pbonzini@...hat.com, corbet@....net,
linux@...linux.org.uk, catalin.marinas@....com, will@...nel.org,
oliver.upton@...ux.dev, mizhang@...gle.com, joey.gouly@....com,
suzuki.poulose@....com, yuzenghui@...wei.com, mark.rutland@....com,
shuah@...nel.org, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev,
linux-perf-users@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v3 10/22] KVM: arm64: Set up FGT for Partitioned PMU
Marc Zyngier <maz@...nel.org> writes:
> On Thu, 26 Jun 2025 21:04:46 +0100,
> Colton Lewis <coltonlewis@...gle.com> 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.
>> There should be no information leaks between guests as all these
>> registers are context swapped by a later patch in this series.
>> Untrapped:
>> * PMCR_EL0
>> * PMUSERENR_EL0
>> * PMSELR_EL0
>> * PMCCNTR_EL0
>> * PMINTEN_EL0
>> * PMEVCNTRn_EL0
>> Trapped:
>> * PMOVS_EL0
>> * PMEVTYPERn_EL0
>> * PMCCFILTR_EL0
>> * PMICNTR_EL0
>> * PMICFILTR_EL0
>> PMOVS remains trapped so KVM can track overflow IRQs that will need to
>> be injected into the guest.
>> PMICNTR remains trapped because KVM is not handling that yet.
>> PMEVTYPERn remains trapped so KVM can limit which events guests can
>> count, such as disallowing counting at EL2. PMCCFILTR and PMCIFILTR
>> are the same.
> I'd rather you explain why it is safe not to trap the rest.
Okay, I will reverse my explanation.
>> Signed-off-by: Colton Lewis <coltonlewis@...gle.com>
>> ---
>> arch/arm64/include/asm/kvm_pmu.h | 23 ++++++++++
>> arch/arm64/kvm/hyp/include/hyp/switch.h | 58 +++++++++++++++++++++++++
>> arch/arm64/kvm/pmu-part.c | 32 ++++++++++++++
>> 3 files changed, 113 insertions(+)
>> diff --git a/arch/arm64/include/asm/kvm_pmu.h
>> b/arch/arm64/include/asm/kvm_pmu.h
>> index 6328e90952ba..73b7161e3f4e 100644
>> --- a/arch/arm64/include/asm/kvm_pmu.h
>> +++ b/arch/arm64/include/asm/kvm_pmu.h
>> @@ -94,6 +94,21 @@ 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
>> +
>> /*
>> * Updates the vcpu's view of the pmu events for this cpu.
>> * Must be called before every vcpu run after disabling interrupts, to
>> ensure
>> @@ -133,6 +148,14 @@ 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 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/hyp/include/hyp/switch.h
>> b/arch/arm64/kvm/hyp/include/hyp/switch.h
>> index 825b81749972..47d2db8446df 100644
>> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
>> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
>> @@ -191,6 +191,61 @@ static inline bool cpu_has_amu(void)
>> ID_AA64PFR0_EL1_AMU_SHIFT);
>> }
>> +/**
>> + * __activate_pmu_fgt() - Activate fine grain traps for partitioned PMU
>> + * @vcpu: Pointer to struct kvm_vcpu
>> + *
>> + * Clear the most commonly accessed registers for a partitioned
>> + * PMU. Trap the rest.
>> + */
>> +static inline void __activate_pmu_fgt(struct kvm_vcpu *vcpu)
>> +{
>> + struct kvm_cpu_context *hctxt = host_data_ptr(host_ctxt);
>> + struct kvm *kvm = kern_hyp_va(vcpu->kvm);
>> + u64 set;
>> + u64 clr;
>> +
>> + set = HDFGRTR_EL2_PMOVS
>> + | HDFGRTR_EL2_PMCCFILTR_EL0
>> + | HDFGRTR_EL2_PMEVTYPERn_EL0;
>> + clr = HDFGRTR_EL2_PMUSERENR_EL0
>> + | HDFGRTR_EL2_PMSELR_EL0
>> + | HDFGRTR_EL2_PMINTEN
>> + | HDFGRTR_EL2_PMCNTEN
>> + | HDFGRTR_EL2_PMCCNTR_EL0
>> + | HDFGRTR_EL2_PMEVCNTRn_EL0;
>> +
>> + update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGRTR_EL2, clr, set);
>> +
>> + set = HDFGWTR_EL2_PMOVS
>> + | HDFGWTR_EL2_PMCCFILTR_EL0
>> + | HDFGWTR_EL2_PMEVTYPERn_EL0;
>> + clr = HDFGWTR_EL2_PMUSERENR_EL0
>> + | HDFGWTR_EL2_PMCR_EL0
>> + | HDFGWTR_EL2_PMSELR_EL0
>> + | HDFGWTR_EL2_PMINTEN
>> + | HDFGWTR_EL2_PMCNTEN
>> + | HDFGWTR_EL2_PMCCNTR_EL0
>> + | HDFGWTR_EL2_PMEVCNTRn_EL0;
>> +
>> + update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGWTR_EL2, clr, set);
>> +
>> + if (!cpus_have_final_cap(ARM64_HAS_FGT2))
>> + return;
>> +
>> + set = HDFGRTR2_EL2_nPMICFILTR_EL0
>> + | HDFGRTR2_EL2_nPMICNTR_EL0;
>> + clr = 0;
>> +
>> + update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGRTR2_EL2, clr, set);
>> +
>> + set = HDFGWTR2_EL2_nPMICFILTR_EL0
>> + | HDFGWTR2_EL2_nPMICNTR_EL0;
>> + clr = 0;
>> +
>> + update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGWTR2_EL2, clr, set);
> This feels wrong. There should be one place to populate the FGTs that
> apply to a guest as set from the host, not two or more.
> There is such a construct in the SME series, and maybe you could have
> a look at it, specially if the trap configuration is this static.
> M.
> --
> Without deviation from the norm, progress is not possible.
I'm assuming you are referring to Mark Brown's series [1], specifically
patches 5 and 18 and I see what you mean.
You are probably thinking configuration should happen from
sys_regs.c:kvm_calculate_traps or thereabout and should be setting bits
in the existing kvm->arch.fgt array.
Correct me if I'm mistaken.
[1]
https://lore.kernel.org/kvm/20250625-kvm-arm64-sme-v6-0-114cff4ffe04@kernel.org/
Powered by blists - more mailing lists