RISC-V: hwprobe: Clarify cpus size parameter
authorAndrew Jones <ajones@ventanamicro.com>
Wed, 22 Nov 2023 16:47:02 +0000 (17:47 +0100)
committerPalmer Dabbelt <palmer@rivosinc.com>
Wed, 3 Jan 2024 11:36:47 +0000 (03:36 -0800)
The "count" parameter associated with the 'cpus' parameter of the
hwprobe syscall is the size in bytes of 'cpus'. Naming it 'cpu_count'
may mislead users (it did me) to think it's the number of CPUs that
are or can be represented by 'cpus' instead. This is particularly
easy (IMO) to get wrong since 'cpus' is documented to be defined by
CPU_SET(3) and CPU_SET(3) also documents a CPU_COUNT() (the number
of CPUs in set) macro. CPU_SET(3) refers to the size of cpu sets
with 'setsize'. Adopt 'cpusetsize' for the hwprobe parameter and
specifically state it is in bytes in Documentation/riscv/hwprobe.rst
to clarify.

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Link: https://lore.kernel.org/r/20231122164700.127954-7-ajones@ventanamicro.com
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
Documentation/arch/riscv/hwprobe.rst
arch/riscv/kernel/sys_riscv.c
arch/riscv/kernel/vdso/hwprobe.c
tools/testing/selftests/riscv/hwprobe/hwprobe.c
tools/testing/selftests/riscv/hwprobe/hwprobe.h
tools/testing/selftests/riscv/vector/vstate_prctl.c

index 7b2384de471f8fa5813271a010f8f5f011e7d8c3..132e9acaa8f45a8337b89512ff2398191275ceb1 100644 (file)
@@ -12,7 +12,7 @@ is defined in <asm/hwprobe.h>::
     };
 
     long sys_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-                           size_t cpu_count, cpu_set_t *cpus,
+                           size_t cpusetsize, cpu_set_t *cpus,
                            unsigned int flags);
 
 The arguments are split into three groups: an array of key-value pairs, a CPU
@@ -20,12 +20,13 @@ set, and some flags. The key-value pairs are supplied with a count. Userspace
 must prepopulate the key field for each element, and the kernel will fill in the
 value if the key is recognized. If a key is unknown to the kernel, its key field
 will be cleared to -1, and its value set to 0. The CPU set is defined by
-CPU_SET(3). For value-like keys (eg. vendor/arch/impl), the returned value will
-be only be valid if all CPUs in the given set have the same value. Otherwise -1
-will be returned. For boolean-like keys, the value returned will be a logical
-AND of the values for the specified CPUs. Usermode can supply NULL for cpus and
-0 for cpu_count as a shortcut for all online CPUs. There are currently no flags,
-this value must be zero for future compatibility.
+CPU_SET(3) with size ``cpusetsize`` bytes. For value-like keys (eg. vendor,
+arch, impl), the returned value will only be valid if all CPUs in the given set
+have the same value. Otherwise -1 will be returned. For boolean-like keys, the
+value returned will be a logical AND of the values for the specified CPUs.
+Usermode can supply NULL for ``cpus`` and 0 for ``cpusetsize`` as a shortcut for
+all online CPUs. There are currently no flags, this value must be zero for
+future compatibility.
 
 On success 0 is returned, on failure a negative error code is returned.
 
index c712037dbe10ec88b9ee8f5d8559f9b73c6608fa..5e4deb1308b8697f4fdcde4bbdeb060b846d86e4 100644 (file)
@@ -246,7 +246,7 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
 }
 
 static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs,
