linux-kernel - Re: [PATCH v3] kbuild: Enable DT schema checks for %.dtb targets

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: <CAK7LNAT_e0sGuzfT3FMJ=afutxiWYQxAJtV2ZcMAPiySg1+aGg@mail.gmail.com>
Date:   Sat, 16 Jul 2022 18:48:03 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Rob Herring <robh@...nel.org>
Cc:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Tom Rini <trini@...sulko.com>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH v3] kbuild: Enable DT schema checks for %.dtb targets

On Sat, Jul 16, 2022 at 5:12 PM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> On Sat, Jul 16, 2022 at 8:02 AM Rob Herring <robh@...nel.org> wrote:
> >
> > On Thu, Jun 23, 2022 at 8:44 AM Dmitry Baryshkov
> > <dmitry.baryshkov@...aro.org> wrote:
> > >
> > > It is possible to build a single dtb, but not with DT schema validation
> > > enabled. Enable the schema validation to run for %.dtb and %.dtbo
> > > targets. Anyone building a dtb for a specific platform *should* pay
> > > attention to schema warnings.
> > >
> > > This could be supported with a separate %.dt.yaml target instead.
> > > However, the .dt.yaml format is considered an intermediate format and
> > > could possibly go away at some point if schema checking is integrated
> > > into dtc. Also, the plan is to enable the schema checks by default once
> > > platforms are free of warnings, and this is a move in that direction.
> > >
> > > This patch differs from the previous one ([1]) in the fact that it
> > > requires specifying VALIDATE_DT=1 to run the checks while doing the
> > > build. Thus default build procedures would not obtain additional build
> > > dependency, while maintainers can still build a single DTB file an get
> > > only corresponding warnings.
> >
> > I'd rather this be a kconfig option, so that eventually 'make
> > allmodconfig; make dtbs' also runs the schema checks. If something can
> > be enabled for allmodconfig, then builders will automatically start
> > testing it. Though the extra dependency is a problem here.
>
>
> The dependency on libyaml is gone.
>
> As for the dependency on dt-schema, is it a good idea to
> pull it into the kernel tree somewhere,
> like we periodically sync scripts/dtc/ with its upstream?
>
> Any other problematic tool dependency?
>
>
>
>
>
> >
> > >
> > > [1] https://lore.kernel.org/all/20210913145146.766080-1-robh@kernel.org/
> > >
> > > Cc: Bjorn Andersson <bjorn.andersson@...aro.org>
> > > Cc: Tom Rini <trini@...sulko.com>
> > > Cc: Masahiro Yamada <masahiroy@...nel.org>
> > > Cc: linux-kbuild@...r.kernel.org
> > > Co-developed-by: Rob Herring <robh@...nel.org>
> > > Signed-off-by: Rob Herring <robh@...nel.org>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > > ---
> > >  Makefile | 18 ++++++++++++++----
> > >  1 file changed, 14 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index c43d825a3c4c..0942922384c4 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1365,11 +1365,17 @@ endif
> > >
> > >  ifneq ($(dtstree),)
> > >
> > > -%.dtb: include/config/kernel.release scripts_dtc
> > > -       $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
> > > +ifneq ($(VALIDATE_DT),)
> > > +DT_YAML = $(dtstree)/$*.dt.yaml
> >
> > .dt.yaml files are deprecated now. This probably doesn't do anything.
>
> Right, this causes a build error.
>
>
> masahiro@...ver:~/ref/linux$ make ARCH=arm64 VALIDATE_DT=1
> arm/foundation-v8.dtb
> arch/arm64/Makefile:36: Detected assembler with broken .inst;
> disassembly will be unreliable
>   DTC     arch/arm64/boot/dts/arm/foundation-v8.dtb
>   CHECK   arch/arm64/boot/dts/arm/foundation-v8.dtb
> /home/masahiro/ref/linux/arch/arm64/boot/dts/arm/foundation-v8.dtb:
> sysreg@...00: '#address-cells' is a required property
> From schema: /home/masahiro/ref/linux/Documentation/devicetree/bindings/arm/vexpress-sysreg.yaml
> /home/masahiro/ref/linux/arch/arm64/boot/dts/arm/foundation-v8.dtb:
> sysreg@...00: '#size-cells' is a required property
> From schema: /home/masahiro/ref/linux/Documentation/devicetree/bindings/arm/vexpress-sysreg.yaml
> make[1]: *** No rule to make target
> 'arch/arm64/boot/dts/arm/foundation-v8.dt.yaml'.  Stop.
> make: *** [Makefile:1379: arm/foundation-v8.dtb] Error 2
>
>
>
>
>
>
>
> --
> Best Regards
>
> Masahiro Yamada





I think v2 was better than v3 at least.
(v2 reuses existing CHECK_DTBS instead of adding new VALIDATE_DT)

v2: https://lore.kernel.org/linux-kbuild/20220706114407.1507412-1-dmitry.baryshkov@linaro.org/

I commented there for a simpler implementation if we go this way.






-- 
Best Regards
Masahiro Yamada