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

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

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

On 09.08.24 12:59, '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

Fairly subtle optimization. Can't we identify the default image(s) the
kernel would build and fill this automatically?
Yes, we can do that, but I don't think we want that because if there are multiple targets(image types) enabled 
we wouldn't know what to choose. The reason being the difference between a full build and a specific target build. 
So far I have identified 2 differences between a full build and a specific target build: one is the dtbs target and the
other one is the different image types that are produced in one build session. e.g: for arm the kernel might have the 
configuration to build both zImage and Image.gz(full-build case) but we need only one image for deployment(specific target build).
However, building all of these images every time might not be necessary, which is where the KERNEL_IMAGETYPE comes into
the picture hence the reason for these changes. 

> 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.

That 50M is build-time space, right? What's the size difference on the
target?
Yes, 50M disk space. I don't have a phyboard-mira at my disposal, but I can
try on a iot2050 board to see the difference on the target.

> 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

We are not yet using dtbo files (overlays) built by the kernel in any
projects I know, but those would no longer be built then, right?
Exactly, but these changes are still keeping the full-build case in place
As you said, because I haven't seen any dtbo files around I left this out for the moment
and because the changes were starting to be quite big.(can be treated as a follow up on this
as compiling the dtbo is in line with the dtb)

>
> Signed-off-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>
> ---
>  meta-isar/conf/machine/imx6-sabrelite.conf    |  2 +
>  meta-isar/conf/machine/phyboard-mira.conf     |  2 +
>  .../linux/files/debian/isar/build.tmpl        | 63 +++++++++++++++++--
>  .../linux/files/debian/isar/install.tmpl      | 12 +++-
>  meta/recipes-kernel/linux/linux-custom.inc    |  6 ++
>  5 files changed, 79 insertions(+), 6 deletions(-)
>
> diff --git a/meta-isar/conf/machine/imx6-sabrelite.conf b/meta-isar/conf/machine/imx6-sabrelite.conf
> index e48823d6..bffebdc1 100644
> --- a/meta-isar/conf/machine/imx6-sabrelite.conf
> +++ b/meta-isar/conf/machine/imx6-sabrelite.conf
> @@ -15,4 +15,6 @@ MKUBIFS_ARGS := "-m 0x1000 -e 0x3e000 -c 1500"
>  UBINIZE_ARGS = "-vv -m 0x1000 -p 0x40000"
>  IMAGE_FSTYPES ?= "ubi-ubifs"
>
> +KERNEL_IMAGETYPE = "zImage"
> +
>  DTB_FILES = "imx6q-sabrelite.dtb"
> diff --git a/meta-isar/conf/machine/phyboard-mira.conf b/meta-isar/conf/machine/phyboard-mira.conf
> index feec4e54..1d2b3e1c 100644
> --- a/meta-isar/conf/machine/phyboard-mira.conf
> +++ b/meta-isar/conf/machine/phyboard-mira.conf
> @@ -20,6 +20,8 @@ DTB_FILES = "imx6q-phytec-mira-rdk-nand.dtb"
>
>  IMAGE_INSTALL += "barebox"
>
> +KERNEL_IMAGETYPE = "zImage"
> +
>  BAREBOX_CONFIG = "imx_v7_defconfig"
>  BAREBOX_ENV = "phytec-mira-env"
>  BAREBOX_BASE_BIN = "barebox-phytec-phycore-imx6q-som-nand-1gib"
> diff --git a/meta/recipes-kernel/linux/files/debian/isar/build.tmpl b/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> index bafc5ca4..90222adc 100644
> --- a/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> +++ b/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> @@ -21,15 +21,21 @@ do_build() {
>      KR=$(${MAKE} O=${KERNEL_BUILD_DIR} -s --no-print-directory kernelrelease)
>      sed -i "s/@KR@/${KR}/g" ${S}/debian/control ${S}/debian/linux-image-${KERNEL_NAME_PROVIDED}.*
>
> -    if echo "${DEB_BUILD_PROFILES}" | grep -q "kernel"; then # Build kernel scripts and tools
> -        ${MAKE} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} ${KERNEL_EXTRA_BUILDARGS} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}"
> +    MAKE_COMMON_ARGS="O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} ${KERNEL_EXTRA_BUILDARGS} KCFLAGS=${KCFLAGS} KAFLAGS=${KAFLAGS}"
> +    if echo "${DEB_BUILD_PROFILES}" | grep -q "kernel"; then
> +        if [ -n "${KERNEL_IMAGETYPE}" ]; then # Override the default compilation
> +            ${MAKE} ${MAKE_COMMON_ARGS} ${KERNEL_IMAGETYPE} modules

You are losing KERNEL_EXTRA_BUILDARGS.

> +            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} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} ${KERNEL_EXTRA_BUILDARGS} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}" scripts
> +        ${MAKE} ${MAKE_COMMON_ARGS} ${KERNEL_EXTRA_BUILDARGS} scripts

Why this change and those 2 below? They look wrong.
These changes are some optimizations but after carefully inspecting these I saw that I need to fix some ${KERNEL_EXTRA_BUILDARGS} addition
and loosing in some areas.

>          if grep -q -E "CONFIG_STACK_VALIDATION=y|CONFIG_HAVE_OBJTOOL=y" ${KERNEL_BUILD_DIR}/.config && [ -d "tools/objtool" ]; then
> -            ${MAKE} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}" tools/objtool || true
> +            ${MAKE} ${MAKE_COMMON_ARGS} tools/objtool || true
>          fi
>          if grep -q "CONFIG_MODULES=y" ${KERNEL_BUILD_DIR}/.config; then
> -            ${MAKE} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}" modules_prepare
> +            ${MAKE} ${MAKE_COMMON_ARGS} modules_prepare
>          fi
>      fi
>
> @@ -37,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}=."

Quite a lot of new code for rather moderate optimizations. Maybe we
should rather filter the built files when installing? That should be
doable in 3 lines or so.

Jan

--
Siemens AG, Technology
Linux Expert Center

Yes, indeed seems a lot of code but these changes were made having in mind fixing the compilation of those extra dtbs that were not
belonging to that specific target bsp. And the optimizations are just a side effect(maybe as a bonus), they are not on the foreground. 
The reason because it's a lot of code is primarily because I wanted to keep the DTB_FILES declarations as is in the upper layers and bring
new features e.g full architecture path, etc. plus this handles a KERNEL Pre 6.x dtb builds

-- 
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/DB3PR10MB690833C214CB30C07972D8CFE6852%40DB3PR10MB6908.EURPRD10.PROD.OUTLOOK.COM.