-                           size_t pair_count, size_t cpu_count,
+                           size_t pair_count, size_t cpusetsize,
                            unsigned long __user *cpus_user,
                            unsigned int flags)
 {
@@ -264,13 +264,13 @@ static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs,
         * 0 as a shortcut to all online CPUs.
         */
        cpumask_clear(&cpus);
-       if (!cpu_count && !cpus_user) {
+       if (!cpusetsize && !cpus_user) {
                cpumask_copy(&cpus, cpu_online_mask);
        } else {
-               if (cpu_count > cpumask_size())
-                       cpu_count = cpumask_size();
+               if (cpusetsize > cpumask_size())
+                       cpusetsize = cpumask_size();
 
-               ret = copy_from_user(&cpus, cpus_user, cpu_count);
+               ret = copy_from_user(&cpus, cpus_user, cpusetsize);
                if (ret)
                        return -EFAULT;
 
@@ -347,10 +347,10 @@ arch_initcall_sync(init_hwprobe_vdso_data);
 #endif /* CONFIG_MMU */
 
 SYSCALL_DEFINE5(riscv_hwprobe, struct riscv_hwprobe __user *, pairs,
-               size_t, pair_count, size_t, cpu_count, unsigned long __user *,
+               size_t, pair_count, size_t, cpusetsize, unsigned long __user *,
                cpus, unsigned int, flags)
 {
-       return do_riscv_hwprobe(pairs, pair_count, cpu_count,
+       return do_riscv_hwprobe(pairs, pair_count, cpusetsize,
                                cpus, flags);
 }
 
index cadf725ef798370bbe248f80dcf207c7fd439e7b..026b7645c5ab05cae0239db7d3a583f8b31147e7 100644 (file)
@@ -8,21 +8,21 @@
 #include <vdso/helpers.h>
 
 extern int riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-                        size_t cpu_count, unsigned long *cpus,
+                        size_t cpusetsize, unsigned long *cpus,
                         unsigned int flags);
 
 /* Add a prototype to avoid -Wmissing-prototypes warning. */
 int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-                        size_t cpu_count, unsigned long *cpus,
+                        size_t cpusetsize, unsigned long *cpus,
                         unsigned int flags);
 
 int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-                        size_t cpu_count, unsigned long *cpus,
+                        size_t cpusetsize, unsigned long *cpus,
                         unsigned int flags)
 {
        const struct vdso_data *vd = __arch_get_vdso_data();
        const struct arch_vdso_data *avd = &vd->arch_data;
-       bool all_cpus = !cpu_count && !cpus;
+       bool all_cpus = !cpusetsize && !cpus;
        struct riscv_hwprobe *p = pairs;
        struct riscv_hwprobe *end = pairs + pair_count;
 
@@ -33,7 +33,7 @@ int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
         * masks.
         */
        if ((flags != 0) || (!all_cpus && !avd->homogeneous_cpus))
-               return riscv_hwprobe(pairs, pair_count, cpu_count, cpus, flags);
+               return riscv_hwprobe(pairs, pair_count, cpusetsize, cpus, flags);
 
        /* This is something we can handle, fill out the pairs. */
        while (p < end) {
index c474891df307140fdd21a2a89b4d495790b47181..d53e0889b59e1e148f033501b4ba48176a2cb81b 100644 (file)
@@ -47,7 +47,7 @@ int main(int argc, char **argv)
        ksft_test_result(out != 0, "Bad CPU set\n");
 
        out = riscv_hwprobe(pairs, 8, 1, 0, 0);
-       ksft_test_result(out != 0, "NULL CPU set with non-zero count\n");
+       ksft_test_result(out != 0, "NULL CPU set with non-zero size\n");
 
        pairs[0].key = RISCV_HWPROBE_KEY_BASE_BEHAVIOR;
        out = riscv_hwprobe(pairs, 1, 1, &cpus, 0);
index 721b0ce73a56907d1759a666ca4a43cc0a2aa957..e3fccb390c4dc94d0c224223192f767606b4da17 100644 (file)
@@ -10,6 +10,6 @@
  * contain the call.
  */
 long riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-                  size_t cpu_count, unsigned long *cpus, unsigned int flags);
+                  size_t cpusetsize, unsigned long *cpus, unsigned int flags);
 
 #endif
index b348b475be570cdd14d9c7de13c35ece1dc3901c..8dcd399ef7fc9fb719863999a60b26a538605877 100644 (file)
@@ -1,20 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-only
 #include <sys/prctl.h>
 #include <unistd.h>
-#include <asm/hwprobe.h>
 #include <errno.h>
 #include <sys/wait.h>
 
+#include "../hwprobe/hwprobe.h"
 #include "../../kselftest.h"
 
-/*
- * Rather than relying on having a new enough libc to define this, just do it
- * ourselves.  This way we don't need to be coupled to a new-enough libc to
- * contain the call.
- */
-long riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-                  size_t cpu_count, unsigned long *cpus, unsigned int flags);
-
 #define NEXT_PROGRAM "./vstate_exec_nolibc"
 static int launch_test(int test_inherit)
 {