| 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
| ||
|
Message-ID: <568E026D.4030105@gmail.com>
Date: Wed, 06 Jan 2016 22:15:09 -0800
From: Frank Rowand <frowand.list@...il.com>
To: Rob Herring <robh+dt@...nel.org>
CC: Grant Likely <grant.likely@...aro.org>,
David Gibson <david@...son.dropbear.id.au>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Linux Kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] dtc: create tool to diff device trees
Hi Rob,
Thanks for the comments.
On 1/4/2016 1:29 PM, Rob Herring wrote:
> On Thu, Dec 31, 2015 at 3:12 AM, Frank Rowand <frowand.list@...il.com> wrote:
>> From: Frank Rowand <frank.rowand@...ymobile.com>
>>
>> Create script to diff device trees.
>>
>> The device tree can be in any of the forms recognized by the dtc compiler:
>> - source
>> - binary blob
>> - file system tree (from /proc/devicetree)
>>
>> If the device tree is a source file, then it is pre-processed in the
>> same way as it would be when built in the linux kernel source tree
>> before diffing.
>
> In general, I'd like to see some of this move to dtc and the kernel
> dependencies minimized.
>
>>
>> Signed-off-by: Frank Rowand <frank.rowand@...ymobile.com>
>> ---
>>
>> Tools to develop and debug device tree are somewhat inadequate. This is a
>> small step in improving the situation.
>>
>> Rationale for and examples of using the script are provided in slides
>> 1 - 78 of the elce 2015 presentation "Solving Device Tree Issues",
>> which can be found at:
>>
>> http://elinux.org/images/0/04/Dt_debugging_elce_2015_151006_0421.pdf
>>
>> (The script was named dtdiff instead of dtx_diff in the presentation.)
>>
>>
>> scripts/dtc/dtx_diff | 368 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 368 insertions(+)
>>
>> Index: b/scripts/dtc/dtx_diff
>> ===================================================================
>> --- /dev/null
>> +++ b/scripts/dtc/dtx_diff
>> @@ -0,0 +1,368 @@
>> +#! /bin/bash
>
> Add "-e" by default?
Nope. There are several commands in the script that can fail in
normal operation, but the script needs to continue.
>
>> +
>> +# Copyright (C) 2015 Frank Rowand
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; version 2 of the License.
>> +
>> +
>> +usage() {
>> +
>> + # use spaces instead of tabs in the usage message
>> + cat >&2 <<eod
>> +
>> +Usage:
>> +
>> + `basename $0` DTx
>> + decompile DTx
>> +
>> + `basename $0` DTx_1 DTx_2
>> + diff DTx_1 and DTx_2
>> +
>> +
>> + -f print full dts in diff (--unified=99999)
>> + -h synonym for --help
>> + -help synonym for --help
>> + --help print this message and exit
>> + -s SRCTREE linux kernel source tree is at path SRCTREE
>> + (default is current directory)
>> + -S linux kernel source tree is at root of current git repo
>
> Can't this be detected? There are some cases of kernel trees which are
> not in git.
-S is just an easier option to use than -s if the script is run from within
the linux kernel source tree if it is a git repo. If the script is run
from the root of the tree then there is no need to specify -s or -S -- it
just works.
>
>> + -u unsorted, do not sort DTx
>> +
>> +
>> +Each DTx is processed by the dtc compiler to produce a sorted dts source
>> +file. If DTx is a dts source file then it is pre-processed in the same
>> +manner as done for the compile of the dts source file in the Linux kernel
>> +build system ('#include' and '/include/' directives are processed).
>> +
>> +If two DTx are provided, the resulting dts source files are diffed.
>> +
>> +If DTx is a directory, it is treated as a DT subtree, such as
>> + /proc/device-tree.
>> +
>> +If DTx contains the binary blob magic value in the first four bytes,
>> + it is treated as a binary blob (aka .dtb or FDT).
>> +
>> +Otherwise DTx is treated as a dts source file (aka .dts).
>> +
>> + If this script is not run from the root of the linux source tree,
>> + and DTx utilizes '#include' or '/include/' then the path of the
>> + linux source tree can be provided by '-s SRCTREE' or '-S' so that
>> + include paths will be set properly.
>> +
>> + The shell variable \${ARCH} must provide the architecture containing
>> + the dts source file for include paths to be set properly for '#include'
>> + or '/include/' to be processed.
>> +
>> + If DTx_1 and DTx_2 are in different architectures, then this script
>> + may not work since \${ARCH} is part of the include path. Two possible
>> + workarounds:
>> +
>> + `basename $0` \\
>> + <(ARCH=arch_of_dtx_1 `basename $0` DTx_1) \\
>> + <(ARCH=arch_of_dtx_2 `basename $0` DTx_2)
>> +
>> + `basename $0` ARCH=arch_of_dtx_1 DTx_1 >tmp_dtx_1.dts
>> + `basename $0` ARCH=arch_of_dtx_2 DTx_2 >tmp_dtx_2.dts
>> + `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
>> + rm tmp_dtx_1.dts tmp_dtx_2.dts
>> +
>> + If DTx_1 and DTx_2 are in different directories, then this script will
>> + add the path of DTx_1 and DTx_2 to the include paths. If DTx_2 includes
>> + a local file that exists in both the path of DTx_1 and DTx_2 then the
>> + file in the path of DTx_1 will incorrectly be included. Possible
>> + workaround:
>> +
>> + `basename $0` DTx_1 >tmp_dtx_1.dts
>> + `basename $0` DTx_2 >tmp_dtx_2.dts
>> + `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
>> + rm tmp_dtx_1.dts tmp_dtx_2.dts
>> +
>> +eod
>> +}
>> +
>> +
>> +compile_to_dts() {
>> +
>> + dtx="$1"
>> +
>> + if [ -d "${dtx}" ] ; then
>> +
>> + # ----- input is file tree
>> +
>> + if ( ! ${DTC} -I fs ${dtx} ) ; then
>> + exit 3
>> + fi
>> +
>> + elif [ -f "${dtx}" ] && [ -r "${dtx}" ] ; then
>> +
>> + magic=`hexdump -n 4 -e '/1 "%02x"' ${dtx}`
>> + if [ "${magic}" = "d00dfeed" ] ; then
>
> Perhaps this auto-detection should be part of dtc?
The script needs to invoke dtc differently for a source dts (it
needs to preprocess with cpp) so we need this check here
anyway.
>
>> +
>> + # ----- input is FDT (binary blob)
>> +
>> + if ( ! ${DTC} -I dtb ${dtx} ) ; then
>> + exit 3
>> + fi
>> +
>> + else
>> +
>> + # ----- input is DTS (source)
>> +
>> + if ( cpp ${cpp_flags} -x assembler-with-cpp ${dtx} | ${DTC} -I dts ) ; then
>> + return
>> + fi
>> +
>> + echo "" >&2
>> + echo "Possible hints to resolve the above error:" >&2
>> + echo " (hints might not fix the problem)" >&2
>> +
>> + hint_given=0
>> +
>> + if [ "${ARCH}" = "" ] ; then
>> + hint_given=1
>> + echo "" >&2
>> + echo " shell variable \$ARCH not set" >&2
>> + fi
>> +
>> + dtx_arch=`echo "/${dtx}" | sed -e 's|.*/arch/||' -e 's|/.*||'`
>> +
>> + if [ "${dtx_arch}" != "" -a "${dtx_arch}" != "${ARCH}" ] ; then
>> + hint_given=1
>> + echo "" >&2
>> + echo " architecture ${dtx_arch} is in file path, but" >&2
>> + echo " does not match shell variable \$ARCH (${ARCH})" >&2
>> + fi
>> +
>> + if [ ! -d ${srctree}/arch/${ARCH} ] ; then
>> + hint_given=1
>> + echo "" >&2
>> + echo " ${srctree}/arch/${ARCH}/ does not exist" >&2
>> + echo " Is \$ARCH='${ARCH}' correct?" >&2
>> + echo " Possible fix: use '-s' option" >&2
>> +
>> + git_root=`git rev-parse --show-toplevel 2>/dev/null`
>> + if [ -d ${git_root}/arch/ ] ; then
>> + echo " Possible fix: use '-S' option" >&2
>> + fi
>> + fi
>> +
>> + if [ $hint_given = 0 ] ; then
>> + echo "" >&2
>> + echo " No hints available." >&2
>> + fi
>> +
>> + echo "" >&2
>> +
>> + exit 3
>> +
>> + fi
>> + else
>> + echo "" >&2
>> + echo "ERROR: ${dtx} does not exist or is not readable" >&2
>> + echo "" >&2
>> + exit 2
>> + fi
>> +
>> +}
>> +
>> +
>> +# ----- start of script
>> +
>> +cmd_diff=0
>> +diff_flags="-u"
>> +dtx_file_1=""
>> +dtx_file_2=""
>> +dtc_sort="-s"
>> +help=0
>> +srctree=""
>> +
>> +
>> +while [ $# -gt 0 ] ; do
>> +
>> + case $1 in
>> +
>> + -f )
>> + diff_flags="--unified=999999"
>> + shift
>> + ;;
>> +
>> + -h | -help | --help )
>> + help=1
>> + shift
>> + ;;
>> +
>> + -s )
>> + srctree="$2"
>> + shift 2
>> + ;;
>> +
>> + -S )
>> + git_root=`git rev-parse --show-toplevel 2>/dev/null`
>> + srctree="${git_root}"
>> + shift
>> + ;;
>> +
>> + -u )
>> + dtc_sort=""
>> + shift
>> + ;;
>> +
>> + *)
>> + if [ "${dtx_file_1}" = "" ] ; then
>> + dtx_file_1="$1"
>> + elif [ "${dtx_file_2}" = "" ] ; then
>> + dtx_file_2="$1"
>> + else
>> + echo "" >&2
>> + echo "ERROR: Unexpected parameter: $1" >&2
>> + echo "" >&2
>> + exit 2
>> + fi
>> + shift
>> + ;;
>> +
>> + esac
>> +
>> +done
>> +
>> +if [ "${srctree}" = "" ] ; then
>> + srctree="."
>> +fi
>> +
>> +if [ "${dtx_file_2}" != "" ]; then
>> + cmd_diff=1
>> +fi
>> +
>> +if (( ${help} )) ; then
>> + usage
>> + exit 1
>> +fi
>> +
>> +# this must follow check for ${help}
>> +if [ "${dtx_file_1}" = "" ]; then
>> + echo "" >&2
>> + echo "ERROR: parameter DTx required" >&2
>> + echo "" >&2
>> + exit 2
>> +fi
>> +
>> +
>> +# ----- prefer dtc from linux kernel, allow fallback to dtc in $PATH
>> +
>> +if [ "${KBUILD_OUTPUT:0:2}" = ".." ] ; then
>> + __KBUILD_OUTPUT="${srctree}/${KBUILD_OUTPUT}"
>> +elif [ "${KBUILD_OUTPUT}" = "" ] ; then
>> + __KBUILD_OUTPUT="."
>> +else
>> + __KBUILD_OUTPUT="${KBUILD_OUTPUT}"
>> +fi
>> +
>> +DTC="${__KBUILD_OUTPUT}/scripts/dtc/dtc"
>> +
>> +if [ ! -x ${DTC} ] ; then
>> + __DTC="dtc"
>> + if ( ! which ${__DTC} >/dev/null ) ; then
>> +
>> + # use spaces instead of tabs in the error message
>> + cat >&2 <<eod
>> +
>> +ERROR: unable to find a 'dtc' program
>> +
>> + Preferred 'dtc' (built from Linux kernel source tree) was not found or
>> + is not executable.
>> +
>> + 'dtc' is: ${DTC}
>> +
>> + If it does not exist, create it from the root of the Linux source tree:
>> +
>> + 'make scripts'.
>> +
>> + If not at the root of the Linux kernel source tree -s SRCTREE or -S
>> + may need to be specified to find 'dtc'.
>> +
>> + If 'O=\${dir}' is specified in your Linux builds, this script requires
>> + 'export KBUILD_OUTPUT=\${dir}' or add \${dir}/scripts/dtc to \$PATH
>> + before running.
>> +
>> + If \${KBUILD_OUTPUT} is a relative path, then '-s SRCDIR', -S, or run
>> + this script from the root of the Linux kernel source tree is required.
>> +
>> + Fallback '${__DTC}' was also not in \${PATH} or is not executable.
>> +
>> +eod
>> + exit 2
>> + fi
>> + DTC=${__DTC}
>> +fi
>> +
>> +
>> +# ----- cpp and dtc flags same as for linux source tree build of .dtb files,
>> +# plus directories of the dtx file(s)
>> +
>> +dtx_path_1_dtc_include="-i `dirname ${dtx_file_1}`"
>> +
>> +dtx_path_2_dtc_include=""
>> +if (( ${cmd_diff} )) ; then
>> + dtx_path_2_dtc_include="-i `dirname ${dtx_file_2}`"
>> +fi
>> +
>> +cpp_flags="\
>> + -nostdinc \
>> + -I${srctree}/arch/${ARCH}/boot/dts \
>> + -I${srctree}/arch/${ARCH}/boot/dts/include \
>> + -I${srctree}/arch/${ARCH}/boot/dts/include/dt-bindings \
>
> kernel doesn't use this path. The less dependency on ARCH, the better.
> The dependency in the kernel is artificial.
Yep, ARCH is a pain. But it is used in the kernel build of dt files.
See "dtc_cpp_flags" in scripts/Makefile.lib. I'm not sure why I
have dt-bindings in there though. I'll have to check that.
>
>> + -I${srctree}/drivers/of/testcase-data \
>> + -undef -D__DTS__"
>> +
>> +_dts_makefile=""
>> +
>> +if [ -f ${srctree}/arch/${ARCH}/boot/dts/Makefile ] ; then
>> + _dts_makefile="${srctree}/arch/${ARCH}/boot/dts/Makefile"
>> +elif [ -f ${srctree}/arch/${ARCH}/boot/Makefile ] ; then
>> + # arch/powerpc
>> + _dts_makefile="${srctree}/arch/${ARCH}/boot/Makefile"
>
> We need to move PPC to be in line with other arches rather than
> working around it.
>
>> +fi
>> +
>> +if [ "${_dts_makefile}" != "" ] ; then
>> + arch_dtc_flags=`grep DTC_FLAGS ${_dts_makefile} \
>
> We should just kill DTC_FLAGS being per arch. Plus it is only used for
> padding, so it is irrelevant to this tool.
Yes it is irrelevant. The only reason I included it was just to future
proof for if anyone snuck anything else into the DTC_FLAGS in the future.
I'll just remove this section (resolving the issue with working around
the location of the PPC makefile too).
>
>> + | grep -v "^#" \
>> + | sed -e 's|.*=||' `
>> +else
>> + arch_dtc_flags=""
>> +fi
>> +
>> +dtc_flags="\
>> + -i ${srctree}/arch/${ARCH}/boot/dts/ \
>> + -i ${srctree}/kernel/dts \
>> + ${dtx_path_1_dtc_include} \
>> + ${dtx_path_2_dtc_include} \
>> + ${arch_dtc_flags}"
>> +
>> +DTC="${DTC} ${dtc_flags} -O dts -qq -f ${dtc_sort} -o -"
>> +
>> +
>> +# ----- do the diff or decompile
>> +
>> +if (( ${cmd_diff} )) ; then
>> +
>> + diff ${diff_flags} \
>> + <(compile_to_dts "${dtx_file_1}") \
>> + <(compile_to_dts "${dtx_file_2}")
>> +
>> +else
>> +
>> + compile_to_dts "${dtx_file_1}"
>> +
>> +fi
>> +
>> +
>> +# ==============================================================================
>> +# ______________________________________________________________________________
>> +# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
>> +# ______________________________________________________________________________
>> +# vi config follows:
>> +#
>> +# add "set modelines" to ~/.exrc or ~/.vimrc to set tabs automatically
>> +# ex:set tabstop=4 shiftwidth=4 sts=4:
>
> Kernel standard is 8 characters. I assume that applies to bash files too.
I'll reformat to 8 characters.
>
> Rob
-Frank
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists