[PULL REQ] Big Endian initial patch series

[PULL REQ] Big Endian initial patch series

[Ben Dooks]

This series has been well tested and it would be great to get this
merged now. So here is the pull request:

The following changes since commit 8b5ede69d24db939f52b47effff2f6fe1e83e08b:

  powerpc/irq: Don't switch to irq stack from softirq stack (2013-10-07 14:19:39 -0700)

are available in the git repository at:

  ssh://git at git.baserock.org/delta/linux baserock/bjdooks/312-rc4/be/core-v2

for you to fetch changes up to 41c4973a021d57e85f49f2c43311f6a327be4fa9:

  ARM: cci driver need big endian fixes in asm code (2013-10-18 21:43:34 +0100)

----------------------------------------------------------------
Ben Dooks (18):
      ARM: fix ARCH_IXP4xx usage of ARCH_SUPPORTS_BIG_ENDIAN
      ARM: asm: Add ARM_BE8() assembly helper
      ARM: fixup_pv_table bug when CPU_ENDIAN_BE8
      ARM: set BE8 if LE in head code
      ARM: pl01x debug code endian fix
      ARM: twd: data endian fix
      ARM: smp_scu: data endian fixes
      ARM: highbank: enable big-endian
      ARM: mvebu: support running big-endian
      ARM: vexpress: add big endian support
      ARM: alignment: correctly decode instructions in BE8 mode.
      ARM: traps: use <asm/opcodes.h> to get correct instruction order
      ARM: module: correctly relocate instructions in BE8
      ARM: set --be8 when linking modules
      ARM: hardware: fix endian-ness in <hardware/coresight.h>
      ARM: net: fix arm instruction endian-ness in bpf_jit_32.c
      ARM: Correct BUG() assembly to ensure it is endian-agnostic
      ARM: kdgb: use <asm/opcodes.h> for data to be assembled as intruction

Victor Kamensky (5):
      ARM: atomic64: fix endian-ness in atomic.h
      ARM: signal: sigreturn_codes should be endian neutral to work in BE8
      ARM: mcpm: fix big endian issue in mcpm startup code
      ARM: tlb: ASID macro should give 32bit result for BE correct operation
      ARM: cci driver need big endian fixes in asm code

 arch/arm/Kconfig                          |  1 +
 arch/arm/Makefile                         |  1 +
 arch/arm/boot/compressed/head.S           |  9 ++--
 arch/arm/common/mcpm_head.S               |  2 +
 arch/arm/include/asm/assembler.h          |  7 +++
 arch/arm/include/asm/atomic.h             | 26 +++++-----
 arch/arm/include/asm/bug.h                | 10 ++--
 arch/arm/include/asm/hardware/coresight.h |  8 ++--
 arch/arm/include/asm/kgdb.h               |  3 +-
 arch/arm/include/asm/mmu.h                |  2 +-
 arch/arm/include/debug/pl01x.S            |  2 +
 arch/arm/kernel/Makefile                  |  3 +-
 arch/arm/kernel/entry-armv.S              |  5 +-
 arch/arm/kernel/entry-common.S            |  4 +-
 arch/arm/kernel/head.S                    | 12 +++++
 arch/arm/kernel/module.c                  | 57 +++++++++++++---------
 arch/arm/kernel/signal.c                  | 24 +---------
 arch/arm/kernel/sigreturn_codes.S         | 80 +++++++++++++++++++++++++++++++
 arch/arm/kernel/sleep.S                   |  1 +
 arch/arm/kernel/smp_scu.c                 | 14 +++---
 arch/arm/kernel/smp_twd.c                 | 24 +++++-----
 arch/arm/kernel/traps.c                   | 24 ++++++----
 arch/arm/mach-highbank/Kconfig            |  1 +
 arch/arm/mach-ixp4xx/Kconfig              |  4 --
 arch/arm/mach-mvebu/Kconfig               |  1 +
 arch/arm/mach-mvebu/coherency_ll.S        |  3 ++
 arch/arm/mach-mvebu/headsmp.S             |  4 ++
 arch/arm/mach-vexpress/Kconfig            |  1 +
 arch/arm/mm/Kconfig                       |  6 +++
 arch/arm/mm/abort-ev6.S                   |  5 +-
 arch/arm/mm/alignment.c                   |  9 +++-
 arch/arm/mm/proc-v6.S                     |  4 +-
 arch/arm/mm/proc-v7.S                     |  4 +-
 arch/arm/net/bpf_jit_32.c                 |  6 ++-
 arch/arm/plat-versatile/headsmp.S         |  2 +
 drivers/bus/arm-cci.c                     |  6 ++-
 36 files changed, 247 insertions(+), 128 deletions(-)
 create mode 100644 arch/arm/kernel/sigreturn_codes.S

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[PULL REQ] Big Endian initial patch series

[Will Deacon]

Hi Ben,

A few nits here...

Firstly, I'd suggest using [GIT PULL] instead of [PULL REQ] in the subject.
Many people filter on the first, so you might get missed with the latter.

On Fri, Oct 18, 2013 at 09:54:05PM +0100, Ben Dooks wrote:
> This series has been well tested and it would be great to get this
> merged now. So here is the pull request:
> 
> The following changes since commit 8b5ede69d24db939f52b47effff2f6fe1e83e08b:
> 
>   powerpc/irq: Don't switch to irq stack from softirq stack (2013-10-07 14:19:39 -0700)

