diff mbox

[0/3] make XIP kernel .data compressed in ROM

Message ID SG2PR06MB1165EDDA573890B891E542CB8A9E0@SG2PR06MB1165.apcprd06.prod.outlook.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Brandt Aug. 28, 2017, 5:40 p.m. UTC
On Friday, August 25, 2017 1, Nicolas Pitre wrote:
> This patch series provides the ability to store the kernel .data
> segment compressed in ROM. It has to be copied to RAM anyway so
> storing it uncompressed is arguably a waste of ROM resources.
> 
> While at it, the copying of .data (when not compressed) and the
> clearing of .bss is performed using optimized string routines rather
> than doing it one word at a time. And throw in small linker script
> cleanups for good measure.
> 

I like the idea, but unfortunately it won't build for my RZ/A1 
(CONFIG_ARCH_R7S72100=y) XIP system. It's a Cortex-A9 with MMU (meaning
you have to hack the Kconfig because apparently allowing
CONFIG_ARCH_MULTIPLATFORM=y and CONFIG_XIP_KERNEL=y in the same build
is considered a ridiculous thing to do).


  LD      vmlinux.o
  MODPOST vmlinux.o
  KSYM    .tmp_kallsyms1.o
  KSYM    .tmp_kallsyms2.o
  LD      vmlinux
  SORTEX  vmlinux
  SYSMAP  System.map
  XIPZ    arch/arm/boot/xipImage
data segment doesn't match end of xipImage
../arch/arm/boot/Makefile:46: recipe for target 'arch/arm/boot/xipImage' failed
make[2]: *** [arch/arm/boot/xipImage] Error 1
arch/arm/Makefile:334: recipe for target 'xipImage' failed
make[1]: *** [xipImage] Error 2
make[1]: Leaving directory '/home/renesas/tools/upstream/renesas-drivers/.out'
Makefile:145: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2


I admit, I did not do any debug yet to find out why.
But if you want to see it, you can simply apply this patch below and 
then build the shmobile_defconfig (after you go in and enable XIP_KERNEL=y 
and XIP_DEFLATED_DATA=y of course).


---------------------------



Chris

Comments

Nicolas Pitre Aug. 29, 2017, 7:07 p.m. UTC | #1
On Mon, 28 Aug 2017, Chris Brandt wrote:

> On Friday, August 25, 2017 1, Nicolas Pitre wrote:
> > This patch series provides the ability to store the kernel .data
> > segment compressed in ROM. It has to be copied to RAM anyway so
> > storing it uncompressed is arguably a waste of ROM resources.
> > 
> > While at it, the copying of .data (when not compressed) and the
> > clearing of .bss is performed using optimized string routines rather
> > than doing it one word at a time. And throw in small linker script
> > cleanups for good measure.
> > 
> 
> I like the idea, but unfortunately it won't build for my RZ/A1 
> (CONFIG_ARCH_R7S72100=y) XIP system. It's a Cortex-A9 with MMU (meaning
> you have to hack the Kconfig because apparently allowing
> CONFIG_ARCH_MULTIPLATFORM=y and CONFIG_XIP_KERNEL=y in the same build
> is considered a ridiculous thing to do).

If you want XIP, it is likely that your kernel won't have the same 
physical address across different platforms. It is also likely that the 
RAM offset will be different across different platforms. And the very 
nature of XIP means the kernel cannot be be self-modified to adjust 
those discrepencies like it does in the multiplatform case. And Flash is 
a precious resource so why would you waste it with platform code that 
you'll never execute?

If the Kconfig language could let you confirm that only one platform is 
selected then the XIP config option could be offered only in that case.  
Maybe something like:

config PLATFORM_FOO
	bool "blah"
	select MANY_PLATFORMS if ONE_PLATFORM_SELECTED
	select ONE_PLATFORM_SELECTED

config XIP_KERNEL
	bool "blah"
	depends on !MANY_PLATFORMS

But I don't think the above works and that's unfortunate. For having 
spent some time in the kconfig code a while ago, it shouldn't be hard to 
implement something like incrementable variables but that's not in my 
short term plans.

>   LD      vmlinux
>   SORTEX  vmlinux
>   SYSMAP  System.map
>   XIPZ    arch/arm/boot/xipImage
> data segment doesn't match end of xipImage

OK......... I now know why.

Executive summary: our linker script is a complete mess. In your case 
that would be partly solved by the patch that Arnd sent on Jul 28th 2017 
titled "ARM: move __bug_table into .data for XIP_KERNEL". However the 
XIP linker script is still broken for similar reasons.

I've done a first pass at fixing the worst of it and I'll post a new 
series soon.

@Arnd: please don't send that patch upstream. I have a better fix for 
it now.


Nicolas
Chris Brandt Aug. 29, 2017, 7:58 p.m. UTC | #2
On Tuesday, August 29, 2017, Nicolas Pitre wrote:
> On Mon, 28 Aug 2017, Chris Brandt wrote:
> 
> > On Friday, August 25, 2017 1, Nicolas Pitre wrote:
> > > This patch series provides the ability to store the kernel .data
> > > segment compressed in ROM. It has to be copied to RAM anyway so
> > > storing it uncompressed is arguably a waste of ROM resources.
> > >
> > > While at it, the copying of .data (when not compressed) and the
> > > clearing of .bss is performed using optimized string routines rather
> > > than doing it one word at a time. And throw in small linker script
> > > cleanups for good measure.
> > >
> >
> > I like the idea, but unfortunately it won't build for my RZ/A1
> > (CONFIG_ARCH_R7S72100=y) XIP system. It's a Cortex-A9 with MMU (meaning
> > you have to hack the Kconfig because apparently allowing
> > CONFIG_ARCH_MULTIPLATFORM=y and CONFIG_XIP_KERNEL=y in the same build
> > is considered a ridiculous thing to do).
> 
> If you want XIP, it is likely that your kernel won't have the same
> physical address across different platforms. It is also likely that the
> RAM offset will be different across different platforms. And the very
> nature of XIP means the kernel cannot be be self-modified to adjust
> those discrepencies like it does in the multiplatform case. And Flash is
> a precious resource so why would you waste it with platform code that
> you'll never execute?

