From: Eric Biggers <ebiggers@kernel.org>
To: Tal Zussman <tz2294@columbia.edu>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
Ard Biesheuvel <ardb@kernel.org>,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] lib/crypto: ensure generated *.S files are removed on make clean
Date: Wed, 13 Aug 2025 19:40:12 -0700 [thread overview]
Message-ID: <20250814024012.GA1296@sol> (raw)
In-Reply-To: <CAKha_srSRA9HftM+zLeRVrONKmPdtm-wTXq3n2NC60Gynuvwyw@mail.gmail.com>
On Thu, Aug 14, 2025 at 03:47:05AM +0300, Tal Zussman wrote:
> On Thu, Aug 14, 2025 at 3:39 AM Tal Zussman <tz2294@columbia.edu> wrote:
> >
> > make clean does not check the kernel config when removing files. As
> > such, additions to clean-files under CONFIG_ARM or CONFIG_ARM64 are not
> > evaluated. For example, when building on arm64, this means that
> > lib/crypto/arm64/sha{256,512}-core.S are left over after make clean.
> >
> > Set clean-files unconditionally to ensure that make clean removes these
> > files.
> >
> > Fixes: e96cb9507f2d ("lib/crypto: sha256: Consolidate into single module")
> > Fixes: 24c91b62ac50 ("lib/crypto: arm/sha512: Migrate optimized SHA-512 code to library")
> > Fixes: 60e3f1e9b7a5 ("lib/crypto: arm64/sha512: Migrate optimized SHA-512 code to library")
> > Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> > ---
> > An alternative approach is to rename the generated files to *.s and
> > remove the clean-files lines, as make clean removes *.s files
> > automatically. However, this would require explicitly defining the
> > corresponding *.o rules.
> > ---
> > lib/crypto/Makefile | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
> > index e4151be2ebd4..44f6a1fdc808 100644
> > --- a/lib/crypto/Makefile
> > +++ b/lib/crypto/Makefile
> > @@ -100,7 +100,6 @@ ifeq ($(CONFIG_ARM),y)
> > libsha256-y += arm/sha256-ce.o arm/sha256-core.o
> > $(obj)/arm/sha256-core.S: $(src)/arm/sha256-armv4.pl
> > $(call cmd,perlasm)
> > -clean-files += arm/sha256-core.S
> > AFLAGS_arm/sha256-core.o += $(aflags-thumb2-y)
> > endif
> >
> > @@ -108,7 +107,6 @@ ifeq ($(CONFIG_ARM64),y)
> > libsha256-y += arm64/sha256-core.o
> > $(obj)/arm64/sha256-core.S: $(src)/arm64/sha2-armv8.pl
> > $(call cmd,perlasm_with_args)
> > -clean-files += arm64/sha256-core.S
> > libsha256-$(CONFIG_KERNEL_MODE_NEON) += arm64/sha256-ce.o
> > endif
> >
> > @@ -132,7 +130,6 @@ ifeq ($(CONFIG_ARM),y)
> > libsha512-y += arm/sha512-core.o
> > $(obj)/arm/sha512-core.S: $(src)/arm/sha512-armv4.pl
> > $(call cmd,perlasm)
> > -clean-files += arm/sha512-core.S
> > AFLAGS_arm/sha512-core.o += $(aflags-thumb2-y)
> > endif
> >
> > @@ -140,7 +137,6 @@ ifeq ($(CONFIG_ARM64),y)
> > libsha512-y += arm64/sha512-core.o
> > $(obj)/arm64/sha512-core.S: $(src)/arm64/sha2-armv8.pl
> > $(call cmd,perlasm_with_args)
> > -clean-files += arm64/sha512-core.S
> > libsha512-$(CONFIG_KERNEL_MODE_NEON) += arm64/sha512-ce-core.o
> > endif
> >
> > @@ -167,3 +163,7 @@ obj-$(CONFIG_PPC) += powerpc/
> > obj-$(CONFIG_RISCV) += riscv/
> > obj-$(CONFIG_S390) += s390/
> > obj-$(CONFIG_X86) += x86/
> > +
> > +# clean-files must be defined unconditionally
> > +clean-files += arm/sha256-core.S arm/sha256-core.S
> > +clean-files += arm64/sha512-core.S arm64/sha512-core.S
>
> Sorry this is broken, needs the following fix on top.
> I'll fix in v2.
>
> diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
> index 44f6a1fdc808..539d5d59a50e 100644
> --- a/lib/crypto/Makefile
> +++ b/lib/crypto/Makefile
> @@ -165,5 +165,5 @@ obj-$(CONFIG_S390) += s390/
> obj-$(CONFIG_X86) += x86/
>
> # clean-files must be defined unconditionally
> -clean-files += arm/sha256-core.S arm/sha256-core.S
> -clean-files += arm64/sha512-core.S arm64/sha512-core.S
> +clean-files += arm/sha256-core.S arm/sha512-core.S
> +clean-files += arm64/sha256-core.S arm64/sha512-core.S
Hmm, interesting. So 'make clean' works in kind of a unique way. It
looks at $(ARCH), but not .config. And though no CONFIG_* options are
set for it, it still processes all the subdirectories like
'obj-$(CONFIG_FOO) += subdir/'. But it doesn't process the contents of
'if' statements conditional on $(CONFIG_FOO). I guess it couldn't
really work in any other way, but it's a bit unique.
Yes, just defining the clean-files unconditionally looks like the best
option here. Please go ahead and send out v2 with the corrected
filenames. Thanks!
- Eric
prev parent reply other threads:[~2025-08-14 2:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-14 0:38 [PATCH] lib/crypto: ensure generated *.S files are removed on make clean Tal Zussman
2025-08-14 0:47 ` Tal Zussman
2025-08-14 2:40 ` Eric Biggers [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250814024012.GA1296@sol \
--to=ebiggers@kernel.org \
--cc=Jason@zx2c4.com \
--cc=ardb@kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tz2294@columbia.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.