Re: [PATCH 1/5] config/riscv: add flag for using Zbc extension

["Stanisław Kardach" <stanislaw.kardach@gmail.com>]

On Wed, Jun 19, 2024 at 6:41 PM Daniel Gregory
<daniel.gregory@bytedance.com> wrote:
>
> On Wed, Jun 19, 2024 at 09:08:14AM +0200, Morten Brørup wrote:
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > 1/5] config/riscv: add flag for using Zbc extension
> > >
> > > On Tue, 18 Jun 2024 18:41:29 +0100
> > > Daniel Gregory <daniel.gregory@bytedance.com> wrote:
> > >
> > > > diff --git a/config/riscv/meson.build b/config/riscv/meson.build
> > > > index 07d7d9da23..4bda4089bd 100644
> > > > --- a/config/riscv/meson.build
> > > > +++ b/config/riscv/meson.build
> > > > @@ -26,6 +26,13 @@ flags_common = [
> > > >      # read from /proc/device-tree/cpus/timebase-frequency. This property is
> > > >      # guaranteed on Linux, as riscv time_init() requires it.
> > > >      ['RTE_RISCV_TIME_FREQ', 0],
> > > > +
> > > > +    # Use RISC-V Carry-less multiplication extension (Zbc) for hardware
> > > > +    # implementations of CRC-32C (lib/hash/rte_crc_riscv64.h), CRC-32 and
> > > CRC-16
> > > > +    # (lib/net/net_crc_zbc.c). Requires intrinsics available in GCC 14.1.0+
> > > and
> > > > +    # Clang 18.1.0+
> > > > +    # Make sure to add '_zbc' to your target's -march below
> > > > +    ['RTE_RISCV_ZBC', false],
> > > >  ]
> > >
> > > Please do not add more config options via compile flags.
> > > It makes it impossible for distros to ship one version.
That is a problem with RISC-V in general. Since all features are
"extensions" and there is no limit (up to a point) on the permutation
of those, we cannot statically build the code for all extensions.
Fortunately instructions tend to resolve to nops if an instruction is
not present but that still increases the code size for no benefit on
platforms without a given extension.
> > >
> > > Instead, detect at compile or runtime
> >
> > Build time detection is not possible for cross builds.
> >
>
> How about build time detection based on the target's configured
> instruction set (either specified by cross-file or passed in through
> -Dinstruction_set)? We could have a map from extensions present in the
> ISA string to compile flags that should be enabled.
>
> I suggested this whilst discussing a previous patch adding support for
> the Zawrs extension, but haven't heard back from Stanisław yet:
> https://lore.kernel.org/dpdk-dev/20240520094854.GA3672529@ste-uk-lab-gw/
I think we already have 1 case of a cross compile config:
https://git.dpdk.org/dpdk/tree/config/riscv/riscv64_sifive_u740_linux_gcc.
This could serve as a stop gap before runtime detection is sorted out.
I would prefer the static option to rather list all the hardware
platforms explicitly. This way we will support existing platforms, not
some RISC-V vendor plans. Maybe at some point the extension mess gets
fixed in the arch.
>
> As for runtime detection, newer kernel versions have a hardware probing
> interface for detecting the presence of extensions, support could be
> added to rte_cpuflags.c?
> https://docs.kernel.org/arch/riscv/hwprobe.html
>
> In combination, distros on newer kernels could ship a version that has
> these optimisations baked in that falls back to a generic implementation
> when the extension is detected to not be present, and systems without
> the latest GCC/Clang can still compile by specifying a target ISA that
> doesn't include "_zbc".
hwprobe sounds like a good idea, although the key name for extensions
(RISCV_HWPROBE_KEY_IMA_EXT_0) suggests that there will be more (it's
64bit and we already have 46 bits taken). That I wonder what options
we have to keep the performance characteristics of the code. We either
need to live-patch the code (which is problematic for userspace
processes) or resort to some .so or a driver-like model. Neither
option sounds very appealing.