* [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode @ 2016-05-05 21:33 Aurelien Jarno 2016-05-05 21:40 ` David Daney 0 siblings, 1 reply; 7+ messages in thread From: Aurelien Jarno @ 2016-05-05 21:33 UTC (permalink / raw) To: linux-mips; +Cc: Aurelien Jarno, David Daney The initramfs if loaded in memory by U-Boot running in big endian mode. When the kernel is running in little endian mode, we need to byteswap it as it is accessed byte by byte. Cc: David Daney <david.daney@cavium.com> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- arch/mips/kernel/setup.c | 10 ++++++++++ 1 file changed, 10 insertions(+) Note: It might not be the best place to byteswap the initramfs not the best way to do it. At least it shows the problem and what shoudl be done. Suggestions to improve the patch are welcome. diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c index 4f60734..e7d015e 100644 --- a/arch/mips/kernel/setup.c +++ b/arch/mips/kernel/setup.c @@ -263,6 +263,16 @@ static void __init finalize_initrd(void) goto disable; } +#if defined(CONFIG_CPU_CAVIUM_OCTEON) && defined(CONFIG_CPU_LITTLE_ENDIAN) + { + unsigned long i; + pr_info("Cavium Octeon kernel in little endian mode " + "detected, byteswapping ramdisk\n"); + for (i = initrd_start; i < ALIGN(initrd_end, 8); i += 8) + *((unsigned long *)i) = swab64(*((unsigned long *)i)); + } +#endif + reserve_bootmem(__pa(initrd_start), size, BOOTMEM_DEFAULT); initrd_below_start_ok = 1; -- 2.8.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode @ 2016-05-05 21:40 ` David Daney 0 siblings, 0 replies; 7+ messages in thread From: David Daney @ 2016-05-05 21:40 UTC (permalink / raw) To: Aurelien Jarno, linux-mips; +Cc: David Daney On 05/05/2016 02:33 PM, Aurelien Jarno wrote: > The initramfs if loaded in memory by U-Boot running in big endian mode. > When the kernel is running in little endian mode, we need to byteswap it > as it is accessed byte by byte. Ouch! Really it should be fixed in the bootloader, but that probably won't happen. I wonder, is there a magic number that the initrd has? If so, we could probe for a byteswapped initrd and not do the byte reversal unconditionally. The logic seems correct, we need to byte swap each 8-byte aligned 8-byte word in the image. > > Cc: David Daney <david.daney@cavium.com> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > arch/mips/kernel/setup.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > Note: It might not be the best place to byteswap the initramfs not the > best way to do it. At least it shows the problem and what shoudl be done. > Suggestions to improve the patch are welcome. > > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c > index 4f60734..e7d015e 100644 > --- a/arch/mips/kernel/setup.c > +++ b/arch/mips/kernel/setup.c > @@ -263,6 +263,16 @@ static void __init finalize_initrd(void) > goto disable; > } > > +#if defined(CONFIG_CPU_CAVIUM_OCTEON) && defined(CONFIG_CPU_LITTLE_ENDIAN) > + { > + unsigned long i; > + pr_info("Cavium Octeon kernel in little endian mode " > + "detected, byteswapping ramdisk\n"); > + for (i = initrd_start; i < ALIGN(initrd_end, 8); i += 8) > + *((unsigned long *)i) = swab64(*((unsigned long *)i)); > + } > +#endif > + > reserve_bootmem(__pa(initrd_start), size, BOOTMEM_DEFAULT); > initrd_below_start_ok = 1; > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode @ 2016-05-05 21:40 ` David Daney 0 siblings, 0 replies; 7+ messages in thread From: David Daney @ 2016-05-05 21:40 UTC (permalink / raw) To: Aurelien Jarno, linux-mips; +Cc: David Daney On 05/05/2016 02:33 PM, Aurelien Jarno wrote: > The initramfs if loaded in memory by U-Boot running in big endian mode. > When the kernel is running in little endian mode, we need to byteswap it > as it is accessed byte by byte. Ouch! Really it should be fixed in the bootloader, but that probably won't happen. I wonder, is there a magic number that the initrd has? If so, we could probe for a byteswapped initrd and not do the byte reversal unconditionally. The logic seems correct, we need to byte swap each 8-byte aligned 8-byte word in the image. > > Cc: David Daney <david.daney@cavium.com> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > arch/mips/kernel/setup.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > Note: It might not be the best place to byteswap the initramfs not the > best way to do it. At least it shows the problem and what shoudl be done. > Suggestions to improve the patch are welcome. > > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c > index 4f60734..e7d015e 100644 > --- a/arch/mips/kernel/setup.c > +++ b/arch/mips/kernel/setup.c > @@ -263,6 +263,16 @@ static void __init finalize_initrd(void) > goto disable; > } > > +#if defined(CONFIG_CPU_CAVIUM_OCTEON) && defined(CONFIG_CPU_LITTLE_ENDIAN) > + { > + unsigned long i; > + pr_info("Cavium Octeon kernel in little endian mode " > + "detected, byteswapping ramdisk\n"); > + for (i = initrd_start; i < ALIGN(initrd_end, 8); i += 8) > + *((unsigned long *)i) = swab64(*((unsigned long *)i)); > + } > +#endif > + > reserve_bootmem(__pa(initrd_start), size, BOOTMEM_DEFAULT); > initrd_below_start_ok = 1; > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode 2016-05-05 21:40 ` David Daney (?) @ 2016-05-05 22:16 ` Aurelien Jarno 2016-05-05 22:53 ` David Daney -1 siblings, 1 reply; 7+ messages in thread From: Aurelien Jarno @ 2016-05-05 22:16 UTC (permalink / raw) To: David Daney; +Cc: linux-mips, David Daney On 2016-05-05 14:40, David Daney wrote: > On 05/05/2016 02:33 PM, Aurelien Jarno wrote: > >The initramfs if loaded in memory by U-Boot running in big endian mode. > >When the kernel is running in little endian mode, we need to byteswap it > >as it is accessed byte by byte. > > Ouch! > > Really it should be fixed in the bootloader, but that probably won't happen. How would you see that fixed in the bootloader? I doubt it's difficult to autodetect that, as the initramfs is basically loaded in memory first, and later only the address and size are passed on the kernel command line using the rd_start= and rd_size= options. The other alternative would be to provide reversed endian fatload, ext2load, ext4load, ... versions. Or maybe a better alternative would be a function to byteswap a memory area, it could be inserted in a bootcmd between the load and the bootoctlinux. > I wonder, is there a magic number that the initrd has? If so, we could > probe for a byteswapped initrd and not do the byte reversal unconditionally. There is a magic number... after it has been uncompressed. The magics for the compressed version are defined in lib/decompress.c. Maybe we can call decompress_method() from the finalize_initrd with the first 8 bytes byteswapped and check for the result. > The logic seems correct, we need to byte swap each 8-byte aligned 8-byte > word in the image. > > > > >Cc: David Daney <david.daney@cavium.com> > >Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > >--- > > arch/mips/kernel/setup.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > >Note: It might not be the best place to byteswap the initramfs not the > >best way to do it. At least it shows the problem and what shoudl be done. > >Suggestions to improve the patch are welcome. > > > >diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c > >index 4f60734..e7d015e 100644 > >--- a/arch/mips/kernel/setup.c > >+++ b/arch/mips/kernel/setup.c > >@@ -263,6 +263,16 @@ static void __init finalize_initrd(void) > > goto disable; > > } > > > >+#if defined(CONFIG_CPU_CAVIUM_OCTEON) && defined(CONFIG_CPU_LITTLE_ENDIAN) > >+ { > >+ unsigned long i; > >+ pr_info("Cavium Octeon kernel in little endian mode " > >+ "detected, byteswapping ramdisk\n"); > >+ for (i = initrd_start; i < ALIGN(initrd_end, 8); i += 8) > >+ *((unsigned long *)i) = swab64(*((unsigned long *)i)); > >+ } > >+#endif > >+ > > reserve_bootmem(__pa(initrd_start), size, BOOTMEM_DEFAULT); > > initrd_below_start_ok = 1; > > > > > > -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode @ 2016-05-05 22:53 ` David Daney 0 siblings, 0 replies; 7+ messages in thread From: David Daney @ 2016-05-05 22:53 UTC (permalink / raw) To: linux-mips, David Daney On 05/05/2016 03:16 PM, Aurelien Jarno wrote: > On 2016-05-05 14:40, David Daney wrote: >> On 05/05/2016 02:33 PM, Aurelien Jarno wrote: >>> The initramfs if loaded in memory by U-Boot running in big endian mode. >>> When the kernel is running in little endian mode, we need to byteswap it >>> as it is accessed byte by byte. >> >> Ouch! >> >> Really it should be fixed in the bootloader, but that probably won't happen. > > How would you see that fixed in the bootloader? I doubt it's difficult > to autodetect that, as the initramfs is basically loaded in memory > first, and later only the address and size are passed on the kernel > command line using the rd_start= and rd_size= options. > > The other alternative would be to provide reversed endian fatload, > ext2load, ext4load, ... versions. Or maybe a better alternative would be > a function to byteswap a memory area, it could be inserted in a bootcmd > between the load and the bootoctlinux. The easiest thing might be to byteswap it before u-boot ever gets involved, and leave the kernel alone. > >> I wonder, is there a magic number that the initrd has? If so, we could >> probe for a byteswapped initrd and not do the byte reversal unconditionally. > > There is a magic number... after it has been uncompressed. The magics > for the compressed version are defined in lib/decompress.c. Maybe we can > call decompress_method() from the finalize_initrd with the first 8 bytes > byteswapped and check for the result. I guess you could look for magic numbers of the supported compression protocols (gzip, xz, etc), but that could be error prone. I don't really object to the original patch, but was just trying to consider alternatives. > >> The logic seems correct, we need to byte swap each 8-byte aligned 8-byte >> word in the image. >> >>> >>> Cc: David Daney <david.daney@cavium.com> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >>> --- >>> arch/mips/kernel/setup.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> Note: It might not be the best place to byteswap the initramfs not the >>> best way to do it. At least it shows the problem and what shoudl be done. >>> Suggestions to improve the patch are welcome. >>> >>> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c >>> index 4f60734..e7d015e 100644 >>> --- a/arch/mips/kernel/setup.c >>> +++ b/arch/mips/kernel/setup.c >>> @@ -263,6 +263,16 @@ static void __init finalize_initrd(void) >>> goto disable; >>> } >>> >>> +#if defined(CONFIG_CPU_CAVIUM_OCTEON) && defined(CONFIG_CPU_LITTLE_ENDIAN) >>> + { >>> + unsigned long i; >>> + pr_info("Cavium Octeon kernel in little endian mode " >>> + "detected, byteswapping ramdisk\n"); >>> + for (i = initrd_start; i < ALIGN(initrd_end, 8); i += 8) >>> + *((unsigned long *)i) = swab64(*((unsigned long *)i)); >>> + } >>> +#endif >>> + >>> reserve_bootmem(__pa(initrd_start), size, BOOTMEM_DEFAULT); >>> initrd_below_start_ok = 1; >>> >>> >> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode @ 2016-05-05 22:53 ` David Daney 0 siblings, 0 replies; 7+ messages in thread From: David Daney @ 2016-05-05 22:53 UTC (permalink / raw) To: linux-mips, David Daney On 05/05/2016 03:16 PM, Aurelien Jarno wrote: > On 2016-05-05 14:40, David Daney wrote: >> On 05/05/2016 02:33 PM, Aurelien Jarno wrote: >>> The initramfs if loaded in memory by U-Boot running in big endian mode. >>> When the kernel is running in little endian mode, we need to byteswap it >>> as it is accessed byte by byte. >> >> Ouch! >> >> Really it should be fixed in the bootloader, but that probably won't happen. > > How would you see that fixed in the bootloader? I doubt it's difficult > to autodetect that, as the initramfs is basically loaded in memory > first, and later only the address and size are passed on the kernel > command line using the rd_start= and rd_size= options. > > The other alternative would be to provide reversed endian fatload, > ext2load, ext4load, ... versions. Or maybe a better alternative would be > a function to byteswap a memory area, it could be inserted in a bootcmd > between the load and the bootoctlinux. The easiest thing might be to byteswap it before u-boot ever gets involved, and leave the kernel alone. > >> I wonder, is there a magic number that the initrd has? If so, we could >> probe for a byteswapped initrd and not do the byte reversal unconditionally. > > There is a magic number... after it has been uncompressed. The magics > for the compressed version are defined in lib/decompress.c. Maybe we can > call decompress_method() from the finalize_initrd with the first 8 bytes > byteswapped and check for the result. I guess you could look for magic numbers of the supported compression protocols (gzip, xz, etc), but that could be error prone. I don't really object to the original patch, but was just trying to consider alternatives. > >> The logic seems correct, we need to byte swap each 8-byte aligned 8-byte >> word in the image. >> >>> >>> Cc: David Daney <david.daney@cavium.com> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >>> --- >>> arch/mips/kernel/setup.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> Note: It might not be the best place to byteswap the initramfs not the >>> best way to do it. At least it shows the problem and what shoudl be done. >>> Suggestions to improve the patch are welcome. >>> >>> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c >>> index 4f60734..e7d015e 100644 >>> --- a/arch/mips/kernel/setup.c >>> +++ b/arch/mips/kernel/setup.c >>> @@ -263,6 +263,16 @@ static void __init finalize_initrd(void) >>> goto disable; >>> } >>> >>> +#if defined(CONFIG_CPU_CAVIUM_OCTEON) && defined(CONFIG_CPU_LITTLE_ENDIAN) >>> + { >>> + unsigned long i; >>> + pr_info("Cavium Octeon kernel in little endian mode " >>> + "detected, byteswapping ramdisk\n"); >>> + for (i = initrd_start; i < ALIGN(initrd_end, 8); i += 8) >>> + *((unsigned long *)i) = swab64(*((unsigned long *)i)); >>> + } >>> +#endif >>> + >>> reserve_bootmem(__pa(initrd_start), size, BOOTMEM_DEFAULT); >>> initrd_below_start_ok = 1; >>> >>> >> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode 2016-05-05 22:53 ` David Daney (?) @ 2016-05-11 15:44 ` Aurelien Jarno -1 siblings, 0 replies; 7+ messages in thread From: Aurelien Jarno @ 2016-05-11 15:44 UTC (permalink / raw) To: David Daney; +Cc: linux-mips, David Daney [ I have just realized this mail was never sent. Doing it now so that it can given a bit of context for the v2 ] On 2016-05-05 15:53, David Daney wrote: > > > On 05/05/2016 03:16 PM, Aurelien Jarno wrote: > > On 2016-05-05 14:40, David Daney wrote: > > > On 05/05/2016 02:33 PM, Aurelien Jarno wrote: > > > > The initramfs if loaded in memory by U-Boot running in big endian mode. > > > > When the kernel is running in little endian mode, we need to byteswap it > > > > as it is accessed byte by byte. > > > > > > Ouch! > > > > > > Really it should be fixed in the bootloader, but that probably won't happen. > > > > How would you see that fixed in the bootloader? I doubt it's difficult > > to autodetect that, as the initramfs is basically loaded in memory > > first, and later only the address and size are passed on the kernel > > command line using the rd_start= and rd_size= options. > > > > The other alternative would be to provide reversed endian fatload, > > ext2load, ext4load, ... versions. Or maybe a better alternative would be > > a function to byteswap a memory area, it could be inserted in a bootcmd > > between the load and the bootoctlinux. > > The easiest thing might be to byteswap it before u-boot ever gets involved, > and leave the kernel alone. I guess you basically mean in userland when generating the initramfs. It means that we have to store the initramfs byte swapped on the hard drive. It will break standard tools which might try to access it (e.g. lsinitramfs). In addition to that it will also breaks kexec, which will be run in little endian mode. This is also a reason to not always byte swap the initramfs in the kernel. > > > > > I wonder, is there a magic number that the initrd has? If so, we could > > > probe for a byteswapped initrd and not do the byte reversal unconditionally. > > > > There is a magic number... after it has been uncompressed. The magics > > for the compressed version are defined in lib/decompress.c. Maybe we can > > call decompress_method() from the finalize_initrd with the first 8 bytes > > byteswapped and check for the result. > > I guess you could look for magic numbers of the supported compression > protocols (gzip, xz, etc), but that could be error prone. > > I don't really object to the original patch, but was just trying to consider > alternatives. I have just sent a patch which does that. Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-11 15:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-05 21:33 [PATCH] MIPS: Octeon: byteswap initramfs in little endian mode Aurelien Jarno 2016-05-05 21:40 ` David Daney 2016-05-05 21:40 ` David Daney 2016-05-05 22:16 ` Aurelien Jarno 2016-05-05 22:53 ` David Daney 2016-05-05 22:53 ` David Daney 2016-05-11 15:44 ` Aurelien Jarno
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.