| Message ID | 20170608125847.2611-1-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Gah, it's rather unfortunate that this has been discovered only a few days after the other decompressor fix was merged and hit Linus' tree... On Thu, Jun 08, 2017 at 12:58:47PM +0000, Ard Biesheuvel wrote: > Commit 06a4b6d009a1 ("ARM: 8677/1: boot/compressed: fix decompressor > header layout for v7-M") fixed an issue in the layout of the header > of the compressed kernel image that was caused by the assembler > emitting narrow opcodes for 'mov r0, r0', and for this reason, the > mnemonic was updated to use the W() macro, which will append the .w > suffix (which forces a wide encoding) if required, i.e., when building > the kernel in Thumb2 mode. > > However, this failed to take into account that on Thumb2 kernels built > for CPUs that are also ARM capable, the entry point is entered in ARM > mode, and so the instructions emitted here will be ARM instructions > that only exist in a wide encoding to begin with, which is why the > assembler rejects the .w suffix here and aborts the build with the > following message: > > head.S: Assembler messages: > head.S:132: Error: width suffixes are invalid in ARM mode -- `mov.w r0,r0' > > So replace the W(mov) with separate ARM and Thumb2 instructions, where > the latter will only be used for CPU_THUMBONLY builds. > > Fixes: 06a4b6d009a1 ("ARM: 8677/1: boot/compressed: fix decompressor ...") > Reported-by: Arnd Bergmann <arnd@arndb.de> > Acked-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > Apologies for the breakage: apparently, the combination CONFIG_THUMB2_KERNEL=y, > CONFIG_CPU_THUMBONLY=n and CONFIG_EFI_STUB=y was not among the configurations > I tested before sending out the original patch. > > arch/arm/boot/compressed/efi-header.S | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S > index 3f7d1b74c5e0..a17ca8d78656 100644 > --- a/arch/arm/boot/compressed/efi-header.S > +++ b/arch/arm/boot/compressed/efi-header.S > @@ -17,7 +17,8 @@ > @ there. > .inst 'M' | ('Z' << 8) | (0x1310 << 16) @ tstne r0, #0x4d000 > #else > - W(mov) r0, r0 > + AR_CLASS( mov r0, r0 ) > + M_CLASS( nop.w ) > #endif > .endm > > -- > 2.9.3 >
On 8 June 2017 at 16:29, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > Gah, it's rather unfortunate that this has been discovered only a > few days after the other decompressor fix was merged and hit Linus' > tree... > I guess that is where Arnd found it, although he didn't specify. In any case, apologies for the breakage and thanks to Arnd for reporting it. I have dropped this into the patch system. > On Thu, Jun 08, 2017 at 12:58:47PM +0000, Ard Biesheuvel wrote: >> Commit 06a4b6d009a1 ("ARM: 8677/1: boot/compressed: fix decompressor >> header layout for v7-M") fixed an issue in the layout of the header >> of the compressed kernel image that was caused by the assembler >> emitting narrow opcodes for 'mov r0, r0', and for this reason, the >> mnemonic was updated to use the W() macro, which will append the .w >> suffix (which forces a wide encoding) if required, i.e., when building >> the kernel in Thumb2 mode. >> >> However, this failed to take into account that on Thumb2 kernels built >> for CPUs that are also ARM capable, the entry point is entered in ARM >> mode, and so the instructions emitted here will be ARM instructions >> that only exist in a wide encoding to begin with, which is why the >> assembler rejects the .w suffix here and aborts the build with the >> following message: >> >> head.S: Assembler messages: >> head.S:132: Error: width suffixes are invalid in ARM mode -- `mov.w r0,r0' >> >> So replace the W(mov) with separate ARM and Thumb2 instructions, where >> the latter will only be used for CPU_THUMBONLY builds. >> >> Fixes: 06a4b6d009a1 ("ARM: 8677/1: boot/compressed: fix decompressor ...") >> Reported-by: Arnd Bergmann <arnd@arndb.de> >> Acked-by: Arnd Bergmann <arnd@arndb.de> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> >> Apologies for the breakage: apparently, the combination CONFIG_THUMB2_KERNEL=y, >> CONFIG_CPU_THUMBONLY=n and CONFIG_EFI_STUB=y was not among the configurations >> I tested before sending out the original patch. >> >> arch/arm/boot/compressed/efi-header.S | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S >> index 3f7d1b74c5e0..a17ca8d78656 100644 >> --- a/arch/arm/boot/compressed/efi-header.S >> +++ b/arch/arm/boot/compressed/efi-header.S >> @@ -17,7 +17,8 @@ >> @ there. >> .inst 'M' | ('Z' << 8) | (0x1310 << 16) @ tstne r0, #0x4d000 >> #else >> - W(mov) r0, r0 >> + AR_CLASS( mov r0, r0 ) >> + M_CLASS( nop.w ) >> #endif >> .endm >> >> -- >> 2.9.3 >> > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net.
On Fri, Jun 9, 2017 at 11:15 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 8 June 2017 at 16:29, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: >> Gah, it's rather unfortunate that this has been discovered only a >> few days after the other decompressor fix was merged and hit Linus' >> tree... >> > > I guess that is where Arnd found it, although he didn't specify. In > any case, apologies for the breakage and thanks to Arnd for reporting > it. I found it in linux-next randconfig tests, but I had not updated my tree for a few days before it, so it probably escaped my testing when it was in linux-next and not yet in mainline. Arnd
diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S index 3f7d1b74c5e0..a17ca8d78656 100644 --- a/arch/arm/boot/compressed/efi-header.S +++ b/arch/arm/boot/compressed/efi-header.S @@ -17,7 +17,8 @@ @ there. .inst 'M' | ('Z' << 8) | (0x1310 << 16) @ tstne r0, #0x4d000 #else - W(mov) r0, r0 + AR_CLASS( mov r0, r0 ) + M_CLASS( nop.w ) #endif .endm
Commit 06a4b6d009a1 ("ARM: 8677/1: boot/compressed: fix decompressor header layout for v7-M") fixed an issue in the layout of the header of the compressed kernel image that was caused by the assembler emitting narrow opcodes for 'mov r0, r0', and for this reason, the mnemonic was updated to use the W() macro, which will append the .w suffix (which forces a wide encoding) if required, i.e., when building the kernel in Thumb2 mode. However, this failed to take into account that on Thumb2 kernels built for CPUs that are also ARM capable, the entry point is entered in ARM mode, and so the instructions emitted here will be ARM instructions that only exist in a wide encoding to begin with, which is why the assembler rejects the .w suffix here and aborts the build with the following message: head.S: Assembler messages: head.S:132: Error: width suffixes are invalid in ARM mode -- `mov.w r0,r0' So replace the W(mov) with separate ARM and Thumb2 instructions, where the latter will only be used for CPU_THUMBONLY builds. Fixes: 06a4b6d009a1 ("ARM: 8677/1: boot/compressed: fix decompressor ...") Reported-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- Apologies for the breakage: apparently, the combination CONFIG_THUMB2_KERNEL=y, CONFIG_CPU_THUMBONLY=n and CONFIG_EFI_STUB=y was not among the configurations I tested before sending out the original patch. arch/arm/boot/compressed/efi-header.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)