Re: [PATCH v1 3/6] RISC-V: hwprobe: Introduce which-cpus flag

Andrew Jones
2023-10-19T17:17:38Z

On Wed, Oct 11, 2023 at 03:56:14PM +0200, Andrew Jones wrote:
...
> diff --git a/arch/riscv/kernel/vdso/hwprobe.c b/arch/riscv/kernel/vdso/hwprobe.c
> index 026b7645c5ab..e6c324d64544 100644
> --- a/arch/riscv/kernel/vdso/hwprobe.c
> +++ b/arch/riscv/kernel/vdso/hwprobe.c
> @@ -3,6 +3,7 @@
> * Copyright 2023 Rivos, Inc
> */
>
> +#include <linux/string.h>
> #include <linux/types.h>
> #include <vdso/datapage.h>
> #include <vdso/helpers.h>
> @@ -11,14 +12,9 @@ extern int riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
> 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 cpusetsize, unsigned long *cpus,
> - unsigned int flags);
> -
> -int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
> - size_t cpusetsize, unsigned long *cpus,
> - unsigned int flags)
> +static int riscv_vdso_get_values(struct riscv_hwprobe *pairs, size_t pair_count,
> + 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;
> @@ -50,3 +46,59 @@ int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
>
> return 0;
> }
> +
> +static int riscv_vdso_get_cpus(struct riscv_hwprobe *pairs, size_t pair_count,
> + 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;
> + struct riscv_hwprobe *p = pairs;
> + struct riscv_hwprobe *end = pairs + pair_count;
> + bool clear_all = false;
> +
> + if (!cpusetsize || !cpus)
> + return -EINVAL;
> +
> + if (flags != RISCV_HWPROBE_WHICH_CPUS || !avd->homogeneous_cpus)
> + return riscv_hwprobe(pairs, pair_count, cpusetsize, cpus, flags);
> +
> + while (p < end) {
> + if (riscv_hwprobe_key_is_valid(p->key)) {
> + struct riscv_hwprobe t = {
> + .key = p->key,
> + .value = avd->all_cpu_hwprobe_values[p->key],
> + };
> +
> + if (!riscv_hwprobe_pair_cmp(&t, p))
> + clear_all = true;
> + } else {
> + clear_all = true;
> + p->key = -1;
> + p->value = 0;
> + }
> + p++;
> + }
> +
> + if (clear_all)
> + memset(cpus, 0, cpusetsize);

This memset will go away in v2. It wasn't very smart of me to put it there
in the first place as there's no memset available to vdso code, which
means it would segfault when attempting to use it. My initial testing
failed to find this, because I had forgotten that avd->homogeneous_cpus
would be false on a default QEMU machine since it doesn't set mvendorid to
anything. I actually found this problem when trying to add a memcmp which
wasn't skipped with !avd->homogeneous_cpus, and that promptly segfaulted.
(I won't be adding that memcmp either :-)

Thanks,
drew

> +
> + return 0;
> +}
> +
> +/* Add a prototype to avoid -Wmissing-prototypes warning. */
> +int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
> + size_t cpusetsize, unsigned long *cpus,
> + unsigned int flags);
> +
> +int __vdso_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
> + size_t cpusetsize, unsigned long *cpus,
> + unsigned int flags)
> +{
> + if (flags & RISCV_HWPROBE_WHICH_CPUS)
> + return riscv_vdso_get_cpus(pairs, pair_count, cpusetsize,
> + cpus, flags);
> +
> + return riscv_vdso_get_values(pairs, pair_count, cpusetsize,
> + cpus, flags);
> +}
> --
> 2.41.0