* [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support @ 2024-06-03 7:37 Jinjie Ruan 2024-06-03 8:39 ` Russell King (Oracle) 0 siblings, 1 reply; 5+ messages in thread From: Jinjie Ruan @ 2024-06-03 7:37 UTC (permalink / raw) To: linux, oleg, arnd, afd, akpm, rmk+kernel, linus.walleij, eric.devolder, robh, linux-arm-kernel, linux-kernel Cc: ruanjinjie Currently, kprobe on ARM32 can not use the '$argx' syntax available on other architecture. So implement regs_get_kernel_argument() and add HAVE_FUNCTION_ARG_ACCESS_API support. Before there is a following error: echo 'p:kprobe_submit_bio submit_bio bio=$arg1' > kprobe_events sh: write error: Invalid argument After: # echo 'p:kprobe_submit_bio submit_bio bio=$arg1' > kprobe_events # echo 1 > events/kprobes/enable # echo 1 > events/kprobes/kprobe_submit_bio/enable # echo 0 > tracing_on # echo > trace # echo 1 > tracing_on # cat trace kworker/u19:0-36 [002] d.... 54.175322: kprobe_submit_bio: (submit_bio+0x0/0xf8) bio=0xc24e6000 Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- arch/arm/Kconfig | 1 + arch/arm/include/asm/ptrace.h | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index ee5115252aac..4ed504139763 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -102,6 +102,7 @@ config ARM select HAVE_EXIT_THREAD select HAVE_GUP_FAST if ARM_LPAE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL + select HAVE_FUNCTION_ARG_ACCESS_API select HAVE_FUNCTION_ERROR_INJECTION select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_TRACER if !XIP_KERNEL diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h index 6eb311fb2da0..da5dd4cd0324 100644 --- a/arch/arm/include/asm/ptrace.h +++ b/arch/arm/include/asm/ptrace.h @@ -52,6 +52,28 @@ struct svc_pt_regs { #define fast_interrupts_enabled(regs) \ (!((regs)->ARM_cpsr & PSR_F_BIT)) +/** + * regs_get_kernel_argument() - get Nth function argument in kernel + * @regs: pt_regs of that context + * @n: function argument number (start from 0) + * + * regs_get_argument() returns @n th argument of the function call. + * + * Note that this chooses the most likely register mapping. In very rare + * cases this may not return correct data, for example, if one of the + * function parameters is 16 bytes or bigger. In such cases, we cannot + * get access the parameter correctly and the register assignment of + * subsequent parameters will be shifted. + */ +static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs, + unsigned int n) +{ +#define NR_REG_ARGUMENTS 4 + if (n < NR_REG_ARGUMENTS) + return regs->uregs[n]; + return 0; +} + /* Are the current registers suitable for user mode? * (used to maintain security in signal handlers) */ -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support 2024-06-03 7:37 [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support Jinjie Ruan @ 2024-06-03 8:39 ` Russell King (Oracle) 2024-06-04 1:36 ` Jinjie Ruan 0 siblings, 1 reply; 5+ messages in thread From: Russell King (Oracle) @ 2024-06-03 8:39 UTC (permalink / raw) To: Jinjie Ruan Cc: oleg, arnd, afd, akpm, linus.walleij, eric.devolder, robh, linux-arm-kernel, linux-kernel On Mon, Jun 03, 2024 at 03:37:51PM +0800, Jinjie Ruan wrote: > Currently, kprobe on ARM32 can not use the '$argx' syntax available on > other architecture. So implement regs_get_kernel_argument() and add > HAVE_FUNCTION_ARG_ACCESS_API support. This may work in the simple case, but it just doesn't work in the general case, where a function accepts 64-bit arguments. For example, for EABI and a function taking a 64-bit argument followed by a 32-bit argument: R0/R1 = 64-bit argument R2 = 32-bit argument Now consider 32-bit argument followed by 64-bit argument: R0 = 32-bit argument R1 = unused R2/R3 = 64-bit argument Note that the mapping isn't argN = RN. Also, given that "unsigned long" is 32-bit on 32-bit Arm, one can't return a 64-bit argument through this interface. Even if one typed the function as u64, it still wouldn't work because the caller assigns the return value to an unsigned long. This seems to be an issue throughout the kernel tracing - it isn't written to support 64-bit arguments on 32-bit architectures. See, for example, fetch_store_raw(), where the unsigned long gets cast to a u64. It'll still only have 32-bits of significant value. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support 2024-06-03 8:39 ` Russell King (Oracle) @ 2024-06-04 1:36 ` Jinjie Ruan 2024-06-04 8:14 ` Russell King (Oracle) 0 siblings, 1 reply; 5+ messages in thread From: Jinjie Ruan @ 2024-06-04 1:36 UTC (permalink / raw) To: Russell King (Oracle) Cc: oleg, arnd, afd, akpm, linus.walleij, eric.devolder, robh, linux-arm-kernel, linux-kernel On 2024/6/3 16:39, Russell King (Oracle) wrote: > On Mon, Jun 03, 2024 at 03:37:51PM +0800, Jinjie Ruan wrote: >> Currently, kprobe on ARM32 can not use the '$argx' syntax available on >> other architecture. So implement regs_get_kernel_argument() and add >> HAVE_FUNCTION_ARG_ACCESS_API support. > > This may work in the simple case, but it just doesn't work in the > general case, where a function accepts 64-bit arguments. For example, > for EABI and a function taking a 64-bit argument followed by a 32-bit > argument: > > R0/R1 = 64-bit argument > R2 = 32-bit argument > > Now consider 32-bit argument followed by 64-bit argument: > > R0 = 32-bit argument > R1 = unused > R2/R3 = 64-bit argument I agree with you, the current implementation considers a very simple case, where all parameters are 32-bit. From "Procedure Call Standard for the Arm® Architecture", the "6.1.1.1 Handling values larger than 32 bits" describes it this way: A double-word sized type is passed in two consecutive registers (e.g., r0 and r1, or r2 and r3). > > Note that the mapping isn't argN = RN. > > Also, given that "unsigned long" is 32-bit on 32-bit Arm, one can't > return a 64-bit argument through this interface. Even if one typed > the function as u64, it still wouldn't work because the caller > assigns the return value to an unsigned long. This seems to be an > issue throughout the kernel tracing - it isn't written to support How about updating this interface to solve this problem? Let regs_get_kernel_argument() return u64. > 64-bit arguments on 32-bit architectures. See, for example, > fetch_store_raw(), where the unsigned long gets cast to a u64. > It'll still only have 32-bits of significant value. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support 2024-06-04 1:36 ` Jinjie Ruan @ 2024-06-04 8:14 ` Russell King (Oracle) 2024-06-04 8:23 ` Jinjie Ruan 0 siblings, 1 reply; 5+ messages in thread From: Russell King (Oracle) @ 2024-06-04 8:14 UTC (permalink / raw) To: Jinjie Ruan Cc: oleg, arnd, afd, akpm, linus.walleij, eric.devolder, robh, linux-arm-kernel, linux-kernel On Tue, Jun 04, 2024 at 09:36:04AM +0800, Jinjie Ruan wrote: > On 2024/6/3 16:39, Russell King (Oracle) wrote: > > On Mon, Jun 03, 2024 at 03:37:51PM +0800, Jinjie Ruan wrote: > >> Currently, kprobe on ARM32 can not use the '$argx' syntax available on > >> other architecture. So implement regs_get_kernel_argument() and add > >> HAVE_FUNCTION_ARG_ACCESS_API support. > > > > This may work in the simple case, but it just doesn't work in the > > general case, where a function accepts 64-bit arguments. For example, > > for EABI and a function taking a 64-bit argument followed by a 32-bit > > argument: > > > > R0/R1 = 64-bit argument > > R2 = 32-bit argument > > > > Now consider 32-bit argument followed by 64-bit argument: > > > > R0 = 32-bit argument > > R1 = unused > > R2/R3 = 64-bit argument > > I agree with you, the current implementation considers a very simple > case, where all parameters are 32-bit. > > From "Procedure Call Standard for the Arm® Architecture", the > "6.1.1.1 Handling values larger than 32 bits" describes it this way: > > A double-word sized type is passed in two consecutive registers (e.g., > r0 and r1, or r2 and r3). > > > > > Note that the mapping isn't argN = RN. > > > > Also, given that "unsigned long" is 32-bit on 32-bit Arm, one can't > > return a 64-bit argument through this interface. Even if one typed > > the function as u64, it still wouldn't work because the caller > > assigns the return value to an unsigned long. This seems to be an > > issue throughout the kernel tracing - it isn't written to support > > How about updating this interface to solve this problem? Let > regs_get_kernel_argument() return u64. That doesn't solve the first problem. The issue is that once we enable this, it becomes userspace ABI, and any changes to it then become regressions. So no, I'm not going to have it enabled in mainline in a half-broken state. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support 2024-06-04 8:14 ` Russell King (Oracle) @ 2024-06-04 8:23 ` Jinjie Ruan 0 siblings, 0 replies; 5+ messages in thread From: Jinjie Ruan @ 2024-06-04 8:23 UTC (permalink / raw) To: Russell King (Oracle) Cc: oleg, arnd, afd, akpm, linus.walleij, eric.devolder, robh, linux-arm-kernel, linux-kernel On 2024/6/4 16:14, Russell King (Oracle) wrote: > On Tue, Jun 04, 2024 at 09:36:04AM +0800, Jinjie Ruan wrote: >> On 2024/6/3 16:39, Russell King (Oracle) wrote: >>> On Mon, Jun 03, 2024 at 03:37:51PM +0800, Jinjie Ruan wrote: >>>> Currently, kprobe on ARM32 can not use the '$argx' syntax available on >>>> other architecture. So implement regs_get_kernel_argument() and add >>>> HAVE_FUNCTION_ARG_ACCESS_API support. >>> >>> This may work in the simple case, but it just doesn't work in the >>> general case, where a function accepts 64-bit arguments. For example, >>> for EABI and a function taking a 64-bit argument followed by a 32-bit >>> argument: >>> >>> R0/R1 = 64-bit argument >>> R2 = 32-bit argument >>> >>> Now consider 32-bit argument followed by 64-bit argument: >>> >>> R0 = 32-bit argument >>> R1 = unused >>> R2/R3 = 64-bit argument >> >> I agree with you, the current implementation considers a very simple >> case, where all parameters are 32-bit. >> >> From "Procedure Call Standard for the Arm® Architecture", the >> "6.1.1.1 Handling values larger than 32 bits" describes it this way: >> >> A double-word sized type is passed in two consecutive registers (e.g., >> r0 and r1, or r2 and r3). >> >>> >>> Note that the mapping isn't argN = RN. >>> >>> Also, given that "unsigned long" is 32-bit on 32-bit Arm, one can't >>> return a 64-bit argument through this interface. Even if one typed >>> the function as u64, it still wouldn't work because the caller >>> assigns the return value to an unsigned long. This seems to be an >>> issue throughout the kernel tracing - it isn't written to support >> >> How about updating this interface to solve this problem? Let >> regs_get_kernel_argument() return u64. > > That doesn't solve the first problem. The issue is that once we enable > this, it becomes userspace ABI, and any changes to it then become > regressions. Thank you! I understand what you mean. For the existing "$arg" API interface, ARM32 cannot determine whether an input parameter of the instrumentation function is a 32-bit or 64-bit parameter, and there is no way to control the user not to instrument the function with complex parameters, so this config cannot be enabled. > > So no, I'm not going to have it enabled in mainline in a half-broken > state. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-04 8:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-03 7:37 [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support Jinjie Ruan 2024-06-03 8:39 ` Russell King (Oracle) 2024-06-04 1:36 ` Jinjie Ruan 2024-06-04 8:14 ` Russell King (Oracle) 2024-06-04 8:23 ` Jinjie Ruan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).