Skip to contentSkip to conversation

KVM: arm64: Filtering PMU events

PATCH Superseded Marc Zyngier 2 participants 7 messages Feb 14, 2020 – Feb 17, 2020

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

cover KVM: arm64: Filtering PMU events

It is at times necessary to prevent a guest from being able to sample certain events if multiple CPUs share resources such as a cache level. In this case, it would be interesting if the VMM could simply prevent certain events from being counted instead of simply not exposing a PMU.

Given that most events are not architected, there is no easy way to designate which events shouldn't be counted other than specifying the raw event number.

Since I have no idea whether it is better to use an event whitelist or blacklist, the proposed API takes a cue from the x86 version and allows either allowing or denying counting of ranges of events. The event space being pretty large (16bits on ARMv8.1), the default policy is set by the first filter that gets installed (default deny if we first allow, default allow if we first deny).

The filter state is global to the guest, despite the PMU being per CPU. I'm not sure whether it would be worth it making it CPU-private.

Anyway, I'd be interesting in comments on how people would use this. I'll try to push a patch against kvmtool that implement this shortly (what I have currently is a harcoded set of hacks).

Marc Zyngier (2):
KVM: arm64: Add PMU event filtering infrastructure
KVM: arm64: Document PMU filtering API

 Documentation/virt/kvm/devices/vcpu.txt | 28 +++++++++
 arch/arm64/include/asm/kvm_host.h       |  6 ++
 arch/arm64/include/uapi/asm/kvm.h       | 16 ++++++
 virt/kvm/arm/arm.c                      |  2 +
 virt/kvm/arm/pmu.c                      | 76 +++++++++++++++++++++----
 5 files changed, 116 insertions(+), 12 deletions(-)
Signature · 1 line
2.20.1
author maintainer
#1 lore raw

PATCH 1/2 KVM: arm64: Add PMU event filtering infrastructure

It can be desirable to expose a PMU to a guest, and yet not want the guest to be able to count some of the implemented events (because this would give information on shared resources, for example.

For this, let's extend the PMUv3 device API, and offer a way to setup a bitmap of the allowed events (the default being no bitmap, and thus no filtering).

Userspace can thus allow/deny ranges of event. The default policy depends on the "polarity" of the first filter setup (default deny if the filter allows events, and default allow if the filter denies events). This allows to setup exactly what is allowed for a given guest.

Note that although the ioctl is per-vcpu, the map of allowed events is global to the VM (it can be setup from any vcpu until the vcpu PMU is initialized).

---
 arch/arm64/include/asm/kvm_host.h |  6 +++
 arch/arm64/include/uapi/asm/kvm.h | 16 +++++++
 virt/kvm/arm/arm.c                |  2 +
 virt/kvm/arm/pmu.c                | 76 ++++++++++++++++++++++++++-----
 4 files changed, 88 insertions(+), 12 deletions(-)
arch/arm64/include/asm/kvm_host.h +6 -0
index d87aa609d2b6..b594c604215f 100644
@@ -91,6 +91,12 @@ struct kvm_arch {
 	 * supported.
 	 */
 	bool return_nisv_io_abort_to_user;
+
+	/*
+	 * VM-wide PMU filter, implemented as a bitmap and big enough
+	 * for up to 65536 events
+	 */
+	unsigned long *pmu_filter;
 };
 
 #define KVM_NR_MEM_OBJS     40
arch/arm64/include/uapi/asm/kvm.h +16 -0
index ba85bb23f060..7b1511d6ce44 100644
@@ -159,6 +159,21 @@ struct kvm_sync_regs {
 struct kvm_arch_memory_slot {
 };
 
+/*
+ * PMU filter structure. Describe a range of events with a particular
+ * action. To be used with KVM_ARM_VCPU_PMU_V3_FILTER.
+ */
+struct kvm_pmu_event_filter {
+	__u16	base_event;
+	__u16	nevents;
+
+#define KVM_PMU_EVENT_ALLOW	0
+#define KVM_PMU_EVENT_DENY	1
+
+	__u8	action;
+	__u8	pad[3];
+};
+
 /* for KVM_GET/SET_VCPU_EVENTS */
 struct kvm_vcpu_events {
 	struct {
@@ -329,6 +344,7 @@ struct kvm_vcpu_events {
 #define KVM_ARM_VCPU_PMU_V3_CTRL	0
 #define   KVM_ARM_VCPU_PMU_V3_IRQ	0
 #define   KVM_ARM_VCPU_PMU_V3_INIT	1
+#define   KVM_ARM_VCPU_PMU_V3_FILTER	2
 #define KVM_ARM_VCPU_TIMER_CTRL		1
 #define   KVM_ARM_VCPU_TIMER_IRQ_VTIMER		0
 #define   KVM_ARM_VCPU_TIMER_IRQ_PTIMER		1
virt/kvm/arm/arm.c +2 -0
index d65a0faa46d8..9132f7508975 100644
@@ -164,6 +164,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	free_percpu(kvm->arch.last_vcpu_ran);
 	kvm->arch.last_vcpu_ran = NULL;
 
+	bitmap_free(kvm->arch.pmu_filter);
+
 	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
 		if (kvm->vcpus[i]) {
 			kvm_vcpu_destroy(kvm->vcpus[i]);
virt/kvm/arm/pmu.c +64 -12

114 lines not on this page · raw message

Signature · 1 line
2.20.1
#2 lore raw

Re: [PATCH 1/2] KVM: arm64: Add PMU event filtering infrastructure

Hi Marc,

On 2020-02-14 6:36 pm, Marc Zyngier wrote:
[...]

virt/kvm/arm/pmu.c static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) · line 594

Quoted 11 lines
virt/kvm/arm/pmu.c +7 -0
@@ -585,6 +585,14 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx)
  	    pmc->idx != ARMV8_PMU_CYCLE_IDX)
  		return;
 
+	/*
+	 * If we have a filter in place and that the event isn't allowed, do
+	 * not install a perf event either.
+	 */
+	if (vcpu->kvm->arch.pmu_filter &&
+	    !test_bit(eventsel, vcpu->kvm->arch.pmu_filter))
+		return;

If I'm reading the derivation of eventsel right, this will end up treating cycle counter events (aliased to SW_INCR) differently from CPU_CYCLES, which doesn't seem desirable.

Also, if the user did try to blacklist SW_INCR for ridiculous reasons, we'd need to special-case kvm_pmu_software_increment() to make it (not) work as expected, right?

Robin.

virt/kvm/arm/pmu.c static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) · line 598

Quoted 4 lines
virt/kvm/arm/pmu.c +1 -0
+
  	memset(&attr, 0, sizeof(struct perf_event_attr));
  	attr.type = PERF_TYPE_RAW;
  	attr.size = sizeof(attr);

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

author maintainer
#3 lore raw

Re: [PATCH 1/2] KVM: arm64: Add PMU event filtering infrastructure

On Fri, 14 Feb 2020 22:01:01 +0000,
Robin Murphy <[email protected]> wrote:

Hi Robin,

Quoted 5 lines

Hi Marc,

On 2020-02-14 6:36 pm, Marc Zyngier wrote:
[...]

Quoted 14 lines

virt/kvm/arm/pmu.c static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) · line 593

Quoted 10 lines
virt/kvm/arm/pmu.c +6 -0
@@ -585,6 +585,14 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx)
  	    pmc->idx != ARMV8_PMU_CYCLE_IDX)
  		return;
  +	/*
+	 * If we have a filter in place and that the event isn't allowed, do
+	 * not install a perf event either.
+	 */
+	if (vcpu->kvm->arch.pmu_filter &&
+	    !test_bit(eventsel, vcpu->kvm->arch.pmu_filter))
+		return;

