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

[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: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[Dirk Brandewie]

Hi David,

Thanks for looking at the patch.

On 10/27/2010 05:57 PM, David VomLehn wrote:
> 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.
>

I like this idea in general but I have to admit I don't completely understand
all the implications of adding multiple blobs.  I can see a use case where a
vendor would want to make a single static image that supported multiple
platforms.  I am not sure how common this would be.  One issue I see with
linking multiple DTB's into the image is now the platform init code is back to
trying to detect which hardware it is running on to select the correct blob to
get its configuration from.

As part of the complete DTB story on x86 I am adding the ability to pass the DTB
in from the boot loader.  I think this mechanism would be the more common case
for supporting multiple platforms with a single vmlinux image.


> 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.
I am still trying to settle on the right way to specify the DTB to be linked in
that won't generate a lot of hate from the build/distribution people where they
are supporting multiple platforms with/without the need of the DTB.

--Dirk

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 11:26 AM, Dirk Brandewie
<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 10/27/2010 05:57 PM, David VomLehn wrote:
[...]
>> 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.
>
> I am still trying to settle on the right way to specify the DTB to be linked
> in
> that won't generate a lot of hate from the build/distribution people where
> they
> are supporting multiple platforms with/without the need of the DTB.

Take a look at the arch/powerpc/boot/Makefile where multiple images
are produced depending on the build target.  There are cuImage.%
targets which link in a different .dtb file based on the % part of the
target.  That allows images for multiple machines to be build from a
single config, rather than depending on a magic setting in the .config
file.

g.

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

[Grant Likely]

On Thu, Oct 28, 2010 at 11:26 AM, Dirk Brandewie
<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi David,
>
> Thanks for looking at the patch.
>
> On 10/27/2010 05:57 PM, David VomLehn wrote:
>>
>> 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.
>>
>
> I like this idea in general but I have to admit I don't completely
> understand
> all the implications of adding multiple blobs.  I can see a use case where a
> vendor would want to make a single static image that supported multiple
> platforms.  I am not sure how common this would be.

It is quite common.  ARM currently supports this.

>  One issue I see with
> linking multiple DTB's into the image is now the platform init code is back
> to
> trying to detect which hardware it is running on to select the correct blob
> to
> get its configuration from.

This is the bit that is architecture specific.  In arm and in powerpc,
each platform or each platform family is recorded in the machine_desc
table (the structure of the entries is completely different for each
arch).  Does x86 have any such mechanism for differentiating between
machines?

g.

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: [RFC] [PATCH V2] Adding DTB to architecture independent vmlinux

[Grant Likely]

Hi Dirk,

As promised when we talked at plumbers, here is a method that you can
use to bind multiple .dtbs into the kernel and to have symbols
pointing to the beginning of each of them.

Take a look at arch/arm/boot/compressed/piggy.*.S

You'll see this:

        .section .piggydata,#alloc
        .globl  input_data
input_data:
        .incbin "arch/arm/boot/compressed/piggy.gzip"
        .globl  input_data_end
input_data_end:


Which is used to link in a raw binary (compressed vmlinux) and
reference it with the input_data label.

A similar thing could be used for .dtbs with some script and
makefile-fu.  If you added targets for .dts--->.dtb and .dtb-->.S,
where the .dtb-->.S target generates a .S file that looks something
like this:

        .section .init.dtbdata
        .globl dtb_init_<dtb-filename>
dtb_init_<dtb-filename>:
        .incbin "<full-path-to-dtb-filename>"

Then you'd need to add pre and post linker sections with labels that
wraps the entire region of dtbs so that can be found & parsed, and so
that they can be freed.  The individual .dtbs can then be referenced
using the dtb_init_<dtb-filename> symbol.

Make sense?

g.


On Thu, Oct 28, 2010 at 9:26 AM, Dirk Brandewie
<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi David,
>
> Thanks for looking at the patch.
>
> On 10/27/2010 05:57 PM, David VomLehn wrote:
>>
>> 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.
>>
>
> I like this idea in general but I have to admit I don't completely
> understand
> all the implications of adding multiple blobs.  I can see a use case where a
> vendor would want to make a single static image that supported multiple
> platforms.  I am not sure how common this would be.  One issue I see with
> linking multiple DTB's into the image is now the platform init code is back
> to
> trying to detect which hardware it is running on to select the correct blob
> to
> get its configuration from.
>
> As part of the complete DTB story on x86 I am adding the ability to pass the
> DTB
> in from the boot loader.  I think this mechanism would be the more common
> case
> for supporting multiple platforms with a single vmlinux image.
>
>
>> 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.
>
> I am still trying to settle on the right way to specify the DTB to be linked
> in
> that won't generate a lot of hate from the build/distribution people where
> they
> are supporting multiple platforms with/without the need of the DTB.
>
> --Dirk
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>



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