* [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev
@ 2024-08-05 8:30 Madhavan Srinivasan
2024-08-06 6:54 ` Christophe Leroy
0 siblings, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2024-08-05 8:30 UTC (permalink / raw)
To: mpe, npiggin, christophe.leroy, shuah
Cc: Madhavan Srinivasan, linuxppc-dev, linux-kselftest
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"
+ );
}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev
2024-08-05 8:30 [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev Madhavan Srinivasan
@ 2024-08-06 6:54 ` Christophe Leroy
2024-08-09 4:25 ` Madhavan Srinivasan
2024-08-12 0:21 ` Michael Ellerman
0 siblings, 2 replies; 6+ messages in thread
From: Christophe Leroy @ 2024-08-06 6:54 UTC (permalink / raw)
To: Madhavan Srinivasan, mpe, npiggin, shuah; +Cc: linuxppc-dev, linux-kselftest
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.
Christophe
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev
2024-08-06 6:54 ` Christophe Leroy
@ 2024-08-09 4:25 ` Madhavan Srinivasan
2024-08-09 4:54 ` Christophe Leroy
2024-08-12 0:21 ` Michael Ellerman
1 sibling, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2024-08-09 4:25 UTC (permalink / raw)
To: Christophe Leroy, mpe, npiggin, shuah; +Cc: linuxppc-dev, linux-kselftest
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
Maddy
>
> Christophe
>
>> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev
2024-08-09 4:25 ` Madhavan Srinivasan
@ 2024-08-09 4:54 ` Christophe Leroy
2024-08-09 5:32 ` Madhavan Srinivasan
0 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2024-08-09 4:54 UTC (permalink / raw)
To: Madhavan Srinivasan, mpe, npiggin, shuah; +Cc: linuxppc-dev, linux-kselftest
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.
Christophe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev
2024-08-09 4:54 ` Christophe Leroy
@ 2024-08-09 5:32 ` Madhavan Srinivasan
0 siblings, 0 replies; 6+ messages in thread
From: Madhavan Srinivasan @ 2024-08-09 5:32 UTC (permalink / raw)
To: Christophe Leroy, mpe, npiggin, shuah; +Cc: linuxppc-dev, linux-kselftest
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev
2024-08-06 6:54 ` Christophe Leroy
2024-08-09 4:25 ` Madhavan Srinivasan
@ 2024-08-12 0:21 ` Michael Ellerman
1 sibling, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2024-08-12 0:21 UTC (permalink / raw)
To: Christophe Leroy, Madhavan Srinivasan, npiggin, shuah
Cc: linuxppc-dev, linux-kselftest
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> 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/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.
That's my fault for just blurting out that diff on slack without
thinking about it too hard.
We should probably just add the proper clobbers, in case anyone copies
it in future. Which should be:
asm volatile (
"li %%r0, %[sys_exit];"
"li %%r3, 0;"
"sc;"
:
: [sys_exit] "i" (SYS_exit)
: "r0", "r3", "r4", "r5", "r6", "r7", "r8", "r9", "r10",
"r11", "r12", "r13", "cr0", "ctr", "xer", "memory"
);
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-12 0:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-05 8:30 [PATCH] selftest/powerpc/benchmark: remove requirement libc-dev Madhavan Srinivasan
2024-08-06 6:54 ` Christophe Leroy
2024-08-09 4:25 ` Madhavan Srinivasan
2024-08-09 4:54 ` Christophe Leroy
2024-08-09 5:32 ` Madhavan Srinivasan
2024-08-12 0:21 ` Michael Ellerman
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).