[PATCH v2] powerpc/hv-gpci: Fix the H_GET_PERF_COUNTER_INFO hcall return value checks
Kajol Jain
kjain at linux.ibm.com
Thu Feb 29 23:28:47 AEDT 2024
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 with
detail_rc as GEN_BUF_TOO_SMALL, for which we need to act accordingly.
Fix this issue by adding new checks in the single_gpci_request and
h_gpci_event_init functions.
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 <akanksha at linux.ibm.com>
Signed-off-by: Kajol Jain <kjain at linux.ibm.com>
---
Changelog:
v1 -> v2
- To make sure the hcall failure with H_PARAMETER is only because of
buffer size issue, add check for detail_rc value to be
GEN_BUF_TOO_SMALL(0x1B) as suggested by Michael Ellerman.
arch/powerpc/perf/hv-gpci.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 27f18119fda1..89bfdc2ce8bc 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -695,6 +695,20 @@ 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);
+
+ /*
+ * ret value as 'H_PARAMETER' with detail_rc as 'GEN_BUF_TOO_SMALL',
+ * specifies that the current buffer size cannot accommodate
+ * all the information and a partial buffer returned.
+ * 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 first entry data.
+ * Hence hcall fails only incase the ret value is other than H_SUCCESS or
+ * H_PARAMETER with detail_rc value as GEN_BUF_TOO_SMALL(0x1B).
+ */
+ if (ret == H_PARAMETER && be32_to_cpu(arg->params.detail_rc) == 0x1B)
+ ret = 0;
+
if (ret) {
pr_devel("hcall failed: 0x%lx\n", ret);
goto out;
@@ -759,6 +773,7 @@ static int h_gpci_event_init(struct perf_event *event)
{
u64 count;
u8 length;
+ unsigned long ret;
/* Not our event */
if (event->attr.type != event->pmu->type)
@@ -789,13 +804,23 @@ static int h_gpci_event_init(struct perf_event *event)
}
/* check if the request works... */
- if (single_gpci_request(event_get_request(event),
+ ret = single_gpci_request(event_get_request(event),
event_get_starting_index(event),
event_get_secondary_index(event),
event_get_counter_info_version(event),
event_get_offset(event),
length,
- &count)) {
+ &count);
+
+ /*
+ * ret value as H_AUTHORITY implies that partition is not permitted to retrieve
+ * performance information, and required to set
+ * "Enable Performance Information Collection" option.
+ */
+ if (ret == H_AUTHORITY)
+ return -EPERM;
+
+ if (ret) {
pr_devel("gpci hcall failed\n");
return -EINVAL;
}
--
2.43.0
More information about the Linuxppc-dev
mailing list