Re: [PATCH 2/3] linux-custom: compile only those specified dtbs

["'nicusor.huhulea@siemens.com' via isar-users" <isar-users@googlegroups.com>]

The most important gain is that there will be alternative to the currently full build approach.

The change for Kernel Pre and Post 6.x seems to be in my opinion an improvement to handle the
increasing complexity particularly for ARM and happened once as far as I know, therefore I believe that the next
major restructuring will happen far in the future.

An evil vendor kernel? I don't think so. Based on this https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Makefile?h=v6.11-rc3#n1380 , the Makefile decide where the DTS should be located and how they are processed. So an evil vendor has to modify the Makefile to
place his dts in some other location and potentially introduce significant complexity. Second the evil vendor will have serious issues with the upstream compatibility e.g rebasing or merging upstream changes, etc. So my conclusion would be that it's very unlikely for a vendor to deviate
from the standard.

The Install-time filtering that you suggested won't give you the answer to the question: Why do I see several kernel image types being compiled
and why don't I have just the corresponding dtb for my board?

I have started another thread summarizing the discussion from this one and other pros for the specific target build.
https://groups.google.com/g/isar-users/c/e7EY6zCbZcs

________________________________________
From: Kiszka, Jan (T CED) <jan.kiszka@siemens.com>
Sent: Tuesday, August 13, 2024 3:33 PM
To: Huhulea, Nicusor Liviu (DI CTO FDS CES LX SVCS); isar-users@googlegroups.com
Subject: Re: [PATCH 2/3] linux-custom: compile only those specified dtbs

