Hi Alex,
On 9/10/20 9:21 AM, Auger Eric wrote:
> 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?
Looking again at the switch code (__pmu_switch_to_guest), I see we just
enable/disable the event counters owned by either the host/guest. I
thought this was more involved. So now it is clear.
Thanks
Eric
>>
>> 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
>