It's not really that I plan on running an XIP_KERNEL on multiple 
different images, it's that if you look at an MMU based system, you have no 
choice but to build it as a ARCH_MULTIPLATFORM in the current Kconfig 
structure.
Of course for my .config, I go in and uncheck every other CPU and driver
that has nothing to do with my system which is why my XIP KERNEL binary
is only 4MB which include all static drivers built in.



> If the Kconfig language could let you confirm that only one platform is
> selected then the XIP config option could be offered only in that case.
> Maybe something like:
> 
> config PLATFORM_FOO
> 	bool "blah"
> 	select MANY_PLATFORMS if ONE_PLATFORM_SELECTED
> 	select ONE_PLATFORM_SELECTED
> 
> config XIP_KERNEL
> 	bool "blah"
> 	depends on !MANY_PLATFORMS


Yes, that's the argument. But, I've tried many different ways with the 
current Kconfig system, and submitted many different patches over the 
last year and a half and not everyone is ever completely happy...so I'm 
stuck with hacking the 2 lines in the Kconfig.
But, that's not what this thread is about.


> But I don't think the above works and that's unfortunate. For having
> spent some time in the kconfig code a while ago, it shouldn't be hard to
> implement something like incrementable variables but that's not in my
> short term plans.

The only way that would seem to make people happy is to make some new 
'Kconfig type', but that's a bit more involved than just editing some 
Kconfig files.


> >   LD      vmlinux
> >   SORTEX  vmlinux
> >   SYSMAP  System.map
> >   XIPZ    arch/arm/boot/xipImage
> > data segment doesn't match end of xipImage
> 
> OK......... I now know why.
> 
> Executive summary: our linker script is a complete mess. In your case
> that would be partly solved by the patch that Arnd sent on Jul 28th 2017
> titled "ARM: move __bug_table into .data for XIP_KERNEL". However the
> XIP linker script is still broken for similar reasons.

Ya, I thought that would happen eventually. The only way I could get 
agreement to fix existing broken XIP_KERNEL stuff was to split the linker 
script into XIP and non-XIP. But, trying to follow every vmlinux.lds.S 
change and checking to see if it is needed for vmlinux-xip.lds.S is a bit 
daunting.


Chris
Nicolas Pitre Aug. 29, 2017, 8:04 p.m. UTC | #3
On Tue, 29 Aug 2017, Chris Brandt wrote:

> On Tuesday, August 29, 2017, Nicolas Pitre wrote:
> > Executive summary: our linker script is a complete mess. In your case
> > that would be partly solved by the patch that Arnd sent on Jul 28th 2017
> > titled "ARM: move __bug_table into .data for XIP_KERNEL". However the
> > XIP linker script is still broken for similar reasons.
> 
> Ya, I thought that would happen eventually. The only way I could get 
> agreement to fix existing broken XIP_KERNEL stuff was to split the linker 
> script into XIP and non-XIP. But, trying to follow every vmlinux.lds.S 
> change and checking to see if it is needed for vmlinux-xip.lds.S is a bit 
> daunting.

It is. And even in the non-XIP case with regards to changes in 
include/asm-generic/vmlinux.lds.h too.


Nicolas
Russell King (Oracle) Aug. 29, 2017, 9:50 p.m. UTC | #4
On Tue, Aug 29, 2017 at 03:07:09PM -0400, Nicolas Pitre wrote:
> @Arnd: please don't send that patch upstream. I have a better fix for 
> it now.

It's not really up to Arnd to send it to Linus, it's something that
should come through me...
Nicolas Pitre Aug. 29, 2017, 9:52 p.m. UTC | #5
On Tue, 29 Aug 2017, Russell King - ARM Linux wrote:

> On Tue, Aug 29, 2017 at 03:07:09PM -0400, Nicolas Pitre wrote:
> > @Arnd: please don't send that patch upstream. I have a better fix for 
> > it now.
> 
> It's not really up to Arnd to send it to Linus, it's something that
> should come through me...

By upstream I did mean you.


Nicolas
diff mbox

Patch

diff --git a/arch/arm/Kconfigb/arch/arm/Kconfig
index 80191e93f09b..eeb4aa37e8e9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -338,7 +338,7 @@  config ARCH_MULTIPLATFORM
 	bool "Allow multiple platforms to be selected"
 	depends on MMU
 	select ARM_HAS_SG_CHAIN
-	select ARM_PATCH_PHYS_VIRT
+	select ARM_PATCH_PHYS_VIRT if !XIP_KERNEL
 	select AUTO_ZRELADDR
 	select TIMER_OF
 	select COMMON_CLK
@@ -1977,7 +1977,7 @@  endchoice
 
 config XIP_KERNEL
 	bool "Kernel Execute-In-Place from ROM"
-	depends on !ARM_LPAE && !ARCH_MULTIPLATFORM
+	depends on !ARM_LPAE
 	help
 	  Execute-In-Place allows the kernel to run from non-volatile storage
 	  directly addressable by the CPU, such as NOR flash. This saves RAM