If I'm reading the derivation of eventsel right, this will end up treating cycle counter events (aliased to SW_INCR) differently from CPU_CYCLES, which doesn't seem desirable.

Indeed, this doesn't look quite right.

Looking at the description of event 0x11, it doesn't seem to count exactly like the cycle counter (there are a number of PMCR controls affecting it). But none of these actually apply to our PMU emulation (no secure mode, and the idea of dealing with virtual EL2 in the context of the PMU is... not appealing).

Now, given that we implement the cycle counter with event 0x11 anyway, I don't think there is any reason to deal with them separately.

Quoted 3 lines

Also, if the user did try to blacklist SW_INCR for ridiculous reasons, we'd need to special-case kvm_pmu_software_increment() to make it (not) work as expected, right?

I thought of that one, and couldn't see a reason to blacklist it (after all, the guest could also increment a variable) and send itself an interrupt. I'm tempted to simply document that event 0 is never filtered.

Thanks,

	M.
Signature · 1 line
Jazz is not dead, it just smells funny.
#4 lore raw

Re: [PATCH 1/2] KVM: arm64: Add PMU event filtering infrastructure

Quoted 27 lines · Marc Zyngier

On Fri, 14 Feb 2020 22:01:01 +0000,
Robin Murphy <[email protected]> wrote:

Hi Robin,

Quoted 5 lines

Hi Marc,

On 2020-02-14 6:36 pm, Marc Zyngier wrote:
[...]

Quoted 14 lines

virt/kvm/arm/pmu.c static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) · line 593

Quoted 10 lines
virt/kvm/arm/pmu.c +6 -0
@@ -585,6 +585,14 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx)
   	    pmc->idx != ARMV8_PMU_CYCLE_IDX)
   		return;
   +	/*
+	 * If we have a filter in place and that the event isn't allowed, do
+	 * not install a perf event either.
+	 */
+	if (vcpu->kvm->arch.pmu_filter &&
+	    !test_bit(eventsel, vcpu->kvm->arch.pmu_filter))
+		return;

If I'm reading the derivation of eventsel right, this will end up treating cycle counter events (aliased to SW_INCR) differently from CPU_CYCLES, which doesn't seem desirable.

