perf/x86/intel/uncore: Bug fixes and cleanups
Subsystems (3)
v3 cover perf/x86/intel/uncore: Bug fixes and cleanups
This series includes bug fixes and cleanups for the Intel uncore PMU driver.
- Patch 1 fixes a theoretical bug in discovery unit lookup on multi-die systems.
- Patch 2 guards against an invalid box control address.
- Patch 3 fixes a PCI device refcount leak in UPI topology discovery.
- Patch 4 works around a hardware issue on Raptor Cove CPUs.
- Patches 5-7 implement a global MSR init callback for GNR/GRR/SRF/CWF uncore.
Changes in v3:
- Protect uncore_discovery_pci() with cpus_read_lock(). (Sashiko)
- Add Fixes tag for patch 4/7 and 7/7.
- Add the missing '---' in patch 5.
Changes in v2:
- Add patch 2 to guard against invalid box control address (Sashiko)
- Remove WARN_ON_ONCE() from patch 1
- Move cpus_read_{lock,unlock}() out of uncore_die_to_cpu() (Sashiko)
Zide Chen (7):
perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems perf/x86/intel/uncore: Guard against invalid box control address perf/x86/intel/uncore: Fix PCI device refcount leak in UPI discovery perf/x86/intel/uncore: Defer ADL global PMON enable to enable_box() perf/x86/intel/uncore: Move die_to_cpu() to uncore.c perf/x86/intel/uncore: Fix uncore_die_to_cpu() for offline dies perf/x86/intel/uncore: Implement global init callback for GNR uncore
arch/x86/events/intel/uncore.c | 32 ++++++++++++++++- arch/x86/events/intel/uncore.h | 3 +- arch/x86/events/intel/uncore_discovery.c | 46 ++++++++++++++++-------- arch/x86/events/intel/uncore_snb.c | 7 ---- arch/x86/events/intel/uncore_snbep.c | 36 +++++++------------ 5 files changed, 76 insertions(+), 48 deletions(-)
Signature · 1 line
2.54.0
PATCH v3 1/7 perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems
In uncore_find_add_unit(), PMON units with the same unit ID may be added to the uncore discovery RB-tree for different dies. These units are distinguished by node->die.
However, intel_generic_uncore_box_ctl() uses a fixed die ID of -1 when looking up the discovery unit, which may retrieve the wrong node on multi-die systems.
Use box->dieid instead so the correct discovery unit is selected.
No functional issue has been observed so far because currently supported platforms happen to use the same unit control register for such units.
Remove WARN_ON_ONCE() because with the above change a NULL unit can be expected, e.g. when a CPU die is offline during uncore enumeration and the unit is not added to the RB-tree. In this case, intel_uncore_find_discovery_unit() returns NULL once the die becomes online, and it is expected that the PMU box is not functional for that die.
---
v2:
- Remove WARN_ON_ONCE().
--- arch/x86/events/intel/uncore_discovery.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/events/intel/uncore_discovery.c +2 -2
@@ -481,8 +481,8 @@ static u64 intel_generic_uncore_box_ctl(struct intel_uncore_box *box)struct intel_uncore_discovery_unit *unit; unit =intel_uncore_find_discovery_unit (box->pmu->type->boxes, --1 , box->pmu->pmu_idx); -if (WARN_ON_ONCE (!unit)) + box->dieid, box->pmu->pmu_idx); +if (!unit)return 0 ;return unit->addr;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH V3 1/7] perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The commit message inaccurately claims that a NULL unit can be expected when a CPU die is…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message inaccurately claims that a NULL unit can be expected when a CPU die is offline, justifying the removal of a valid WARN_ON_ONCE assertion, but the code path is structurally unreachable for missing units. --
commit ddb51b3d3e54081e6dbacecec75a18fa773ae364
Author: Zide Chen <[email protected]>
perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems
This commit fixes an issue where PMON units with the same unit ID may retrieve the wrong node on multi-die systems by using box->dieid. It also removes a WARN_ON_ONCE() under the assumption that a NULL unit can be expected during CPU die offline/online scenarios.
Quoted 12 lines
Remove
WARN_ON_ONCE()because with the above change a NULL unit can be expected, e.g. when a CPU die is offline during uncore enumeration and the unit is not added to the RB-tree. In this case,intel_uncore_find_discovery_unit()returns NULL once the die becomes online, and it is expected that the PMU box is not functional for that die.arch/x86/events/intel/uncore_discovery.c +0
-0
arch/x86/events/intel/uncore_discovery.c static u64 intel_generic_uncore_box_ctl(struct intel_uncore_box *box) · line 486
Quoted 8 lines
arch/x86/events/intel/uncore_discovery.c +2
-2@@ -481,8 +481,8 @@ static u64 intel_generic_uncore_box_ctl(struct intel_uncore_box *box)struct intel_uncore_discovery_unit *unit; unit =intel_uncore_find_discovery_unit (box->pmu->type->boxes, --1 , box->pmu->pmu_idx); -if (WARN_ON_ONCE (!unit)) + box->dieid, box->pmu->pmu_idx); +if (!unit)return 0 ;
[Severity: Medium]
Is the scenario described in the commit message actually possible in the current codebase?
When a CPU die comes online, uncore_event_cpu_online() invokes uncore_change_type_ctx(), which relies on uncore_die_has_box() to check for the unit. If the unit is missing from the RB-tree, uncore_die_has_box() returns false, and the box cpu assignment is skipped:
arch/x86/events/intel/uncore.c:uncore_change_type_ctx() {
...
if (uncore_die_has_box(type, die, pmu->pmu_idx)) {
box->cpu = new_cpu;
cpumask_set_cpu(new_cpu, &pmu->cpu_mask);
}
...
}Because box->cpu remains -1, uncore_box_ref() bypasses initialization:
arch/x86/events/intel/uncore.c:uncore_box_ref() {
...
if (box && box->cpu >= 0 && atomic_inc_return(&box->refcnt) == 1)
uncore_box_init(box);
...
}Furthermore, uncore_pmu_event_init() rejects event creations if box->cpu is negative. Because of this, it appears the PMU operations (init, enable, disable) are never executed for this box, making the call to intel_generic_uncore_box_ctl() unreachable.
Does this mean the removed WARN_ON_ONCE(!unit) was actually a valid assertion, and the !unit check is effectively dead code?
Quoted 2 lines
arch/x86/events/intel/uncore_discovery.c +0
-0return unit->addr;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
[tip: perf/core] perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems
In reply to Zide ChenThe following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 63f48abd55d0417996bca86022925c853a2b436b Gitweb: https://git.kernel.org/tip/63f48abd55d0417996bca86022925c853a2b436b Author: Zide Chen <[email protected]> AuthorDate: Tue, 02 Jun 2026 07:49:02 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Thu, 04 Jun 2026 11:38:36 +02:00
perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems
In uncore_find_add_unit(), PMON units with the same unit ID may be added to the uncore discovery RB-tree for different dies. These units are distinguished by node->die.
However, intel_generic_uncore_box_ctl() uses a fixed die ID of -1 when looking up the discovery unit, which may retrieve the wrong node on multi-die systems.
Use box->dieid instead so the correct discovery unit is selected.
No functional issue has been observed so far because currently supported platforms happen to use the same unit control register for such units.
Remove WARN_ON_ONCE() because with the above change a NULL unit can be expected, e.g. when a CPU die is offline during uncore enumeration and the unit is not added to the RB-tree. In this case, intel_uncore_find_discovery_unit() returns NULL once the die becomes online, and it is expected that the PMU box is not functional for that die.
--- arch/x86/events/intel/uncore_discovery.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/events/intel/uncore_discovery.c +2 -2
@@ -481,8 +481,8 @@ static u64 intel_generic_uncore_box_ctl(struct intel_uncore_box *box)struct intel_uncore_discovery_unit *unit; unit =intel_uncore_find_discovery_unit (box->pmu->type->boxes, --1 , box->pmu->pmu_idx); -if (WARN_ON_ONCE (!unit)) + box->dieid, box->pmu->pmu_idx); +if (!unit)return 0 ;return unit->addr;
PATCH v3 2/7 perf/x86/intel/uncore: Guard against invalid box control address
In reply to Zide ChenTheoretically, intel_uncore_find_discovery_unit() could return NULL, e.g., when a CPU die is offline during uncore enumeration and its PMU units are not added to the discovery RB-tree.
Guard against a NULL return value and the resulting invalid box control address (0) before accessing hardware.
---
V2:
- New patch.
- Address pre-existing invalid box control address issue (Sashiko).
--- arch/x86/events/intel/uncore_discovery.c | 33 +++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-)
arch/x86/events/intel/uncore_discovery.c +24 -9
@@ -490,17 +490,28 @@ static u64 intel_generic_uncore_box_ctl(struct intel_uncore_box *box)void intel_generic_uncore_msr_init_box (struct intel_uncore_box *box) { -wrmsrq (intel_generic_uncore_box_ctl (box),GENERIC_PMON_BOX_CTL_INT ); +u64 box_ctl =intel_generic_uncore_box_ctl (box); + +if (!box_ctl) +return ; + +wrmsrq (box_ctl,GENERIC_PMON_BOX_CTL_INT ); }void intel_generic_uncore_msr_disable_box (struct intel_uncore_box *box) { -wrmsrq (intel_generic_uncore_box_ctl (box),GENERIC_PMON_BOX_CTL_FRZ ); +u64 box_ctl =intel_generic_uncore_box_ctl (box); + +if (box_ctl) +wrmsrq (box_ctl,GENERIC_PMON_BOX_CTL_FRZ ); }void intel_generic_uncore_msr_enable_box (struct intel_uncore_box *box) { -wrmsrq (intel_generic_uncore_box_ctl (box),0 ); +u64 box_ctl =intel_generic_uncore_box_ctl (box); + +if (box_ctl) +wrmsrq (box_ctl,0 ); } static void intel_generic_uncore_msr_enable_event(struct intel_uncore_box *box,
@@ -549,6 +560,7 @@ bool intel_generic_uncore_assign_hw_event(struct perf_event *event,if (box->pci_dev) { box_ctl =UNCORE_DISCOVERY_PCI_BOX_CTRL (box_ctl); + hwc->config_base = box_ctl +uncore_pci_event_ctl (box, hwc->idx); hwc->event_base = box_ctl +uncore_pci_perf_ctr (box, hwc->idx);return true;
@@ -567,27 +579,30 @@ static inline int intel_pci_uncore_box_ctl(struct intel_uncore_box *box)void intel_generic_uncore_pci_init_box (struct intel_uncore_box *box) { -struct pci_dev *pdev = box->pci_dev;int box_ctl =intel_pci_uncore_box_ctl (box); +if (!box_ctl) +return ; +__set_bit (UNCORE_BOX_FLAG_CTL_OFFS8 , &box->flags); -pci_write_config_dword (pdev, box_ctl,GENERIC_PMON_BOX_CTL_INT ); +pci_write_config_dword (box->pci_dev, box_ctl,GENERIC_PMON_BOX_CTL_INT ); }void intel_generic_uncore_pci_disable_box (struct intel_uncore_box *box) { -struct pci_dev *pdev = box->pci_dev;int box_ctl =intel_pci_uncore_box_ctl (box); -pci_write_config_dword (pdev, box_ctl,GENERIC_PMON_BOX_CTL_FRZ ); +if (box_ctl) +pci_write_config_dword (box->pci_dev, box_ctl, +GENERIC_PMON_BOX_CTL_FRZ ); }void intel_generic_uncore_pci_enable_box (struct intel_uncore_box *box) { -struct pci_dev *pdev = box->pci_dev;int box_ctl =intel_pci_uncore_box_ctl (box); -pci_write_config_dword (pdev, box_ctl,0 ); +if (box_ctl) +pci_write_config_dword (box->pci_dev, box_ctl,0 ); } static void intel_generic_uncore_pci_enable_event(struct intel_uncore_box *box,
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH V3 2/7] perf/x86/intel/uncore: Guard against invalid box control address
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing memory barriers when publishing and reading pmu->boxes[die], which can cause uncore_pmu_event_init() to use a completely uninitialized PMU box. --
commit a8ebe9f88eb163279373dfcbb348bed622973d26
Author: Zide Chen <[email protected]>
perf/x86/intel/uncore: Guard against invalid box control address
This commit adds null pointer checks against the return value of intel_generic_uncore_box_ctl() to prevent invalid hardware accesses when a CPU die is offline during uncore enumeration.
arch/x86/events/intel/uncore_discovery.c
Quoted 4 lines
arch/x86/events/intel/uncore_discovery.c +0
-0
[ ... ]
[Severity: High]
This is a pre-existing issue, but does the initialization of PMU boxes lack necessary memory barriers?
In arch/x86/events/intel/uncore.c, a new PMU box is allocated, initialized, and published to the array without a write memory barrier:
arch/x86/events/intel/uncore.c:allocate_boxes() {
...
box->pmu->boxes[die] = box;
...
}Because MSR/MMIO PMUs are registered to the perf core before allocate_boxes() is invoked via CPU hotplug callbacks, could userspace concurrently invoke perf_event_open()?
This would trigger uncore_pmu_event_init(), which locklessly reads the box without a read memory barrier:
arch/x86/events/intel/uncore.c:uncore_pmu_to_box() {
...
return dieid < uncore_max_dies() ? pmu->boxes[dieid] : NULL;
}This data race might allow the CPU to observe the box pointer before its contents (such as box->cpu and box->pmu) are fully initialized or zeroed. If the uninitialized memory happens to have a value >= 0 for box->cpu, would it bypass the validation check?
arch/x86/events/intel/uncore.c:uncore_pmu_event_init() {
...
if (!box || box->cpu < 0)
return -EINVAL;
...
}Consequently, a partially initialized box might be associated with the event, and when the event is later added or scheduled, uninitialized fields like box->pmu could be dereferenced.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
[tip: perf/core] perf/x86/intel/uncore: Guard against invalid box control address
In reply to Zide ChenThe following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 3c19ea24f02658c4b8ad364458fae4d77fdb3fae Gitweb: https://git.kernel.org/tip/3c19ea24f02658c4b8ad364458fae4d77fdb3fae Author: Zide Chen <[email protected]> AuthorDate: Tue, 02 Jun 2026 07:49:03 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Thu, 04 Jun 2026 11:38:36 +02:00
perf/x86/intel/uncore: Guard against invalid box control address
Theoretically, intel_uncore_find_discovery_unit() could return NULL, e.g., when a CPU die is offline during uncore enumeration and its PMU units are not added to the discovery RB-tree.
Guard against a NULL return value and the resulting invalid box control address (0) before accessing hardware.
--- arch/x86/events/intel/uncore_discovery.c | 33 ++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-)
arch/x86/events/intel/uncore_discovery.c +24 -9
@@ -490,17 +490,28 @@ static u64 intel_generic_uncore_box_ctl(struct intel_uncore_box *box)void intel_generic_uncore_msr_init_box (struct intel_uncore_box *box) { -wrmsrq (intel_generic_uncore_box_ctl (box),GENERIC_PMON_BOX_CTL_INT ); +u64 box_ctl =intel_generic_uncore_box_ctl (box); + +if (!box_ctl) +return ; + +wrmsrq (box_ctl,GENERIC_PMON_BOX_CTL_INT ); }void intel_generic_uncore_msr_disable_box (struct intel_uncore_box *box) { -wrmsrq (intel_generic_uncore_box_ctl (box),GENERIC_PMON_BOX_CTL_FRZ ); +u64 box_ctl =intel_generic_uncore_box_ctl (box); + +if (box_ctl) +wrmsrq (box_ctl,GENERIC_PMON_BOX_CTL_FRZ ); }void intel_generic_uncore_msr_enable_box (struct intel_uncore_box *box) { -wrmsrq (intel_generic_uncore_box_ctl (box),0 ); +u64 box_ctl =intel_generic_uncore_box_ctl (box); + +if (box_ctl) +wrmsrq (box_ctl,0 ); } static void intel_generic_uncore_msr_enable_event(struct intel_uncore_box *box,
@@ -549,6 +560,7 @@ bool intel_generic_uncore_assign_hw_event(struct perf_event *event,if (box->pci_dev) { box_ctl =UNCORE_DISCOVERY_PCI_BOX_CTRL (box_ctl); + hwc->config_base = box_ctl +uncore_pci_event_ctl (box, hwc->idx); hwc->event_base = box_ctl +uncore_pci_perf_ctr (box, hwc->idx);return true;
@@ -567,27 +579,30 @@ static inline int intel_pci_uncore_box_ctl(struct intel_uncore_box *box)void intel_generic_uncore_pci_init_box (struct intel_uncore_box *box) { -struct pci_dev *pdev = box->pci_dev;int box_ctl =intel_pci_uncore_box_ctl (box); +if (!box_ctl) +return ; +__set_bit (UNCORE_BOX_FLAG_CTL_OFFS8 , &box->flags); -pci_write_config_dword (pdev, box_ctl,GENERIC_PMON_BOX_CTL_INT ); +pci_write_config_dword (box->pci_dev, box_ctl,GENERIC_PMON_BOX_CTL_INT ); }void intel_generic_uncore_pci_disable_box (struct intel_uncore_box *box) { -struct pci_dev *pdev = box->pci_dev;int box_ctl =intel_pci_uncore_box_ctl (box); -pci_write_config_dword (pdev, box_ctl,GENERIC_PMON_BOX_CTL_FRZ ); +if (box_ctl) +pci_write_config_dword (box->pci_dev, box_ctl, +GENERIC_PMON_BOX_CTL_FRZ ); }void intel_generic_uncore_pci_enable_box (struct intel_uncore_box *box) { -struct pci_dev *pdev = box->pci_dev;int box_ctl =intel_pci_uncore_box_ctl (box); -pci_write_config_dword (pdev, box_ctl,0 ); +if (box_ctl) +pci_write_config_dword (box->pci_dev, box_ctl,0 ); } static void intel_generic_uncore_pci_enable_event(struct intel_uncore_box *box,
PATCH v3 3/7 perf/x86/intel/uncore: Fix PCI device refcount leak in UPI discovery
In reply to Zide Chenpci_get_domain_bus_and_slot() increments the reference count of the returned PCI device and therefore requires a matching pci_dev_put().
In skx_upi_topology_cb() and discover_upi_topology(), the lookup is performed inside a loop, but pci_dev_put() is only called once after the loop. As a result, references from all previous iterations are leaked.
Move pci_dev_put(dev) into the if (dev) block immediately after upi_fill_topology() returns.
Opportunistically, fix uninitialized variable in skx_upi_topology_cb().
--- arch/x86/events/intel/uncore_snbep.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/events/intel/uncore_snbep.c +3 -3
@@ -4261,7 +4261,7 @@ static int upi_fill_topology(struct pci_dev *dev, struct intel_uncore_topology *static int skx_upi_topology_cb (struct intel_uncore_type *type,int segment,int die,u64 cpu_bus_msr) { -int idx, ret; +int idx, ret =0 ;struct intel_uncore_topology *upi;unsigned int devfn;struct pci_dev *dev =NULL ;
@@ -4274,12 +4274,12 @@ static int skx_upi_topology_cb(struct intel_uncore_type *type, int segment, dev =pci_get_domain_bus_and_slot (segment, bus, devfn);if (dev) { ret =upi_fill_topology (dev, upi, idx); +pci_dev_put (dev);if (ret)break ; } } -pci_dev_put (dev);return ret; }
@@ -5499,6 +5499,7 @@ static int discover_upi_topology(struct intel_uncore_type *type, int ubox_did, i devfn);if (dev) { ret =upi_fill_topology (dev, upi, idx); +pci_dev_put (dev);if (ret) goto err; }
@@ -5506,7 +5507,6 @@ static int discover_upi_topology(struct intel_uncore_type *type, int ubox_did, i } err:pci_dev_put (ubox); -pci_dev_put (dev);return ret; }
Signature · 1 line
2.54.0
[tip: perf/core] perf/x86/intel/uncore: Fix PCI device refcount leak in UPI discovery
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 81ec618f59415bdcf3e8989e2691c1f240be27fb Gitweb: https://git.kernel.org/tip/81ec618f59415bdcf3e8989e2691c1f240be27fb Author: Zide Chen <[email protected]> AuthorDate: Tue, 02 Jun 2026 07:49:04 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Thu, 04 Jun 2026 11:38:37 +02:00
perf/x86/intel/uncore: Fix PCI device refcount leak in UPI discovery
pci_get_domain_bus_and_slot() increments the reference count of the returned PCI device and therefore requires a matching pci_dev_put().
In skx_upi_topology_cb() and discover_upi_topology(), the lookup is performed inside a loop, but pci_dev_put() is only called once after the loop. As a result, references from all previous iterations are leaked.
Move pci_dev_put(dev) into the if (dev) block immediately after upi_fill_topology() returns.
Opportunistically, fix uninitialized variable in skx_upi_topology_cb().
--- arch/x86/events/intel/uncore_snbep.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/events/intel/uncore_snbep.c +3 -3
@@ -4261,7 +4261,7 @@ err:static int skx_upi_topology_cb (struct intel_uncore_type *type,int segment,int die,u64 cpu_bus_msr) { -int idx, ret; +int idx, ret =0 ;struct intel_uncore_topology *upi;unsigned int devfn;struct pci_dev *dev =NULL ;
@@ -4274,12 +4274,12 @@ static int skx_upi_topology_cb(struct intel_uncore_type *type, int segment, dev =pci_get_domain_bus_and_slot (segment, bus, devfn);if (dev) { ret =upi_fill_topology (dev, upi, idx); +pci_dev_put (dev);if (ret)break ; } } -pci_dev_put (dev);return ret; }
@@ -5499,6 +5499,7 @@ static int discover_upi_topology(struct intel_uncore_type *type, int ubox_did, i devfn);if (dev) { ret =upi_fill_topology (dev, upi, idx); +pci_dev_put (dev);if (ret) goto err; }
@@ -5506,7 +5507,6 @@ static int discover_upi_topology(struct intel_uncore_type *type, int ubox_did, i } err:pci_dev_put (ubox); -pci_dev_put (dev);return ret; }
PATCH v3 4/7 perf/x86/intel/uncore: Defer ADL global PMON enable to enable_box()
In reply to Zide ChenOn some Raptor Cove CPUs, enabling uncore PMON globally at driver init may increase power consumption even when no perf events are in use.
Drop adl_uncore_msr_init_box() and defer programming the global control register to enable_box(), so it is only set when a box is actually used.
IMC and IMC freerunning counters use a separate control path and are unaffected.
---
V3:
- Add Fixes tag. (Dapeng)
--- arch/x86/events/intel/uncore_snb.c | 7 ------- 1 file changed, 7 deletions(-)
arch/x86/events/intel/uncore_snb.c +0 -7
@@ -563,12 +563,6 @@ void tgl_uncore_cpu_init(void) skl_uncore_msr_ops.init_box = rkl_uncore_msr_init_box; } -static void adl_uncore_msr_init_box (struct intel_uncore_box *box) -{ -if (box->pmu->pmu_idx ==0 ) -wrmsrq (ADL_UNC_PERF_GLOBAL_CTL ,SNB_UNC_GLOBAL_CTL_EN ); -} -static void adl_uncore_msr_enable_box (struct intel_uncore_box *box) {wrmsrq (ADL_UNC_PERF_GLOBAL_CTL ,SNB_UNC_GLOBAL_CTL_EN );
@@ -587,7 +581,6 @@ static void adl_uncore_msr_exit_box(struct intel_uncore_box *box) }static struct intel_uncore_ops adl_uncore_msr_ops = { - .init_box = adl_uncore_msr_init_box, .enable_box = adl_uncore_msr_enable_box, .disable_box = adl_uncore_msr_disable_box, .exit_box = adl_uncore_msr_exit_box,
Signature · 1 line
2.54.0
Re: [PATCH V3 4/7] perf/x86/intel/uncore: Defer ADL global PMON enable to enable_box()
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The patch defers ADL global PMON enablement to `enable_box()` to save power when no events are…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch defers ADL global PMON enablement to enable_box() to save power when no events are in use, but fails to realize that enable_box() is unconditionally called by the perf core during event teardown, permanently re-enabling the PMON block after its first use. --
commit c42f45660bd3d0d4d416922cef01dcc3c1fb5e35
Author: Zide Chen <[email protected]>
perf/x86/intel/uncore: Defer ADL global PMON enable to enable_box()
This patch removes adl_uncore_msr_init_box() and defers programming the global control register to enable_box(). The goal is to reduce power consumption on Raptor Cove by enabling uncore PMON globally only when a box is actually used.
Quoted 4 lines
arch/x86/events/intel/uncore_snb.c +0
-0
[ ... ]
Quoted 13 lines
arch/x86/events/intel/uncore_snb.c +0
-6@@ -563,12 +563,6 @@ void tgl_uncore_cpu_init(void) skl_uncore_msr_ops.init_box = rkl_uncore_msr_init_box; } -static void adl_uncore_msr_init_box (struct intel_uncore_box *box) -{ -if (box->pmu->pmu_idx ==0 ) -wrmsrq (ADL_UNC_PERF_GLOBAL_CTL ,SNB_UNC_GLOBAL_CTL_EN ); -} -static void adl_uncore_msr_enable_box (struct intel_uncore_box *box) {wrmsrq (ADL_UNC_PERF_GLOBAL_CTL ,SNB_UNC_GLOBAL_CTL_EN );
Quoted 1 line
}
[Severity: Medium]
Will this leave the PMU permanently enabled after its first use?
During event teardown, the perf core brackets the removal with calls to perf_pmu_disable() and perf_pmu_enable(). perf_pmu_enable() will ultimately invoke uncore_pmu_enable(), which unconditionally calls enable_box().
Because adl_uncore_msr_enable_box() unconditionally writes the enable bit without checking if there are any remaining active events (for example, checking if box->n_active > 0), the global MSR appears to be immediately re-enabled at the end of the teardown process.
Does this mean the intended power savings are lost the first time the PMU is used?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
[tip: perf/core] perf/x86/intel/uncore: Defer ADL global PMON enable to enable_box()
In reply to Zide ChenThe following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 9a0bb848a37150aeccc10088e141339917d995dc Gitweb: https://git.kernel.org/tip/9a0bb848a37150aeccc10088e141339917d995dc Author: Zide Chen <[email protected]> AuthorDate: Tue, 02 Jun 2026 07:49:05 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Thu, 04 Jun 2026 11:38:37 +02:00
perf/x86/intel/uncore: Defer ADL global PMON enable to enable_box()
On some Raptor Cove CPUs, enabling uncore PMON globally at driver init may increase power consumption even when no perf events are in use.
Drop adl_uncore_msr_init_box() and defer programming the global control register to enable_box(), so it is only set when a box is actually used.
IMC and IMC freerunning counters use a separate control path and are unaffected.
--- arch/x86/events/intel/uncore_snb.c | 7 ------- 1 file changed, 7 deletions(-)
arch/x86/events/intel/uncore_snb.c +0 -7
@@ -563,12 +563,6 @@ void tgl_uncore_cpu_init(void) skl_uncore_msr_ops.init_box = rkl_uncore_msr_init_box; } -static void adl_uncore_msr_init_box (struct intel_uncore_box *box) -{ -if (box->pmu->pmu_idx ==0 ) -wrmsrq (ADL_UNC_PERF_GLOBAL_CTL ,SNB_UNC_GLOBAL_CTL_EN ); -} -static void adl_uncore_msr_enable_box (struct intel_uncore_box *box) {wrmsrq (ADL_UNC_PERF_GLOBAL_CTL ,SNB_UNC_GLOBAL_CTL_EN );
@@ -587,7 +581,6 @@ static void adl_uncore_msr_exit_box(struct intel_uncore_box *box) }static struct intel_uncore_ops adl_uncore_msr_ops = { - .init_box = adl_uncore_msr_init_box, .enable_box = adl_uncore_msr_enable_box, .disable_box = adl_uncore_msr_disable_box, .exit_box = adl_uncore_msr_exit_box,
PATCH v3 5/7 perf/x86/intel/uncore: Move die_to_cpu() to uncore.c
In reply to Zide ChenMove die_to_cpu() into uncore.c so it can be reused by the MSR initialization path, preparing for the introduction of an MSR global initialization callback.
Move the cpus_read_{lock,unlock}() out of the API, in order to make it possible to be called when the lock is being held.
Add the uncore_ prefix for consistency with other uncore APIs.
---
V2:
- Move cpus_read_{lock,unlock}() out of uncore_die_to_cpu() and rely on callers to manage the lock. (Sashiko)
- Remove "No functional change intended" from the changelog.
--- arch/x86/events/intel/uncore.c | 19 +++++++++++++++++++ arch/x86/events/intel/uncore.h | 1 + arch/x86/events/intel/uncore_snbep.c | 23 +++-------------------- 3 files changed, 23 insertions(+), 20 deletions(-)
arch/x86/events/intel/uncore.c +19 -0
@@ -83,6 +83,25 @@ int uncore_device_to_die(struct pci_dev *dev)return -1 ; } +/* +* Using cpus_read_lock() to ensure cpu is not going down between +* looking at cpu_online_mask. +* +* The lock must be held by the caller. +*/ +int uncore_die_to_cpu (int die) +{ +int res =0 , cpu; + +for_each_online_cpu (cpu) { +if (topology_logical_die_id (cpu) == die) { + res = cpu; +break ; + } + } +return res; +} +static void uncore_free_pcibus_map (void ) {struct pci2phy_map *map, *tmp;
arch/x86/events/intel/uncore.h +1 -0
@@ -235,6 +235,7 @@ struct pci2phy_map *__find_pci2phy_map(int segment);int uncore_pcibus_to_dieid (struct pci_bus *bus);int uncore_die_to_segment (int die);int uncore_device_to_die (struct pci_dev *dev); +int uncore_die_to_cpu (int die);ssize_t uncore_event_show (struct device *dev,struct device_attribute *attr,char *buf);
arch/x86/events/intel/uncore_snbep.c +3 -20
@@ -3704,25 +3704,6 @@ static int skx_msr_cpu_bus_read(int cpu, u64 *topology)return 0 ; } -static int die_to_cpu (int die) -{ -int res =0 , cpu, current_die; -/* -* Using cpus_read_lock() to ensure cpu is not going down between -* looking at cpu_online_mask. -*/ -cpus_read_lock (); -for_each_online_cpu (cpu) { - current_die =topology_logical_die_id (cpu); -if (current_die == die) { - res = cpu; -break ; - } - } -cpus_read_unlock (); -return res; -} -enum {IIO_TOPOLOGY_TYPE ,UPI_TOPOLOGY_TYPE ,
@@ -3794,8 +3775,9 @@ static int skx_pmu_get_topology(struct intel_uncore_type *type,int die, ret = -EPERM ;u64 cpu_bus_msr; +cpus_read_lock ();for (die =0 ; die <uncore_max_dies (); die++) { - ret =skx_msr_cpu_bus_read (die_to_cpu (die), &cpu_bus_msr); + ret =skx_msr_cpu_bus_read (uncore_die_to_cpu (die), &cpu_bus_msr);if (ret)break ;
@@ -3807,6 +3789,7 @@ static int skx_pmu_get_topology(struct intel_uncore_type *type,if (ret)break ; } +cpus_read_unlock ();return ret; }
Signature · 1 line
2.54.0
[tip: perf/core] perf/x86/intel/uncore: Move die_to_cpu() to uncore.c
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: ce044cfb7a365742b2e9229a4b70cf2a663d7270 Gitweb: https://git.kernel.org/tip/ce044cfb7a365742b2e9229a4b70cf2a663d7270 Author: Zide Chen <[email protected]> AuthorDate: Tue, 02 Jun 2026 07:49:06 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Thu, 04 Jun 2026 11:38:37 +02:00
perf/x86/intel/uncore: Move die_to_cpu() to uncore.c
Move die_to_cpu() into uncore.c so it can be reused by the MSR initialization path, preparing for the introduction of an MSR global initialization callback.
Move the cpus_read_{lock,unlock}() out of the API, in order to make it possible to be called when the lock is being held.
Add the uncore_ prefix for consistency with other uncore APIs.
--- arch/x86/events/intel/uncore.c | 19 +++++++++++++++++++ arch/x86/events/intel/uncore.h | 1 + arch/x86/events/intel/uncore_snbep.c | 23 +++-------------------- 3 files changed, 23 insertions(+), 20 deletions(-)
arch/x86/events/intel/uncore.c +19 -0
@@ -83,6 +83,25 @@ int uncore_device_to_die(struct pci_dev *dev)return -1 ; } +/* +* Using cpus_read_lock() to ensure cpu is not going down between +* looking at cpu_online_mask. +* +* The lock must be held by the caller. +*/ +int uncore_die_to_cpu (int die) +{ +int res =0 , cpu; + +for_each_online_cpu (cpu) { +if (topology_logical_die_id (cpu) == die) { + res = cpu; +break ; + } + } +return res; +} +static void uncore_free_pcibus_map (void ) {struct pci2phy_map *map, *tmp;
arch/x86/events/intel/uncore.h +1 -0
@@ -235,6 +235,7 @@ struct pci2phy_map *__find_pci2phy_map(int segment);int uncore_pcibus_to_dieid (struct pci_bus *bus);int uncore_die_to_segment (int die);int uncore_device_to_die (struct pci_dev *dev); +int uncore_die_to_cpu (int die);ssize_t uncore_event_show (struct device *dev,struct device_attribute *attr,char *buf);
arch/x86/events/intel/uncore_snbep.c +3 -20
@@ -3704,25 +3704,6 @@ static int skx_msr_cpu_bus_read(int cpu, u64 *topology)return 0 ; } -static int die_to_cpu (int die) -{ -int res =0 , cpu, current_die; -/* -* Using cpus_read_lock() to ensure cpu is not going down between -* looking at cpu_online_mask. -*/ -cpus_read_lock (); -for_each_online_cpu (cpu) { - current_die =topology_logical_die_id (cpu); -if (current_die == die) { - res = cpu; -break ; - } - } -cpus_read_unlock (); -return res; -} -enum {IIO_TOPOLOGY_TYPE ,UPI_TOPOLOGY_TYPE ,
@@ -3794,8 +3775,9 @@ static int skx_pmu_get_topology(struct intel_uncore_type *type,int die, ret = -EPERM ;u64 cpu_bus_msr; +cpus_read_lock ();for (die =0 ; die <uncore_max_dies (); die++) { - ret =skx_msr_cpu_bus_read (die_to_cpu (die), &cpu_bus_msr); + ret =skx_msr_cpu_bus_read (uncore_die_to_cpu (die), &cpu_bus_msr);if (ret)break ;
@@ -3807,6 +3789,7 @@ static int skx_pmu_get_topology(struct intel_uncore_type *type,if (ret)break ; } +cpus_read_unlock ();return ret; }
PATCH v3 6/7 perf/x86/intel/uncore: Fix uncore_die_to_cpu() for offline dies
In reply to Zide ChenIf the die is offline when uncore_die_to_cpu() is called, it silently returns 0, which is misleading. Return -1 in this case to indicate that all CPUs on the die are offline and the caller can take care of it accordingly.
Opportunistically, replace -EPERM with -ENODEV, as -ENODEV is the appropriate error when no CPUs are online across all dies.
--- arch/x86/events/intel/uncore.c | 2 +- arch/x86/events/intel/uncore_snbep.c | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-)
arch/x86/events/intel/uncore.c +1 -1
@@ -91,7 +91,7 @@ int uncore_device_to_die(struct pci_dev *dev)*/ int uncore_die_to_cpu (int die) { -int res =0 , cpu; +int res =-1 , cpu;for_each_online_cpu (cpu) {if (topology_logical_die_id (cpu) == die) {
arch/x86/events/intel/uncore_snbep.c +7 -2
@@ -3772,12 +3772,17 @@ static void pmu_free_topology(struct intel_uncore_type *type)static int skx_pmu_get_topology (struct intel_uncore_type *type,int (*topology_cb)(struct intel_uncore_type *,int ,int ,u64 )) { -int die, ret = -EPERM ; +int die, ret = -ENODEV ;u64 cpu_bus_msr; +int cpu;cpus_read_lock ();for (die =0 ; die <uncore_max_dies (); die++) { - ret =skx_msr_cpu_bus_read (uncore_die_to_cpu (die), &cpu_bus_msr); + cpu =uncore_die_to_cpu (die); +if (cpu ==-1 ) +continue ; + + ret =skx_msr_cpu_bus_read (cpu, &cpu_bus_msr);if (ret)break ;
Signature · 1 line
2.54.0
[tip: perf/core] perf/x86/intel/uncore: Fix uncore_die_to_cpu() for offline dies
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 2aa7dacf8a3d928303df9c770c1bd7f50d1f8f2a Gitweb: https://git.kernel.org/tip/2aa7dacf8a3d928303df9c770c1bd7f50d1f8f2a Author: Zide Chen <[email protected]> AuthorDate: Tue, 02 Jun 2026 07:49:07 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Thu, 04 Jun 2026 11:38:37 +02:00
perf/x86/intel/uncore: Fix uncore_die_to_cpu() for offline dies
If the die is offline when uncore_die_to_cpu() is called, it silently returns 0, which is misleading. Return -1 in this case to indicate that all CPUs on the die are offline and the caller can take care of it accordingly.
Opportunistically, replace -EPERM with -ENODEV, as -ENODEV is the appropriate error when no CPUs are online across all dies.
--- arch/x86/events/intel/uncore.c | 2 +- arch/x86/events/intel/uncore_snbep.c | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-)
arch/x86/events/intel/uncore.c +1 -1
@@ -91,7 +91,7 @@ int uncore_device_to_die(struct pci_dev *dev)*/ int uncore_die_to_cpu (int die) { -int res =0 , cpu; +int res =-1 , cpu;for_each_online_cpu (cpu) {if (topology_logical_die_id (cpu) == die) {
arch/x86/events/intel/uncore_snbep.c +7 -2
@@ -3772,12 +3772,17 @@ static void pmu_free_topology(struct intel_uncore_type *type)static int skx_pmu_get_topology (struct intel_uncore_type *type,int (*topology_cb)(struct intel_uncore_type *,int ,int ,u64 )) { -int die, ret = -EPERM ; +int die, ret = -ENODEV ;u64 cpu_bus_msr; +int cpu;cpus_read_lock ();for (die =0 ; die <uncore_max_dies (); die++) { - ret =skx_msr_cpu_bus_read (uncore_die_to_cpu (die), &cpu_bus_msr); + cpu =uncore_die_to_cpu (die); +if (cpu ==-1 ) +continue ; + + ret =skx_msr_cpu_bus_read (cpu, &cpu_bus_msr);if (ret)break ;
PATCH v3 7/7 perf/x86/intel/uncore: Implement global init callback for GNR uncore
In reply to Zide ChenOn Sierra Forest and Clearwater Forest, the FRZ_ALL bit in the global control register defaults to 0 at boot, but UBOX PMON units do not work until the global control register is explicitly written with 0 to trigger hardware initialization properly.
Implement the generic uncore_msr_global_init() callback and add it to gnr_uncore_init[], which is shared by GNR, GRR, SRF, and CWF.
---
v3:
- Guard uncore_discovery_pci() with cpus_read_lock() to fix a theoretical race where CPUs could go offline between uncore_die_to_cpu() and wrmsrq_on_cpu(). (Sashiko)
- Add Fixes tag. (Dapeng)
v2:
- Propagate return value of wrmsrq_on_cpu() to global_init().
--- arch/x86/events/intel/uncore.c | 13 ++++++++++++- arch/x86/events/intel/uncore.h | 2 +- arch/x86/events/intel/uncore_discovery.c | 9 +++++---- 3 files changed, 18 insertions(+), 6 deletions(-)
arch/x86/events/intel/uncore.c +12 -1
@@ -1716,7 +1716,7 @@ static int __init uncore_mmio_init(void)return ret; } -static int uncore_mmio_global_init (u64 ctl) +static int uncore_mmio_global_init (int die,u64 ctl) {void __iomem *io_addr;
@@ -1731,6 +1731,16 @@ static int uncore_mmio_global_init(u64 ctl)return 0 ; } +static int uncore_msr_global_init (int die,u64 msr) +{ +int cpu =uncore_die_to_cpu (die); + +if (cpu ==-1 ) +return -ENODEV ; + +return wrmsrq_on_cpu (cpu, msr,0 ); +} +static const struct uncore_plat_init nhm_uncore_init__initconst = { .cpu_init = nhm_uncore_cpu_init, };
@@ -1871,6 +1881,7 @@ static const struct uncore_plat_init gnr_uncore_init __initconst = { .domain[0].base_is_pci = true, .domain[0].discovery_base = UNCORE_DISCOVERY_TABLE_DEVICE, .domain[0].units_ignore = gnr_uncore_units_ignore, + .domain[0].global_init = uncore_msr_global_init, };static const struct uncore_plat_init dmr_uncore_init__initconst = {
arch/x86/events/intel/uncore.h +1 -1
@@ -53,7 +53,7 @@ struct uncore_discovery_domain {/* MSR address or PCI device used as the discovery base */ u32 discovery_base;bool base_is_pci; -int (*global_init)(u64 ctl); +int (*global_init)(int die,u64 ctl);/* The units in the discovery table should be ignored. */ int *units_ignore;
arch/x86/events/intel/uncore_discovery.c +5 -4
@@ -287,7 +287,7 @@ static int __parse_discovery_table(struct uncore_discovery_domain *domain,if (!io_addr)return -ENOMEM ; -if (domain->global_init && domain->global_init (global.ctl)) { +if (domain->global_init && domain->global_init (die, global.ctl)) { ret = -ENODEV ; goto out; }
@@ -399,7 +399,6 @@ static bool uncore_discovery_msr(struct uncore_discovery_domain *domain)if (!die_mask)return false; -cpus_read_lock ();for_each_online_cpu (cpu) { die =topology_logical_die_id (cpu);if (__test_and_set_bit (die, die_mask))
@@ -414,8 +413,6 @@ static bool uncore_discovery_msr(struct uncore_discovery_domain *domain)__parse_discovery_table (domain, base, die, &parsed); } -cpus_read_unlock (); -kfree (die_mask);return parsed; }
@@ -429,10 +426,14 @@ bool uncore_discovery(struct uncore_plat_init *init)for (i =0 ; i <UNCORE_DISCOVERY_DOMAINS ; i++) { domain = &init->domain[i];if (domain->discovery_base) { +cpus_read_lock (); +if (!domain->base_is_pci) ret |=uncore_discovery_msr (domain);else ret |=uncore_discovery_pci (domain); + +cpus_read_unlock (); } }
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH V3 7/7] perf/x86/intel/uncore: Implement global init callback for GNR uncore
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] Hardware trust boundary violation: A spoofed PCIe device can supply an arbitrary `global.ctl`…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Hardware trust boundary violation: A spoofed PCIe device can supply an arbitrary global.ctl value, leading to arbitrary MSR writes.
- [Medium] The newly added uncore_msr_global_init() uses the unchecked wrmsrq_on_cpu() function to write to an MSR address obtained from the hardware discovery table. --
commit b0a74f9b5d22ecfcf334d277878f153181fb3107
Author: Zide Chen <[email protected]>
perf/x86/intel/uncore: Implement global init callback for GNR uncore
On Sierra Forest and Clearwater Forest, the FRZ_ALL bit in the global control register defaults to 0 at boot, but UBOX PMON units do not work until the global control register is explicitly written with 0 to trigger hardware initialization properly.
Quoted 3 lines
arch/x86/events/intel/uncore.c +0
-0
[ ... ]
arch/x86/events/intel/uncore.c static int uncore_mmio_global_init(u64 ctl) · line 1741
Quoted 12 lines
arch/x86/events/intel/uncore.c +8
-0@@ -1731,6 +1731,16 @@ static int uncore_mmio_global_init(u64 ctl)return 0 ; } +static int uncore_msr_global_init (int die,u64 msr) +{ +int cpu =uncore_die_to_cpu (die); + +if (cpu ==-1 ) +return -ENODEV ; + +return wrmsrq_on_cpu (cpu, msr,0 );
[Severity: Medium]
Since the msr parameter is derived from global.ctl which is read from the hardware discovery table, could an incorrect value from firmware trigger an unchecked MSR access warning? Would it be better to use wrmsrq_safe_on_cpu() here to prevent scary traces if the hardware table is invalid?
Quoted 3 lines
arch/x86/events/intel/uncore.c +2
-0+} +static const struct uncore_plat_init nhm_uncore_init__initconst = {
[ ... ]
Quoted 3 lines
arch/x86/events/intel/uncore_discovery.c +0
-0
[ ... ]
arch/x86/events/intel/uncore_discovery.c static int __parse_discovery_table(struct uncore_discovery_domain *domain, · line 290
Quoted 6 lines
arch/x86/events/intel/uncore_discovery.c +1
-1@@ -287,7 +287,7 @@ static int __parse_discovery_table(struct uncore_discovery_domain *domain,if (!io_addr)return -ENOMEM ; -if (domain->global_init && domain->global_init (global.ctl)) { +if (domain->global_init && domain->global_init (die, global.ctl)) {
[Severity: Critical]
Does this allow an external device to write 0 to arbitrary MSRs?
The uncore discovery code scans the PCI bus for devices matching the Intel Vendor ID and discovery Device ID without restricting the search to internal CPU buses. If a malicious external PCIe device (e.g., via Thunderbolt) spoofs these IDs, it can provide a fake discovery table.
Could this attacker-controlled global.ctl value then be passed to uncore_msr_global_init() and used as the msr index, allowing an attacker to overwrite security-sensitive MSRs like MSR_LSTAR or MSR_GS_BASE with 0?
Quoted 3 lines
arch/x86/events/intel/uncore_discovery.c +0
-0ret = -ENODEV ; goto out; }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
[tip: perf/core] perf/x86/intel/uncore: Implement global init callback for GNR uncore
In reply to Zide ChenThe following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 1a308a168c9286a335a05926204ef3ebb68fba1c Gitweb: https://git.kernel.org/tip/1a308a168c9286a335a05926204ef3ebb68fba1c Author: Zide Chen <[email protected]> AuthorDate: Tue, 02 Jun 2026 07:49:08 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Thu, 04 Jun 2026 11:38:38 +02:00
perf/x86/intel/uncore: Implement global init callback for GNR uncore
On Sierra Forest and Clearwater Forest, the FRZ_ALL bit in the global control register defaults to 0 at boot, but UBOX PMON units do not work until the global control register is explicitly written with 0 to trigger hardware initialization properly.
Implement the generic uncore_msr_global_init() callback and add it to gnr_uncore_init[], which is shared by GNR, GRR, SRF, and CWF.
--- arch/x86/events/intel/uncore.c | 13 ++++++++++++- arch/x86/events/intel/uncore.h | 2 +- arch/x86/events/intel/uncore_discovery.c | 9 +++++---- 3 files changed, 18 insertions(+), 6 deletions(-)
arch/x86/events/intel/uncore.c +12 -1
@@ -1716,7 +1716,7 @@ err:return ret; } -static int uncore_mmio_global_init (u64 ctl) +static int uncore_mmio_global_init (int die,u64 ctl) {void __iomem *io_addr;
@@ -1731,6 +1731,16 @@ static int uncore_mmio_global_init(u64 ctl)return 0 ; } +static int uncore_msr_global_init (int die,u64 msr) +{ +int cpu =uncore_die_to_cpu (die); + +if (cpu ==-1 ) +return -ENODEV ; + +return wrmsrq_on_cpu (cpu, msr,0 ); +} +static const struct uncore_plat_init nhm_uncore_init__initconst = { .cpu_init = nhm_uncore_cpu_init, };
@@ -1871,6 +1881,7 @@ static const struct uncore_plat_init gnr_uncore_init __initconst = { .domain[0].base_is_pci = true, .domain[0].discovery_base = UNCORE_DISCOVERY_TABLE_DEVICE, .domain[0].units_ignore = gnr_uncore_units_ignore, + .domain[0].global_init = uncore_msr_global_init, };static const struct uncore_plat_init dmr_uncore_init__initconst = {
arch/x86/events/intel/uncore.h +1 -1
@@ -53,7 +53,7 @@ struct uncore_discovery_domain {/* MSR address or PCI device used as the discovery base */ u32 discovery_base;bool base_is_pci; -int (*global_init)(u64 ctl); +int (*global_init)(int die,u64 ctl);/* The units in the discovery table should be ignored. */ int *units_ignore;
arch/x86/events/intel/uncore_discovery.c +5 -4
@@ -287,7 +287,7 @@ static int __parse_discovery_table(struct uncore_discovery_domain *domain,if (!io_addr)return -ENOMEM ; -if (domain->global_init && domain->global_init (global.ctl)) { +if (domain->global_init && domain->global_init (die, global.ctl)) { ret = -ENODEV ; goto out; }
@@ -399,7 +399,6 @@ static bool uncore_discovery_msr(struct uncore_discovery_domain *domain)if (!die_mask)return false; -cpus_read_lock ();for_each_online_cpu (cpu) { die =topology_logical_die_id (cpu);if (__test_and_set_bit (die, die_mask))
@@ -414,8 +413,6 @@ static bool uncore_discovery_msr(struct uncore_discovery_domain *domain)__parse_discovery_table (domain, base, die, &parsed); } -cpus_read_unlock (); -kfree (die_mask);return parsed; }
@@ -429,10 +426,14 @@ bool uncore_discovery(struct uncore_plat_init *init)for (i =0 ; i <UNCORE_DISCOVERY_DOMAINS ; i++) { domain = &init->domain[i];if (domain->discovery_base) { +cpus_read_lock (); +if (!domain->base_is_pci) ret |=uncore_discovery_msr (domain);else ret |=uncore_discovery_pci (domain); + +cpus_read_unlock (); } }
All patches look good to me. Thanks.
Quoted 37 lines · Zide Chen
This series includes bug fixes and cleanups for the Intel uncore PMU driver.
- Patch 1 fixes a theoretical bug in discovery unit lookup on multi-die systems.
- Patch 2 guards against an invalid box control address.
- Patch 3 fixes a PCI device refcount leak in UPI topology discovery.
- Patch 4 works around a hardware issue on Raptor Cove CPUs.
- Patches 5-7 implement a global MSR init callback for GNR/GRR/SRF/CWF uncore.Changes in v3:
- Protectuncore_discovery_pci()withcpus_read_lock(). (Sashiko)
- Add Fixes tag for patch 4/7 and 7/7.
- Add the missing '---' in patch 5.Changes in v2:
- Add patch 2 to guard against invalid box control address (Sashiko)
- RemoveWARN_ON_ONCE()from patch 1
- Move cpus_read_{lock,unlock}() out ofuncore_die_to_cpu()(Sashiko)Zide Chen (7):
perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems perf/x86/intel/uncore: Guard against invalid box control address perf/x86/intel/uncore: Fix PCI device refcount leak in UPI discovery perf/x86/intel/uncore: Defer ADL global PMON enable toenable_box()perf/x86/intel/uncore: Movedie_to_cpu()touncore.cperf/x86/intel/uncore: Fixuncore_die_to_cpu()for offline dies perf/x86/intel/uncore: Implement global init callback for GNR uncorearch/x86/events/intel/uncore.c | 32 ++++++++++++++++- arch/x86/events/intel/uncore.h | 3 +- arch/x86/events/intel/uncore_discovery.c | 46 ++++++++++++++++-------- arch/x86/events/intel/uncore_snb.c | 7 ---- arch/x86/events/intel/uncore_snbep.c | 36 +++++++------------ 5 files changed, 76 insertions(+), 48 deletions(-)