Re: [PATCH v1 4/5] RISC-V: hwprobe: Support probing of misaligned accesss performance

["Heiko Stübner" <heiko@sntech.de>]

Am Dienstag, 29. November 2022, 22:18:18 CET schrieb Palmer Dabbelt:
> On Tue, 29 Nov 2022 13:09:49 PST (-0800), heiko@sntech.de wrote:
> > Am Donnerstag, 13. Oktober 2022, 18:35:50 CET schrieb Palmer Dabbelt:
> >> This allows userspace to select various routines to use based on the
> >> performance of misaligned access on the target hardware.
> >>
> >> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> >
> > [...]
> >
> >> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> >> index cbda062de9bd..54bdcf9a5049 100644
> >> --- a/arch/riscv/include/asm/cpufeature.h
> >> +++ b/arch/riscv/include/asm/cpufeature.h
> >> @@ -18,4 +18,6 @@ struct riscv_cpuinfo {
> >>
> >>  DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> >>
> >> +DECLARE_PER_CPU(long, misaligned_access_speed);
> >
> > just my 2ct ... wouldn't it make sense to have struct riscv_cpuinfo
> > as the central instance for all cpu-related stuff, so
> > misaligned_access_speed could also be part of it?
> 
> I remember going through this one a few times and ending up here despite 
> some cleaner-looking ways of doing it.  That way does look cleaner, 
> though, so I'll give it a shot and we'll see what happens...
> 
> >> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> >> index 553d755483ed..1599e40cd170 100644
> >> --- a/arch/riscv/kernel/cpufeature.c
> >> +++ b/arch/riscv/kernel/cpufeature.c
> >> @@ -222,6 +226,22 @@ void __init riscv_fill_hwcap(void)
> >>  			bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> >>  		else
> >>  			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> >> +
> >> +		/*
> >> +		 * Check for the performance of misaligned accesses.
> >> +		 */
> >> +		cpu = hartid_to_cpuid_map(hartid);
> >> +		if (cpu < 0)
> >> +			continue;
> >> +
> >> +		if (of_property_read_string(node, "riscv,misaligned-access-performance", &misaligned)) {
> >
> > I think this wants a "!" in front :-) .
> >
> > of_property_read_string() returns 0 on success, so running this
> > results in a nullptr right now.
> 
> Thanks.  I'd not gotten around to actually running this so I bet it's 
> broken in a bunch of ways.  Did you try it out?  I was really hoping to 
> find some time to get at least the simple stuff in for this cycle, but 
> too many things keep coming up.

I ran the code today, so bumped into the "!" issue.
I'm not yet "using" it, but that will come in the next days.

And no worries about that time-thingy, your series already provides
a nice pointer for the general direction to with things, so that is helpful
in its own right.

I will simply report any brokeness I find when it happens ;-) .


Heiko




_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv