[RFC] [PATCH] Adding DTB to architecture independent vmlinux

[RFC] [PATCH] Adding DTB to architecture independent vmlinux

[Dirk Brandewie]

Hi All,

I am working on working on adding support device tree support to an
x86 based platform, hpa pointed out that the device tree blob is
architecture independent and should be part of the generic kernel.

The patch below is a first cut at adding dtb support to the architecture
independent kernel and adding a generic dts->dtb build rule.

This has only been tested on x86.

Comments/suggestions gratefully accepted.

--Dirk

of: add support for linking platform dtb into vmlinux

From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch adds support for linking a device tree blob into
vmlinux. The platform DTB to be built and linked into the kernel is
specified by passing PLATFORM_DTB=<platform name> to make.

The command:
make PLATFORM_DTB=ce4100

will link the device tree blob into vmlinux

Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
  arch/powerpc/boot/zImage.coff.lds.S |    5 ++---
  arch/powerpc/boot/zImage.lds.S      |    5 ++---
  arch/powerpc/boot/zImage.ps3.lds.S  |    5 ++---
  arch/x86/kernel/Makefile            |   15 +++++++++++++++
  include/asm-generic/vmlinux.lds.h   |   12 ++++++++++++
  scripts/Makefile.lib                |    7 +++++++
  7 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/boot/zImage.coff.lds.S 
b/arch/powerpc/boot/zImage.coff.lds.S
index 856dc78..c23145f 100644
--- a/arch/powerpc/boot/zImage.coff.lds.S
+++ b/arch/powerpc/boot/zImage.coff.lds.S
@@ -1,3 +1,4 @@
+#include <asm-generic/vmlinux.lds.h>
  OUTPUT_ARCH(powerpc:common)
  ENTRY(_zimage_start_opd)
  EXTERN(_zimage_start_opd)
@@ -21,9 +22,7 @@ SECTIONS
      *(.got2)
      __got2_end = .;

-    _dtb_start = .;
-    *(.kernel:dtb)
-    _dtb_end = .;
+    KERNEL_DTB()

      _vmlinux_start =  .;
      *(.kernel:vmlinux.strip)
diff --git a/arch/powerpc/boot/zImage.lds.S b/arch/powerpc/boot/zImage.lds.S
index 0962d62..f5b9c56 100644
--- a/arch/powerpc/boot/zImage.lds.S
+++ b/arch/powerpc/boot/zImage.lds.S
@@ -1,3 +1,4 @@
+#include <asm-generic/vmlinux.lds.h>
  OUTPUT_ARCH(powerpc:common)
  ENTRY(_zimage_start)
  EXTERN(_zimage_start)
@@ -22,9 +23,7 @@ SECTIONS
    }

    . = ALIGN(8);
-  _dtb_start = .;
-  .kernel:dtb : { *(.kernel:dtb) }
-  _dtb_end = .;
+  KERNEL_DTB()

    . = ALIGN(4096);
    _vmlinux_start =  .;
diff --git a/arch/powerpc/boot/zImage.ps3.lds.S b/arch/powerpc/boot/zImage.ps3.lds.S
index aaa469c..246d227 100644
--- a/arch/powerpc/boot/zImage.ps3.lds.S
+++ b/arch/powerpc/boot/zImage.ps3.lds.S
@@ -1,3 +1,4 @@
+#include <asm-generic/vmlinux.lds.h>
  OUTPUT_ARCH(powerpc:common)
  ENTRY(_zimage_start)
  EXTERN(_zimage_start)
@@ -8,9 +9,7 @@ SECTIONS
    _vmlinux_end =  .;

    . = ALIGN(4096);
-  _dtb_start = .;
-  .kernel:dtb : { *(.kernel:dtb) }
-  _dtb_end = .;
+  KERNEL_DTB()

    . = ALIGN(4096);
    _initrd_start =  .;
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 3068e1e..992b3dd 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -120,6 +120,21 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
  obj-$(CONFIG_SWIOTLB)			+= pci-swiotlb.o
  obj-$(CONFIG_X86_OF)			+= prom.o

+ifeq ($(CONFIG_X86_OF),y)
+ifneq ($(PLATFORM_DTB),)
+obj-y += $(PLATFORM_DTB).dtb.o
+endif
+endif
+
+dtstree	:= $(srctree)/arch/x86/boot/dts
+
+$(obj)/%.dtb: $(dtstree)/%.dts
+	$(call if_changed,dtc)
+
+$(obj)/%.dtb.S: $(obj)/%.dtb
+	@echo '.section .kernel:dtb,"a"' > $@
+	@echo '.incbin "$<" ' >> $@
+
  ###
  # 64 bit specific files
  ifeq ($(CONFIG_X86_64),y)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8a92a17..5729a24 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -146,6 +146,14 @@
  #define TRACE_SYSCALLS()
  #endif

+#ifdef CONFIG_OF
+#define KERNEL_DTB() _dtb_start = .;	    				\
+		   *(.kernel:dtb)					\
+		   _dtb_end = .;					
+#else
+ KERNEL_DTB()
+#endif
+
  /* .data section */
  #define DATA_DATA							\
  	*(.data)							\
@@ -245,6 +253,10 @@
  		VMLINUX_SYMBOL(__end_pci_fixups_suspend) = .;		\
  	}								\
  									\