Indeed, this doesn't look quite right.

Quoted 8 lines

Looking at the description of event 0x11, it doesn't seem to count exactly like the cycle counter (there are a number of PMCR controls affecting it). But none of these actually apply to our PMU emulation (no secure mode, and the idea of dealing with virtual EL2 in the context of the PMU is... not appealing).

Now, given that we implement the cycle counter with event 0x11 anyway, I don't think there is any reason to deal with them separately.

Right, from the user's PoV they can only ask for event 0x11, and where it gets scheduled is more of a black-box implementation detail. Reading the Arm ARM doesn't leave me entirely convinced that cycles couldn't ever leak idle/not-idle information between closely-coupled PEs, so this might not be entirely academic.

Quoted 8 lines
Quoted 3 lines

Also, if the user did try to blacklist SW_INCR for ridiculous reasons, we'd need to special-case kvm_pmu_software_increment() to make it (not) work as expected, right?

I thought of that one, and couldn't see a reason to blacklist it (after all, the guest could also increment a variable) and send itself an interrupt. I'm tempted to simply document that event 0 is never filtered.

I'd say you're on even stronger ground simply because KVM's implementation of SW_INCR doesn't go near the PMU hardware at all, thus is well beyond the purpose of the blacklist anyway. I believe it's important that how the code behaves matches expectations, but there's no harm in changing the latter as appropriate ;)

Cheers,
Robin.

author maintainer
#5 lore raw

PATCH 2/2 KVM: arm64: Document PMU filtering API

In reply to Marc Zyngier

Add a small blurb describing how the event filtering API gets used.

---
 Documentation/virt/kvm/devices/vcpu.txt | 28 +++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
Documentation/virt/kvm/devices/vcpu.txt +28 -0
index 6f3bd64a05b0..76902e6bbc05 100644
@@ -36,6 +36,34 @@ Request the initialization of the PMUv3.  If using the PMUv3 with an in-kernel
 virtual GIC implementation, this must be done after initializing the in-kernel
 irqchip.
 
+1.3 ATTRIBUTE: KVM_ARM_VCPU_PMU_V3_FILTER
+Parameters: in kvm_device_attr.addr the address for a PMU event filter is a
+            pointer to a struct kvm_pmu_event_filter
+Returns: -ENODEV: PMUv3 not supported or GIC not initialized
+         -ENXIO: PMUv3 not properly configured or in-kernel irqchip not
+                 configured as required prior to calling this attribute
+         -EBUSY: PMUv3 already initialized
+
+Request the installation of a PMU event filter describe as follows:
+
+struct kvm_pmu_event_filter {
+	__u16	base_event;
+	__u16	nevents;
+
+#define KVM_PMU_EVENT_ALLOW	0
+#define KVM_PMU_EVENT_DENY	1
+
+	__u8	action;
+	__u8	pad[3];
+};
+
+A filter range is defined as the range [base_event, base_event + nevents[,
+together with an action (KVM_PMU_EVENT_ALLOW or KVM_PMU_EVENT_DENY). The
+first registered range defines the global policy (global ALLOW if the first
+action is DENY, global DENY if the first action is ALLOW). Multiple ranges
+can be programmed, and but fit within the 16bit space defined by the ARMv8.1
+PMU architecture.
+
 
 2. GROUP: KVM_ARM_VCPU_TIMER_CTRL
 Architectures: ARM,ARM64
Signature · 1 line
2.20.1
author maintainer
#6 lore raw
In reply to Marc Zyngier
Quoted 16 lines · On Fri, 14 Feb 2020 18:36:13 +0000 Marc Zyngier <[email protected]> wrote:

It is at times necessary to prevent a guest from being able to sample certain events if multiple CPUs share resources such as a cache level. In this case, it would be interesting if the VMM could simply prevent certain events from being counted instead of simply not exposing a PMU.

Given that most events are not architected, there is no easy way to designate which events shouldn't be counted other than specifying the raw event number.

Since I have no idea whether it is better to use an event whitelist or blacklist, the proposed API takes a cue from the x86 version and allows either allowing or denying counting of ranges of events. The event space being pretty large (16bits on ARMv8.1), the default policy is set by the first filter that gets installed (default deny if we first allow, default allow if we first deny).

Quoted 6 lines

The filter state is global to the guest, despite the PMU being per CPU. I'm not sure whether it would be worth it making it CPU-private.

Anyway, I'd be interesting in comments on how people would use this. I'll try to push a patch against kvmtool that implement this shortly (what I have currently is a harcoded set of hacks).

I now have a small extension to kvmtool allowing a --pmu-filter option to be passed on the command line (see [1]).

I've also pushed out an update[2] to the kernel side of things, making the filtering of the cycle counter consistent and documenting that neither SW_INCR nor CHAIN could be filtered with this mechanism (but this is of course up for discussion).

Thanks,

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/commit/?h=pmu-filter
[2] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pmu-event-filter

Signature · 5 lines
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm