| Message ID | 20231013052901.170138-3-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: PMU event filtering fixes | expand |
On Fri, Oct 13, 2023 at 05:29:01AM +0000, Oliver Upton wrote: > Suzuki noticed that KVM's PMU emulation is oblivious to the NSU and NSK > event filter bits. On systems that have EL3 these bits modify the > filter behavior in non-secure EL0 and EL1, respectively. Even though the > kernel doesn't use these bits, it is entirely possible some other guest > OS does. > > Implement the behavior of NSU and NSK as it appears in the pseudocode. > > Reported-by: Suzuki K Poulose <suzuki.poulose@arm.com> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> This is obviously EL0, not EL1. Assuing there's no respin, I intend to fix up the shortlog after applying the series.
On Fri, 13 Oct 2023 06:29:01 +0100, Oliver Upton <oliver.upton@linux.dev> wrote: > > Suzuki noticed that KVM's PMU emulation is oblivious to the NSU and NSK > event filter bits. On systems that have EL3 these bits modify the > filter behavior in non-secure EL0 and EL1, respectively. Even though the > kernel doesn't use these bits, it is entirely possible some other guest > OS does. But what does it mean for KVM itself? We have no EL3 to speak of as far as a guest is concerned. And the moment we allow things like NSU/NSK to be set, why don't we allow M as well? Thanks, M.
On Wed, Oct 18, 2023 at 02:31:11PM +0100, Marc Zyngier wrote: > On Fri, 13 Oct 2023 06:29:01 +0100, > Oliver Upton <oliver.upton@linux.dev> wrote: > > > > Suzuki noticed that KVM's PMU emulation is oblivious to the NSU and NSK > > event filter bits. On systems that have EL3 these bits modify the > > filter behavior in non-secure EL0 and EL1, respectively. Even though the > > kernel doesn't use these bits, it is entirely possible some other guest > > OS does. > > But what does it mean for KVM itself? We have no EL3 to speak of as > far as a guest is concerned. And the moment we allow things like > NSU/NSK to be set, why don't we allow M as well? Yeah, we need to have a think about all these extra bits TBH. KVM doesn't filter the advertised ELs in PFR0, so from the guest POV both EL2 and EL3 could potentially be implemented by the vCPU. Based on that I think the bits at least need to be stateful, even though KVM's emulation will never let the guest count events in a higher EL. My patches aren't even consistent with the above statement, as NSH gets RES0 treatment and the NS{U,K} bits do not. So how about this: - If EL3 is advertised in the guest's ID registers NS{U,K}, and M can be set. NS{U,K} work as proposed, M is ignored in KVM emulation. - If EL2 is advertised in the guest's ID registers NSH can be set but is ignored in KVM emulation. Thoughts?
On Wed, 18 Oct 2023 19:24:11 +0100, Oliver Upton <oliver.upton@linux.dev> wrote: > > On Wed, Oct 18, 2023 at 02:31:11PM +0100, Marc Zyngier wrote: > > On Fri, 13 Oct 2023 06:29:01 +0100, > > Oliver Upton <oliver.upton@linux.dev> wrote: > > > > > > Suzuki noticed that KVM's PMU emulation is oblivious to the NSU and NSK > > > event filter bits. On systems that have EL3 these bits modify the > > > filter behavior in non-secure EL0 and EL1, respectively. Even though the > > > kernel doesn't use these bits, it is entirely possible some other guest > > > OS does. > > > > But what does it mean for KVM itself? We have no EL3 to speak of as > > far as a guest is concerned. And the moment we allow things like > > NSU/NSK to be set, why don't we allow M as well? > > Yeah, we need to have a think about all these extra bits TBH. > > KVM doesn't filter the advertised ELs in PFR0, so from the guest POV > both EL2 and EL3 could potentially be implemented by the vCPU. Based > on that I think the bits at least need to be stateful, even though KVM's > emulation will never let the guest count events in a higher EL. > > My patches aren't even consistent with the above statement, as NSH gets > RES0 treatment and the NS{U,K} bits do not. So how about this: > > - If EL3 is advertised in the guest's ID registers NS{U,K}, and M can > be set. NS{U,K} work as proposed, M is ignored in KVM emulation. > > - If EL2 is advertised in the guest's ID registers NSH can be set but > is ignored in KVM emulation. > > Thoughts? This would be consistent with the pseudocode (and what KVM can reasonably achieve at this stage). Care to respin it? Thanks, M.
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index f0d4a9ace5ad..d28e0e989c98 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -63,6 +63,7 @@ static u32 kvm_pmu_event_mask(struct kvm *kvm) u64 kvm_pmu_evtyper_mask(struct kvm *kvm) { return ARMV8_PMU_EXCLUDE_EL1 | ARMV8_PMU_EXCLUDE_EL0 | + ARMV8_PMU_EXCLUDE_NS_EL1 | ARMV8_PMU_EXCLUDE_NS_EL0 | kvm_pmu_event_mask(kvm); } @@ -590,6 +591,7 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc) struct perf_event *event; struct perf_event_attr attr; u64 eventsel, reg, data; + bool p, u, nsk, nsu; reg = counter_index_to_evtreg(pmc->idx); data = __vcpu_sys_reg(vcpu, reg); @@ -616,13 +618,18 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc) !test_bit(eventsel, vcpu->kvm->arch.pmu_filter)) return; + p = data & ARMV8_PMU_EXCLUDE_EL1; + u = data & ARMV8_PMU_EXCLUDE_EL0; + nsk = data & ARMV8_PMU_EXCLUDE_NS_EL1; + nsu = data & ARMV8_PMU_EXCLUDE_NS_EL0; + memset(&attr, 0, sizeof(struct perf_event_attr)); attr.type = arm_pmu->pmu.type; attr.size = sizeof(attr); attr.pinned = 1; attr.disabled = !kvm_pmu_counter_is_enabled(pmc); - attr.exclude_user = data & ARMV8_PMU_EXCLUDE_EL0 ? 1 : 0; - attr.exclude_kernel = data & ARMV8_PMU_EXCLUDE_EL1 ? 1 : 0; + attr.exclude_user = (u != nsu); + attr.exclude_kernel = (p != nsk); attr.exclude_hv = 1; /* Don't count EL2 events */ attr.exclude_host = 1; /* Don't count host events */ attr.config = eventsel; diff --git a/include/linux/perf/arm_pmuv3.h b/include/linux/perf/arm_pmuv3.h index e3899bd77f5c..b74e71da1fa7 100644 --- a/include/linux/perf/arm_pmuv3.h +++ b/include/linux/perf/arm_pmuv3.h @@ -234,9 +234,11 @@ /* * Event filters for PMUv3 */ -#define ARMV8_PMU_EXCLUDE_EL1 (1U << 31) -#define ARMV8_PMU_EXCLUDE_EL0 (1U << 30) -#define ARMV8_PMU_INCLUDE_EL2 (1U << 27) +#define ARMV8_PMU_EXCLUDE_EL1 (1U << 31) +#define ARMV8_PMU_EXCLUDE_EL0 (1U << 30) +#define ARMV8_PMU_EXCLUDE_NS_EL1 (1U << 29) +#define ARMV8_PMU_EXCLUDE_NS_EL0 (1U << 28) +#define ARMV8_PMU_INCLUDE_EL2 (1U << 27) /* * PMUSERENR: user enable reg
Suzuki noticed that KVM's PMU emulation is oblivious to the NSU and NSK event filter bits. On systems that have EL3 these bits modify the filter behavior in non-secure EL0 and EL1, respectively. Even though the kernel doesn't use these bits, it is entirely possible some other guest OS does. Implement the behavior of NSU and NSK as it appears in the pseudocode. Reported-by: Suzuki K Poulose <suzuki.poulose@arm.com> Signed-off-by: Oliver Upton <oliver.upton@linux.dev> --- arch/arm64/kvm/pmu-emul.c | 11 +++++++++-- include/linux/perf/arm_pmuv3.h | 8 +++++--- 2 files changed, 14 insertions(+), 5 deletions(-)