Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 30 Aug 2021 19:15:54 GMT
From:      Mitchell Horne <mhorne@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 315cd55dba61 - main - hwpmc: consistently validate PMC class in allocation method
Message-ID:  <202108301915.17UJFsce056494@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by mhorne:

URL: https://cgit.FreeBSD.org/src/commit/?id=315cd55dba61416495a847f0eed6e522421e2347

commit 315cd55dba61416495a847f0eed6e522421e2347
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2021-08-30 17:02:23 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2021-08-30 19:12:59 +0000

    hwpmc: consistently validate PMC class in allocation method
    
    It is always a good idea. In one case, attempting to allocate N+1 PMCs
    from a class with N hardware counters would incorrectly attempt to
    allocate from the next class in the list. Without this validation, this
    can lead to all kinds of strange behaviour.
    
    Since powerpc_allocate_pmc() is used by both the mpc7xxx and ppc970
    classes, add a new global to track which is active (it will never be
    both).
    
    Reviewed by:    luporl, ray
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31387
---
 sys/dev/hwpmc/hwpmc_core.c    | 3 +++
 sys/dev/hwpmc/hwpmc_e500.c    | 3 +++
 sys/dev/hwpmc/hwpmc_mpc7xxx.c | 1 +
 sys/dev/hwpmc/hwpmc_power8.c  | 3 +++
 sys/dev/hwpmc/hwpmc_powerpc.c | 4 ++++
 sys/dev/hwpmc/hwpmc_powerpc.h | 1 +
 sys/dev/hwpmc/hwpmc_ppc970.c  | 1 +
 sys/dev/hwpmc/hwpmc_uncore.c  | 3 +++
 8 files changed, 19 insertions(+)

