linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PULL REQ] Big Endian initial patch series
Date: Mon, 28 Oct 2013 08:53:29 +0000	[thread overview]
Message-ID: <20131028085328.GI16735@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20131028084455.GA20218@mudshark.cambridge.arm.com>

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.

  reply	other threads:[~2013-10-28  8:53 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 [this message]
2013-10-28  9:12           ` Sricharan R
2013-10-29  5:09             ` Victor Kamensky
2013-10-29  5:24               ` Sricharan R
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=20131028085328.GI16735@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).