Hi Marc, On 9/8/20 9:58 AM, Marc Zyngier wrote: > 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). > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> > --- > arch/arm64/include/asm/kvm_host.h | 5 +++ > arch/arm64/include/uapi/asm/kvm.h | 16 +++++++ > arch/arm64/kvm/arm.c | 2 + > arch/arm64/kvm/pmu-emul.c | 70 ++++++++++++++++++++++++++++--- > 4 files changed, 87 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 6cd60be69c28..1e64260b7e2b 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -111,6 +111,11 @@ struct kvm_arch { > */ > bool return_nisv_io_abort_to_user; > > + /* > + * VM-wide PMU filter, implemented as a bitmap and big enough for > + * up to 2^10 events (ARMv8.0) or 2^16 events (ARMv8.1+). > + */ > + unsigned long *pmu_filter; > unsigned int pmuver; > }; > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index ba85bb23f060..7b1511d6ce44 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -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 > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 691d21e4c717..0f11d0009c17 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -145,6 +145,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm) > { > int i; > > + bitmap_free(kvm->arch.pmu_filter); > + > kvm_vgic_destroy(kvm); > > for (i = 0; i < KVM_MAX_VCPUS; ++i) { > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > index 8a5f65763814..67a731bafbc9 100644 > --- a/arch/arm64/kvm/pmu-emul.c > +++ b/arch/arm64/kvm/pmu-emul.c > @@ -30,6 +30,7 @@ static u32 kvm_pmu_event_mask(struct kvm *kvm) > case 6: /* ARMv8.5 */ > return GENMASK(15, 0); > default: /* Shouldn't be there, just for sanity */ > + WARN_ONCE(1, "Unknown PMU version %d\n", kvm->arch.pmuver); rather belongs to previous patch > return 0; > } > } > @@ -592,11 +593,21 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) > data = __vcpu_sys_reg(vcpu, reg); > > kvm_pmu_stop_counter(vcpu, pmc); > - eventsel = data & kvm_pmu_event_mask(vcpu->kvm);; > + if (pmc->idx == ARMV8_PMU_CYCLE_IDX) > + eventsel = ARMV8_PMUV3_PERFCTR_CPU_CYCLES; So from a filter pov the cycle counter is assimilated to the event counter CPU_CYCLES event (0x11). I see there are some differences between PMCCNTR and 0x11 counting (wrt PMCR, ...) though. Shouldn't we mention the cycle counter is getting filtered as well > + else > + eventsel = data & kvm_pmu_event_mask(vcpu->kvm); > + > + /* Software increment event doesn't need to be backed by a perf event */ > + if (eventsel == ARMV8_PMUV3_PERFCTR_SW_INCR) > + return; > > - /* Software increment event does't need to be backed by a perf event */ > - if (eventsel == ARMV8_PMUV3_PERFCTR_SW_INCR && > - pmc->idx != ARMV8_PMU_CYCLE_IDX) > + /* > + * 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; > > memset(&attr, 0, sizeof(struct perf_event_attr)); > @@ -608,8 +619,7 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) > attr.exclude_kernel = data & ARMV8_PMU_EXCLUDE_EL1 ? 1 : 0; > attr.exclude_hv = 1; /* Don't count EL2 events */ > attr.exclude_host = 1; /* Don't count host events */ > - attr.config = (pmc->idx == ARMV8_PMU_CYCLE_IDX) ? > - ARMV8_PMUV3_PERFCTR_CPU_CYCLES : eventsel; > + attr.config = eventsel; > > counter = kvm_pmu_get_pair_counter_value(vcpu, pmc); > > @@ -892,6 +902,53 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > vcpu->arch.pmu.irq_num = irq; > return 0; > } > + case KVM_ARM_VCPU_PMU_V3_FILTER: { > + struct kvm_pmu_event_filter __user *uaddr; > + struct kvm_pmu_event_filter filter; > + int nr_events; > + > + nr_events = kvm_pmu_event_mask(vcpu->kvm) + 1; > + > + uaddr = (struct kvm_pmu_event_filter __user *)(long)attr->addr; > + > + if (copy_from_user(&filter, uaddr, sizeof(filter))) > + return -EFAULT; > + > + if (((u32)filter.base_event + filter.nevents) > nr_events || > + (filter.action != KVM_PMU_EVENT_ALLOW && > + filter.action != KVM_PMU_EVENT_DENY)) > + return -EINVAL; > + > + mutex_lock(&vcpu->kvm->lock); > + > + if (!vcpu->kvm->arch.pmu_filter) { > + vcpu->kvm->arch.pmu_filter = bitmap_alloc(nr_events, GFP_KERNEL); > + if (!vcpu->kvm->arch.pmu_filter) { > + mutex_unlock(&vcpu->kvm->lock); > + return -ENOMEM; > + } > + > + /* > + * The default depends on the first applied filter. > + * If it allows events, the default is to deny. > + * Conversely, if the first filter denies a set of > + * events, the default is to allow. > + */ > + if (filter.action == KVM_PMU_EVENT_ALLOW) > + bitmap_zero(vcpu->kvm->arch.pmu_filter, nr_events); > + else > + bitmap_fill(vcpu->kvm->arch.pmu_filter, nr_events); > + } > + > + if (filter.action == KVM_PMU_EVENT_ALLOW) > + bitmap_set(vcpu->kvm->arch.pmu_filter, filter.base_event, filter.nevents); > + else > + bitmap_clear(vcpu->kvm->arch.pmu_filter, filter.base_event, filter.nevents); > + > + mutex_unlock(&vcpu->kvm->lock); > + > + return 0; > + } > case KVM_ARM_VCPU_PMU_V3_INIT: > return kvm_arm_pmu_v3_init(vcpu); > } > @@ -928,6 +985,7 @@ int kvm_arm_pmu_v3_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > switch (attr->attr) { > case KVM_ARM_VCPU_PMU_V3_IRQ: > case KVM_ARM_VCPU_PMU_V3_INIT: > + case KVM_ARM_VCPU_PMU_V3_FILTER: > if (kvm_arm_support_pmu_v3() && > test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features)) > return 0; > Thanks Eric _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm