[PATCH] scripts/spdxcheck.py: improve Python 3 compat
When reading lines from a text-mode fd strings are returned. These can not be decoded again into strings, breaking the logic in parser. Just make sure all files are opened in binary mode on Python 3, so the current logic keeps working. This remains compatible with Python 2 and should have no functional change. Signed-off-by: Thomas Weißschuh --- scripts/spdxcheck.py | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py index 839e190bbd7a..8f472f995d70 100755 --- a/scripts/spdxcheck.py +++ b/scripts/spdxcheck.py @@ -250,12 +250,15 @@ if __name__ == '__main__': try: if len(args.path) and args.path[0] == '-': -parser.parse_lines(sys.stdin, args.maxlines, '-') +parser.parse_lines( +# always get the binary fd +getattr(sys.stdin, 'buffer', sys.stdin), +args.maxlines, '-') else: if args.path: for p in args.path: if os.path.isfile(p): -parser.parse_lines(open(p), args.maxlines, p) +parser.parse_lines(open(p, 'rb'), args.maxlines, p) elif os.path.isdir(p): scan_git_subtree(repo.head.reference.commit.tree, p) else: -- 2.18.0
[PATCH net-next 2/5] net/ipv4/sysctl: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/ipv4/sysctl_net_ipv4.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index 162a0a3b6ba5..d7892f34a15b 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -130,7 +130,8 @@ static int ipv4_privileged_ports(struct ctl_table *table, int write, return ret; } -static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low, kgid_t *high) +static void inet_get_ping_group_range_table(const struct ctl_table *table, + kgid_t *low, kgid_t *high) { kgid_t *data = table->data; struct net *net = @@ -145,7 +146,8 @@ static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low } /* Update system visible IP port range */ -static void set_ping_group_range(struct ctl_table *table, kgid_t low, kgid_t high) +static void set_ping_group_range(const struct ctl_table *table, +kgid_t low, kgid_t high) { kgid_t *data = table->data; struct net *net = -- 2.45.1
[PATCH net-next 3/5] net/ipv6/addrconf: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/ipv6/addrconf.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 5c424a0e7232..1e69756d53d9 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -863,7 +863,7 @@ static void addrconf_forward_change(struct net *net, __s32 newf) } } -static int addrconf_fixup_forwarding(struct ctl_table *table, int *p, int newf) +static int addrconf_fixup_forwarding(const struct ctl_table *table, int *p, int newf) { struct net *net; int old; @@ -931,7 +931,7 @@ static void addrconf_linkdown_change(struct net *net, __s32 newf) } } -static int addrconf_fixup_linkdown(struct ctl_table *table, int *p, int newf) +static int addrconf_fixup_linkdown(const struct ctl_table *table, int *p, int newf) { struct net *net; int old; @@ -6378,7 +6378,7 @@ static void addrconf_disable_change(struct net *net, __s32 newf) } } -static int addrconf_disable_ipv6(struct ctl_table *table, int *p, int newf) +static int addrconf_disable_ipv6(const struct ctl_table *table, int *p, int newf) { struct net *net = (struct net *)table->extra2; int old; @@ -6669,7 +6669,7 @@ void addrconf_disable_policy_idev(struct inet6_dev *idev, int val) } static -int addrconf_disable_policy(struct ctl_table *ctl, int *valp, int val) +int addrconf_disable_policy(const struct ctl_table *ctl, int *valp, int val) { struct net *net = (struct net *)ctl->extra2; struct inet6_dev *idev; -- 2.45.1
[PATCH net-next 4/5] net/ipv6/ndisc: constify ctl_table arguments of utility function
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/ipv6/ndisc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index d914b23256ce..254b192c5705 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -1936,7 +1936,7 @@ static struct notifier_block ndisc_netdev_notifier = { }; #ifdef CONFIG_SYSCTL -static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl, +static void ndisc_warn_deprecated_sysctl(const struct ctl_table *ctl, const char *func, const char *dev_name) { static char warncomm[TASK_COMM_LEN]; -- 2.45.1
[PATCH net-next 0/5] net: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. This patch(set) is meant to be applied through your subsystem tree. Or at your preference through the sysctl tree. Motivation == Moving structures containing function pointers into unmodifiable .rodata prevents attackers or bugs from corrupting and diverting those pointers. Also the "struct ctl_table" exposed by the sysctl core were never meant to be mutated by users. For this goal changes to both the sysctl core and "const" qualifiers for various sysctl APIs are necessary. Full Process * Drop ctl_table modifications from the sysctl core ([0], in mainline) * Constify arguments to ctl_table_root::{set_ownership,permissions} ([1], in mainline) * Migrate users of "ctl_table_header::ctl_table_arg" to "const". (in mainline) * Afterwards convert "ctl_table_header::ctl_table_arg" itself to const. (in mainline) * Prepare helpers used to implement proc_handlers throughout the tree to use "const struct ctl_table *". ([2], in progress, this patch) * Afterwards switch over all proc_handlers callbacks to use "const struct ctl_table *" in one commit. ([2], in progress) Only custom handlers will be affected, the big commit avoids a disruptive and messy transition phase. * Switch over the internals of the sysctl core to "const struct ctl_table *" (to be done) * Switch include/linux/sysctl.h to "const struct ctl_table *" (to be done) * Transition instances of "struct ctl_table" through the tree to const (to be done) A work-in-progress view containing all the outlined changes can be found at https://git.sr.ht/~t-8ch/linux sysctl-constfy [0] https://lore.kernel.org/lkml/20240322-sysctl-empty-dir-v2-0-e559cf8ec...@weissschuh.net/ [1] https://lore.kernel.org/lkml/20240315-sysctl-const-ownership-v3-0-b86680eae...@weissschuh.net/ [2] https://lore.kernel.org/lkml/20240423-sysctl-const-handler-v3-0-e0beccb83...@weissschuh.net/ --- Thomas Weißschuh (5): net/neighbour: constify ctl_table arguments of utility function net/ipv4/sysctl: constify ctl_table arguments of utility functions net/ipv6/addrconf: constify ctl_table arguments of utility functions net/ipv6/ndisc: constify ctl_table arguments of utility function ipvs: constify ctl_table arguments of utility functions net/core/neighbour.c | 2 +- net/ipv4/sysctl_net_ipv4.c | 6 -- net/ipv6/addrconf.c| 8 net/ipv6/ndisc.c | 2 +- net/netfilter/ipvs/ip_vs_ctl.c | 7 --- 5 files changed, 14 insertions(+), 11 deletions(-) --- base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 change-id: 20240523-sysctl-const-handler-net-824d4ad5a15a Best regards, -- Thomas Weißschuh
[PATCH net-next 5/5] ipvs: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/netfilter/ipvs/ip_vs_ctl.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index b6d0dcf3a5c3..78a1cc72dc38 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -1924,7 +1924,8 @@ proc_do_sync_ports(struct ctl_table *table, int write, return rc; } -static int ipvs_proc_est_cpumask_set(struct ctl_table *table, void *buffer) +static int ipvs_proc_est_cpumask_set(const struct ctl_table *table, +void *buffer) { struct netns_ipvs *ipvs = table->extra2; cpumask_var_t *valp = table->data; @@ -1962,8 +1963,8 @@ static int ipvs_proc_est_cpumask_set(struct ctl_table *table, void *buffer) return ret; } -static int ipvs_proc_est_cpumask_get(struct ctl_table *table, void *buffer, -size_t size) +static int ipvs_proc_est_cpumask_get(const struct ctl_table *table, +void *buffer, size_t size) { struct netns_ipvs *ipvs = table->extra2; cpumask_var_t *valp = table->data; -- 2.45.1
[PATCH net-next 1/5] net/neighbour: constify ctl_table arguments of utility function
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/core/neighbour.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 45fd88405b6b..277751375b0a 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -3578,7 +3578,7 @@ static void neigh_copy_dflt_parms(struct net *net, struct neigh_parms *p, rcu_read_unlock(); } -static void neigh_proc_update(struct ctl_table *ctl, int write) +static void neigh_proc_update(const struct ctl_table *ctl, int write) { struct net_device *dev = ctl->extra1; struct neigh_parms *p = ctl->extra2; -- 2.45.1
[PATCH v2] modpost: compile constant module information only once
Various information about modules is compiled into the info sections. For that a dedicated .mod.c file is generated by modpost for each module and then linked into the module. However most of the information in the .mod.c is the same for all modules, internal and external. Split the shared information into a dedicated source file that is compiled once and then linked into all modules. This avoids frequent rebuilds for all .mod.c files when using CONFIG_LOCALVERSION_AUTO because the local version ends up in .mod.c through UTS_RELEASE and VERMAGIC_STRING. The modules are still relinked in this case. The code is also easier to maintain as it's now in a proper source file instead of an inline string literal. Signed-off-by: Thomas Weißschuh --- Changes in v2: - Remove RFC status - Incorporate Masahiro's proposals - Rename modinfo.o to .module-common.o - Build a dedicated .module-common.o for external modules - Link to v1: https://lore.kernel.org/r/20240824-modinfo-const-v1-1-485f9c64b...@weissschuh.net --- Masahiro, feel free to add some attribution for yourself when applying. The new appraoch is pleasantly simpler. --- scripts/Makefile.modfinal | 7 +-- scripts/mod/modpost.c | 23 --- scripts/module-common.c | 25 + 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 306a6bb86e4d..6b1b72257b29 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -30,6 +30,9 @@ quiet_cmd_cc_o_c = CC [M] $@ %.mod.o: %.mod.c FORCE $(call if_changed_dep,cc_o_c) +$(extmod_prefix).module-common.o: $(srctree)/scripts/module-common.c FORCE + $(call if_changed_dep,cc_o_c) + quiet_cmd_ld_ko_o = LD [M] $@ cmd_ld_ko_o += \ $(LD) -r $(KBUILD_LDFLAGS) \ @@ -54,13 +57,13 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) # Re-generate module BTFs if either module's .ko or vmlinux changed -%.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE +%.ko: %.o %.mod.o $(extmod_prefix).module-common.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE +$(call if_changed_except,ld_ko_o,vmlinux) ifdef CONFIG_DEBUG_INFO_BTF_MODULES +$(if $(newer-prereqs),$(call cmd,btf_ko)) endif -targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) +targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) $(extmod_prefix).module-common.o # Add FORCE to the prerequisites of a target to force it to be always rebuilt. # --- diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index c8cd5d822bb6..107393a8c48a 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1755,26 +1755,9 @@ static void check_modname_len(struct module *mod) static void add_header(struct buffer *b, struct module *mod) { buf_printf(b, "#include \n"); - /* -* Include build-salt.h after module.h in order to -* inherit the definitions. -*/ - buf_printf(b, "#define INCLUDE_VERMAGIC\n"); - buf_printf(b, "#include \n"); - buf_printf(b, "#include \n"); buf_printf(b, "#include \n"); - buf_printf(b, "#include \n"); buf_printf(b, "#include \n"); buf_printf(b, "\n"); - buf_printf(b, "#ifdef CONFIG_UNWINDER_ORC\n"); - buf_printf(b, "#include \n"); - buf_printf(b, "ORC_HEADER;\n"); - buf_printf(b, "#endif\n"); - buf_printf(b, "\n"); - buf_printf(b, "BUILD_SALT;\n"); - buf_printf(b, "BUILD_LTO_INFO;\n"); - buf_printf(b, "\n"); - buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n"); buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n"); buf_printf(b, "\n"); buf_printf(b, "__visible struct module __this_module\n"); @@ -1792,12 +1775,6 @@ static void add_header(struct buffer *b, struct module *mod) if (!external_module) buf_printf(b, "\nMODULE_INFO(intree, \"Y\");\n"); - buf_printf(b, - "\n" - "#ifdef CONFIG_MITIGATION_RETPOLINE\n" - "MODULE_INFO(retpoline, \"Y\");\n" - "#endif\n"); - if (strstarts(mod->name, "drivers/staging")) buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n"); diff --git a
Re: [PATCH] selftests: kselftest: Use strerror() on nolibc
Hi Shuah, On 2024-09-11 09:36:50+, Shuah Khan wrote: > On 9/10/24 22:42, zhangjiao2 wrote: > > From: zhang jiao > > > > Nolibc gained an implementation of strerror() recently. > > Use it and drop the ifndef. > > > > Signed-off-by: zhang jiao > > --- > > tools/testing/selftests/kselftest.h | 8 > > 1 file changed, 8 deletions(-) > > > > diff --git a/tools/testing/selftests/kselftest.h > > b/tools/testing/selftests/kselftest.h > > index e195ec156859..29fedf609611 100644 > > --- a/tools/testing/selftests/kselftest.h > > +++ b/tools/testing/selftests/kselftest.h > > @@ -373,15 +373,7 @@ static inline __noreturn __printf(1, 2) void > > ksft_exit_fail_msg(const char *msg, > > static inline __noreturn void ksft_exit_fail_perror(const char *msg) > > { > > -#ifndef NOLIBC > > ksft_exit_fail_msg("%s: %s (%d)\n", msg, strerror(errno), errno); > > -#else > > - /* > > -* nolibc doesn't provide strerror() and it seems > > -* inappropriate to add one, just print the errno. > > -*/ > > - ksft_exit_fail_msg("%s: %d)\n", msg, errno); > > -#endif > > } > > static inline __noreturn void ksft_exit_xfail(void) > > Adding nolibc maintainers for review. > > Willy and Thomas, please review. Acked-by: Thomas Weißschuh I did the same for another kselftests function when introducing strerror(). This one was apparently missed or didn't exist yet. Thomas
[PATCH] selftests/nolibc: libc-test: avoid -Wstringop-overflow warnings
Newer versions of glibc annotate the poll() function with __attribute__(access) which triggers a compiler warning inside the testcase poll_fault. Avoid this by using a plain NULL which is enough for the testcase. To avoid potential future warnings also adapt the other EFAULT testcases, except select_fault as NULL is a valid value for its argument. nolibc-test.c: In function ‘run_syscall’: nolibc-test.c:338:62: warning: ‘poll’ writing 8 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=] 338 | do { if (!(cond)) result(llen, SKIPPED); else ret += expect_syserr2(expr, expret, experr1, experr2, llen); } while (0) | ^~~~ nolibc-test.c:341:9: note: in expansion of macro ‘EXPECT_SYSER2’ 341 | EXPECT_SYSER2(cond, expr, expret, experr, 0) | ^ nolibc-test.c:905:47: note: in expansion of macro ‘EXPECT_SYSER’ 905 | CASE_TEST(poll_fault);EXPECT_SYSER(1, poll((void *)1, 1, 0), -1, EFAULT); break; | ^~~~ cc1: note: destination object is likely at address zero In file included from /usr/include/poll.h:1, from nolibc-test.c:33: /usr/include/sys/poll.h:54:12: note: in a call to function ‘poll’ declared with attribute ‘access (write_only, 1, 2)’ 54 | extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout) |^~~~ Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/nolibc-test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index e2b70641a1e7..a0478f8eaee8 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -895,14 +895,14 @@ int run_syscall(int min, int max) CASE_TEST(lseek_0); EXPECT_SYSER(1, lseek(0, 0, SEEK_SET), -1, ESPIPE); break; CASE_TEST(mkdir_root);EXPECT_SYSER(1, mkdir("/", 0755), -1, EEXIST); break; CASE_TEST(mmap_bad); EXPECT_PTRER(1, mmap(NULL, 0, PROT_READ, MAP_PRIVATE, 0, 0), MAP_FAILED, EINVAL); break; - CASE_TEST(munmap_bad);EXPECT_SYSER(1, munmap((void *)1, 0), -1, EINVAL); break; + CASE_TEST(munmap_bad);EXPECT_SYSER(1, munmap(NULL, 0), -1, EINVAL); break; CASE_TEST(mmap_munmap_good); EXPECT_SYSZR(1, test_mmap_munmap()); break; CASE_TEST(open_tty); EXPECT_SYSNE(1, tmp = open("/dev/null", 0), -1); if (tmp != -1) close(tmp); break; CASE_TEST(open_blah); EXPECT_SYSER(1, tmp = open("/proc/self/blah", 0), -1, ENOENT); if (tmp != -1) close(tmp); break; CASE_TEST(pipe); EXPECT_SYSZR(1, test_pipe()); break; CASE_TEST(poll_null); EXPECT_SYSZR(1, poll(NULL, 0, 0)); break; CASE_TEST(poll_stdout); EXPECT_SYSNE(1, ({ struct pollfd fds = { 1, POLLOUT, 0}; poll(&fds, 1, 0); }), -1); break; - CASE_TEST(poll_fault);EXPECT_SYSER(1, poll((void *)1, 1, 0), -1, EFAULT); break; + CASE_TEST(poll_fault);EXPECT_SYSER(1, poll(NULL, 1, 0), -1, EFAULT); break; CASE_TEST(prctl); EXPECT_SYSER(1, prctl(PR_SET_NAME, (unsigned long)NULL, 0, 0, 0), -1, EFAULT); break; CASE_TEST(read_badf); EXPECT_SYSER(1, read(-1, &tmp, 1), -1, EBADF); break; CASE_TEST(rmdir_blah);EXPECT_SYSER(1, rmdir("/blah"), -1, ENOENT); break; @@ -911,7 +911,7 @@ int run_syscall(int min, int max) CASE_TEST(select_stdout); EXPECT_SYSNE(1, ({ fd_set fds; FD_ZERO(&fds); FD_SET(1, &fds); select(2, NULL, &fds, NULL, NULL); }), -1); break; CASE_TEST(select_fault); EXPECT_SYSER(1, select(1, (void *)1, NULL, NULL, 0), -1, EFAULT); break; CASE_TEST(stat_blah); EXPECT_SYSER(1, stat("/proc/self/blah", &stat_buf), -1, ENOENT); break; - CASE_TEST(stat_fault);EXPECT_SYSER(1, stat((void *)1, &stat_buf), -1, EFAULT); break; + CASE_TEST(stat_fault);EXPECT_SYSER(1, stat(NULL, &stat_buf), -1, EFAULT); break; CASE_TEST(stat_timestamps); EXPECT_SYSZR(1, test_stat_timestamps()); break; CASE_TEST(symlink_root); EXPECT_SYSER(1, symlink("/", "/"), -1, EEXIST); break; CASE_TEST(unlink_root); EXPECT_SYSER(1, unlink("/"), -1, EISDIR); break; --- base-commit: f7a6e4791e3d685eddca29b5d16d183ee0407caa change-id: 20230910-nolibc-poll-fault-4152a6836ef8 Best regards, -- Thomas Weißschuh
Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver
Hi Hans, On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote: > Thank you for your new driver and thank you for the quick respin > addressing Barnabás' request to make it a WMI driver. > > The code looks good, so merging this should be a no-brainer, > yet I'm not sure if I should merge this driver as-is, let me > explain. thanks for the encouraging words. > The problem is that I assume that this is based on reverse-engineering? Yes, it is completely reverse-engineered. Essentially I stumbled upon Matthews comment at https://bugzilla.kernel.org/show_bug.cgi?id=204807#c37 and went from there. > We have some mixes experiences with reverse-engineered WMI drivers, > sometimes a feature is not supported yet the wmi_evaluate_method() > call happily succeeds. One example of this causing trouble is: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1797d588af15174d4a4e7159dac8c800538e4f8c There actually are reports of recent, similar mainboards with recent firmware and similar sensor chips that do not support the temperature query. (https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/3 and https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/2) Unfortunately for unknown WMI queries the firmware does not return any value. This ends up as an ACPI integer with value 0 on the driver side. (Which I could not yet find documentation for if that is expected) In the current version of the driver EIO is returned for 0 values which get translated to N/A by lm-sensors. > At a minimum I think your driver should check in its > probe function that > > gigabyte_wmi_query_integer(GIGABYTE_WMI_TEMPERATURE_QUERY, ...) > > actually succeeds on the machine the driver is running on chances > are that Gigabyte has been using the DEADBEEF-2001--00A0-C9062910 > GUID for ages, where as the 0x125 value for GIGABYTE_WMI_TEMPERATURE_QUERY > suggests that this is a pretty new API. Would it be enough to probe all six sensors and check if all return 0? > It would be good if you can see if you can find some DSDT-s for older > gigabyte motherboards attached to various distro's bug-tracking systems > or forum posts and see how those respond to an unknown > gigabyte_wmi_commandtype. Will do. > Another open question to make sure this driver is suitable > as a generic driver (and does not misbehave) is how to figure out > how many temperature sensors there actually are. So far I could not find out how to query this from the firmware. The IT8688 chip can report the state of each sensor but that is not exposed by the firmware. But even the state information from the IT8688 is not accurate as is. One of the sensors that is reported as being active (directly via it87) on my machine always reports -55°C (yes, negative). > Perhaps the WMI interface returns an error when you query an out-of-range > temperature channel? Also "0" as mentioned above. > One option here might be to add a DMI matching table and only load on > systems on that table for now. That table could then perhaps also provide > labels for each of the temperature channels, which is something which > would be nice to have regardless of my worries about how well this driver > will work on motherboards on which it has not been tested. I am collecting reports for working motherboards at https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/1 . > You could combine this DMI matching table with a "force" module option to > continue with probing on boards which are not on the table to allow users > to test and report their results to you. > > And hopefully after a while, when we're confident that the code works > well on most gigabyte boards we can drop the DMI table, or at least > only use it for the channel labels. That sounds good. > Please don't take this the wrong way; I think it is great that you are > working on this. And the quick turnaround of the v2 of this drivers makes > me pretty certain that we can figure something out and get this merged. Thank you for the feedback! > Have you tried contacting Gigabyte about this? I don't think the WMI > interface is something which they need to keep secret for competitive > reasons, so maybe they can help? Note if they want you to sign a NDA > of sorts to view docs, then make sure that it contains some language > about them allowing you to release an opensource driver for their > hardware based on the "protected" information. I have not contacted them yet, will do. As mentioned in the initial patch submission there would be different ways to access this information firmware: * Directly call the underlying ACPI methods (these are present in all so far observed firmwares, even if not exposed via WMI). * Directly access the ACPI IndexField representing the it87 chip. * Directly access the it87 registers while holding the relevant locks via ACPI. I assume all of those mechanisms have no place in a proper kernel driver but would like to get your opinion on it. Thomas
Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver
Hi, On Mi, 2021-04-07T18:27+, Barnabás Pőcze wrote: > 2021. április 5., hétfő 22:48 keltezéssel, Thomas Weißschuh írta: > > Tested with a X570 I Aorus Pro Wifi. > > The mainboard contains an ITE IT8688E chip for management. > > This chips is also handled by drivers/hwmon/i87.c but as it is also used > > by the firmware itself it needs an ACPI driver. > > I gather this means you're getting the > > ACPI Warning: SystemIO range ... conflicts with ... > ACPI: If an ACPI driver is available for this device, you should use it > instead of the native driver > > warning? Exactly. > > +struct gigabyte_wmi_args { > > + u32 arg1; > > +}; > > + > > +static int gigabyte_wmi_perform_query(enum gigabyte_wmi_commandtype > > command, > > + struct gigabyte_wmi_args *args, struct acpi_buffer *out) > > +{ > > + const struct acpi_buffer in = { > > + .length = sizeof(*args), > > + .pointer = args, > > + }; > > + > > + acpi_status ret = wmi_evaluate_method(GIGABYTE_WMI_GUID, 0x0, command, > > &in, out); > > Ideally, you'd use the WMI device that was passed to the probe method to do > the query > using `wmidev_evaluate_method()`. You can pass the WMI device pointer > to `devm_hwmon_device_register_with_info()` in the `drvdata` argument, > then in the ->read() callback you can retrieve it: > > static int gigabyte_wmi_hwmon_read(struct device *dev, ...) > { > struct wmi_device *wdev = dev_get_drvdata(dev); > > and then you can pass that to the other functions. Done. > > + if (ret == AE_OK) { > > + return 0; > > + } else { > > + return -EIO; > > + }; > > The `;` is not needed. And please use `ACPI_FAILURE()` or `ACPI_SUCCESS()` > to check the returned value. For example: > > acpi_status ret = ...; > if (ACPI_FAILURE(ret)) > return -EIO; > > return 0; Done. > > +} > > + > > +static int gigabyte_wmi_query_integer(enum gigabyte_wmi_commandtype > > command, > > + struct gigabyte_wmi_args *args, u64 *res) > > +{ > > + union acpi_object *obj; > > + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; > > + int ret; > > + > > + ret = gigabyte_wmi_perform_query(command, args, &result); > > + if (ret) { > > + goto out; > > I believe if this branch is taken, no buffer is allocated (due to the > failure), > so you can just `return ret;` here and do away with the goto completely - if > I'm not mistaken. Done. > > +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { > > + HWMON_CHANNEL_INFO(temp, > > + HWMON_T_INPUT, > > + HWMON_T_INPUT, > > + HWMON_T_INPUT, > > + HWMON_T_INPUT, > > + HWMON_T_INPUT, > > + HWMON_T_INPUT), > > + NULL, > ^ > Minor thing: usually commas after sentinel values are omitted. Done. > > +static const struct wmi_device_id gigabyte_wmi_id_table[] = { > > + { GIGABYTE_WMI_GUID, NULL }, > > + { }, >^ > Same here. Done. > > > +}; > > + > > +static struct wmi_driver gigabyte_wmi_driver = { > > + .driver = { > > + .name = "gigabyte-wmi", > > + }, > > + .id_table = gigabyte_wmi_id_table, > > + .probe = gigabyte_wmi_probe, > > +}; > > +module_wmi_driver(gigabyte_wmi_driver); > > + > > +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); > > +MODULE_AUTHOR("Thomas Weißschuh "); > > +MODULE_DESCRIPTION("Gigabyte Temperature WMI Driver"); > > It's a very minor thing, but could you please > synchronize this description with the Kconfig? Of course. Thanks again for the review! Thomas
Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver
Hi Hans, On Do, 2021-04-08T11:36+0200, Hans de Goede wrote: > On 4/7/21 9:43 PM, Thomas Weißschuh wrote: > > Hi Hans, > > > > On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote: > >> Thank you for your new driver and thank you for the quick respin > >> addressing Barnabás' request to make it a WMI driver. > >> > >> The code looks good, so merging this should be a no-brainer, > >> yet I'm not sure if I should merge this driver as-is, let me > >> explain. > > > > thanks for the encouraging words. > > > >> The problem is that I assume that this is based on reverse-engineering? > > > > Yes, it is completely reverse-engineered. > > Essentially I stumbled upon Matthews comment at > > https://bugzilla.kernel.org/show_bug.cgi?id=204807#c37 and went from there. > > > >> We have some mixes experiences with reverse-engineered WMI drivers, > >> sometimes a feature is not supported yet the wmi_evaluate_method() > >> call happily succeeds. One example of this causing trouble is: > >> > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1797d588af15174d4a4e7159dac8c800538e4f8c > > > > There actually are reports of recent, similar mainboards with recent > > firmware and > > similar sensor chips that do not support the temperature query. > > (https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/3 and > > https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/2) > > > > Unfortunately for unknown WMI queries the firmware does not return any > > value. > > This ends up as an ACPI integer with value 0 on the driver side. > > (Which I could not yet find documentation for if that is expected) > > In the current version of the driver EIO is returned for 0 values which > > get translated to N/A by lm-sensors. > > > >> At a minimum I think your driver should check in its > >> probe function that > >> > >> gigabyte_wmi_query_integer(GIGABYTE_WMI_TEMPERATURE_QUERY, ...) > >> > >> actually succeeds on the machine the driver is running on chances > >> are that Gigabyte has been using the DEADBEEF-2001--00A0-C9062910 > >> GUID for ages, where as the 0x125 value for GIGABYTE_WMI_TEMPERATURE_QUERY > >> suggests that this is a pretty new API. > > > > Would it be enough to probe all six sensors and check if all return 0? > > I think that failing the probe with -ENODEV, with a dev_info() explaining why > when > all six sensors return 0 would be good yes, that should also fix those 2 > issues on https://github.com/t-8ch/linux-gigabyte-wmi-driver/. I added such a validation step. > >> It would be good if you can see if you can find some DSDT-s for older > >> gigabyte motherboards attached to various distro's bug-tracking systems > >> or forum posts and see how those respond to an unknown > >> gigabyte_wmi_commandtype. > > > > Will do. > > Since you alreayd have bugreports of boards where this does not work, > please don't spend too much time on this. I guess those older DSDT-s will > also just return an integer with value 0. Ok. > >> Another open question to make sure this driver is suitable > >> as a generic driver (and does not misbehave) is how to figure out > >> how many temperature sensors there actually are. > > > > So far I could not find out how to query this from the firmware. > > The IT8688 chip can report the state of each sensor but that is not exposed > > by > > the firmware. > > But even the state information from the IT8688 is not accurate as is. > > One of the sensors that is reported as being active (directly via it87) on > > my > > machine always reports -55°C (yes, negative). > > Ok. > > >> Perhaps the WMI interface returns an error when you query an out-of-range > >> temperature channel? > > > > Also "0" as mentioned above. > > Hmm, so maybe this can be used to limit the amount of reported temperature > sensors, IOW if sensors 5 and 6 report 0, only register 4 sensors ? So far the 0-returning sensors have not been at the end of the list but in the middle. Is it worth building logic to properly probe a bitmask of useful sensors? > >> One option here might be to add a DMI matching table and only load on > >> systems on that table for now. That table could then perhaps also provide > >> labels for each of the temperature channels, which is something which > >> would be nice to have regardless of my worries about how well this d
Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver
On Do, 2021-04-08T08:00-0700, Guenter Roeck wrote: > On 4/8/21 2:36 AM, Hans de Goede wrote: > > On 4/7/21 9:43 PM, Thomas Weißschuh wrote: > >> On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote: > > Jean, Guenter, > > > > Thomas has been working on a WMI driver to expose various motherboard > > temperatures on a gigabyte board where the IO-addresses for the it87 chip > > are reserved by ACPI. We are discussing how best to deal with this, there > > are some ACPI methods to directly access the super-IO registers (with > > locking > > to protect against other ACPI accesses). This reminded me of an idea I had > > a while ago to solve a similar issue with an other superIO chip, abstract > > the superIO register access-es using some reg_ops struct and allow an > > ACPI/WMI > > driver to provide alternative reg_ops: > > https://bugzilla.kernel.org/show_bug.cgi?id=204807#c47 > > > > Do you think this is a good idea (or a bad one)? And would something like > > that > > be acceptable to you ? > > > > The upstream it87 driver is severely out of date. I had an out-of-tree driver > with various improvements which I didn't upstream, first because no one was > willing > to review changes and then because it had deviated too much. I pulled it from > public view because I got pounded for not upstreaming it, because people > started > demanding support (not asking, demanding) for it, and because Gigabyte stopped > providing datasheets for the more recent ITE chips and it became effectively > unmaintainable. > > Some ITE chips have issues which can cause system hangs if accessed directly. > I put some work to remedy that into the non-upstream driver, but that was all > just guesswork. Gigabyte knows about the problem (or so I was told from > someone > who has an NDA with them), but I didn't get them or ITE to even acknowledge it > to me. I even had a support case open with Gigabyte for a while, but all I > could > get out of them is that they don't support Linux and what I would have to > reproduce > the problem with Windows for them to provide assistance (even though, again, > they knew about it). > > As for using ACPI locks or WMI to ensure that ACPI leaves the chip alone while > the driver accesses chips directly: That is an option, but it has (at least) > two problems. > > First, ACPI access methods are not well documented or standardized. I had > tried > to find useful means to do that some time ago, but I gave up because each > board > (even from the same vendor) handles locking and accesses differently. We would > end up with lots of board specific code. Coincidentally, that was for ASUS > boards > and the nct6775 driver. At least for all the Gigabyte ACPI tables I have looked at all access is done via two-byte "OperationRegion" over the Index/Data addresses, a "Field" with two entries for these and an "IndexField" that is actually used to perform the accesses. As the IndexField is synchronized via "Lock" it should take a lock on the OperationRegion itself. So I think we should be technically fine with validating these assumption and then also taking locks on the OperationRegion. If it is reasonable to do so is another question. > Second, access through ACPI is only one of the issues. Turns out there are two > ITE chips on many of the Gigabyte boards nowadays, and the two chips talk to > each > other using I2C. My out-of-tree driver tried to remedy that by blocking those > accesses while the driver used the chip, but, again, without Gigabyte / ITE > support this was never a perfect solution, and there was always the risk that > the board ended up hanging because that access was blocked for too long. > Recent ITE chips solve that problem by providing memory mapped access to the > chip registers, but that is only useful if one has a datasheet. Are both of these chips available at the two well-known registers 0x2e and 0x4e? Would this too-long blocking also occur when only accessing single registers for read-only access? Any write access would probably have to be blocked anyways. > Overall, I don't think it makes much sense trying to make significant changes > to the it87 driver without pulling in all the changes I had made, and without > finding a better fix for the cross-chip access problems. I for sure won't have > time for that (and getting hwmon patches reviewed is still very much an > issue). > > Having said that, I am of course open to adding WMI/ACPI drivers for the > various > boards. Good luck getting support from Gigabyte, though. Or from ASUS, for > that > matter. Thomas
[PATCH v5] platform/x86: add Gigabyte WMI temperature driver
Tested with * X570 I Aorus Pro Wifi (rev 1.0) * B550M DS3H * B550 Gaming X V2 (rev.1.x) * Z390 I AORUS PRO WIFI (rev. 1.0) Those mainboards contain an ITE chips for management and monitoring. They could also be handled by drivers/hwmon/i87.c. But the SuperIO range used by i87 is already claimed and used by the firmware. The following warning is printed at boot: kernel: ACPI Warning: SystemIO range 0x0A45-0x0A46 conflicts with OpRegion 0x0A45-0x0A46 (\GSA1.SIO1) (20200528/utaddress-204) kernel: ACPI: This conflict may cause random problems and system instability kernel: ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver This driver implements such an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh Reviewed-by: Guenter Roeck --- Changes since v4: * Style * Wording * Alignment of email addresses --- MAINTAINERS | 6 + drivers/platform/x86/Kconfig| 11 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 195 4 files changed, 213 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..7fb5e2ba489b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* F: fs/gfs2/ F: include/uapi/linux/gfs2_ondisk.h +GIGABYTE WMI DRIVER +M: Thomas Weißschuh +L: platform-driver-...@vger.kernel.org +S: Maintained +F: drivers/platform/x86/gigabyte-wmi.c + GNSS SUBSYSTEM M: Johan Hovold S: Maintained diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..96622a2106f7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,17 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature reporting on + Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index ..bb1b0b205fa7 --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,195 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 Thomas Weißschuh + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910" +#define NUM_TEMPERATURE_SENSORS6 + +static bool force_load; +module_param(force_load, bool, 0444); +MODULE_PARM_DESC(force_load, "Force loading on unknown platform"); + +static u8 usable_sensors_mask; + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +struct gigabyte_wmi_args { + u32 arg1; +}; + +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, struct acpi_buffer *out) +{ + const struct acpi_buffer in = { + .length = sizeof(*args), + .pointer = args, + }; + + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); + + if (ACPI_FAILURE(ret)) + return -EIO; + + return 0; +} + +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *arg
[PATCH v3] platform/x86: add Gigabyte WMI temperature driver
Changes since v1: * Incorporate feedback from Barnabás Pőcze * Use a WMI driver instead of a platform driver * Let the kernel manage the driver lifecycle * Fix errno/ACPI error confusion * Fix resource cleanup * Document reason for integer casting Changes since v2: * Style cleanups * Test for usability during probing * DMI-based whitelist * CC hwmon maintainers -- >8 -- Tested with a X570 I Aorus Pro Wifi. The mainboard contains an ITE IT8688E chip for management. This chips is also handled by drivers/hwmon/i87.c but as it is also used by the firmware itself it needs an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh --- MAINTAINERS | 6 + drivers/platform/x86/Kconfig| 11 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 194 4 files changed, 212 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..9c10cfc00fe8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* F: fs/gfs2/ F: include/uapi/linux/gfs2_ondisk.h +GIGABYTE WMI DRIVER +M: Thomas Weißschuh +L: platform-driver-...@vger.kernel.org +S: Maintained +F: drivers/platform/x86/gigabyte-wmi.c + GNSS SUBSYSTEM M: Johan Hovold S: Maintained diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..96622a2106f7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,17 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature reporting on + Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index ..fb4e6d4c1823 --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,194 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 Thomas Weißschuh + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910" +#define NUM_TEMPERATURE_SENSORS 6 + +static bool force_load; +module_param(force_load, bool, 0); +MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform"); + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +struct gigabyte_wmi_args { + u32 arg1; +}; + +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, struct acpi_buffer *out) +{ + const struct acpi_buffer in = { + .length = sizeof(*args), + .pointer = args, + }; + + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); + + if ACPI_FAILURE(ret) + return -EIO; + + return 0; +} + +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, u64 *res) +{ + union acpi_object *obj; + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; + int ret; + + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); + if (ret) + return ret; + obj = result.pointer; + if (obj && obj->ty
[PATCH v4] platform/x86: add Gigabyte WMI temperature driver
Changes since v3: * Completely hide unusable sensors * Expose force_load parameter read-only via sysfs * Naming * Style cleanups -- >8 -- Tested with * X570 I Aorus Pro Wifi (rev 1.0) * B550M DS3H * B550 Gaming X V2 (rev.1.x) * Z390 I AORUS PRO WIFI (rev. 1.0) The mainboard contains an ITE IT8688E chip for management. This chips is also handled by drivers/hwmon/i87.c but as it is also used by the firmware itself it needs an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh --- MAINTAINERS | 6 + drivers/platform/x86/Kconfig| 11 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 195 4 files changed, 213 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..9c10cfc00fe8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* F: fs/gfs2/ F: include/uapi/linux/gfs2_ondisk.h +GIGABYTE WMI DRIVER +M: Thomas Weißschuh +L: platform-driver-...@vger.kernel.org +S: Maintained +F: drivers/platform/x86/gigabyte-wmi.c + GNSS SUBSYSTEM M: Johan Hovold S: Maintained diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..96622a2106f7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,17 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature reporting on + Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index ..c17e51fcf000 --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,195 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 Thomas Weißschuh + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910" +#define NUM_TEMPERATURE_SENSORS6 + +static bool force_load; +module_param(force_load, bool, 0444); +MODULE_PARM_DESC(force_load, "Force loading on unknown platform"); + +static u8 usable_sensors_mask; + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +struct gigabyte_wmi_args { + u32 arg1; +}; + +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, struct acpi_buffer *out) +{ + const struct acpi_buffer in = { + .length = sizeof(*args), + .pointer = args, + }; + + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); + + if ACPI_FAILURE(ret) + return -EIO; + + return 0; +} + +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, u64 *res) +{ + union acpi_object *obj; + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; + int ret; + + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); + if (ret) + return ret; + obj = result.pointer; + if (obj && obj->type == ACPI_TYPE_INTEGER) + *res = obj->integer.value; + else +
[PATCH] platform/x86: add Gigabyte WMI temperature driver
Hi, this patch adds support for temperature readings on Gigabyte Mainboards. (At least on mine) The current code should be usable as is. I'd like for people to test it though and report their results with different hardware. Further development I have some general questions: The ASL IndexField does not cover all relevant registers, can it be extended by driver code? * Not all registers are exposed via ACPI methods, can they be accessed via ACPI directly? * Some registers are exposed via ACPI methods but are not reachable directly from the WMI dispatcher. * Does ASL have some sort of reflection that could enable those methods? * Is it possible to call those methods directly, bypassing WMI? I suspect the answer to be "no" for all of these, but maybe I am wrong. Furthermore there are WMI methods to return information about the installed firmware. * Version * Build-date * Motherboard name Would it make sense to add this information as attributes to the platform_device? The ACPI tables can be found here: https://github.com/t-8ch/linux-gigabyte-wmi-driver/blob/main/ssdt8.dsl Thanks, Thomas -- >8 -- Tested with a X570 I Aorus Pro Wifi. The mainboard contains a ITE IT8688E chip for management. This chips is also handled by drivers/hwmon/i87.c but as it is also used by the firmware itself it needs an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh --- drivers/platform/x86/Kconfig| 10 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 178 3 files changed, 189 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..40d593ff1f01 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,16 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature on Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index ..a3749cf248cb --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,178 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Gigabyte WMI temperature driver + * + * Copyright (C) 2021 Thomas Weißschuh + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include +#include +#include + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C90629100000" +#define DRVNAME "gigabyte-wmi" + +MODULE_AUTHOR("Thomas Weißschuh "); +MODULE_DESCRIPTION("Gigabyte Generic WMI Driver"); +MODULE_LICENSE("GPL"); + +MODULE_ALIAS("wmi:" GIGABYTE_WMI_GUID); + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +static int gigabyte_wmi_temperature(u8 sensor, s8 *res); + +static ssize_t temp_show(struct device *dev, struct device_attribute *attr, +char *buf) +{ + struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr); + int index = sattr->index; + s8 temp; + acpi_status res; + + res = gigabyte_wmi_temperature(index, &temp); + if (ACPI_FAILURE(res)) + return -res; + + return sprintf(buf, "%d\n", temp * 1000); +} + +static SENSOR_DEVICE_ATTR_2_RO(temp1_input, temp, 0, 0); +static SENSOR_DEVICE_ATTR_2_RO(temp2_input, temp, 0, 1); +static SENSOR_DEVICE_ATTR_2_RO(temp3_input, temp, 0, 2); +static SENSOR_DEVICE_ATTR_2_RO(temp4_input, temp, 0, 3);
[PATCH v2] platform/x86: add Gigabyte WMI temperature driver
Changes since v1: * Incorporate feedback from Barnabás Pőcze * Use a WMI driver instead of a platform driver * Let the kernel manage the driver lifecycle * Fix errno/ACPI error confusion * Fix resource cleanup * Document reason for integer casting Thank you Barnabás for your review, it is much appreciated. -- >8 -- Tested with a X570 I Aorus Pro Wifi. The mainboard contains an ITE IT8688E chip for management. This chips is also handled by drivers/hwmon/i87.c but as it is also used by the firmware itself it needs an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh --- drivers/platform/x86/Kconfig| 11 +++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 138 3 files changed, 150 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..96622a2106f7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,17 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature reporting on + Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index ..8618363e3ccf --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 Thomas Weißschuh + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910" + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +struct gigabyte_wmi_args { + u32 arg1; +}; + +static int gigabyte_wmi_perform_query(enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, struct acpi_buffer *out) +{ + const struct acpi_buffer in = { + .length = sizeof(*args), + .pointer = args, + }; + + acpi_status ret = wmi_evaluate_method(GIGABYTE_WMI_GUID, 0x0, command, &in, out); + if (ret == AE_OK) { + return 0; + } else { + return -EIO; + }; +} + +static int gigabyte_wmi_query_integer(enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, u64 *res) +{ + union acpi_object *obj; + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; + int ret; + + ret = gigabyte_wmi_perform_query(command, args, &result); + if (ret) { + goto out; + } + obj = result.pointer; + if (obj && obj->type == ACPI_TYPE_INTEGER) { + *res = obj->integer.value; + ret = 0; + } else { + ret = -EIO; + } +out: + kfree(result.pointer); + return ret; +} + +static int gigabyte_wmi_temperature(u8 sensor, long *res) +{ + struct gigabyte_wmi_args args = { + .arg1 = sensor, + }; + u64 temp; + acpi_status ret; + + ret = gigabyte_wmi_query_integer(GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); + if (ret == 0) + *res = (s8) temp * 1000; // value is a signed 8-bit integer + return ret; +} + +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + return gigabyte_wmi_temperature(channel, val); +} + +static umode_t gigabyte_wmi_hwmon_is_visib
Re: [PATCH 3.16 310/366] vmxnet3: fix checks for dma mapping errors
> 3.16.60-rc1 review patch. If anyone has any objections, please let me know. Sorry for the late response, this just hit the kernel in Debian Jessie (oldstable) a few days ago. > -- > > From: Alexey Khoroshilov > > commit 5738a09d58d5ad2871f1f9a42bf6a3aa9ece5b3c upstream. > > vmxnet3_drv does not check dma_addr with dma_mapping_error() > after mapping dma memory. The patch adds the checks and > tries to handle failures. We are seeing kernel panics/machine freezes/BUGs with the new 3.16.64 from Debian. I bisected it with the vanilla stable kernel and it boiled down to this commit. VMs of multiple nodes of our vmware cluster are affected. The bug can be triggered in multiple ways, I have seen it when an external network request is served, when installing packages over the network and performing a git clone. I will try to get the specific versions of the involved hardware components next week. The 4.9.144 stable kernel (which also contains this commit works fine on the affected machine) Below you can see the dmesg log of one affected machine: [1.772994] vmxnet3 :03:00.0 eth0: intr type 3, mode 0, 5 vectors allocated [1.774079] vmxnet3 :03:00.0 eth0: NIC Link is Up 1 Mbps [9.622787] gunicorn: worke: Corrupted page table at address 362d000 [9.622817] PGD 8000753b7067 PUD 6f84e067 PMD 76cbb067 PTE 6461685368637845 [9.622848] Bad pagetable: 000d [#1] SMP [9.622866] Modules linked in: binfmt_misc ip6table_filter ip6_tables ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_comment xt_multiport xt_conntrack nf_conntrack iptable_filter ip_tables x_tables crc32_pclmul crc32c_intel aesni_intel aes_x86_64 glue_helper lrw vmw_vsock_vmci_transport vsock gf128mul vmw_balloon ppdev evdev ablk_helper cryptd pcspkr serio_raw vmwgfx drm_kms_helper ttm ac processor battery button parport_pc thermal_sys drm parport shpchp vmw_vmci autofs4 ext4 crc16 mbcache jbd2 dm_mod sg sr_mod cdrom sd_mod crc_t10dif crct10dif_generic ata_generic crct10dif_pclmul crct10dif_common psmouse vmxnet3 ata_piix mptspi scsi_transport_spi mptscsih libata i2c_piix4 mptbase scsi_mod i2c_core [9.623168] CPU: 1 PID: 717 Comm: gunicorn: worke Not tainted 3.16.59+ #18 [9.623191] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016 [9.623225] task: 88007835e090 ti: 88006f834000 task.ti: 88006f834000 [9.623249] RIP: 0033:[<7fb4bfb6d123>] [<7fb4bfb6d123>] 0x7fb4bfb6d123 [9.623278] RSP: 002b:7fff6e4718b8 EFLAGS: 00010206 [9.623296] RAX: fff7b8c0 RBX: 036aadc0 RCX: 036b1740 [9.623318] RDX: 0372f500 RSI: 03626690 RDI: 036aade0 [9.623341] RBP: 00084740 R08: fff7b8b0 R09: fff7b8a0 [9.623363] R10: fff7b890 R11: 0037 R12: 00085760 [9.623385] R13: 004cd810 R14: 1000 R15: 03589dd0 [9.623408] FS: 7fb4c0ffe700() GS:88007fc8() knlGS: [9.623433] CS: 0010 DS: ES: CR0: 80050033 [9.623451] CR2: 0362d000 CR3: 753fa000 CR4: 00360770 [9.623524] DR0: DR1: DR2: [9.623547] DR3: DR6: fffe0ff0 DR7: 0400 [9.623577] RIP [<7fb4bfb6d123>] 0x7fb4bfb6d123 [9.623600] RSP <7fff6e4718b8> [9.623614] ---[ end trace f863ea854df6c9a5 ]--- [9.624169] swap_free: Bad swap file entry 1001a1e5a32423f7 [9.624189] BUG: Bad page map in process gunicorn: worke pte:417869736f702024 pmd:76cbb067 [9.624215] addr:0360 vm_flags:08100073 anon_vma:88007538c470 mapping: (null) index:3600 [9.625444] CPU: 1 PID: 717 Comm: gunicorn: worke Tainted: G D 3.16.59+ #18 [9.626070] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016 [9.627321] 8151fda4 0360 8800753700d0 [9.627968] 8116f380 0008 880076cbb000 417869736f702024 [9.628596] 0373f000 88006f837dd0 0360 [9.629213] Call Trace: [9.629811] [] ? dump_stack+0x5d/0x78 [9.630413] [] ? print_bad_pte+0x1b0/0x280 [9.630991] [] ? unmap_single_vma+0x4c2/0x830 [9.631556] [] ? unmap_vmas+0x4c/0xa0 [9.632106] [] ? exit_mmap+0x92/0x160 [9.632640] [] ? mmput+0x5c/0x120 [9.633162] [] ? do_exit+0x333/0xae0 [9.633677] [] ? printk+0x4f/0x57 [9.634171] [] ? oops_end+0x97/0xe0 [9.634653] [] ? __do_page_fault+0x376/0x470 [9.635129] [] ? page_fault+0x28/0x30 [9.635629] BUG: Bad page map in process gunicorn: worke pte:2420746e756f6363 pmd:76cbb067 [9.636111] addr:03601000 vm_flags:08100073 anon_vma:88007538c470 mapping: (null) index:3601
Re: [PATCH 1/4] LoongArch: Don't crash in stack_top() for tasks without vDSO
Hi Huacai, On Tue, Oct 15, 2024 at 10:15:39AM +0800, Huacai Chen wrote: > I can take this patch to the loongarch tree, but I think others should > get upstream via kselftests tree? Yes, sounds good. Could you take a look at patches 2 and 4, too? Thanks, Thomas > On Mon, Oct 14, 2024 at 7:36 PM Thomas Weißschuh > wrote: > > > > Not all tasks have a vDSO mapped, for example kthreads never do. > > If such a task ever ends up calling stack_top(), it will derefence the > > NULL vdso pointer and crash. > > > > This can for example happen when using kunit: > > > > [<90203874>] stack_top+0x58/0xa8 > > [<902956cc>] arch_pick_mmap_layout+0x164/0x220 > > [<903c284c>] kunit_vm_mmap_init+0x108/0x12c > > [<903c1fbc>] __kunit_add_resource+0x38/0x8c > > [<903c2704>] kunit_vm_mmap+0x88/0xc8 > > [<90410b14>] usercopy_test_init+0xbc/0x25c > > [<903c1db4>] kunit_try_run_case+0x5c/0x184 > > [<903c3d54>] kunit_generic_run_threadfn_adapter+0x24/0x48 > > [<9022e4bc>] kthread+0xc8/0xd4 > > [<90200ce8>] ret_from_kernel_thread+0xc/0xa4 > > > > Fixes: 803b0fc5c3f2 ("LoongArch: Add process management") > > Signed-off-by: Thomas Weißschuh > > --- > > arch/loongarch/kernel/process.c | 16 +--- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/arch/loongarch/kernel/process.c > > b/arch/loongarch/kernel/process.c > > index > > f2ff8b5d591e4fd638109d2c98d75543c01a112c..6e58f65455c7ca3eae2e88ed852c8655a6701e5c > > 100644 > > --- a/arch/loongarch/kernel/process.c > > +++ b/arch/loongarch/kernel/process.c > > @@ -293,13 +293,15 @@ unsigned long stack_top(void) > > { > > unsigned long top = TASK_SIZE & PAGE_MASK; > > > > - /* Space for the VDSO & data page */ > > - top -= PAGE_ALIGN(current->thread.vdso->size); > > - top -= VVAR_SIZE; > > - > > - /* Space to randomize the VDSO base */ > > - if (current->flags & PF_RANDOMIZE) > > - top -= VDSO_RANDOMIZE_SIZE; > > + if (current->thread.vdso) { > > + /* Space for the VDSO & data page */ > > + top -= PAGE_ALIGN(current->thread.vdso->size); > > + top -= VVAR_SIZE; > > + > > + /* Space to randomize the VDSO base */ > > + if (current->flags & PF_RANDOMIZE) > > + top -= VDSO_RANDOMIZE_SIZE; > > + } > > > > return top; > > } > > > > -- > > 2.47.0 > >
[PATCH 3/4] kunit: tool: Allow overriding the shutdown mode from qemu config
Not all platforms support machine reboot. If it a proper reboot is not supported the machine will hang. Allow the QEMU configuration to override the necessary shutdown mode for the specific system under test. Signed-off-by: Thomas Weißschuh --- tools/testing/kunit/kunit_kernel.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 61931c4926fd6645f2c62dd13f9842a432ec4167..e76d7894b6c5195ece49f0d8c7ac35130df428a9 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -105,7 +105,9 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): self._kconfig = qemu_arch_params.kconfig self._qemu_arch = qemu_arch_params.qemu_arch self._kernel_path = qemu_arch_params.kernel_path - self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot' + self._kernel_command_line = qemu_arch_params.kernel_command_line + if 'kunit_shutdown=' not in self._kernel_command_line: + self._kernel_command_line += ' kunit_shutdown=reboot' self._extra_qemu_params = qemu_arch_params.extra_qemu_params self._serial = qemu_arch_params.serial -- 2.47.0
[PATCH 4/4] kunit: qemu_configs: loongarch: Enable shutdown
QEMU for LoongArch does not yet support shutdown/restart through ACPI. Use the pvpanic driver to enable shutdowns. This requires 9.1.0 for shutdown support in pvpanic, but that is the requirement of kunit on LoongArch anyways. Signed-off-by: Thomas Weißschuh --- tools/testing/kunit/qemu_configs/loongarch.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/kunit/qemu_configs/loongarch.py b/tools/testing/kunit/qemu_configs/loongarch.py index e7bb7c07819677dfdefac012821a732555813cae..1d2b780fbd5c0bde20aa6a5cd1217d0b3b443a93 100644 --- a/tools/testing/kunit/qemu_configs/loongarch.py +++ b/tools/testing/kunit/qemu_configs/loongarch.py @@ -4,13 +4,16 @@ QEMU_ARCH = QemuArchParams(linux_arch='loongarch', kconfig=''' CONFIG_EFI_STUB=n CONFIG_PCI_HOST_GENERIC=y +CONFIG_PVPANIC=y +CONFIG_PVPANIC_PCI=y CONFIG_SERIAL_8250=y CONFIG_SERIAL_8250_CONSOLE=y CONFIG_SERIAL_OF_PLATFORM=y ''', qemu_arch='loongarch64', kernel_path='arch/loongarch/boot/vmlinux.elf', - kernel_command_line='console=ttyS0', + kernel_command_line='console=ttyS0 kunit_shutdown=poweroff', extra_qemu_params=[ '-machine', 'virt', + '-device', 'pvpanic-pci', '-cpu', 'max',]) -- 2.47.0
[PATCH 2/4] kunit: qemu_configs: add LoongArch config
Add a basic config to run kunit tests on LoongArch. This requires QEMU 9.1.0 or later for the necessary direct kernel boot support. Signed-off-by: Thomas Weißschuh --- tools/testing/kunit/qemu_configs/loongarch.py | 16 1 file changed, 16 insertions(+) diff --git a/tools/testing/kunit/qemu_configs/loongarch.py b/tools/testing/kunit/qemu_configs/loongarch.py new file mode 100644 index ..e7bb7c07819677dfdefac012821a732555813cae --- /dev/null +++ b/tools/testing/kunit/qemu_configs/loongarch.py @@ -0,0 +1,16 @@ +from ..qemu_config import QemuArchParams + +QEMU_ARCH = QemuArchParams(linux_arch='loongarch', + kconfig=''' +CONFIG_EFI_STUB=n +CONFIG_PCI_HOST_GENERIC=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_SERIAL_OF_PLATFORM=y +''', + qemu_arch='loongarch64', + kernel_path='arch/loongarch/boot/vmlinux.elf', + kernel_command_line='console=ttyS0', + extra_qemu_params=[ + '-machine', 'virt', + '-cpu', 'max',]) -- 2.47.0
[PATCH 0/4] kunit: Add support for LoongArch
Enable LoongArch support in kunit. Example: $ ./tools/testing/kunit/kunit.py run --arch=loongarch --cross_compile=$CROSS_COMPILE [13:32:45] Configuring KUnit Kernel ... [13:32:45] Building KUnit Kernel ... Populating config with: $ make ARCH=loongarch olddefconfig CROSS_COMPILE=$CROSS_COMPILE Building with: $ make all compile_commands.json ARCH=loongarch --jobs=8 CROSS_COMPILE=$CROSS_COMPILE [13:32:48] Starting KUnit Kernel (1/1)... [13:32:48] Running tests with: $ qemu-system-loongarch64 -nodefaults -m 1024 -kernel .kunit/arch/loongarch/boot/vmlinux.elf -append 'kunit.enable=1 console=ttyS0 kunit_shutdown=poweroff' -no-reboot -nographic -serial stdio -machine virt -device pvpanic-pci -cpu max ... [13:33:14] [13:33:14] Testing complete. Ran 493 tests: passed: 453, skipped: 40 [13:33:14] Elapsed time: 28.862s total, 0.002s configuring, 2.526s building, 26.302s running Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (4): LoongArch: Don't crash in stack_top() for tasks without vDSO kunit: qemu_configs: add LoongArch config kunit: tool: Allow overriding the shutdown mode from qemu config kunit: qemu_configs: loongarch: Enable shutdown arch/loongarch/kernel/process.c | 16 +--- tools/testing/kunit/kunit_kernel.py | 4 +++- tools/testing/kunit/qemu_configs/loongarch.py | 19 +++ 3 files changed, 31 insertions(+), 8 deletions(-) --- base-commit: 6485cf5ea253d40d507cd71253c9568c5470cd27 change-id: 20241014-kunit-loongarch-98a5b756e818 Best regards, -- Thomas Weißschuh
[PATCH 1/4] LoongArch: Don't crash in stack_top() for tasks without vDSO
Not all tasks have a vDSO mapped, for example kthreads never do. If such a task ever ends up calling stack_top(), it will derefence the NULL vdso pointer and crash. This can for example happen when using kunit: [<90203874>] stack_top+0x58/0xa8 [<902956cc>] arch_pick_mmap_layout+0x164/0x220 [<903c284c>] kunit_vm_mmap_init+0x108/0x12c [<903c1fbc>] __kunit_add_resource+0x38/0x8c [<903c2704>] kunit_vm_mmap+0x88/0xc8 [<90410b14>] usercopy_test_init+0xbc/0x25c [<903c1db4>] kunit_try_run_case+0x5c/0x184 [<903c3d54>] kunit_generic_run_threadfn_adapter+0x24/0x48 [<9022e4bc>] kthread+0xc8/0xd4 [<90200ce8>] ret_from_kernel_thread+0xc/0xa4 Fixes: 803b0fc5c3f2 ("LoongArch: Add process management") Signed-off-by: Thomas Weißschuh --- arch/loongarch/kernel/process.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c index f2ff8b5d591e4fd638109d2c98d75543c01a112c..6e58f65455c7ca3eae2e88ed852c8655a6701e5c 100644 --- a/arch/loongarch/kernel/process.c +++ b/arch/loongarch/kernel/process.c @@ -293,13 +293,15 @@ unsigned long stack_top(void) { unsigned long top = TASK_SIZE & PAGE_MASK; - /* Space for the VDSO & data page */ - top -= PAGE_ALIGN(current->thread.vdso->size); - top -= VVAR_SIZE; - - /* Space to randomize the VDSO base */ - if (current->flags & PF_RANDOMIZE) - top -= VDSO_RANDOMIZE_SIZE; + if (current->thread.vdso) { + /* Space for the VDSO & data page */ + top -= PAGE_ALIGN(current->thread.vdso->size); + top -= VVAR_SIZE; + + /* Space to randomize the VDSO base */ + if (current->flags & PF_RANDOMIZE) + top -= VDSO_RANDOMIZE_SIZE; + } return top; } -- 2.47.0
[PATCH] selftests/nolibc: start qemu with 1 GiB of memory
Recently the loongarch defconfig stopped working with the default 128 MiB of memory. The VM just spins infinitively. Increasing the available memory to 1 GiB, similar to s390, fixes the issue. To avoid having to do this for each architecture on its own, proactively apply to all architectures. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 8de98ea7af8071caa0597aa7b86d91a2d1d50e68..e92e0b88586111072a0e043cb15f3b59cf42c3a6 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -130,9 +130,9 @@ QEMU_ARGS_ppc= -M g3beige -append "console=ttyS0 panic=-1 $(TEST:%=NOLIB QEMU_ARGS_ppc64 = -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_ppc64le= -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" -QEMU_ARGS_s390 = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" +QEMU_ARGS_s390 = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)" -QEMU_ARGS= $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_BIOS) $(QEMU_ARGS_EXTRA) +QEMU_ARGS= -m 1G $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_BIOS) $(QEMU_ARGS_EXTRA) # OUTPUT is only set when run from the main makefile, otherwise # it defaults to this nolibc directory. --- base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b change-id: 20241007-nolibc-qemu-mem-5ed605520472 Best regards, -- Thomas Weißschuh
Re: [PATCH 2/4] kunit: qemu_configs: add LoongArch config
Hi Shuah, Oct 17, 2024 22:27:29 Shuah Khan : > On 10/14/24 05:36, Thomas Weißschuh wrote: >> Add a basic config to run kunit tests on LoongArch. >> This requires QEMU 9.1.0 or later for the necessary direct kernel boot >> support. >> Signed-off-by: Thomas Weißschuh >> --- >> tools/testing/kunit/qemu_configs/loongarch.py | 16 >> 1 file changed, 16 insertions(+) >> diff --git a/tools/testing/kunit/qemu_configs/loongarch.py >> b/tools/testing/kunit/qemu_configs/loongarch.py >> new file mode 100644 >> index >> ..e7bb7c07819677dfdefac012821a732555813cae >> --- /dev/null >> +++ b/tools/testing/kunit/qemu_configs/loongarch.py > > Missing SPDX-License-Identifier. Tue others configs don't have one either. >> @@ -0,0 +1,16 @@ >> +from ..qemu_config import QemuArchParams >> + >> +QEMU_ARCH = QemuArchParams(linux_arch='loongarch', >> + kconfig=''' >> +CONFIG_EFI_STUB=n >> +CONFIG_PCI_HOST_GENERIC=y >> +CONFIG_SERIAL_8250=y >> +CONFIG_SERIAL_8250_CONSOLE=y >> +CONFIG_SERIAL_OF_PLATFORM=y >> +''', >> + qemu_arch='loongarch64', >> + kernel_path='arch/loongarch/boot/vmlinux.elf', >> + kernel_command_line='console=ttyS0', >> + extra_qemu_params=[ >> + '-machine', 'virt', >> + '-cpu', 'max',]) >> > > Please send v2 with all the reviewed by tags. If there > is a resend 3.4 and 4/4 in this series, send them. I'll do that. But it will take some weeks, as I just went on vacation. Thomas
Re: [PATCH 2/2] kunit: enable hardware acceleration when available
On 2024-11-01 10:49:36-0400, Tamir Duberstein wrote: > On Fri, Nov 1, 2024, 09:52 Alyssa Ross wrote: > > > > On Fri, Oct 25, 2024 at 05:03:54PM -0400, Tamir Duberstein wrote: > > > @@ -124,6 +125,29 @@ class > > > LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): > > > '-no-reboot', > > > '-nographic', > > > '-serial', self._serial] + > > > self._extra_qemu_params > > > + accelerators = { > > > + line.strip() > > > + for line in subprocess.check_output([qemu_binary, > > > "-accel", "help"], text=True).splitlines() > > > + if line and line.islower() > > > + } > > > + if 'kvm' in accelerators: > > > + try: > > > + with open('/dev/kvm', 'rb+'): > > > + qemu_command.extend(['-accel', > > > 'kvm']) > > > + except OSError as e: > > > + print(e) > > > + elif 'hvf' in accelerators: > > > + try: > > > + for line in > > > subprocess.check_output(['sysctl', 'kern.hv_support'], > > > text=True).splitlines(): > > > + if not line: > > > + continue > > > + key, value = line.split(':') > > > + if key == 'kern.hv_support' and > > > bool(value): > > > + > > > qemu_command.extend(['-accel', 'hvf']) > > > + break > > > + except subprocess.CalledProcessError as e: > > > + print(e) > > > + > > > > QEMU supports falling back if one accelerator is not available, if you > > specify multiple like -accel kvm:tcg. Couldn't you rely on that rather > > than re-implementing the availability checks here? > > Have you ever used that? Here's what I get when I try: > > tamird@Tamirs-MacBook-Pro linux % tools/testing/kunit/kunit.py run > --arch arm64 --make_options LLVM=1 --raw_output=all > [10:45:03] Configuring KUnit Kernel ... > [10:45:03] Building KUnit Kernel ... > Populating config with: > $ make ARCH=arm64 O=.kunit olddefconfig LLVM=1 > Building with: > $ make all compile_commands.json ARCH=arm64 O=.kunit --jobs=12 LLVM=1 > [10:45:07] Starting KUnit Kernel (1/1)... > Running tests with: > $ qemu-system-aarch64 -nodefaults -m 1024 -kernel > .kunit/arch/arm64/boot/Image.gz -append 'kunit.enable=1 > console=ttyAMA0 kunit_shutdown=reboot' -no-reboot -nographic -serial > stdio -accel kvm:tcg -machine virt -cpu max > qemu-system-aarch64: -accel kvm:tcg: invalid accelerator kvm:tcg > > The same thing happens with hvf:kvm:tcg or just hvf:tcg. That syntax is for -machine accel=hvf:kvm:tcg. But you can also specify -accel multiple times. > I also can't find this in the documentation. -accel name[,prop=value[,...]] This is used to enable an accelerator. [..] If there is more than one accelerator specified, the next one is used if the previous one fails to initialize. -machine [type=]name[,prop=value[,...]] Supported machine properties are: accel=accels1[:accels2[:...]] [..] If there is more than one accelerator specified, the next one is used if the previous one fails to initialize. Thomas
[PATCH v2 3/3] kunit: qemu_configs: loongarch: Enable shutdown
QEMU for LoongArch does not yet support shutdown/restart through ACPI. Use the pvpanic driver to enable shutdowns. This requires 9.1.0 for shutdown support in pvpanic, but that is the requirement of kunit on LoongArch anyways. Signed-off-by: Thomas Weißschuh Reviewed-by: Bibo Mao Reviewed-by: David Gow --- tools/testing/kunit/qemu_configs/loongarch.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/kunit/qemu_configs/loongarch.py b/tools/testing/kunit/qemu_configs/loongarch.py index a874a2156e0f791caaaf30e5f7b17fc175bb55ee..a92422967d1da9f1658ef1e80d0d7365ddbae307 100644 --- a/tools/testing/kunit/qemu_configs/loongarch.py +++ b/tools/testing/kunit/qemu_configs/loongarch.py @@ -6,13 +6,16 @@ QEMU_ARCH = QemuArchParams(linux_arch='loongarch', kconfig=''' CONFIG_EFI_STUB=n CONFIG_PCI_HOST_GENERIC=y +CONFIG_PVPANIC=y +CONFIG_PVPANIC_PCI=y CONFIG_SERIAL_8250=y CONFIG_SERIAL_8250_CONSOLE=y CONFIG_SERIAL_OF_PLATFORM=y ''', qemu_arch='loongarch64', kernel_path='arch/loongarch/boot/vmlinux.elf', - kernel_command_line='console=ttyS0', + kernel_command_line='console=ttyS0 kunit_shutdown=poweroff', extra_qemu_params=[ '-machine', 'virt', + '-device', 'pvpanic-pci', '-cpu', 'max',]) -- 2.47.0
[PATCH v2 0/3] kunit: Add support for LoongArch
Enable LoongArch support in kunit. Example: $ ./tools/testing/kunit/kunit.py run --arch=loongarch --cross_compile=$CROSS_COMPILE [13:32:45] Configuring KUnit Kernel ... [13:32:45] Building KUnit Kernel ... Populating config with: $ make ARCH=loongarch olddefconfig CROSS_COMPILE=$CROSS_COMPILE Building with: $ make all compile_commands.json ARCH=loongarch --jobs=8 CROSS_COMPILE=$CROSS_COMPILE [13:32:48] Starting KUnit Kernel (1/1)... [13:32:48] Running tests with: $ qemu-system-loongarch64 -nodefaults -m 1024 -kernel .kunit/arch/loongarch/boot/vmlinux.elf -append 'kunit.enable=1 console=ttyS0 kunit_shutdown=poweroff' -no-reboot -nographic -serial stdio -machine virt -device pvpanic-pci -cpu max ... [13:33:14] [13:33:14] Testing complete. Ran 493 tests: passed: 453, skipped: 40 [13:33:14] Elapsed time: 28.862s total, 0.002s configuring, 2.526s building, 26.302s running This series is meant to be merged through the kselftest tree. Signed-off-by: Thomas Weißschuh --- Changes in v2: - Drop already applied patch - Pick up review tags - Add SPDX header - Link to v1: https://lore.kernel.org/r/20241014-kunit-loongarch-v1-0-1699b2ad6...@linutronix.de --- Thomas Weißschuh (3): kunit: qemu_configs: Add LoongArch config kunit: tool: Allow overriding the shutdown mode from qemu config kunit: qemu_configs: loongarch: Enable shutdown tools/testing/kunit/kunit_kernel.py | 4 +++- tools/testing/kunit/qemu_configs/loongarch.py | 21 + 2 files changed, 24 insertions(+), 1 deletion(-) --- base-commit: 2d5404caa8c7bb5c4e0435f94b28834ae5456623 change-id: 20241014-kunit-loongarch-98a5b756e818 Best regards, -- Thomas Weißschuh
[PATCH v2 1/3] kunit: qemu_configs: Add LoongArch config
Add a basic config to run kunit tests on LoongArch. This requires QEMU 9.1.0 or later for the necessary direct kernel boot support. Signed-off-by: Thomas Weißschuh Reviewed-by: Bibo Mao Reviewed-by: David Gow --- tools/testing/kunit/qemu_configs/loongarch.py | 18 ++ 1 file changed, 18 insertions(+) diff --git a/tools/testing/kunit/qemu_configs/loongarch.py b/tools/testing/kunit/qemu_configs/loongarch.py new file mode 100644 index ..a874a2156e0f791caaaf30e5f7b17fc175bb55ee --- /dev/null +++ b/tools/testing/kunit/qemu_configs/loongarch.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: GPL-2.0 + +from ..qemu_config import QemuArchParams + +QEMU_ARCH = QemuArchParams(linux_arch='loongarch', + kconfig=''' +CONFIG_EFI_STUB=n +CONFIG_PCI_HOST_GENERIC=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_SERIAL_OF_PLATFORM=y +''', + qemu_arch='loongarch64', + kernel_path='arch/loongarch/boot/vmlinux.elf', + kernel_command_line='console=ttyS0', + extra_qemu_params=[ + '-machine', 'virt', + '-cpu', 'max',]) -- 2.47.0
[PATCH v2 2/3] kunit: tool: Allow overriding the shutdown mode from qemu config
Not all platforms support machine reboot. If it a proper reboot is not supported the machine will hang. Allow the QEMU configuration to override the necessary shutdown mode for the specific system under test. Signed-off-by: Thomas Weißschuh Reviewed-by: David Gow --- tools/testing/kunit/kunit_kernel.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 61931c4926fd6645f2c62dd13f9842a432ec4167..e76d7894b6c5195ece49f0d8c7ac35130df428a9 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -105,7 +105,9 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): self._kconfig = qemu_arch_params.kconfig self._qemu_arch = qemu_arch_params.qemu_arch self._kernel_path = qemu_arch_params.kernel_path - self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot' + self._kernel_command_line = qemu_arch_params.kernel_command_line + if 'kunit_shutdown=' not in self._kernel_command_line: + self._kernel_command_line += ' kunit_shutdown=reboot' self._extra_qemu_params = qemu_arch_params.extra_qemu_params self._serial = qemu_arch_params.serial -- 2.47.0
[PATCH 3/4] btf: Switch vmlinux BTF attribute to sysfs_bin_attr_simple_read()
The generic function from the sysfs core can replace the custom one. Signed-off-by: Thomas Weißschuh --- This is a replacement for [0], as Alexei was not happy about BIN_ATTR_SIMPLE_RO() [0] https://lore.kernel.org/lkml/20241122-sysfs-const-bin_attr-bpf-v1-1-823aea399...@weissschuh.net/ --- kernel/bpf/sysfs_btf.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c index fedb54c94cdb830a4890d33677dcc5a6e236c13f..81d6cf90584a7157929c50f62a5c6862e7a3d081 100644 --- a/kernel/bpf/sysfs_btf.c +++ b/kernel/bpf/sysfs_btf.c @@ -12,24 +12,16 @@ extern char __start_BTF[]; extern char __stop_BTF[]; -static ssize_t -btf_vmlinux_read(struct file *file, struct kobject *kobj, -struct bin_attribute *bin_attr, -char *buf, loff_t off, size_t len) -{ - memcpy(buf, __start_BTF + off, len); - return len; -} - static struct bin_attribute bin_attr_btf_vmlinux __ro_after_init = { .attr = { .name = "vmlinux", .mode = 0444, }, - .read = btf_vmlinux_read, + .read_new = sysfs_bin_attr_simple_read, }; struct kobject *btf_kobj; static int __init btf_vmlinux_init(void) { + bin_attr_btf_vmlinux.private = __start_BTF; bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF; if (bin_attr_btf_vmlinux.size == 0) -- 2.47.1
[PATCH 0/4] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
Most users use this function through the BIN_ATTR_SIMPLE* macros, they can handle the switch transparently. This series is meant to be merged through the driver core tree. Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (4): sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read() platform/x86: wmi-bmof: Switch to sysfs_bin_attr_simple_read() btf: Switch vmlinux BTF attribute to sysfs_bin_attr_simple_read() btf: Switch module BTF attribute to sysfs_bin_attr_simple_read() arch/powerpc/platforms/powernv/opal.c | 2 +- drivers/platform/x86/wmi-bmof.c | 12 ++-- fs/sysfs/file.c | 2 +- include/linux/sysfs.h | 4 ++-- kernel/bpf/btf.c | 15 ++- kernel/bpf/sysfs_btf.c| 12 ++-- kernel/module/sysfs.c | 2 +- 7 files changed, 11 insertions(+), 38 deletions(-) --- base-commit: feffde684ac29a3b7aec82d2df850fbdbdee55e4 change-id: 20241122-sysfs-const-bin_attr-simple-7c0ddb2fcf12 Best regards, -- Thomas Weißschuh
[PATCH 4/4] btf: Switch module BTF attribute to sysfs_bin_attr_simple_read()
The generic function from the sysfs core can replace the custom one. Signed-off-by: Thomas Weißschuh --- kernel/bpf/btf.c | 15 ++- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index e7a59e6462a9331d0acb17a88a4ebf641509c050..69caa86ae6085dce17e95107c4497d2d8cf81544 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -7870,17 +7870,6 @@ struct btf_module { static LIST_HEAD(btf_modules); static DEFINE_MUTEX(btf_module_mutex); -static ssize_t -btf_module_read(struct file *file, struct kobject *kobj, - struct bin_attribute *bin_attr, - char *buf, loff_t off, size_t len) -{ - const struct btf *btf = bin_attr->private; - - memcpy(buf, btf->data + off, len); - return len; -} - static void purge_cand_cache(struct btf *btf); static int btf_module_notify(struct notifier_block *nb, unsigned long op, @@ -7941,8 +7930,8 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op, attr->attr.name = btf->name; attr->attr.mode = 0444; attr->size = btf->data_size; - attr->private = btf; - attr->read = btf_module_read; + attr->private = btf->data; + attr->read_new = sysfs_bin_attr_simple_read; err = sysfs_create_bin_file(btf_kobj, attr); if (err) { -- 2.47.1
[PATCH 2/4] platform/x86: wmi-bmof: Switch to sysfs_bin_attr_simple_read()
The generic function from the sysfs core can replace the custom one. Signed-off-by: Thomas Weißschuh --- drivers/platform/x86/wmi-bmof.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c index df6f0ae6e6c7904f97c125297a21166f56d0b1f0..e6c217d70086a2896dc70cf8ac1c27dafb501a95 100644 --- a/drivers/platform/x86/wmi-bmof.c +++ b/drivers/platform/x86/wmi-bmof.c @@ -25,15 +25,6 @@ struct bmof_priv { struct bin_attribute bmof_bin_attr; }; -static ssize_t read_bmof(struct file *filp, struct kobject *kobj, struct bin_attribute *attr, -char *buf, loff_t off, size_t count) -{ - struct bmof_priv *priv = container_of(attr, struct bmof_priv, bmof_bin_attr); - - return memory_read_from_buffer(buf, count, &off, priv->bmofdata->buffer.pointer, - priv->bmofdata->buffer.length); -} - static int wmi_bmof_probe(struct wmi_device *wdev, const void *context) { struct bmof_priv *priv; @@ -60,7 +51,8 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const void *context) sysfs_bin_attr_init(&priv->bmof_bin_attr); priv->bmof_bin_attr.attr.name = "bmof"; priv->bmof_bin_attr.attr.mode = 0400; - priv->bmof_bin_attr.read = read_bmof; + priv->bmof_bin_attr.read_new = sysfs_bin_attr_simple_read; + priv->bmof_bin_attr.private = priv->bmofdata->buffer.pointer; priv->bmof_bin_attr.size = priv->bmofdata->buffer.length; ret = device_create_bin_file(&wdev->dev, &priv->bmof_bin_attr); -- 2.47.1
[PATCH 1/4] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
Most users use this function through the BIN_ATTR_SIMPLE* macros, they can handle the switch transparently. Also adapt the two non-macro users in the same change. Signed-off-by: Thomas Weißschuh --- arch/powerpc/platforms/powernv/opal.c | 2 +- fs/sysfs/file.c | 2 +- include/linux/sysfs.h | 4 ++-- kernel/module/sysfs.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c index 5d0f35bb917ebced8c741cd3af2c511949a1d2ef..013637e2b2a8e6a4ec6b93a520f8d5d9d3245467 100644 --- a/arch/powerpc/platforms/powernv/opal.c +++ b/arch/powerpc/platforms/powernv/opal.c @@ -818,7 +818,7 @@ static int opal_add_one_export(struct kobject *parent, const char *export_name, sysfs_bin_attr_init(attr); attr->attr.name = name; attr->attr.mode = 0400; - attr->read = sysfs_bin_attr_simple_read; + attr->read_new = sysfs_bin_attr_simple_read; attr->private = __va(vals[0]); attr->size = vals[1]; diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 785408861c01c89fc84c787848243a13c1338367..6931308876c4ac3b4c19878d5e1158ad8fe4f16f 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -817,7 +817,7 @@ EXPORT_SYMBOL_GPL(sysfs_emit_at); * Returns number of bytes written to @buf. */ ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { memcpy(buf, attr->private + off, count); diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 0f2fcd244523f050c5286f19d4fe1846506f9214..2205561159afdb57d0a250bb0439b28c01d9010e 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -511,7 +511,7 @@ __printf(3, 4) int sysfs_emit_at(char *buf, int at, const char *fmt, ...); ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count); #else /* CONFIG_SYSFS */ @@ -774,7 +774,7 @@ static inline int sysfs_emit_at(char *buf, int at, const char *fmt, ...) static inline ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj, -struct bin_attribute *attr, +const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 456358e1fdc43e6b5b24f383bbefa37812971174..254017b58b645d4afcf6876d29bcc2e2113a8dc4 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -196,7 +196,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) nattr->attr.mode = 0444; nattr->size = info->sechdrs[i].sh_size; nattr->private = (void *)info->sechdrs[i].sh_addr; - nattr->read = sysfs_bin_attr_simple_read; + nattr->read_new = sysfs_bin_attr_simple_read; ++nattr; } ++loaded; -- 2.47.1
Re: [PATCH] selftests/nolibc: start qemu with 1 GiB of memory
On 2025-01-06 10:51:52-0700, Shuah Khan wrote: > On 10/7/24 02:10, Thomas Weißschuh wrote: > > Recently the loongarch defconfig stopped working with the default 128 MiB > > of memory. The VM just spins infinitively. > > Increasing the available memory to 1 GiB, similar to s390, fixes the > > issue. To avoid having to do this for each architecture on its own, > > proactively apply to all architectures. > > > > Signed-off-by: Thomas Weißschuh > > --- > > tools/testing/selftests/nolibc/Makefile | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/nolibc/Makefile > > b/tools/testing/selftests/nolibc/Makefile > > index > > 8de98ea7af8071caa0597aa7b86d91a2d1d50e68..e92e0b88586111072a0e043cb15f3b59cf42c3a6 > > 100644 > > --- a/tools/testing/selftests/nolibc/Makefile > > +++ b/tools/testing/selftests/nolibc/Makefile > > @@ -130,9 +130,9 @@ QEMU_ARGS_ppc= -M g3beige -append > > "console=ttyS0 panic=-1 $(TEST:%=NOLIB > > QEMU_ARGS_ppc64 = -M powernv -append "console=hvc0 panic=-1 > > $(TEST:%=NOLIBC_TEST=%)" > > QEMU_ARGS_ppc64le= -M powernv -append "console=hvc0 panic=-1 > > $(TEST:%=NOLIBC_TEST=%)" > > QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 > > $(TEST:%=NOLIBC_TEST=%)" > > -QEMU_ARGS_s390 = -M s390-ccw-virtio -m 1G -append "console=ttyS0 > > panic=-1 $(TEST:%=NOLIBC_TEST=%)" > > +QEMU_ARGS_s390 = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 > > $(TEST:%=NOLIBC_TEST=%)" > > QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 > > $(TEST:%=NOLIBC_TEST=%)" > > -QEMU_ARGS= $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_BIOS) > > $(QEMU_ARGS_EXTRA) > > +QEMU_ARGS= -m 1G $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_BIOS) > > $(QEMU_ARGS_EXTRA) > > # OUTPUT is only set when run from the main makefile, otherwise > > # it defaults to this nolibc directory. > > > > --- > > base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b > > change-id: 20241007-nolibc-qemu-mem-5ed605520472 > > > > Best regards, > > Did we take care of this one? Apologies if I didn't. We can include > this in 6.14 pr. This is already in mainline. Thomas
Re: [PATCH v2 0/3] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
On 2024-12-30 16:50:41-0800, Alexei Starovoitov wrote: > On Sat, Dec 28, 2024 at 12:43 AM Thomas Weißschuh > wrote: > > > > Most users use this function through the BIN_ATTR_SIMPLE* macros, > > they can handle the switch transparently. > > > > This series is meant to be merged through the driver core tree. > > hmm. why? Patch 1 changes the signature of sysfs_bin_attr_simple_read(). Before patch 1 sysfs_bin_attr_simple_read() needs to be assigned to the callback member .read, after patch 1 it's .read_new. (Both callbacks work exactly the same, except for their signature, .read_new is only a transition mechanism and will go away again) > I'd rather take patches 2 and 3 into bpf-next to avoid > potential conflicts. > Patch 1 looks orthogonal and independent. If you pick up 2 and 3 through bpf-next you would need to adapt these assignments. As soon as both patch 1 and the modified 2 and 3 hit Linus' tree, the build would break due to mismatches function pointers. (Casting function pointers to avoid the mismatch will blow up with KCFI) Of course Linus can fix this up easily, but it somebody would need to keep track of it and I wanted to avoid manual intervention. Or we spread out both parts over two development cycles; maybe Greg can even pick up patch 1 late in the 6.13 cycle. Personally I am fine with any approach.
[PATCH 3/3] selftests/mm: virtual_address_range: Dump to /dev/null
During the execution of validate_complete_va_space() a lot of memory is on the VM subsystem. When running on a low memory subsystem an OOM may be triggered, when writing to the dump file as the filesystem may also require memory. On my test system with 1100MiB physical memory: Tasks state (memory values in pages): [ pid ] uid tgid total_vm rss rss_anon rss_file rss_shmem pgtables_bytes swapents oom_score_adj name [ 57] 057 34359215953 695 2560 439 10643906560 0 virtual_address Out of memory: Killed process 57 (virtual_address) total-vm:137436863812kB, anon-rss:1024kB, file-rss:0kB, shmem-rss:1756kB, UID:0 pgtables:1039444kB oom_score_adj:0 fault_in_iov_iter_readable+0x4a/0xd0 generic_perform_write+0x9c/0x280 shmem_file_write_iter+0x86/0x90 vfs_write+0x29c/0x480 ksys_write+0x6c/0xe0 do_syscall_64+0x9e/0x1a0 entry_SYSCALL_64_after_hwframe+0x77/0x7f Write the dumped data into /dev/null instead which does not require additional memory during write(), making the code simpler as a side-effect. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/mm/virtual_address_range.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c index 484f82c7b7c871f82a7d9ec6d6c649f2ab1eb0cd..4042fd878acd702d23da2c3293292de33bd48143 100644 --- a/tools/testing/selftests/mm/virtual_address_range.c +++ b/tools/testing/selftests/mm/virtual_address_range.c @@ -103,10 +103,9 @@ static int validate_complete_va_space(void) FILE *file; int fd; - fd = open("va_dump", O_CREAT | O_WRONLY, 0600); - unlink("va_dump"); + fd = open("/dev/null", O_WRONLY); if (fd < 0) { - ksft_test_result_skip("cannot create or open dump file\n"); + ksft_test_result_skip("cannot create or open /dev/null\n"); ksft_finished(); } @@ -152,7 +151,6 @@ static int validate_complete_va_space(void) while (start_addr + hop < end_addr) { if (write(fd, (void *)(start_addr + hop), 1) != 1) return 1; - lseek(fd, 0, SEEK_SET); hop += MAP_CHUNK_SIZE; } -- 2.47.1
[PATCH 0/3] selftests/mm: virtual_address_range: Two bugfixes and a cleanup
The selftest started failing since commit e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") was merged. While debugging I stumbled upon another bug and potential cleanup. Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (3): selftests/mm: virtual_address_range: Fix error when CommitLimit < 1GiB selftests/mm: virtual_address_range: Avoid reading VVAR mappings selftests/mm: virtual_address_range: Dump to /dev/null tools/testing/selftests/mm/virtual_address_range.c | 21 +++-- 1 file changed, 15 insertions(+), 6 deletions(-) --- base-commit: fbfd64d25c7af3b8695201ebc85efe90be28c5a3 change-id: 20250107-virtual_address_range-tests-95843766fa97 Best regards, -- Thomas Weißschuh
[PATCH 1/3] selftests/mm: virtual_address_range: Fix error when CommitLimit < 1GiB
If not enough physical memory is available the kernel may fail mmap(); see __vm_enough_memory() and vm_commit_limit(). In that case the logic in validate_complete_va_space() does not make sense and will even incorrectly fail. Instead skip the test if no mmap() succeeded. Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance on correctness of mmap()") Cc: sta...@vger.kernel.org Signed-off-by: Thomas Weißschuh --- The logic in __vm_enough_memory() seems weird. It describes itself as "Check that a process has enough memory to allocate a new virtual mapping", however it never checks the current memory usage of the process. So it only disallows large mappings. But many small mappings taking the same amount of memory are allowed; and then even automatically merged into one big mapping. --- tools/testing/selftests/mm/virtual_address_range.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c index 2a2b69e91950a37999f606847c9c8328d79890c2..d7bf8094d8bcd4bc96e2db4dc3fcb41968def859 100644 --- a/tools/testing/selftests/mm/virtual_address_range.c +++ b/tools/testing/selftests/mm/virtual_address_range.c @@ -178,6 +178,12 @@ int main(int argc, char *argv[]) validate_addr(ptr[i], 0); } lchunks = i; + + if (!lchunks) { + ksft_test_result_skip("Not enough memory for a single chunk\n"); + ksft_finished(); + } + hptr = (char **) calloc(NR_CHUNKS_HIGH, sizeof(char *)); if (hptr == NULL) { ksft_test_result_skip("Memory constraint not fulfilled\n"); -- 2.47.1
[PATCH 2/3] selftests/mm: virtual_address_range: Avoid reading VVAR mappings
The virtual_address_range selftest reads from the start of each mapping listed in /proc/self/maps. However not all mappings are valid to be arbitrarily accessed. For example the vvar data used for virtual clocks on x86 can only be accessed if 1) the kernel configuration enables virtual clocks and 2) the hypervisor provided the data for it, which can only determined by the VDSO code itself. Since commit e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") the virtual clock data was split out into its own mapping, triggering faulting accesses by virtual_address_range. Skip the various vvar mappings in virtual_address_range to avoid errors. Fixes: e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance on correctness of mmap()") Cc: sta...@vger.kernel.org Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-lkp/202412271148.2656e485-...@intel.com Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/mm/virtual_address_range.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c index d7bf8094d8bcd4bc96e2db4dc3fcb41968def859..484f82c7b7c871f82a7d9ec6d6c649f2ab1eb0cd 100644 --- a/tools/testing/selftests/mm/virtual_address_range.c +++ b/tools/testing/selftests/mm/virtual_address_range.c @@ -116,10 +116,11 @@ static int validate_complete_va_space(void) prev_end_addr = 0; while (fgets(line, sizeof(line), file)) { + int path_offset = 0; unsigned long hop; - if (sscanf(line, "%lx-%lx %s[rwxp-]", - &start_addr, &end_addr, prot) != 3) + if (sscanf(line, "%lx-%lx %4s %*s %*s %*s %n", + &start_addr, &end_addr, prot, &path_offset) != 3) ksft_exit_fail_msg("cannot parse /proc/self/maps\n"); /* end of userspace mappings; ignore vsyscall mapping */ @@ -135,6 +136,10 @@ static int validate_complete_va_space(void) if (prot[0] != 'r') continue; + /* Only the VDSO can know if a VVAR mapping is really readable */ + if (path_offset && !strncmp(line + path_offset, "[vvar", 5)) + continue; + /* * Confirm whether MAP_CHUNK_SIZE chunk can be found or not. * If write succeeds, no need to check MAP_CHUNK_SIZE - 1 -- 2.47.1
Re: [PATCH 3/3] selftests/mm: virtual_address_range: Dump to /dev/null
On Wed, Jan 08, 2025 at 11:39:40AM +0530, Dev Jain wrote: > > On 07/01/25 8:44 pm, Thomas Weißschuh wrote: > > During the execution of validate_complete_va_space() a lot of memory is > > on the VM subsystem. When running on a low memory subsystem an OOM may > > be triggered, when writing to the dump file as the filesystem may also > > require memory. > > > > On my test system with 1100MiB physical memory: > > > > Tasks state (memory values in pages): > > [ pid ] uid tgid total_vm rss rss_anon rss_file rss_shmem > > pgtables_bytes swapents oom_score_adj name > > [ 57] 057 34359215953 695 2560 439 > > 10643906560 0 virtual_address > > > > Out of memory: Killed process 57 (virtual_address) > > total-vm:137436863812kB, anon-rss:1024kB, file-rss:0kB, shmem-rss:1756kB, > > UID:0 pgtables:1039444kB oom_score_adj:0 > > > > fault_in_iov_iter_readable+0x4a/0xd0 > > generic_perform_write+0x9c/0x280 > > shmem_file_write_iter+0x86/0x90 > > vfs_write+0x29c/0x480 > > ksys_write+0x6c/0xe0 > > do_syscall_64+0x9e/0x1a0 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > Write the dumped data into /dev/null instead which does not require > > additional memory during write(), making the code simpler as a > > side-effect. > > > > Signed-off-by: Thomas Weißschuh > > --- > > tools/testing/selftests/mm/virtual_address_range.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/tools/testing/selftests/mm/virtual_address_range.c > > b/tools/testing/selftests/mm/virtual_address_range.c > > index > > 484f82c7b7c871f82a7d9ec6d6c649f2ab1eb0cd..4042fd878acd702d23da2c3293292de33bd48143 > > 100644 > > --- a/tools/testing/selftests/mm/virtual_address_range.c > > +++ b/tools/testing/selftests/mm/virtual_address_range.c > > @@ -103,10 +103,9 @@ static int validate_complete_va_space(void) > > FILE *file; > > int fd; > > - fd = open("va_dump", O_CREAT | O_WRONLY, 0600); > > - unlink("va_dump"); > > + fd = open("/dev/null", O_WRONLY); > > if (fd < 0) { > > - ksft_test_result_skip("cannot create or open dump file\n"); > > + ksft_test_result_skip("cannot create or open /dev/null\n"); > > ksft_finished(); > > } > > @@ -152,7 +151,6 @@ static int validate_complete_va_space(void) > > while (start_addr + hop < end_addr) { > > if (write(fd, (void *)(start_addr + hop), 1) != 1) > > return 1; > > - lseek(fd, 0, SEEK_SET); > > hop += MAP_CHUNK_SIZE; > > } > > > > The reason I had not used /dev/null was that write() was succeeding to > /dev/null > even from an address not in my VA space. I was puzzled about this behaviour of > /dev/null and I chose to ignore it and just use a real file. That makes sense and I can reproduce your example. Switching to another dummy file which reads the written data like /dev/random also leads to OOM, so wouldn't help either. Thanks for the explanation. @Andrew, could you drop this patch? > To test this behaviour, run the following program: [..] PS: Your mail contained HTML and did not make it to the list archives. (And the text variant of the example program was corrupted)
Re: [PATCH 1/3] selftests/mm: virtual_address_range: Fix error when CommitLimit < 1GiB
On Wed, Jan 08, 2025 at 11:46:19AM +0530, Dev Jain wrote: > > On 07/01/25 8:44 pm, Thomas Weißschuh wrote: > > If not enough physical memory is available the kernel may fail mmap(); > > see __vm_enough_memory() and vm_commit_limit(). > > In that case the logic in validate_complete_va_space() does not make > > sense and will even incorrectly fail. > > Instead skip the test if no mmap() succeeded. > > > > Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance > > on correctness of mmap()") > > Cc: sta...@vger.kernel.org > > Signed-off-by: Thomas Weißschuh > > > > --- > > The logic in __vm_enough_memory() seems weird. > > It describes itself as "Check that a process has enough memory to > > allocate a new virtual mapping", however it never checks the current > > memory usage of the process. > > So it only disallows large mappings. But many small mappings taking the > > same amount of memory are allowed; and then even automatically merged > > into one big mapping. > > --- > > tools/testing/selftests/mm/virtual_address_range.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/tools/testing/selftests/mm/virtual_address_range.c > > b/tools/testing/selftests/mm/virtual_address_range.c > > index > > 2a2b69e91950a37999f606847c9c8328d79890c2..d7bf8094d8bcd4bc96e2db4dc3fcb41968def859 > > 100644 > > --- a/tools/testing/selftests/mm/virtual_address_range.c > > +++ b/tools/testing/selftests/mm/virtual_address_range.c > > @@ -178,6 +178,12 @@ int main(int argc, char *argv[]) > > validate_addr(ptr[i], 0); > > } > > lchunks = i; > > + > > + if (!lchunks) { > > + ksft_test_result_skip("Not enough memory for a single chunk\n"); > > + ksft_finished(); > > + } > > + > > hptr = (char **) calloc(NR_CHUNKS_HIGH, sizeof(char *)); > > if (hptr == NULL) { > > ksft_test_result_skip("Memory constraint not fulfilled\n"); > > > > I do not know about __vm_enough_memory(), but I am going by your description: > You say that the kernel may fail mmap() when enough physical memory is not > there, but it may happen that we have already done 100 mmap()'s, and then > the kernel fails mmap(), so if (!lchunks) won't be able to handle this case. > Basically, lchunks == 0 is not a complete indicator of kernel failing mmap(). __vm_enough_memory() only checks the size of each single mmap() on its own. It does not actually check the current memory or address space usage of the process. This seems a bit weird, as indicated in my after-the-fold explanation. > The basic assumption of the test is that any process should be able to exhaust > its virtual address space, and running the test under memory pressure and the > kernel violating this behaviour defeats the point of the test I think? The assumption is correct, as soon as one mapping succeeds the others will also succeed, until the actual address space is exhausted. Looking at it again, __vm_enough_memory() is only called for writable mappings, so it would be possible to use only readable mappings in the test. The test will still fail with OOM, as the many PTEs need more than 1GiB of physical memory anyways, but at least that produces a usable error message. However I'm not sure if this would violate other test assumptions.
[PATCH v2 3/3] btf: Switch module BTF attribute to sysfs_bin_attr_simple_read()
The generic function from the sysfs core can replace the custom one. Signed-off-by: Thomas Weißschuh Acked-by: Andrii Nakryiko --- kernel/bpf/btf.c | 15 ++- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index e5a5f023cedd5c288c2774218862c69db469917f..76de36d2552002d1e25c826d701340d9cde07ba1 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -8011,17 +8011,6 @@ struct btf_module { static LIST_HEAD(btf_modules); static DEFINE_MUTEX(btf_module_mutex); -static ssize_t -btf_module_read(struct file *file, struct kobject *kobj, - struct bin_attribute *bin_attr, - char *buf, loff_t off, size_t len) -{ - const struct btf *btf = bin_attr->private; - - memcpy(buf, btf->data + off, len); - return len; -} - static void purge_cand_cache(struct btf *btf); static int btf_module_notify(struct notifier_block *nb, unsigned long op, @@ -8082,8 +8071,8 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op, attr->attr.name = btf->name; attr->attr.mode = 0444; attr->size = btf->data_size; - attr->private = btf; - attr->read = btf_module_read; + attr->private = btf->data; + attr->read_new = sysfs_bin_attr_simple_read; err = sysfs_create_bin_file(btf_kobj, attr); if (err) { -- 2.47.1
[PATCH v2 2/3] btf: Switch vmlinux BTF attribute to sysfs_bin_attr_simple_read()
The generic function from the sysfs core can replace the custom one. Signed-off-by: Thomas Weißschuh Acked-by: Andrii Nakryiko --- This is a replacement for [0], as Alexei was not happy about BIN_ATTR_SIMPLE_RO() [0] https://lore.kernel.org/lkml/20241122-sysfs-const-bin_attr-bpf-v1-1-823aea399...@weissschuh.net/ --- kernel/bpf/sysfs_btf.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c index fedb54c94cdb830a4890d33677dcc5a6e236c13f..81d6cf90584a7157929c50f62a5c6862e7a3d081 100644 --- a/kernel/bpf/sysfs_btf.c +++ b/kernel/bpf/sysfs_btf.c @@ -12,24 +12,16 @@ extern char __start_BTF[]; extern char __stop_BTF[]; -static ssize_t -btf_vmlinux_read(struct file *file, struct kobject *kobj, -struct bin_attribute *bin_attr, -char *buf, loff_t off, size_t len) -{ - memcpy(buf, __start_BTF + off, len); - return len; -} - static struct bin_attribute bin_attr_btf_vmlinux __ro_after_init = { .attr = { .name = "vmlinux", .mode = 0444, }, - .read = btf_vmlinux_read, + .read_new = sysfs_bin_attr_simple_read, }; struct kobject *btf_kobj; static int __init btf_vmlinux_init(void) { + bin_attr_btf_vmlinux.private = __start_BTF; bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF; if (bin_attr_btf_vmlinux.size == 0) -- 2.47.1
[PATCH v2 0/3] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
Most users use this function through the BIN_ATTR_SIMPLE* macros, they can handle the switch transparently. This series is meant to be merged through the driver core tree. Signed-off-by: Thomas Weißschuh --- Changes in v2: - Rebase on torvalds/master - Drop wmi-bmof patch - Pick up Acks from Andrii - Link to v1: https://lore.kernel.org/r/20241205-sysfs-const-bin_attr-simple-v1-0-4a4e4ced7...@weissschuh.net --- Thomas Weißschuh (3): sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read() btf: Switch vmlinux BTF attribute to sysfs_bin_attr_simple_read() btf: Switch module BTF attribute to sysfs_bin_attr_simple_read() arch/powerpc/platforms/powernv/opal.c | 2 +- fs/sysfs/file.c | 2 +- include/linux/sysfs.h | 4 ++-- kernel/bpf/btf.c | 15 ++- kernel/bpf/sysfs_btf.c| 12 ++-- kernel/module/sysfs.c | 2 +- 6 files changed, 9 insertions(+), 28 deletions(-) --- base-commit: d6ef8b40d075c425f548002d2f35ae3f06e9cf96 change-id: 20241122-sysfs-const-bin_attr-simple-7c0ddb2fcf12 Best regards, -- Thomas Weißschuh
[PATCH v2 1/3] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
Most users use this function through the BIN_ATTR_SIMPLE* macros, they can handle the switch transparently. Also adapt the two non-macro users in the same change. Signed-off-by: Thomas Weißschuh --- arch/powerpc/platforms/powernv/opal.c | 2 +- fs/sysfs/file.c | 2 +- include/linux/sysfs.h | 4 ++-- kernel/module/sysfs.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c index 5d0f35bb917ebced8c741cd3af2c511949a1d2ef..013637e2b2a8e6a4ec6b93a520f8d5d9d3245467 100644 --- a/arch/powerpc/platforms/powernv/opal.c +++ b/arch/powerpc/platforms/powernv/opal.c @@ -818,7 +818,7 @@ static int opal_add_one_export(struct kobject *parent, const char *export_name, sysfs_bin_attr_init(attr); attr->attr.name = name; attr->attr.mode = 0400; - attr->read = sysfs_bin_attr_simple_read; + attr->read_new = sysfs_bin_attr_simple_read; attr->private = __va(vals[0]); attr->size = vals[1]; diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 785408861c01c89fc84c787848243a13c1338367..6931308876c4ac3b4c19878d5e1158ad8fe4f16f 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -817,7 +817,7 @@ EXPORT_SYMBOL_GPL(sysfs_emit_at); * Returns number of bytes written to @buf. */ ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { memcpy(buf, attr->private + off, count); diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 0f2fcd244523f050c5286f19d4fe1846506f9214..2205561159afdb57d0a250bb0439b28c01d9010e 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -511,7 +511,7 @@ __printf(3, 4) int sysfs_emit_at(char *buf, int at, const char *fmt, ...); ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count); #else /* CONFIG_SYSFS */ @@ -774,7 +774,7 @@ static inline int sysfs_emit_at(char *buf, int at, const char *fmt, ...) static inline ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj, -struct bin_attribute *attr, +const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 456358e1fdc43e6b5b24f383bbefa37812971174..254017b58b645d4afcf6876d29bcc2e2113a8dc4 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -196,7 +196,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) nattr->attr.mode = 0444; nattr->size = info->sechdrs[i].sh_size; nattr->private = (void *)info->sechdrs[i].sh_addr; - nattr->read = sysfs_bin_attr_simple_read; + nattr->read_new = sysfs_bin_attr_simple_read; ++nattr; } ++loaded; -- 2.47.1
[PATCH 04/16] elf, uapi: Add definitions for VER_FLG_BASE and VER_FLG_WEAK
The definitions are used by tools/testing/selftests/vDSO/parse_vdso.c. To be able to build the vDSO selftests without a libc dependency, add the definitions to the kernels own UAPI headers. Link: https://docs.oracle.com/cd/E19683-01/816-1386/chapter6-80869/index.html Signed-off-by: Thomas Weißschuh --- include/uapi/linux/elf.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index c5383cc7bb13c931fea083de5243c4006f795006..d040f12ff1c0ae3dde5c371c81d6089118fbe8ed 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -136,6 +136,9 @@ typedef __s64 Elf64_Sxword; #define STT_COMMON 5 #define STT_TLS 6 +#define VER_FLG_BASE 0x1 +#define VER_FLG_WEAK 0x2 + #define ELF_ST_BIND(x) ((x) >> 4) #define ELF_ST_TYPE(x) ((x) & 0xf) #define ELF32_ST_BIND(x) ELF_ST_BIND(x) -- 2.48.1
[PATCH 00/16] selftests: vDSO: parse_vdso: Make compatible with nolibc
For testing the functionality of the vDSO, it is necessary to build userspace programs for multiple different architectures. It is additional work to acquire matching userspace cross-compilers with full C libraries and then building root images out of those. The kernel tree already contains nolibc, a small, header-only C library. By using it, it is possible to build userspace programs without any additional dependencies. For example the kernel.org crosstools or multi-target clang can be used to build test programs for a multitude of architectures. While nolibc is very limited, it is enough for many selftests. With some minor adjustments it is possible to make parse_vdso.c compatible with nolibc. As an example, vdso_standalone_test_x86 is now built from the same C code as the regular vdso_test_gettimeofday, while still being completely standalone. This should probably go through the kselftest tree. Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (16): MAINTAINERS: Add vDSO selftests elf, uapi: Add definition for STN_UNDEF elf, uapi: Add definition for DT_GNU_HASH elf, uapi: Add definitions for VER_FLG_BASE and VER_FLG_WEAK elf, uapi: Add type ElfXX_Versym elf, uapi: Add types ElfXX_Verdef and ElfXX_Veraux tools/include: Add uapi/linux/elf.h selftests: Add headers target selftests: vDSO: vdso_standalone_test_x86: Use vdso_init_form_sysinfo_ehdr selftests: vDSO: parse_vdso: Drop vdso_init_from_auxv() selftests: vDSO: parse_vdso: Use UAPI headers instead of libc headers selftests: vDSO: parse_vdso: Test __SIZEOF_LONG__ instead of ULONG_MAX selftests: vDSO: parse_vdso: Make compatible with nolibc selftests: vDSO: vdso_test_gettimeofday: Clean up includes selftests: vDSO: vdso_test_gettimeofday: Make compatible with nolibc selftests: vDSO: vdso_standalone_test_x86: Switch to nolibc MAINTAINERS| 1 + include/uapi/linux/elf.h | 38 ++ tools/include/uapi/linux/elf.h | 524 + tools/testing/selftests/lib.mk | 5 +- tools/testing/selftests/vDSO/Makefile | 11 +- tools/testing/selftests/vDSO/parse_vdso.c | 21 +- tools/testing/selftests/vDSO/parse_vdso.h | 1 - .../selftests/vDSO/vdso_standalone_test_x86.c | 143 +- .../selftests/vDSO/vdso_test_gettimeofday.c| 4 +- 9 files changed, 584 insertions(+), 164 deletions(-) --- base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b change-id: 20241017-parse_vdso-nolibc-e069baa7ff48 Best regards, -- Thomas Weißschuh
[PATCH 01/16] MAINTAINERS: Add vDSO selftests
These currently have no maintainer besides the default kselftest ones. Add the general vDSO maintainers, too. Signed-off-by: Thomas Weißschuh --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 896a307fa06545e2861abe46ea7029f9b4d3628e..959c8a86844eb1e5c6218e8fdbde6c3ebf68e25d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9777,6 +9777,7 @@ F:include/asm-generic/vdso/vsyscall.h F: include/vdso/ F: kernel/time/vsyscall.c F: lib/vdso/ +F: tools/testing/selftests/vDSO/ GENWQE (IBM Generic Workqueue Card) M: Frank Haverkamp -- 2.48.1
[PATCH 02/16] elf, uapi: Add definition for STN_UNDEF
The definition is used by tools/testing/selftests/vDSO/parse_vdso.c. To be able to build the vDSO selftests without a libc dependency, add the definition to the kernels own UAPI headers. Link: https://refspecs.linuxfoundation.org/elf/gabi4+/ch4.symtab.html Signed-off-by: Thomas Weißschuh --- include/uapi/linux/elf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index b44069d29cecc0f9de90ee66bfffd2137f4275a8..448695c7364042b10682acc8223eb6053ad039dd 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -125,6 +125,8 @@ typedef __s64 Elf64_Sxword; #define STB_GLOBAL 1 #define STB_WEAK 2 +#define STN_UNDEF 0 + #define STT_NOTYPE 0 #define STT_OBJECT 1 #define STT_FUNC2 -- 2.48.1
[PATCH 03/16] elf, uapi: Add definition for DT_GNU_HASH
The definition is used by tools/testing/selftests/vDSO/parse_vdso.c. To be able to build the vDSO selftests without a libc dependency, add the define to the kernels own UAPI headers. Link: https://refspecs.linuxbase.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/libc-ddefs.html Signed-off-by: Thomas Weißschuh --- include/uapi/linux/elf.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index 448695c7364042b10682acc8223eb6053ad039dd..c5383cc7bb13c931fea083de5243c4006f795006 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -107,6 +107,7 @@ typedef __s64 Elf64_Sxword; #define DT_VALRNGLO0x6d00 #define DT_VALRNGHI0x6dff #define DT_ADDRRNGLO 0x6e00 +#define DT_GNU_HASH0x6ef5 #define DT_ADDRRNGHI 0x6eff #define DT_VERSYM 0x6ff0 #define DT_RELACOUNT 0x6ff9 -- 2.48.1
[PATCH 07/16] tools/include: Add uapi/linux/elf.h
It will be used by the vDSO selftests. Signed-off-by: Thomas Weißschuh --- tools/include/uapi/linux/elf.h | 524 + 1 file changed, 524 insertions(+) diff --git a/tools/include/uapi/linux/elf.h b/tools/include/uapi/linux/elf.h new file mode 100644 index ..3d0935a11613bbcf8c381188de3a10bafd6cc29f --- /dev/null +++ b/tools/include/uapi/linux/elf.h @@ -0,0 +1,524 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef _LINUX_ELF_H +#define _LINUX_ELF_H + +#include +#include + +/* 32-bit ELF base types. */ +typedef __u32 Elf32_Addr; +typedef __u16 Elf32_Half; +typedef __u32 Elf32_Off; +typedef __s32 Elf32_Sword; +typedef __u32 Elf32_Word; +typedef __u16 Elf32_Versym; + +/* 64-bit ELF base types. */ +typedef __u64 Elf64_Addr; +typedef __u16 Elf64_Half; +typedef __s16 Elf64_SHalf; +typedef __u64 Elf64_Off; +typedef __s32 Elf64_Sword; +typedef __u32 Elf64_Word; +typedef __u64 Elf64_Xword; +typedef __s64 Elf64_Sxword; +typedef __u16 Elf64_Versym; + +/* These constants are for the segment types stored in the image headers */ +#define PT_NULL0 +#define PT_LOAD1 +#define PT_DYNAMIC 2 +#define PT_INTERP 3 +#define PT_NOTE4 +#define PT_SHLIB 5 +#define PT_PHDR6 +#define PT_TLS 7 /* Thread local storage segment */ +#define PT_LOOS0x6000 /* OS-specific */ +#define PT_HIOS0x6fff /* OS-specific */ +#define PT_LOPROC 0x7000 +#define PT_HIPROC 0x7fff +#define PT_GNU_EH_FRAME(PT_LOOS + 0x474e550) +#define PT_GNU_STACK (PT_LOOS + 0x474e551) +#define PT_GNU_RELRO (PT_LOOS + 0x474e552) +#define PT_GNU_PROPERTY(PT_LOOS + 0x474e553) + + +/* ARM MTE memory tag segment type */ +#define PT_AARCH64_MEMTAG_MTE (PT_LOPROC + 0x2) + +/* + * Extended Numbering + * + * If the real number of program header table entries is larger than + * or equal to PN_XNUM(0x), it is set to sh_info field of the + * section header at index 0, and PN_XNUM is set to e_phnum + * field. Otherwise, the section header at index 0 is zero + * initialized, if it exists. + * + * Specifications are available in: + * + * - Oracle: Linker and Libraries. + * Part No: 817–1984–19, August 2011. + * https://docs.oracle.com/cd/E18752_01/pdf/817-1984.pdf + * + * - System V ABI AMD64 Architecture Processor Supplement + * Draft Version 0.99.4, + * January 13, 2010. + * http://www.cs.washington.edu/education/courses/cse351/12wi/supp-docs/abi.pdf + */ +#define PN_XNUM 0x + +/* These constants define the different elf file types */ +#define ET_NONE 0 +#define ET_REL1 +#define ET_EXEC 2 +#define ET_DYN3 +#define ET_CORE 4 +#define ET_LOPROC 0xff00 +#define ET_HIPROC 0x + +/* This is the info that is needed to parse the dynamic section of the file */ +#define DT_NULL0 +#define DT_NEEDED 1 +#define DT_PLTRELSZ2 +#define DT_PLTGOT 3 +#define DT_HASH4 +#define DT_STRTAB 5 +#define DT_SYMTAB 6 +#define DT_RELA7 +#define DT_RELASZ 8 +#define DT_RELAENT 9 +#define DT_STRSZ 10 +#define DT_SYMENT 11 +#define DT_INIT12 +#define DT_FINI13 +#define DT_SONAME 14 +#define DT_RPATH 15 +#define DT_SYMBOLIC16 +#define DT_REL 17 +#define DT_RELSZ 18 +#define DT_RELENT 19 +#define DT_PLTREL 20 +#define DT_DEBUG 21 +#define DT_TEXTREL 22 +#define DT_JMPREL 23 +#define DT_ENCODING32 +#define OLD_DT_LOOS0x6000 +#define DT_LOOS0x600d +#define DT_HIOS0x6000 +#define DT_VALRNGLO0x6d00 +#define DT_VALRNGHI0x6dff +#define DT_ADDRRNGLO 0x6e00 +#define DT_GNU_HASH0x6ef5 +#define DT_ADDRRNGHI 0x6eff +#define DT_VERSYM 0x6ff0 +#define DT_RELACOUNT 0x6ff9 +#define DT_RELCOUNT0x6ffa +#define DT_FLAGS_1 0x6ffb +#define DT_VERDEF 0x6ffc +#defineDT_VERDEFNUM0x6ffd +#define DT_VERNEED 0x6ffe +#defineDT_VERNEEDNUM 0x6fff +#define OLD_DT_HIOS 0x6fff +#define DT_LOPROC 0x7000 +#define DT_HIPROC 0x7fff + +/* This info is needed when parsing the symbol table */ +#define STB_LOCAL 0 +#define STB_GLOBAL 1 +#define STB_WEAK 2 + +#define STN_UNDEF 0 + +#define STT_NOTYPE 0 +#define STT_OBJECT 1 +#define STT_FUNC2 +#define STT_SECTION 3 +#define STT_FILE4 +#define STT_COMMON 5 +#define STT_TLS 6 + +#define VER_FLG_BASE 0x1 +#define VER_FLG_WEAK 0x2 + +#define ELF_ST_BIND(x) ((x) >> 4) +#define ELF_ST_TYPE(x) ((x) & 0xf) +#define ELF32_ST_BIND(x) ELF_ST_BIND(x) +#define ELF32_ST_TYPE(x) ELF_ST_TYPE(x) +#define ELF64_ST_BIND(x) ELF_ST_BIND(x) +#define ELF64_ST_TYPE(x) ELF_ST_TYPE(x) + +typedef struct { + Elf32_Sword d_tag; + union { +E
[PATCH 09/16] selftests: vDSO: vdso_standalone_test_x86: Use vdso_init_form_sysinfo_ehdr
vdso_standalone_test_x86 is the only user of vdso_init_from_auxv(). Instead of combining the parsing the aux vector with the parsing of the vDSO, split them apart into getauxval() and the regular vdso_init_from_sysinfo_ehdr(). The implementation of getauxval() is taken from tools/include/nolibc/stdlib.h. Signed-off-by: Thomas Weißschuh --- All of this code will be deleted later again. --- .../selftests/vDSO/vdso_standalone_test_x86.c | 27 +- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c index 644915862af8883131e5defd336f1bd80736fc0f..500608f89c66b5747e3d845ebc54e4c3a35b6ccd 100644 --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "parse_vdso.h" @@ -84,6 +85,30 @@ void to_base10(char *lastdig, time_t n) } } +unsigned long getauxval(const unsigned long *auxv, unsigned long type) +{ + unsigned long ret; + + if (!auxv) + return 0; + + while (1) { + if (!auxv[0] && !auxv[1]) { + ret = 0; + break; + } + + if (auxv[0] == type) { + ret = auxv[1]; + break; + } + + auxv += 2; + } + + return ret; +} + void c_main(void **stack) { /* Parse the stack */ @@ -96,7 +121,7 @@ void c_main(void **stack) stack++; /* Now we're pointing at auxv. Initialize the vDSO parser. */ - vdso_init_from_auxv((void *)stack); + vdso_init_from_sysinfo_ehdr(getauxval((unsigned long *)stack, AT_SYSINFO_EHDR)); /* Find gettimeofday. */ typedef long (*gtod_t)(struct timeval *tv, struct timezone *tz); -- 2.48.1
[PATCH 05/16] elf, uapi: Add type ElfXX_Versym
The types are used by tools/testing/selftests/vDSO/parse_vdso.c. To be able to build the vDSO selftests without a libc dependency, add the types to the kernels own UAPI headers. As documented by elf(5). Signed-off-by: Thomas Weißschuh --- include/uapi/linux/elf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index d040f12ff1c0ae3dde5c371c81d6089118fbe8ed..8846fe03ca5b836c96aad1be6d8fb9daf3d4b1d9 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -11,6 +11,7 @@ typedef __u16 Elf32_Half; typedef __u32 Elf32_Off; typedef __s32 Elf32_Sword; typedef __u32 Elf32_Word; +typedef __u16 Elf32_Versym; /* 64-bit ELF base types. */ typedef __u64 Elf64_Addr; @@ -21,6 +22,7 @@ typedef __s32 Elf64_Sword; typedef __u32 Elf64_Word; typedef __u64 Elf64_Xword; typedef __s64 Elf64_Sxword; +typedef __u16 Elf64_Versym; /* These constants are for the segment types stored in the image headers */ #define PT_NULL0 -- 2.48.1
[PATCH 11/16] selftests: vDSO: parse_vdso: Use UAPI headers instead of libc headers
To allow the usage of parse_vdso.c together with a limited libc like nolibc, use the kernels own elf.h and auxvec.h headers. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/vDSO/Makefile | 3 +++ tools/testing/selftests/vDSO/parse_vdso.c | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 1cf14a8da43803249f72fe1b09689c8834806986..bc8ca186fb877dc11740c37f1e07e45e84c2ae92 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -19,6 +19,9 @@ LDLIBS += -lgcc_s endif include ../lib.mk + +CFLAGS += $(TOOLS_INCLUDES) + $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c index 3638fe605e80ca41b29d43c6ac452964eef35d56..200c534cc70e2c2381fce3be5c0ebe4cb5675e84 100644 --- a/tools/testing/selftests/vDSO/parse_vdso.c +++ b/tools/testing/selftests/vDSO/parse_vdso.c @@ -19,7 +19,8 @@ #include #include #include -#include +#include +#include #include "parse_vdso.h" -- 2.48.1
[PATCH 06/16] elf, uapi: Add types ElfXX_Verdef and ElfXX_Veraux
The types are used by tools/testing/selftests/vDSO/parse_vdso.c. To be able to build the vDSO selftests without a libc dependency, add the types to the kernels own UAPI headers. Link: https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/symversion.html#VERDEFEXTS Signed-off-by: Thomas Weißschuh --- include/uapi/linux/elf.h | 30 ++ 1 file changed, 30 insertions(+) diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index 8846fe03ca5b836c96aad1be6d8fb9daf3d4b1d9..49f9f90458d8ca8e7b8f823d32be0a719ff827b3 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -491,4 +491,34 @@ typedef struct elf64_note { /* Bits for GNU_PROPERTY_AARCH64_FEATURE_1_BTI */ #define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0) +typedef struct { + Elf32_Half vd_version; + Elf32_Half vd_flags; + Elf32_Half vd_ndx; + Elf32_Half vd_cnt; + Elf32_Word vd_hash; + Elf32_Word vd_aux; + Elf32_Word vd_next; +} Elf32_Verdef; + +typedef struct { + Elf64_Half vd_version; + Elf64_Half vd_flags; + Elf64_Half vd_ndx; + Elf64_Half vd_cnt; + Elf64_Word vd_hash; + Elf64_Word vd_aux; + Elf64_Word vd_next; +} Elf64_Verdef; + +typedef struct { + Elf32_Wordvda_name; + Elf32_Wordvda_next; +} Elf32_Verdaux; + +typedef struct { + Elf64_Wordvda_name; + Elf64_Wordvda_next; +} Elf64_Verdaux; + #endif /* _UAPI_LINUX_ELF_H */ -- 2.48.1
[PATCH 08/16] selftests: Add headers target
Some selftests need access to a full UAPI headers tree, for example when building with nolibc which heavily relies on UAPI headers. A reference to such a tree is available in the KHDR_INCLUDES variable, but there is currently no way to populate such a tree automatically. Provide a target that the tests can depend on to get access to usable UAPI headers. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/lib.mk | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index d6edcfcb5be832ddee4c3d34b5ad221e9295f878..5303900339292e618dee4fd7ff8a7c2fa3209a68 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -228,4 +228,7 @@ $(OUTPUT)/%:%.S $(LINK.S) $^ $(LDLIBS) -o $@ endif -.PHONY: run_tests all clean install emit_tests gen_mods_dir clean_mods_dir +headers: + $(Q)$(MAKE) -C $(top_srcdir) headers + +.PHONY: run_tests all clean install emit_tests gen_mods_dir clean_mods_dir headers -- 2.48.1
[PATCH 16/16] selftests: vDSO: vdso_standalone_test_x86: Switch to nolibc
vdso_standalone_test_x86 provides its own ASM syscall wrappers and _start() implementation. The in-tree nolibc library already provides this functionality for multiple architectures. By making use of nolibc, the standalone testcase can be built from the exact same codebase as the non-standalone version. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/vDSO/Makefile | 8 +- .../selftests/vDSO/vdso_standalone_test_x86.c | 168 + 2 files changed, 7 insertions(+), 169 deletions(-) diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index bc8ca186fb877dc11740c37f1e07e45e84c2ae92..12a0614b9fd4983deffe5d6a7cfa06ba8d92a516 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -22,13 +22,17 @@ include ../lib.mk CFLAGS += $(TOOLS_INCLUDES) +CFLAGS_NOLIBC := -nostdlib -nostdinc -ffreestanding -fno-asynchronous-unwind-tables \ +-fno-stack-protector -include $(top_srcdir)/tools/include/nolibc/nolibc.h \ +-I$(top_srcdir)/tools/include/nolibc/ $(KHDR_INCLUDES) + $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c $(OUTPUT)/vdso_test_clock_getres: vdso_test_clock_getres.c -$(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c -$(OUTPUT)/vdso_standalone_test_x86: CFLAGS +=-nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector +$(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c | headers +$(OUTPUT)/vdso_standalone_test_x86: CFLAGS:=$(CFLAGS_NOLIBC) $(CFLAGS) $(OUTPUT)/vdso_test_correctness: vdso_test_correctness.c $(OUTPUT)/vdso_test_correctness: LDFLAGS += -ldl diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c deleted file mode 100644 index 500608f89c66b5747e3d845ebc54e4c3a35b6ccd.. --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c +++ /dev/null @@ -1,167 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * vdso_test.c: Sample code to test parse_vdso.c on x86 - * Copyright (c) 2011-2014 Andy Lutomirski - * - * You can amuse yourself by compiling with: - * gcc -std=gnu99 -nostdlib - * -Os -fno-asynchronous-unwind-tables -flto -lgcc_s - * vdso_standalone_test_x86.c parse_vdso.c - * to generate a small binary. On x86_64, you can omit -lgcc_s - * if you want the binary to be completely standalone. - */ - -#include -#include -#include -#include -#include - -#include "parse_vdso.h" - -/* We need some libc functions... */ -int strcmp(const char *a, const char *b) -{ - /* This implementation is buggy: it never returns -1. */ - while (*a || *b) { - if (*a != *b) - return 1; - if (*a == 0 || *b == 0) - return 1; - a++; - b++; - } - - return 0; -} - -/* - * The clang build needs this, although gcc does not. - * Stolen from lib/string.c. - */ -void *memcpy(void *dest, const void *src, size_t count) -{ - char *tmp = dest; - const char *s = src; - - while (count--) - *tmp++ = *s++; - return dest; -} - -/* ...and two syscalls. This is x86-specific. */ -static inline long x86_syscall3(long nr, long a0, long a1, long a2) -{ - long ret; -#ifdef __x86_64__ - asm volatile ("syscall" : "=a" (ret) : "a" (nr), - "D" (a0), "S" (a1), "d" (a2) : - "cc", "memory", "rcx", - "r8", "r9", "r10", "r11" ); -#else - asm volatile ("int $0x80" : "=a" (ret) : "a" (nr), - "b" (a0), "c" (a1), "d" (a2) : - "cc", "memory" ); -#endif - return ret; -} - -static inline long linux_write(int fd, const void *data, size_t len) -{ - return x86_syscall3(__NR_write, fd, (long)data, (long)len); -} - -static inline void linux_exit(int code) -{ - x86_syscall3(__NR_exit, code, 0, 0); -} - -void to_base10(char *lastdig, time_t n) -{ - while (n) { - *lastdig = (n % 10) + '0'; - n /= 10; - lastdig--; - } -} - -unsigned long getauxval(const unsigned long *auxv, unsigned long type) -{ - unsigned long ret; - - if (!auxv) - return 0; - - while (1) { - if (!auxv[0] && !auxv[1]) { - ret = 0; - break; - } - - if (auxv[0] == type) { - ret = auxv[
[PATCH 10/16] selftests: vDSO: parse_vdso: Drop vdso_init_from_auxv()
There are no users left. Also remove the usage of ElfXX_auxv_t, which is not formally standardized. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/vDSO/parse_vdso.c | 14 -- tools/testing/selftests/vDSO/parse_vdso.h | 1 - 2 files changed, 15 deletions(-) diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c index 2fe5e983cb22f1ed066d0310a54f6aef2ed77ed8..3638fe605e80ca41b29d43c6ac452964eef35d56 100644 --- a/tools/testing/selftests/vDSO/parse_vdso.c +++ b/tools/testing/selftests/vDSO/parse_vdso.c @@ -297,17 +297,3 @@ void *vdso_sym(const char *version, const char *name) return 0; } - -void vdso_init_from_auxv(void *auxv) -{ - ELF(auxv_t) *elf_auxv = auxv; - for (int i = 0; elf_auxv[i].a_type != AT_NULL; i++) - { - if (elf_auxv[i].a_type == AT_SYSINFO_EHDR) { - vdso_init_from_sysinfo_ehdr(elf_auxv[i].a_un.a_val); - return; - } - } - - vdso_info.valid = false; -} diff --git a/tools/testing/selftests/vDSO/parse_vdso.h b/tools/testing/selftests/vDSO/parse_vdso.h index de0453067d7cd0d8b63f7d3738842f60370db813..09d068ed11f97f0c5c8f4e7b341f08fa261c9735 100644 --- a/tools/testing/selftests/vDSO/parse_vdso.h +++ b/tools/testing/selftests/vDSO/parse_vdso.h @@ -26,6 +26,5 @@ */ void *vdso_sym(const char *version, const char *name); void vdso_init_from_sysinfo_ehdr(uintptr_t base); -void vdso_init_from_auxv(void *auxv); #endif -- 2.48.1
[PATCH 15/16] selftests: vDSO: vdso_test_gettimeofday: Make compatible with nolibc
nolibc does not provide these headers, instead those definitions are available unconditionally. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/vDSO/vdso_test_gettimeofday.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c index 636a56ccf8e4e7943ca446fe3fad6897598ca77f..9ce795b806f0992b83cef78c7e16fac0e54750da 100644 --- a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c +++ b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c @@ -11,8 +11,10 @@ */ #include +#ifndef NOLIBC #include #include +#endif #include "../kselftest.h" #include "parse_vdso.h" -- 2.48.1
[PATCH 12/16] selftests: vDSO: parse_vdso: Test __SIZEOF_LONG__ instead of ULONG_MAX
According to limits.h(2) ULONG_MAX is only guaranteed to expand to an expression, not a symbolic constant which can be evaluated by the preprocessor. Specifically the definition of ULONG_MAX from nolibc can no be evaluated by the preprocessor. To provide compatibility with nolibc, check with __SIZEOF_LONG__ instead, with is provided directly by the preprocessor and therefore always a symbolic constant. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/vDSO/parse_vdso.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c index 200c534cc70e2c2381fce3be5c0ebe4cb5675e84..902b8f9984a1f70049d46bcd4f199df24f507dcb 100644 --- a/tools/testing/selftests/vDSO/parse_vdso.c +++ b/tools/testing/selftests/vDSO/parse_vdso.c @@ -26,7 +26,7 @@ /* And here's the code. */ #ifndef ELF_BITS -# if ULONG_MAX > 0xUL +# if __SIZEOF_LONG__ >= 8 # define ELF_BITS 64 # else # define ELF_BITS 32 -- 2.48.1
[PATCH 14/16] selftests: vDSO: vdso_test_gettimeofday: Clean up includes
Some unnecessary headers are included, remove them. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/vDSO/vdso_test_gettimeofday.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c index e31b18ffae338c1576449b54ca7f84d9816d2ecb..636a56ccf8e4e7943ca446fe3fad6897598ca77f 100644 --- a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c +++ b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c @@ -10,8 +10,6 @@ * Tested on x86, 32-bit and 64-bit. It may work on other architectures, too. */ -#include -#include #include #include #include -- 2.48.1
[PATCH 13/16] selftests: vDSO: parse_vdso: Make compatible with nolibc
nolibc does not provide this header, instead its definitions are available unconditionally. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/vDSO/parse_vdso.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c index 902b8f9984a1f70049d46bcd4f199df24f507dcb..6ea1d7cca6a3da910097630e8237a3a6daa0cd06 100644 --- a/tools/testing/selftests/vDSO/parse_vdso.c +++ b/tools/testing/selftests/vDSO/parse_vdso.c @@ -18,7 +18,9 @@ #include #include #include +#ifndef NOLIBC #include +#endif #include #include -- 2.48.1
Re: [PATCH 00/16] selftests: vDSO: parse_vdso: Make compatible with nolibc
On Mon, Feb 03, 2025 at 12:23:29PM +0100, Christophe Leroy wrote: > Le 03/02/2025 à 10:05, Thomas Weißschuh a écrit : > > For testing the functionality of the vDSO, it is necessary to build > > userspace programs for multiple different architectures. > > It is additional work to acquire matching userspace cross-compilers with > > full C libraries and then building root images out of those. > > The kernel tree already contains nolibc, a small, header-only C library. > > By using it, it is possible to build userspace programs without any > > additional dependencies. > > For example the kernel.org crosstools or multi-target clang can be used > > to build test programs for a multitude of architectures. > > While nolibc is very limited, it is enough for many selftests. > > With some minor adjustments it is possible to make parse_vdso.c > > compatible with nolibc. > > As an example, vdso_standalone_test_x86 is now built from the same C > > code as the regular vdso_test_gettimeofday, while still being completely > > standalone. > > > > This should probably go through the kselftest tree. > > Not sure what are the expectations with this series. In general it should also work for PPC, thanks for testing it. > I gave it a try with vdso_test_gettimeofday and get the following: > > $ powerpc64-linux-gcc -nostdlib -nostdinc -ffreestanding > -fno-asynchronous-unwind-tables -fno-stack-protector -include > /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include/nolibc/nolibc.h > > -I/home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include/nolibc/ > -isystem > /home/chleroy/linux-powerpc/tools/testing/selftests/../../../usr/include > -std=gnu99 -O2 -D_GNU_SOURCE= -isystem > /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include/uapi > vdso_test_gettimeofday.c parse_vdso.c -o > /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_gettimeofday > > make: Entering directory > '/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO' > powerpc64-linux-gcc -nostdlib -nostdinc -ffreestanding > -fno-asynchronous-unwind-tables -fno-stack-protector -include > /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include/nolibc/nolibc.h > > -I/home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include/nolibc/ > -isystem > /home/chleroy/linux-powerpc/tools/testing/selftests/../../../usr/include > -std=gnu99 -O2 -D_GNU_SOURCE= -isystem > /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include/uapi > vdso_test_gettimeofday.c parse_vdso.c -o > /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_gettimeofday This log is confusing. It contains traces of "make" output but no "make" invocation. I'm also not sure if there were two different compilations or only one. Can you give the full commandline? For your testing it should be enough to enable "vdso_test_standalone_x86" in the Makefile for PPC. > parse_vdso.c:38:34: error: unknown type name 'Elf64_Versym' >38 | #define ELF_BITS_XFORM2(bits, x) Elf##bits##_##x > | ^~~ > parse_vdso.c:39:33: note: in expansion of macro 'ELF_BITS_XFORM2' >39 | #define ELF_BITS_XFORM(bits, x) ELF_BITS_XFORM2(bits, x) > | ^~~ > parse_vdso.c:40:16: note: in expansion of macro 'ELF_BITS_XFORM' >40 | #define ELF(x) ELF_BITS_XFORM(ELF_BITS, x) > |^~ > parse_vdso.c:63:9: note: in expansion of macro 'ELF' >63 | ELF(Versym) *versym; > | ^~~ > parse_vdso.c:38:34: error: unknown type name 'Elf64_Verdef' >38 | #define ELF_BITS_XFORM2(bits, x) Elf##bits##_##x > | ^~~ > parse_vdso.c:39:33: note: in expansion of macro 'ELF_BITS_XFORM2' >39 | #define ELF_BITS_XFORM(bits, x) ELF_BITS_XFORM2(bits, x) > | ^~~ > parse_vdso.c:40:16: note: in expansion of macro 'ELF_BITS_XFORM' >40 | #define ELF(x) ELF_BITS_XFORM(ELF_BITS, x) > |^~ > parse_vdso.c:64:9: note: in expansion of macro 'ELF' >64 | ELF(Verdef) *verdef; > | ^~~ > parse_vdso.c: In function 'vdso_init_from_sysinfo_ehdr': > parse_vdso.c:38:34: error: 'Elf64_Versym' undeclared (first use in this > function); did you mean 'Elf64_Sym'? >38 | #define ELF_BITS_XFORM2(bits, x) Elf##bits##_##x > | ^~~ > parse_vdso.c:39:33: note: in expans
Re: [PATCH 00/16] selftests: vDSO: parse_vdso: Make compatible with nolibc
On Mon, Feb 03, 2025 at 04:43:22PM +0100, Christophe Leroy wrote: > Do you have any plan to get it work with nolibc for all test programs in > selftests/vDSO, not only the standalone x86 test ? Not directly as next step. I am focussing on some other work which will integrate (vDSO) selftests with nolibc slightly differently. However if you have interest in converting the tests now, that would be great and will also be useful for my future changes. The current issues I see: * Missing architecture support in nolibc (should be fairly easy to implement) * Missing kselftest_harness.h support in nolibc (I'm working on that) * Maybe some users want to stick with their regular libc (Nothing should prevent that, but the mechanism may need some discussion) Thomas
[PATCH 2/2] tools/nolibc: add support for directory access
From: Thomas Weißschuh Add an allocation-free implementation of readdir() and related functions. The implementation is modelled after the one for FILE. Signed-off-by: Thomas Weißschuh Signed-off-by: Thomas Weißschuh --- I'm not entirely sure where to put it. It doesn't really belong into stdio.h, but that's where the FILE stuff is. sys.h wants alphabetical ordering, but IMO these functions should stick together. --- tools/include/nolibc/stdio.h | 76 tools/include/nolibc/types.h | 5 ++ tools/testing/selftests/nolibc/nolibc-test.c | 37 ++ 3 files changed, 118 insertions(+) diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h index 3892034198dd566d21a5cc0a9f67cf097d428393..1f275a0a7b6b2c6f1c15405d027c282bb77aa618 100644 --- a/tools/include/nolibc/stdio.h +++ b/tools/include/nolibc/stdio.h @@ -387,6 +387,82 @@ const char *strerror(int errno) return buf; } +/* See comment of FILE above */ +typedef struct { + char dummy[1]; +} DIR; + +static __attribute__((unused)) +DIR *fdopendir(int fd) +{ + if (fd < 0) { + SET_ERRNO(EBADF); + return NULL; + } + return (DIR *)(intptr_t)~fd; +} + +static __attribute__((unused)) +DIR *opendir(const char *name) +{ + int fd; + + fd = open(name, O_RDONLY); + if (fd == -1) + return NULL; + return fdopendir(fd); +} + +static __attribute__((unused)) +int closedir(DIR *dirp) +{ + intptr_t i = (intptr_t)dirp; + + if (i >= 0) { + SET_ERRNO(EBADF); + return -1; + } + return close(~i); +} + +static __attribute__((unused)) +struct dirent *readdir(DIR *dirp) +{ + static struct dirent dirent; + + char buf[sizeof(struct linux_dirent64) + NAME_MAX]; + struct linux_dirent64 *ldir = (void *)buf; + intptr_t i = (intptr_t)dirp; + int fd, ret; + + if (i >= 0) { + SET_ERRNO(EBADF); + return NULL; + } + + fd = ~i; + + ret = getdents64(fd, ldir, sizeof(buf)); + if (ret == -1 || ret == 0) + return NULL; + + /* +* getdents64() returns as many entries as fit the buffer. +* readdir() can only return one entry at a time. +* Make sure the non-returned ones are not skipped. +*/ + ret = lseek(fd, ldir->d_off, SEEK_SET); + if (ret == -1) + return NULL; + + dirent = (struct dirent) { + .d_ino = ldir->d_ino, + }; + strlcpy(dirent.d_name, ldir->d_name, sizeof(dirent.d_name)); + + return &dirent; +} + /* make sure to include all global symbols */ #include "nolibc.h" diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h index b26a5d0c417c7c3b232f28e78cdd6d94a759b7bc..67885731fff3b2008efffe7c02f93f978ef7b078 100644 --- a/tools/include/nolibc/types.h +++ b/tools/include/nolibc/types.h @@ -179,6 +179,11 @@ struct linux_dirent64 { char d_name[]; }; +struct dirent { + ino_t d_ino; + chard_name[256]; +}; + /* The format of the struct as returned by the libc to the application, which * significantly differs from the format returned by the stat() syscall flavours. */ diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 0e0e3b48a8c3a6802c6989954b6f3a7c7258db43..18f769bcf68dfb190b157aeeba14f7904965377b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -767,6 +767,42 @@ int test_getdents64(const char *dir) return ret; } +int test_directories(void) +{ + int comm = 0, cmdline = 0; + struct dirent *dirent; + DIR *dir; + int ret; + + dir = opendir("/proc/self"); + if (!dir) + return 1; + + while (1) { + errno = 0; + dirent = readdir(dir); + if (!dirent) + break; + + if (strcmp(dirent->d_name, "comm") == 0) + comm++; + else if (strcmp(dirent->d_name, "cmdline") == 0) + cmdline++; + } + + if (errno) + return 1; + + ret = closedir(dir); + if (ret) + return 1; + + if (comm != 1 || cmdline != 1) + return 1; + + return 0; +} + int test_getpagesize(void) { int x = getpagesize(); @@ -1059,6 +1095,7 @@ int run_syscall(int min, int max) CASE_TEST(fork); EXPECT_SYSZR(1, test_fork()); break; CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"),
[PATCH 1/2] tools/nolibc: add support for sys_llseek()
From: Thomas Weißschuh Not all architectures have the old sys_lseek(), notably riscv32. Implement lseek() in terms of sys_llseek() in that case. Signed-off-by: Thomas Weißschuh Signed-off-by: Thomas Weißschuh --- tools/include/nolibc/sys.h | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index d4a5c2399a66b200ebf7ab249569cce2285481a5..cf8d72f99c87c99622eca410cf80b0c5c1aab8f0 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -601,10 +601,37 @@ off_t sys_lseek(int fd, off_t offset, int whence) #endif } +static __attribute__((unused)) +int sys_llseek(int fd, unsigned long offset_high, unsigned long offset_low, + __kernel_loff_t *result, int whence) +{ +#ifdef __NR_llseek + return my_syscall5(__NR_llseek, fd, offset_high, offset_low, result, whence); +#else + return __nolibc_enosys(__func__, fd, offset_high, offset_low, result, whence); +#endif +} + static __attribute__((unused)) off_t lseek(int fd, off_t offset, int whence) { - return __sysret(sys_lseek(fd, offset, whence)); + __kernel_loff_t loff = 0; + off_t result; + int ret; + + result = sys_lseek(fd, offset, whence); + if (result == -ENOSYS) { + /* Only exists on 32bit where nolibc off_t is also 32bit */ + ret = sys_llseek(fd, 0, offset, &loff, whence); + if (ret < 0) + result = ret; + else if (loff != (off_t)loff) + result = -EOVERFLOW; + else + result = loff; + } + + return __sysret(result); } -- 2.48.1
[PATCH 0/2] tools/nolibc: add support for directory access
Add support opendir(), readdir(), closedir() and friends. Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (2): tools/nolibc: add support for sys_llseek() tools/nolibc: add support for directory access tools/include/nolibc/stdio.h | 76 tools/include/nolibc/sys.h | 29 ++- tools/include/nolibc/types.h | 5 ++ tools/testing/selftests/nolibc/nolibc-test.c | 37 ++ 4 files changed, 146 insertions(+), 1 deletion(-) --- base-commit: d3d90cc2891c9cf4ecba7b85c0af716ab755c7e5 change-id: 20250130-nolibc-dir-980c2e2b661a Best regards, -- Thomas Weißschuh
Re: [PATCH 04/16] elf, uapi: Add definitions for VER_FLG_BASE and VER_FLG_WEAK
On Tue, Feb 04, 2025 at 07:10:00AM -0800, Kees Cook wrote: > On Mon, Feb 03, 2025 at 10:05:05AM +0100, Thomas Weißschuh wrote: > > The definitions are used by tools/testing/selftests/vDSO/parse_vdso.c. > > To be able to build the vDSO selftests without a libc dependency, > > add the definitions to the kernels own UAPI headers. > > For all the UAPI changes, where are the defines "normally" found? i.e. > how does adding these to UAPI not break something that already has them? > Or have these never been defined before? I'm confused about how removing > the libc dependency exposes the lack of these defines. Are they defined > in a non-exported libc header somewhere? They are normally defined directly in libc , which does not use UAPI headers. Libc elf.h and Linux UAPI elf.h can not be used at the same time because they define the same symbols. In theory some user of UAPI elf.h could have defined these new symbols on their own without ifdef guards. However UAPI elf.h is regularly updated with new symbols.
[PATCH v2 0/2] tools/nolibc: support for 32-bit s390
Support for 32-bit s390 is very easy to implement and useful for testing. For example I used to test some generic compat_ptr() logic, which is only testable on 32-bit s390. The series depends on my other series "selftests/nolibc: test kernel configuration cleanups". (It's not a hard dependency, only a minor diff conflict) Signed-off-by: Thomas Weißschuh --- Changes in v2: - Rebase unto nolibc-next - Use 96 bytes of stack frame size - Pick up Ack from Willy - Link to v1: https://lore.kernel.org/r/20250122-nolibc-s390-v1-0-8c765f00e...@weissschuh.net --- Thomas Weißschuh (2): selftests/nolibc: rename s390 to s390x tools/nolibc: add support for 32-bit s390 tools/include/nolibc/arch-s390.h| 5 + tools/include/nolibc/arch.h | 2 +- tools/testing/selftests/nolibc/Makefile | 10 -- tools/testing/selftests/nolibc/run-tests.sh | 7 ++- 4 files changed, 20 insertions(+), 4 deletions(-) --- base-commit: c1f4a7a84037249d086a4114c0c4332a260e9091 change-id: 20250122-nolibc-s390-e57141682c88 Best regards, -- Thomas Weißschuh
[PATCH v2 2/2] tools/nolibc: add support for 32-bit s390
32-bit s390 is very close to the existing 64-bit implementation. Some special handling is necessary as there is neither LLVM nor QEMU support. Also the kernel itself can not build natively for 32-bit s390, so instead the test program is executed with a 64-bit kernel. Acked-by: Willy Tarreau Signed-off-by: Thomas Weißschuh --- tools/include/nolibc/arch-s390.h| 5 + tools/include/nolibc/arch.h | 2 +- tools/testing/selftests/nolibc/Makefile | 5 + tools/testing/selftests/nolibc/run-tests.sh | 6 +- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h index f9ab83a219b8a2d5e53b0b303d8bf0bf78280d5f..acfee7e9d5e2bb65718cb947110d2c5e1cdeba9b 100644 --- a/tools/include/nolibc/arch-s390.h +++ b/tools/include/nolibc/arch-s390.h @@ -143,8 +143,13 @@ void __attribute__((weak, noreturn)) __nolibc_entrypoint __no_stack_protector _start(void) { __asm__ volatile ( +#ifdef __s390x__ "lgr%r2, %r15\n" /* save stack pointer to %r2, as arg1 of _start_c */ "aghi %r15, -160\n" /* allocate new stackframe */ +#else + "lr %r2, %r15\n" + "ahi%r15, -96\n" +#endif "xc 0(8,%r15), 0(%r15)\n" /* clear backchain */ "brasl %r14, _start_c\n" /* transfer to c runtime */ ); diff --git a/tools/include/nolibc/arch.h b/tools/include/nolibc/arch.h index c8f4e5d3add9eb5b8a438900c084dc0449fcfbd6..8a2c143c0fba288147e5a7bf9db38ffb08367616 100644 --- a/tools/include/nolibc/arch.h +++ b/tools/include/nolibc/arch.h @@ -29,7 +29,7 @@ #include "arch-powerpc.h" #elif defined(__riscv) #include "arch-riscv.h" -#elif defined(__s390x__) +#elif defined(__s390x__) || defined(__s390__) #include "arch-s390.h" #elif defined(__loongarch__) #include "arch-loongarch.h" diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 97e08b92bee32300922c27ccd7c07e9eddc7162e..14fc8c7e7c3067efddf0f729890fb78df731efb3 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -72,6 +72,7 @@ IMAGE_riscv = arch/riscv/boot/Image IMAGE_riscv32= arch/riscv/boot/Image IMAGE_riscv64= arch/riscv/boot/Image IMAGE_s390x = arch/s390/boot/bzImage +IMAGE_s390 = arch/s390/boot/bzImage IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi IMAGE= $(objtree)/$(IMAGE_$(XARCH)) IMAGE_NAME = $(notdir $(IMAGE)) @@ -91,6 +92,7 @@ DEFCONFIG_riscv = defconfig DEFCONFIG_riscv32= rv32_defconfig DEFCONFIG_riscv64= defconfig DEFCONFIG_s390x = defconfig +DEFCONFIG_s390 = defconfig compat.config DEFCONFIG_loongarch = defconfig DEFCONFIG= $(DEFCONFIG_$(XARCH)) @@ -114,6 +116,7 @@ QEMU_ARCH_riscv = riscv64 QEMU_ARCH_riscv32= riscv32 QEMU_ARCH_riscv64= riscv64 QEMU_ARCH_s390x = s390x +QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 QEMU_ARCH= $(QEMU_ARCH_$(XARCH)) @@ -142,6 +145,7 @@ QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_T QEMU_ARGS_riscv32= -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv64= -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_s390x = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" +QEMU_ARGS_s390 = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS= -m 1G $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_BIOS) $(QEMU_ARGS_EXTRA) @@ -160,6 +164,7 @@ CFLAGS_ppc = -m32 -mbig-endian -mno-vsx $(call cc-option,-mmultiple) CFLAGS_ppc64 = -m64 -mbig-endian -mno-vsx $(call cc-option,-mmultiple) CFLAGS_ppc64le = -m64 -mlittle-endian -mno-vsx $(call cc-option,-mabi=elfv2) CFLAGS_s390x = -m64 +CFLAGS_s390 = -m31 CFLAGS_mips32le = -EL -mabi=32 -fPIC CFLAGS_mips32be = -EB -mabi=32 CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) diff --git a/tools/testing/selftests/nolibc/run-tests.sh b/tools/testing/selftests/nolibc/run-tests.sh index e1c6e86912d5d4910f45d7061ee619841cf1ffd9..fd01aa4900c912e62722603fb9750bd23f27e45c 100755 --- a/tools/testing/selftests/nolibc/run-tests.sh +++ b/tools/testing/selftests/nolibc/run-tests.sh @@ -17,7 +17,7 @@ perform_download=0 test_mode=system werror=1 llvm= -archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv32 riscv64 s390x loongarch" +archs=
[PATCH v2 1/2] selftests/nolibc: rename s390 to s390x
Support for 32-bit s390 is about to be added. As "s39032" would look horrible, use the another naming scheme. 32-bit s390 is "s390" and 64-bit s390 is "s390x", similar to how it is handled in various toolchain components. Acked-by: Willy Tarreau Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/Makefile | 11 ++- tools/testing/selftests/nolibc/run-tests.sh | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 983985b7529b65b7ce4a00c28f3f915d83974eea..97e08b92bee32300922c27ccd7c07e9eddc7162e 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -54,6 +54,7 @@ ARCH_mips32le= mips ARCH_mips32be= mips ARCH_riscv32 = riscv ARCH_riscv64 = riscv +ARCH_s390x = s390 ARCH:= $(or $(ARCH_$(XARCH)),$(XARCH)) # kernel image names by architecture @@ -70,7 +71,7 @@ IMAGE_ppc64le= arch/powerpc/boot/zImage IMAGE_riscv = arch/riscv/boot/Image IMAGE_riscv32= arch/riscv/boot/Image IMAGE_riscv64= arch/riscv/boot/Image -IMAGE_s390 = arch/s390/boot/bzImage +IMAGE_s390x = arch/s390/boot/bzImage IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi IMAGE= $(objtree)/$(IMAGE_$(XARCH)) IMAGE_NAME = $(notdir $(IMAGE)) @@ -89,7 +90,7 @@ DEFCONFIG_ppc64le= powernv_defconfig DEFCONFIG_riscv = defconfig DEFCONFIG_riscv32= rv32_defconfig DEFCONFIG_riscv64= defconfig -DEFCONFIG_s390 = defconfig +DEFCONFIG_s390x = defconfig DEFCONFIG_loongarch = defconfig DEFCONFIG= $(DEFCONFIG_$(XARCH)) @@ -112,7 +113,7 @@ QEMU_ARCH_ppc64le= ppc64 QEMU_ARCH_riscv = riscv64 QEMU_ARCH_riscv32= riscv32 QEMU_ARCH_riscv64= riscv64 -QEMU_ARCH_s390 = s390x +QEMU_ARCH_s390x = s390x QEMU_ARCH_loongarch = loongarch64 QEMU_ARCH= $(QEMU_ARCH_$(XARCH)) @@ -140,7 +141,7 @@ QEMU_ARGS_ppc64le= -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv32= -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv64= -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" -QEMU_ARGS_s390 = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" +QEMU_ARGS_s390x = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS= -m 1G $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_BIOS) $(QEMU_ARGS_EXTRA) @@ -158,7 +159,7 @@ CFLAGS_i386 = $(call cc-option,-m32) CFLAGS_ppc = -m32 -mbig-endian -mno-vsx $(call cc-option,-mmultiple) CFLAGS_ppc64 = -m64 -mbig-endian -mno-vsx $(call cc-option,-mmultiple) CFLAGS_ppc64le = -m64 -mlittle-endian -mno-vsx $(call cc-option,-mabi=elfv2) -CFLAGS_s390 = -m64 +CFLAGS_s390x = -m64 CFLAGS_mips32le = -EL -mabi=32 -fPIC CFLAGS_mips32be = -EB -mabi=32 CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) diff --git a/tools/testing/selftests/nolibc/run-tests.sh b/tools/testing/selftests/nolibc/run-tests.sh index bc4e92b4f1b98278a0a72345a5cd67f1a429b6a2..e1c6e86912d5d4910f45d7061ee619841cf1ffd9 100755 --- a/tools/testing/selftests/nolibc/run-tests.sh +++ b/tools/testing/selftests/nolibc/run-tests.sh @@ -17,7 +17,7 @@ perform_download=0 test_mode=system werror=1 llvm= -archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv32 riscv64 s390 loongarch" +archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv32 riscv64 s390x loongarch" TEMP=$(getopt -o 'j:d:c:b:a:m:pelh' -n "$0" -- "$@") @@ -100,6 +100,7 @@ crosstool_arch() { riscv) echo riscv64;; loongarch) echo loongarch64;; mips*) echo mips;; + s390*) echo s390;; *) echo "$1";; esac } -- 2.48.1
[PATCH v2 2/2] Revert "selftests: kselftest: Fix build failure with NOLIBC"
This reverts commit 16767502aa990cca2cb7d1372b31d328c4c85b40. Nolibc gained support for uname(2) and sscanf(3) which are the dependencies of ksft_min_kernel_version(). So re-enable support for ksft_min_kernel_version() under nolibc. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/kselftest.h | 5 - 1 file changed, 5 deletions(-) diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index cdf91b0ca40fbdc4fb825b86d4dc547b5afa673c..c3b6d2604b1e486af5a224a11386f75fe0a83495 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -444,10 +444,6 @@ static inline __noreturn __printf(1, 2) void ksft_exit_skip(const char *msg, ... static inline int ksft_min_kernel_version(unsigned int min_major, unsigned int min_minor) { -#ifdef NOLIBC - ksft_print_msg("NOLIBC: Can't check kernel version: Function not implemented\n"); - return 0; -#else unsigned int major, minor; struct utsname info; @@ -455,7 +451,6 @@ static inline int ksft_min_kernel_version(unsigned int min_major, ksft_exit_fail_msg("Can't parse kernel version\n"); return major > min_major || (major == min_major && minor >= min_minor); -#endif } #endif /* __KSELFTEST_H */ -- 2.48.1
[PATCH v2 0/2] tools/nolibc: add support for [v]sscanf()
The implementation is limited and only supports numeric arguments. Signed-off-by: Thomas Weißschuh --- Changes in v2: - Return __LINE__ from different testcases to directly point to the failed testcase - Add some comments - Expand commit message - Link to v1: https://lore.kernel.org/r/20240731-nolibc-scanf-v1-0-f71bcc4ab...@weissschuh.net --- Thomas Weißschuh (2): tools/nolibc: add support for [v]sscanf() Revert "selftests: kselftest: Fix build failure with NOLIBC" tools/include/nolibc/stdio.h | 98 tools/testing/selftests/kselftest.h | 5 -- tools/testing/selftests/nolibc/nolibc-test.c | 68 +++ 3 files changed, 166 insertions(+), 5 deletions(-) --- base-commit: 665fa8dea90d9fbc0e7137c7e1315d6f7e15757e change-id: 20240414-nolibc-scanf-f1db6930d0c6 Best regards, -- Thomas Weißschuh
[PATCH v2 1/2] tools/nolibc: add support for [v]sscanf()
These functions are used often, also in selftests. sscanf() itself is also used by kselftest.h itself. The implementation is limited and only supports numeric arguments. Signed-off-by: Thomas Weißschuh --- tools/include/nolibc/stdio.h | 98 tools/testing/selftests/nolibc/nolibc-test.c | 68 +++ 2 files changed, 166 insertions(+) diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h index 3892034198dd566d21a5cc0a9f67cf097d428393..a403351dbf6098ac8292b9589ed8a054b4c5669f 100644 --- a/tools/include/nolibc/stdio.h +++ b/tools/include/nolibc/stdio.h @@ -349,6 +349,104 @@ int printf(const char *fmt, ...) return ret; } +static __attribute__((unused)) +int vsscanf(const char *str, const char *format, va_list args) +{ + uintmax_t uval; + intmax_t ival; + int base; + char *endptr; + int matches; + int lpref; + + matches = 0; + + while (1) { + if (*format == '%') { + /* start of pattern */ + lpref = 0; + format++; + + if (*format == 'l') { + /* same as in printf() */ + lpref = 1; + format++; + if (*format == 'l') { + lpref = 2; + format++; + } + } + + if (*format == '%') { + /* literal % */ + if ('%' != *str) + goto done; + str++; + format++; + continue; + } else if (*format == 'd') { + ival = strtoll(str, &endptr, 10); + if (lpref == 0) + *va_arg(args, int *) = ival; + else if (lpref == 1) + *va_arg(args, long *) = ival; + else if (lpref == 2) + *va_arg(args, long long *) = ival; + } else if (*format == 'u' || *format == 'x' || *format == 'X') { + base = *format == 'u' ? 10 : 16; + uval = strtoull(str, &endptr, base); + if (lpref == 0) + *va_arg(args, unsigned int *) = uval; + else if (lpref == 1) + *va_arg(args, unsigned long *) = uval; + else if (lpref == 2) + *va_arg(args, unsigned long long *) = uval; + } else if (*format == 'p') { + *va_arg(args, void **) = (void *)strtoul(str, &endptr, 16); + } else { + SET_ERRNO(EILSEQ); + goto done; + } + + format++; + str = endptr; + matches++; + + } else if (*format == '\0') { + goto done; + } else if (isspace(*format)) { + /* skip spaces in format and str */ + while (isspace(*format)) + format++; + while (isspace(*str)) + str++; + } else if (*format == *str) { + /* literal match */ + format++; + str++; + } else { + if (!matches) + matches = EOF; + goto done; + } + } + +done: + return matches; +} + +static __attribute__((unused, format(scanf, 2, 3))) +int sscanf(const char *str, const char *format, ...) +{ + va_list args; + int ret; + + va_start(args, format); + ret = vsscanf(str, format, args); + va_end(args); + return ret; +} + static __attribute__((unused)) void perror(const char *msg) { diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 798fbdcd3ff8c36b514feb3fa1c7b8d7701cccd7..81e942c7d1684a7dd59af0b05c8330378532ecc1 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -1325,6 +1325,73 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm
Re: [v2 2/3] kernel: refactor and globalize lookup_or_create_module_kobject()
On 2025-02-06 21:45:37-0800, Shyam Saini wrote: > lookup_or_create_module_kobject() is static and marked as __init, > this is not ideal for global usage. FYI missing "PATCH" in patch subject. > Fix this limitation by refactoring and declaring this as global: > - Refactor it by removing BUG_ON() and 'if else' construct by returning >early This does look like an unrelated change, could be in its own patch. > - Remove static and __init markers from the function and add its >declaration in module.h > - Mark this function as "__modinit". To facilitate this, move the >__modinit macro construct to module.h > > Suggested-by: Rasmus Villemoes > Signed-off-by: Shyam Saini > --- > include/linux/module.h | 8 +++ > kernel/params.c| 48 ++ > 2 files changed, 29 insertions(+), 27 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index 12f8a7d4fc1c..57d09b4e4385 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -162,6 +162,14 @@ extern void cleanup_module(void); > #define __INITRODATA_OR_MODULE __INITRODATA > #endif /*CONFIG_MODULES*/ > > +#ifdef CONFIG_MODULES > +#define __modinit > +#else > +#define __modinit __init > +#endif > + > +struct module_kobject __modinit * lookup_or_create_module_kobject(const char > *name); __init / __modinit is not necessary on the declaration. You can remove it here and keep the #define private. > + > /* Generic info of form tag = "info" */ > #define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info) > > diff --git a/kernel/params.c b/kernel/params.c > index 4b43baaf7c83..5d16696b1daa 100644 > --- a/kernel/params.c > +++ b/kernel/params.c > @@ -595,12 +595,6 @@ static ssize_t param_attr_store(const struct > module_attribute *mattr, > } > #endif > > -#ifdef CONFIG_MODULES > -#define __modinit > -#else > -#define __modinit __init > -#endif > - > #ifdef CONFIG_SYSFS > void kernel_param_lock(struct module *mod) > { > @@ -763,38 +757,38 @@ void destroy_params(const struct kernel_param *params, > unsigned num) > params[i].ops->free(params[i].arg); > } > > -static struct module_kobject * __init lookup_or_create_module_kobject(const > char *name) > +struct module_kobject __modinit * lookup_or_create_module_kobject(const char > *name) > { > struct module_kobject *mk; > struct kobject *kobj; > int err; > > kobj = kset_find_obj(module_kset, name); > - if (kobj) { > - mk = to_module_kobject(kobj); > - } else { > - mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL); > - BUG_ON(!mk); > - > - mk->mod = THIS_MODULE; > - mk->kobj.kset = module_kset; > - err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL, > + if (kobj) > + return to_module_kobject(kobj); > + > + mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL); > + if (!mk) > + return NULL; > + > + mk->mod = THIS_MODULE; > + mk->kobj.kset = module_kset; > + err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL, > "%s", name); > #ifdef CONFIG_MODULES As you are cleaning this up anyways: The #ifdef above should become if (IS_ENABLED(CONFIG_MODULES)) > - if (!err) > - err = sysfs_create_file(&mk->kobj, &module_uevent.attr); > + if (!err) > + err = sysfs_create_file(&mk->kobj, &module_uevent.attr); > #endif > - if (err) { > - kobject_put(&mk->kobj); > - pr_crit("Adding module '%s' to sysfs failed (%d), the > system may be unstable.\n", > - name, err); > - return NULL; > - } > - > - /* So that we hold reference in both cases. */ > - kobject_get(&mk->kobj); > + if (err) { > + kobject_put(&mk->kobj); > + pr_crit("Adding module '%s' to sysfs failed (%d), the system > may be unstable.\n", > + name, err); > + return NULL; > } > > + /* So that we hold reference in both cases. */ > + kobject_get(&mk->kobj); > + > return mk; > } > > -- > 2.34.1 >
[PATCH] selftests/nolibc: split up architecture list in run-tests.sh
The list is getting overly long and any modifications introduce a lot of noise and are prone to conflicts. Split the string into a bash array and break that into multiple lines. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/run-tests.sh | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/nolibc/run-tests.sh b/tools/testing/selftests/nolibc/run-tests.sh index bc4e92b4f1b98278a0a72345a5cd67f1a429b6a2..6db01115276888bc89f6ec5532153c37e55c83d3 100755 --- a/tools/testing/selftests/nolibc/run-tests.sh +++ b/tools/testing/selftests/nolibc/run-tests.sh @@ -17,7 +17,16 @@ perform_download=0 test_mode=system werror=1 llvm= -archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv32 riscv64 s390 loongarch" +all_archs=( + i386 x86_64 + arm64 arm + mips32le mips32be + ppc ppc64 ppc64le + riscv32 riscv64 + s390 + loongarch +) +archs="${all_archs[@]}" TEMP=$(getopt -o 'j:d:c:b:a:m:pelh' -n "$0" -- "$@") --- base-commit: 665fa8dea90d9fbc0e7137c7e1315d6f7e15757e change-id: 20250211-nolibc-test-archs-a5990cfc9e52 Best regards, -- Thomas Weißschuh
[PATCH 0/2] kunit: qemu_configs: Add MIPS configurations
Add basic support to run various MIPS variants via kunit_tool using the virtualized malta platform. Various kunit tests from drivers/firmware/cirrus/ are failing on MIPS. They are fixed in [0]. [0] https://lore.kernel.org/lkml/20250211-cs_dsp-kunit-strings-v1-1-d9bc2035d...@linutronix.de/ Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (2): MIPS: mm: Avoid blocking DMA zone with memory map memblock allocation kunit: qemu_configs: Add MIPS configurations arch/mips/mm/init.c | 2 ++ tools/testing/kunit/qemu_configs/mips.py | 18 ++ tools/testing/kunit/qemu_configs/mips64.py | 19 +++ tools/testing/kunit/qemu_configs/mips64el.py | 19 +++ tools/testing/kunit/qemu_configs/mipsel.py | 18 ++ 5 files changed, 76 insertions(+) --- base-commit: 6e24361511062dba8c5f7e59d51b29cdfa859523 change-id: 20241014-kunit-mips-e4fe1c265ed7 Best regards, -- Thomas Weißschuh
[PATCH 1/2] MIPS: mm: Avoid blocking DMA zone with memory map memblock allocation
On MIPS the memblock allocator is configured to allocate bottom-up. The memory map is allocated by the mm core through memblock and uses MEMBLOCK_LOW_LIMIT as minimal address. This constant is defined as zero because it assumes that "we are using top down, so it is safe to use 0 here". So the memory map is allocated as close to 0 as possible, right where the DMA zone will end up. As the memory map is allocated permanently and also larger than the DMA zone, it makes the DMA zone unusable. Temporarily switch to top-down allocation for the call to free_area_init() so the memory map allocation does not fall into the DMA zone. Signed-off-by: Thomas Weißschuh --- Another solution would be to change alloc_node_mem_map() in the mm core to use __pa(MAX_DMA_ADDRESS) for the min_addr when calling memmap_alloc(), as is done by the other callers of memmap_alloc(). Looping in the memblock maintainers for discussion. This is reliably reproducible in QEMU. To reproduce, use the kunit configuration from patch 2 of this series and run it like so: ./tools/testing/kunit/kunit.py run --arch mips64el --cross_compile $CROSS_COMPILE cs_dsp_wmfwV3_err_halo.wmfw_v2_coeff_description_exceeds_block To: Mike Rapoport To: Andrew Morton Cc: linux...@kvack.org --- arch/mips/mm/init.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c index 4583d1a2a73e7ff36a42f6017d9aef008e45df6e..712eb4762917261416f1fca2d9925a42107ef6c1 100644 --- a/arch/mips/mm/init.c +++ b/arch/mips/mm/init.c @@ -435,7 +435,9 @@ void __init paging_init(void) #endif high_memory = (void *) __va(max_low_pfn << PAGE_SHIFT); + memblock_set_bottom_up(false); free_area_init(max_zone_pfns); + memblock_set_bottom_up(true); } #ifdef CONFIG_64BIT -- 2.48.1
[PATCH 2/2] kunit: qemu_configs: Add MIPS configurations
Add basic support to run various MIPS variants via kunit_tool using the virtualized malta platform. Signed-off-by: Thomas Weißschuh --- tools/testing/kunit/qemu_configs/mips.py | 18 ++ tools/testing/kunit/qemu_configs/mips64.py | 19 +++ tools/testing/kunit/qemu_configs/mips64el.py | 19 +++ tools/testing/kunit/qemu_configs/mipsel.py | 18 ++ 4 files changed, 74 insertions(+) diff --git a/tools/testing/kunit/qemu_configs/mips.py b/tools/testing/kunit/qemu_configs/mips.py new file mode 100644 index ..8899ac157b30bd2ee847eacd5b90fe6ad4e5fb04 --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mips.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: GPL-2.0 + +from ..qemu_config import QemuArchParams + +QEMU_ARCH = QemuArchParams(linux_arch='mips', + kconfig=''' +CONFIG_32BIT=y +CONFIG_CPU_BIG_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''', + qemu_arch='mips', + kernel_path='vmlinuz', + kernel_command_line='console=ttyS0', + extra_qemu_params=['-M', 'malta']) diff --git a/tools/testing/kunit/qemu_configs/mips64.py b/tools/testing/kunit/qemu_configs/mips64.py new file mode 100644 index ..1478aed05b94da4914f34c6a8affdcfe34eb88ea --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mips64.py @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: GPL-2.0 + +from ..qemu_config import QemuArchParams + +QEMU_ARCH = QemuArchParams(linux_arch='mips', + kconfig=''' +CONFIG_CPU_MIPS64_R2=y +CONFIG_64BIT=y +CONFIG_CPU_BIG_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''', + qemu_arch='mips64', + kernel_path='vmlinuz', + kernel_command_line='console=ttyS0', + extra_qemu_params=['-M', 'malta', '-cpu', '5KEc']) diff --git a/tools/testing/kunit/qemu_configs/mips64el.py b/tools/testing/kunit/qemu_configs/mips64el.py new file mode 100644 index ..300c711d7a82500b2ebcb4cf1467b6f72b5c17aa --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mips64el.py @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: GPL-2.0 + +from ..qemu_config import QemuArchParams + +QEMU_ARCH = QemuArchParams(linux_arch='mips', + kconfig=''' +CONFIG_CPU_MIPS64_R2=y +CONFIG_64BIT=y +CONFIG_CPU_LITTLE_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''', + qemu_arch='mips64el', + kernel_path='vmlinuz', + kernel_command_line='console=ttyS0', + extra_qemu_params=['-M', 'malta', '-cpu', '5KEc']) diff --git a/tools/testing/kunit/qemu_configs/mipsel.py b/tools/testing/kunit/qemu_configs/mipsel.py new file mode 100644 index ..3d3543315b45776d0e77fb5c00c8c0a89eafdffd --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mipsel.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: GPL-2.0 + +from ..qemu_config import QemuArchParams + +QEMU_ARCH = QemuArchParams(linux_arch='mips', + kconfig=''' +CONFIG_32BIT=y +CONFIG_CPU_LITTLE_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''', + qemu_arch='mipsel', + kernel_path='vmlinuz', + kernel_command_line='console=ttyS0', + extra_qemu_params=['-M', 'malta']) -- 2.48.1
Re: [PATCH 2/2] tools/nolibc: add support for directory access
On 2025-02-01 11:46:59+0100, Willy Tarreau wrote: > On Sat, Feb 01, 2025 at 11:41:58AM +0100, Thomas Weißschuh wrote: > > On 2025-02-01 11:34:38+0100, Willy Tarreau wrote: > > > On Thu, Jan 30, 2025 at 08:54:03PM +0100, Thomas Weißschuh wrote: > > > > From: Thomas Weißschuh > > > > > > > > Add an allocation-free implementation of readdir() and related > > > > functions. The implementation is modelled after the one for FILE. > > > > > > I think you'd need to mention/remind the two important points that > > > come out of that choice, one being that DIR is a fake pointer that > > > instead stores ~fd so that it can be turned back to a valid FD, and > > > that subsequent readdir() calls will only work from the same file > > > unit since it relies on a local static storage. > > > > Point one is true. > > Point two not so much. It is fine to have multiple directories open at > > the same time. All state is kept inside the kernel. > > Only the *current* return value is in the static buffer. > > Excellent point! It also needs to be mentioned. Unfortunately POSIX is more specific in it's definition and forbids this: The returned pointer, and pointers within the structure, might be invalidated or the structure or the storage areas might be overwritten by a subsequent call to readdir() on the same directory stream. I see two possibilities: Allocate one 'struct dirent' as part of DIR. Only implement readdir_r(). While readdir_r() is deprecated (according to readdir(3) but not readdir(3p)) this seems to be due to ABI issues, which shouldn't matter for nolibc anyways. Personally I would prefer readdir_r().
Re: [PATCH v2 1/5] selftests/nolibc: drop custom EXTRACONFIG functionality
On 2025-02-01 11:13:05+0100, Willy Tarreau wrote: > On Thu, Jan 23, 2025 at 08:37:37AM +0100, Thomas Weißschuh wrote: > > kbuild already contains logic to merge predefines snippets into a > > defconfig file. This already works nicely with the current "defconfig" > > target. Make use of the snippet and drop the custom logic. > > > > Signed-off-by: Thomas Weißschuh > > --- > > tools/testing/selftests/nolibc/Makefile | 9 + > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/tools/testing/selftests/nolibc/Makefile > > b/tools/testing/selftests/nolibc/Makefile > > index > > 7d14a7c0cb62608f328b251495264517d333db2e..ba044c8a042ce345ff90bdd35569de4b5acd117d > > 100644 > > --- a/tools/testing/selftests/nolibc/Makefile > > +++ b/tools/testing/selftests/nolibc/Makefile > > @@ -82,7 +82,7 @@ DEFCONFIG_x86= defconfig > > DEFCONFIG_arm64 = defconfig > > DEFCONFIG_arm= multi_v7_defconfig > > DEFCONFIG_mips32le = malta_defconfig > > -DEFCONFIG_mips32be = malta_defconfig > > +DEFCONFIG_mips32be = malta_defconfig generic/eb.config > > DEFCONFIG_ppc= pmac32_defconfig > > DEFCONFIG_ppc64 = powernv_be_defconfig > > DEFCONFIG_ppc64le= powernv_defconfig > > @@ -93,9 +93,6 @@ DEFCONFIG_s390 = defconfig > > DEFCONFIG_loongarch = defconfig > > DEFCONFIG= $(DEFCONFIG_$(XARCH)) > > > > -EXTRACONFIG_mips32be = -d CONFIG_CPU_LITTLE_ENDIAN -e CONFIG_CPU_BIG_ENDIAN > > -EXTRACONFIG = $(EXTRACONFIG_$(XARCH)) > > - > > # optional tests to run (default = all) > > TEST = > > > > @@ -265,10 +262,6 @@ initramfs: nolibc-test > > > > defconfig: > > $(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) > > CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare > > - $(Q)if [ -n "$(EXTRACONFIG)" ]; then \ > > - $(srctree)/scripts/config --file $(objtree)/.config > > $(EXTRACONFIG); \ > > - $(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) > > CROSS_COMPILE=$(CROSS_COMPILE) olddefconfig < /dev/null; \ > > - fi > > OK it's no longer needed thanks to your change above, but to we really > want to get rid of that feature allowing anyone to add their own extra > config ? I'm not sure. For example maybe the current build script helps > on bare metal, when trying to add support for new archs or other features > not yet in defconfig ? We could leave EXTRACONFIG_* empty by default and > user-defined, as I don't feel like it blocks anything to keep it. Makes sense, let's keep it. It's currently not possible for kbuild to pick up config snippets outside of a few special locations. I'll fix this up locally and apply the series directly. Thomas
Re: [PATCH 2/2] tools/nolibc: add support for directory access
On 2025-02-01 11:34:38+0100, Willy Tarreau wrote: > On Thu, Jan 30, 2025 at 08:54:03PM +0100, Thomas Weißschuh wrote: > > From: Thomas Weißschuh > > > > Add an allocation-free implementation of readdir() and related > > functions. The implementation is modelled after the one for FILE. > > I think you'd need to mention/remind the two important points that > come out of that choice, one being that DIR is a fake pointer that > instead stores ~fd so that it can be turned back to a valid FD, and > that subsequent readdir() calls will only work from the same file > unit since it relies on a local static storage. Point one is true. Point two not so much. It is fine to have multiple directories open at the same time. All state is kept inside the kernel. Only the *current* return value is in the static buffer. So multithreading wouldn't work, but nolibc doesn't support that anyways. > Better have this visible in the commit message so that in the event > someone faces a difficulty due to this, they can easily find that it's > an on-purpose design choice. Ack. > > Signed-off-by: Thomas Weißschuh > > Signed-off-by: Thomas Weißschuh > > > > --- > > I'm not entirely sure where to put it. It doesn't really belong into > > stdio.h, but that's where the FILE stuff is. > > sys.h wants alphabetical ordering, but IMO these functions should stick > > together. > > My man pages suggest that userland code will include , thus > I think it could be the moment to create it with that new code. Ack. Now that you suggest it, it seems obvious. > > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h > > index > > 3892034198dd566d21a5cc0a9f67cf097d428393..1f275a0a7b6b2c6f1c15405d027c282bb77aa618 > > 100644 > > --- a/tools/include/nolibc/stdio.h > > +++ b/tools/include/nolibc/stdio.h > (...) > > +static __attribute__((unused)) > > +struct dirent *readdir(DIR *dirp) > > +{ > > + static struct dirent dirent; > > + > > + char buf[sizeof(struct linux_dirent64) + NAME_MAX]; > > I'm uncertain where NAME_MAX is defined, I haven't found it in the > nolibc sources, just double-checking that it's not just in your build > environment by accident. It comes from linux/limits.h. I don't think it can ever go away. > > + struct linux_dirent64 *ldir = (void *)buf; > > + intptr_t i = (intptr_t)dirp; > > + int fd, ret; > > + > > + if (i >= 0) { > > + SET_ERRNO(EBADF); > > + return NULL; > > + } > > + > > + fd = ~i; > > + > > + ret = getdents64(fd, ldir, sizeof(buf)); > > + if (ret == -1 || ret == 0) > > + return NULL; > > + > > + /* > > +* getdents64() returns as many entries as fit the buffer. > > +* readdir() can only return one entry at a time. > > +* Make sure the non-returned ones are not skipped. > > +*/ > > + ret = lseek(fd, ldir->d_off, SEEK_SET); > > + if (ret == -1) > > + return NULL; > > + > > + dirent = (struct dirent) { > > + .d_ino = ldir->d_ino, > > + }; > > + strlcpy(dirent.d_name, ldir->d_name, sizeof(dirent.d_name)); > > Just out of curiosity, could this copy fail, and if so, should we handle > it (e.g. NAME_MAX != 255) ? My guess here is that if it could almost never > fail and checking it would needlessly complicate the function, let's just > handle it with a comment for now. And if it cannot at all, let's mention > why on top of it as well. If NAME_MAX changes it would be a userspace regression IMO. I'll add a comment. Thanks! Thomas
[PATCH 3/3] module: Constify 'struct module_attribute'
These structs are never modified, move them to read-only memory. This makes the API clearer and also prepares for the constification of 'struct attribute' itself. While at it, also constify 'modinfo_attrs_count'. Signed-off-by: Thomas Weißschuh --- include/linux/module.h | 8 kernel/module/internal.h | 4 ++-- kernel/module/main.c | 40 kernel/module/sysfs.c| 4 ++-- kernel/params.c | 12 ++-- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index de2f2293204a4681072fba9ea3439e5582c81fbf..81a0dd46a5d2c29c30ea2cb8d82147ba2fa2a0a8 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -52,9 +52,9 @@ struct module_kobject { struct module_attribute { struct attribute attr; - ssize_t (*show)(struct module_attribute *, struct module_kobject *, + ssize_t (*show)(const struct module_attribute *, struct module_kobject *, char *); - ssize_t (*store)(struct module_attribute *, struct module_kobject *, + ssize_t (*store)(const struct module_attribute *, struct module_kobject *, const char *, size_t count); void (*setup)(struct module *, const char *); int (*test)(struct module *); @@ -67,10 +67,10 @@ struct module_version_attribute { const char *version; }; -extern ssize_t __modver_version_show(struct module_attribute *, +extern ssize_t __modver_version_show(const struct module_attribute *, struct module_kobject *, char *); -extern struct module_attribute module_uevent; +extern const struct module_attribute module_uevent; /* These are either module local, or the kernel's dummy ones. */ extern int init_module(void); diff --git a/kernel/module/internal.h b/kernel/module/internal.h index daef2be8390222c0e2f168baa8d35ad531b9..ac73da5f15bccfa6e280669c6ce048868120822b 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -47,8 +47,8 @@ struct kernel_symbol { extern struct mutex module_mutex; extern struct list_head modules; -extern struct module_attribute *modinfo_attrs[]; -extern size_t modinfo_attrs_count; +extern const struct module_attribute *const modinfo_attrs[]; +extern const size_t modinfo_attrs_count; /* Provided by the linker */ extern const struct kernel_symbol __start___ksymtab[]; diff --git a/kernel/module/main.c b/kernel/module/main.c index 5399c182b3cbed2dbeea0291f717f30358d8e7fc..69be1dad032abe53d55b437411f152aa95e4adf6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -538,7 +538,7 @@ static void setup_modinfo_##field(struct module *mod, const char *s) \ { \ mod->field = kstrdup(s, GFP_KERNEL); \ } \ -static ssize_t show_modinfo_##field(struct module_attribute *mattr, \ +static ssize_t show_modinfo_##field(const struct module_attribute *mattr, \ struct module_kobject *mk, char *buffer) \ { \ return scnprintf(buffer, PAGE_SIZE, "%s\n", mk->mod->field); \ @@ -552,7 +552,7 @@ static void free_modinfo_##field(struct module *mod) \ kfree(mod->field);\ mod->field = NULL;\ } \ -static struct module_attribute modinfo_##field = {\ +static const struct module_attribute modinfo_##field = { \ .attr = { .name = __stringify(field), .mode = 0444 }, \ .show = show_modinfo_##field, \ .setup = setup_modinfo_##field, \ @@ -842,13 +842,13 @@ void symbol_put_addr(void *addr) } EXPORT_SYMBOL_GPL(symbol_put_addr); -static ssize_t show_refcnt(struct module_attribute *mattr, +static ssize_t show_refcnt(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { return sprintf(buffer, "%i\n", module_refcount(mk->mod)); } -static struct module_attribute modinfo_refcnt = +static const struct module_attribute modinfo_refcnt = __ATTR(refcnt, 0444, show_refcnt, NULL); void __module_get(struct module *module) @@ -917,7 +917,7 @@ size_t module_flags_taint(unsigned long taints, char *buf) return l; } -static ssize_t show_initstate(struct module_attribute *mattr, +static ssize_t show_initstate(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { const char *state = "
[PATCH 2/3] module: Handle 'struct module_version_attribute' as const
The structure is always read-only due to its placement in the read-only section __modver. Reflect this at its usage sites. Also prepare for the const handling of 'struct module_attribute' itself. Signed-off-by: Thomas Weißschuh --- include/linux/module.h | 2 +- kernel/params.c| 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index 94acbacdcdf189e27013088de2202bccac9717e0..de2f2293204a4681072fba9ea3439e5582c81fbf 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -275,7 +275,7 @@ extern typeof(name) __mod_device_table__##type##__##name \ #else #define MODULE_VERSION(_version) \ MODULE_INFO(version, _version); \ - static struct module_version_attribute __modver_attr\ + static const struct module_version_attribute __modver_attr \ __used __section("__modver")\ __aligned(__alignof__(struct module_version_attribute)) \ = { \ diff --git a/kernel/params.c b/kernel/params.c index e90733824528eacc77046f85c9ab4243467ca841..763261a7fef94d02503fa0d365d155c223fc382b 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -860,8 +860,8 @@ static void __init param_sysfs_builtin(void) ssize_t __modver_version_show(struct module_attribute *mattr, struct module_kobject *mk, char *buf) { - struct module_version_attribute *vattr = - container_of(mattr, struct module_version_attribute, mattr); + const struct module_version_attribute *vattr = + container_of_const(mattr, struct module_version_attribute, mattr); return scnprintf(buf, PAGE_SIZE, "%s\n", vattr->version); } -- 2.47.1
[PATCH 0/3] module: Constify 'struct module_attribute'
These structs are never modified, move them to read-only memory. This makes the API clearer and also prepares for the constification of 'struct attribute' itself. Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (3): params: Prepare for 'const struct module_attribute *' module: Handle 'struct module_version_attribute' as const module: Constify 'struct module_attribute' include/linux/module.h | 10 +- kernel/module/internal.h | 4 ++-- kernel/module/main.c | 40 kernel/module/sysfs.c| 4 ++-- kernel/params.c | 22 +++--- 5 files changed, 40 insertions(+), 40 deletions(-) --- base-commit: 78d4f34e2115b517bcbfe7ec0d0186f9b0b8 change-id: 20241204-sysfs-const-attr-module-927afe76eda4 Best regards, -- Thomas Weißschuh
[PATCH 1/3] params: Prepare for 'const struct module_attribute *'
The 'struct module_attribute' sysfs callbacks are about to change to receive a 'const struct module_attribute *' parameter. Prepare for that by avoid casting away the constness through container_of() and using const pointers to 'struct param_attribute'. Signed-off-by: Thomas Weißschuh --- kernel/params.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/params.c b/kernel/params.c index 2e447f8ae183e7ec6d1815c862e0cec6572099d4..e90733824528eacc77046f85c9ab4243467ca841 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -555,13 +555,13 @@ struct module_param_attrs }; #ifdef CONFIG_SYSFS -#define to_param_attr(n) container_of(n, struct param_attribute, mattr) +#define to_param_attr(n) container_of_const(n, struct param_attribute, mattr) static ssize_t param_attr_show(struct module_attribute *mattr, struct module_kobject *mk, char *buf) { int count; - struct param_attribute *attribute = to_param_attr(mattr); + const struct param_attribute *attribute = to_param_attr(mattr); if (!attribute->param->ops->get) return -EPERM; @@ -578,7 +578,7 @@ static ssize_t param_attr_store(struct module_attribute *mattr, const char *buf, size_t len) { int err; - struct param_attribute *attribute = to_param_attr(mattr); + const struct param_attribute *attribute = to_param_attr(mattr); if (!attribute->param->ops->set) return -EPERM; -- 2.47.1
Re: [PATCH 2/4] platform/x86: wmi-bmof: Switch to sysfs_bin_attr_simple_read()
Hi Armin, On 2024-12-13 01:21:37+0100, Armin Wolf wrote: > Am 05.12.24 um 18:35 schrieb Thomas Weißschuh: > > > The generic function from the sysfs core can replace the custom one. > > Sorry for taking quite a bit to respond, i totally overlooked this patch. > > This patch is superseded by a patch of mine: > https://lore.kernel.org/platform-driver-x86/20241206215650.2977-1-w_ar...@gmx.de/ > > This reworks the binary attribute handling inside the driver to use the new > .bin_size() callback. This allows the > driver to have a static binary attribute which does not need a memory > allocation. > > Because i think we cannot use sysfs_bin_attr_simple_read() anymore. So maybe > you can just drop this patch? Works for me, thanks! Thomas > > Signed-off-by: Thomas Weißschuh > > --- > > drivers/platform/x86/wmi-bmof.c | 12 ++-- > > 1 file changed, 2 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/platform/x86/wmi-bmof.c > > b/drivers/platform/x86/wmi-bmof.c > > index > > df6f0ae6e6c7904f97c125297a21166f56d0b1f0..e6c217d70086a2896dc70cf8ac1c27dafb501a95 > > 100644 > > --- a/drivers/platform/x86/wmi-bmof.c > > +++ b/drivers/platform/x86/wmi-bmof.c > > @@ -25,15 +25,6 @@ struct bmof_priv { > > struct bin_attribute bmof_bin_attr; > > }; > > > > -static ssize_t read_bmof(struct file *filp, struct kobject *kobj, struct > > bin_attribute *attr, > > -char *buf, loff_t off, size_t count) > > -{ > > - struct bmof_priv *priv = container_of(attr, struct bmof_priv, > > bmof_bin_attr); > > - > > - return memory_read_from_buffer(buf, count, &off, > > priv->bmofdata->buffer.pointer, > > - priv->bmofdata->buffer.length); > > -} > > - > > static int wmi_bmof_probe(struct wmi_device *wdev, const void *context) > > { > > struct bmof_priv *priv; > > @@ -60,7 +51,8 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const > > void *context) > > sysfs_bin_attr_init(&priv->bmof_bin_attr); > > priv->bmof_bin_attr.attr.name = "bmof"; > > priv->bmof_bin_attr.attr.mode = 0400; > > - priv->bmof_bin_attr.read = read_bmof; > > + priv->bmof_bin_attr.read_new = sysfs_bin_attr_simple_read; > > + priv->bmof_bin_attr.private = priv->bmofdata->buffer.pointer; > > priv->bmof_bin_attr.size = priv->bmofdata->buffer.length; > > > > ret = device_create_bin_file(&wdev->dev, &priv->bmof_bin_attr); > >
[PATCH 3/6] selftests/nolibc: use a pipe to in vfprintf tests
Not all architectures implement lseek(), for example riscv32 only implements llseek() which is not equivalent to normal lseek(). Remove the need for lseek() by using a pipe instead. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/nolibc-test.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 60c50968d3630e4909a5ecb2400770baaf7c2add..3685c13a9a6b8fd5110715b95ff323cdcb29481a 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -1229,19 +1229,20 @@ int run_stdlib(int min, int max) static int expect_vfprintf(int llen, int c, const char *expected, const char *fmt, ...) { - int ret, fd; + int ret, pipefd[2]; ssize_t w, r; char buf[100]; FILE *memfile; va_list args; - fd = open("/tmp", O_TMPFILE | O_EXCL | O_RDWR, 0600); - if (fd == -1) { - result(llen, SKIPPED); - return 0; + ret = pipe(pipefd); + if (ret == -1) { + llen += printf(" pipe() != %s", strerror(errno)); + result(llen, FAIL); + return 1; } - memfile = fdopen(fd, "w+"); + memfile = fdopen(pipefd[1], "w"); if (!memfile) { result(llen, FAIL); return 1; @@ -1257,13 +1258,10 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm return 1; } - fflush(memfile); - lseek(fd, 0, SEEK_SET); - - r = read(fd, buf, sizeof(buf) - 1); - fclose(memfile); + r = read(pipefd[0], buf, sizeof(buf) - 1); + if (r != w) { llen += printf(" written(%d) != read(%d)", (int)w, (int)r); result(llen, FAIL); -- 2.47.1
[PATCH 2/6] selftests/nolibc: use waitid() over waitpid()
Newer archs like riscv32 don't provide waitpid() anymore. Switch to waitid() which is available everywhere. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/nolibc-test.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 6fba7025c5e3c002085862fdf6fa950da6000d6c..60c50968d3630e4909a5ecb2400770baaf7c2add 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -1323,7 +1323,8 @@ static int run_protection(int min __attribute__((unused)), int max __attribute__((unused))) { pid_t pid; - int llen = 0, status; + int llen = 0, ret; + siginfo_t siginfo = {}; struct rlimit rlimit = { 0, 0 }; llen += printf("0 -fstackprotector "); @@ -1361,10 +1362,11 @@ static int run_protection(int min __attribute__((unused)), return 1; default: - pid = waitpid(pid, &status, 0); + ret = waitid(P_PID, pid, &siginfo, WEXITED); - if (pid == -1 || !WIFSIGNALED(status) || WTERMSIG(status) != SIGABRT) { - llen += printf("waitpid()"); + if (ret != 0 || siginfo.si_signo != SIGCHLD || + siginfo.si_code != CLD_KILLED || siginfo.si_status != SIGABRT) { + llen += printf("waitid()"); result(llen, FAIL); return 1; } -- 2.47.1
[PATCH 5/6] selftests/nolibc: rename riscv to riscv64
riscv32 support is about the be added. To keep the naming clear and consistent with other architectures rename riscv to riscv64, as that is what it actually represents. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/Makefile | 6 ++ tools/testing/selftests/nolibc/run-tests.sh | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index e92e0b88586111072a0e043cb15f3b59cf42c3a6..78f47e85b389ac229ac13f3e7c8299fb3ec92197 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -43,6 +43,7 @@ cc-option = $(call __cc-option, $(CC),$(CLANG_CROSS_FLAGS),$(1),$(2)) # configure default variants for target kernel supported architectures XARCH_powerpc= ppc XARCH_mips = mips32le +XARCH_riscv = riscv64 XARCH= $(or $(XARCH_$(ARCH)),$(ARCH)) # map from user input variants to their kernel supported architectures @@ -51,6 +52,7 @@ ARCH_ppc64 = powerpc ARCH_ppc64le = powerpc ARCH_mips32le= mips ARCH_mips32be= mips +ARCH_riscv64 = riscv ARCH:= $(or $(ARCH_$(XARCH)),$(XARCH)) # kernel image names by architecture @@ -65,6 +67,7 @@ IMAGE_ppc= vmlinux IMAGE_ppc64 = vmlinux IMAGE_ppc64le= arch/powerpc/boot/zImage IMAGE_riscv = arch/riscv/boot/Image +IMAGE_riscv64= arch/riscv/boot/Image IMAGE_s390 = arch/s390/boot/bzImage IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi IMAGE= $(objtree)/$(IMAGE_$(XARCH)) @@ -82,6 +85,7 @@ DEFCONFIG_ppc= pmac32_defconfig DEFCONFIG_ppc64 = powernv_be_defconfig DEFCONFIG_ppc64le= powernv_defconfig DEFCONFIG_riscv = defconfig +DEFCONFIG_riscv64= defconfig DEFCONFIG_s390 = defconfig DEFCONFIG_loongarch = defconfig DEFCONFIG= $(DEFCONFIG_$(XARCH)) @@ -104,6 +108,7 @@ QEMU_ARCH_ppc= ppc QEMU_ARCH_ppc64 = ppc64 QEMU_ARCH_ppc64le= ppc64 QEMU_ARCH_riscv = riscv64 +QEMU_ARCH_riscv64= riscv64 QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 QEMU_ARCH= $(QEMU_ARCH_$(XARCH)) @@ -130,6 +135,7 @@ QEMU_ARGS_ppc= -M g3beige -append "console=ttyS0 panic=-1 $(TEST:%=NOLIB QEMU_ARGS_ppc64 = -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_ppc64le= -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" +QEMU_ARGS_riscv64= -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_s390 = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS= -m 1G $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_BIOS) $(QEMU_ARGS_EXTRA) diff --git a/tools/testing/selftests/nolibc/run-tests.sh b/tools/testing/selftests/nolibc/run-tests.sh index e7ecda4ae796fbf0d389f20144511e66ce4f0b30..caa1ae40fe9a2faf8931c299aacd19716227e2b8 100755 --- a/tools/testing/selftests/nolibc/run-tests.sh +++ b/tools/testing/selftests/nolibc/run-tests.sh @@ -17,7 +17,7 @@ perform_download=0 test_mode=system werror=1 llvm= -archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv s390 loongarch" +archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv64 s390 loongarch" TEMP=$(getopt -o 'j:d:c:b:a:m:pelh' -n "$0" -- "$@") -- 2.47.1
[PATCH 4/6] selftests/nolibc: skip tests for unimplemented syscalls
The riscv32 architecture is missing many of the older syscalls. Instead of providing wrappers for everything at once, introducing a lot of complexity, skip the tests for those syscalls for now. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/nolibc-test.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 3685c13a9a6b8fd5110715b95ff323cdcb29481a..0e0e3b48a8c3a6802c6989954b6f3a7c7258db43 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -302,7 +302,10 @@ int expect_syszr(int expr, int llen) { int ret = 0; - if (expr) { + if (errno == ENOSYS) { + llen += printf(" = ENOSYS"); + result(llen, SKIPPED); + } else if (expr) { ret = 1; llen += printf(" = %d %s ", expr, errorname(errno)); result(llen, FAIL); @@ -342,7 +345,10 @@ int expect_sysne(int expr, int llen, int val) { int ret = 0; - if (expr == val) { + if (errno == ENOSYS) { + llen += printf(" = ENOSYS"); + result(llen, SKIPPED); + } else if (expr == val) { ret = 1; llen += printf(" = %d %s ", expr, errorname(errno)); result(llen, FAIL); @@ -367,7 +373,9 @@ int expect_syserr2(int expr, int expret, int experr1, int experr2, int llen) int _errno = errno; llen += printf(" = %d %s ", expr, errorname(_errno)); - if (expr != expret || (_errno != experr1 && _errno != experr2)) { + if (errno == ENOSYS) { + result(llen, SKIPPED); + } else if (expr != expret || (_errno != experr1 && _errno != experr2)) { ret = 1; if (experr2 == 0) llen += printf(" != (%d %s) ", expret, errorname(experr1)); -- 2.47.1
[PATCH 6/6] selftests/nolibc: add configurations for riscv32
nolibc already supports riscv32. Wire it up in the testsuite. Signed-off-by: Thomas Weißschuh --- tools/testing/selftests/nolibc/Makefile | 5 + tools/testing/selftests/nolibc/run-tests.sh | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 78f47e85b389ac229ac13f3e7c8299fb3ec92197..7d14a7c0cb62608f328b251495264517d333db2e 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -52,6 +52,7 @@ ARCH_ppc64 = powerpc ARCH_ppc64le = powerpc ARCH_mips32le= mips ARCH_mips32be= mips +ARCH_riscv32 = riscv ARCH_riscv64 = riscv ARCH:= $(or $(ARCH_$(XARCH)),$(XARCH)) @@ -67,6 +68,7 @@ IMAGE_ppc= vmlinux IMAGE_ppc64 = vmlinux IMAGE_ppc64le= arch/powerpc/boot/zImage IMAGE_riscv = arch/riscv/boot/Image +IMAGE_riscv32= arch/riscv/boot/Image IMAGE_riscv64= arch/riscv/boot/Image IMAGE_s390 = arch/s390/boot/bzImage IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi @@ -85,6 +87,7 @@ DEFCONFIG_ppc= pmac32_defconfig DEFCONFIG_ppc64 = powernv_be_defconfig DEFCONFIG_ppc64le= powernv_defconfig DEFCONFIG_riscv = defconfig +DEFCONFIG_riscv32= rv32_defconfig DEFCONFIG_riscv64= defconfig DEFCONFIG_s390 = defconfig DEFCONFIG_loongarch = defconfig @@ -108,6 +111,7 @@ QEMU_ARCH_ppc= ppc QEMU_ARCH_ppc64 = ppc64 QEMU_ARCH_ppc64le= ppc64 QEMU_ARCH_riscv = riscv64 +QEMU_ARCH_riscv32= riscv32 QEMU_ARCH_riscv64= riscv64 QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 @@ -135,6 +139,7 @@ QEMU_ARGS_ppc= -M g3beige -append "console=ttyS0 panic=-1 $(TEST:%=NOLIB QEMU_ARGS_ppc64 = -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_ppc64le= -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" +QEMU_ARGS_riscv32= -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv64= -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_s390 = -M s390-ccw-virtio -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)" diff --git a/tools/testing/selftests/nolibc/run-tests.sh b/tools/testing/selftests/nolibc/run-tests.sh index caa1ae40fe9a2faf8931c299aacd19716227e2b8..67fefc847d77165f817c3befa578cfa27e6f93d8 100755 --- a/tools/testing/selftests/nolibc/run-tests.sh +++ b/tools/testing/selftests/nolibc/run-tests.sh @@ -17,7 +17,7 @@ perform_download=0 test_mode=system werror=1 llvm= -archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv64 s390 loongarch" +archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv32 riscv64 s390 loongarch" TEMP=$(getopt -o 'j:d:c:b:a:m:pelh' -n "$0" -- "$@") -- 2.47.1
[PATCH 0/6] selftests/nolibc: wire up riscv32
Nolibc has support for riscv32. But the testsuite did not allow to test it so far. Add a test configuration. Signed-off-by: Thomas Weißschuh --- Thomas Weißschuh (6): tools/nolibc: add support for waitid() selftests/nolibc: use waitid() over waitpid() selftests/nolibc: use a pipe to in vfprintf tests selftests/nolibc: skip tests for unimplemented syscalls selftests/nolibc: rename riscv to riscv64 selftests/nolibc: add configurations for riscv32 tools/include/nolibc/sys.h | 18 tools/testing/selftests/nolibc/Makefile | 11 +++ tools/testing/selftests/nolibc/nolibc-test.c | 44 tools/testing/selftests/nolibc/run-tests.sh | 2 +- 4 files changed, 56 insertions(+), 19 deletions(-) --- base-commit: 499551201b5f4fd3c0618a3e95e3d0d15ea18f31 change-id: 20241219-nolibc-rv32-cff8a3e22394 Best regards, -- Thomas Weißschuh
[PATCH 1/6] tools/nolibc: add support for waitid()
waitid() is the modern variant of the family of wait-like syscalls. Some architectures have dropped support for wait(), wait4() and waitpid() but all of them support waitid(). It is more flexible and easier to use than the older ones. Signed-off-by: Thomas Weißschuh --- tools/include/nolibc/sys.h | 18 ++ 1 file changed, 18 insertions(+) diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 7b82bc3cf107439a3f09f98b99d4d540ffb9ba2a..d4a5c2399a66b200ebf7ab249569cce2285481a5 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -23,6 +23,7 @@ #include #include #include +#include #include "arch.h" #include "errno.h" @@ -1225,6 +1226,23 @@ pid_t waitpid(pid_t pid, int *status, int options) } +/* + * int waitid(idtype_t idtype, id_t id, siginfo_t *infop, int options); + */ + +static __attribute__((unused)) +int sys_waitid(int which, pid_t pid, siginfo_t *infop, int options, struct rusage *rusage) +{ + return my_syscall5(__NR_waitid, which, pid, infop, options, rusage); +} + +static __attribute__((unused)) +int waitid(int which, pid_t pid, siginfo_t *infop, int options) +{ + return __sysret(sys_waitid(which, pid, infop, options, NULL)); +} + + /* * ssize_t write(int fd, const void *buf, size_t count); */ -- 2.47.1
Re: [PATCH 1/3] selftests/mm: virtual_address_range: Fix error when CommitLimit < 1GiB
On Wed, Jan 08, 2025 at 05:46:37PM +0100, David Hildenbrand wrote: > On 08.01.25 17:13, Thomas Weißschuh wrote: > > On Wed, Jan 08, 2025 at 02:36:57PM +0100, David Hildenbrand wrote: > > > On 08.01.25 09:05, Thomas Weißschuh wrote: > > > > On Wed, Jan 08, 2025 at 11:46:19AM +0530, Dev Jain wrote: > > > > > > > > > > On 07/01/25 8:44 pm, Thomas Weißschuh wrote: > > > > > > If not enough physical memory is available the kernel may fail > > > > > > mmap(); > > > > > > see __vm_enough_memory() and vm_commit_limit(). > > > > > > In that case the logic in validate_complete_va_space() does not make > > > > > > sense and will even incorrectly fail. > > > > > > Instead skip the test if no mmap() succeeded. > > > > > > > > > > > > Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without > > > > > > reliance on correctness of mmap()") > > > > > > Cc: sta...@vger.kernel.org > > > > > > CC stable on tests is ... odd. > > > > I thought it was fairly common, but it isn't. > > Will drop it. > > As it's not really a "kernel BUG", it's rather uncommon. I also used it on patch 2, which is now reproducibly broken on x86 mainline since my commit mentioned in that patch. But I'll drop it there, too. > > > Note that with MAP_NORESRVE, most setups we care about will allow mapping > > > as > > > much as you want, but on access OOM will fire. > > > > Thanks for the hint. > > > > > So one could require that /proc/sys/vm/overcommit_memory is setup properly > > > and use MAP_NORESRVE. > > > > Isn't the check for lchunks == 0 essentially exactly this? > > I assume paired with MAP_NORESERVE? Yes. > Maybe, but it could be better to have something that says "if > overcommit_memory is not setup properly I will SKIP this test", but > otherwise I expect this to work and will FAIL if it doesn't". Ok, I'll validate the sysctl value. > Or would you expect to run into lchunks == 0 even if overcommit_memory is > setup properly and MAP_NORESERVE is used? (very very low memory that we > cannot even create all the VMAs?) No. > > > Reading from anonymous memory will populate the shared zeropage. To > > > mitigate > > > OOM from "too many page tables", one could simply unmap the pieces as they > > > are verified (or MAP_FIXED over them, to free page tables). > > > > The code has to figure out if a verified region was created by mmap(), > > otherwise an munmap() could crash the process. > > As the entries from /proc/self/maps may have been merged and (I assume) > > Yes, and partial unmap (in chunk granularity?) would split them again. > > > the ordering of mappings is not guaranteed, some bespoke logic to establish > > the link will be needed. > > My thinking was that you simply process one /proc/self/maps entry in some > chunks. After processing a chunk, you munmap() it. > > So you would process + munmap in chunks. That is clear. The issue would be to figure which chunks are valid to unmap. If something critical like the executable file is unmapped, the process crashes. But see below. > > Is it fine to rely on CONFIG_ANON_VMA_NAME? > > That would make it much easier to implement. > > Can you elaborate how you would do it? First set the VMA name after mmap(): for (i = 0; i < NR_CHUNKS_LOW; i++) { ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE, MAP_NORESERVE | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); if (ptr[i] == MAP_FAILED) { if (validate_lower_address_hint()) ksft_exit_fail_msg("mmap unexpectedly succeeded with hint\n"); break; } validate_addr(ptr[i], 0); if (prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ptr[i], MAP_CHUNK_SIZE, "virtual_address_range")) ksft_exit_fail_msg("prctl(PR_SET_VMA_ANON_NAME) failed: %s\n", strerror(errno)); } During validation: hop = 0; while (start_addr + hop < end_addr) { if (write(fd, (void *)(start_addr + hop), 1) != 1) return 1; lseek(fd, 0, SEEK_SET); if (!strncmp(line + path_offset, "[anon:virtual_address_range]", 28)) munmap((char *)(start_addr + hop), MAP_CHUNK_SIZE); hop += MAP_CHUNK_SIZE; } It is done for each chunk, as all chunks may have been merged into a single VMA and a per-VMA unmap would not happen before OOM. > > Using MAP_NORESERVE and eager munmap()s, the testcase works nicely even > > in very low physical memory conditions. > > Cool.