lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAK7LNASwKy=wjJT1YNCACLfnn6VY0-7scN=wi4hRoCssh=6cGQ@mail.gmail.com>
Date: Sat, 4 May 2024 23:58:59 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Emil Renner Berthing <emil.renner.berthing@...onical.com>
Cc: Björn Töpel <bjorn@...nel.org>, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	linux-kbuild@...r.kernel.org, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas@...sle.eu>, Nick Terrell <terrelln@...com>
Subject: Re: [PATCH v1 1/3] riscv: make image compression configurable

On Sat, May 4, 2024 at 10:54 PM Emil Renner Berthing
<emil.renner.berthing@...onical.com> wrote:
>
> Masahiro Yamada wrote:
> > On Thu, May 2, 2024 at 10:05 PM Björn Töpel <bjorn@...nel.org> wrote:
> > >
> > > Emil Renner Berthing <emil.renner.berthing@...onical.com> writes:
> > >
> > > > Previously the build process would always set KBUILD_IMAGE to the
> > > > uncompressed Image file (unless XIP_KERNEL or EFI_ZBOOT was enabled) and
> > > > unconditionally compress it into Image.gz. However there are already
> > > > build targets for Image.bz2, Image.lz4, Image.lzma, Image.lzo and
> > > > Image.zstd, so let's make use of those, make the compression method
> > > > configurable and set KBUILD_IMAGE accordingly so that targets like
> > > > 'make install' and 'make bindeb-pkg' will use the chosen image.
> > > >
> > > > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@...onicalcom>
> > > > ---
> > > >  arch/riscv/Kconfig         |  7 +++++++
> > > >  arch/riscv/Makefile        | 43 ++++++++++++++++++++------------------
> > > >  arch/riscv/boot/install.sh |  9 +++++---
> > > >  3 files changed, 36 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index be09c8836d56..6c092d1ea7db 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -138,6 +138,13 @@ config RISCV
> > > >       select HAVE_GCC_PLUGINS
> > > >       select HAVE_GENERIC_VDSO if MMU && 64BIT
> > > >       select HAVE_IRQ_TIME_ACCOUNTING
> > > > +     select HAVE_KERNEL_BZIP2 if !XIP_KERNEL && !EFI_ZBOOT
> > > > +     select HAVE_KERNEL_GZIP if !XIP_KERNEL && !EFI_ZBOOT
> > > > +     select HAVE_KERNEL_LZ4 if !XIP_KERNEL && !EFI_ZBOOT
> > > > +     select HAVE_KERNEL_LZMA if !XIP_KERNEL && !EFI_ZBOOT
> > > > +     select HAVE_KERNEL_LZO if !XIP_KERNEL && !EFI_ZBOOT
> > > > +     select HAVE_KERNEL_UNCOMPRESSED if !XIP_KERNEL && !EFI_ZBOOT
> > > > +     select HAVE_KERNEL_ZSTD if !XIP_KERNEL && !EFI_ZBOOT
> > > >       select HAVE_KPROBES if !XIP_KERNEL
> > > >       select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
> > > >       select HAVE_KRETPROBES if !XIP_KERNEL
> > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > > > index 5b3115a19852..29be676415d6 100644
> > > > --- a/arch/riscv/Makefile
> > > > +++ b/arch/riscv/Makefile
> > > > @@ -129,11 +129,27 @@ endif
> > > >  CHECKFLAGS += -D__riscv -D__riscv_xlen=$(BITS)
> > > >
> > > >  # Default target when executing plain make
> > > > -boot         := arch/riscv/boot
> > > > +boot := arch/riscv/boot
> > > >  ifeq ($(CONFIG_XIP_KERNEL),y)
> > > >  KBUILD_IMAGE := $(boot)/xipImage
> > > > +else ifeq ($(CONFIG_RISCV_M_MODE)$(CONFIG_ARCH_CANAAN),yy)
> > > > +KBUILD_IMAGE := $(boot)/loader.bin
> > > > +else ifeq ($(CONFIG_EFI_ZBOOT),y)
> > > > +KBUILD_IMAGE := $(boot)/vmlinuz.efi
> > > > +else ifeq ($(CONFIG_KERNEL_GZIP),y)
> > > > +KBUILD_IMAGE := $(boot)/Image.gz
> > > > +else ifeq ($(CONFIG_KERNEL_BZIP2),y)
> > > > +KBUILD_IMAGE := $(boot)/Image.bz2
> > > > +else ifeq ($(CONFIG_KERNEL_LZ4),y)
> > > > +KBUILD_IMAGE := $(boot)/Image.lz4
> > > > +else ifeq ($(CONFIG_KERNEL_LZMA),y)
> > > > +KBUILD_IMAGE := $(boot)/Image.lzma
> > > > +else ifeq ($(CONFIG_KERNEL_LZO),y)
> > > > +KBUILD_IMAGE := $(boot)/Image.lzo
> > > > +else ifeq ($(CONFIG_KERNEL_ZSTD),y)
> > > > +KBUILD_IMAGE := $(boot)/Image.zst
> > > >  else
> > > > -KBUILD_IMAGE := $(boot)/Image.gz
> > > > +KBUILD_IMAGE := $(boot)/Image
> > > >  endif
> > >
> > > Really a nit/change if you want, but maybe doing something like
> > > arch/s390/boot/Makefile does is easier to read:
> > >
> > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > > index 024482c68835..70f08e9999b4 100644
> > > --- a/arch/riscv/Makefile
> > > +++ b/arch/riscv/Makefile
> > > @@ -128,6 +128,14 @@ endif
> > >  # arch specific predefines for sparse
> > >  CHECKFLAGS += -D__riscv -D__riscv_xlen=$(BITS)
> > >
> > > +suffix-$(CONFIG_KERNEL_GZIP)  := .gz
> > > +suffix-$(CONFIG_KERNEL_BZIP2) := .bz2
> > > +suffix-$(CONFIG_KERNEL_LZ4)  := .lz4
> > > +suffix-$(CONFIG_KERNEL_LZMA)  := .lzma
> > > +suffix-$(CONFIG_KERNEL_LZO)  := .lzo
> > > +suffix-$(CONFIG_KERNEL_XZ)  := .xz
> > > +suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
> > > +
> > >  # Default target when executing plain make
> > >  boot := arch/riscv/boot
> > >  ifeq ($(CONFIG_XIP_KERNEL),y)
> > > @@ -136,20 +144,8 @@ else ifeq ($(CONFIG_RISCV_M_MODE)$(CONFIG_ARCH_CANAAN),yy)
> > >  KBUILD_IMAGE := $(boot)/loader.bin
> > >  else ifeq ($(CONFIG_EFI_ZBOOT),y)
> > >  KBUILD_IMAGE := $(boot)/vmlinuz.efi
> > > -else ifeq ($(CONFIG_KERNEL_GZIP),y)
> > > -KBUILD_IMAGE := $(boot)/Image.gz
> > > -else ifeq ($(CONFIG_KERNEL_BZIP2),y)
> > > -KBUILD_IMAGE := $(boot)/Image.bz2
> > > -else ifeq ($(CONFIG_KERNEL_LZ4),y)
> > > -KBUILD_IMAGE := $(boot)/Image.lz4
> > > -else ifeq ($(CONFIG_KERNEL_LZMA),y)
> > > -KBUILD_IMAGE := $(boot)/Image.lzma
> > > -else ifeq ($(CONFIG_KERNEL_LZO),y)
> > > -KBUILD_IMAGE := $(boot)/Image.lzo
> > > -else ifeq ($(CONFIG_KERNEL_ZSTD),y)
> > > -KBUILD_IMAGE := $(boot)/Image.zst
> > >  else
> > > -KBUILD_IMAGE := $(boot)/Image
> > > +KBUILD_IMAGE := $(boot)/Image$(suffix-y)
> > >  endif
> >
> >
> >
> >
> > Good idea.
> >
> >
> > If you avoid the 'else ifeq' chain completely,
> > you also could do like this:
> >
> >
> >
> > boot-image-$(CONFIG_KERNEL_GZIP)         := Image.gz
> >    ...
> > boot-image-$(CONFIG_KERNEL_ZSTD)         := Image.zst
> > boot-image-$(CONFIG_KERNEL_UNCOMPRESSED) := Image
> > boot-image-$(CONFIG_RISCV_M_MODE)        := loader.bin
> > boot-image-$(CONFIG_ARCH_CANAAN)         := loader.bin
> > boot-image-$(CONFIG_EFI_ZBOOT)           := vmlinuz.efi
> > boot-image-$(CONFIG_XIP_KERNEL)          := xipImage
> >
> > KBUILD_IMAGE := $(boot)/$(boot-image-y)
>
> Hi Masahiro and Björn.
>
> I like this approach. But I think it doesn't quite do the same when fx.
> CONFIG_RISCV_M_MODE=n but CONFIG_ARCH_CANAAN=y which I think is a valid
> configuration for the new Kendryte K230 support. In this case boot-image-y
> would be overwritten with the loader.bin value even for S-mode kernels.
>
> To keep the previous behaviour we'd need something like
>
>   boot-image-$(CONFIG_RISCV_M_MODE *and* CONFIG_ARCH_CANAAN) := loader.bin
>
> Maybe just
>
>   ifeq ($(CONFIG_RISCV_M_MODE),y)
>   boot-image-$(CONFIG_ARCH_CANAAN) := loader.bin
>   endif
>
> Do you guys have a better solution?


Hi Emil, sorry I misunderstood AND and OR.


ifdef CONFIG_RISCV_M_MODE
boot-image-$(CONFIG_ARCH_CANAAN) := loader.bin
endif

is slightly shorter.



boot-image-$(and $(CONFIG_RISCV_M_MODE),$(CONFIG_ARCH_CANAAN)) := loader.bin

is also possible, but the line is a bit long.


Otherwise, I do not have a better idea.




I am fine with any form.
I leave it to you and Palmer.





-- 
Best Regards
Masahiro Yamada

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