Secondly, I'd suggest basing against a specific tag in mainline if possible,
rather than a random commit. I think -rc3 matches rmk's current
devel-stable branch.

> are available in the git repository at:
> 
>   ssh://git at git.baserock.org/delta/linux baserock/bjdooks/312-rc4/be/core-v2

This is an ssh URL, so it can't be pulled without some form of
authentication! Use git:// instead for pull requests.

It's also good form to put your pull requests on separate branches from what
you have been using for development, to make it clear that it is stable.
Another method is to use a signed tag (although this seems more common with
arm-soc than others).

Do you think you could send another pull request please?

Cheers,

Will

[PULL REQ] Big Endian initial patch series

[Ben Dooks]

On 19/10/13 18:09, Will Deacon wrote:
> Hi Ben,
>
> A few nits here...
>
> Firstly, I'd suggest using [GIT PULL] instead of [PULL REQ] in the subject.
> Many people filter on the first, so you might get missed with the latter.
>
> On Fri, Oct 18, 2013 at 09:54:05PM +0100, Ben Dooks wrote:
>> This series has been well tested and it would be great to get this
>> merged now. So here is the pull request:
>>
>> The following changes since commit 8b5ede69d24db939f52b47effff2f6fe1e83e08b:
>>
>>    powerpc/irq: Don't switch to irq stack from softirq stack (2013-10-07 14:19:39 -0700)
>
> Secondly, I'd suggest basing against a specific tag in mainline if possible,
> rather than a random commit. I think -rc3 matches rmk's current
> devel-stable branch.
>
>> are available in the git repository at:
>>
>>    ssh://git at git.baserock.org/delta/linux baserock/bjdooks/312-rc4/be/core-v2
>
> This is an ssh URL, so it can't be pulled without some form of
> authentication! Use git:// instead for pull requests.
>
> It's also good form to put your pull requests on separate branches from what
> you have been using for development, to make it clear that it is stable.
> Another method is to use a signed tag (although this seems more common with
> arm-soc than others).
>
> Do you think you could send another pull request please?

Ok, sorted.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[PULL REQ] Big Endian initial patch series

[Russell King - ARM Linux]

On Sat, Oct 19, 2013 at 08:51:35PM +0100, Ben Dooks wrote:
> On 19/10/13 18:09, Will Deacon wrote:
>> Hi Ben,
>>
>> A few nits here...
>>
>> Firstly, I'd suggest using [GIT PULL] instead of [PULL REQ] in the subject.
>> Many people filter on the first, so you might get missed with the latter.
>>
>> On Fri, Oct 18, 2013 at 09:54:05PM +0100, Ben Dooks wrote:
>>> This series has been well tested and it would be great to get this
>>> merged now. So here is the pull request:
>>>
>>> The following changes since commit 8b5ede69d24db939f52b47effff2f6fe1e83e08b:
>>>
>>>    powerpc/irq: Don't switch to irq stack from softirq stack (2013-10-07 14:19:39 -0700)
>>
>> Secondly, I'd suggest basing against a specific tag in mainline if possible,
>> rather than a random commit. I think -rc3 matches rmk's current
>> devel-stable branch.
>>
>>> are available in the git repository at:
>>>
>>>    ssh://git at git.baserock.org/delta/linux baserock/bjdooks/312-rc4/be/core-v2
>>
>> This is an ssh URL, so it can't be pulled without some form of
>> authentication! Use git:// instead for pull requests.
>>
>> It's also good form to put your pull requests on separate branches from what
>> you have been using for development, to make it clear that it is stable.
>> Another method is to use a signed tag (although this seems more common with
>> arm-soc than others).
>>
>> Do you think you could send another pull request please?
>
> Ok, sorted.

Pulled, but there was a conflict.  Please check this resolution (it's
copy'n'pasted).  I'll probably be in linux-next tomorrow in any case,
but any mistake here can be fixed.

Second thing, I prefer that pull requests have some blurb in them
describing what they are and the changes.  This makes composing my
pull request to Linus easier.