diff --git a/sys/dev/hwpmc/hwpmc_core.c b/sys/dev/hwpmc/hwpmc_core.c
index afd587296e01..507b20488132 100644
--- a/sys/dev/hwpmc/hwpmc_core.c
+++ b/sys/dev/hwpmc/hwpmc_core.c
@@ -750,6 +750,9 @@ iap_allocate_pmc(int cpu, int ri, struct pmc *pm,
 	KASSERT(ri >= 0 && ri < core_iap_npmc,
 	    ("[core,%d] illegal row-index value %d", __LINE__, ri));
 
+	if (a->pm_class != PMC_CLASS_IAP)
+		return (EINVAL);
+
 	/* check requested capabilities */
 	caps = a->pm_caps;
 	if ((IAP_PMC_CAPS & caps) != caps)
diff --git a/sys/dev/hwpmc/hwpmc_e500.c b/sys/dev/hwpmc/hwpmc_e500.c
index 72c0868f08b5..b82bada95fe3 100644
--- a/sys/dev/hwpmc/hwpmc_e500.c
+++ b/sys/dev/hwpmc/hwpmc_e500.c
@@ -376,6 +376,9 @@ e500_allocate_pmc(int cpu, int ri, struct pmc *pm,
 	KASSERT(ri >= 0 && ri < E500_MAX_PMCS,
 	    ("[powerpc,%d] illegal row index %d", __LINE__, ri));
 
+	if (a->pm_class != PMC_CLASS_E500)
+		return (EINVAL);
+
 	caps = a->pm_caps;
 
 	pe = a->pm_ev;
diff --git a/sys/dev/hwpmc/hwpmc_mpc7xxx.c b/sys/dev/hwpmc/hwpmc_mpc7xxx.c
index 987480e89efd..c62c0791af01 100644
--- a/sys/dev/hwpmc/hwpmc_mpc7xxx.c
+++ b/sys/dev/hwpmc/hwpmc_mpc7xxx.c
@@ -471,6 +471,7 @@ pmc_mpc7xxx_initialize(struct pmc_mdep *pmc_mdep)
 	ppc_event_first = PMC_EV_PPC7450_FIRST;
 	ppc_event_last = PMC_EV_PPC7450_LAST;
 	ppc_max_pmcs = MPC7XXX_MAX_PMCS;
+	ppc_class = pcd->pcd_class;
 
 	powerpc_set_pmc = mpc7xxx_set_pmc;
 	powerpc_pmcn_read = mpc7xxx_pmcn_read;
diff --git a/sys/dev/hwpmc/hwpmc_power8.c b/sys/dev/hwpmc/hwpmc_power8.c
index 0d93e4ae2964..287aa45a966d 100644
--- a/sys/dev/hwpmc/hwpmc_power8.c
+++ b/sys/dev/hwpmc/hwpmc_power8.c
@@ -167,6 +167,9 @@ power8_allocate_pmc(int cpu, int ri, struct pmc *pm,
 	counter = PM_EVENT_COUNTER(pe);
 	config = PM_EVENT_CODE(pe);
 
+	if (a->pm_class != PMC_CLASS_POWER8)
+		return (EINVAL);
+
 	/*
 	 * PMC5 and PMC6 are not programmable and always count instructions
 	 * completed and cycles, respectively.
diff --git a/sys/dev/hwpmc/hwpmc_powerpc.c b/sys/dev/hwpmc/hwpmc_powerpc.c
index 0f14f93e6415..4377c3075bca 100644
--- a/sys/dev/hwpmc/hwpmc_powerpc.c
+++ b/sys/dev/hwpmc/hwpmc_powerpc.c
@@ -59,6 +59,7 @@ size_t ppc_event_codes_size;
 int ppc_event_first;
 int ppc_event_last;
 int ppc_max_pmcs;
+enum pmc_class ppc_class;
 
 void (*powerpc_set_pmc)(int cpu, int ri, int config);
 pmc_value_t (*powerpc_pmcn_read)(unsigned int pmc);
@@ -211,6 +212,9 @@ powerpc_allocate_pmc(int cpu, int ri, struct pmc *pm,
 	KASSERT(ri >= 0 && ri < ppc_max_pmcs,
 	    ("[powerpc,%d] illegal row index %d", __LINE__, ri));
 
+	if (a->pm_class != ppc_class)
+		return (EINVAL);
+
 	caps = a->pm_caps;
 
 	pe = a->pm_ev;
diff --git a/sys/dev/hwpmc/hwpmc_powerpc.h b/sys/dev/hwpmc/hwpmc_powerpc.h
index 31c20592a21f..10aa0ad75b14 100644
--- a/sys/dev/hwpmc/hwpmc_powerpc.h
+++ b/sys/dev/hwpmc/hwpmc_powerpc.h
@@ -82,6 +82,7 @@ extern size_t ppc_event_codes_size;
 extern int ppc_event_first;
 extern int ppc_event_last;
 extern int ppc_max_pmcs;
+extern enum pmc_class ppc_class;
 
 extern void (*powerpc_set_pmc)(int cpu, int ri, int config);
 extern pmc_value_t (*powerpc_pmcn_read)(unsigned int pmc);
diff --git a/sys/dev/hwpmc/hwpmc_ppc970.c b/sys/dev/hwpmc/hwpmc_ppc970.c
index 39dc4efb290a..3e92c4dc6aa2 100644
--- a/sys/dev/hwpmc/hwpmc_ppc970.c
+++ b/sys/dev/hwpmc/hwpmc_ppc970.c
@@ -380,6 +380,7 @@ pmc_ppc970_initialize(struct pmc_mdep *pmc_mdep)
 	ppc_event_first = PMC_EV_PPC970_FIRST;
 	ppc_event_last = PMC_EV_PPC970_LAST;
 	ppc_max_pmcs = PPC970_MAX_PMCS;
+	ppc_class = pcd->pcd_class;
 
 	powerpc_set_pmc = ppc970_set_pmc;
 	powerpc_pmcn_read = powerpc_pmcn_read_default;
diff --git a/sys/dev/hwpmc/hwpmc_uncore.c b/sys/dev/hwpmc/hwpmc_uncore.c
index 2c638833dcd9..19017cabddd9 100644
--- a/sys/dev/hwpmc/hwpmc_uncore.c
+++ b/sys/dev/hwpmc/hwpmc_uncore.c
@@ -530,6 +530,9 @@ ucp_allocate_pmc(int cpu, int ri, struct pmc *pm,
 	KASSERT(ri >= 0 && ri < uncore_ucp_npmc,
 	    ("[uncore,%d] illegal row-index value %d", __LINE__, ri));
 
+	if (a->pm_class != PMC_CLASS_UCP)
+		return (EINVAL);
+
 	/* check requested capabilities */
 	caps = a->pm_caps;
 	if ((UCP_PMC_CAPS & caps) != caps)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202108301915.17UJFsce056494>