Re: [PATCH] powerpc/hv-gpci: Fix hv_gpci event list

[Michael Ellerman <mpe@ellerman.id.au>]

Kajol Jain <kjain@linux.ibm.com> writes:
> Based on getPerfCountInfo v1.018 documentation, some of the
> hv_gpci events got deprecated for platforms firmware that
> supports counter_info_version 0x8 or above.
>
> Patch fixes the hv_gpci event list by adding a new attribute
> group called "hv_gpci_event_attrs_v6" and a "EVENT_ENABLE"
> macro to enable these events for platform firmware
> that supports counter_info_version 0x6 or below.

Does this handle CPUs booted in compat mode?

ie. where the firmware is newer but the kernel is told to behave as if
the CPU is an older version - so cpu_has_feature() doesn't necessarily
match the underlying hardware.

Is there some reason the event list is populated based on the CPU
features rather than by calling the hypervisor and asking what version
is supported?

> Fixes: 97bf2640184f4 ("powerpc/perf/hv-gpci: add the remaining gpci
> requests")

Please don't wrap the fixes tag.

cheers

> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
>  arch/powerpc/perf/hv-gpci-requests.h |  4 ++++
>  arch/powerpc/perf/hv-gpci.c          |  9 +++++++--
>  arch/powerpc/perf/hv-gpci.h          |  1 +
>  arch/powerpc/perf/req-gen/perf.h     | 17 +++++++++++++++++
>  4 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/perf/hv-gpci-requests.h b/arch/powerpc/perf/hv-gpci-requests.h
> index 8965b4463d43..baef3d082de9 100644
> --- a/arch/powerpc/perf/hv-gpci-requests.h
> +++ b/arch/powerpc/perf/hv-gpci-requests.h
> @@ -79,6 +79,7 @@ REQUEST(__field(0,	8,	partition_id)
>  )
>  #include I(REQUEST_END)
>  
> +#ifdef EVENT_ENABLE
>  /*
>   * Not available for counter_info_version >= 0x8, use
>   * run_instruction_cycles_by_partition(0x100) instead.
> @@ -92,6 +93,7 @@ REQUEST(__field(0,	8,	partition_id)
>  	__count(0x10,	8,	cycles)
>  )
>  #include I(REQUEST_END)
> +#endif
>  
>  #define REQUEST_NAME system_performance_capabilities
>  #define REQUEST_NUM 0x40
> @@ -103,6 +105,7 @@ REQUEST(__field(0,	1,	perf_collect_privileged)
>  )
>  #include I(REQUEST_END)
>  
> +#ifdef EVENT_ENABLE
>  #define REQUEST_NAME processor_bus_utilization_abc_links
>  #define REQUEST_NUM 0x50
>  #define REQUEST_IDX_KIND "hw_chip_id=?"
> @@ -194,6 +197,7 @@ REQUEST(__field(0,	4,	phys_processor_idx)
>  	__count(0x28,	8,	instructions_completed)
>  )
>  #include I(REQUEST_END)
> +#endif
>  
>  /* Processor_core_power_mode (0x95) skipped, no counters */
>  /* Affinity_domain_information_by_virtual_processor (0xA0) skipped,
> diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
> index 5eb60ed5b5e8..065a01812b3e 100644
> --- a/arch/powerpc/perf/hv-gpci.c
> +++ b/arch/powerpc/perf/hv-gpci.c
> @@ -70,9 +70,9 @@ static const struct attribute_group format_group = {
>  	.attrs = format_attrs,
>  };
>  
> -static const struct attribute_group event_group = {
> +static struct attribute_group event_group = {
>  	.name  = "events",
> -	.attrs = hv_gpci_event_attrs,
> +	/* .attrs is set in init */
>  };
>  
>  #define HV_CAPS_ATTR(_name, _format)				\
> @@ -353,6 +353,11 @@ static int hv_gpci_init(void)
>  	/* sampling not supported */
>  	h_gpci_pmu.capabilities |= PERF_PMU_CAP_NO_INTERRUPT;
>  
> +	if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +		event_group.attrs = hv_gpci_event_attrs;
> +	else
> +		event_group.attrs = hv_gpci_event_attrs_v6;
> +
>  	r = perf_pmu_register(&h_gpci_pmu, h_gpci_pmu.name, -1);
>  	if (r)
>  		return r;
> diff --git a/arch/powerpc/perf/hv-gpci.h b/arch/powerpc/perf/hv-gpci.h
> index 4d108262bed7..866172c1651c 100644
> --- a/arch/powerpc/perf/hv-gpci.h
> +++ b/arch/powerpc/perf/hv-gpci.h
> @@ -26,6 +26,7 @@ enum {
>  #define REQUEST_FILE "../hv-gpci-requests.h"
>  #define NAME_LOWER hv_gpci
>  #define NAME_UPPER HV_GPCI
> +#define EVENT_ENABLE	1
>  #include "req-gen/perf.h"
>  #undef REQUEST_FILE
>  #undef NAME_LOWER
> diff --git a/arch/powerpc/perf/req-gen/perf.h b/arch/powerpc/perf/req-gen/perf.h
> index fa9bc804e67a..78d407e3fcc6 100644
> --- a/arch/powerpc/perf/req-gen/perf.h
> +++ b/arch/powerpc/perf/req-gen/perf.h
> @@ -139,6 +139,23 @@ PMU_EVENT_ATTR_STRING(							\
>  #define REQUEST_(r_name, r_value, r_idx_1, r_fields)			\
>  	r_fields
>  
> +/* Generate event list for platforms with counter_info_version 0x6 or below */
> +static __maybe_unused struct attribute *hv_gpci_event_attrs_v6[] = {
> +#include REQUEST_FILE
> +	NULL
> +};
> +
> +/*
> + * Based on getPerfCountInfo v1.018 documentation, some of the hv-gpci
> + * events got deprecated for platforms firmware that supports
> + * counter_info_version 0x8 or above.
> + * Undefining macro EVENT_ENABLE, to disable the addition of deprecated
> + * events in "hv_gpci_event_attrs" attribute group, for platforms that
> + * supports counter_info_version 0x8 or above.
> + */
> +#undef EVENT_ENABLE
> +
> +/* Generate event list for platforms with counter_info_version 0x8 or above*/
>  static __maybe_unused struct attribute *hv_gpci_event_attrs[] = {
>  #include REQUEST_FILE
>  	NULL
> -- 
> 2.31.1