From: r.sricharan@ti•com (Sricharan R)
To: linux-arm-kernel@lists•infradead.org
Subject: [PULL REQ] Big Endian initial patch series
Date: Tue, 29 Oct 2013 10:54:08 +0530 [thread overview]
Message-ID: <526F4678.3050609@ti.com> (raw)
In-Reply-To: <CAA3XUr1rW5q4=-eAXSaZ4xP75Cmt98-j=aAggreB4wgjt_YThg@mail.gmail.com>
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
next prev parent reply other threads:[~2013-10-29 5:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-18 20:54 [PULL REQ] Big Endian initial patch series Ben Dooks
2013-10-19 17:09 ` Will Deacon
2013-10-19 19:51 ` Ben Dooks
2013-10-28 0:47 ` Russell King - ARM Linux
2013-10-28 8:44 ` Will Deacon
2013-10-28 8:53 ` Russell King - ARM Linux
2013-10-28 9:12 ` Sricharan R
2013-10-29 5:09 ` Victor Kamensky
2013-10-29 5:24 ` Sricharan R [this message]
2013-10-28 15:59 ` Ben Dooks
2013-10-30 4:05 ` Victor Kamensky
2013-10-30 17:43 ` Russell King - ARM Linux
2013-10-30 18:01 ` Russell King - ARM Linux
2013-10-30 18:23 ` Victor Kamensky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=526F4678.3050609@ti.com \
--to=r.sricharan@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox