| Message ID | HK2PR06MB056196B2E169A504A16ED9738A1D0@HK2PR06MB0561.apcprd06.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 17 Nov 2015, Chris Brandt wrote: > > The old method with conditional placement in the linker script is > > broken beyond repair. There is simply way too much stuff these days > > to do this sanely. I think the only way to fix it properly is to > > have a separate linker script for XIP usage with proper (and obvious) > > section placement. > > > > Step 1: Change the Makefiles to select an alternative linker script. > > What do you think about doing the following? That's fine. Obviously this is the first step when developing this, but logically this will have to be the last patch in the series when you're done. > > > diff --git a/Makefile b/Makefile > index fd46821..956ad21 100644 > --- a/Makefile > +++ b/Makefile > @@ -898,7 +898,11 @@ libs-y := $(libs-y1) $(libs-y2) > # Externally visible symbols (used by link-vmlinux.sh) > export KBUILD_VMLINUX_INIT := $(head-y) $(init-y) > export KBUILD_VMLINUX_MAIN := $(core-y) $(libs-y) $(drivers-y) $(net-y) > +ifdef CONFIG_XIP_KERNEL > +export KBUILD_LDS := arch/$(SRCARCH)/kernel/vmlinux-xip.lds > +else > export KBUILD_LDS := arch/$(SRCARCH)/kernel/vmlinux.lds > +endif > export LDFLAGS_vmlinux > # used by scripts/pacmage/Makefile > export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Documentation include samples scripts tools virt) > > > > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile > index af9e59b..d97fc2e 100644 > --- a/arch/arm/kernel/Makefile > +++ b/arch/arm/kernel/Makefile > @@ -3,6 +3,7 @@ > # > > CPPFLAGS_vmlinux.lds := -DTEXT_OFFSET=$(TEXT_OFFSET) > +CPPFLAGS_vmlinux-xip.lds := -DTEXT_OFFSET=$(TEXT_OFFSET) > AFLAGS_head.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > ifdef CONFIG_FUNCTION_TRACER > @@ -92,4 +93,8 @@ obj-y += psci-call.o > obj-$(CONFIG_SMP) += psci_smp.o > endif > > +ifdef CONFIG_XIP_KERNEL > +extra-y := $(head-y) vmlinux-xip.lds > +else > extra-y := $(head-y) vmlinux.lds > +endif > > > > > > > Chris > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >
So after looking back in history, I think I now understand how/when the XIP stuff broke. It looks like originally, the init section came first, followed by text and data. This is why all the XIP code used _etext as its marker for the end of what needed to be mapped. Like you said, the initialized data values get copied from ROM to RAM early in boot so it's not an issue if the data section (that comes after text) gets blown away when the full MMU setup is done. However, after the init section was moved from the first section to almost the last, all the _etext references scatter through the kernel were never updated. So basically, even though I've now got a separate linker script for XIP builds, it really hasn't changed much because the real issue all along was that the XIP_KERNEL references to _etext should have been changed to _data_loc. With that said, I still think the correct way to fix this issue is to export _data_loc and use that as the end marker. (notice I changed my mind and now say _data_loc instead of _edata_loc because there's no reason to map the full data section anymore). The other option, if we really wanted to keep the references to _etext as they are, is to simply move the one line " _etext = .;" to the bottom of the file....right before _data_loc is declared. But...that would be a bit misleading if you ask me. It sounds like there would still be a request to split up the linker scripts in order to remove the 6 XIP_KERNEL references scattered throughout vmlinux.lds.S. However, it would still be nice to keep the two versions looking as close as they can to each other with only the obvious changes (like removing XIP_KERNEL from the non-XIP version, and removing SMP_ON_UP from the XIP version) Thoughts? Chris
On Wed, 18 Nov 2015, Chris Brandt wrote: > It sounds like there would still be a request to split up the linker > scripts in order to remove the 6 XIP_KERNEL references scattered > throughout vmlinux.lds.S. However, it would still be nice to keep the > two versions looking as close as they can to each other with only the > obvious changes (like removing XIP_KERNEL from the non-XIP version, > and removing SMP_ON_UP from the XIP version) I don't think so. XIP and non-XIP are fundamentally looking different these days. Trying to keep them as similar as possible is rather an impediment to both cases and brings no real benefits. Not only should .init.smpalt be gone from the XIP version, but it might be safer to put that into a special "should_be_empty" section and bail out with ASSERT() if that section size isn't zero. Same thing for pv_table. This way we'll be sure that nothing in the kernel config expects self-modified code to be present. With XIP there is still an init section, but it is pointless to store .init.text stuff in there. That should instead stay in ROM and never be discarded (obviously). No need to page align it either at that point which should save some ROM space. Things like .kprobes.text should be in ROM, but the entire executable in ROM should be surrounded by __kprobes_text_start and __kprobes_text_end to prevent any attempt to put kprobes there. This way, kprobes could still work for modules. Etc. Etc. There are simply too many things these days that require special considerations with XIP. And freeing the main script from XIP considerations will ease maintenance as well, more than the added maintenance from the duplicated parts. Nicolas
On Tuesday 17 November 2015 22:17:41 Nicolas Pitre wrote: > On Wed, 18 Nov 2015, Chris Brandt wrote: > > > It sounds like there would still be a request to split up the linker > > scripts in order to remove the 6 XIP_KERNEL references scattered > > throughout vmlinux.lds.S. However, it would still be nice to keep the > > two versions looking as close as they can to each other with only the > > obvious changes (like removing XIP_KERNEL from the non-XIP version, > > and removing SMP_ON_UP from the XIP version) > > I don't think so. XIP and non-XIP are fundamentally looking different > these days. Trying to keep them as similar as possible is rather an > impediment to both cases and brings no real benefits. > > Not only should .init.smpalt be gone from the XIP version, but it might > be safer to put that into a special "should_be_empty" section and bail > out with ASSERT() if that section size isn't zero. Same thing for > pv_table. This way we'll be sure that nothing in the kernel config > expects self-modified code to be present. > > With XIP there is still an init section, but it is pointless to store > .init.text stuff in there. That should instead stay in ROM and never be > discarded (obviously). No need to page align it either at that point > which should save some ROM space. > > Things like .kprobes.text should be in ROM, but the entire executable in > ROM should be surrounded by __kprobes_text_start and __kprobes_text_end > to prevent any attempt to put kprobes there. This way, kprobes could > still work for modules. > > Etc. Etc. > > There are simply too many things these days that require special > considerations with XIP. And freeing the main script from XIP > considerations will ease maintenance as well, more than the added > maintenance from the duplicated parts. [Adding Uwe and Maxime to Cc] I believe Uwe has been running XIP_KERNEL on efm32 for a while now, but he also mentioned that he needed some extra hacks in the linker script and elsewhere to get it to work. It would be good to coordinate this, he may have found additional problems, or he can maybe help test the new approach on his hardware. Arnd
> With XIP there is still an init section, [snip] > No need to page align it either at that point which should save some > ROM space. Good point. > Things like .kprobes.text should be in ROM, [snip] > > Etc. Etc. Here's the thing...I'm probably out of my depth on most of this. But, I'd be happy to post up a RFC patch and then start collecting the feedback on what can be removed. I can at least test it on a real XIP system. Chris
> [Adding Uwe and Maxime to Cc] > > I believe Uwe has been running XIP_KERNEL on efm32 for a while now, > but he also mentioned that he needed some extra hacks in the linker > script and elsewhere to get it to work. Yes, Maxime got the XIP fix for scripts/link-vmlinux.sh into the kernel ([PATCH v6 01/15] scripts: link-vmlinux: Don't pass page offset to kallsyms if XIP Kernel), which of course made one less file I have to patch. > It would be good to coordinate this, he may have found additional > problems, or he can maybe help test the new approach on his hardware. The part I'm using (RZ/A1H) is a Cortex A9 and the EFM32 is a Cortex M3, so we'd be able to test both MMU and non-MMU XIP systems. Chris
On Wed, 18 Nov 2015, Chris Brandt wrote: > > With XIP there is still an init section, [snip] > > No need to page align it either at that point which should save some > > ROM space. > > Good point. > > > Things like .kprobes.text should be in ROM, [snip] > > > > Etc. Etc. > > Here's the thing...I'm probably out of my depth on most of this. But, > I'd be happy to post up a RFC patch and then start collecting the > feedback on what can be removed. I can at least test it on a real XIP > system. Sure. You should start by splitting the XIP stuff out to a separate script so functionally it is the same (broken) end result. And from there adding incremental fixes to the XIP script. Nicolas
> Sure. You should start by splitting the XIP stuff out to a separate > script so functionally it is the same (broken) end result. And from > there adding incremental fixes to the XIP script. More or less that's what I'm doing now on my system. You can diff the 2 in order to see what I've removed. But wait...you said 'the last step' was to add a 2nd linker file and change the Makefiles to use it. Are you saying I should first submit a patch that simply adds the new file, but not hook it up to any Makefiles yet? Then, once it has been updated a few times and is in better shape, I submit another patch that hooks it up with the Makefiles so I can then finally remove the XIP_KERNEL parts from the current vmlinux.lds.S? Chris
On Wed, 18 Nov 2015, Chris Brandt wrote: > > Sure. You should start by splitting the XIP stuff out to a separate > > script so functionally it is the same (broken) end result. And from > > there adding incremental fixes to the XIP script. > > More or less that's what I'm doing now on my system. You can diff the 2 in order to see what I've removed. > > But wait...you said 'the last step' was to add a 2nd linker file and change the Makefiles to use it. > > > Are you saying I should first submit a patch that simply adds the new file, but not hook it up to any Makefiles yet? > Then, once it has been updated a few times and is in better shape, I submit another patch that hooks it up with the Makefiles so I can then finally remove the XIP_KERNEL parts from the current vmlinux.lds.S? Well... I'm of the opinion now that the first patch should be the creation of the new script being a copy of the original with the XIP conditionals removed from both, plus the makefile changes. That should be easy to verify that nothing changed result wise, and then additional patches could bring XIP efinements on top without disturbing the non-XIP case. Nicolas
>> Are you saying I should first submit a patch that simply adds the >> new file, but not hook it up to any Makefiles yet? >> Then, once it has been updated a few times and is in better shape, >> I submit another patch that hooks it up with the Makefiles so I can >> then finally remove the XIP_KERNEL parts from the current >> vmlinux.lds.S? > > Well... I'm of the opinion now that the first patch should be the > creation of the new script being a copy of the original with the > XIP conditionals removed from both, plus the makefile changes. That > should be easy to verify that nothing changed result wise, and then > additional patches could bring XIP efinements on top without > disturbing the non-XIP case. > > Nicolas OK. I'm good with that. I'll submit a patch and see what happens. Chris
diff --git a/Makefile b/Makefile index fd46821..956ad21 100644 --- a/Makefile +++ b/Makefile @@ -898,7 +898,11 @@ libs-y := $(libs-y1) $(libs-y2) # Externally visible symbols (used by link-vmlinux.sh) export KBUILD_VMLINUX_INIT := $(head-y) $(init-y) export KBUILD_VMLINUX_MAIN := $(core-y) $(libs-y) $(drivers-y) $(net-y) +ifdef CONFIG_XIP_KERNEL +export KBUILD_LDS := arch/$(SRCARCH)/kernel/vmlinux-xip.lds +else export KBUILD_LDS := arch/$(SRCARCH)/kernel/vmlinux.lds +endif export LDFLAGS_vmlinux # used by scripts/pacmage/Makefile export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Documentation include samples scripts tools virt) diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index af9e59b..d97fc2e 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -3,6 +3,7 @@ # CPPFLAGS_vmlinux.lds := -DTEXT_OFFSET=$(TEXT_OFFSET) +CPPFLAGS_vmlinux-xip.lds := -DTEXT_OFFSET=$(TEXT_OFFSET) AFLAGS_head.o := -DTEXT_OFFSET=$(TEXT_OFFSET) ifdef CONFIG_FUNCTION_TRACER @@ -92,4 +93,8 @@ obj-y += psci-call.o obj-$(CONFIG_SMP) += psci_smp.o endif +ifdef CONFIG_XIP_KERNEL +extra-y := $(head-y) vmlinux-xip.lds +else extra-y := $(head-y) vmlinux.lds +endif