From nobody Mon Sep 25 17:20:41 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RvV552XJVz4tpfp; Mon, 25 Sep 2023 17:20:41 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RvV551s5Fz4pw7; Mon, 25 Sep 2023 17:20:41 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1695662441; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=rKJomyz6N0n17fVxpdwgCTUGb1JPs+1ZjIooPrNeYl4=; b=Xg6fPnRrTL09ZUPtRZtET92C8tiashP7ktY+HVQjUc05ZcEtwhBmZaBeKvz3iWj3JOx8QS XPCxlUKcMjNIrntiP8YiF0//NFH/dM517sGReGh7AeaCQ7M2ZZoY+XtBJ7bDFt9bcaLpGH JRGFjCAnrxpbCodJQfIqVUq0deWp6KbiImSM+zj05CsjW4Bay2bPAktomMEo6hdA77Yprr 9rLpPtPWhT8zkXAikF23mes9NBvgKjV34nIo8DaElZycuuTF8B9fJhjtWIkQ6WJEj21RzM 6q6Ew9zt4yK/1dgoNKtGmSpLI1dOnUGZXcPqluTCrKFvMZK7GKcliK0xGJvPww== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1695662441; a=rsa-sha256; cv=none; b=Yt9hzsztcYfGJePeA7hRHG8NYJ6B6Zb+00lkSdGw2EjBxQxWuFt3Wm6GN34BLJH47bUeCO 1CoKbA5ScAOiVUUduWC+Jxfy86h9iG+ZJyLTYtOfDewn1fkMJK0L+WkKzOys2H2fo9hTbi Bvw9AUYc02kKVK0NK8nuhe5DvZk0J+B8sy+uMcSygrWl2Y5s3PakbMbtkXLZMGl6urLAAT eU+4Et9mn4wVFvfD2tj6AP354iw8QkJnTlLSHhUfx91/ByasXsH66X19Vnc/AawoSh7ZQ8 vNWLBWRNI+0CTIpcuf5vyXS7TVPqxQyNw3TBpcEcbHXnSZK8wavAdG5QEcsKZg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1695662441; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=rKJomyz6N0n17fVxpdwgCTUGb1JPs+1ZjIooPrNeYl4=; b=PhrD4/yA/fDAxOfw/7dlbDtZywOeNnVVO1QmZhjGQkdLwaBC4ErdUa10ht8Zuju0P/ElBx WW2L5a8rk99uO9bgm3C1RsihG0ZQc6lLWvwRKsbqxnCqRmj7RGApaEUVKbqaWa8tHBbnhT pff6GcdA2mXpoHS9jAtStIB3TDtO5sgCPakkbgxcUg08ACcnRabwaA+oahHcMIrWS4hVP1 h+GvvQRn4YVsrMDQNSB9rk4dBLRbu4pnCtUy84CAFYdaPaq7jLlfZ09MxAXR/iO2U+wuME kwGRBnOIHE+OAw9VMtGxvBoKIkeeMB0fYRgkCF26wK1aL6ViFHYCMTuO6XksHw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4RvV550wztz7F3; Mon, 25 Sep 2023 17:20:41 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 38PHKfEW087303; Mon, 25 Sep 2023 17:20:41 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 38PHKfGY087300; Mon, 25 Sep 2023 17:20:41 GMT (envelope-from git) Date: Mon, 25 Sep 2023 17:20:41 GMT Message-Id: <202309251720.38PHKfGY087300@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mitchell Horne Subject: git: 5f74163bdefc - releng/14.0 - pmc: better distinguish pmu-events allocation path List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mhorne X-Git-Repository: src X-Git-Refname: refs/heads/releng/14.0 X-Git-Reftype: branch X-Git-Commit: 5f74163bdefc96c2723a66cf6283319848b861aa Auto-Submitted: auto-generated The branch releng/14.0 has been updated by mhorne: URL: https://cgit.FreeBSD.org/src/commit/?id=5f74163bdefc96c2723a66cf6283319848b861aa commit 5f74163bdefc96c2723a66cf6283319848b861aa Author: Mitchell Horne AuthorDate: 2023-06-06 17:26:46 +0000 Commit: Mitchell Horne CommitDate: 2023-09-25 17:20:08 +0000 pmc: better distinguish pmu-events allocation path Background: The pm_ev field of struct pmc_op_pmcallocate and struct pmc traditionally contains the index of the chosen event, corresponding to the __PMC_EVENTS array in pmc_events.h. This is a static list of events, maintained by FreeBSD. In the usual case, libpmc translates the user supplied event name (string) into the pm_ev index, which is passed as an argument to the allocation syscall. On the kernel side, the allocation method for the relevant hwpmc class translates the given index into the event code that will be written to an event selection register. In 2018, a new source of performance event definitions was introduced: the pmu-events json files, which are maintained by the Linux kernel. The result was better coverage for newer Intel processors with a reduced maintenance burden for libpmc/hwpmc. Intel and AMD CPUs were unconditionally switched to allocate events from pmu-events instead of the traditional scheme (959826ca1bb0a, 81eb4dcf9e0d). Under the pmu-events scheme, the pm_ev field contains an index corresponding to the selected event from the pmu-events table, something which the kernel has no knowledge of. The configuration for the performance counting registers is instead passed via class-dependent fields (struct pmc_md_op_pmcallocate). In 2021 I changed the allocation logic so that it would attempt to pull from the pmu-events table first, and fall-back to the traditional method (dfb4fb41166bc3). Later, pmu-events support for arm64 and power8 CPUs was added (28dd6730a5d6 and b48a2770d48b). The problem that remains is that the pm_ev field is overloaded, without a definitive way to determine whether the event allocation came from the pmu-events table or FreeBSD's statically-defined PMC events. This resulted in a recent fix, 21f7397a61f7. Change: To disambiguate these two supported but separate use-cases, add a new flag, PMC_F_EV_PMU, to be set as part of the allocation, indicating that the event index came from pmu-events. This is useful in two ways: 1. On the kernel side, we can validate the syscall arguments better. Some classes support only the traditional event scheme (e.g. hwpmc_armv7), while others support only the pmu-events method (e.g. hwpmc_core for Intel). We can now check for this. The hwpmc_arm64 class supports both methods, so the new flag supersedes the existing MD flag, PM_MD_EVENT_RAW. 2. The flag will be tracked in struct pmc for the duration of its lifetime, meaning it is communicated back to userspace. This allows libpmc to perform the reverse index-to-event-name translation without speculating about the meaning of the index value. Adding the flag is a backwards-incompatible ABI change. We recently bumped the major version of the hwpmc module, so this breakage is acceptable. Reviewed by: jkoshy Approved by: re (gjb) Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D40753 (cherry picked from commit c190fb35f35cc163b61e582a49115680b0d49dcc) (cherry picked from commit 5adb91c72985b840b2c7fbf3c3308619acd2b61a) --- lib/libpmc/libpmc.c | 9 +-------- lib/libpmc/libpmc_pmu_util.c | 2 +- lib/libpmc/pmclog.c | 11 +++-------- sys/arm64/include/pmc_mdep.h | 2 -- sys/dev/hwpmc/hwpmc_amd.c | 3 +++ sys/dev/hwpmc/hwpmc_arm64.c | 6 +++--- sys/dev/hwpmc/hwpmc_core.c | 6 ++++++ sys/dev/hwpmc/hwpmc_mod.c | 3 ++- sys/dev/hwpmc/hwpmc_power8.c | 3 +++ sys/dev/hwpmc/hwpmc_uncore.c | 6 ++++++ sys/sys/pmc.h | 8 ++++++++ 11 files changed, 36 insertions(+), 23 deletions(-) diff --git a/lib/libpmc/libpmc.c b/lib/libpmc/libpmc.c index a7ed1c3d9ac8..f10a64e28cac 100644 --- a/lib/libpmc/libpmc.c +++ b/lib/libpmc/libpmc.c @@ -33,7 +33,6 @@ #include #include -#include #include #include #include @@ -1083,14 +1082,8 @@ 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) { - /* - * XXX: pmclog_get_event exploits this to disambiguate - * PMU from PMC event codes in PMCALLOCATE events. - */ - assert(pmc_config.pm_ev < PMC_EVENT_FIRST); + if (pmc_pmu_pmcallocate(ctrname, &pmc_config) == 0) goto found; - } } free(spec_copy); spec_copy = NULL; diff --git a/lib/libpmc/libpmc_pmu_util.c b/lib/libpmc/libpmc_pmu_util.c index 772dec7a9d53..fa2e76e8d026 100644 --- a/lib/libpmc/libpmc_pmu_util.c +++ b/lib/libpmc/libpmc_pmu_util.c @@ -649,7 +649,6 @@ pmc_pmu_pmcallocate_md(const char *event_name, struct pmc_op_pmcallocate *pm) assert(idx >= 0); pm->pm_ev = idx; pm->pm_md.pm_md_config = ped.ped_event; - pm->pm_md.pm_md_flags |= PM_MD_RAW_EVENT; pm->pm_class = PMC_CLASS_ARMV8; pm->pm_caps |= (PMC_CAP_READ | PMC_CAP_WRITE); @@ -680,5 +679,6 @@ pmc_pmu_pmcallocate(const char *event_name, struct pmc_op_pmcallocate *pm) return (error); } + pm->pm_flags |= PMC_F_EV_PMU; return (0); } diff --git a/lib/libpmc/pmclog.c b/lib/libpmc/pmclog.c index a21fe42c9947..3b1572baaa2c 100644 --- a/lib/libpmc/pmclog.c +++ b/lib/libpmc/pmclog.c @@ -357,15 +357,10 @@ pmclog_get_event(void *cookie, char **data, ssize_t *len, PMCLOG_READ64(le,ev->pl_u.pl_a.pl_rate); /* - * 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. + * pl_event could contain either a PMC event code or a PMU + * event index. */ - if (ev->pl_u.pl_a.pl_event < PMC_EVENT_FIRST) + if ((ev->pl_u.pl_a.pl_flags & PMC_F_EV_PMU) != 0) ev->pl_u.pl_a.pl_evname = pmc_pmu_event_get_by_idx(ps->ps_cpuid, ev->pl_u.pl_a.pl_event); diff --git a/sys/arm64/include/pmc_mdep.h b/sys/arm64/include/pmc_mdep.h index 5c43a7924b01..97d0f30c9c09 100644 --- a/sys/arm64/include/pmc_mdep.h +++ b/sys/arm64/include/pmc_mdep.h @@ -45,8 +45,6 @@ union pmc_md_op_pmcallocate { struct { uint32_t pm_md_config; - uint32_t pm_md_flags; -#define PM_MD_RAW_EVENT 0x1 }; struct pmc_md_cmn600_pmu_op_pmcallocate pm_cmn600; struct pmc_md_dmc620_pmu_op_pmcallocate pm_dmc620; diff --git a/sys/dev/hwpmc/hwpmc_amd.c b/sys/dev/hwpmc/hwpmc_amd.c index b15d223bc7a5..fbbaf92a1547 100644 --- a/sys/dev/hwpmc/hwpmc_amd.c +++ b/sys/dev/hwpmc/hwpmc_amd.c @@ -582,6 +582,9 @@ amd_allocate_pmc(int cpu, int ri, struct pmc *pm, if (pd->pd_class != a->pm_class) return EINVAL; + if ((a->pm_flags & PMC_F_EV_PMU) == 0) + return (EINVAL); + caps = pm->pm_caps; PMCDBG2(MDP,ALL,1,"amd-allocate ri=%d caps=0x%x", ri, caps); diff --git a/sys/dev/hwpmc/hwpmc_arm64.c b/sys/dev/hwpmc/hwpmc_arm64.c index 995b7158ac20..9a5debb8016b 100644 --- a/sys/dev/hwpmc/hwpmc_arm64.c +++ b/sys/dev/hwpmc/hwpmc_arm64.c @@ -177,9 +177,9 @@ arm64_allocate_pmc(int cpu, int ri, struct pmc *pm, } pe = a->pm_ev; - /* Adjust the config value if needed. */ - config = a->pm_md.pm_md_config; - if ((a->pm_md.pm_md_flags & PM_MD_RAW_EVENT) == 0) { + if ((a->pm_flags & PMC_F_EV_PMU) != 0) { + config = a->pm_md.pm_md_config; + } else { config = (uint32_t)pe - PMC_EV_ARMV8_FIRST; if (config > (PMC_EV_ARMV8_LAST - PMC_EV_ARMV8_FIRST)) return (EINVAL); diff --git a/sys/dev/hwpmc/hwpmc_core.c b/sys/dev/hwpmc/hwpmc_core.c index 15b875e3af94..3829a03eb729 100644 --- a/sys/dev/hwpmc/hwpmc_core.c +++ b/sys/dev/hwpmc/hwpmc_core.c @@ -239,6 +239,9 @@ iaf_allocate_pmc(int cpu, int ri, struct pmc *pm, if (a->pm_class != PMC_CLASS_IAF) return (EINVAL); + if ((a->pm_flags & PMC_F_EV_PMU) == 0) + return (EINVAL); + iap = &a->pm_md.pm_iap; config = iap->pm_iap_config; ev = IAP_EVSEL_GET(config); @@ -721,6 +724,9 @@ iap_allocate_pmc(int cpu, int ri, struct pmc *pm, if (a->pm_class != PMC_CLASS_IAP) return (EINVAL); + if ((a->pm_flags & PMC_F_EV_PMU) == 0) + return (EINVAL); + iap = &a->pm_md.pm_iap; ev = IAP_EVSEL_GET(iap->pm_iap_config); diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c index de012b74d558..5dd8bc67d60a 100644 --- a/sys/dev/hwpmc/hwpmc_mod.c +++ b/sys/dev/hwpmc/hwpmc_mod.c @@ -3349,7 +3349,8 @@ pmc_do_op_pmcallocate(struct thread *td, struct pmc_op_pmcallocate *pa) * Look for valid values for 'pm_flags'. */ if ((flags & ~(PMC_F_DESCENDANTS | PMC_F_LOG_PROCCSW | - PMC_F_LOG_PROCEXIT | PMC_F_CALLCHAIN | PMC_F_USERCALLCHAIN)) != 0) + PMC_F_LOG_PROCEXIT | PMC_F_CALLCHAIN | PMC_F_USERCALLCHAIN | + PMC_F_EV_PMU)) != 0) return (EINVAL); /* PMC_F_USERCALLCHAIN is only valid with PMC_F_CALLCHAIN. */ diff --git a/sys/dev/hwpmc/hwpmc_power8.c b/sys/dev/hwpmc/hwpmc_power8.c index fc6b878eff7d..d7ccbc5c6c0a 100644 --- a/sys/dev/hwpmc/hwpmc_power8.c +++ b/sys/dev/hwpmc/hwpmc_power8.c @@ -168,6 +168,9 @@ power8_allocate_pmc(int cpu, int ri, struct pmc *pm, if (a->pm_class != PMC_CLASS_POWER8) return (EINVAL); + if ((a->pm_flags & PMC_F_EV_PMU) == 0) + return (EINVAL); + /* * PMC5 and PMC6 are not programmable and always count instructions * completed and cycles, respectively. diff --git a/sys/dev/hwpmc/hwpmc_uncore.c b/sys/dev/hwpmc/hwpmc_uncore.c index c16800c14ce1..fd4266b605ef 100644 --- a/sys/dev/hwpmc/hwpmc_uncore.c +++ b/sys/dev/hwpmc/hwpmc_uncore.c @@ -199,6 +199,9 @@ ucf_allocate_pmc(int cpu, int ri, struct pmc *pm, if (a->pm_class != PMC_CLASS_UCF) return (EINVAL); + if ((a->pm_flags & PMC_F_EV_PMU) == 0) + return (EINVAL); + flags = UCF_EN; pm->pm_md.pm_ucf.pm_ucf_ctrl = (flags << (ri * 4)); @@ -498,6 +501,9 @@ ucp_allocate_pmc(int cpu, int ri, struct pmc *pm, if (a->pm_class != PMC_CLASS_UCP) return (EINVAL); + if ((a->pm_flags & PMC_F_EV_PMU) == 0) + return (EINVAL); + ucp = &a->pm_md.pm_ucp; ev = UCP_EVSEL(ucp->pm_ucp_config); switch (uncore_cputype) { diff --git a/sys/sys/pmc.h b/sys/sys/pmc.h index a4d03efb6aac..714d8a7c65b7 100644 --- a/sys/sys/pmc.h +++ b/sys/sys/pmc.h @@ -369,6 +369,14 @@ enum pmc_ops { #define PMC_F_CALLCHAIN 0x00000080 /*OP ALLOCATE capture callchains */ #define PMC_F_USERCALLCHAIN 0x00000100 /*OP ALLOCATE use userspace stack */ +/* V10 API */ +#define PMC_F_EV_PMU 0x00000200 /* + * OP ALLOCATE: pm_ev has special + * userspace meaning; counter + * configuration is communicated + * through class-dependent fields + */ + /* internal flags */ #define PMC_F_ATTACHED_TO_OWNER 0x00010000 /*attached to owner*/ #define PMC_F_NEEDS_LOGFILE 0x00020000 /*needs log file */