Commit 36d842d6 authored by Andrew Jones's avatar Andrew Jones Committed by Palmer Dabbelt

RISC-V: hwprobe: Clarify cpus size parameter

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: default avatarPalmer Dabbelt <palmer@rivosinc.com>
Reviewed-by: default avatarConor Dooley <conor.dooley@microchip.com>
Signed-off-by: default avatarAndrew Jones <ajones@ventanamicro.com>
Link: https://lore.kernel.org/r/20231122164700.127954-7-ajones@ventanamicro.comSigned-off-by: default avatarPalmer Dabbelt <palmer@rivosinc.com>
parent b85ea95d
...@@ -12,7 +12,7 @@ is defined in <asm/hwprobe.h>:: ...@@ -12,7 +12,7 @@ is defined in <asm/hwprobe.h>::
}; };
long sys_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count, 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); unsigned int flags);
The arguments are split into three groups: an array of key-value pairs, a CPU 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 ...@@ -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 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 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 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 CPU_SET(3) with size ``cpusetsize`` bytes. For value-like keys (eg. vendor,
be only be valid if all CPUs in the given set have the same value. Otherwise -1 arch, impl), the returned value will only be valid if all CPUs in the given set
will be returned. For boolean-like keys, the value returned will be a logical have the same value. Otherwise -1 will be returned. For boolean-like keys, the
AND of the values for the specified CPUs. Usermode can supply NULL for cpus and value returned will be a logical AND of the values for the specified CPUs.
0 for cpu_count as a shortcut for all online CPUs. There are currently no flags, Usermode can supply NULL for ``cpus`` and 0 for ``cpusetsize`` as a shortcut for
this value must be zero for future compatibility. 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. On success 0 is returned, on failure a negative error code is returned.
......
...@@ -246,7 +246,7 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, ...@@ -246,7 +246,7 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
} }
static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, 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 long __user *cpus_user,
unsigned int flags) unsigned int flags)
{ {
...@@ -264,13 +264,13 @@ static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, ...@@ -264,13 +264,13 @@ static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs,
* 0 as a shortcut to all online CPUs. * 0 as a shortcut to all online CPUs.
*/ */
cpumask_clear(&cpus); cpumask_clear(&cpus);
if (!cpu_count && !cpus_user) { if (!cpusetsize && !cpus_user) {
cpumask_copy(&cpus, cpu_online_mask); cpumask_copy(&cpus, cpu_online_mask);
} else { } else {
if (cpu_count > cpumask_size()) if (cpusetsize > cpumask_size())
cpu_count = cpumask_size(); cpusetsize = cpumask_size();
ret = copy_from_user(&cpus, cpus_user, cpu_count); ret = copy_from_user(&cpus, cpus_user, cpusetsize);
if (ret) if (ret)
return -EFAULT; return -EFAULT;
...@@ -347,10 +347,10 @@ arch_initcall_sync(init_hwprobe_vdso_data); ...@@ -347,10 +347,10 @@ arch_initcall_sync(init_hwprobe_vdso_data);
#endif /* CONFIG_MMU */ #endif /* CONFIG_MMU */
SYSCALL_DEFINE5(riscv_hwprobe, struct riscv_hwprobe __user *, pairs, 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) cpus, unsigned int, flags)
{ {
return do_riscv_hwprobe(pairs, pair_count, cpu_count, return do_riscv_hwprobe(pairs, pair_count, cpusetsize,
cpus, flags); cpus, flags);
} }
......
...@@ -8,21 +8,21 @@ ...@@ -8,21 +8,21 @@
#include <vdso/helpers.h> #include <vdso/helpers.h>
extern int riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count, 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); unsigned int flags);
/* Add a prototype to avoid -Wmissing-prototypes warning. */ /* Add a prototype to avoid -Wmissing-prototypes warning. */
int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count, 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); unsigned int flags);
int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count, 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) unsigned int flags)
{ {
const struct vdso_data *vd = __arch_get_vdso_data(); const struct vdso_data *vd = __arch_get_vdso_data();
const struct arch_vdso_data *avd = &vd->arch_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 *p = pairs;
struct riscv_hwprobe *end = pairs + pair_count; struct riscv_hwprobe *end = pairs + pair_count;
...@@ -33,7 +33,7 @@ int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count, ...@@ -33,7 +33,7 @@ int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
* masks. * masks.
*/ */
if ((flags != 0) || (!all_cpus && !avd->homogeneous_cpus)) 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. */ /* This is something we can handle, fill out the pairs. */
while (p < end) { while (p < end) {
......
...@@ -47,7 +47,7 @@ int main(int argc, char **argv) ...@@ -47,7 +47,7 @@ int main(int argc, char **argv)
ksft_test_result(out != 0, "Bad CPU set\n"); ksft_test_result(out != 0, "Bad CPU set\n");
out = riscv_hwprobe(pairs, 8, 1, 0, 0); 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; pairs[0].key = RISCV_HWPROBE_KEY_BASE_BEHAVIOR;
out = riscv_hwprobe(pairs, 1, 1, &cpus, 0); out = riscv_hwprobe(pairs, 1, 1, &cpus, 0);
......
...@@ -10,6 +10,6 @@ ...@@ -10,6 +10,6 @@
* contain the call. * contain the call.
*/ */
long riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count, 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 #endif
// SPDX-License-Identifier: GPL-2.0-only // SPDX-License-Identifier: GPL-2.0-only
#include <sys/prctl.h> #include <sys/prctl.h>
#include <unistd.h> #include <unistd.h>
#include <asm/hwprobe.h>
#include <errno.h> #include <errno.h>
#include <sys/wait.h> #include <sys/wait.h>
#include "../hwprobe/hwprobe.h"
#include "../../kselftest.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" #define NEXT_PROGRAM "./vstate_exec_nolibc"
static int launch_test(int test_inherit) static int launch_test(int test_inherit)
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment