Re: [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: oleg@redhat.com, arnd@arndb.de, afd@ti.com,
	akpm@linux-foundation.org, linus.walleij@linaro.org,
	eric.devolder@oracle.com, robh@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support
Date: Tue, 4 Jun 2024 09:14:26 +0100	[thread overview]
Message-ID: <Zl7M4rjQbN2c9Bje@shell.armlinux.org.uk> (raw)
In-Reply-To: <3089651e-d8eb-f193-5eac-db925000dce9@huawei.com>

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