| Message ID | e54e48ab228aa386fb4c5f64ee02ef5291f2d3da.1737511963.git.jpoimboe@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | unwind, perf: sframe user space unwinding | expand |
On Tue, Jan 21, 2025 at 06:31:06PM -0800, Josh Poimboeuf wrote: > get_segment_base() will be used by the unwind_user code, so make it > global and rename it so it doesn't conflict with a KVM function of the > same name. Should it not also get moved out of the perf code in this case?
On Wed, Jan 22, 2025 at 01:51:54PM +0100, Peter Zijlstra wrote: > On Tue, Jan 21, 2025 at 06:31:06PM -0800, Josh Poimboeuf wrote: > > get_segment_base() will be used by the unwind_user code, so make it > > global and rename it so it doesn't conflict with a KVM function of the > > same name. > > Should it not also get moved out of the perf code in this case? True, I'll try to find it a better home.
On Tue, 21 Jan 2025 18:31:06 -0800 Josh Poimboeuf <jpoimboe@kernel.org> wrote: > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > index c75c482d4c52..23ac6343cf86 100644 > --- a/arch/x86/events/core.c > +++ b/arch/x86/events/core.c > @@ -2790,7 +2790,7 @@ valid_user_frame(const void __user *fp, unsigned long size) > return __access_ok(fp, size); > } > > -static unsigned long get_segment_base(unsigned int segment) > +unsigned long segment_base_address(unsigned int segment) > { > struct desc_struct *desc; > unsigned int idx = segment >> 3; As this requires interrupts disabled, and if you do move this out of this file, you probably should add: lockdep_assert_irqs_disabled(); -- Steve
On Fri, Jan 24, 2025 at 03:09:27PM -0500, Steven Rostedt wrote: > On Tue, 21 Jan 2025 18:31:06 -0800 > Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > > index c75c482d4c52..23ac6343cf86 100644 > > --- a/arch/x86/events/core.c > > +++ b/arch/x86/events/core.c > > @@ -2790,7 +2790,7 @@ valid_user_frame(const void __user *fp, unsigned long size) > > return __access_ok(fp, size); > > } > > > > -static unsigned long get_segment_base(unsigned int segment) > > +unsigned long segment_base_address(unsigned int segment) > > { > > struct desc_struct *desc; > > unsigned int idx = segment >> 3; > > As this requires interrupts disabled, and if you do move this out of this > file, you probably should add: > > lockdep_assert_irqs_disabled(); Indeed...
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index c75c482d4c52..23ac6343cf86 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -2790,7 +2790,7 @@ valid_user_frame(const void __user *fp, unsigned long size) return __access_ok(fp, size); } -static unsigned long get_segment_base(unsigned int segment) +unsigned long segment_base_address(unsigned int segment) { struct desc_struct *desc; unsigned int idx = segment >> 3; @@ -2874,8 +2874,8 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent if (user_64bit_mode(regs)) return 0; - cs_base = get_segment_base(regs->cs); - ss_base = get_segment_base(regs->ss); + cs_base = segment_base_address(regs->cs); + ss_base = segment_base_address(regs->ss); fp = compat_ptr(ss_base + regs->bp); pagefault_disable(); @@ -2994,11 +2994,11 @@ static unsigned long code_segment_base(struct pt_regs *regs) return 0x10 * regs->cs; if (user_mode(regs) && regs->cs != __USER_CS) - return get_segment_base(regs->cs); + return segment_base_address(regs->cs); #else if (user_mode(regs) && !user_64bit_mode(regs) && regs->cs != __USER32_CS) - return get_segment_base(regs->cs); + return segment_base_address(regs->cs); #endif return 0; } diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h index d95f902acc52..75956c68356f 100644 --- a/arch/x86/include/asm/perf_event.h +++ b/arch/x86/include/asm/perf_event.h @@ -639,4 +639,6 @@ static __always_inline void perf_lopwr_cb(bool lopwr_in) #define arch_perf_out_copy_user copy_from_user_nmi +unsigned long segment_base_address(unsigned int segment); + #endif /* _ASM_X86_PERF_EVENT_H */
get_segment_base() will be used by the unwind_user code, so make it global and rename it so it doesn't conflict with a KVM function of the same name. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- arch/x86/events/core.c | 10 +++++----- arch/x86/include/asm/perf_event.h | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-)