On 13.08.24 13:55, 'Nicusor Huhulea' via isar-users wrote:
> The current implementation makes a full kernel build and that means it will
> compile all dtbs specified by the bsp makefile e.g arch/arm64/boot/dts/ti/Makefile
> Currently the required dtbs are given by the DTB_FILES.
>
> These changes are checking if the DTB_FILES is specified, compile only those
> and avoids compiling all the dtbs from the bsp directory. Otherwise build all dtbs
> specified by the kernel source.
> Preventing the automatic dtb builds is made by using target specific build
> commands e.g KERNEL_IMAGETYPE
> The compilation of the dtbs accepts the following formats e.g:
> Kernel Post 6.x:
> a) full path from the architecture directory:
>    arch/${ARCH}/boot/dts/bsp_dir/test.dts
>    arch/${ARCH}/boot/dts/bsp_dir/test.dtb
> b) relative path from the dts directory: bsp_dir/test.dts
> c) device tree blob file: bsp_dir/test.dtb
> Kernel Pre 6.x:
> The same as on Kernel Post 6.x but without the bsp_dir
>
> The differences when building a full build and when using a specific target
> build are:(tests were made on the machine phyboard-mira on 6.x and 4.x versions)
> specific target: Build needed 00:02:08, 2219688k disk space linux-phy 6.x
> full build:      Build needed 00:02:14, 2271528k disk space linux-phy 6.x
> The time difference: the full build took 6s longer than the specific target build
> The disk space difference: the full build used 50.6MB more disk space than
> the specific target build.
> In conclusion the specific target build is slightly faster and uses less
> disk space.
>
> For now the DTB_FILES variable should accept only Device Tree Blobs(.dtb)
> because some areas still depends on it e.g: do_copy_boot_files, plugins
>
> Signed-off-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>
> ---
>  .../linux/files/debian/isar/build.tmpl        | 56 ++++++++++++++++++-
>  .../linux/files/debian/isar/install.tmpl      | 12 +++-
>  meta/recipes-kernel/linux/linux-custom.inc    |  6 ++
>  3 files changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-kernel/linux/files/debian/isar/build.tmpl b/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> index 1fd6f948..15201686 100644
> --- a/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> +++ b/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> @@ -22,8 +22,13 @@ do_build() {
>      sed -i "s/@KR@/${KR}/g" ${S}/debian/control ${S}/debian/linux-image-${KERNEL_NAME_PROVIDED}.*
>
>      MAKE_COMMON_ARGS="O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} KCFLAGS=${KCFLAGS} KAFLAGS=${KAFLAGS}"
> -    if echo "${DEB_BUILD_PROFILES}" | grep -q "kernel"; then # Full build
> -        ${MAKE} ${MAKE_COMMON_ARGS} ${KERNEL_EXTRA_BUILDARGS}
> +    if echo "${DEB_BUILD_PROFILES}" | grep -q "kernel"; then
> +        if [ -n "${KERNEL_IMAGETYPE}" ]; then # Override the default compilation
> +            ${MAKE} ${MAKE_COMMON_ARGS} ${KERNEL_EXTRA_BUILDARGS} ${KERNEL_IMAGETYPE} modules
> +            compile_dtbs
> +        else # Full build
> +            ${MAKE} ${MAKE_COMMON_ARGS} ${KERNEL_EXTRA_BUILDARGS}
> +        fi
>      elif echo "${DEB_BUILD_PROFILES}" | grep -q "kbuild"; then # Build kernel scripts and tools
>          ${MAKE} ${MAKE_COMMON_ARGS} ${KERNEL_EXTRA_BUILDARGS} scripts
>          if grep -q -E "CONFIG_STACK_VALIDATION=y|CONFIG_HAVE_OBJTOOL=y" ${KERNEL_BUILD_DIR}/.config && [ -d "tools/objtool" ]; then
> @@ -38,6 +43,53 @@ do_build() {
>      set +x
>  }
>
> +compile_dtbs() {
> +    local prefix="arch/${ARCH}/boot/dts/"
> +
> +    for dtb in ${DTB_FILES}; do
> +        # Handle the case where a standalone(with no path .dtb/.dts) file may reside within
> +        # a specific BSP directory or not(kernel pre-6.x)
> +        if [[ ("${dtb}" == *.dtb || "${dtb}" == *.dts) && "${dtb}" != */* ]]; then
> +            bsp_dir=""
> +            dts_path=""
> +            if [[ "${dtb}" == *.dtb ]]; then
> +                dts="${dtb%.dtb}.dts"
> +            else
> +                dts="${dtb}"
> +            fi
> +
> +            # recursively search for the dts file in all possible BSP directories
> +            while IFS= read -r dts_path; do
> +                if [ -n "${dts_path}" ]; then
> +                    # Eliminate the prefix and dts to get only the bsp_dir
> +                    bsp_dir="${dts_path#${prefix}}"
> +                    bsp_dir="${bsp_dir%${dts}}"
> +
> +                    relative_dtb_path="${bsp_dir}${dtb}"
> +                else
> +                    relative_dtb_path="${prefix}${dtb}"
> +                fi
> +            done < <(find "${prefix}" -type f -name "${dts}")
> +
> +        # Check if the path is relative (starts with arch/${ARCH}/)
> +        # and if it does then extract the relative path from the architecture specific path
> +        elif [[ "${dtb}" == "${prefix}"* ]]; then
> +            relative_dtb_path="${dtb#${prefix}}"
> +        else
> +            # directly use the given dtb
> +            relative_dtb_path="${dtb}"
> +        fi
> +
> +        # Check if it's a .dts file
> +        if [[ "${relative_dtb_path}" == *.dts ]]; then
> +            #  Change .dts to .dtb
> +            relative_dtb_path="${relative_dtb_path%.dts}.dtb"
> +        fi
> +
> +        ${MAKE} ${MAKE_COMMON_ARGS} ${relative_dtb_path}
> +    done
> +}
> +
>  print_settings() {
>      cat <<EOF
>  # Build settings:
> diff --git a/meta/recipes-kernel/linux/files/debian/isar/install.tmpl b/meta/recipes-kernel/linux/files/debian/isar/install.tmpl
> index 00011517..c0a60f4e 100644
> --- a/meta/recipes-kernel/linux/files/debian/isar/install.tmpl
> +++ b/meta/recipes-kernel/linux/files/debian/isar/install.tmpl
> @@ -124,7 +124,17 @@ install_hooks() {
>
>  install_dtbs() {
>      [ -n "${CONFIG_OF}" ] || return 0
> -    ${MAKE} O=${O} INSTALL_DTBS_PATH=${deb_img_dir}/usr/lib/linux-image-${krel} dtbs_install
> +    prefix="arch/${ARCH}/boot/dts/"
> +    src_dir="${O}/${prefix}"
> +    dst_dir=""
> +
> +    dst_dir="${deb_img_dir}/usr/lib/linux-image-${krel}/"
> +    find "${src_dir}" -type f -name '*.dtb' | while IFS= read -r dtb_file; do
> +        local relative_path="${dtb_file#"${src_dir}"}"
> +        mkdir -p "${dst_dir}$(dirname "${relative_path}")"
> +        cp "${dtb_file}" "${dst_dir}/${relative_path}"
> +        echo "Copying ${dtb_file} to ${dest_dir}/${relative_path}"
> +    done
>  }
>
>  install_kmods() {
> diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
> index 6aa70fd3..31cc480b 100644
> --- a/meta/recipes-kernel/linux/linux-custom.inc
> +++ b/meta/recipes-kernel/linux/linux-custom.inc
> @@ -76,6 +76,7 @@ TEMPLATE_VARS += "                \
>      KERNEL_DEBIAN_DEPENDS         \
>      KERNEL_BUILD_DIR              \
>      KERNEL_FILE                   \
> +    KERNEL_IMAGETYPE              \
>      KERNEL_HEADERS_DEBIAN_DEPENDS \
>      LINUX_VERSION_EXTENSION       \
>      KERNEL_NAME_PROVIDED          \
> @@ -84,12 +85,17 @@ TEMPLATE_VARS += "                \
>      KAFLAGS                       \
>      DISTRIBUTOR                   \
>      KERNEL_EXTRA_BUILDARGS        \
> +    DTB_FILES                     \
>  "
>
>  inherit dpkg
>  inherit template
>  inherit kbuildtarget
>
> +DTB_FILES ?= ""
> +
> +KERNEL_IMAGETYPE ?= ""
> +
>  # Add custom cflags to the kernel build
>  KCFLAGS ?= "-fdebug-prefix-map=${CURDIR}=."
>  KAFLAGS ?= "-fdebug-prefix-map=${CURDIR}=."

I'm still not convinces about effort / gain here. In addition: You
already had to address < 6.x (6.0?) vs. newer. Won't this open-coded dtb
build break again in future releases? Can't it cause problems with evil
vendor kernels?

I would rather vote for install-time filtering, not build-time.

Jan

--
Siemens AG, Technology
Linux Expert Center

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/DB3PR10MB69082B90DA26153EBB5D9F81E6872%40DB3PR10MB6908.EURPRD10.PROD.OUTLOOK.COM.