Kajol Jain <kj...@linux.ibm.com> writes:
> Running event
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/
> in one of the system throws below error:
>
> ---Logs---
> # perf list | grep
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles
>
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=?/[Kernel
> PMU event]
>
>
> # perf stat -v -e
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/
> sleep 2
> Using CPUID 00800200
> Control descriptor is not initialized
> Warning:
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/
> event is not supported by the kernel.
> failed to read counter
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/
>
> Performance counter stats for 'system wide':
>
> <not supported>
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/
>
> 2.000700771 seconds time elapsed
>
> The above error is because of the hcall failure as required
> permission "Enable Performance Information Collection" is not set.
> Based on current code, single_gpci_request function did not check the
> error type incase hcall fails and by default returns EINVAL. But we can
> have other reasons for hcall failures like H_AUTHORITY/H_PARAMETER for which
> we need to act accordingly.
> Fix this issue by adding new checks in the single_gpci_request function.
>
> Result after fix patch changes:
>
> # perf stat -e
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/
> sleep 2
> Error:
> No permission to enable
> hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/
> event.
>
> Fixes: 220a0c609ad1 ("powerpc/perf: Add support for the hv gpci (get
> performance counter info) interface")
> Reported-by: Akanksha J N <akank...@linux.ibm.com>
> Signed-off-by: Kajol Jain <kj...@linux.ibm.com>
> ---
> arch/powerpc/perf/hv-gpci.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
> index 27f18119fda1..101060facd81 100644
> --- a/arch/powerpc/perf/hv-gpci.c
> +++ b/arch/powerpc/perf/hv-gpci.c
> @@ -695,7 +695,17 @@ static unsigned long single_gpci_request(u32 req, u32
> starting_index,
>
> ret = plpar_hcall_norets(H_GET_PERF_COUNTER_INFO,
> virt_to_phys(arg), HGPCI_REQ_BUFFER_SIZE);
> - if (ret) {
> +
> + /*
> + * ret value as 'H_PARAMETER' corresponds to 'GEN_BUF_TOO_SMALL',
Don't we expect H_PARAMETER if any parameter value is incorrect?
> + * which means that the current buffer size cannot accommodate
> + * all the information and a partial buffer returned.
I don't see how we can infer that H_PARAMETER means the buffer is too
small and accessing the first entry is OK?
cheers
> + * Since in this function we are only accessing data for a given
> starting index,
> + * we don't need to accommodate whole data and can get required count by
> + * accessing very first entry.
> + * Hence hcall fails only incase the ret value is other than H_SUCCESS
> or H_PARAMETER.
> + */
> + if (ret && (ret != H_PARAMETER)) {
> pr_devel("hcall failed: 0x%lx\n", ret);
> goto out;
> }
