Hi Alex,
On 9/8/20 10:57 PM, Alexander Graf wrote:
> We currently pass through the number of PMU counters that we have available
> in hardware to guests. So if my host supports 10 concurrently active PMU
> counters, my guest will be able to spawn 10 counters as well.
>
> This is undesireable if we also want to use the PMU on the host for
> monitoring. In that case, we want to split the PMU between guest and
> host.
but don't we have a trap and emulate approach as opposed to current SPE
implementation?
>
> To help that case, let's add a PMU attr that allows us to limit the number
> of PMU counters that we expose. With this patch in place, user space can
> keep some counters free for host use.
>
> Signed-off-by: Alexander Graf <graf@xxxxxxxxxx>
>
> ---
>
> Because this patch touches the same code paths as the vPMU filtering one
> and the vPMU filtering generalized a few conditions in the attr path,
> I've based it on top. Please let me know if you want it independent instead.
>
> v1 -> v2:
>
> - Add documentation
> - Add read support
> ---
> Documentation/virt/kvm/devices/vcpu.rst | 25 +++++++++++++++++++++++++
> arch/arm64/include/uapi/asm/kvm.h | 7 ++++---
> arch/arm64/kvm/pmu-emul.c | 32 ++++++++++++++++++++++++++++++++
> arch/arm64/kvm/sys_regs.c | 5 +++++
> include/kvm/arm_pmu.h | 1 +
> 5 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/virt/kvm/devices/vcpu.rst b/Documentation/virt/kvm/devices/vcpu.rst
> index 203b91e93151..1a1c8d8c8b1d 100644
> --- a/Documentation/virt/kvm/devices/vcpu.rst
> +++ b/Documentation/virt/kvm/devices/vcpu.rst
> @@ -102,6 +102,31 @@ isn't strictly speaking an event. Filtering the cycle counter is possible
> using event 0x11 (CPU_CYCLES).
>
>
> +1.4 ATTRIBUTE: KVM_ARM_VCPU_PMU_V3_NUM_EVENTS
> +---------------------------------------------
> +
> +:Parameters: in kvm_device_attr.addr the address for the limit of concurrent
> + events is a pointer to an int
> +
> +:Returns:
> +
> + ======= ======================================================
> + -ENODEV: PMUv3 not supported
> + -EBUSY: PMUv3 already initialized
> + -EINVAL: Too large number of events
s/events/event counters
I see that in perf code indeed num_events is used for that but I think
for the end-user the event counter terminology is better as it fits the
ARM spec.
> + ======= ======================================================
> +
> +Reconfigure the limit of concurrent PMU events that the guest can monitor.
here also
> +This number is directly exposed as part of the PMCR_EL0 register.
Maybe quote the "N" field
> +
> +On vcpu creation, this attribute is set to the hardware limit of the current
> +platform. If you need to determine the hardware limit, you can read this
> +attribute before setting it.
> +
> +Restrictions: The default value for this property is the number of hardware
> +supported events. Only values that are smaller than the hardware limit can
event counters
> +be set.
> +
> 2. GROUP: KVM_ARM_VCPU_TIMER_CTRL
> =================================
>
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 7b1511d6ce44..db025c0b5a40 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -342,9 +342,10 @@ struct kvm_vcpu_events {
>
> /* Device Control API on vcpu fd */
> #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_PMU_V3_IRQ 0
> +#define KVM_ARM_VCPU_PMU_V3_INIT 1
> +#define KVM_ARM_VCPU_PMU_V3_FILTER 2
> +#define KVM_ARM_VCPU_PMU_V3_NUM_EVENTS 3
> #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/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 0458860bade2..c7915b95fec0 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -253,6 +253,8 @@ void kvm_pmu_vcpu_init(struct kvm_vcpu *vcpu)
>
> for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++)
> pmu->pmc[i].idx = i;
> +
> + pmu->num_events = perf_num_counters() - 1;
> }
>
> /**
> @@ -978,6 +980,25 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>
> return 0;
> }
> + case KVM_ARM_VCPU_PMU_V3_NUM_EVENTS: {
> + u64 mask = ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT;
> + int __user *uaddr = (int __user *)(long)attr->addr;
> + u32 num_events;
> +
> + if (get_user(num_events, uaddr))
> + return -EFAULT;
> +
> + if (num_events >= perf_num_counters())
> + return -EINVAL;
> +
> + vcpu->arch.pmu.num_events = num_events;
> +
> + num_events <<= ARMV8_PMU_PMCR_N_SHIFT;
> + __vcpu_sys_reg(vcpu, SYS_PMCR_EL0) &= ~mask;
> + __vcpu_sys_reg(vcpu, SYS_PMCR_EL0) |= num_events;
> +
> + return 0;
> + }
> case KVM_ARM_VCPU_PMU_V3_INIT:
> return kvm_arm_pmu_v3_init(vcpu);
> }
> @@ -1004,6 +1025,16 @@ int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
> irq = vcpu->arch.pmu.irq_num;
> return put_user(irq, uaddr);
> }
> + case KVM_ARM_VCPU_PMU_V3_NUM_EVENTS: {
> + int __user *uaddr = (int __user *)(long)attr->addr;
> + u32 num_events;
> +
> + if (!test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features))
> + return -ENODEV;
> +
> + num_events = vcpu->arch.pmu.num_events;
> + return put_user(num_events, uaddr);
> + }
> }
>
> return -ENXIO;
> @@ -1015,6 +1046,7 @@ int kvm_arm_pmu_v3_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
> case KVM_ARM_VCPU_PMU_V3_IRQ:
> case KVM_ARM_VCPU_PMU_V3_INIT:
> case KVM_ARM_VCPU_PMU_V3_FILTER:
> + case KVM_ARM_VCPU_PMU_V3_NUM_EVENTS:
> if (kvm_arm_support_pmu_v3() &&
> test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features))
> return 0;
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 20ab2a7d37ca..d51e39600bbd 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -672,6 +672,11 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> | (ARMV8_PMU_PMCR_MASK & 0xdecafbad)) & (~ARMV8_PMU_PMCR_E);
> if (!system_supports_32bit_el0())
> val |= ARMV8_PMU_PMCR_LC;
> +
> + /* Override number of event selectors */
> + val &= ~(ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT);
> + val |= (u32)vcpu->arch.pmu.num_events << ARMV8_PMU_PMCR_N_SHIFT;
> +
> __vcpu_sys_reg(vcpu, r->reg) = val;
> }
>
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 98cbfe885a53..ea3fc96a37d9 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -27,6 +27,7 @@ struct kvm_pmu {
> bool ready;
> bool created;
> bool irq_level;
> + u8 num_events;
> };
>
> #define kvm_arm_pmu_v3_ready(v) ((v)->arch.pmu.ready)
>
Thanks
Eric
