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

The RISC-V Zbc extension adds carry-less multiply instructions we can
use to implement more efficient CRC hashing algorithms.

Signed-off-by: Daniel Gregory <daniel.gregory@bytedance.com>
---
 config/riscv/meson.build | 7 +++++++
 1 file changed, 7 insertions(+)
  

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.

Instead, detect at compile or runtime
  

> 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.
> 
> Instead, detect at compile or runtime

Build time detection is not possible for cross builds.
  

On Wed, 19 Jun 2024 09:08:14 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > 
> > Please do not add more config options via compile flags.
> > It makes it impossible for distros to ship one version.
> > 
> > Instead, detect at compile or runtime  
> 
> Build time detection is not possible for cross builds.

I was thinking some mechanism like the ARM configs.
  

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.
> > 
> > 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/

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".
  

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.
  

On Mon, 7 Oct 2024 10:14:22 +0200
Stanisław Kardach <stanislaw.kardach@gmail.com> wrote:

> > > >
> > > > 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.

X86 already has the cpu feature flag infrastructure, why not use similar
mechanism on RiscV?
  

On Mon, Oct 7, 2024 at 5:20 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 7 Oct 2024 10:14:22 +0200
> Stanisław Kardach <stanislaw.kardach@gmail.com> wrote:
>
> > > > >
> > > > > 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.
>
> X86 already has the cpu feature flag infrastructure, why not use similar
> mechanism on RiscV?
We can and some further patches I've seen on the list implemented
that. However if that has to be applied in basic intrinsics like
rte_prefetch() which means we'd have to pay a conditional or an
indirect call. It'd be best to test it on a real dataplane platform to
which I don't have access.
  

On Tue, 8 Oct 2024 07:52:42 +0200
Stanisław Kardach <stanislaw.kardach@gmail.com> wrote:

> On Mon, Oct 7, 2024 at 5:20 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Mon, 7 Oct 2024 10:14:22 +0200
> > Stanisław Kardach <stanislaw.kardach@gmail.com> wrote:
> >  
> > > > > >
> > > > > > 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.  
> >
> > X86 already has the cpu feature flag infrastructure, why not use similar
> > mechanism on RiscV?  
> We can and some further patches I've seen on the list implemented
> that. However if that has to be applied in basic intrinsics like
> rte_prefetch() which means we'd have to pay a conditional or an
> indirect call. It'd be best to test it on a real dataplane platform to
> which I don't have access.

That makes sense, is there some config file per cpu type like Arm?