| Message ID | 1416830200-11114-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 13d1b9575ac2c2da143cd2236b6cf0fc314570f8 |
| Headers | show |
On Mon, 24 Nov 2014, Ard Biesheuvel wrote: > Two files that get included when building the multi_v7_defconfig target > fail to build when selecting THUMB2_KERNEL for this configuration. > > In both cases, we can just build the file as ARM code, as none of its > symbols are exported to modules, so there are no interworking concerns. > In the iwmmxt.S case, add ENDPROC() declarations so the symbols are > annotated as functions, resulting in the linker to emit the appropriate > mode switches. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Acked-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/kernel/Makefile | 1 + > arch/arm/kernel/iwmmxt.S | 13 +++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile > index 4c5d1a5982dd..575a5e424b0a 100644 > --- a/arch/arm/kernel/Makefile > +++ b/arch/arm/kernel/Makefile > @@ -85,6 +85,7 @@ obj-$(CONFIG_CPU_PJ4B) += pj4-cp0.o > obj-$(CONFIG_IWMMXT) += iwmmxt.o > obj-$(CONFIG_PERF_EVENTS) += perf_regs.o > obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o perf_event_cpu.o > +CFLAGS_pj4-cp0.o := -marm > AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt > obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o > > diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S > index ad58e565fe98..49fadbda8c63 100644 > --- a/arch/arm/kernel/iwmmxt.S > +++ b/arch/arm/kernel/iwmmxt.S > @@ -58,6 +58,7 @@ > #define MMX_SIZE (0x98) > > .text > + .arm > > /* > * Lazy switching of Concan coprocessor context > @@ -182,6 +183,8 @@ concan_load: > tmcr wCon, r2 > ret lr > > +ENDPROC(iwmmxt_task_enable) > + > /* > * Back up Concan regs to save area and disable access to them > * (mainly for gdb or sleep mode usage) > @@ -232,6 +235,8 @@ ENTRY(iwmmxt_task_disable) > 1: msr cpsr_c, ip @ restore interrupt mode > ldmfd sp!, {r4, pc} > > +ENDPROC(iwmmxt_task_disable) > + > /* > * Copy Concan state to given memory address > * > @@ -268,6 +273,8 @@ ENTRY(iwmmxt_task_copy) > msr cpsr_c, ip @ restore interrupt mode > ret r3 > > +ENDPROC(iwmmxt_task_copy) > + > /* > * Restore Concan state from given memory address > * > @@ -304,6 +311,8 @@ ENTRY(iwmmxt_task_restore) > msr cpsr_c, ip @ restore interrupt mode > ret r3 > > +ENDPROC(iwmmxt_task_restore) > + > /* > * Concan handling on task switch > * > @@ -335,6 +344,8 @@ ENTRY(iwmmxt_task_switch) > mrc p15, 0, r1, c2, c0, 0 > sub pc, lr, r1, lsr #32 @ cpwait and return > > +ENDPROC(iwmmxt_task_switch) > + > /* > * Remove Concan ownership of given task > * > @@ -353,6 +364,8 @@ ENTRY(iwmmxt_task_release) > msr cpsr_c, r2 @ restore interrupts > ret lr > > +ENDPROC(iwmmxt_task_release) > + > .data > concan_owner: > .word 0 > -- > 1.8.3.2 > >
On 24 November 2014 at 16:29, Arnd Bergmann <arnd@arndb.de> wrote: > On Monday 24 November 2014 12:56:40 Ard Biesheuvel wrote: >> Two files that get included when building the multi_v7_defconfig target >> fail to build when selecting THUMB2_KERNEL for this configuration. >> >> In both cases, we can just build the file as ARM code, as none of its >> symbols are exported to modules, so there are no interworking concerns. >> In the iwmmxt.S case, add ENDPROC() declarations so the symbols are >> annotated as functions, resulting in the linker to emit the appropriate >> mode switches. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Ah, very nice. I tried this before, but my version didn't actually > work, presumably because I didn't know about the ENDPROC() stuff. > > Have you tested this on a machine that has IWMMXT enabled? > No, I don't have access to such a machine, unfortunately.
On 24 November 2014 at 18:36, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > Hi Arnd, > > On 24/11/2014 18:17, Arnd Bergmann wrote: >> On Monday 24 November 2014 16:34:47 Ard Biesheuvel wrote: >>> On 24 November 2014 at 16:29, Arnd Bergmann <arnd@arndb.de> wrote: >>>> On Monday 24 November 2014 12:56:40 Ard Biesheuvel wrote: >>>>> Two files that get included when building the multi_v7_defconfig target >>>>> fail to build when selecting THUMB2_KERNEL for this configuration. >>>>> >>>>> In both cases, we can just build the file as ARM code, as none of its >>>>> symbols are exported to modules, so there are no interworking concerns. >>>>> In the iwmmxt.S case, add ENDPROC() declarations so the symbols are >>>>> annotated as functions, resulting in the linker to emit the appropriate >>>>> mode switches. >>>>> >>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>> >>>> Ah, very nice. I tried this before, but my version didn't actually >>>> work, presumably because I didn't know about the ENDPROC() stuff. >>>> >>>> Have you tested this on a machine that has IWMMXT enabled? >>>> >>> >>> No, I don't have access to such a machine, unfortunately. >> >> Adding a few mvebu folks to Cc, maybe one of them can test your patch. > > Actually even it is a feature of the PJ4 machine non of the mvebu machines > currently use it. I only see this configuration enabled for the pxa family. > So I think you would have more feedback with the pxa maintainers/owners. > Are you saying PJ4 should be dropped from 'default' here? config IWMMXT bool "Enable iWMMXt support" depends on CPU_XSCALE || CPU_XSC3 || CPU_MOHAWK || CPU_PJ4 || CPU_PJ4B default y if PXA27x || PXA3xx || ARCH_MMP || CPU_PJ4 || CPU_PJ4B
On 24 November 2014 at 18:56, Olof Johansson <olof@lixom.net> wrote: > On Mon, Nov 24, 2014 at 9:17 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> On Monday 24 November 2014 16:34:47 Ard Biesheuvel wrote: >>> On 24 November 2014 at 16:29, Arnd Bergmann <arnd@arndb.de> wrote: >>> > On Monday 24 November 2014 12:56:40 Ard Biesheuvel wrote: >>> >> Two files that get included when building the multi_v7_defconfig target >>> >> fail to build when selecting THUMB2_KERNEL for this configuration. >>> >> >>> >> In both cases, we can just build the file as ARM code, as none of its >>> >> symbols are exported to modules, so there are no interworking concerns. >>> >> In the iwmmxt.S case, add ENDPROC() declarations so the symbols are >>> >> annotated as functions, resulting in the linker to emit the appropriate >>> >> mode switches. >>> >> >>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> > >>> > Ah, very nice. I tried this before, but my version didn't actually >>> > work, presumably because I didn't know about the ENDPROC() stuff. >>> > >>> > Have you tested this on a machine that has IWMMXT enabled? >>> > >>> >>> No, I don't have access to such a machine, unfortunately. >> >> Adding a few mvebu folks to Cc, maybe one of them can test your patch. >> It's also possible that Olof or Kevin have a PJ4 machine with iwmmxt >> in their boot farms. > > I have a Cubox with Dove, which does implement iWMMXt v2, based on > boot messages. > > I however do not have any kind of userspace programs that make use of > it to test with. > Well, I think the idea is to make sure it doesn't explode when running multi_v7_defconfig in thumb2 mode. That requires a machine that has the actual coprocessor, but as far as I can figure out from the code, this should not require any actual floating point use.
On Mon, 24 Nov 2014, Olof Johansson wrote: > On Mon, Nov 24, 2014 at 9:17 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Monday 24 November 2014 16:34:47 Ard Biesheuvel wrote: > >> On 24 November 2014 at 16:29, Arnd Bergmann <arnd@arndb.de> wrote: > >> > On Monday 24 November 2014 12:56:40 Ard Biesheuvel wrote: > >> >> Two files that get included when building the multi_v7_defconfig target > >> >> fail to build when selecting THUMB2_KERNEL for this configuration. > >> >> > >> >> In both cases, we can just build the file as ARM code, as none of its > >> >> symbols are exported to modules, so there are no interworking concerns. > >> >> In the iwmmxt.S case, add ENDPROC() declarations so the symbols are > >> >> annotated as functions, resulting in the linker to emit the appropriate > >> >> mode switches. > >> >> > >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> > > >> > Ah, very nice. I tried this before, but my version didn't actually > >> > work, presumably because I didn't know about the ENDPROC() stuff. > >> > > >> > Have you tested this on a machine that has IWMMXT enabled? > >> > > >> > >> No, I don't have access to such a machine, unfortunately. > > > > Adding a few mvebu folks to Cc, maybe one of them can test your patch. > > It's also possible that Olof or Kevin have a PJ4 machine with iwmmxt > > in their boot farms. > > I have a Cubox with Dove, which does implement iWMMXt v2, based on > boot messages. > > I however do not have any kind of userspace programs that make use of > it to test with. The housekeeping code will be called on task switch even if the coprocessor is not involved by user space. That's what needs testing i.e. that code is still callable and doesn't crash in a Thumb2 config. From the looks of it I wouldn't see any problem... but nothing beats actual testing. Nicolas
On 24 November 2014 at 20:01, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > On 24/11/2014 18:48, Ard Biesheuvel wrote: >> On 24 November 2014 at 18:36, Gregory CLEMENT >> <gregory.clement@free-electrons.com> wrote: >>> Hi Arnd, >>> >>> On 24/11/2014 18:17, Arnd Bergmann wrote: >>>> On Monday 24 November 2014 16:34:47 Ard Biesheuvel wrote: >>>>> On 24 November 2014 at 16:29, Arnd Bergmann <arnd@arndb.de> wrote: >>>>>> On Monday 24 November 2014 12:56:40 Ard Biesheuvel wrote: >>>>>>> Two files that get included when building the multi_v7_defconfig target >>>>>>> fail to build when selecting THUMB2_KERNEL for this configuration. >>>>>>> >>>>>>> In both cases, we can just build the file as ARM code, as none of its >>>>>>> symbols are exported to modules, so there are no interworking concerns. >>>>>>> In the iwmmxt.S case, add ENDPROC() declarations so the symbols are >>>>>>> annotated as functions, resulting in the linker to emit the appropriate >>>>>>> mode switches. >>>>>>> >>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>>>> >>>>>> Ah, very nice. I tried this before, but my version didn't actually >>>>>> work, presumably because I didn't know about the ENDPROC() stuff. >>>>>> >>>>>> Have you tested this on a machine that has IWMMXT enabled? >>>>>> >>>>> >>>>> No, I don't have access to such a machine, unfortunately. >>>> >>>> Adding a few mvebu folks to Cc, maybe one of them can test your patch. >>> >>> Actually even it is a feature of the PJ4 machine non of the mvebu machines >>> currently use it. I only see this configuration enabled for the pxa family. >>> So I think you would have more feedback with the pxa maintainers/owners. >>> >> >> Are you saying PJ4 should be dropped from 'default' here? > > Not at all. I didn't find it in the configuration file, but I didn't realized > it was because of this "default y". > > Sorry for the noise. > Ah, ok. In that case, could you perhaps also test this patch on a PJ4 machine with multi_v7_defconfig but with CONFIG_THUMB2_KERNEL enabled? Olof has already done so successfully, but just to be sure. Thanks, Ard.
On 24 November 2014 at 19:48, Olof Johansson <olof@lixom.net> wrote: > On Mon, Nov 24, 2014 at 10:28 AM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> On 24 November 2014 at 18:56, Olof Johansson <olof@lixom.net> wrote: >>> On Mon, Nov 24, 2014 at 9:17 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>>> On Monday 24 November 2014 16:34:47 Ard Biesheuvel wrote: >>>>> On 24 November 2014 at 16:29, Arnd Bergmann <arnd@arndb.de> wrote: >>>>> > On Monday 24 November 2014 12:56:40 Ard Biesheuvel wrote: >>>>> >> Two files that get included when building the multi_v7_defconfig target >>>>> >> fail to build when selecting THUMB2_KERNEL for this configuration. >>>>> >> >>>>> >> In both cases, we can just build the file as ARM code, as none of its >>>>> >> symbols are exported to modules, so there are no interworking concerns. >>>>> >> In the iwmmxt.S case, add ENDPROC() declarations so the symbols are >>>>> >> annotated as functions, resulting in the linker to emit the appropriate >>>>> >> mode switches. >>>>> >> >>>>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>>> > >>>>> > Ah, very nice. I tried this before, but my version didn't actually >>>>> > work, presumably because I didn't know about the ENDPROC() stuff. >>>>> > >>>>> > Have you tested this on a machine that has IWMMXT enabled? >>>>> > >>>>> >>>>> No, I don't have access to such a machine, unfortunately. >>>> >>>> Adding a few mvebu folks to Cc, maybe one of them can test your patch. >>>> It's also possible that Olof or Kevin have a PJ4 machine with iwmmxt >>>> in their boot farms. >>> >>> I have a Cubox with Dove, which does implement iWMMXt v2, based on >>> boot messages. >>> >>> I however do not have any kind of userspace programs that make use of >>> it to test with. >>> >> >> Well, I think the idea is to make sure it doesn't explode when running >> multi_v7_defconfig in thumb2 mode. That requires a machine that has >> the actual coprocessor, but as far as I can figure out from the code, >> this should not require any actual floating point use. > > Well, that's easy to confirm, and I've done so now. > > Not sure I'd claim that's worth a tested-by though, but whatever: > > Tested-by: Olof Johansson <olof@lixom.net> > Submitted as 8221/1 Thanks for testing, Ard.
On 25 November 2014 at 17:14, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > On 24/11/2014 20:49, Ard Biesheuvel wrote: >> On 24 November 2014 at 20:01, Gregory CLEMENT >> <gregory.clement@free-electrons.com> wrote: >>> On 24/11/2014 18:48, Ard Biesheuvel wrote: >>>> On 24 November 2014 at 18:36, Gregory CLEMENT >>>> <gregory.clement@free-electrons.com> wrote: >>>>> Hi Arnd, >>>>> >>>>> On 24/11/2014 18:17, Arnd Bergmann wrote: >>>>>> On Monday 24 November 2014 16:34:47 Ard Biesheuvel wrote: >>>>>>> On 24 November 2014 at 16:29, Arnd Bergmann <arnd@arndb.de> wrote: >>>>>>>> On Monday 24 November 2014 12:56:40 Ard Biesheuvel wrote: >>>>>>>>> Two files that get included when building the multi_v7_defconfig target >>>>>>>>> fail to build when selecting THUMB2_KERNEL for this configuration. >>>>>>>>> >>>>>>>>> In both cases, we can just build the file as ARM code, as none of its >>>>>>>>> symbols are exported to modules, so there are no interworking concerns. >>>>>>>>> In the iwmmxt.S case, add ENDPROC() declarations so the symbols are >>>>>>>>> annotated as functions, resulting in the linker to emit the appropriate >>>>>>>>> mode switches. >>>>>>>>> >>>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>>>>>> >>>>>>>> Ah, very nice. I tried this before, but my version didn't actually >>>>>>>> work, presumably because I didn't know about the ENDPROC() stuff. >>>>>>>> >>>>>>>> Have you tested this on a machine that has IWMMXT enabled? >>>>>>>> >>>>>>> >>>>>>> No, I don't have access to such a machine, unfortunately. >>>>>> >>>>>> Adding a few mvebu folks to Cc, maybe one of them can test your patch. >>>>> >>>>> Actually even it is a feature of the PJ4 machine non of the mvebu machines >>>>> currently use it. I only see this configuration enabled for the pxa family. >>>>> So I think you would have more feedback with the pxa maintainers/owners. >>>>> >>>> >>>> Are you saying PJ4 should be dropped from 'default' here? >>> >>> Not at all. I didn't find it in the configuration file, but I didn't realized >>> it was because of this "default y". >>> >>> Sorry for the noise. >>> >> >> Ah, ok. >> >> In that case, could you perhaps also test this patch on a PJ4 machine >> with multi_v7_defconfig but with CONFIG_THUMB2_KERNEL enabled? > > Actually as explained in the commit "ARM: 8040/1: pj4: properly detect > existence of the iWMMXt co-processor", the Armada 370 and Armada XP have > a PJ4B CPU without the iWMMXt extension. The only boards I have using CPus > belonging to the PJ4 family use these SoCs so I wasn't able to fully > tested it. At least I can say that these SoC are now buildable in Thumb > mode with this patch and I didn't see any regression while booting the > system compiled in Thumb2. > OK. If the iWMMXt code is never actually called, testing on such a SoC doesn't make a lot of sense But Olof's test should be sufficient. Thanks, Ard.
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index 4c5d1a5982dd..575a5e424b0a 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -85,6 +85,7 @@ obj-$(CONFIG_CPU_PJ4B) += pj4-cp0.o obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_PERF_EVENTS) += perf_regs.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o perf_event_cpu.o +CFLAGS_pj4-cp0.o := -marm AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S index ad58e565fe98..49fadbda8c63 100644 --- a/arch/arm/kernel/iwmmxt.S +++ b/arch/arm/kernel/iwmmxt.S @@ -58,6 +58,7 @@ #define MMX_SIZE (0x98) .text + .arm /* * Lazy switching of Concan coprocessor context @@ -182,6 +183,8 @@ concan_load: tmcr wCon, r2 ret lr +ENDPROC(iwmmxt_task_enable) + /* * Back up Concan regs to save area and disable access to them * (mainly for gdb or sleep mode usage) @@ -232,6 +235,8 @@ ENTRY(iwmmxt_task_disable) 1: msr cpsr_c, ip @ restore interrupt mode ldmfd sp!, {r4, pc} +ENDPROC(iwmmxt_task_disable) + /* * Copy Concan state to given memory address * @@ -268,6 +273,8 @@ ENTRY(iwmmxt_task_copy) msr cpsr_c, ip @ restore interrupt mode ret r3 +ENDPROC(iwmmxt_task_copy) + /* * Restore Concan state from given memory address * @@ -304,6 +311,8 @@ ENTRY(iwmmxt_task_restore) msr cpsr_c, ip @ restore interrupt mode ret r3 +ENDPROC(iwmmxt_task_restore) + /* * Concan handling on task switch * @@ -335,6 +344,8 @@ ENTRY(iwmmxt_task_switch) mrc p15, 0, r1, c2, c0, 0 sub pc, lr, r1, lsr #32 @ cpwait and return +ENDPROC(iwmmxt_task_switch) + /* * Remove Concan ownership of given task * @@ -353,6 +364,8 @@ ENTRY(iwmmxt_task_release) msr cpsr_c, r2 @ restore interrupts ret lr +ENDPROC(iwmmxt_task_release) + .data concan_owner: .word 0
Two files that get included when building the multi_v7_defconfig target fail to build when selecting THUMB2_KERNEL for this configuration. In both cases, we can just build the file as ARM code, as none of its symbols are exported to modules, so there are no interworking concerns. In the iwmmxt.S case, add ENDPROC() declarations so the symbols are annotated as functions, resulting in the linker to emit the appropriate mode switches. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm/kernel/Makefile | 1 + arch/arm/kernel/iwmmxt.S | 13 +++++++++++++ 2 files changed, 14 insertions(+)