diff mbox

[02/10] ARM: fixup_pv_table bug when CPU_ENDIAN_BE8

Message ID 1371645406-16873-3-git-send-email-ben.dooks@codethink.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Dooks June 19, 2013, 12:36 p.m. UTC
The fixup_pv_table assumes that the instructions are in the same
endian configuration as the data, but when the CPU is running in
BE8 the instructions stay in little-endian format.

Make sure if CONFIG_CPU_ENDIAN_BE8 is set that we do all the
alterations to the instructions taking in to account the LDR/STR
will be swapping the data endian-ness.

Since the code is only modifying a byte, we avoid dual-swapping
the data, and just change the bits we clear and ORR in.

Note, not tested against an actual bug, it looked wrong.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 arch/arm/kernel/head.S |    6 ++++++
 1 file changed, 6 insertions(+)

Comments

Thomas Petazzoni June 19, 2013, 5:33 p.m. UTC | #1
Dear Ben Dooks,

On Wed, 19 Jun 2013 13:36:38 +0100, Ben Dooks wrote:

> +#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
> +#else
>  	bic	ip, ip, #0x000000ff
>  	orr	ip, ip, r6	@ mask in offset bits 31-24
> +#endif
>  	str	ip, [r7, r3]
>  2:	cmp	r4, r5
>  	ldrcc	r7, [r4], #4	@ use branch for delay slot

As was suggested in
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/150782.html,
you could put this patch later in the series and use the ARM_BE8()
macro you introduce in the next patch.

Also in
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/150784.html,
the same reviewer suggested to name the macro BE8() to make it fit like
ARM() and THUMB() in the margin left by the indentation of assembly
code. And also to add LE() to make the above code look like:

BE8(	bic ip, ip, #0xff000000)
BE8(	orr ip, ip, r6, lsl#24)
LE(	bic ip, ip, #0x000000ff)
LE(	orr ip, ip, r6)

Thomas
Ben Dooks June 20, 2013, 10:54 a.m. UTC | #2
On 19/06/13 18:33, Thomas Petazzoni wrote:
> Dear Ben Dooks,
>
> On Wed, 19 Jun 2013 13:36:38 +0100, Ben Dooks wrote:
>
>> +#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
>> +#else
>>   	bic	ip, ip, #0x000000ff
>>   	orr	ip, ip, r6	@ mask in offset bits 31-24
>> +#endif
>>   	str	ip, [r7, r3]
>>   2:	cmp	r4, r5
>>   	ldrcc	r7, [r4], #4	@ use branch for delay slot
>
> As was suggested in
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/150782.html,
> you could put this patch later in the series and use the ARM_BE8()
> macro you introduce in the next patch.
>
> Also in
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/150784.html,
> the same reviewer suggested to name the macro BE8() to make it fit like
> ARM() and THUMB() in the margin left by the indentation of assembly
> code. And also to add LE() to make the above code look like:
>
> BE8(	bic ip, ip, #0xff000000)
> BE8(	orr ip, ip, r6, lsl#24)
> LE(	bic ip, ip, #0x000000ff)
> LE(	orr ip, ip, r6)

Actually, that doesn't work as LE is not the inverse of BE8 as
it could also be when the kernel is being compiled in BE32.
Thomas Petazzoni June 20, 2013, 4:09 p.m. UTC | #3
Dear Ben Dooks,

On Thu, 20 Jun 2013 11:54:58 +0100, Ben Dooks wrote:

> > Also in
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/150784.html,
> > the same reviewer suggested to name the macro BE8() to make it fit like
> > ARM() and THUMB() in the margin left by the indentation of assembly
> > code. And also to add LE() to make the above code look like:
> >
> > BE8(	bic ip, ip, #0xff000000)
> > BE8(	orr ip, ip, r6, lsl#24)
> > LE(	bic ip, ip, #0x000000ff)
> > LE(	orr ip, ip, r6)
> 
> Actually, that doesn't work as LE is not the inverse of BE8 as
> it could also be when the kernel is being compiled in BE32.

Aah, correct. In BE32, data and instructions have the same endianness.

Thanks,

Thomas
diff mbox

Patch

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 8bac553..56ddd47 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -594,8 +594,14 @@  __fixup_a_pv_table:
 #else
 	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
+#else
 	bic	ip, ip, #0x000000ff
 	orr	ip, ip, r6	@ mask in offset bits 31-24
+#endif
 	str	ip, [r7, r3]
 2:	cmp	r4, r5
 	ldrcc	r7, [r4], #4	@ use branch for delay slot