-
Notifications
You must be signed in to change notification settings - Fork 105
[Deepin-Kernel-SIG] [linux 6.6-y] [Deepin] Fix kabi for CWF PMU support #1378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Deepin-Kernel-SIG] [linux 6.6-y] [Deepin] Fix kabi for CWF PMU support #1378
Conversation
Intel inclusion category: bugfix bugzilla: https://gitee.com/openeuler/intel-kernel/issues/ICZHEB CVE: NA -------------------------------- Following upstream commits introduced 2 fields (config1 and dyn_constraint) in struct hw_perf_event, which breaks kABI. ec980e4 ("perf/x86/intel: Support auto counter reload") 4dfe323 ("perf/x86: Add dynamic constraint") To fix this kABI breakage, we introduce struct hw_perf_event_ext, and use one KABI_RESERVE field in struct perf_event as pointer to this struct hw_perf_event_ext. This is viable because hw_perf_event is always embedded in struct perf_event, so we can always access hw_perf_event_ext from perf_event when needed. We also create a kmem_cache for struct hw_per_event_ext. Another kABI changes are caused by the following commit: 0e102ce ("KVM: x86/pmu: Change ambiguous _mask suffix to _rsvd in kvm_pmu") But the fix is trivial. Fixes: ec980e4 ("perf/x86/intel: Support auto counter reload") Fixes: 4dfe323 ("perf/x86: Add dynamic constraint") Signed-off-by: Jason Zeng <[email protected]> Link: deepin-community#1356 [Backport: drop arch/x86/include/asm/kvm_host.h for no rename it] Signed-off-by: Wentao Guan <[email protected]>
Reviewer's GuideIntroduces a kABI-safe extension structure for perf event hardware fields used by Intel CWF PMU (config1 and dyn_constraint), wiring it through perf core allocation/free paths and updating x86 PMU code to use the new extension pointer instead of embedding these fields directly in struct hw_perf_event. Sequence diagram for perf_event and hw_perf_event_ext allocation and freesequenceDiagram
participant Init as perf_event_init
participant Alloc as perf_event_alloc
participant Free as free_event_rcu
participant PEC as perf_event_cache
participant PHC as perf_hw_event_cache
Init->>PEC: perf_event_cache = KMEM_CACHE(perf_event, SLAB_PANIC)
Init->>PHC: perf_hw_event_cache = KMEM_CACHE(hw_perf_event_ext, SLAB_PANIC)
Alloc->>PEC: kmem_cache_alloc_node(perf_event_cache, GFP_KERNEL, node)
PEC-->>Alloc: perf_event* event
Alloc->>PHC: kmem_cache_alloc_node(perf_hw_event_cache, GFP_KERNEL|__GFP_ZERO, node)
PHC-->>Alloc: hw_perf_event_ext* hw_ext
Alloc->>Alloc: event->hw_ext = hw_ext
Alloc->>Alloc: event->hw_ext->dyn_constraint = ~0ULL
Free->>Free: perf_event_free_filter(event)
Free->>PHC: kmem_cache_free(perf_hw_event_cache, event->hw_ext)
Free->>PEC: kmem_cache_free(perf_event_cache, event)
Class diagram for kABI-safe perf event hardware extensionclassDiagram
class perf_event {
+u32 orig_type
+hw_perf_event hw
+hw_perf_event_ext* hw_ext
}
class hw_perf_event {
+u64 config
+u64 last_tag
+unsigned long config_base
+unsigned long event_base
+int event_base_rdpmc
+int idx
+int last_cpu
+u64 sample_period
+u64 period_left
+u64 interrupt_period
+u64 interrupts
+u64 prev_count
+u64 period
+u64 pwritten
+u32 state
+u16 extra_reg_idx
+u16 branch_reg_idx
+unsigned long flags
+u64 addr_filters
}
class hw_perf_event_ext {
+u64 config1
+u64 dyn_constraint
}
class kmem_cache {
+void* objects
}
perf_event --> hw_perf_event : embeds
perf_event --> hw_perf_event_ext : hw_ext
kmem_cache <.. perf_event : perf_event_cache
kmem_cache <.. hw_perf_event_ext : perf_hw_event_cache
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这是一个关于Linux内核性能事件(perf_event)子系统的代码变更审查。我来从多个角度分析这个改动:
总的来说,这是一个合理的重构,提高了代码的模块化程度,但需要确保正确处理了所有边界情况和错误条件。建议在合并前进行充分的测试,特别是在内存压力和高并发场景下的测试。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Several call sites now unconditionally dereference
event->hw_ext(e.g., inintel_pmu_enable_acr,intel_get_event_constraints,__x86_pmu_event_init); consider adding assertions or defensive checks, or clearly documenting that all perf events must go throughperf_event_allocsohw_extis always initialized and never NULL. perf_hw_event_cacheis allocated inperf_event_initbut used infree_event_rcuandperf_event_alloc; please double‑check that no perf events can be allocated or freed beforeperf_event_initruns, and that all perf event free paths (not onlyfree_event_rcu) go through the newkmem_cache_free(perf_hw_event_cache, event->hw_ext).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several call sites now unconditionally dereference `event->hw_ext` (e.g., in `intel_pmu_enable_acr`, `intel_get_event_constraints`, `__x86_pmu_event_init`); consider adding assertions or defensive checks, or clearly documenting that all perf events must go through `perf_event_alloc` so `hw_ext` is always initialized and never NULL.
- `perf_hw_event_cache` is allocated in `perf_event_init` but used in `free_event_rcu` and `perf_event_alloc`; please double‑check that no perf events can be allocated or freed before `perf_event_init` runs, and that all perf event free paths (not only `free_event_rcu`) go through the new `kmem_cache_free(perf_hw_event_cache, event->hw_ext)`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lanlanxiyiji, shy129 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes kernel ABI (kABI) breakage introduced by upstream commits that added config1 and dyn_constraint fields to struct hw_perf_event. To maintain kABI compatibility, these fields are moved to a new struct hw_perf_event_ext structure, which is allocated separately and accessed through a pointer stored in a reserved DEEPIN_KABI field in struct perf_event.
Key Changes
- Created
hw_perf_event_extstructure to holdconfig1anddyn_constraintfields - Added dedicated kmem_cache for
hw_perf_event_extallocation - Updated all field accesses from
event->hw.fieldtoevent->hw_ext->fieldin x86 PMU code
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| include/linux/perf_event.h | Defines hw_perf_event_ext struct, removes fields from hw_perf_event, uses DEEPIN_KABI_USE macro for hw_ext pointer |
| kernel/events/core.c | Adds kmem_cache for hw_perf_event_ext, allocates/frees extension structure |
| arch/x86/events/core.c | Updates dyn_constraint initialization to use hw_ext |
| arch/x86/events/intel/core.c | Updates all config1 and dyn_constraint references to use hw_ext pointer |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| union { | ||
| struct { | ||
| u64 config1; | ||
| u64 dyn_constraint; | ||
| }; | ||
| }; |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The union wrapper around the struct fields serves no purpose since there's only one member in the union. Consider removing the union and keeping only the struct with config1 and dyn_constraint fields. This would simplify the code structure without affecting functionality.
Intel inclusion
category: bugfix
bugzilla: https://gitee.com/openeuler/intel-kernel/issues/ICZHEB CVE: NA
Following upstream commits introduced 2 fields (config1 and dyn_constraint) in struct hw_perf_event, which breaks kABI.
To fix this kABI breakage, we introduce struct hw_perf_event_ext, and use one KABI_RESERVE field in struct perf_event as pointer to this struct hw_perf_event_ext. This is viable because hw_perf_event is always embedded in struct perf_event, so we can always access hw_perf_event_ext from perf_event when needed.
We also create a kmem_cache for struct hw_per_event_ext.
Another kABI changes are caused by the following commit:
But the fix is trivial.
Fixes: ec980e4 ("perf/x86/intel: Support auto counter reload")
Fixes: 4dfe323 ("perf/x86: Add dynamic constraint")
Link: #1356
[Backport: drop arch/x86/include/asm/kvm_host.h for no rename it]
Summary by Sourcery
Maintain kABI compatibility for Intel PMU CWF support by moving new perf event fields into an extension struct and wiring it through allocation and usage paths.
Bug Fixes:
Enhancements: