git: 051f41ddb517 - stable/13 - libpmc: Handle PMCALLOCATE log with PMC code on PMU event system

The branch stable/13 has been updated by jrtc27:

URL: https://cgit.FreeBSD.org/src/commit/?id=051f41ddb517a9d3f6872678ccc3d0b6c0fffca1

commit 051f41ddb517a9d3f6872678ccc3d0b6c0fffca1
Author:     Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2023-06-07 14:21:18 +0000
Commit:     Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2023-08-01 20:42:53 +0000

    libpmc: Handle PMCALLOCATE log with PMC code on PMU event system
    
    On an arm64 system that reports as a Cortex A72 r0p3, running
    
      pmcstat -P CPU_CYCLES command
    
    works, but
    
      pmcstat -P cpu-cycles command
    
    does not. This is because the former uses the PMU event from the JSON
    source, resulting in pl_event in the log event being a small index
    (here, 5) into the generated events table, whilst the latter does not
    match any of the JSON events and falls back on PMC's own tables, mapping
    it to the PMC event 0x14111, i.e. PMC_EV_ARMV8_EVENT_11H. Then, when
    libpmc gets the PMCALLOCATE event, it tries to use the event as an index
    into the JSON-derived table, but doing so only makes sense for the
    former, whilst for the latter it will go way out of bounds and either
    read junk (which may trigger the != NULL assertion) or segfault. As far
    as I can tell we don't have anything lying around to tell us which of
    the two cases we're in, but we can exploit the fact that the first
    0x1000 PMC event codes are reserved, and that none of our PMU events
    tables reach that number of entries yet.
    
    PR:             268857
    Reviewed by:    mhorne
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D39592
    
    (cherry picked from commit 21f7397a61f7bff61a1221cc6340cd980a922540)
---
 lib/libpmc/libpmc.c |  9 ++++++++-
 lib/libpmc/pmclog.c | 27 +++++++++++++++++++++------
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/lib/libpmc/libpmc.c b/lib/libpmc/libpmc.c
index dae3b4fb2d67..10a0aec03cc7 100644
--- a/lib/libpmc/libpmc.c
+++ b/lib/libpmc/libpmc.c
@@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/pmc.h>
 #include <sys/syscall.h>
 
+#include <assert.h>
 #include <ctype.h>
 #include <errno.h>
 #include <err.h>
@@ -1014,8 +1015,14 @@ pmc_allocate(const char *ctrspec, enum pmc_mode mode,
 	r = spec_copy = strdup(ctrspec);
 	ctrname = strsep(&r, ",");
 	if (pmc_pmu_enabled()) {
-		if (pmc_pmu_pmcallocate(ctrname, &pmc_config) == 0)
+		if (pmc_pmu_pmcallocate(ctrname, &pmc_config) == 0) {
+			/*
+			 * XXX: pmclog_get_event exploits this to disambiguate
+			 *      PMU from PMC event codes in PMCALLOCATE events.
+			 */
+			assert(pmc_config.pm_ev < PMC_EVENT_FIRST);
 			goto found;
+		}
 
 		/* Otherwise, reset any changes */
 		pmc_config.pm_ev = 0;
diff --git a/lib/libpmc/pmclog.c b/lib/libpmc/pmclog.c
index babcdc3c8d0d..b219ed53fe3f 100644
--- a/lib/libpmc/pmclog.c
+++ b/lib/libpmc/pmclog.c
@@ -356,12 +356,27 @@ pmclog_get_event(void *cookie, char **data, ssize_t *len,
 		PMCLOG_READ32(le,ev->pl_u.pl_a.pl_flags);
 		PMCLOG_READ32(le,noop);
 		PMCLOG_READ64(le,ev->pl_u.pl_a.pl_rate);
-		ev->pl_u.pl_a.pl_evname = pmc_pmu_event_get_by_idx(ps->ps_cpuid, ev->pl_u.pl_a.pl_event);
-		if (ev->pl_u.pl_a.pl_evname != NULL)
-			break;
-		else if ((ev->pl_u.pl_a.pl_evname =
-		    _pmc_name_of_event(ev->pl_u.pl_a.pl_event, ps->ps_arch))
-		    == NULL) {
+
+		/*
+		 * Could be either a PMC event code or a PMU event index;
+		 * assume that their encodings don't overlap (i.e. no PMU event
+		 * table is more than 0x1000 entries) to distinguish them here.
+		 * Otherwise pmc_pmu_event_get_by_idx will go out of bounds if
+		 * given a PMC event code when it knows about that CPU.
+		 *
+		 * XXX: Ideally we'd have user flags to give us that context.
+		 */
+		if (ev->pl_u.pl_a.pl_event < PMC_EVENT_FIRST)
+			ev->pl_u.pl_a.pl_evname =
+			    pmc_pmu_event_get_by_idx(ps->ps_cpuid,
+				ev->pl_u.pl_a.pl_event);
+		else if (ev->pl_u.pl_a.pl_event <= PMC_EVENT_LAST)
+			ev->pl_u.pl_a.pl_evname =
+			    _pmc_name_of_event(ev->pl_u.pl_a.pl_event,
+				ps->ps_arch);
+		else
+			ev->pl_u.pl_a.pl_evname = NULL;
+		if (ev->pl_u.pl_a.pl_evname == NULL) {
 			printf("unknown event\n");
 			goto error;
 		}