Re: [PATCH v4 6/6] powerpc/kvm-hv-pmu: Add perf-events for Hostwide counters

[Vaibhav Jain <vaibhav@linux.ibm.com>]

Athira Rajeev <atrajeev@linux.ibm.com> writes:

>> On 24 Feb 2025, at 6:45 PM, Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>> 
>> Update 'kvm-hv-pmu.c' to add five new perf-events mapped to the five
>> Hostwide counters. Since these newly introduced perf events are at system
>> wide scope and can be read from any L1-Lpar CPU, 'kvmppc_pmu' scope and
>> capabilities are updated appropriately.
>> 
>> Also introduce two new helpers. First is kvmppc_update_l0_stats() that uses
>> the infrastructure introduced in previous patches to issues the
>> H_GUEST_GET_STATE hcall L0-PowerVM to fetch guest-state-buffer holding the
>> latest values of these counters which is then parsed and 'l0_stats'
>> variable updated.
>> 
>> Second helper is kvmppc_pmu_event_update() which is called from
>> 'kvmppv_pmu' callbacks and uses kvmppc_update_l0_stats() to update
>> 'l0_stats' and the update the 'struct perf_event's event-counter.
>> 
>> Some minor updates to kvmppc_pmu_{add, del, read}() to remove some debug
>> scaffolding code.
>> 
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Changelog
>> 
>> v3->v4:
>> * Minor tweaks to patch description and code as its now being built as a
>> separate kernel module.
>> 
>> v2->v3:
>> None
>> 
>> v1->v2:
>> None
>> ---
>> arch/powerpc/perf/kvm-hv-pmu.c | 92 +++++++++++++++++++++++++++++++++-
>> 1 file changed, 91 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/perf/kvm-hv-pmu.c b/arch/powerpc/perf/kvm-hv-pmu.c
>> index ed371454f7b5..274459bb32d6 100644
>> --- a/arch/powerpc/perf/kvm-hv-pmu.c
>> +++ b/arch/powerpc/perf/kvm-hv-pmu.c
>> @@ -30,6 +30,11 @@
>> #include "asm/guest-state-buffer.h"
>> 
>> enum kvmppc_pmu_eventid {
>> + KVMPPC_EVENT_HOST_HEAP,
>> + KVMPPC_EVENT_HOST_HEAP_MAX,
>> + KVMPPC_EVENT_HOST_PGTABLE,
>> + KVMPPC_EVENT_HOST_PGTABLE_MAX,
>> + KVMPPC_EVENT_HOST_PGTABLE_RECLAIM,
>> KVMPPC_EVENT_MAX,
>> };
>> 
>> @@ -61,8 +66,14 @@ static DEFINE_SPINLOCK(lock_l0_stats);
>> /* GSB related structs needed to talk to L0 */
>> static struct kvmppc_gs_msg *gsm_l0_stats;
>> static struct kvmppc_gs_buff *gsb_l0_stats;
>> +static struct kvmppc_gs_parser gsp_l0_stats;
>> 
>> static struct attribute *kvmppc_pmu_events_attr[] = {
>> + KVMPPC_PMU_EVENT_ATTR(host_heap, KVMPPC_EVENT_HOST_HEAP),
>> + KVMPPC_PMU_EVENT_ATTR(host_heap_max, KVMPPC_EVENT_HOST_HEAP_MAX),
>> + KVMPPC_PMU_EVENT_ATTR(host_pagetable, KVMPPC_EVENT_HOST_PGTABLE),
>> + KVMPPC_PMU_EVENT_ATTR(host_pagetable_max, KVMPPC_EVENT_HOST_PGTABLE_MAX),
>> + KVMPPC_PMU_EVENT_ATTR(host_pagetable_reclaim, KVMPPC_EVENT_HOST_PGTABLE_RECLAIM),
>> NULL,
>> };
>> 
>> @@ -71,7 +82,7 @@ static const struct attribute_group kvmppc_pmu_events_group = {
>> .attrs = kvmppc_pmu_events_attr,
>> };
>> 
>> -PMU_FORMAT_ATTR(event, "config:0");
>> +PMU_FORMAT_ATTR(event, "config:0-5");
>> static struct attribute *kvmppc_pmu_format_attr[] = {
>> &format_attr_event.attr,
>> NULL,
>> @@ -88,6 +99,79 @@ static const struct attribute_group *kvmppc_pmu_attr_groups[] = {
>> NULL,
>> };
>> 
>> +/*
>> + * Issue the hcall to get the L0-host stats.
>> + * Should be called with l0-stat lock held
>> + */
>> +static int kvmppc_update_l0_stats(void)
>> +{
>> + int rc;
>> +
>> + /* With HOST_WIDE flags guestid and vcpuid will be ignored */
>> + rc = kvmppc_gsb_recv(gsb_l0_stats, KVMPPC_GS_FLAGS_HOST_WIDE);
>> + if (rc)
>> + goto out;
>> +
>> + /* Parse the guest state buffer is successful */
>> + rc = kvmppc_gse_parse(&gsp_l0_stats, gsb_l0_stats);
>> + if (rc)
>> + goto out;
>> +
>> + /* Update the l0 returned stats*/
>> + memset(&l0_stats, 0, sizeof(l0_stats));
>> + rc = kvmppc_gsm_refresh_info(gsm_l0_stats, gsb_l0_stats);
>> +
>> +out:
>> + return rc;
>> +}
>> +
>> +/* Update the value of the given perf_event */
>> +static int kvmppc_pmu_event_update(struct perf_event *event)
>> +{
>> + int rc;
>> + u64 curr_val, prev_val;
>> + unsigned long flags;
>> + unsigned int config = event->attr.config;
>> +
>> + /* Ensure no one else is modifying the l0_stats */
>> + spin_lock_irqsave(&lock_l0_stats, flags);
>> +
>> + rc = kvmppc_update_l0_stats();
>> + if (!rc) {
>> + switch (config) {
>> + case KVMPPC_EVENT_HOST_HEAP:
>> + curr_val = l0_stats.guest_heap;
>> + break;
>> + case KVMPPC_EVENT_HOST_HEAP_MAX:
>> + curr_val = l0_stats.guest_heap_max;
>> + break;
>> + case KVMPPC_EVENT_HOST_PGTABLE:
>> + curr_val = l0_stats.guest_pgtable_size;
>> + break;
>> + case KVMPPC_EVENT_HOST_PGTABLE_MAX:
>> + curr_val = l0_stats.guest_pgtable_size_max;
>> + break;
>> + case KVMPPC_EVENT_HOST_PGTABLE_RECLAIM:
>> + curr_val = l0_stats.guest_pgtable_reclaim;
>> + break;
>> + default:
>> + rc = -ENOENT;
>> + break;
>> + }
>> + }
>> +
>> + spin_unlock_irqrestore(&lock_l0_stats, flags);
>> +
>> + /* If no error than update the perf event */
>> + if (!rc) {
>> + prev_val = local64_xchg(&event->hw.prev_count, curr_val);
>> + if (curr_val > prev_val)
>> + local64_add(curr_val - prev_val, &event->count);
>> + }
>> +
>> + return rc;
>> +}
>> +
>> static int kvmppc_pmu_event_init(struct perf_event *event)
>> {
>> unsigned int config = event->attr.config;
>> @@ -110,15 +194,19 @@ static int kvmppc_pmu_event_init(struct perf_event *event)
>> 
>> static void kvmppc_pmu_del(struct perf_event *event, int flags)
>> {
>> + /* Do nothing */
>> }
>
> If we don’t read the counter stats in “del” call back, we will loose the final count getting updated, right ?
> Del callback needs to call kvmppc_pmu_read. Can you check the difference in count stats by calling kvmppc_pmu_read here ?
>

Yes, agreed. Will address this in next version of the patch series

> Thanks
> Athira
>
>> 
>> static int kvmppc_pmu_add(struct perf_event *event, int flags)
>> {
>> + if (flags & PERF_EF_START)
>> + return kvmppc_pmu_event_update(event);
>> return 0;
>> }
>> 
>> static void kvmppc_pmu_read(struct perf_event *event)
>> {
>> + kvmppc_pmu_event_update(event);
>> }
>> 
>> /* Return the size of the needed guest state buffer */
>> @@ -302,6 +390,8 @@ static struct pmu kvmppc_pmu = {
>> .read = kvmppc_pmu_read,
>> .attr_groups = kvmppc_pmu_attr_groups,
>> .type = -1,
>> + .scope = PERF_PMU_SCOPE_SYS_WIDE,
>> + .capabilities = PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT,
>> };
>> 
>> static int __init kvmppc_register_pmu(void)
>> -- 
>> 2.48.1
>> 
>> 
>> 
>
>

-- 
Cheers
~ Vaibhav