Date: Tue, 12 Dec 2023 10:35:25 -0700 From: Warner Losh <imp@bsdimp.com> To: Dimitry Andric <dim@freebsd.org> Cc: John Baldwin <jhb@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: fb17dfa0c83c - main - libicp: unbreak for armv6 after recent OpenZFS import Message-ID: <CANCZdfoNj8BZYXsN_fM3E06fJTUPT%2B3D9zVBe=q-TK_mLXOeRw@mail.gmail.com> In-Reply-To: <C9583E92-D6EB-4B7D-B26E-3B86C8E84EB0@FreeBSD.org> References: <202312082310.3B8NA5cI026712@gitrepo.freebsd.org> <098c85ab-4210-455c-a3b9-773bf0ce87b7@FreeBSD.org> <C9583E92-D6EB-4B7D-B26E-3B86C8E84EB0@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000007cb2aa060c537a1c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Dec 12, 2023 at 10:15=E2=80=AFAM Dimitry Andric <dim@freebsd.org> w= rote: > 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=3Dfb17dfa0c83cc213400fe7e1ed7a392= 53a4fcefa > >> 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 =3D \ > >> asm-x86_64/blake3/blake3_sse41.S > >> CFLAGS+=3D -D__amd64 -D_SYS_STACK_H -UHAVE_AES > >> -.elif ${MACHINE_ARCH} =3D=3D "armv7" > >> +.elif ${MACHINE_ARCH} =3D=3D "armv6" || ${MACHINE_ARCH} =3D=3D "armv7= " > > > > Since this applies to all 32-bit arm flavors, should this be using > > ${MACHINE_CPUARCH} =3D=3D "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 al= l > > 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 --0000000000007cb2aa060c537a1c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Tue, Dec 12, 2023 at 10:15=E2=80= =AFAM Dimitry Andric <<a href=3D"mailto:dim@freebsd.org">dim@freebsd.org= </a>> wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:= 0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">= On 12 Dec 2023, at 17:01, John Baldwin <<a href=3D"mailto:jhb@freebsd.or= g" target=3D"_blank">jhb@freebsd.org</a>> wrote:<br> > <br> > On 12/8/23 3:10 PM, Dimitry Andric wrote:<br> >> The branch main has been updated by dim:<br> >> URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3Dfb17dfa0= c83cc213400fe7e1ed7a39253a4fcefa" rel=3D"noreferrer" target=3D"_blank">http= s://cgit.FreeBSD.org/src/commit/?id=3Dfb17dfa0c83cc213400fe7e1ed7a39253a4fc= efa</a><br> >> commit fb17dfa0c83cc213400fe7e1ed7a39253a4fcefa<br> >> Author:=C2=A0 =C2=A0 =C2=A0Dimitry Andric <dim@FreeBSD.org><= br> >> AuthorDate: 2023-12-08 23:09:36 +0000<br> >> Commit:=C2=A0 =C2=A0 =C2=A0Dimitry Andric <dim@FreeBSD.org><= br> >> CommitDate: 2023-12-08 23:09:50 +0000<br> >>=C2=A0 =C2=A0 =C2=A0libicp: unbreak for armv6 after recent OpenZFS = import<br> >>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 The following upstream commit:<b= r> >>=C2=A0 =C2=A0 =C2=A0727497ccdfcc module/icp/asm-arm/sha2: enable no= n-SIMD asm kernels on armv5/6<br> >>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 does indeed enable sha2 asm for = earlier arm CPUs, but since libicp's<br> >>=C2=A0 =C2=A0 =C2=A0Makefile was not updated, this leads to:<br> >>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ld: error: undefined refe= rence due to --no-allow-shlib-undefined: zfs_sha256_block_armv7<br> >>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Fix it by compiling sha256-armv7= .S and sha512-armv7.S for armv6 too.<br> >>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Fixes:=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 3494f7c019fc<br> >> ---<br> >>=C2=A0 cddl/lib/libicp/Makefile | 2 +-<br> >>=C2=A0 1 file changed, 1 insertion(+), 1 deletion(-)<br> >> diff --git a/cddl/lib/libicp/Makefile b/cddl/lib/libicp/Makefile<b= r> >> index 2d9bb3c67cb4..085818f2371a 100644<br> >> --- a/cddl/lib/libicp/Makefile<br> >> +++ b/cddl/lib/libicp/Makefile<br> >> @@ -21,7 +21,7 @@ ASM_SOURCES_AS =3D \<br> >>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 asm-x86_64/blake3/blake3_sse41.S= <br> >>=C2=A0 =C2=A0 CFLAGS+=3D -D__amd64 -D_SYS_STACK_H -UHAVE_AES<br> >> -.elif ${MACHINE_ARCH} =3D=3D "armv7"<br> >> +.elif ${MACHINE_ARCH} =3D=3D "armv6" || ${MACHINE_ARCH}= =3D=3D "armv7"<br> > <br> > Since this applies to all 32-bit arm flavors, should this be using<br> > ${MACHINE_CPUARCH} =3D=3D "arm" instead?<br> <br> You may be right; if I read<br> <a href=3D"https://github.com/openzfs/zfs/commit/727497ccdfcc" rel=3D"noref= errer" target=3D"_blank">https://github.com/openzfs/zfs/commit/727497ccdfcc= </a> correctly:<br> <br> > module/icp/asm-arm/sha2: enable non-SIMD asm kernels on armv5/6<br> > My merged pull request #15557 fixes compilation of sha2 kernels on arm= <br> > v5/6. However, the compiler guards only allows sha256/512_armv7_impl t= o<br> > be used when __ARM_ARCH > 6. This patch enables these ASM kernels o= n all<br> > arm architectures. Some compiler guards are adjusted accordingly to<br= > > avoid the unnecessary compilation of SIMD (e.g., neon, armv8ce) kernel= s<br> > on old architectures.<br> ><br> > Reviewed-by: Brian Behlendorf <<a href=3D"mailto:behlendorf1@llnl.g= ov" target=3D"_blank">behlendorf1@llnl.gov</a>><br> > Signed-off-by: Shengqi Chen <<a href=3D"mailto:harry-chen@outlook.c= om" target=3D"_blank">harry-chen@outlook.com</a>><br> > Closes #15623<br> <br> It's talking about "all arm architectures", but I'm not s= ure if this<br> means all arm architectures supported by OpenZFS, or in general. I would<br= > assume the latter.<br></blockquote><div><br></div><div>Yea, John's comm= ents are correct. The OpenZFS code expects this interface</div><div>to be t= here and guards appropriately against the use of features only on armv7.</d= iv><div><br></div><div>Warner=C2=A0</div></div></div> --0000000000007cb2aa060c537a1c--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfoNj8BZYXsN_fM3E06fJTUPT%2B3D9zVBe=q-TK_mLXOeRw>