Re: git: fb17dfa0c83c - main - libicp: unbreak for armv6 after recent OpenZFS import

On Tue, Dec 12, 2023 at 10:15 AM Dimitry Andric <dim@freebsd.org> wrote:

> On 12 Dec 2023, at 17:01, John Baldwin <jhb@freebsd.org> wrote:
> >
> > On 12/8/23 3:10 PM, Dimitry Andric wrote:
> >> The branch main has been updated by dim:
> >> URL:
> https://cgit.FreeBSD.org/src/commit/?id=fb17dfa0c83cc213400fe7e1ed7a39253a4fcefa
> >> commit fb17dfa0c83cc213400fe7e1ed7a39253a4fcefa
> >> Author:     Dimitry Andric <dim@FreeBSD.org>
> >> AuthorDate: 2023-12-08 23:09:36 +0000
> >> Commit:     Dimitry Andric <dim@FreeBSD.org>
> >> CommitDate: 2023-12-08 23:09:50 +0000
> >>     libicp: unbreak for armv6 after recent OpenZFS import
> >>          The following upstream commit:
> >>     727497ccdfcc module/icp/asm-arm/sha2: enable non-SIMD asm kernels
> on armv5/6
> >>          does indeed enable sha2 asm for earlier arm CPUs, but since
> libicp's
> >>     Makefile was not updated, this leads to:
> >>            ld: error: undefined reference due to
> --no-allow-shlib-undefined: zfs_sha256_block_armv7
> >>          Fix it by compiling sha256-armv7.S and sha512-armv7.S for
> armv6 too.
> >>          Fixes:          3494f7c019fc
> >> ---
> >>  cddl/lib/libicp/Makefile | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> diff --git a/cddl/lib/libicp/Makefile b/cddl/lib/libicp/Makefile
> >> index 2d9bb3c67cb4..085818f2371a 100644
> >> --- a/cddl/lib/libicp/Makefile
> >> +++ b/cddl/lib/libicp/Makefile
> >> @@ -21,7 +21,7 @@ ASM_SOURCES_AS = \
> >>          asm-x86_64/blake3/blake3_sse41.S
> >>    CFLAGS+= -D__amd64 -D_SYS_STACK_H -UHAVE_AES
> >> -.elif ${MACHINE_ARCH} == "armv7"
> >> +.elif ${MACHINE_ARCH} == "armv6" || ${MACHINE_ARCH} == "armv7"
> >
> > Since this applies to all 32-bit arm flavors, should this be using
> > ${MACHINE_CPUARCH} == "arm" instead?
>
> You may be right; if I read
> https://github.com/openzfs/zfs/commit/727497ccdfcc correctly:
>
> > module/icp/asm-arm/sha2: enable non-SIMD asm kernels on armv5/6
> > My merged pull request #15557 fixes compilation of sha2 kernels on arm
> > v5/6. However, the compiler guards only allows sha256/512_armv7_impl to
> > be used when __ARM_ARCH > 6. This patch enables these ASM kernels on all
> > arm architectures. Some compiler guards are adjusted accordingly to
> > avoid the unnecessary compilation of SIMD (e.g., neon, armv8ce) kernels
> > on old architectures.
> >
> > Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
> > Signed-off-by: Shengqi Chen <harry-chen@outlook.com>
> > Closes #15623
>
> It's talking about "all arm architectures", but I'm not sure if this
> means all arm architectures supported by OpenZFS, or in general. I would
> assume the latter.
>

Yea, John's comments are correct. The OpenZFS code expects this interface
to be there and guards appropriately against the use of features only on
armv7.

Warner