From: Yicong Yang yangyicong@hisilicon.com
[ Upstream commit 43de0ac332b815cf56dbdce63687de9acfd35d49 ]
Event ID is only using the attr::config bit [7, 0] but we check the event range using the whole 64bit field. It blocks the usage of the rest field of attr::config. Relax the check by only using the bit [7, 0].
Acked-by: Jonathan Cameron jonathan.cameron@huawei.com Signed-off-by: Yicong Yang yangyicong@hisilicon.com Signed-off-by: Yushan Wang wangyushan12@huawei.com Signed-off-by: Will Deacon will@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Backport Analysis: HiSilicon PMU Event ID Check Fix
**Backport Recommendation: YES**
### Detailed Analysis
#### Bug Being Fixed
The commit fixes an overly restrictive validation bug in `hisi_uncore_pmu_event_init()` at **drivers/perf/hisilicon/hisi_uncore_pmu.c:237**.
**Original code:** ```c if (event->attr.config > hisi_pmu->check_event) return -EINVAL; ```
**Fixed code:** ```c if ((event->attr.config & HISI_EVENTID_MASK) > hisi_pmu->check_event) return -EINVAL; ```
Where `HISI_EVENTID_MASK = GENMASK(7, 0) = 0xFF`.
#### Root Cause Analysis
1. **Event ID Layout:** HiSilicon uncore PMUs use only bits [7:0] of `attr.config` for the event ID. This is evident from format attributes across all drivers: - `hisi_uncore_l3c_pmu.c:386`: `"config:0-7"` - `hisi_uncore_hha_pmu.c:336`: `"config:0-7"` - `hisi_uncore_pa_pmu.c:304`: `"config:0-7"` - `hisi_uncore_sllc_pmu.c:368`: `"config:0-7"` - `hisi_uncore_uc_pmu.c:402`: `"config:0-7"` - `hisi_uncore_ddrc_pmu.c:272`: `"config:0-4"` (V1) and `"config:0-7"` (V2) - `hisi_uncore_cpa_pmu.c:205`: `"config:0-15"` (exception)
2. **Incorrect Validation:** The validation was comparing the entire 64-bit `attr.config` value against `check_event` (typically 0xFF for 8-bit event IDs), which would incorrectly reject valid event configurations if any upper bits were set.
3. **Blocking Future Extensions:** The commit message explicitly states: "It blocks the usage of the rest field of attr::config." This indicates that upper bits of `attr.config` may be needed for additional configuration parameters beyond the base event ID.
#### Scope of Impact
This fix affects all HiSilicon uncore PMU drivers that use the shared `hisi_pmu_init()` function, which sets `pmu->event_init = hisi_uncore_pmu_event_init` (at **hisi_uncore_pmu.c:610**):
- L3C PMU (L3 Cache) - HHA PMU (Hydra Home Agent) - DDRC PMU (DDR Controller) - PA PMU (Protocol Adapter) - SLLC PMU (Super L3 Cache) - UC PMU (Uncore) - CPA PMU (Coherent Protocol Agent)
#### Code Changes Analysis
**File 1: drivers/perf/hisilicon/hisi_uncore_pmu.c** - **Line 237:** Mask `attr.config` with `HISI_EVENTID_MASK` before comparison - **Impact:** Only validates event ID bits [7:0], allowing upper bits for other purposes
**File 2: drivers/perf/hisilicon/hisi_uncore_pmu.h** - **Line 46:** Define `HISI_EVENTID_MASK` as `GENMASK(7, 0)` - **Line 47:** Update `HISI_GET_EVENTID` macro to use the new mask (consistency improvement) - **Impact:** Provides centralized, self-documenting definition of event ID field
#### Backport Suitability Assessment
**Positive Factors:** 1. ✅ **Fixes a clear bug:** Incorrect validation logic blocking legitimate use cases 2. ✅ **Small and contained:** Only 5 lines changed across 2 files 3. ✅ **Low regression risk:** The change makes validation less strict, not more strict 4. ✅ **Well-reviewed:** Acked by Jonathan Cameron (prominent kernel maintainer) 5. ✅ **No dependencies:** Self-contained fix with no related commits 6. ✅ **Driver-level fix:** Affects only HiSilicon PMU drivers, not core kernel 7. ✅ **Minimal side effects:** Only affects event validation path during initialization
**Risk Assessment:** - **Regression risk:** Very low - relaxing validation cannot break working configurations - **Functional risk:** None - the fix enables correct behavior - **Architectural risk:** None - no architectural changes - **Dependency risk:** None - no follow-up fixes or related patches required
**Follows Stable Tree Rules:** - ✅ Fixes an important bug affecting users of HiSilicon hardware - ✅ Small, obvious, and correct change - ✅ No new features introduced - ✅ Minimal risk of regression - ✅ Confined to specific driver subsystem
### Conclusion
This commit is an **excellent candidate for backporting** to stable kernel trees. It fixes a genuine validation bug that prevents legitimate usage of the perf event configuration interface on HiSilicon hardware. The fix is minimal, well-contained, properly reviewed, and carries virtually no regression risk.
drivers/perf/hisilicon/hisi_uncore_pmu.c | 2 +- drivers/perf/hisilicon/hisi_uncore_pmu.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c index a449651f79c9f..6594d64b03a9e 100644 --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c @@ -234,7 +234,7 @@ int hisi_uncore_pmu_event_init(struct perf_event *event) return -EINVAL;
hisi_pmu = to_hisi_pmu(event->pmu); - if (event->attr.config > hisi_pmu->check_event) + if ((event->attr.config & HISI_EVENTID_MASK) > hisi_pmu->check_event) return -EINVAL;
if (hisi_pmu->on_cpu == -1) diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h index 777675838b808..e69660f72be67 100644 --- a/drivers/perf/hisilicon/hisi_uncore_pmu.h +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h @@ -43,7 +43,8 @@ return FIELD_GET(GENMASK_ULL(hi, lo), event->attr.config); \ }
-#define HISI_GET_EVENTID(ev) (ev->hw.config_base & 0xff) +#define HISI_EVENTID_MASK GENMASK(7, 0) +#define HISI_GET_EVENTID(ev) ((ev)->hw.config_base & HISI_EVENTID_MASK)
#define HISI_PMU_EVTYPE_BITS 8 #define HISI_PMU_EVTYPE_SHIFT(idx) ((idx) % 4 * HISI_PMU_EVTYPE_BITS)