+	.dtb : AT(ADDR(.dtb) - LOAD_OFFSET) {				\
+	     KERNEL_DTB()      		    				\
+	}		   						\
+								\
  	/* Built-in firmware blobs */					\
  	.builtin_fw        : AT(ADDR(.builtin_fw) - LOAD_OFFSET) {	\
  		VMLINUX_SYMBOL(__start_builtin_fw) = .;			\
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 54fd1b7..ce32644 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -207,6 +207,13 @@ quiet_cmd_gzip = GZIP    $@
  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
  	(rm -f $@ ; false)

+# DTC
+#  ---------------------------------------------------------------------------
+
+DTC = $(objtree)/scripts/dtc/dtc
+
+quiet_cmd_dtc = DTC	$@
+      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 -p 1024 $(dtstree)/$*.dts

  # Bzip2
  # ---------------------------------------------------------------------------

Re: [RFC] [PATCH] Adding DTB to architecture independent vmlinux

[Grant Likely]

On Tue, Oct 26, 2010 at 07:24:17AM -0700, Dirk Brandewie wrote:
> Hi All,
> 
> I am working on working on adding support device tree support to an
> x86 based platform, hpa pointed out that the device tree blob is
> architecture independent and should be part of the generic kernel.
> 
> The patch below is a first cut at adding dtb support to the architecture
> independent kernel and adding a generic dts->dtb build rule.
> 
> This has only been tested on x86.
> 
> Comments/suggestions gratefully accepted.
> 
> --Dirk
> 
> of: add support for linking platform dtb into vmlinux
> 
> From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This patch adds support for linking a device tree blob into
> vmlinux. The platform DTB to be built and linked into the kernel is
> specified by passing PLATFORM_DTB=<platform name> to make.

I don't think it is worth trying to do this in a cross-architecture
way.  The powerpc zImage wrappers are already very platform-specific
code, and there are no assumptions made about how a dtb image is
linked into the wrapper.  A wrapper could even link in multiple dtb
images.


For the x86 code, linking the dtb into the vmlinux itself is not
something that is desired without the option of selecting it between
multiple linked-in dtb files if firmware doesn't pass one in for you.
This will require some discussion about how best to manage dtbs before
merging a patch like this.

> 
> The command:
> make PLATFORM_DTB=ce4100
> 
> will link the device tree blob into vmlinux
> 
> Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/powerpc/boot/zImage.coff.lds.S |    5 ++---
>  arch/powerpc/boot/zImage.lds.S      |    5 ++---
>  arch/powerpc/boot/zImage.ps3.lds.S  |    5 ++---
>  arch/x86/kernel/Makefile            |   15 +++++++++++++++
>  include/asm-generic/vmlinux.lds.h   |   12 ++++++++++++
>  scripts/Makefile.lib                |    7 +++++++
>  7 files changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/boot/zImage.coff.lds.S
> b/arch/powerpc/boot/zImage.coff.lds.S
> index 856dc78..c23145f 100644
> --- a/arch/powerpc/boot/zImage.coff.lds.S
> +++ b/arch/powerpc/boot/zImage.coff.lds.S
> @@ -1,3 +1,4 @@
> +#include <asm-generic/vmlinux.lds.h>
>  OUTPUT_ARCH(powerpc:common)
>  ENTRY(_zimage_start_opd)
>  EXTERN(_zimage_start_opd)
> @@ -21,9 +22,7 @@ SECTIONS
>      *(.got2)
>      __got2_end = .;
> 
> -    _dtb_start = .;
> -    *(.kernel:dtb)
> -    _dtb_end = .;
> +    KERNEL_DTB()

Has this been compile tested?  I don't believe the powerpc boot
wrapper code (arch/powerpc/boot/*) will include stuff from
linux/asm-generic because it is intended to be a completely
independent C environment.

> 
>      _vmlinux_start =  .;
>      *(.kernel:vmlinux.strip)
> diff --git a/arch/powerpc/boot/zImage.lds.S b/arch/powerpc/boot/zImage.lds.S
> index 0962d62..f5b9c56 100644
> --- a/arch/powerpc/boot/zImage.lds.S
> +++ b/arch/powerpc/boot/zImage.lds.S
> @@ -1,3 +1,4 @@
> +#include <asm-generic/vmlinux.lds.h>
>  OUTPUT_ARCH(powerpc:common)
>  ENTRY(_zimage_start)
>  EXTERN(_zimage_start)
> @@ -22,9 +23,7 @@ SECTIONS
>    }
> 
>    . = ALIGN(8);
> -  _dtb_start = .;
> -  .kernel:dtb : { *(.kernel:dtb) }
> -  _dtb_end = .;
> +  KERNEL_DTB()
> 
>    . = ALIGN(4096);
>    _vmlinux_start =  .;
> diff --git a/arch/powerpc/boot/zImage.ps3.lds.S b/arch/powerpc/boot/zImage.ps3.lds.S
> index aaa469c..246d227 100644
> --- a/arch/powerpc/boot/zImage.ps3.lds.S
> +++ b/arch/powerpc/boot/zImage.ps3.lds.S
> @@ -1,3 +1,4 @@
> +#include <asm-generic/vmlinux.lds.h>
>  OUTPUT_ARCH(powerpc:common)
>  ENTRY(_zimage_start)
>  EXTERN(_zimage_start)
> @@ -8,9 +9,7 @@ SECTIONS
>    _vmlinux_end =  .;
> 
>    . = ALIGN(4096);
> -  _dtb_start = .;
> -  .kernel:dtb : { *(.kernel:dtb) }
> -  _dtb_end = .;
> +  KERNEL_DTB()
> 
>    . = ALIGN(4096);
>    _initrd_start =  .;
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 3068e1e..992b3dd 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -120,6 +120,21 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
>  obj-$(CONFIG_SWIOTLB)			+= pci-swiotlb.o
>  obj-$(CONFIG_X86_OF)			+= prom.o
> 
> +ifeq ($(CONFIG_X86_OF),y)
> +ifneq ($(PLATFORM_DTB),)
> +obj-y += $(PLATFORM_DTB).dtb.o
> +endif
> +endif
> +
> +dtstree	:= $(srctree)/arch/x86/boot/dts
> +
> +$(obj)/%.dtb: $(dtstree)/%.dts
> +	$(call if_changed,dtc)
> +
> +$(obj)/%.dtb.S: $(obj)/%.dtb
> +	@echo '.section .kernel:dtb,"a"' > $@
> +	@echo '.incbin "$<" ' >> $@
> +

Hmmm, I wonder if this hunk can be common for all architectures.

>  ###
>  # 64 bit specific files
>  ifeq ($(CONFIG_X86_64),y)
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 8a92a17..5729a24 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -146,6 +146,14 @@
>  #define TRACE_SYSCALLS()
>  #endif
> 
> +#ifdef CONFIG_OF
> +#define KERNEL_DTB() _dtb_start = .;	    				\
> +		   *(.kernel:dtb)					\
> +		   _dtb_end = .;					
> +#else
> + KERNEL_DTB()
> +#endif
> +
>  /* .data section */
>  #define DATA_DATA							\
>  	*(.data)							\
> @@ -245,6 +253,10 @@
>  		VMLINUX_SYMBOL(__end_pci_fixups_suspend) = .;		\
>  	}								\
>  									\
> +	.dtb : AT(ADDR(.dtb) - LOAD_OFFSET) {				\
> +	     KERNEL_DTB()      		    				\
> +	}		   						\
> +								\
>  	/* Built-in firmware blobs */					\
>  	.builtin_fw        : AT(ADDR(.builtin_fw) - LOAD_OFFSET) {	\
>  		VMLINUX_SYMBOL(__start_builtin_fw) = .;			\
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 54fd1b7..ce32644 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -207,6 +207,13 @@ quiet_cmd_gzip = GZIP    $@
>  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
>  	(rm -f $@ ; false)
> 
> +# DTC
> +#  ---------------------------------------------------------------------------
> +
> +DTC = $(objtree)/scripts/dtc/dtc
> +
> +quiet_cmd_dtc = DTC	$@
> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 -p 1024 $(dtstree)/$*.dts
> 
>  # Bzip2
>  # ---------------------------------------------------------------------------
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [sodaville] [RFC] [PATCH] Adding DTB to architecture independent vmlinux

[H. Peter Anvin]

On 10/27/2010 4:09 AM, Grant Likely wrote:
> On Tue, Oct 26, 2010 at 07:24:17AM -0700, Dirk Brandewie wrote:
>> Hi All,
>>
>> I am working on working on adding support device tree support to an
>> x86 based platform, hpa pointed out that the device tree blob is
>> architecture independent and should be part of the generic kernel.
>>
>> The patch below is a first cut at adding dtb support to the architecture
>> independent kernel and adding a generic dts->dtb build rule.
>>
>> This has only been tested on x86.
>>
>> Comments/suggestions gratefully accepted.
>>
>> --Dirk
>>
>> of: add support for linking platform dtb into vmlinux
>>
>> From: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> This patch adds support for linking a device tree blob into
>> vmlinux. The platform DTB to be built and linked into the kernel is
>> specified by passing PLATFORM_DTB=<platform name>  to make.
>
> I don't think it is worth trying to do this in a cross-architecture
> way.  The powerpc zImage wrappers are already very platform-specific
> code, and there are no assumptions made about how a dtb image is
> linked into the wrapper.  A wrapper could even link in multiple dtb
> images.
>
> For the x86 code, linking the dtb into the vmlinux itself is not
> something that is desired without the option of selecting it between
> multiple linked-in dtb files if firmware doesn't pass one in for you.
> This will require some discussion about how best to manage dtbs before
> merging a patch like this.
>

I have to disagree, strongly, that it is not worth doing a 
cross-architectural patch.  There is nothing that prevents an 
architecture from having any number of different ones *in addition to* 
the independent way, but that would clearly establish a baseline that 
everyone can benefit from.  This is a Very Good Thing[TM].  We already 
have too many gratuitous differences between architectures in Linux.

As far as linking in multiple dtbs, the one architecture-independent way 
we have to pass data into the kernel is the kernel command line, and it 
might make sense to have a command line parameter like "dtb=foo", 
although I expect the vast majority of all users who don't rely on the 
firmware/bootloader to feed them information are going to have one 
kernel image per platform anyway, just for space reasons.

	-hpa

[RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[Dirk Brandewie]

Here is V2 of the patch.

The support for linking in the DTB is now a config option. The location
of the config option is completely arbitrary.

The name of the added section has been changed to avoid possible collision
with the powerpc linkage of the DTB into the final image.

ATM this patch would add a redundant/unused section (without name collision) to 
the microblaze if configured in since I do not understand why the padding added 
to _fdt_start.

This has only been tested on x86.

Comments Suggestions?

--Dirk

of: add support for linking platform dtb into vmlinux

From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch adds support for linking a device tree blob into
vmlinux. The platform DTB to be built and linked into the kernel is
specified by passing PLATFORM_DTB=<platform name> to make.

The command:
make PLATFORM_DTB=ce4100

will link the device tree blob into vmlinux

Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
  arch/x86/kernel/Makefile          |   15 +++++++++++++++
  include/asm-generic/vmlinux.lds.h |   15 +++++++++++++++
  init/Kconfig                      |    7 +++++++
  scripts/Makefile.lib              |    7 +++++++
  5 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 3068e1e..0f5eb1d 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -120,6 +120,21 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
  obj-$(CONFIG_SWIOTLB)			+= pci-swiotlb.o
  obj-$(CONFIG_X86_OF)			+= prom.o

+ifeq ($(CONFIG_KERNEL_DTB),y)
+ifneq ($(PLATFORM_DTB),)
+obj-y += $(PLATFORM_DTB).dtb.o
+endif
+endif
+
+dtstree	:= $(srctree)/arch/x86/boot/dts
+
+$(obj)/%.dtb: $(dtstree)/%.dts
+	$(call if_changed,dtc)
+
+$(obj)/%.dtb.S: $(obj)/%.dtb
+	@echo '.section .dtb,"a"' > $@
+	@echo '.incbin "$<" ' >> $@
+
  ###
  # 64 bit specific files
  ifeq ($(CONFIG_X86_64),y)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8a92a17..b18123a 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -146,6 +146,19 @@
  #define TRACE_SYSCALLS()
  #endif

+#ifdef CONFIG_KERNEL_DTB
+#define KERNEL_DTB							\
+	. = ALIGN(4);							\
+	.dtb : AT(ADDR(.dtb) - LOAD_OFFSET) {				\
+		VMLINUX_SYMBOL(__dtb_start) = .;			\
+		*(.dtb)							\
+		VMLINUX_SYMBOL(__dtb_end) = .;				\
+	}
+
+#else
+#define KERNEL_DTB
+#endif
+
  /* .data section */
  #define DATA_DATA							\
  	*(.data)							\
@@ -245,6 +258,8 @@
  		VMLINUX_SYMBOL(__end_pci_fixups_suspend) = .;		\
  	}								\
  									\
+	KERNEL_DTB							\
+									\
  	/* Built-in firmware blobs */					\
  	.builtin_fw        : AT(ADDR(.builtin_fw) - LOAD_OFFSET) {	\
  		VMLINUX_SYMBOL(__start_builtin_fw) = .;			\
diff --git a/init/Kconfig b/init/Kconfig
index 2de5b1c..4a8802e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1055,6 +1055,13 @@ config PCI_QUIRKS
            bugs/quirks. Disable this only if your target machine is
            unaffected by PCI quirks.

+config KERNEL_DTB
+       bool "Support linking a device tree blob into vmlinux"
+       default n
+       help
+         This option provides support for adding a device tree blob
+	 directly to vmlinux
+
  config SLUB_DEBUG
  	default y
  	bool "Enable SLUB debugging support" if EMBEDDED
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 54fd1b7..ce32644 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -207,6 +207,13 @@ quiet_cmd_gzip = GZIP    $@
  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
  	(rm -f $@ ; false)

+# DTC
+#  ---------------------------------------------------------------------------
+
+DTC = $(objtree)/scripts/dtc/dtc
+
+quiet_cmd_dtc = DTC	$@
+      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 -p 1024 $(dtstree)/$*.dts

  # Bzip2
  # ---------------------------------------------------------------------------

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[David VomLehn]

On Wed, Oct 27, 2010 at 05:30:27PM -0700, Dirk Brandewie wrote:
> Here is V2 of the patch.
...
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 3068e1e..0f5eb1d 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -120,6 +120,21 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
>  obj-$(CONFIG_SWIOTLB)			+= pci-swiotlb.o
>  obj-$(CONFIG_X86_OF)			+= prom.o
>
> +ifeq ($(CONFIG_KERNEL_DTB),y)
> +ifneq ($(PLATFORM_DTB),)
> +obj-y += $(PLATFORM_DTB).dtb.o
> +endif
> +endif
> +
> +dtstree	:= $(srctree)/arch/x86/boot/dts
> +
> +$(obj)/%.dtb: $(dtstree)/%.dts
> +	$(call if_changed,dtc)
> +
> +$(obj)/%.dtb.S: $(obj)/%.dtb
> +	@echo '.section .dtb,"a"' > $@
> +	@echo '.incbin "$<" ' >> $@
> +

I've been playing a bit with the patch, and would suggest something
like the following for the second target:

	$(obj)/%.dtb.S: $(obj)/%.dtb
		@echo '#include <asm/page.h>' >$@
		@echo '.balign PAGE_SIZE' >> $@
		@echo '.section .kernel:dtb,"a"' >> $@
		@echo '.global __$(*F)_dtb' >> $@
		@echo '__$(*F)_dtb:' >> $@
		@echo '.incbin "$<" ' >> $@
		@echo '.balign PAGE_SIZE' >> $@

Advantages:
1.	Each blob gets a name that can be used to refer to it. This
	allows multiple blobs to be built into a kernel, each with
	its own name.  The name of each blob is taken from the file
	name, so a source
	file abc.dts would produce a blob referred to as __abc_dtb.
2.	Blobs are aligned on a page boundary and extend to the nearest
	page boundary. Any blobs you don't care about can then easily
	be completely freed.

You might then use:

	obj-y += $(addprefix .dtb.o,$(PLATFORM_DTB))

to add blob names, but I'm not completely confident this is the way to go.
-- 
David VL

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[H. Peter Anvin]

On 10/27/2010 5:57 PM, David VomLehn wrote:
>
> I've been playing a bit with the patch, and would suggest something
> like the following for the second target:
>
> 	$(obj)/%.dtb.S: $(obj)/%.dtb
> 		@echo '#include<asm/page.h>'>$@
> 		@echo '.balign PAGE_SIZE'>>  $@
> 		@echo '.section .kernel:dtb,"a"'>>  $@
> 		@echo '.global __$(*F)_dtb'>>  $@
> 		@echo '__$(*F)_dtb:'>>  $@
> 		@echo '.incbin "$<" '>>  $@
> 		@echo '.balign PAGE_SIZE'>>  $@
>
> Advantages:
> 1.	Each blob gets a name that can be used to refer to it. This
> 	allows multiple blobs to be built into a kernel, each with
> 	its own name.  The name of each blob is taken from the file
> 	name, so a source
> 	file abc.dts would produce a blob referred to as __abc_dtb.
> 2.	Blobs are aligned on a page boundary and extend to the nearest
> 	page boundary. Any blobs you don't care about can then easily
> 	be completely freed.
>
> You might then use:
>
> 	obj-y += $(addprefix .dtb.o,$(PLATFORM_DTB))
>
> to add blob names, but I'm not completely confident this is the way to go.

To be able to specify "dtb=<name>" on the command line, you want the 
name to be manifest in string form, rather than as a symbol.  That means 
putting a header or something similar in front of the blobs.

How big are these blobs in the typical case?  Padding to the page size 
could easily add more waste than it saves.  In that case it probably 
would be better to put the stuff in the init area and copy the active 
blob to an allocated location.

	-hpa

Re: [sodaville] [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[Sebastian Andrzej Siewior]

H. Peter Anvin wrote:
> How big are these blobs in the typical case?  Padding to the page size 
You usually pad the dtb by 1kib (-p 1024 in dtc). Converting all dtbs in
powerpc's tree leads to:
  991545 / 130 = 7627 bytes average.

> 
> 	-hpa

Sebastian

Re: [sodaville] [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[H. Peter Anvin]

On 10/28/2010 9:35 AM, Sebastian Andrzej Siewior wrote:
> H. Peter Anvin wrote:
>> How big are these blobs in the typical case? Padding to the page size
> You usually pad the dtb by 1kib (-p 1024 in dtc). Converting all dtbs in
> powerpc's tree leads to:
> 991545 / 130 = 7627 bytes average.
>

So expanding to a page is a lot of memory, but it's pretty important to 
jettison unused DTBs.

	-hpa

Re: [sodaville] [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[David VomLehn]

On Thu, Oct 28, 2010 at 09:38:29AM -0700, H. Peter Anvin wrote:
> On 10/28/2010 9:35 AM, Sebastian Andrzej Siewior wrote:
>> H. Peter Anvin wrote:
>>> How big are these blobs in the typical case? Padding to the page size
>> You usually pad the dtb by 1kib (-p 1024 in dtc). Converting all dtbs in
>> powerpc's tree leads to:
>> 991545 / 130 = 7627 bytes average.
>>
>
> So expanding to a page is a lot of memory, but it's pretty important to  
> jettison unused DTBs.
>
> 	-hpa

IIRC, the slab allocator will simply use the buddy allocator when allocating
close to or above the page size. So, this will use 8192 bytes whether
you use what I had proposed or you dynamically allocate space and copy it
from the .init section.

If memory usage is not an issue, it would be nice to avoid the copy. Another
possible consideration is image size. If you don't do the rounding, you'll
have a smaller boot image. Not really sure how to weigh these and whether there
are other considerations.
-- 
David VL

Re: [sodaville] [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[Grant Likely]

On Thu, Oct 28, 2010 at 2:00 PM, David VomLehn <dvomlehn-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Oct 28, 2010 at 09:38:29AM -0700, H. Peter Anvin wrote:
>> On 10/28/2010 9:35 AM, Sebastian Andrzej Siewior wrote:
>>> H. Peter Anvin wrote:
>>>> How big are these blobs in the typical case? Padding to the page size
>>> You usually pad the dtb by 1kib (-p 1024 in dtc). Converting all dtbs in
>>> powerpc's tree leads to:
>>> 991545 / 130 = 7627 bytes average.
>>>
>>
>> So expanding to a page is a lot of memory, but it's pretty important to
>> jettison unused DTBs.
>>
>>       -hpa
>
> IIRC, the slab allocator will simply use the buddy allocator when allocating
> close to or above the page size. So, this will use 8192 bytes whether
> you use what I had proposed or you dynamically allocate space and copy it
> from the .init section.
>
> If memory usage is not an issue, it would be nice to avoid the copy. Another
> possible consideration is image size. If you don't do the rounding, you'll
> have a smaller boot image. Not really sure how to weigh these and whether there
> are other considerations.

The choice to copy could be made a Kconfig option, but I doubt it is
warranted.  A one time copy of one or two pages at boot is pretty
speedy.  I'll let someone with benchmarks results prove me otherwise.

g.

Re: [sodaville] [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[David VomLehn]

On Mon, Nov 01, 2010 at 12:15:07AM -0400, Grant Likely wrote:
> On Thu, Oct 28, 2010 at 2:00 PM, David VomLehn <dvomlehn-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> wrote:
> > On Thu, Oct 28, 2010 at 09:38:29AM -0700, H. Peter Anvin wrote:
> >> On 10/28/2010 9:35 AM, Sebastian Andrzej Siewior wrote:
> >>> H. Peter Anvin wrote:
> >>>> How big are these blobs in the typical case? Padding to the page size
> >>> You usually pad the dtb by 1kib (-p 1024 in dtc). Converting all dtbs in
> >>> powerpc's tree leads to:
> >>> 991545 / 130 = 7627 bytes average.
> >>>
> >>
> >> So expanding to a page is a lot of memory, but it's pretty important to
> >> jettison unused DTBs.
> >>
> >>       -hpa
> >
> > IIRC, the slab allocator will simply use the buddy allocator when allocating
> > close to or above the page size. So, this will use 8192 bytes whether
> > you use what I had proposed or you dynamically allocate space and copy it
> > from the .init section.
> >
> > If memory usage is not an issue, it would be nice to avoid the copy. Another
> > possible consideration is image size. If you don't do the rounding, you'll
> > have a smaller boot image. Not really sure how to weigh these and whether there
> > are other considerations.
> 
> The choice to copy could be made a Kconfig option, but I doubt it is
> warranted.  A one time copy of one or two pages at boot is pretty
> speedy.  I'll let someone with benchmarks results prove me otherwise.
> 
> g.

It wouldn't surprise me if the time to copy weren't made up in one place or another
by the smaller size. I'm happy with .init and a memcpy(). :-)
-- 
David VL

Re: [sodaville] [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[David Gibson]

On Thu, Oct 28, 2010 at 06:35:55PM +0200, Sebastian Andrzej Siewior wrote:
> H. Peter Anvin wrote:
> >How big are these blobs in the typical case?  Padding to the page
> >size
> You usually pad the dtb by 1kib (-p 1024 in dtc). Converting all dtbs in
> powerpc's tree leads to:
>  991545 / 130 = 7627 bytes average.

If you're folding a whole batch of dtbs into the kernel, doing the
1kib padding on each is probably not sensible.  The padding is so that
tweaks to the dtb which require expansion can be done easily, but if
you're relocating the dtb anyway, you can allocate more space at that
time, with fdt_open_into().

That would bring the average size down to 6603 bytes (-p 1024 is "1024
bytes of extra space", not "expand to a 1024 byte boundary").

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[David VomLehn]

On Thu, Oct 28, 2010 at 08:18:25AM -0700, H. Peter Anvin wrote:
> On 10/27/2010 5:57 PM, David VomLehn wrote:
>>
>> I've been playing a bit with the patch, and would suggest something
>> like the following for the second target:
>>
>> 	$(obj)/%.dtb.S: $(obj)/%.dtb
>> 		@echo '#include<asm/page.h>'>$@
>> 		@echo '.balign PAGE_SIZE'>>  $@
>> 		@echo '.section .kernel:dtb,"a"'>>  $@
>> 		@echo '.global __$(*F)_dtb'>>  $@
>> 		@echo '__$(*F)_dtb:'>>  $@
>> 		@echo '.incbin "$<" '>>  $@
>> 		@echo '.balign PAGE_SIZE'>>  $@
>>
>> Advantages:
>> 1.	Each blob gets a name that can be used to refer to it. This
>> 	allows multiple blobs to be built into a kernel, each with
>> 	its own name.  The name of each blob is taken from the file
>> 	name, so a source
>> 	file abc.dts would produce a blob referred to as __abc_dtb.
>> 2.	Blobs are aligned on a page boundary and extend to the nearest
>> 	page boundary. Any blobs you don't care about can then easily
>> 	be completely freed.
>>
>> You might then use:
>>
>> 	obj-y += $(addprefix .dtb.o,$(PLATFORM_DTB))
>>
>> to add blob names, but I'm not completely confident this is the way to go.
>
> To be able to specify "dtb=<name>" on the command line, you want the  
> name to be manifest in string form, rather than as a symbol.  That means  
> putting a header or something similar in front of the blobs.

Could you not iterate over the blobs and use the "compatible" property to
match the string? It would seem to me that this would be the right thing
to do in any case since it would then follow the standard method for
interpreting that property.

> How big are these blobs in the typical case?  Padding to the page size  
> could easily add more waste than it saves.  In that case it probably  
> would be better to put the stuff in the init area and copy the active  
> blob to an allocated location.

In my case, where there are a lot of up-front reservations of memory
at a static address, there is a fair amount of work to do before
it's possible to do the dynamic address allocation for the blob's
ultimate destination, but I'm okay with either doing blob size and address
rounding or copying it from init. I don't see a benefit of supporting
multiple approaches for Linux.

> 	-hpa

-- 
David VL

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[H. Peter Anvin]

On 10/28/2010 10:32 AM, David VomLehn wrote:
> 
> In my case, where there are a lot of up-front reservations of memory
> at a static address, there is a fair amount of work to do before
> it's possible to do the dynamic address allocation for the blob's
> ultimate destination, but I'm okay with either doing blob size and address
> rounding or copying it from init. I don't see a benefit of supporting
> multiple approaches for Linux.
> 

The reason I mentioned dynamic allocation is that you still need to do
dynamic allocation if you're using a blob from an external source.  If
you don't end up with pointers *into* the blob, though, then you don't
need to do the relocation until some time before you jettison the init
section.

However, I don't have a strong opinion about which way it's done.

	-hpa

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[David Gibson]

On Thu, Oct 28, 2010 at 02:44:47PM -0700, H. Peter Anvin wrote:
> On 10/28/2010 10:32 AM, David VomLehn wrote:
> > 
> > In my case, where there are a lot of up-front reservations of memory
> > at a static address, there is a fair amount of work to do before
> > it's possible to do the dynamic address allocation for the blob's
> > ultimate destination, but I'm okay with either doing blob size and address
> > rounding or copying it from init. I don't see a benefit of supporting
> > multiple approaches for Linux.
> > 
> 
> The reason I mentioned dynamic allocation is that you still need to do
> dynamic allocation if you're using a blob from an external source.  If
> you don't end up with pointers *into* the blob, though, then you don't
> need to do the relocation until some time before you jettison the init
> section.

The way the dtb format is designed, you should almost never work with
pointers into the the middle of the blob.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[H. Peter Anvin]

On 10/28/2010 09:04 PM, David Gibson wrote:
> On Thu, Oct 28, 2010 at 02:44:47PM -0700, H. Peter Anvin wrote:
>> On 10/28/2010 10:32 AM, David VomLehn wrote:
>>>
>>> In my case, where there are a lot of up-front reservations of memory
>>> at a static address, there is a fair amount of work to do before
>>> it's possible to do the dynamic address allocation for the blob's
>>> ultimate destination, but I'm okay with either doing blob size and address
>>> rounding or copying it from init. I don't see a benefit of supporting
>>> multiple approaches for Linux.
>>>
>>
>> The reason I mentioned dynamic allocation is that you still need to do
>> dynamic allocation if you're using a blob from an external source.  If
>> you don't end up with pointers *into* the blob, though, then you don't
>> need to do the relocation until some time before you jettison the init
>> section.
> 
> The way the dtb format is designed, you should almost never work with
> pointers into the the middle of the blob.
> 

"Almost never"?  That scares me when I hear it...

	-hpa

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[David Gibson]

On Fri, Oct 29, 2010 at 01:29:07PM -0700, H. Peter Anvin wrote:
> On 10/28/2010 09:04 PM, David Gibson wrote:
> > On Thu, Oct 28, 2010 at 02:44:47PM -0700, H. Peter Anvin wrote:
> >> On 10/28/2010 10:32 AM, David VomLehn wrote:
> >>>
> >>> In my case, where there are a lot of up-front reservations of memory
> >>> at a static address, there is a fair amount of work to do before
> >>> it's possible to do the dynamic address allocation for the blob's
> >>> ultimate destination, but I'm okay with either doing blob size and address
> >>> rounding or copying it from init. I don't see a benefit of supporting
> >>> multiple approaches for Linux.
> >>>
> >>
> >> The reason I mentioned dynamic allocation is that you still need to do
> >> dynamic allocation if you're using a blob from an external source.  If
> >> you don't end up with pointers *into* the blob, though, then you don't
> >> need to do the relocation until some time before you jettison the init
> >> section.
> > 
> > The way the dtb format is designed, you should almost never work with
> > pointers into the the middle of the blob.
> 
> "Almost never"?  That scares me when I hear it...

Heh, understandable.  So, the dtb format is such that you want to work
with internal offsets most of the time, not pointers into the middle.
Except that of course, you will use transient internal pointers to
grab pieces from the dtb - it's just best to avoid passing them around
whenever you can.

The other case is that once the dtb is "frozen" (no more changes), it
can be useful to keep around pointers to bits of data within the dtb,
rather than duplicating each such thing in its own allocated buffer
somewhere.  e.g. I think once we unflatten the tree in powerpc,
property values will still actually point to within the original dtb.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[Grant Likely]

On Thu, Oct 28, 2010 at 1:32 PM, David VomLehn <dvomlehn-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Oct 28, 2010 at 08:18:25AM -0700, H. Peter Anvin wrote:
>> On 10/27/2010 5:57 PM, David VomLehn wrote:
>>>
>>> I've been playing a bit with the patch, and would suggest something
>>> like the following for the second target:
>>>
>>>      $(obj)/%.dtb.S: $(obj)/%.dtb
>>>              @echo '#include<asm/page.h>'>$@
>>>              @echo '.balign PAGE_SIZE'>>  $@
>>>              @echo '.section .kernel:dtb,"a"'>>  $@
>>>              @echo '.global __$(*F)_dtb'>>  $@
>>>              @echo '__$(*F)_dtb:'>>  $@
>>>              @echo '.incbin "$<" '>>  $@
>>>              @echo '.balign PAGE_SIZE'>>  $@
>>>
>>> Advantages:
>>> 1.   Each blob gets a name that can be used to refer to it. This
>>>      allows multiple blobs to be built into a kernel, each with
>>>      its own name.  The name of each blob is taken from the file
>>>      name, so a source
>>>      file abc.dts would produce a blob referred to as __abc_dtb.
>>> 2.   Blobs are aligned on a page boundary and extend to the nearest
>>>      page boundary. Any blobs you don't care about can then easily
>>>      be completely freed.
>>>
>>> You might then use:
>>>
>>>      obj-y += $(addprefix .dtb.o,$(PLATFORM_DTB))
>>>
>>> to add blob names, but I'm not completely confident this is the way to go.
>>
>> To be able to specify "dtb=<name>" on the command line, you want the
>> name to be manifest in string form, rather than as a symbol.  That means
>> putting a header or something similar in front of the blobs.
>
> Could you not iterate over the blobs and use the "compatible" property to
> match the string? It would seem to me that this would be the right thing
> to do in any case since it would then follow the standard method for
> interpreting that property.

Hmmm, I had mistakenly thought that dtb=<blah> was proposed to pass in
the address of a blob already loaded into memory (perhaps by grub),
but I understand now.  Yes, it could be performed by name.  However,
David's suggestion of passing in a compatible string better matches
existing conventions.

>> How big are these blobs in the typical case?  Padding to the page size
>> could easily add more waste than it saves.  In that case it probably
>> would be better to put the stuff in the init area and copy the active
>> blob to an allocated location.

4-8k typically.  They could also be compressed, but that doesn't gain
a whole lot and it would complicate the process of scanning the
compatible values as suggested above.

> In my case, where there are a lot of up-front reservations of memory
> at a static address, there is a fair amount of work to do before
> it's possible to do the dynamic address allocation for the blob's
> ultimate destination, but I'm okay with either doing blob size and address
> rounding or copying it from init. I don't see a benefit of supporting
> multiple approaches for Linux.

It is already possible to do allocation early allocation in both
powerpc and arm.  Also, blob relocation can be deferred until later in
boot if need be.  The flat tree can be accessed before relocating it.
Just as long as it is done so before unflattening the tree (possibly
as part of).

Regardless, Peter is absolutely right; the linked in .dtbs definitely
must be in an init section so they can be discarded after copy.

g.

Re: [sodaville] [RFC] [PATCH] Adding DTB to architecture independent vmlinux

[Grant Likely]

On Wed, Oct 27, 2010 at 1:27 PM, H. Peter Anvin <hpa-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On 10/27/2010 4:09 AM, Grant Likely wrote:
[...]
>> I don't think it is worth trying to do this in a cross-architecture
>> way.  The powerpc zImage wrappers are already very platform-specific
>> code, and there are no assumptions made about how a dtb image is
>> linked into the wrapper.  A wrapper could even link in multiple dtb
>> images.
>>
>> For the x86 code, linking the dtb into the vmlinux itself is not
>> something that is desired without the option of selecting it between
>> multiple linked-in dtb files if firmware doesn't pass one in for you.
>> This will require some discussion about how best to manage dtbs before
>> merging a patch like this.
>>
>
> I have to disagree, strongly, that it is not worth doing a
> cross-architectural patch.  There is nothing that prevents an architecture
> from having any number of different ones *in addition to* the independent
> way, but that would clearly establish a baseline that everyone can benefit
> from.  This is a Very Good Thing[TM].  We already have too many gratuitous
> differences between architectures in Linux.

The problem in this case is that it turns into a magic config option
that explicitly restricts the number of platforms a kernel image will
run on down to exactly one.  This (particular) approach is 100%
counter to the direction I've been trying to take embedded platforms
over the last few years.  I'm opposed to linking a single .dtb as a
'baseline' image because embedded system developers will start using
and depending on that feature instead if implementing something that
is multi-platform friendly.

That said, I'm not at all opposed to common infrastructure for linking
one or more .dtb images into the kernel provided that each image is
referred to by a separate symbol, and that it does not implement any
kind of baseline semantics.  Selection of the correct .dtb image from
platform code should always be explicit; not implicit nor controlled
by a single Kconfig setting.

So, if a patch is floated that provides linking multiple and arbitrary
.dtb images into the kernel for use by other code, then I'm all for
it.  The problem of selecting /which/ dtb to use can be addressed (and
debated) in a separate patch.

> As far as linking in multiple dtbs, the one architecture-independent way we
> have to pass data into the kernel is the kernel command line, and it might
> make sense to have a command line parameter like "dtb=foo",

Patches have been floated before to pass the .dtb address via the
kernel command line, but they've not been pursued to completion.  I
have no problems with accepting cross-architecture patches to
implement a dtb= kernel parameter, with the caveat that it wouldn't be
applicable when the kernel command line is passed via the .dtb.  :-)

> although I
> expect the vast majority of all users who don't rely on the
> firmware/bootloader to feed them information are going to have one kernel
> image per platform anyway, just for space reasons.

Not really true.  Currently on ARM many SoC family support multiple
boards on a single kernel image with firmware only supplying a unique
integer (machine id) to identify the board.  See
arch/arm/include/asm/mach/arch.h, grep for both MACHINE_START and
lookup_machine_type in arch/arm/mach-*.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.