linux-kernel - Re: [PATCH v2 3/3] arm64: boot: Support Flat Image Tree

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK7LNAQCu9tWcjEtak3sBVo5mQYu1abu-v-GQ0b5h7PhzuHKww@mail.gmail.com>
Date:   Tue, 7 Nov 2023 15:05:01 +0200
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Simon Glass <sjg@...omium.org>
Cc:     linux-arm-kernel@...ts.infradead.org,
        U-Boot Mailing List <u-boot@...ts.denx.de>,
        Tom Rini <trini@...sulko.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nick Terrell <terrelln@...com>,
        Nicolas Schier <nicolas@...sle.eu>,
        Will Deacon <will@...nel.org>, linux-kbuild@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] arm64: boot: Support Flat Image Tree

On Sat, Nov 4, 2023 at 9:42 PM Simon Glass <sjg@...omium.org> wrote:
>  PHONY += $(BOOT_TARGETS)
>
> @@ -215,6 +215,7 @@ virtconfig:
>  define archhelp
>    echo  '* Image.gz      - Compressed kernel image (arch/$(ARCH)/boot/Image.gz)'
>    echo  '  Image         - Uncompressed kernel image (arch/$(ARCH)/boot/Image)'
> +$(if $(CONFIG_EFI_ZBOOT),,echo  '  image.fit     - Flat Image Tree (arch/$(ARCH)/boot/image.fit)')



Why should this be hidden when CONFIG_EFI_ZBOOT=y?




>    echo  '  install       - Install uncompressed kernel'
>    echo  '  zinstall      - Install compressed kernel'
>    echo  '                  Install using (your) ~/bin/installkernel or'
> diff --git a/arch/arm64/boot/.gitignore b/arch/arm64/boot/.gitignore
> index af5dc61f8b43..f84a7073dbcd 100644
> --- a/arch/arm64/boot/.gitignore
> +++ b/arch/arm64/boot/.gitignore
> @@ -2,3 +2,5 @@
>  Image
>  Image.gz
>  vmlinuz*
> +image.itk
> +image.fit
> diff --git a/arch/arm64/boot/Makefile b/arch/arm64/boot/Makefile
> index 1761f5972443..2681f54cd2c8 100644
> --- a/arch/arm64/boot/Makefile
> +++ b/arch/arm64/boot/Makefile
> @@ -16,7 +16,8 @@
>
>  OBJCOPYFLAGS_Image :=-O binary -R .note -R .note.gnu.build-id -R .comment -S
>
> -targets := Image Image.bz2 Image.gz Image.lz4 Image.lzma Image.lzo Image.zst
> +targets := Image Image.bz2 Image.gz Image.lz4 Image.lzma Image.lzo \
> +       Image.zst image.fit
>
>  $(obj)/Image: vmlinux FORCE
>         $(call if_changed,objcopy)
> @@ -39,6 +40,10 @@ $(obj)/Image.lzo: $(obj)/Image FORCE
>  $(obj)/Image.zst: $(obj)/Image FORCE
>         $(call if_changed,zstd)
>
> +# Provide the kernel for the FIT
> +$(obj)/image.itk: $(obj)/Image FORCE
> +       $(call if_changed,copy)
> +
>  EFI_ZBOOT_PAYLOAD      := Image
>  EFI_ZBOOT_BFD_TARGET   := elf64-littleaarch64
>  EFI_ZBOOT_MACH_TYPE    := ARM64
> @@ -48,3 +53,5 @@ EFI_ZBOOT_OBJCOPY_FLAGS       = --add-symbol zboot_code_size=0x$(shell \
>                                 $(NM) vmlinux|grep _kernel_codesize|cut -d' ' -f1)
>
>  include $(srctree)/drivers/firmware/efi/libstub/Makefile.zboot
> +
> +clean-files := image.itk
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 03e79e319293..1427dba4f0f9 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -494,7 +494,23 @@ quiet_cmd_uimage = UIMAGE  $@
>                         -C $(UIMAGE_COMPRESSION) $(UIMAGE_OPTS-y) \
>                         -T $(UIMAGE_TYPE) \
>                         -a $(UIMAGE_LOADADDR) -e $(UIMAGE_ENTRYADDR) \
> -                       -n $(UIMAGE_NAME) -d $< $@
> +                       -n "$(UIMAGE_NAME)" -d $< $@


Please do not do this.
Just drop 2/3.




> +
> +# Flat Image Tree (FIT)
> +# This allows for packaging of a kernel and all devicetrees files, using
> +# compression.
> +# ---------------------------------------------------------------------------
> +
> +MAKE_FIT := $(srctree)/scripts/make_fit.py
> +
> +quiet_cmd_fit = FIT     $@
> +      cmd_fit = $(MAKE_FIT) -f $@ --arch $(UIMAGE_ARCH) --os linux \
> +                       --name "$(UIMAGE_NAME)" \
> +                       --compress $(UIMAGE_COMPRESSION) -k $< \
> +                       $(dir $<)/dts
> +
> +$(obj)/%.fit: $(obj)/%.itk $(MAKE_FIT) FORCE
> +       $(call if_changed,fit,gzip)


No. This is worse than v1.

Please do not create a silly copy.





As I said in v1, this if_changed does not catch the DTS updates.
So, there is no point in using it.

I recommend just use 'cmd'.


$(obj)/image.fit: $(obj)/Image FORCE
        $(call cmd,fit)





> +
> +    Returns:
> +        tuple:
> +            str: Model name
> +            bytes: Compatible stringlist
> +    """
> +    with fsw.add_node(f'fdt-{seq}'):
> +        # Get the compatible / model information
> +        with open(fname, 'rb') as inf:
> +            data = inf.read()
> +        fdt = libfdt.FdtRo(data)
> +        model = fdt.getprop(0, 'model').as_str()
> +        compat = fdt.getprop(0, 'compatible')
> +
> +        fsw.property_string('description', model)
> +        fsw.property_string('type', 'flat_dt')
> +        fsw.property_string('arch', 'arm64')


Why hard-code 'arm64' ?





-- 
Best Regards
Masahiro Yamada