| Message ID | 20110601141847.GG23771@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 01, 2011 at 04:18:47PM +0200, Sascha Hauer wrote: > diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile > index 9128fdd..e3b6f02 100644 > --- a/arch/arm/boot/Makefile > +++ b/arch/arm/boot/Makefile > @@ -59,6 +59,11 @@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE > > endif > > +ifeq ($(CONFIG_ARM_PATCH_PHYS_VIRT),y) > +$(obj)/uImage: $(obj)/zImage FORCE > + @echo 'building uImages is incompatible with CONFIG_ARM_PATCH_PHYS_VIRT' > + @false It may be better to drop the zImage dependency too, so that folk don't get the message at the end of an otherwise successful kernel build - they may decide to turn p2v patching off which would result in a rebuild. Or it could suggest the mkimage command which should be run, but with <LOAD-ADDRESS> inserted into it at the appropriate point - iow something like: mkimage -A arm -O linux -T kernel -C none \ -a <LOAD-ADDRESS> -e <START-ADDRESS> \ -n 'Linux-$(KERNELRELEASE)' -d arch/arm/boot/zImage uImage.<PLATFORM>
On Wed, Jun 01, 2011 at 03:24:06PM +0100, Russell King - ARM Linux wrote: > On Wed, Jun 01, 2011 at 04:18:47PM +0200, Sascha Hauer wrote: > > diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile > > index 9128fdd..e3b6f02 100644 > > --- a/arch/arm/boot/Makefile > > +++ b/arch/arm/boot/Makefile > > @@ -59,6 +59,11 @@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE > > > > endif > > > > +ifeq ($(CONFIG_ARM_PATCH_PHYS_VIRT),y) > > +$(obj)/uImage: $(obj)/zImage FORCE > > + @echo 'building uImages is incompatible with CONFIG_ARM_PATCH_PHYS_VIRT' > > + @false > > It may be better to drop the zImage dependency too, so that folk don't get > the message at the end of an otherwise successful kernel build - they may > decide to turn p2v patching off which would result in a rebuild. It's not that easy. We have this in arch/arm/Makefile: zImage Image xipImage bootpImage uImage: vmlinux $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@ So arch/arm/boot/Makefile gets evaluated after a successful build. We have to make the test in arch/arm/Makefile if we want to catch this earlier. > > Or it could suggest the mkimage command which should be run, but with > <LOAD-ADDRESS> inserted into it at the appropriate point - iow something > like: > > mkimage -A arm -O linux -T kernel -C none \ > -a <LOAD-ADDRESS> -e <START-ADDRESS> \ > -n 'Linux-$(KERNELRELEASE)' -d arch/arm/boot/zImage uImage.<PLATFORM> Ok, it's a good idea to give the user a hint what to do. Sascha
Hello, On Wed, Jun 01, 2011 at 04:36:04PM +0200, Sascha Hauer wrote: > On Wed, Jun 01, 2011 at 03:24:06PM +0100, Russell King - ARM Linux wrote: > > On Wed, Jun 01, 2011 at 04:18:47PM +0200, Sascha Hauer wrote: > > > diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile > > > index 9128fdd..e3b6f02 100644 > > > --- a/arch/arm/boot/Makefile > > > +++ b/arch/arm/boot/Makefile > > > @@ -59,6 +59,11 @@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE > > > > > > endif > > > > > > +ifeq ($(CONFIG_ARM_PATCH_PHYS_VIRT),y) > > > +$(obj)/uImage: $(obj)/zImage FORCE > > > + @echo 'building uImages is incompatible with CONFIG_ARM_PATCH_PHYS_VIRT' > > > + @false > > > > It may be better to drop the zImage dependency too, so that folk don't get > > the message at the end of an otherwise successful kernel build - they may > > decide to turn p2v patching off which would result in a rebuild. > > It's not that easy. We have this in arch/arm/Makefile: > > zImage Image xipImage bootpImage uImage: vmlinux > $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@ > > So arch/arm/boot/Makefile gets evaluated after a successful build. We > have to make the test in arch/arm/Makefile if we want to catch this > earlier. > > > > > Or it could suggest the mkimage command which should be run, but with > > <LOAD-ADDRESS> inserted into it at the appropriate point - iow something > > like: > > > > mkimage -A arm -O linux -T kernel -C none \ > > -a <LOAD-ADDRESS> -e <START-ADDRESS> \ > > -n 'Linux-$(KERNELRELEASE)' -d arch/arm/boot/zImage uImage.<PLATFORM> > > Ok, it's a good idea to give the user a hint what to do. Back when I worked on RUNTIME_PHYSOFFSET I addressed that problem. My patch that you can find in the archives allowed to do: make uImage LOAD_ADDRESS=... even with RUNTIME_PHYSOFFSET=y. I didn't try to find that patch, though. Best regards Uwe
Dear Sascha Hauer, In message <20110601141847.GG23771@pengutronix.de> you wrote: > > > We probably should disable the uImage target when p2v patching is > > enabled to prevent people getting nasty surprises. > > > > Agreed. Here is a patch. I added Wolfgang Denk to Cc, maybe > he can prove me wrong. > > 8<---------------------------------------------------------- > ARM: do not allow to build uImages with ARM_PATCH_PHYS_VIRT > > U-Boot uImages expect a load address and a entry point in > the image header. With CONFIG_ARM_PATCH_PHYS_VIRT these > become variable and thus can not be compiled into the uImage. Would it help if we interpret, for example, the values for load address and entry point not as physical addresses, but instead as offsets relative to the start of physical RAM? This would still require that all systems supported by a single image use the same offsets. Is this possible? Best regards, Wolfgang Denk
On Wed, Jun 1, 2011 at 4:08 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Sascha Hauer, > > In message <20110601141847.GG23771@pengutronix.de> you wrote: >> >> > We probably should disable the uImage target when p2v patching is >> > enabled to prevent people getting nasty surprises. >> > >> >> Agreed. Here is a patch. I added Wolfgang Denk to Cc, maybe >> he can prove me wrong. >> >> 8<---------------------------------------------------------- >> ARM: do not allow to build uImages with ARM_PATCH_PHYS_VIRT >> >> U-Boot uImages expect a load address and a entry point in >> the image header. With CONFIG_ARM_PATCH_PHYS_VIRT these >> become variable and thus can not be compiled into the uImage. > > Would it help if we interpret, for example, the values for load > address and entry point not as physical addresses, but instead as > offsets relative to the start of physical RAM? > > This would still require that all systems supported by a single image > use the same offsets. Is this possible? Been having this discussion on and off with various parties and we determined that if U-Boot had a little extra logic when parsing a uImage header it could perfectly validly boot a zImage contained in a uImage header. In this case, just a load address in the uImage header of 0 (or some other totally-impossible value like 0xffffffff in case 0 is perfectly valid somewhere) and then just jump to the entry point by deriving the value from the header size - based on the fact that ARM images are PIC and the Linux kernel always puts the entry point at 0 offset - to the address of the uImage header + size of the uImage header (which U-Boot knows already from parsing the header). Example: on the MX51 the entry point is usually set to 0x90008000 - that's what Freescale put in their BSP U-Boots and everyone has copied it.. There's a variable in our U-Boots at least that is used in boot scripts to ext/fat/whateverload it to 0x90007FC0. That 0x90007FC0 address to load to is a nasty hack meant to match the header size of a legacy uImage (therefore the first byte of the payload will live at 0x90008000). We can't support anything but a legacy image right now because of that, and if we needed to support a new style uImage with FDT, then this load address and entry point magic would be totally wrong anyway requiring both userspace script and kernel changes. So the solution is * Assuming all ARM kernels are PIC (guaranteed, right?) * Assuming all ARM kernels start at entry point 0 (true for Image and zImage) * Assuming there is a globally invalid magic value you can set in the uImage header as load address (not sure) * Assuming you can make sure U-Boot only does this for ARM, kernel-type images and not ramdisks or so (true) Set that magic value, U-Boot magically derives the correct entry point as the first address of the payload, and jumps to it? I tested it.. works great but I don't have a wealth of ARM hardware to TRULY confirm all the above. Let's not kill off uImage generation just yet if we can just patch our firmwares once and for all and let Linux decide whether it sets the load address to a real address or a magic value? Then all variants of kernel will work for the boards, patching phys_to_virt or not.
On Wed, Jun 01, 2011 at 06:04:02PM -0500, Matt Sealey wrote: > On Wed, Jun 1, 2011 at 4:08 PM, Wolfgang Denk <wd@denx.de> wrote: > > Dear Sascha Hauer, > > > > In message <20110601141847.GG23771@pengutronix.de> you wrote: > >> > >> > We probably should disable the uImage target when p2v patching is > >> > enabled to prevent people getting nasty surprises. > >> > > >> > >> Agreed. Here is a patch. I added Wolfgang Denk to Cc, maybe > >> he can prove me wrong. > >> > >> 8<---------------------------------------------------------- > >> ARM: do not allow to build uImages with ARM_PATCH_PHYS_VIRT > >> > >> U-Boot uImages expect a load address and a entry point in > >> the image header. With CONFIG_ARM_PATCH_PHYS_VIRT these > >> become variable and thus can not be compiled into the uImage. > > > > Would it help if we interpret, for example, the values for load > > address and entry point not as physical addresses, but instead as > > offsets relative to the start of physical RAM? > > > > This would still require that all systems supported by a single image > > use the same offsets. Is this possible? > > Been having this discussion on and off with various parties and we > determined that if U-Boot had a little > extra logic when parsing a uImage header it could perfectly validly > boot a zImage contained in a uImage > header. > > In this case, just a load address in the uImage header of 0 (or some > other totally-impossible value like 0xffffffff in case 0 is perfectly > valid somewhere) AFAIK 0x0 is the standard entry point on powerpc, but 0xffffffff should be fine. > and then just jump to the entry point by deriving the > value from the header size - based on the fact that ARM images are PIC > and the Linux kernel always puts the entry point at 0 offset - to the > address of the uImage header + size of the uImage header (which U-Boot > knows already from parsing the header). > > Example: on the MX51 the entry point is usually set to 0x90008000 - > that's what Freescale put in their BSP > U-Boots and everyone has copied it.. There's a variable in our U-Boots > at least that is used in boot scripts to > ext/fat/whateverload it to 0x90007FC0. That 0x90007FC0 address to load > to is a nasty hack meant to match > the header size of a legacy uImage (therefore the first byte of the > payload will live at 0x90008000). > > We can't support anything but a legacy image right now because of > that, and if we needed to support a new > style uImage with FDT, then this load address and entry point magic > would be totally wrong anyway requiring > both userspace script and kernel changes. > > So the solution is > > * Assuming all ARM kernels are PIC (guaranteed, right?) zImages are. The restriction here is: config AUTO_ZRELADDR bool "Auto calculation of the decompressed kernel image address" depends on !ZBOOT_ROM && !ARCH_U300 help ZRELADDR is the physical address where the decompressed kernel image will be placed. If AUTO_ZRELADDR is selected, the address will be determined at run-time by masking the current IP with 0xf8000000. This assumes the zImage being placed in the first 128MB from start of memory. So U-Boot could interpret load address being set to 0xffffffff as 'put it somewhere in the first 128MB it jump to this address'. > * Assuming all ARM kernels start at entry point 0 (true for Image and zImage) You mean that the entry point is load address + 0? That should be true. Even if not, when the load address is 0xffffffff, the entry point field could still describe an offset into the image. > * Assuming there is a globally invalid magic value you can set in the > uImage header as load address (not sure) > * Assuming you can make sure U-Boot only does this for ARM, > kernel-type images and not ramdisks or so (true) ramdisks would need the same mechanism if we want to attach them to multi SoC kernels. > > Set that magic value, U-Boot magically derives the correct entry point > as the first address of the payload, > and jumps to it? > > I tested it.. works great but I don't have a wealth of ARM hardware to > TRULY confirm all the above. > > Let's not kill off uImage generation just yet if we can just patch our > firmwares once and for all and let Linux > decide whether it sets the load address to a real address or a magic > value? Then all variants of kernel will > work for the boards, patching phys_to_virt or not. > > -- > Matt Sealey <matt@genesi-usa.com> > Product Development Analyst, Genesi USA, Inc. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jun 02, 2011 at 12:34:07PM +0200, Sascha Hauer wrote: > > * Assuming all ARM kernels are PIC (guaranteed, right?) > > zImages are. The restriction here is: > > config AUTO_ZRELADDR > bool "Auto calculation of the decompressed kernel image address" > depends on !ZBOOT_ROM && !ARCH_U300 > help > ZRELADDR is the physical address where the decompressed kernel > image will be placed. If AUTO_ZRELADDR is selected, the address > will be determined at run-time by masking the current IP with > 0xf8000000. This assumes the zImage being placed in the first 128MB > from start of memory. > > So U-Boot could interpret load address being set to 0xffffffff as 'put it > somewhere in the first 128MB it jump to this address'. Sascha, I think you're mixing stuff up. The decompressor is carefully coded to be relocatable provided ZBOOT_ROM=n. This will be the case for all zImages built to run from RAM. ZBOOT_ROM needs to be set to 'y' if you're building a zImage to run directly from flash, in which case you're not copying it into RAM first. Even a ZBOOT_ROM=y image can be booted from RAM, but there are additional restrictions on its placement (as it has the .bss address in RAM hard-coded.) But let's ignore this case because it adds far more complexity than is required. The only thing to bear in mind is that we place the page tables at 16K into RAM, so the zImage should not be loaded below the low 32K of RAM. So that implies that uboot must not load a 'relocatable' uImage below START_OF_RAM + 32K. Now, AUTO_ZRELADDR allows us to calculate where to place the _decompressed_ kernel by placing it 32K into the start of the 128MB region which the decompressor is running from. So that restricts the upper boundary to START_OF_RAM + 128M. Note that it's fine to place the decompressor where the decompressed image will be located, as the decompressor sorts itself out. However, that does add some additional overhead, so it may be worth making this offset a parameter which can be set in uboot. Default it to 32K, but allow users to change it if they want to avoid that overlap. > > * Assuming all ARM kernels start at entry point 0 (true for Image and zImage) > > You mean that the entry point is load address + 0? That should be true. > Even if not, when the load address is 0xffffffff, the entry point field > could still describe an offset into the image. All ARM kernels are entered at the first address of the image.
Dear Matt, In message <BANLkTi=HqEBRzyU4_HXnNHTeOPL9vPW8PA@mail.gmail.com> you wrote: > > In this case, just a load address in the uImage header of 0 (or some > other totally-impossible value like 0xffffffff > in case 0 is perfectly valid somewhere) and then just jump to the > entry point by deriving the value from the > header size - based on the fact that ARM images are PIC and the Linux > kernel always puts the entry point > at 0 offset - to the address of the uImage header + size of the uImage > header (which U-Boot knows already > from parsing the header). I'm not a friend of using magic values that are "totally-impossible". This is confusing, and here it is not needed. If we could agree to interpret load address and entry point as offsets, the only remaining problem is to find out if we can use a common offset. > So the solution is > > * Assuming all ARM kernels are PIC (guaranteed, right?) > * Assuming all ARM kernels start at entry point 0 (true for Image and zImage) You mean offset 0 to the start of the image here. > * Assuming there is a globally invalid magic value you can set in the > uImage header as load address (not sure) I dislike this. > Set that magic value, U-Boot magically derives the correct entry point > as the first address of the payload, > and jumps to it? That means you would not want to encode a load address in the image, and always run from the current location, no matter where it is? Best regards, Wolfgang Denk
On Thu, Jun 2, 2011 at 10:46 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Matt, > >> * Assuming all ARM kernels are PIC (guaranteed, right?) >> * Assuming all ARM kernels start at entry point 0 (true for Image and zImage) > > You mean offset 0 to the start of the image here. Yep. I missed a couple words between point and 0 :) >> * Assuming there is a globally invalid magic value you can set in the >> uImage header as load address (not sure) > > I dislike this. Me too but how else would you do it? A flag in the uImage header that says disregard the load address would work. Or a U-Boot that basically disregarded the load address *anyway*. I understand Sascha pointed out 0x0 is the standard load address on PPC but we're talking about an ARM thing here. U-Boot knows both what processor it is built for and it knows from the uImage header which architecture the kernel image is for. It only needs to be true that 0x0 is totally invalid, stupid value to use as a load address on ARM, and it would comply with the expectations that if (address) then it can carry on as before, else do the "new" behavior of deriving the address of the Linux entry point.. >> Set that magic value, U-Boot magically derives the correct entry point >> as the first address of the payload, >> and jumps to it? > > That means you would not want to encode a load address in the image, > and always run from the current location, no matter where it is? Exactly. Isn't this the beauty of position independent code, that you don't need to hardcode a load address? In the case of most Linux images the first thing they do is decompress themselves anyway.
On Thu, Jun 02, 2011 at 05:46:59PM +0200, Wolfgang Denk wrote: > Dear Matt, > > In message <BANLkTi=HqEBRzyU4_HXnNHTeOPL9vPW8PA@mail.gmail.com> you wrote: > > > > In this case, just a load address in the uImage header of 0 (or some > > other totally-impossible value like 0xffffffff > > in case 0 is perfectly valid somewhere) and then just jump to the > > entry point by deriving the value from the > > header size - based on the fact that ARM images are PIC and the Linux > > kernel always puts the entry point > > at 0 offset - to the address of the uImage header + size of the uImage > > header (which U-Boot knows already > > from parsing the header). > > I'm not a friend of using magic values that are "totally-impossible". > This is confusing, and here it is not needed. I find using a field labelled 'entry point' and 'load address' as absolute addresses or offsets into sdram depending on some flag also quite confusing. btw how do you want to decide whether the fields are offsets or absolute addresses? I see no flags field in the uImage format. > > If we could agree to interpret load address and entry point as > offsets, the only remaining problem is to find out if we can use a > common offset. The zImages encapsulated in uImages are relocatable. So pick any offset between 32k and 128MB into sdram like Russell explained. Higher values are not desirable as this would break boards with less sdram. Sascha
Dear Sascha Hauer, In message <20110603120258.GR23771@pengutronix.de> you wrote: > > > I'm not a friend of using magic values that are "totally-impossible". > > This is confusing, and here it is not needed. > > I find using a field labelled 'entry point' and 'load address' as > absolute addresses or offsets into sdram depending on some flag > also quite confusing. btw how do you want to decide whether the > fields are offsets or absolute addresses? I see no flags field in > the uImage format. I would define a new image type. Instead of "-T kernel" we could for example use a new type "-T relkernel" to indicate that addresses are relative to start of system RAM. > > If we could agree to interpret load address and entry point as > > offsets, the only remaining problem is to find out if we can use a > > common offset. > > The zImages encapsulated in uImages are relocatable. So pick any offset > between 32k and 128MB into sdram like Russell explained. Higher values > are not desirable as this would break boards with less sdram. What about the images without the preloader? You know that U-Boot already provides all the needed code to perform the uncompressing, so it would be nice if we could avoid to include this into each and every image. Is the raw kernel image, without the preloader, also relocatable and can be started at any address (keeping above restrictions in mind, of course)? Best regards, Wolfgang Denk
On Fri, Jun 03, 2011 at 02:17:02PM +0200, Wolfgang Denk wrote: > Dear Sascha Hauer, > > In message <20110603120258.GR23771@pengutronix.de> you wrote: > > > > > I'm not a friend of using magic values that are "totally-impossible". > > > This is confusing, and here it is not needed. > > > > I find using a field labelled 'entry point' and 'load address' as > > absolute addresses or offsets into sdram depending on some flag > > also quite confusing. btw how do you want to decide whether the > > fields are offsets or absolute addresses? I see no flags field in > > the uImage format. > > I would define a new image type. Instead of "-T kernel" we could for > example use a new type "-T relkernel" to indicate that addresses are > relative to start of system RAM. > > > > If we could agree to interpret load address and entry point as > > > offsets, the only remaining problem is to find out if we can use a > > > common offset. > > > > The zImages encapsulated in uImages are relocatable. So pick any offset > > between 32k and 128MB into sdram like Russell explained. Higher values > > are not desirable as this would break boards with less sdram. > > What about the images without the preloader? You know that U-Boot > already provides all the needed code to perform the uncompressing, so > it would be nice if we could avoid to include this into each and every > image. Is the raw kernel image, without the preloader, also > relocatable and can be started at any address (keeping above > restrictions in mind, of course)? No, the raw image has to be started at virt_to_phys(PAGE_OFFSET + TEXT_OFFSET) which is usually 32k into sdram. Only some subarchs define it differently: textofs-y := 0x00008000 textofs-$(CONFIG_ARCH_CLPS711X) := 0x00028000 textofs-$(CONFIG_PM_H1940) := 0x00108000 textofs-$(CONFIG_SA1111) := 0x00208000 Sascha
diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile index 9128fdd..e3b6f02 100644 --- a/arch/arm/boot/Makefile +++ b/arch/arm/boot/Makefile @@ -59,6 +59,11 @@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE endif +ifeq ($(CONFIG_ARM_PATCH_PHYS_VIRT),y) +$(obj)/uImage: $(obj)/zImage FORCE + @echo 'building uImages is incompatible with CONFIG_ARM_PATCH_PHYS_VIRT' + @false +else quiet_cmd_uimage = UIMAGE $@ cmd_uimage = $(CONFIG_SHELL) $(MKIMAGE) -A arm -O linux -T kernel \ -C none -a $(LOADADDR) -e $(STARTADDR) \ @@ -75,6 +80,7 @@ $(obj)/uImage: STARTADDR=$(LOADADDR) $(obj)/uImage: $(obj)/zImage FORCE $(call if_changed,uimage) @echo ' Image $@ is ready' +endif $(obj)/bootp/bootp: $(obj)/zImage initrd FORCE $(Q)$(MAKE) $(build)=$(obj)/bootp $@