Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding

[Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>]

Hi,

On 12/8/2025 11:54 PM, Harshal Dev wrote:
> 
> 
> On 12/8/2025 5:50 PM, Sumit Garg via OP-TEE wrote:
>> On Mon, Dec 08, 2025 at 04:24:17PM +1100, Amirreza Zarrabi wrote:
>>> Hi,
>>>
>>> On 12/5/2025 12:27 AM, Jens Wiklander wrote:
>>>> Hi,
>>>>
>>>> On Thu, Dec 4, 2025 at 11:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>>>>
>>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>>
>>>>> The tee_ioctl_object_invoke_arg structure has padding on some
>>>>> architectures but not on x86-32 and a few others:
>>>>>
>>>>> include/linux/tee.h:474:32: error: padding struct to align 'params' [-Werror=padded]
>>>>>
>>>>> I expect that all current users of this are on architectures that do
>>>>> have implicit padding here (arm64, arm, x86, riscv), so make the padding
>>>>> explicit in order to avoid surprises if this later gets used elsewhere.
>>>>>
>>>>> Fixes: d5b8b0fa1775 ("tee: add TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF")
>>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>>> ---
>>>>> The new interface showed up in 6.18, but I only came across this after
>>>>> that was released. Changing it now is technically an ABI change on
>>>>> architectures with unusual padding rules, so please consider carefully
>>>>> whether we want to do it this way or not.
>>>>>
>>>>> Working around the ABI differences without an ABI change is possible,
>>>>> but adds a lot of complexity for compat handling.
>>>>
>>>> This is currently only used by the recently introduced qcomtee backend
>>>> driver. So it's only used on a few arm64 Qualcomm platforms right now.
>>>>
>>>> I think we should take this patch, but let's hear what others think.
>>
>> Yeah since it's not an ABI issue on arm64 platforms where QTEE runs, so:
>>
>> Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>
>>>>
>>>> Thanks,
>>>> Jens
>>>>
>>>
>>> I agree. We should take this patch. As noted, there are not many
>>> clients relying on it yet, so updating the userspace should
>>> be straightforward.
>>
>> You should rather test without any userspace library update to test it's
>> not an ABI issue. Just for correctness sake, you can update the library
>> too.
>>
> 
> I'll take the time to test it at some point this week both with and without updating
> the library ABI.
> 

Summit, that was the plan from the beginning, that's why I did not add "Reviewed-by:"
in the first place. Thanks Harshal for volunteering.

Best regards,
Amir

> Regards,
> Harshal
> 
>> -Sumit
>>
>> [...]
>