[PATCH 1/1] RISC-V: Add parameter to unaligned access speed

[PATCH 1/1] RISC-V: Add parameter to unaligned access speed

[Jesse Taube]

Add a kernel parameter to the unaligned access speed. This allows
skiping of the speed tests for unaligned accesses, which often is very
slow.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/kernel/unaligned_access_speed.c | 81 ++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
index 1548eb10ae4f..02f7a92a5fa0 100644
--- a/arch/riscv/kernel/unaligned_access_speed.c
+++ b/arch/riscv/kernel/unaligned_access_speed.c
@@ -400,13 +400,94 @@ static int vec_check_unaligned_access_speed_all_cpus(void *unused __always_unuse
 }
 #endif
 
+static DEFINE_PER_CPU(long, unaligned_scalar_speed_param) = RISCV_HWPROBE_MISALIGNED_UNKNOWN;
+
+static int __init set_unaligned_scalar_speed_param(char *str)
+{
+	cpumask_var_t mask;
+	int ret, cpu;
+	long speed = RISCV_HWPROBE_MISALIGNED_UNKNOWN;
+
+	if (!strncmp(str, "fast,", 5)) {
+		str += 5;
+		speed = RISCV_HWPROBE_MISALIGNED_FAST;
+	}
+
+	if (!strncmp(str, "slow,", 5)) {
+		str += 5;
+		speed = RISCV_HWPROBE_MISALIGNED_SLOW;
+	}
+	if (speed == RISCV_HWPROBE_MISALIGNED_UNKNOWN) {
+		pr_warn("Invalid unaligned access speed parameter\n");
+		return 1;
+	}
+
+	if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
+		return -ENOMEM;
+
+	ret = cpulist_parse(str, mask);
+
+	for_each_cpu(cpu, mask)
+		if (per_cpu(unaligned_scalar_speed_param, cpu) == RISCV_HWPROBE_MISALIGNED_UNKNOWN)
+			per_cpu(unaligned_scalar_speed_param, cpu) = speed;
+
+	free_cpumask_var(mask);
+	return ret == 0;
+}
+__setup("unaligned_scalar_speed=", set_unaligned_scalar_speed_param);
+
+static DEFINE_PER_CPU(long, unaligned_vector_speed_param) = RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN;
+
+static int __init set_unaligned_vector_speed_param(char *str)
+{
+	cpumask_var_t mask;
+	int ret, cpu;
+	long speed = RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN;
+
+	if (!strncmp(str, "fast,", 5)) {
+		str += 5;
+		speed = RISCV_HWPROBE_VECTOR_MISALIGNED_FAST;
+	}
+
+	if (!strncmp(str, "slow,", 5)) {
+		str += 5;
+		speed = RISCV_HWPROBE_VECTOR_MISALIGNED_SLOW;
+	}
+	if (speed == RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN) {
+		pr_warn("Invalid unaligned access speed parameter\n");
+		return 1;
+	}
+
+	if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
+		return -ENOMEM;
+
+	ret = cpulist_parse(str, mask);
+
+	for_each_cpu(cpu, mask)
+		if (per_cpu(unaligned_vector_speed_param, cpu) == RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN)
+			per_cpu(unaligned_vector_speed_param, cpu) = speed;
+
+	free_cpumask_var(mask);
+	return ret == 0;
+}
+__setup("unaligned_vector_speed=", set_unaligned_vector_speed_param);
+
 static int check_unaligned_access_all_cpus(void)
 {
+	int cpu;
 	bool all_cpus_emulated, all_cpus_vec_unsupported;
 
 	all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
 	all_cpus_vec_unsupported = check_vector_unaligned_access_emulated_all_cpus();
 
+	for_each_online_cpu(cpu) {
+		if (per_cpu(misaligned_access_speed, cpu) == RISCV_HWPROBE_MISALIGNED_UNKNOWN)
+			per_cpu(misaligned_access_speed, cpu) = per_cpu(unaligned_scalar_speed_param, cpu);
+
+		if (per_cpu(vector_misaligned_access, cpu) == RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN)
+			per_cpu(vector_misaligned_access, cpu) = per_cpu(unaligned_vector_speed_param, cpu);
+	}
+
 	pr_info("\e[31m%s vector unaligned access\e[0m\n",
 		all_cpus_vec_unsupported ? "All CPUs do not support" : "At least one cpu supports");
 	if (!all_cpus_vec_unsupported &&
-- 
2.45.2

[PATCH 0/1] RISC-V: kernel parameter for unaligned access speed

[Jesse Taube]

Add a kernel parameter to the unaligned access speed. This allows
skiping of the speed tests for unaligned accesses, which often is very
slow.

This patch requires (RISC-V: Detect and report speed of unaligned vector
accesses) to be applied first.

https://lore.kernel.org/all/20240726163719.1667923-1-jesse@rivosinc.com/

Jesse Taube (1):
  RISC-V: Add parameter to unaligned access speed

 arch/riscv/kernel/unaligned_access_speed.c | 81 ++++++++++++++++++++++
 1 file changed, 81 insertions(+)

-- 
2.45.2

Re: [PATCH 1/1] RISC-V: Add parameter to unaligned access speed

[Evan Green]

On Mon, Aug 5, 2024 at 10:38 AM Jesse Taube <jesse@rivosinc.com> wrote:
>
> Add a kernel parameter to the unaligned access speed. This allows
> skiping of the speed tests for unaligned accesses, which often is very
> slow.
>
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>

How come this is a command line parameter rather than a Kconfig
option? I could be wrong, so I'll lay out my rationale and people can
pick it apart if I've got a bad assumption.

I think of commandline parameters as (mostly) something end users
twiddle with, versus kconfig options as something system builders set
up. I'd largely expect end users not to notice two ticks at boot time.
I'd expect its system builders and fleet managers, who know their
hardware and build their kernels optimized for it, are the ones who
would want to shave off this time and go straight to the known answer.
Anecdotally, at ChromeOS we had a strong preference for Kconfig
options, as they were easier to compose and maintain than a loose pile
of commandline arguments.

The commit text doesn't go into the rationale, intended audience, or
expected usage, so maybe my guesses miss the mark on what you're
thinking.
-Evan

Re: [PATCH 1/1] RISC-V: Add parameter to unaligned access speed

[Charlie Jenkins]

On Mon, Aug 05, 2024 at 11:38:23AM -0700, Evan Green wrote:
> On Mon, Aug 5, 2024 at 10:38 AM Jesse Taube <jesse@rivosinc.com> wrote:
> >
> > Add a kernel parameter to the unaligned access speed. This allows
> > skiping of the speed tests for unaligned accesses, which often is very
> > slow.
> >
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> 
> How come this is a command line parameter rather than a Kconfig
> option? I could be wrong, so I'll lay out my rationale and people can
> pick it apart if I've got a bad assumption.
> 
> I think of commandline parameters as (mostly) something end users
> twiddle with, versus kconfig options as something system builders set
> up. I'd largely expect end users not to notice two ticks at boot time.
> I'd expect its system builders and fleet managers, who know their
> hardware and build their kernels optimized for it, are the ones who
> would want to shave off this time and go straight to the known answer.
> Anecdotally, at ChromeOS we had a strong preference for Kconfig
> options, as they were easier to compose and maintain than a loose pile
> of commandline arguments.
> 
> The commit text doesn't go into the rationale, intended audience, or
> expected usage, so maybe my guesses miss the mark on what you're
> thinking.
> -Evan

There was a brief discussion about this on Jesse's series about vector
unaligned support [1]. The original idea was to use Zicclsm to allow
people to set the unaligned access speed on pre-compiled distro kernels.
However Zicclsm isn't useful so the alternative route was to use a
kernel arg. There is already support for a Kconfig, the kernel arg is
just another option for users.

Link:
https://lore.kernel.org/lkml/af3152b6-adf7-40fa-b2a1-87e66eec45b0@rivosinc.com/
[1]

- Charlie

Re: [PATCH 1/1] RISC-V: Add parameter to unaligned access speed

[Evan Green]

On Mon, Aug 5, 2024 at 11:48 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> On Mon, Aug 05, 2024 at 11:38:23AM -0700, Evan Green wrote:
> > On Mon, Aug 5, 2024 at 10:38 AM Jesse Taube <jesse@rivosinc.com> wrote:
> > >
> > > Add a kernel parameter to the unaligned access speed. This allows
> > > skiping of the speed tests for unaligned accesses, which often is very
> > > slow.
> > >
> > > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> >
> > How come this is a command line parameter rather than a Kconfig
> > option? I could be wrong, so I'll lay out my rationale and people can
> > pick it apart if I've got a bad assumption.
> >
> > I think of commandline parameters as (mostly) something end users
> > twiddle with, versus kconfig options as something system builders set
> > up. I'd largely expect end users not to notice two ticks at boot time.
> > I'd expect its system builders and fleet managers, who know their
> > hardware and build their kernels optimized for it, are the ones who
> > would want to shave off this time and go straight to the known answer.
> > Anecdotally, at ChromeOS we had a strong preference for Kconfig
> > options, as they were easier to compose and maintain than a loose pile
> > of commandline arguments.
> >
> > The commit text doesn't go into the rationale, intended audience, or
> > expected usage, so maybe my guesses miss the mark on what you're
> > thinking.
> > -Evan
>
> There was a brief discussion about this on Jesse's series about vector
> unaligned support [1]. The original idea was to use Zicclsm to allow
> people to set the unaligned access speed on pre-compiled distro kernels.
> However Zicclsm isn't useful so the alternative route was to use a
> kernel arg. There is already support for a Kconfig, the kernel arg is
> just another option for users.
>
> Link:
> https://lore.kernel.org/lkml/af3152b6-adf7-40fa-b2a1-87e66eec45b0@rivosinc.com/
> [1]

Ah got it, thanks for the explanation Charlie! If there are consumers
for this then the concept seems fine with me.
-Evan