linux-kernel - Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

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]

Date:	Tue, 16 Nov 2010 16:39:47 -0800
From:	David Daney <ddaney@...iumnetworks.com>
To:	dirk.brandewie@...il.com
CC:	devicetree-discuss@...ts.ozlabs.org, sodaville@...utronix.de,
	arjan@...ux.intel.com, linuxppc-dev@...ts.ozlabs.org,
	microblaze-uclinux@...e.uq.edu.au, linux-arch@...r.kernel.org,
	linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into
 vmlinux

Thanks for doing this.  However I have a few comments...

On 11/16/2010 02:41 PM, dirk.brandewie@...il.com wrote:
> From: Dirk Brandewie<dirk.brandewie@...il.com>
>
> This patch adds support for linking device tree blobs into
> vmlinux. Modifies asm-generic/vmlinux.lds.h to add linking
> .dtb.init.rodata sections into the .init.data section of the vmlinux
> image. Modifies scripts/Makefile.lib to add a kbuild command to
> compile DTS files to device tree blobs and a rule to create objects to
> wrap the blobs for linking.
>
> The DTB's are placed on 32 byte boundries to allow parsing the blob
> with driver/of/fdt.c during early boot without having to copy the blob
> to get the structure alignment GCC expects.
>
> A DTB is linked in by adding the DTB object to the list of objects to
> be linked into vmlinux in the archtecture specific Makefile using
>     obj-y += foo.dtb.o
>
> Signed-off-by: Dirk Brandewie<dirk.brandewie@...il.com>
> ---
>   include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
>   scripts/Makefile.lib              |   20 ++++++++++++++++++++
>   2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index bd69d79..ea671e7 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -67,7 +67,14 @@
>    * Align to a 32 byte boundary equal to the
>    * alignment gcc 4.5 uses for a struct
>    */
> -#define STRUCT_ALIGN() . = ALIGN(32)
> +#define STRUCT_ALIGNMENT 32
> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
> +
> +/* Device tree blobs linked into the kernel need to have proper
> + * structure alignment to be parsed by the flat device tree library
> + * used in early boot
> +*/
> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>
>   /* The actual configuration determine if the init/exit sections
>    * are handled as text/data or they can be discarded (which
> @@ -146,6 +153,13 @@
>   #define TRACE_SYSCALLS()
>   #endif
>
> +
> +#define KERNEL_DTB()							\
> +	. = ALIGN(DTB_ALIGNMENT);					\
> +	VMLINUX_SYMBOL(__dtb_start) = .;				\
> +	*(.dtb.init.rodata)						\
> +	VMLINUX_SYMBOL(__dtb_end) = .;
> +
>   /* .data section */
>   #define DATA_DATA							\
>   	*(.data)							\
> @@ -468,7 +482,8 @@
>   	MCOUNT_REC()							\
>   	DEV_DISCARD(init.rodata)					\
>   	CPU_DISCARD(init.rodata)					\
> -	MEM_DISCARD(init.rodata)
> +	MEM_DISCARD(init.rodata)					\
> +	KERNEL_DTB()
>

I thought the init.rodata was only for data used by __init things. 
Although the current linker scripts do not put it in the section that 
gets recycled as usable memory.

IIRC the unflattened version of the device tree has pointers to the 
flattened data.  Since the device tree nodes are live for the entire 
kernel lifecycle, shouldn't the device tree blobs be in non-init memory?


>   #define INIT_TEXT							\
>   	*(.init.text)							\
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 4c72c11..29db062 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -200,6 +200,26 @@ quiet_cmd_gzip = GZIP    $@
>   cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9>  $@) || \
>   	(rm -f $@ ; false)
>
> +# DTC
> +#  ---------------------------------------------------------------------------
> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE

Why FORCE?


> +	@echo '#include<asm-generic/vmlinux.lds.h>'>  $@
> +	@echo '.section .dtb.init.rodata,"a"'>>  $@
> +	@echo '.balign DTB_ALIGNMENT'>>  $@
> +	@echo '.global __dtb_$(*F)_begin'>>  $@
> +	@echo '__dtb_$(*F)_begin:'>>  $@
> +	@echo '.incbin "$<" '>>  $@
> +	@echo '__dtb_$(*F)_end:'>>  $@
> +	@echo '.global __dtb_$(*F)_end'>>  $@
> +	@echo '.balign DTB_ALIGNMENT'>>  $@
> +
> +DTC = $(objtree)/scripts/dtc/dtc
> +
> +quiet_cmd_dtc = DTC	$@
> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(src)/dts/$*.dts
> +
> +$(obj)/%.dtb: $(src)/dts/%.dts
> +	$(call if_changed,dtc)
>

Do all the generated files get cleaned up?

I will try it tomorrow to see for sure.


Thanks,
David Daney
--
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/