Changes since v3:
1) Rebased onto Linux 6.10-rc6+.
2) Added Muhammad's acks for the series.
Cover letter for v3:
Hi,
Dave Hansen, Muhammad Usama Anjum, here is the combined series that we discussed yesterday [1].
As I mentioned then, this is a bit intrusive--but no more than necessary, IMHO. Specifically, it moves some clang-un-inlineable things out to "pure" assembly code files.
I've tested this by building with clang, then running each binary on my x86_64 test system with today's 6.10-rc1, and comparing the console and dmesg output to a gcc-based build without these patches applied. Aside from timestamps and virtual addresses, it looks identical.
Earlier cover letter:
Just a bunch of build and warnings fixes that show up when building with clang. Some of these depend on each other, so I'm sending them as a series.
Changes since v2:
1) Dropped my test_FISTTP.c patch, and picked up Muhammad's fix instead, seeing as how that was posted first.
2) Updated patch descriptions to reflect that Valentin Obst's build fix for LLVM [1] has already been merged into Linux main.
3) Minor wording and typo corrections in the commit logs throughout.
Changes since the first version: 1) Rebased onto Linux 6.10-rc1
Enjoy!
[1] https://lore.kernel.org/44428518-4d21-4de7-8587-04eceefb330d@nvidia.com
thanks, John Hubbard
John Hubbard (6): selftests/x86: fix Makefile dependencies to work with clang selftests/x86: build fsgsbase_restore.c with clang selftests/x86: build sysret_rip.c with clang selftests/x86: avoid -no-pie warnings from clang during compilation selftests/x86: remove (or use) unused variables and functions selftests/x86: fix printk warnings reported by clang
Muhammad Usama Anjum (1): selftests: x86: test_FISTTP: use fisttps instead of ambiguous fisttp
tools/testing/selftests/x86/Makefile | 31 +++++++++++++++---- tools/testing/selftests/x86/amx.c | 16 ---------- .../testing/selftests/x86/clang_helpers_32.S | 11 +++++++ .../testing/selftests/x86/clang_helpers_64.S | 28 +++++++++++++++++ tools/testing/selftests/x86/fsgsbase.c | 6 ---- .../testing/selftests/x86/fsgsbase_restore.c | 11 +++---- tools/testing/selftests/x86/sigreturn.c | 2 +- .../testing/selftests/x86/syscall_arg_fault.c | 1 - tools/testing/selftests/x86/sysret_rip.c | 20 ++++-------- tools/testing/selftests/x86/test_FISTTP.c | 8 ++--- tools/testing/selftests/x86/test_vsyscall.c | 15 +++------ tools/testing/selftests/x86/vdso_restorer.c | 2 ++ 12 files changed, 87 insertions(+), 64 deletions(-) create mode 100644 tools/testing/selftests/x86/clang_helpers_32.S create mode 100644 tools/testing/selftests/x86/clang_helpers_64.S
base-commit: 795c58e4c7fc6163d8fb9f2baa86cfe898fa4b19
When building with clang, via:
make LLVM=1 -C tools/testing/selftests
...the following build failure occurs in selftests/x86:
clang: error: cannot specify -o when generating multiple output files
This happens because, although gcc doesn't complain if you invoke it like this:
gcc file1.c header2.h
...clang won't accept that form--it rejects the .h file(s). Also, the above approach is inaccurate anyway, because file.c includes header2.h in this case, and the inclusion of header2.h on the invocation is an artifact of the Makefile's desire to maintain dependencies.
In Makefiles of this type, a better way to do it is to use Makefile dependencies to trigger the appropriate incremental rebuilds, and separately use file lists (see EXTRA_FILES in this commit) to track what to pass to the compiler.
This commit splits those concepts up, by setting up both EXTRA_FILES and the Makefile dependencies with a single call to the new Makefile function extra-files.
That fixes the build failure, while still providing the correct dependencies in all cases.
Acked-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/x86/Makefile | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 0b872c0a42d2..c1269466e0f8 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -73,10 +73,10 @@ all_64: $(BINARIES_64) EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
$(BINARIES_32): $(OUTPUT)/%_32: %.c helpers.h - $(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm + $(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $< $(EXTRA_FILES) -lrt -ldl -lm
$(BINARIES_64): $(OUTPUT)/%_64: %.c helpers.h - $(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl + $(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $< $(EXTRA_FILES) -lrt -ldl
# x86_64 users should be encouraged to install 32-bit libraries ifeq ($(CAN_BUILD_I386)$(CAN_BUILD_X86_64),01) @@ -100,10 +100,19 @@ warn_32bit_failure: exit 0; endif
-# Some tests have additional dependencies. -$(OUTPUT)/sysret_ss_attrs_64: thunks.S -$(OUTPUT)/ptrace_syscall_32: raw_syscall_helper_32.S -$(OUTPUT)/test_syscall_vdso_32: thunks_32.S +# Add an additional file to the source file list for a given target, and also +# add a Makefile dependency on that same file. However, do these separately, so +# that the compiler invocation ("$(CC) file1.c file2.S") is not combined with +# the dependencies ("header3.h"), because clang, unlike gcc, will not accept +# header files as an input to the compiler invocation. +define extra-files +$(OUTPUT)/$(1): EXTRA_FILES := $(2) +$(OUTPUT)/$(1): $(2) +endef + +$(eval $(call extra-files,sysret_ss_attrs_64,thunks.S)) +$(eval $(call extra-files,ptrace_syscall_32,raw_syscall_helper_32.S)) +$(eval $(call extra-files,test_syscall_vdso_32,thunks_32.S))
# check_initial_reg_state is special: it needs a custom entry, and it # needs to be static so that its interpreter doesn't destroy its initial
From: Muhammad Usama Anjum usama.anjum@collabora.com
Use fisttps instead of fisttp to specify correctly that the output variable is of size short.
test_FISTTP.c:28:3: error: ambiguous instructions require an explicit suffix (could be 'fisttps', or 'fisttpl') 28 | " fisttp res16""\n" | ^ <inline asm>:3:2: note: instantiated into assembly here 3 | fisttp res16 | ^
...followed by three more cases of the same warning for other lines.
[jh: removed a bit of duplication from the warnings report, above, and fixed a typo in the title]
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/x86/test_FISTTP.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/x86/test_FISTTP.c b/tools/testing/selftests/x86/test_FISTTP.c index 09789c0ce3e9..b9ae9d8cebcb 100644 --- a/tools/testing/selftests/x86/test_FISTTP.c +++ b/tools/testing/selftests/x86/test_FISTTP.c @@ -25,7 +25,7 @@ int test(void) feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW); asm volatile ("\n" " fld1""\n" - " fisttp res16""\n" + " fisttps res16""\n" " fld1""\n" " fisttpl res32""\n" " fld1""\n" @@ -45,7 +45,7 @@ int test(void) feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW); asm volatile ("\n" " fldpi""\n" - " fisttp res16""\n" + " fisttps res16""\n" " fldpi""\n" " fisttpl res32""\n" " fldpi""\n" @@ -66,7 +66,7 @@ int test(void) asm volatile ("\n" " fldpi""\n" " fchs""\n" - " fisttp res16""\n" + " fisttps res16""\n" " fldpi""\n" " fchs""\n" " fisttpl res32""\n" @@ -88,7 +88,7 @@ int test(void) feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW); asm volatile ("\n" " fldln2""\n" - " fisttp res16""\n" + " fisttps res16""\n" " fldln2""\n" " fisttpl res32""\n" " fldln2""\n"
When building with clang, via:
make LLVM=1 -C tools/testing/selftests
Fix this by moving the inline asm to "pure" assembly, in two new files: clang_helpers_32.S, clang_helpers_64.S.
As a bonus, the pure asm avoids the need for ifdefs, and is now very simple and easy on the eyes.
Acked-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/x86/Makefile | 2 ++ tools/testing/selftests/x86/clang_helpers_32.S | 11 +++++++++++ tools/testing/selftests/x86/clang_helpers_64.S | 12 ++++++++++++ tools/testing/selftests/x86/fsgsbase_restore.c | 11 +++++------ 4 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 tools/testing/selftests/x86/clang_helpers_32.S create mode 100644 tools/testing/selftests/x86/clang_helpers_64.S
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index c1269466e0f8..99bc2ef84f5a 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -113,6 +113,8 @@ endef $(eval $(call extra-files,sysret_ss_attrs_64,thunks.S)) $(eval $(call extra-files,ptrace_syscall_32,raw_syscall_helper_32.S)) $(eval $(call extra-files,test_syscall_vdso_32,thunks_32.S)) +$(eval $(call extra-files,fsgsbase_restore_64,clang_helpers_64.S)) +$(eval $(call extra-files,fsgsbase_restore_32,clang_helpers_32.S))
# check_initial_reg_state is special: it needs a custom entry, and it # needs to be static so that its interpreter doesn't destroy its initial diff --git a/tools/testing/selftests/x86/clang_helpers_32.S b/tools/testing/selftests/x86/clang_helpers_32.S new file mode 100644 index 000000000000..dc16271bac70 --- /dev/null +++ b/tools/testing/selftests/x86/clang_helpers_32.S @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * 32-bit assembly helpers for asm operations that lack support in both gcc and + * clang. For example, clang asm does not support segment prefixes. + */ +.global dereference_seg_base +dereference_seg_base: + mov %fs:(0), %eax + ret + +.section .note.GNU-stack,"",%progbits diff --git a/tools/testing/selftests/x86/clang_helpers_64.S b/tools/testing/selftests/x86/clang_helpers_64.S new file mode 100644 index 000000000000..0d81c71cad97 --- /dev/null +++ b/tools/testing/selftests/x86/clang_helpers_64.S @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * 64-bit assembly helpers for asm operations that lack support in both gcc and + * clang. For example, clang asm does not support segment prefixes. + */ +.global dereference_seg_base + +dereference_seg_base: + mov %gs:(0), %rax + ret + +.section .note.GNU-stack,"",%progbits diff --git a/tools/testing/selftests/x86/fsgsbase_restore.c b/tools/testing/selftests/x86/fsgsbase_restore.c index 6fffadc51579..224058c1e4b2 100644 --- a/tools/testing/selftests/x86/fsgsbase_restore.c +++ b/tools/testing/selftests/x86/fsgsbase_restore.c @@ -39,12 +39,11 @@ # define SEG "%fs" #endif
-static unsigned int dereference_seg_base(void) -{ - int ret; - asm volatile ("mov %" SEG ":(0), %0" : "=rm" (ret)); - return ret; -} +/* + * Defined in clang_helpers_[32|64].S, because unlike gcc, clang inline asm does + * not support segmentation prefixes. + */ +unsigned int dereference_seg_base(void);
static void init_seg(void) {
When building with clang, via:
make LLVM=1 -C tools/testing/selftests
...the build fails because clang's inline asm doesn't support all of the features that are used in the asm() snippet in sysret_rip.c.
Fix this by moving the asm code into the clang_helpers_64.S file, where it can be built with the assembler's full set of features.
Acked-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/x86/Makefile | 1 + .../testing/selftests/x86/clang_helpers_64.S | 16 +++++++++++++++ tools/testing/selftests/x86/sysret_rip.c | 20 ++++++------------- 3 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 99bc2ef84f5a..d0bb32bd5538 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -115,6 +115,7 @@ $(eval $(call extra-files,ptrace_syscall_32,raw_syscall_helper_32.S)) $(eval $(call extra-files,test_syscall_vdso_32,thunks_32.S)) $(eval $(call extra-files,fsgsbase_restore_64,clang_helpers_64.S)) $(eval $(call extra-files,fsgsbase_restore_32,clang_helpers_32.S)) +$(eval $(call extra-files,sysret_rip_64,clang_helpers_64.S))
# check_initial_reg_state is special: it needs a custom entry, and it # needs to be static so that its interpreter doesn't destroy its initial diff --git a/tools/testing/selftests/x86/clang_helpers_64.S b/tools/testing/selftests/x86/clang_helpers_64.S index 0d81c71cad97..185a69dbf39c 100644 --- a/tools/testing/selftests/x86/clang_helpers_64.S +++ b/tools/testing/selftests/x86/clang_helpers_64.S @@ -9,4 +9,20 @@ dereference_seg_base: mov %gs:(0), %rax ret
+.global test_page +.global test_syscall_insn + +.pushsection ".text", "ax" +.balign 4096 +test_page: .globl test_page + .fill 4094,1,0xcc + +test_syscall_insn: + syscall + +.ifne . - test_page - 4096 + .error "test page is not one page long" +.endif +.popsection + .section .note.GNU-stack,"",%progbits diff --git a/tools/testing/selftests/x86/sysret_rip.c b/tools/testing/selftests/x86/sysret_rip.c index 84d74be1d902..b30de9aaa6d4 100644 --- a/tools/testing/selftests/x86/sysret_rip.c +++ b/tools/testing/selftests/x86/sysret_rip.c @@ -22,21 +22,13 @@ #include <sys/mman.h> #include <assert.h>
- -asm ( - ".pushsection ".text", "ax"\n\t" - ".balign 4096\n\t" - "test_page: .globl test_page\n\t" - ".fill 4094,1,0xcc\n\t" - "test_syscall_insn:\n\t" - "syscall\n\t" - ".ifne . - test_page - 4096\n\t" - ".error "test page is not one page long"\n\t" - ".endif\n\t" - ".popsection" - ); - +/* + * These items are in clang_helpers_64.S, in order to avoid clang inline asm + * limitations: + */ +void test_syscall_ins(void); extern const char test_page[]; + static void const *current_test_page_addr = test_page;
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
When building with clang, via:
make LLVM=1 -C tools/testing/selftests
...clang warns that -no-pie is "unused during compilation".
This occurs because clang only wants to see -no-pie during linking. Here, we don't have a separate linking stage, so a compiler warning is unavoidable without (wastefully) restructuring the Makefile.
Avoid the warning by simply disabling that warning, for clang builds.
Acked-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/x86/Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index d0bb32bd5538..5c8757a25998 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -40,6 +40,13 @@ CFLAGS := -O2 -g -std=gnu99 -pthread -Wall $(KHDR_INCLUDES) # call32_from_64 in thunks.S uses absolute addresses. ifeq ($(CAN_BUILD_WITH_NOPIE),1) CFLAGS += -no-pie + +ifneq ($(LLVM),) +# clang only wants to see -no-pie during linking. Here, we don't have a separate +# linking stage, so a compiler warning is unavoidable without (wastefully) +# restructuring the Makefile. Avoid this by simply disabling that warning. +CFLAGS += -Wno-unused-command-line-argument +endif endif
define gen-target-rule-32
When building with clang, via:
make LLVM=1 -C tools/testing/selftests
...quite a few functions are variables are generating "unused" warnings. Fix the warnings by deleting the unused items.
One item, the "nerrs" variable in vsdo_restorer.c's main(), is unused but probably wants to be returned from main(), as a non-zero result. That result is also unused right now, so another option would be to delete it entirely, but this way, main() also gets fixed. It was missing a return value.
Acked-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/x86/amx.c | 16 ---------------- tools/testing/selftests/x86/fsgsbase.c | 6 ------ tools/testing/selftests/x86/syscall_arg_fault.c | 1 - tools/testing/selftests/x86/test_vsyscall.c | 5 ----- tools/testing/selftests/x86/vdso_restorer.c | 2 ++ 5 files changed, 2 insertions(+), 28 deletions(-)
diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c index 95aad6d8849b..1fdf35a4d7f6 100644 --- a/tools/testing/selftests/x86/amx.c +++ b/tools/testing/selftests/x86/amx.c @@ -39,16 +39,6 @@ struct xsave_buffer { }; };
-static inline uint64_t xgetbv(uint32_t index) -{ - uint32_t eax, edx; - - asm volatile("xgetbv;" - : "=a" (eax), "=d" (edx) - : "c" (index)); - return eax + ((uint64_t)edx << 32); -} - static inline void xsave(struct xsave_buffer *xbuf, uint64_t rfbm) { uint32_t rfbm_lo = rfbm; @@ -164,12 +154,6 @@ static inline void clear_xstate_header(struct xsave_buffer *buffer) memset(&buffer->header, 0, sizeof(buffer->header)); }
-static inline uint64_t get_xstatebv(struct xsave_buffer *buffer) -{ - /* XSTATE_BV is at the beginning of the header: */ - return *(uint64_t *)&buffer->header; -} - static inline void set_xstatebv(struct xsave_buffer *buffer, uint64_t bv) { /* XSTATE_BV is at the beginning of the header: */ diff --git a/tools/testing/selftests/x86/fsgsbase.c b/tools/testing/selftests/x86/fsgsbase.c index 8c780cce941d..50cf32de6313 100644 --- a/tools/testing/selftests/x86/fsgsbase.c +++ b/tools/testing/selftests/x86/fsgsbase.c @@ -109,11 +109,6 @@ static inline void wrgsbase(unsigned long gsbase) asm volatile("wrgsbase %0" :: "r" (gsbase) : "memory"); }
-static inline void wrfsbase(unsigned long fsbase) -{ - asm volatile("wrfsbase %0" :: "r" (fsbase) : "memory"); -} - enum which_base { FS, GS };
static unsigned long read_base(enum which_base which) @@ -212,7 +207,6 @@ static void mov_0_gs(unsigned long initial_base, bool schedule) }
static volatile unsigned long remote_base; -static volatile bool remote_hard_zero; static volatile unsigned int ftx;
/* diff --git a/tools/testing/selftests/x86/syscall_arg_fault.c b/tools/testing/selftests/x86/syscall_arg_fault.c index 461fa41a4d02..48ab065a76f9 100644 --- a/tools/testing/selftests/x86/syscall_arg_fault.c +++ b/tools/testing/selftests/x86/syscall_arg_fault.c @@ -29,7 +29,6 @@ static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), err(1, "sigaction"); }
-static volatile sig_atomic_t sig_traps; static sigjmp_buf jmpbuf;
static volatile sig_atomic_t n_errs; diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c index d4c8e8d79d38..1c9895cfc660 100644 --- a/tools/testing/selftests/x86/test_vsyscall.c +++ b/tools/testing/selftests/x86/test_vsyscall.c @@ -97,11 +97,6 @@ static inline long sys_gtod(struct timeval *tv, struct timezone *tz) return syscall(SYS_gettimeofday, tv, tz); }
-static inline int sys_clock_gettime(clockid_t id, struct timespec *ts) -{ - return syscall(SYS_clock_gettime, id, ts); -} - static inline long sys_time(time_t *t) { return syscall(SYS_time, t); diff --git a/tools/testing/selftests/x86/vdso_restorer.c b/tools/testing/selftests/x86/vdso_restorer.c index fe99f2434155..ac8d8e1e9805 100644 --- a/tools/testing/selftests/x86/vdso_restorer.c +++ b/tools/testing/selftests/x86/vdso_restorer.c @@ -92,4 +92,6 @@ int main() printf("[FAIL]\t!SA_SIGINFO handler was not called\n"); nerrs++; } + + return nerrs; }
These warnings are all of the form, "the format specified a short (signed or unsigned) int, but the value is a full length int".
Acked-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/x86/sigreturn.c | 2 +- tools/testing/selftests/x86/test_vsyscall.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c index 5d7961a5f7f6..0b75b29f794b 100644 --- a/tools/testing/selftests/x86/sigreturn.c +++ b/tools/testing/selftests/x86/sigreturn.c @@ -487,7 +487,7 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void) greg_t asm_ss = ctx->uc_mcontext.gregs[REG_CX]; if (asm_ss != sig_ss && sig == SIGTRAP) { /* Sanity check failure. */ - printf("[FAIL]\tSIGTRAP: ss = %hx, frame ss = %hx, ax = %llx\n", + printf("[FAIL]\tSIGTRAP: ss = %hx, frame ss = %x, ax = %llx\n", ss, *ssptr(ctx), (unsigned long long)asm_ss); nerrs++; } diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c index 1c9895cfc660..6de11b4df458 100644 --- a/tools/testing/selftests/x86/test_vsyscall.c +++ b/tools/testing/selftests/x86/test_vsyscall.c @@ -247,7 +247,7 @@ static void test_getcpu(int cpu)
if (ret_sys == 0) { if (cpu_sys != cpu) - ksft_print_msg("syscall reported CPU %hu but should be %d\n", + ksft_print_msg("syscall reported CPU %u but should be %d\n", cpu_sys, cpu);
have_node = true; @@ -265,10 +265,10 @@ static void test_getcpu(int cpu)
if (cpu_vdso != cpu || node_vdso != node) { if (cpu_vdso != cpu) - ksft_print_msg("vDSO reported CPU %hu but should be %d\n", + ksft_print_msg("vDSO reported CPU %u but should be %d\n", cpu_vdso, cpu); if (node_vdso != node) - ksft_print_msg("vDSO reported node %hu but should be %hu\n", + ksft_print_msg("vDSO reported node %u but should be %u\n", node_vdso, node); ksft_test_result_fail("Wrong values\n"); } else { @@ -290,10 +290,10 @@ static void test_getcpu(int cpu)
if (cpu_vsys != cpu || node_vsys != node) { if (cpu_vsys != cpu) - ksft_print_msg("vsyscall reported CPU %hu but should be %d\n", + ksft_print_msg("vsyscall reported CPU %u but should be %d\n", cpu_vsys, cpu); if (node_vsys != node) - ksft_print_msg("vsyscall reported node %hu but should be %hu\n", + ksft_print_msg("vsyscall reported node %u but should be %u\n", node_vsys, node); ksft_test_result_fail("Wrong values\n"); } else {
On 7/4/24 01:24, John Hubbard wrote:
Changes since v3:
Rebased onto Linux 6.10-rc6+.
Added Muhammad's acks for the series.
Cover letter for v3:
Hi,
Dave Hansen, Muhammad Usama Anjum, here is the combined series that we discussed yesterday [1].
As I mentioned then, this is a bit intrusive--but no more than necessary, IMHO. Specifically, it moves some clang-un-inlineable things out to "pure" assembly code files.
I've tested this by building with clang, then running each binary on my x86_64 test system with today's 6.10-rc1, and comparing the console and dmesg output to a gcc-based build without these patches applied. Aside from timestamps and virtual addresses, it looks identical.
Earlier cover letter:
Just a bunch of build and warnings fixes that show up when building with clang. Some of these depend on each other, so I'm sending them as a series.
Changes since v2:
Dropped my test_FISTTP.c patch, and picked up Muhammad's fix instead, seeing as how that was posted first.
Updated patch descriptions to reflect that Valentin Obst's build fix for LLVM [1] has already been merged into Linux main.
Minor wording and typo corrections in the commit logs throughout.
Changes since the first version:
- Rebased onto Linux 6.10-rc1
Enjoy!
[1] https://lore.kernel.org/44428518-4d21-4de7-8587-04eceefb330d@nvidia.com
thanks, John Hubbard
John Hubbard (6): selftests/x86: fix Makefile dependencies to work with clang selftests/x86: build fsgsbase_restore.c with clang selftests/x86: build sysret_rip.c with clang selftests/x86: avoid -no-pie warnings from clang during compilation selftests/x86: remove (or use) unused variables and functions selftests/x86: fix printk warnings reported by clang
Muhammad Usama Anjum (1): selftests: x86: test_FISTTP: use fisttps instead of ambiguous fisttp
Usama and John,
I am seeing checkpatch warnings in this series.
v4-3-7 WARNING: externs should be avoided in .c files #210: FILE: tools/testing/selftests/x86/fsgsbase_restore.c:46: +unsigned int dereference_seg_base(void);
ARNING: Consecutive strings are generally better as a single string #156: FILE: tools/testing/selftests/x86/test_FISTTP.c:28: + " fisttps res16""\n"
WARNING: Consecutive strings are generally better as a single string #165: FILE: tools/testing/selftests/x86/test_FISTTP.c:48: + " fisttps res16""\n"
WARNING: Consecutive strings are generally better as a single string #174: FILE: tools/testing/selftests/x86/test_FISTTP.c:69: + " fisttps res16""\n"
WARNING: Consecutive strings are generally better as a single string #183: FILE: tools/testing/selftests/x86/test_FISTTP.c:91: + " fisttps res16""\n"
total: 0 errors, 5 warnings, 32 lines checked
Can you take a look at these and see if they can be fixed. Send me v5 with these fixed - I will pull these in for 6.11-rc1
thanks, -- Shuah
On 7/9/24 1:34 PM, Shuah Khan wrote:
On 7/4/24 01:24, John Hubbard wrote:
...>> Muhammad Usama Anjum (1):
selftests: x86: test_FISTTP: use fisttps instead of ambiguous fisttp
Usama and John,
I am seeing checkpatch warnings in this series.
v4-3-7 WARNING: externs should be avoided in .c files #210: FILE: tools/testing/selftests/x86/fsgsbase_restore.c:46: +unsigned int dereference_seg_base(void);
ARNING: Consecutive strings are generally better as a single string #156: FILE: tools/testing/selftests/x86/test_FISTTP.c:28: + " fisttps res16""\n"
WARNING: Consecutive strings are generally better as a single string #165: FILE: tools/testing/selftests/x86/test_FISTTP.c:48: + " fisttps res16""\n"
WARNING: Consecutive strings are generally better as a single string #174: FILE: tools/testing/selftests/x86/test_FISTTP.c:69: + " fisttps res16""\n"
WARNING: Consecutive strings are generally better as a single string #183: FILE: tools/testing/selftests/x86/test_FISTTP.c:91: + " fisttps res16""\n"
total: 0 errors, 5 warnings, 32 lines checked
Can you take a look at these and see if they can be fixed. Send me v5 with these fixed - I will pull these in for 6.11-rc1
thanks, -- Shuah
Hi Shuah,
These warnings are pre-existing. For such things, it's usually prudent to avoid changing them--or at least, not in the same patch.
I think it's best to submit the patch (and series) as-is. If the x86 team wants the above things cleaned up (which I seriously doubt), they'll let us know.
Yes?
thanks,
On 7/9/24 14:40, John Hubbard wrote:
On 7/9/24 1:34 PM, Shuah Khan wrote:
On 7/4/24 01:24, John Hubbard wrote:
...>> Muhammad Usama Anjum (1):
selftests: x86: test_FISTTP: use fisttps instead of ambiguous fisttp
Usama and John,
I am seeing checkpatch warnings in this series.
v4-3-7 WARNING: externs should be avoided in .c files #210: FILE: tools/testing/selftests/x86/fsgsbase_restore.c:46: +unsigned int dereference_seg_base(void);
This one looked new though.
thanks, -- Shuah
On 7/9/24 14:51, Shuah Khan wrote:
On 7/9/24 14:40, John Hubbard wrote:
On 7/9/24 1:34 PM, Shuah Khan wrote:
On 7/4/24 01:24, John Hubbard wrote:
...>> Muhammad Usama Anjum (1):
selftests: x86: test_FISTTP: use fisttps instead of ambiguous fisttp
Usama and John,
I am seeing checkpatch warnings in this series.
v4-3-7 WARNING: externs should be avoided in .c files #210: FILE: tools/testing/selftests/x86/fsgsbase_restore.c:46: +unsigned int dereference_seg_base(void);
This one looked new though.
Never mind. Looks like there are a few existing warns - pulling these now for Linux 6.11-rc1.
Thank you both for fixing these.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
John Hubbard -
Shuah Khan