Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
MIME-Version: 1.0
References: <20231203135753.1575-1-jszhang@kernel.org> <20231203135753.1575-2-jszhang@kernel.org>
 <ZW4lUDpl0eZVNjrp@ghost> <20231205021406.GD1168@sol.localdomain>
 <ZW8rbgsTqmuLTvoK@xhacker> <ZW+Oc8sJNcPLWk1Q@ghost>
In-Reply-To: <ZW+Oc8sJNcPLWk1Q@ghost>
From:   Charles Lohr <lohr85@gmail.com>
Date:   Tue, 5 Dec 2023 16:05:27 -0800
Message-ID: <CAGu26P9KiZQhTGO1TrRzGV86GjivmC7_HQaE5p_kNemx+euaEQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] riscv: introduce RISCV_EFFICIENT_UNALIGNED_ACCESS
To:     Charlie Jenkins <charlie@rivosinc.com>
Cc:     Jisheng Zhang <jszhang@kernel.org>,
        Eric Biggers <ebiggers@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Conor Dooley <conor.dooley@microchip.com>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

The automatic detection code has become a bit of a thorn both for
folks like me who use the kernel for some fast-spin up aodr
virtualization (where check_unaligned_access soaks up 1/4 to 1/3 of
the total boot time and unaligned accesses are always fast) as well as
causing issues for the FPGA soft core development where they easily
know ahead of time what the situation is going to be.  It would be
extremely welcome if the access could always be overridden with a
config value that could either force on or force off unaligned access
and avoid execution of the check function permanently.  I don't see a
world where for some of us, we would ever want autodetection on.  In
the RISC-V arena, many times we're dealing with very small systems
where the marginal cost of dead code is rather high.

On Tue, Dec 5, 2023 at 12:57=E2=80=AFPM Charlie Jenkins <charlie@rivosinc.c=
om> wrote:
>
> On Tue, Dec 05, 2023 at 09:53:50PM +0800, Jisheng Zhang wrote:
> > On Mon, Dec 04, 2023 at 06:14:06PM -0800, Eric Biggers wrote:
> > > On Mon, Dec 04, 2023 at 11:15:28AM -0800, Charlie Jenkins wrote:
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > index 7f8aa25457ba..0a76209e9b02 100644
> > > > > --- a/arch/riscv/Kconfig
> > > > > +++ b/arch/riscv/Kconfig
> > > > > @@ -654,6 +654,18 @@ config RISCV_MISALIGNED
> > > > >           load/store for both kernel and userspace. When disable,=
 misaligned
> > > > >           accesses will generate SIGBUS in userspace and panic in=
 kernel.
> > > > >
> > > > > +config RISCV_EFFICIENT_UNALIGNED_ACCESS
> > > >
> > > > There already exists hwprobe for this purpose. If kernel code wants=
 to
> > > > leverage the efficient unaligned accesses of hardware, it can use s=
tatic
> > > > keys. I have a patch that will set this static key if the hardware =
was
> > > > detected to have fast unaligned accesses:
> > > >
> > > > https://lore.kernel.org/linux-riscv/20231117-optimize_checksum-v11-=
2-7d9d954fe361@rivosinc.com/
> > >
> > > Is the plan to make the get_unaligned* and put_unaligned* macros expa=
nd to code
> > > for both cases, and select between them using a static key?  Note tha=
t there are
> > > a very large number of callers of these macros in the kernel.  And wh=
at about
> > > kernel code that checks CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS direct=
ly?
> > >
> > > AFAIK, no other Linux architecture supports kernel images where the u=
naligned
> > > access support is unknown at compile time.  It's not clear to me that=
 such an
> > > approach is feasible.  A static key can easily be provided, but it's =
unclear
> > > what code would use it, given that currently lots of kernel code assu=
mes that
> > > unaligned access support is known at compile time.
> > >
> > > Meanwhile, there are people building kernels they know will only be d=
eployed on
> > > systems where unaligned accesses are supported.  To me, it seems usef=
ul to
> > > provide a kconfig option for them to build a more efficient kernel.
> >
> > Generally, I agree with Eric's above points. Various subsystem such as =
net, mm,
> > lib and so on have different code path for CONFIG_HAVE_EFFICIENT_UNALIG=
NED_ACCESS,
> > while Charlie's patch only touch partial code of arch/riscv, and even i=
f those
> > subsystem maintainers agree with dynamic code patching(I still believe
> > persuading those subsystem maintainers is not easy), that's still a
> > huge task which needs to be done step by step. So before that, we'd
> > better let this series merged and benefit all efficient unaligned acces=
s
> > riscv systems. When the huge task is completed, we can remove the confi=
g
> > option.
> >
> > Thanks
>
> It would be best to enable all of the paths that leverage
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS at runtime (using hwprobe)
> instead of using a compile-time flag to do so. However, as you say, that
> is large task and doesn't need to be done immediately. For now I agree
> it is sufficient to use this new RISCV_EFFICIENT_UNALIGNED_ACCESS
> config.
>
> - Charlie
>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv