linux-kernel - Re: [PATCH 2/2] scripts/dtc: compile separate dtc-yaml

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: <CAL_JsqLRUAEmh3kwEo+T-dMhXZTBH_kwbBwKzZyhJMR5P92pwA@mail.gmail.com>
Date:   Mon, 4 May 2020 14:15:32 -0500
From:   Rob Herring <robh+dt@...nel.org>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     devicetree@...r.kernel.org,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Marek Beh��ân <marek.behun@....cz>,
        Frank Rowand <frowand.list@...il.com>,
        Michal Marek <michal.lkml@...kovi.net>
Subject: Re: [PATCH 2/2] scripts/dtc: compile separate dtc-yaml

On Sun, May 3, 2020 at 9:07 PM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> Marek Behún reported a case where pkg-config fails to detect the
> libyaml-dev package, which is presumably a bug of the distro.
>
> Irrespective of that, I am not a big fan of pkg-config in the Makefile
> parse stage. The cost of pkg-config is quite small, but it is evaluated
> everytime we run make, even when we do 'make mrproper'. This commit
> changes the Makefile to not rely on pkg-config at all.

I don't really love the solution here... I'm inclined to just make
libyaml always required. Anyone building dtbs should care about
validating them. However, there's some dts files sprinkled in the tree
such as DT unittests and I don't want to break allmodconfig for CI,
0-day, etc. Though eventually we may have to.

> The normal build should not require libyaml-dev while we need to compile
> dtc with libyaml for the schema check.
>
> Build two dtc variants:
>
>   scripts/dtc/dtc      for *.dts -> *.dtb
>   scripts/dtc/dtc-yaml for *.dts -> *.dt.yaml

My longer term plan is to integrate the schema checks into dtc. This
would be some sort of plugin to dtc found or specified at run-time. It
would eliminate the need for 2 passes of dtc and the 2nd case will go
away.

> 'make dtbs_check' or 'make dt_binding_check' without libyaml-dev installed
> will fail like this:
>
>   scripts/dtc/yamltree.c:9:10: fatal error: yaml.h: No such file or directory
>
> I hope people will notice what to do.
>
> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> ---
>
>  scripts/Makefile.lib      |  9 +++++----
>  scripts/dtc/.gitignore    |  1 +
>  scripts/dtc/Makefile      | 28 +++++++++++-----------------
>  scripts/dtc/dtc-no-yaml.c |  3 +++
>  4 files changed, 20 insertions(+), 21 deletions(-)
>  create mode 100644 scripts/dtc/dtc-no-yaml.c
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 8f9f2abf3d67..ca12412304b8 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -246,6 +246,7 @@ quiet_cmd_gzip = GZIP    $@
>  # DTC
>  # ---------------------------------------------------------------------------
>  DTC ?= $(objtree)/scripts/dtc/dtc
> +DTC_YAML ?= $(objtree)/scripts/dtc/dtc-yaml

Can we make 'DTC' override both and keep this an internal detail.

>  # Disable noisy checks by default
>  ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
> @@ -286,13 +287,13 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>
>  __cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
>         $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> -       $(DTC) -O $(1) -o $@ -b 0 \
> +       $(1) -O $(2) -o $@ -b 0 \
>                 $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
>                 -d $(depfile).dtc.tmp $(dtc-tmp) ; \
>         cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>
>  quiet_cmd_dtc = DTC     $@
> -      cmd_dtc = $(call __cmd_dtc,dtb)
> +      cmd_dtc = $(call __cmd_dtc,$(DTC),dtb)
>
>  $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
>         $(call if_changed_dep,dtc)
> @@ -303,7 +304,7 @@ DT_BINDING_DIR := Documentation/devicetree/bindings
>  DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml
>
>  quiet_cmd_dtc_yaml = DTCYAML $@
> -      cmd_dtc_yaml = $(call __cmd_dtc,yaml)
> +      cmd_dtc_yaml = $(call __cmd_dtc,$(DTC_YAML),yaml)
>
>  quiet_cmd_dtb_check =  CHECK   $@
>        cmd_dtb_check =  $(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@
> @@ -313,7 +314,7 @@ define rule_dtc_yaml
>         $(call cmd,dtb_check)
>  endef
>
> -$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
> +$(obj)/%.dt.yaml: $(src)/%.dts $(DTC_YAML) $(DT_TMP_SCHEMA) FORCE
>         $(call if_changed_rule,dtc_yaml)
>
>  dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
> diff --git a/scripts/dtc/.gitignore b/scripts/dtc/.gitignore
> index b814e6076bdb..e0cf8222c137 100644
> --- a/scripts/dtc/.gitignore
> +++ b/scripts/dtc/.gitignore
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  dtc
> +dtc-yaml
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index ef85f8b7d4a7..c6d7e2b70f08 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -1,28 +1,22 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # scripts/dtc makefile
>
> -hostprogs                      := dtc
> -always-$(CONFIG_DTC)           += $(hostprogs)
> -always-$(CHECK_DT_BINDING)     += $(hostprogs)
> +# Build the yaml variant for DT schema validation, which requires libyaml
> +# development package. The normal build does not need it.
> +hostprogs                      := dtc dtc-yaml
> +always-$(CONFIG_DTC)           += dtc
> +always-$(CHECK_DTBS)           += dtc-yaml
> +always-$(CHECK_DT_BINDING)     += dtc-yaml
>
> -dtc-objs       := dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
> -                  srcpos.o checks.o util.o
> -dtc-objs       += dtc-lexer.lex.o dtc-parser.tab.o
> +common-objs    := flattree.o fstree.o data.o livetree.o treesource.o \
> +                  srcpos.o checks.o util.o dtc-lexer.lex.o dtc-parser.tab.o
> +dtc-objs       := dtc-no-yaml.o $(common-objs)
> +dtc-yaml-objs  := dtc.o yamltree.o $(common-objs)
> +HOSTLDLIBS_dtc-yaml := -lyaml

Does this work for yocto? As we had this before commit 067c650c456e.
Not clear if this changed for any reason or just 'let's use pkg-config
everywhere'.

Or is there another way to fix yocto issue and we can just check the
header exists. I assume yocto needs some prefix in front of
'/usr/include/yaml.h'?

>  # Source files need to get at the userspace version of libfdt_env.h to compile
>  HOST_EXTRACFLAGS := -I $(srctree)/$(src)/libfdt
>
> -ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
> -ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
> -$(error dtc needs libyaml for DT schema validation support. \
> -       Install the necessary libyaml development package.)
> -endif
> -HOST_EXTRACFLAGS += -DNO_YAML
> -else
> -dtc-objs       += yamltree.o
> -HOSTLDLIBS_dtc := $(shell pkg-config yaml-0.1 --libs)
> -endif
> -
>  # Generated files need one more search path to include headers in source tree
>  HOSTCFLAGS_dtc-lexer.lex.o := -I $(srctree)/$(src)
>  HOSTCFLAGS_dtc-parser.tab.o := -I $(srctree)/$(src)
> diff --git a/scripts/dtc/dtc-no-yaml.c b/scripts/dtc/dtc-no-yaml.c
> new file mode 100644
> index 000000000000..f3459e45835f
> --- /dev/null
> +++ b/scripts/dtc/dtc-no-yaml.c
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#define NO_YAML
> +#include "dtc.c"
> --
> 2.25.1
>