Received-SPF: pass (google.com: domain of linux-kernel+bounces-200244-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <a6e091e0-4825-fcad-41eb-0bdd96a32bde@huawei.com>
Date: Tue, 4 Jun 2024 16:23:23 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.0
Subject: Re: [PATCH] ARM: Add HAVE_FUNCTION_ARG_ACCESS_API support
Content-Language: en-US
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
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>
References: <20240603073751.2282024-1-ruanjinjie@huawei.com>
 <Zl2BNCXzKRG+eTDv@shell.armlinux.org.uk>
 <3089651e-d8eb-f193-5eac-db925000dce9@huawei.com>
 <Zl7M4rjQbN2c9Bje@shell.armlinux.org.uk>
From: Jinjie Ruan <ruanjinjie@huawei.com>
In-Reply-To: <Zl7M4rjQbN2c9Bje@shell.armlinux.org.uk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit


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.
>