diff --cc arch/arm/kernel/head.S
index 54547947a4e9,a047acfa6b6d..000000000000
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@@ -602,28 -586,26 +606,39 @@@ __fixup_a_pv_table
        b       2f
  1:    add     r7, r3
        ldrh    ip, [r7, #2]
+ ARM_BE8(rev16 ip, ip)
 -      and     ip, 0x8f00
 -      orr     ip, r6  @ mask in offset bits 31-24
 +      tst     ip, #0x4000
 +      and     ip, #0x8f00
 +      orrne   ip, r6  @ mask in offset bits 31-24
 +      orreq   ip, r0  @ mask in offset bits 7-0
+ ARM_BE8(rev16 ip, ip)
        strh    ip, [r7, #2]
 +      ldrheq  ip, [r7]
 +      biceq   ip, #0x20
 +      orreq   ip, ip, r0, lsr #16
 +      strheq  ip, [r7]
  2:    cmp     r4, r5
        ldrcc   r7, [r4], #4    @ use branch for delay slot
        bcc     1b
        bx      lr
  #else
 +      moveq   r0, #0x400000   @ set bit 22, mov to mvn instruction
        b       2f
  1:    ldr     ip, [r7, r3]
+ #ifdef CONFIG_CPU_ENDIAN_BE8
+       @ in BE8, we load data in BE, but instructions still in LE
+       bic     ip, ip, #0xff000000
 -      orr     ip, ip, r6, lsl#24
++      tst     ip, #0x000f0000 @ check the rotation field
++      orrne   ip, ip, r6, lsl #24 @ mask in offset bits 31-24
++      biceq   ip, ip, #0x00004000 @ clear bit 22
++      orreq   ip, ip, r0, lsl #24 @ mask in offset bits 7-0
+ #else
        bic     ip, ip, #0x000000ff
 -      orr     ip, ip, r6      @ mask in offset bits 31-24
 +      tst     ip, #0xf00      @ check the rotation field
 +      orrne   ip, ip, r6      @ mask in offset bits 31-24
 +      biceq   ip, ip, #0x400000       @ clear bit 22
 +      orreq   ip, ip, r0      @ mask in offset bits 7-0
+ #endif
        str     ip, [r7, r3]
  2:    cmp     r4, r5
        ldrcc   r7, [r4], #4    @ use branch for delay slot

[PULL REQ] Big Endian initial patch series

[Will Deacon]

Hi Russell,

On Mon, Oct 28, 2013 at 12:47:36AM +0000, Russell King - ARM Linux wrote:
> On Sat, Oct 19, 2013 at 08:51:35PM +0100, Ben Dooks wrote:
> > On 19/10/13 18:09, Will Deacon wrote:
> >> Do you think you could send another pull request please?
> >
> > Ok, sorted.
> 
> Pulled, but there was a conflict.  Please check this resolution (it's
> copy'n'pasted).  I'll probably be in linux-next tomorrow in any case,
> but any mistake here can be fixed.

This doesn't look quite right to me, but unfortunately I'm going be spending
most (all?) of today trying to catch a flight out of the UK. Hopefully Dave
or Ben can investigate further, but comments below.

> diff --cc arch/arm/kernel/head.S
> index 54547947a4e9,a047acfa6b6d..000000000000
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@@ -602,28 -586,26 +606,39 @@@ __fixup_a_pv_table
>         b       2f
>   1:    add     r7, r3
>         ldrh    ip, [r7, #2]
> + ARM_BE8(rev16 ip, ip)
>  -      and     ip, 0x8f00
>  -      orr     ip, r6  @ mask in offset bits 31-24
>  +      tst     ip, #0x4000
>  +      and     ip, #0x8f00
>  +      orrne   ip, r6  @ mask in offset bits 31-24
>  +      orreq   ip, r0  @ mask in offset bits 7-0
> + ARM_BE8(rev16 ip, ip)
>         strh    ip, [r7, #2]
>  +      ldrheq  ip, [r7]
>  +      biceq   ip, #0x20
>  +      orreq   ip, ip, r0, lsr #16
>  +      strheq  ip, [r7]

There are new halfword accesses here without any conditional revs.

>   2:    cmp     r4, r5
>         ldrcc   r7, [r4], #4    @ use branch for delay slot
>         bcc     1b
>         bx      lr
>   #else
>  +      moveq   r0, #0x400000   @ set bit 22, mov to mvn instruction
>         b       2f
>   1:    ldr     ip, [r7, r3]
> + #ifdef CONFIG_CPU_ENDIAN_BE8
> +       @ in BE8, we load data in BE, but instructions still in LE
> +       bic     ip, ip, #0xff000000
>  -      orr     ip, ip, r6, lsl#24
> ++      tst     ip, #0x000f0000 @ check the rotation field

Since that orr with shift has been removed, I think the masks for the BE
case are now incorrect...

> ++      orrne   ip, ip, r6, lsl #24 @ mask in offset bits 31-24
> ++      biceq   ip, ip, #0x00004000 @ clear bit 22
> ++      orreq   ip, ip, r0, lsl #24 @ mask in offset bits 7-0
> + #else
>         bic     ip, ip, #0x000000ff
>  -      orr     ip, ip, r6      @ mask in offset bits 31-24
>  +      tst     ip, #0xf00      @ check the rotation field
>  +      orrne   ip, ip, r6      @ mask in offset bits 31-24
>  +      biceq   ip, ip, #0x400000       @ clear bit 22

...which seems to be confirmed by the updated LE code (everything is off
by a byte).

Somebody should probably sit down with the conflicting patch and port the BE
changes over. I think the relevant patch is "ARM: mm: Correct virt_to_phys
patching for 64 bit physical addresses". In fact, looking at *that* patch,
it's *also* broken for BE! It adds the following to head.S:

+#ifdef __ARMEB_
+#define LOW_OFFSET     0x4
+#define HIGH_OFFSET    0x0
+#else
+#define LOW_OFFSET     0x0
+#define HIGH_OFFSET    0x4
+#endif

(spot the missing underscore).

So I guess that means somebody should test whatever the resulting merge
ends up being too...

Will

[PULL REQ] Big Endian initial patch series

[Russell King - ARM Linux]

On Mon, Oct 28, 2013 at 08:44:55AM +0000, Will Deacon wrote:
> Hi Russell,
> 
> On Mon, Oct 28, 2013 at 12:47:36AM +0000, Russell King - ARM Linux wrote:
> > On Sat, Oct 19, 2013 at 08:51:35PM +0100, Ben Dooks wrote:
> > > On 19/10/13 18:09, Will Deacon wrote:
> > >> Do you think you could send another pull request please?
> > >
> > > Ok, sorted.
> > 
> > Pulled, but there was a conflict.  Please check this resolution (it's
> > copy'n'pasted).  I'll probably be in linux-next tomorrow in any case,
> > but any mistake here can be fixed.
> 
> This doesn't look quite right to me, but unfortunately I'm going be spending
> most (all?) of today trying to catch a flight out of the UK. Hopefully Dave
> or Ben can investigate further, but comments below.
> 
> > diff --cc arch/arm/kernel/head.S
> > index 54547947a4e9,a047acfa6b6d..000000000000
> > --- a/arch/arm/kernel/head.S
> > +++ b/arch/arm/kernel/head.S
> > @@@ -602,28 -586,26 +606,39 @@@ __fixup_a_pv_table
> >         b       2f
> >   1:    add     r7, r3
> >         ldrh    ip, [r7, #2]
> > + ARM_BE8(rev16 ip, ip)
> >  -      and     ip, 0x8f00
> >  -      orr     ip, r6  @ mask in offset bits 31-24
> >  +      tst     ip, #0x4000
> >  +      and     ip, #0x8f00
> >  +      orrne   ip, r6  @ mask in offset bits 31-24
> >  +      orreq   ip, r0  @ mask in offset bits 7-0
> > + ARM_BE8(rev16 ip, ip)
> >         strh    ip, [r7, #2]
> >  +      ldrheq  ip, [r7]
> >  +      biceq   ip, #0x20
> >  +      orreq   ip, ip, r0, lsr #16
> >  +      strheq  ip, [r7]
> 
> There are new halfword accesses here without any conditional revs.

Yes, I missed this one.

> > + #ifdef CONFIG_CPU_ENDIAN_BE8
> > +       @ in BE8, we load data in BE, but instructions still in LE
> > +       bic     ip, ip, #0xff000000
> >  -      orr     ip, ip, r6, lsl#24
> > ++      tst     ip, #0x000f0000 @ check the rotation field
> 
> Since that orr with shift has been removed, I think the masks for the BE
> case are now incorrect...
> 
> > ++      orrne   ip, ip, r6, lsl #24 @ mask in offset bits 31-24
> > ++      biceq   ip, ip, #0x00004000 @ clear bit 22
> > ++      orreq   ip, ip, r0, lsl #24 @ mask in offset bits 7-0

Actually, look closer.  It became the orrne here.

> > + #else
> >         bic     ip, ip, #0x000000ff
> >  -      orr     ip, ip, r6      @ mask in offset bits 31-24
> >  +      tst     ip, #0xf00      @ check the rotation field
> >  +      orrne   ip, ip, r6      @ mask in offset bits 31-24
> >  +      biceq   ip, ip, #0x400000       @ clear bit 22
> 
> ...which seems to be confirmed by the updated LE code (everything is off
> by a byte).

The LE code was left unaltered from Santosh's patch, so that should be
correct.  I just did an endian conversion to the BE case.

> Somebody should probably sit down with the conflicting patch and port the BE
> changes over. I think the relevant patch is "ARM: mm: Correct virt_to_phys
> patching for 64 bit physical addresses". In fact, looking at *that* patch,
> it's *also* broken for BE! It adds the following to head.S:
> 
> +#ifdef __ARMEB_
> +#define LOW_OFFSET     0x4
> +#define HIGH_OFFSET    0x0
> +#else
> +#define LOW_OFFSET     0x0
> +#define HIGH_OFFSET    0x4
> +#endif
> 
> (spot the missing underscore).

Yep, well spotted.

Well, we have some time to get this all fixed, so I'm going to drop
Ben's tree.  I think we need to first commit a patch to fix the error
in Santosh's patch.

[PULL REQ] Big Endian initial patch series

[Sricharan R]

Hi,
On Monday 28 October 2013 02:23 PM, Russell King - ARM Linux wrote:
> On Mon, Oct 28, 2013 at 08:44:55AM +0000, Will Deacon wrote:
>> Hi Russell,
>>
>> On Mon, Oct 28, 2013 at 12:47:36AM +0000, Russell King - ARM Linux wrote:
>>> On Sat, Oct 19, 2013 at 08:51:35PM +0100, Ben Dooks wrote:
>>>> On 19/10/13 18:09, Will Deacon wrote:
>>>>> Do you think you could send another pull request please?
>>>> Ok, sorted.
>>> Pulled, but there was a conflict.  Please check this resolution (it's
>>> copy'n'pasted).  I'll probably be in linux-next tomorrow in any case,
>>> but any mistake here can be fixed.
>> This doesn't look quite right to me, but unfortunately I'm going be spending
>> most (all?) of today trying to catch a flight out of the UK. Hopefully Dave
>> or Ben can investigate further, but comments below.
>>
>>> diff --cc arch/arm/kernel/head.S
>>> index 54547947a4e9,a047acfa6b6d..000000000000
>>> --- a/arch/arm/kernel/head.S
>>> +++ b/arch/arm/kernel/head.S
>>> @@@ -602,28 -586,26 +606,39 @@@ __fixup_a_pv_table
>>>         b       2f
>>>   1:    add     r7, r3
>>>         ldrh    ip, [r7, #2]
>>> + ARM_BE8(rev16 ip, ip)
>>>  -      and     ip, 0x8f00
>>>  -      orr     ip, r6  @ mask in offset bits 31-24
>>>  +      tst     ip, #0x4000
>>>  +      and     ip, #0x8f00
>>>  +      orrne   ip, r6  @ mask in offset bits 31-24
>>>  +      orreq   ip, r0  @ mask in offset bits 7-0
>>> + ARM_BE8(rev16 ip, ip)
>>>         strh    ip, [r7, #2]
>>>  +      ldrheq  ip, [r7]
>>>  +      biceq   ip, #0x20
>>>  +      orreq   ip, ip, r0, lsr #16
>>>  +      strheq  ip, [r7]
>> There are new halfword accesses here without any conditional revs.
> Yes, I missed this one.
>
>>> + #ifdef CONFIG_CPU_ENDIAN_BE8
>>> +       @ in BE8, we load data in BE, but instructions still in LE
>>> +       bic     ip, ip, #0xff000000
>>>  -      orr     ip, ip, r6, lsl#24
>>> ++      tst     ip, #0x000f0000 @ check the rotation field
>> Since that orr with shift has been removed, I think the masks for the BE
>> case are now incorrect...
>>
>>> ++      orrne   ip, ip, r6, lsl #24 @ mask in offset bits 31-24
>>> ++      biceq   ip, ip, #0x00004000 @ clear bit 22
>>> ++      orreq   ip, ip, r0, lsl #24 @ mask in offset bits 7-0
> Actually, look closer.  It became the orrne here.
>
>>> + #else
>>>         bic     ip, ip, #0x000000ff
>>>  -      orr     ip, ip, r6      @ mask in offset bits 31-24
>>>  +      tst     ip, #0xf00      @ check the rotation field
>>>  +      orrne   ip, ip, r6      @ mask in offset bits 31-24
>>>  +      biceq   ip, ip, #0x400000       @ clear bit 22
>> ...which seems to be confirmed by the updated LE code (everything is off
>> by a byte).
> The LE code was left unaltered from Santosh's patch, so that should be
> correct.  I just did an endian conversion to the BE case.
>
>> Somebody should probably sit down with the conflicting patch and port the BE
>> changes over. I think the relevant patch is "ARM: mm: Correct virt_to_phys
>> patching for 64 bit physical addresses". In fact, looking at *that* patch,
>> it's *also* broken for BE! It adds the following to head.S:
>>
>> +#ifdef __ARMEB_
>> +#define LOW_OFFSET     0x4
>> +#define HIGH_OFFSET    0x0
>> +#else
>> +#define LOW_OFFSET     0x0
>> +#define HIGH_OFFSET    0x4
>> +#endif
>>
>> (spot the missing underscore).
> Yep, well spotted.
>
> Well, we have some time to get this all fixed, so I'm going to drop
> Ben's tree.  I think we need to first commit a patch to fix the error
> in Santosh's patch.
Sorry, I will send a patch fix this missing underscore bug.

Regards,
 Sricharan

[PULL REQ] Big Endian initial patch series

[Ben Dooks]

On 28/10/13 08:53, Russell King - ARM Linux wrote:

>> +#ifdef __ARMEB_
>> +#define LOW_OFFSET     0x4
>> +#define HIGH_OFFSET    0x0
>> +#else
>> +#define LOW_OFFSET     0x0
>> +#define HIGH_OFFSET    0x4
>> +#endif
>>
>> (spot the missing underscore).
>
> Yep, well spotted.
>
> Well, we have some time to get this all fixed, so I'm going to drop
> Ben's tree.  I think we need to first commit a patch to fix the error
> in Santosh's patch

Ok, shall I look at re-basing to -rc6?

I have a bug report with a change to the SCU reading for the SMP
fix-up case which could do with fixing, so I could add this and send
another pull request (complete with description this time).

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[PULL REQ] Big Endian initial patch series

[Victor Kamensky]

Hi Sricharan,

Another problem with f52bb72 commit is missing .align at
the end of __fixup_a_pv_table function. In case of thumb2
kernel address at label 3 could be 2 bytes aligned and
cause unaligned access exception.

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 2b3e981..8b03c2c 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -662,6 +662,7 @@ ARM_BE8(rev ip, ip)
 #endif
 ENDPROC(__fixup_a_pv_table)

+       .align
 3:     .long __pv_offset

 ENTRY(fixup_pv_table)

It may work now because it is accidentally 4 bytes aligned
but it could change as code evolves. This happened to
me while I tried to work out how to deal with this code
in BE case (I am still working on that).

Thanks,
Victor


On 28 October 2013 02:12, Sricharan R <r.sricharan@ti.com> wrote:
> Hi,
> On Monday 28 October 2013 02:23 PM, Russell King - ARM Linux wrote:
>> On Mon, Oct 28, 2013 at 08:44:55AM +0000, Will Deacon wrote:
>>> Hi Russell,
>>>
>>> On Mon, Oct 28, 2013 at 12:47:36AM +0000, Russell King - ARM Linux wrote:
>>>> On Sat, Oct 19, 2013 at 08:51:35PM +0100, Ben Dooks wrote:
>>>>> On 19/10/13 18:09, Will Deacon wrote:
>>>>>> Do you think you could send another pull request please?
>>>>> Ok, sorted.
>>>> Pulled, but there was a conflict.  Please check this resolution (it's
>>>> copy'n'pasted).  I'll probably be in linux-next tomorrow in any case,
>>>> but any mistake here can be fixed.
>>> This doesn't look quite right to me, but unfortunately I'm going be spending
>>> most (all?) of today trying to catch a flight out of the UK. Hopefully Dave
>>> or Ben can investigate further, but comments below.
>>>
>>>> diff --cc arch/arm/kernel/head.S
>>>> index 54547947a4e9,a047acfa6b6d..000000000000
>>>> --- a/arch/arm/kernel/head.S
>>>> +++ b/arch/arm/kernel/head.S
>>>> @@@ -602,28 -586,26 +606,39 @@@ __fixup_a_pv_table
>>>>         b       2f
>>>>   1:    add     r7, r3
>>>>         ldrh    ip, [r7, #2]
>>>> + ARM_BE8(rev16 ip, ip)
>>>>  -      and     ip, 0x8f00
>>>>  -      orr     ip, r6  @ mask in offset bits 31-24
>>>>  +      tst     ip, #0x4000
>>>>  +      and     ip, #0x8f00
>>>>  +      orrne   ip, r6  @ mask in offset bits 31-24
>>>>  +      orreq   ip, r0  @ mask in offset bits 7-0
>>>> + ARM_BE8(rev16 ip, ip)
>>>>         strh    ip, [r7, #2]
>>>>  +      ldrheq  ip, [r7]
>>>>  +      biceq   ip, #0x20
>>>>  +      orreq   ip, ip, r0, lsr #16
>>>>  +      strheq  ip, [r7]
>>> There are new halfword accesses here without any conditional revs.
>> Yes, I missed this one.
>>
>>>> + #ifdef CONFIG_CPU_ENDIAN_BE8
>>>> +       @ in BE8, we load data in BE, but instructions still in LE
>>>> +       bic     ip, ip, #0xff000000
>>>>  -      orr     ip, ip, r6, lsl#24
>>>> ++      tst     ip, #0x000f0000 @ check the rotation field
>>> Since that orr with shift has been removed, I think the masks for the BE
>>> case are now incorrect...
>>>
>>>> ++      orrne   ip, ip, r6, lsl #24 @ mask in offset bits 31-24
>>>> ++      biceq   ip, ip, #0x00004000 @ clear bit 22
>>>> ++      orreq   ip, ip, r0, lsl #24 @ mask in offset bits 7-0
>> Actually, look closer.  It became the orrne here.
>>
>>>> + #else
>>>>         bic     ip, ip, #0x000000ff
>>>>  -      orr     ip, ip, r6      @ mask in offset bits 31-24
>>>>  +      tst     ip, #0xf00      @ check the rotation field
>>>>  +      orrne   ip, ip, r6      @ mask in offset bits 31-24
>>>>  +      biceq   ip, ip, #0x400000       @ clear bit 22
>>> ...which seems to be confirmed by the updated LE code (everything is off
>>> by a byte).
>> The LE code was left unaltered from Santosh's patch, so that should be
>> correct.  I just did an endian conversion to the BE case.
>>
>>> Somebody should probably sit down with the conflicting patch and port the BE
>>> changes over. I think the relevant patch is "ARM: mm: Correct virt_to_phys
>>> patching for 64 bit physical addresses". In fact, looking at *that* patch,
>>> it's *also* broken for BE! It adds the following to head.S:
>>>
>>> +#ifdef __ARMEB_
>>> +#define LOW_OFFSET     0x4
>>> +#define HIGH_OFFSET    0x0
>>> +#else
>>> +#define LOW_OFFSET     0x0
>>> +#define HIGH_OFFSET    0x4
>>> +#endif
>>>
>>> (spot the missing underscore).
>> Yep, well spotted.
>>
>> Well, we have some time to get this all fixed, so I'm going to drop
>> Ben's tree.  I think we need to first commit a patch to fix the error
>> in Santosh's patch.
> Sorry, I will send a patch fix this missing underscore bug.
>
> Regards,
>  Sricharan

[PULL REQ] Big Endian initial patch series

[Sricharan R]

Hi,
On Tuesday 29 October 2013 10:39 AM, Victor Kamensky wrote:
> Hi Sricharan,
>
> Another problem with f52bb72 commit is missing .align at
> the end of __fixup_a_pv_table function. In case of thumb2
> kernel address at label 3 could be 2 bytes aligned and
> cause unaligned access exception.
>
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 2b3e981..8b03c2c 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -662,6 +662,7 @@ ARM_BE8(rev ip, ip)
>  #endif
>  ENDPROC(__fixup_a_pv_table)
>
> +       .align
>  3:     .long __pv_offset
>
>  ENTRY(fixup_pv_table)
>
> It may work now because it is accidentally 4 bytes aligned
> but it could change as code evolves. This happened to
> me while I tried to work out how to deal with this code
> in BE case (I am still working on that).
 Ok, then even this is missing i think. We did not see
 any issue because as you said it was aligned on 4 byte.
 I will fix this as well in the previous patch.

Regards,
 Sricharan
> Thanks,
> Victor
>
>
> On 28 October 2013 02:12, Sricharan R <r.sricharan@ti.com> wrote:
>> Hi,
>> On Monday 28 October 2013 02:23 PM, Russell King - ARM Linux wrote:
>>> On Mon, Oct 28, 2013 at 08:44:55AM +0000, Will Deacon wrote:
>>>> Hi Russell,
>>>>
>>>> On Mon, Oct 28, 2013 at 12:47:36AM +0000, Russell King - ARM Linux wrote:
>>>>> On Sat, Oct 19, 2013 at 08:51:35PM +0100, Ben Dooks wrote:
>>>>>> On 19/10/13 18:09, Will Deacon wrote:
>>>>>>> Do you think you could send another pull request please?
>>>>>> Ok, sorted.
>>>>> Pulled, but there was a conflict.  Please check this resolution (it's
>>>>> copy'n'pasted).  I'll probably be in linux-next tomorrow in any case,
>>>>> but any mistake here can be fixed.
>>>> This doesn't look quite right to me, but unfortunately I'm going be spending
>>>> most (all?) of today trying to catch a flight out of the UK. Hopefully Dave
>>>> or Ben can investigate further, but comments below.
>>>>
>>>>> diff --cc arch/arm/kernel/head.S
>>>>> index 54547947a4e9,a047acfa6b6d..000000000000
>>>>> --- a/arch/arm/kernel/head.S
>>>>> +++ b/arch/arm/kernel/head.S
>>>>> @@@ -602,28 -586,26 +606,39 @@@ __fixup_a_pv_table
>>>>>         b       2f
>>>>>   1:    add     r7, r3
>>>>>         ldrh    ip, [r7, #2]
>>>>> + ARM_BE8(rev16 ip, ip)
>>>>>  -      and     ip, 0x8f00
>>>>>  -      orr     ip, r6  @ mask in offset bits 31-24
>>>>>  +      tst     ip, #0x4000
>>>>>  +      and     ip, #0x8f00
>>>>>  +      orrne   ip, r6  @ mask in offset bits 31-24
>>>>>  +      orreq   ip, r0  @ mask in offset bits 7-0
>>>>> + ARM_BE8(rev16 ip, ip)
>>>>>         strh    ip, [r7, #2]
>>>>>  +      ldrheq  ip, [r7]
>>>>>  +      biceq   ip, #0x20
>>>>>  +      orreq   ip, ip, r0, lsr #16
>>>>>  +      strheq  ip, [r7]
>>>> There are new halfword accesses here without any conditional revs.
>>> Yes, I missed this one.
>>>
>>>>> + #ifdef CONFIG_CPU_ENDIAN_BE8
>>>>> +       @ in BE8, we load data in BE, but instructions still in LE
>>>>> +       bic     ip, ip, #0xff000000
>>>>>  -      orr     ip, ip, r6, lsl#24
>>>>> ++      tst     ip, #0x000f0000 @ check the rotation field
>>>> Since that orr with shift has been removed, I think the masks for the BE
>>>> case are now incorrect...
>>>>
>>>>> ++      orrne   ip, ip, r6, lsl #24 @ mask in offset bits 31-24
>>>>> ++      biceq   ip, ip, #0x00004000 @ clear bit 22
>>>>> ++      orreq   ip, ip, r0, lsl #24 @ mask in offset bits 7-0
>>> Actually, look closer.  It became the orrne here.
>>>
>>>>> + #else
>>>>>         bic     ip, ip, #0x000000ff
>>>>>  -      orr     ip, ip, r6      @ mask in offset bits 31-24
>>>>>  +      tst     ip, #0xf00      @ check the rotation field
>>>>>  +      orrne   ip, ip, r6      @ mask in offset bits 31-24
>>>>>  +      biceq   ip, ip, #0x400000       @ clear bit 22
>>>> ...which seems to be confirmed by the updated LE code (everything is off
>>>> by a byte).
>>> The LE code was left unaltered from Santosh's patch, so that should be
>>> correct.  I just did an endian conversion to the BE case.
>>>
>>>> Somebody should probably sit down with the conflicting patch and port the BE
>>>> changes over. I think the relevant patch is "ARM: mm: Correct virt_to_phys
>>>> patching for 64 bit physical addresses". In fact, looking at *that* patch,
>>>> it's *also* broken for BE! It adds the following to head.S:
>>>>
>>>> +#ifdef __ARMEB_
>>>> +#define LOW_OFFSET     0x4
>>>> +#define HIGH_OFFSET    0x0
>>>> +#else
>>>> +#define LOW_OFFSET     0x0
>>>> +#define HIGH_OFFSET    0x4
>>>> +#endif
>>>>
>>>> (spot the missing underscore).
>>> Yep, well spotted.
>>>
>>> Well, we have some time to get this all fixed, so I'm going to drop
>>> Ben's tree.  I think we need to first commit a patch to fix the error
>>> in Santosh's patch.
>> Sorry, I will send a patch fix this missing underscore bug.
>>
>> Regards,
>>  Sricharan

[PULL REQ] Big Endian initial patch series

[Victor Kamensky]

On 28 October 2013 08:59, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> On 28/10/13 08:53, Russell King - ARM Linux wrote:
>
>>> +#ifdef __ARMEB_
>>> +#define LOW_OFFSET     0x4
>>> +#define HIGH_OFFSET    0x0
>>> +#else
>>> +#define LOW_OFFSET     0x0
>>> +#define HIGH_OFFSET    0x4
>>> +#endif
>>>
>>> (spot the missing underscore).
>>
>>
>> Yep, well spotted.
>>
>> Well, we have some time to get this all fixed, so I'm going to drop
>> Ben's tree.  I think we need to first commit a patch to fix the error
>> in Santosh's patch
>
>
> Ok, shall I look at re-basing to -rc6?

I just fetched rmk/for-next as of 10/29/2013 8pm PST. I see that big
endian patch series is in. I believe conflict resolution 580dac83 in head.S
is correct. I've added typo and .align fix as in [1] to head.S and
tested BE image on TC2 in both ARM and THUMB2 modes - both boot
and look fine to me.

> I have a bug report with a change to the SCU reading for the SMP
> fix-up case which could do with fixing, so I could add this and send
> another pull request (complete with description this time).
>

IMHO it would be great if we fix SCU BE A9 SMP issue separately
later, and keep BE changes that are already in rmk/for-next.

Thanks,
Victor

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/thread.html#207597

> --
> Ben Dooks                               http://www.codethink.co.uk/
> Senior Engineer                         Codethink - Providing Genius

[PULL REQ] Big Endian initial patch series

[Russell King - ARM Linux]

On Tue, Oct 29, 2013 at 09:05:09PM -0700, Victor Kamensky wrote:
> I just fetched rmk/for-next as of 10/29/2013 8pm PST. I see that big
> endian patch series is in. I believe conflict resolution 580dac83 in head.S
> is correct. I've added typo and .align fix as in [1] to head.S and
> tested BE image on TC2 in both ARM and THUMB2 modes - both boot
> and look fine to me.

Although it's in for-next, I won't be pushing the series until we know
that there are no known problems - so we need to get the patch you refer
to in as well along side it.

[PULL REQ] Big Endian initial patch series

[Russell King - ARM Linux]

On Mon, Oct 28, 2013 at 03:59:34PM +0000, Ben Dooks wrote:
> On 28/10/13 08:53, Russell King - ARM Linux wrote:
>
>>> +#ifdef __ARMEB_
>>> +#define LOW_OFFSET     0x4
>>> +#define HIGH_OFFSET    0x0
>>> +#else
>>> +#define LOW_OFFSET     0x0
>>> +#define HIGH_OFFSET    0x4
>>> +#endif
>>>
>>> (spot the missing underscore).
>>
>> Yep, well spotted.
>>
>> Well, we have some time to get this all fixed, so I'm going to drop
>> Ben's tree.  I think we need to first commit a patch to fix the error
>> in Santosh's patch
>
> Ok, shall I look at re-basing to -rc6?
>
> I have a bug report with a change to the SCU reading for the SMP
> fix-up case which could do with fixing, so I could add this and send
> another pull request (complete with description this time).

Rebasing on a later kernel from Linus doesn't do much good, because the
conflicting changes aren't in Linus' tree, but are part of Santosh's
patch set for high physical memory for LPAE systems.

I think we're fine with knowing what the merge fixes are now.  There's
only one remaining problem left, which is a missing .align.  Someone
needs to send a patch for that. :)

[PULL REQ] Big Endian initial patch series

[Victor Kamensky]

On 30 October 2013 11:01, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Oct 28, 2013 at 03:59:34PM +0000, Ben Dooks wrote:
>> On 28/10/13 08:53, Russell King - ARM Linux wrote:
>>
>>>> +#ifdef __ARMEB_
>>>> +#define LOW_OFFSET     0x4
>>>> +#define HIGH_OFFSET    0x0
>>>> +#else
>>>> +#define LOW_OFFSET     0x0
>>>> +#define HIGH_OFFSET    0x4
>>>> +#endif
>>>>
>>>> (spot the missing underscore).
>>>
>>> Yep, well spotted.
>>>
>>> Well, we have some time to get this all fixed, so I'm going to drop
>>> Ben's tree.  I think we need to first commit a patch to fix the error
>>> in Santosh's patch
>>
>> Ok, shall I look at re-basing to -rc6?
>>
>> I have a bug report with a change to the SCU reading for the SMP
>> fix-up case which could do with fixing, so I could add this and send
>> another pull request (complete with description this time).
>
> Rebasing on a later kernel from Linus doesn't do much good, because the
> conflicting changes aren't in Linus' tree, but are part of Santosh's
> patch set for high physical memory for LPAE systems.
>
> I think we're fine with knowing what the merge fixes are now.  There's
> only one remaining problem left, which is a missing .align.  Someone
> needs to send a patch for that. :)

Sorry, I referenced [1], but actually Sricharan sent [2] which includes .align
fix as well as fix for __ARMEB_ typo and marked it as V2 for [1].

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/thread.html#207597
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/207809.html

Is there anything else we need to do?

Thanks,
Victor