Re: [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev

[Madhavan Srinivasan <maddy@linux.ibm.com>]

On 8/9/24 10:24 AM, Christophe Leroy wrote:
>
>
> Le 09/08/2024 à 06:25, Madhavan Srinivasan a écrit :
>>
>> On 8/6/24 12:24 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 05/08/2024 à 10:30, Madhavan Srinivasan a écrit :
>>>> Currently exec-target.c file is linked as static and this
>>>> post a requirement to install libc dev package to build.
>>>> Without it, build-breaks when compiling selftest/powerpc/benchmark.
>>>>
>>>>    CC       exec_target
>>>> /usr/bin/ld: cannot find -lc: No such file or directory
>>>> collect2: error: ld returned 1 exit status
>>>>
>>>> exec_target.c is using "syscall" library function which
>>>> could be replaced with a inline assembly and the same is
>>>> proposed as a fix here.
>>>>
>>>> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
>>>> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>>>> ---
>>>>   tools/testing/selftests/powerpc/benchmarks/Makefile    | 2 +-
>>>>   .../testing/selftests/powerpc/benchmarks/exec_target.c | 10 
>>>> ++++++++--
>>>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/powerpc/benchmarks/Makefile 
>>>> b/tools/testing/selftests/powerpc/benchmarks/Makefile
>>>> index 1321922038d0..ca4483c238b9 100644
>>>> --- a/tools/testing/selftests/powerpc/benchmarks/Makefile
>>>> +++ b/tools/testing/selftests/powerpc/benchmarks/Makefile
>>>> @@ -18,4 +18,4 @@ $(OUTPUT)/context_switch: LDLIBS += -lpthread
>>>>     $(OUTPUT)/fork: LDLIBS += -lpthread
>>>>   -$(OUTPUT)/exec_target: CFLAGS += -static -nostartfiles
>>>> +$(OUTPUT)/exec_target: CFLAGS += -nostartfiles
>>>> diff --git 
>>>> a/tools/testing/selftests/powerpc/benchmarks/exec_target.c 
>>>> b/tools/testing/selftests/powerpc/benchmarks/exec_target.c
>>>> index c14b0fc1edde..20027a23b594 100644
>>>> --- a/tools/testing/selftests/powerpc/benchmarks/exec_target.c
>>>> +++ b/tools/testing/selftests/powerpc/benchmarks/exec_target.c
>>>> @@ -7,10 +7,16 @@
>>>>    */
>>>>     #define _GNU_SOURCE
>>>> -#include <unistd.h>
>>>>   #include <sys/syscall.h>
>>>>     void _start(void)
>>>>   {
>>>> -    syscall(SYS_exit, 0);
>>>> +    asm volatile (
>>>> +        "li %%r0, %[sys_exit];"
>>>> +        "li %%r3, 0;"
>>>> +        "sc;"
>>>> +        :
>>>> +        : [sys_exit] "i" (SYS_exit)
>>>> +        : "r0", "r3"
>>>> +    );
>>>
>>> That looks ok because SYS_exit() is not supposed to return, but in 
>>> the general case you should take a lot more precautions regarding 
>>> which registers get clobbered when using sc.
>>>
>>> Maybe it is worth a comment.
>>
>>
>> ok sure and something like this will help?
>>
>>
>> +        : "r0", "r3" //clobber registers, r0 - syscall number, r3 - 
>> exit value
>>
>
> Not really.
>
> sc will clobber r0 and r3-r12, also SO bit in CR.
>
> Here the reason why you have no problem with that is that SYS_exit 
> never returns. At the end, even your "r0" and "r3" clobber are 
> unnecessary because of that.

ah nice. thanks for the details.
I will add comment and post a v2 soon.

Maddy


>
> Christophe