[PATCH 03/14] ARM: fixup_pv_table bug when CPU_ENDIAN_BE8

Ben Dooks
2013-07-25T10:34:38Z

On 25/07/13 11:25, Russell King - ARM Linux wrote:
> On Thu, Jul 25, 2013 at 11:14:42AM +0100, Will Deacon wrote:
>> What's wrong with the following?
>
> Is this like a challenge to find the silly mistake?
>
>> @@ -581,8 +581,13 @@ __fixup_a_pv_table:
>> b 2f
>> 1: add r7, r3
>> ldrh ip, [r7, #2]
>> +#ifdef CONFIG_CPU_ENDIAN_BE8
>> + and ip, 0x008f
>> + orr ip, ip, r6, lsl #24
>> +#else
>> and ip, 0x8f00
>> orr ip, r6 @ mask in offset bits 31-24
>> +#endif
>> strh ip, [r7, #2]
>
> Well, ip is being loaded/stored as 16bit, but you're putting r6 in to
> ip's bits 31-24, which are going to be ignored by the following strh.
>
> The comment "@ mask in offset bits 31-24" refers to the physical address
> bits here, which are already pre-shifted to bits 7-0 in r6.

I looked into this, and the code before this loop does:

clz r7, r6
lsr r6, #24
lsl r6, r7
bic r6, #0x0080
lsrs r7, #1
orrcs r6, #0x0080
orr r6, r6, r7, lsl #12
orr r6, #0x4000
b 2f

Note that r6 has bits that are not in the 0..7 range, which makes life
difficult when dealing with the loop. If we are going to change the
loop then we will have to pre rev16 r6 before going in to it.

IE:

lsls r6, #24
beq 2f
clz r7, r6
lsr r6, #24
lsl r6, r7
bic r6, #0x0080
lsrs r7, #1
orrcs r6, #0x0080
orr r6, r6, r7, lsl #12
orr r6, #0x4000
ARM_BE8(rev16 r6, r6)
b 2f
1: add r7, r3
ldrh ip, [r7, #2]
#ifdef CONFIG_CPU_ENDIAN_BE8
and ip, 0x008f
#else
and ip, 0x8f00
#endif
orr ip, r6 @ mask in offset bits 31-24
strh ip, [r7, #2]
2: cmp r4, r5
ldrcc r7, [r4], #4 @ use branch for delay slot
bcc 1b
bx lr

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