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: [ARM] head.S change broke platform device registration?
Date: Wed, 12 Dec 2012 17:59:25 +0000	[thread overview]
Message-ID: <20121212175925.GA14363@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1355327745.6771.56.camel@ted>

On Wed, Dec 12, 2012 at 03:55:45PM +0000, Richard Purdie wrote:
> On Tue, 2012-12-11 at 00:25 +0000, Russell King - ARM Linux wrote:
> > On Mon, Dec 10, 2012 at 06:21:52PM +0100, Marko Kati? wrote:
> > > > The code which distinguishes between Borzoi and Akita is this:
> > > >
> > > > /* Check for a second SCOOP chip - if found we have Borzoi */
> > > >         ldr     r1, .SCOOP2ADDR
> > > > ==cacheline boundary without patch (0x120)=====
> > > >         ldr     r7, .BORZOIID
> > > >         mov     r6, #0x0140
> > > >         strh    r6, [r1]
> > > > ==cacheline boundary with patch (0x160)======
> > > >         ldrh    r6, [r1]
> > > >         cmp     r6, #0x0140
> > > >         beq     .SHARPEND                       @ We have Borzoi
> > > >
> > > > /* Must be Akita */
> > > >         ldr     r7, .AKITAID
> > > >         b       .SHARPEND                       @ We have Borzoi
> > > >
> > > > and I've marked where the cache line boundary ends up with the patch
> > > > in place... except of course that the caches are off here, so the
> > > > placement of the code should be irrelevant.  It also would have the
> > > > reverse results from those that you're experiencing.
> > > >
> > > > My guess is that because the Scoop2 device is not present, the strh
> > > > access places 0x140 onto the data bus.  By the time the ldrh is
> > > > executed, it reads the data bus, and because nothing drives it, it
> > > > reads back the value last present on the bus, which happens to be
> > > > 0x140 - and this allows us to think we have the Scoop2 device.
> > > >
> > > > In theory, because the caches are off (including the instruction
> > > > cache) the CPU should fetch ldrh between the strh write and the
> > > > ldrh access to the same address.
> > > >
> > > > Having said all that, I'm not impressed by the above code; to write to
> > > > a memory region which may or may not have a device, and immediately
> > > > read it back is a recipe for exactly this kind of stuff happening.  If
> > > > anyone has looked at any real device detection code, they'll know that
> > > > this sequence is typical:
> > > >
> > > >         ldr     r0, =address
> > > >         mov     r1, #probe_data
> > > >         eor     r2, r1, #~0
> > > >         str     r1, [r0, #reg1]
> > > >         str     r2, [r0, #reg2]
> > > >         ldr     r3, [r0, #reg1]
> > > >         ldr     r0, [r0, #reg2]
> > > >         teq     r1, r3
> > > >         teqeq   r2, r0
> > > >         beq     found
> > > >
> > > > This sequence _purposely_ writes the opposite value between the first
> > > > store and the read-back of it to perturb the bus in case it's floating,
> > > > to prevent exactly these kinds of false positives.  The second check is
> > > > additional belt and braces to make the check even more secure.
> > > >
> > > > Can we do that with Scoop2?  I've no idea, I don't know what the weird
> > > > 0x40 offset is that's being probed by this code.  We need whoever wrote
> > > > this code, but I suspect they've long since moved on and aren't
> > > > interested in it.
> 
> I suspect this was me and I was missing some of the subtleties in this
> case. I think the eor to perturb the bus should be fine and fix this
> code.

Except... we're currently hitting the register at 0x08800040 (presumably
a 64-byte offset) which doesn't appear in the scoop header file... so
I'm not sure if this is a special register or not for this.  Also we need
to know another register for the 2nd access which isn't going to cause
problems.  Any suggestions?

  parent reply	other threads:[~2012-12-12 17:59 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-30  1:38 [ARM] head.S change broke platform device registration? Marko Katić
2012-11-30 10:28 ` Marc Zyngier
2012-11-30 12:20   ` Marko Katić
2012-11-30 13:45     ` Marc Zyngier
2012-11-30 14:34     ` Russell King - ARM Linux
2012-12-04 21:48       ` Marko Katić
2012-12-04 22:18         ` Russell King - ARM Linux
2012-12-05 22:18           ` Marko Katić
2012-12-05 23:58             ` Russell King - ARM Linux
2012-12-07 12:28               ` Russell King - ARM Linux
2012-12-07 13:49                 ` Dave Martin
2012-12-10 17:23                   ` Marko Katić
2012-12-10 17:31                     ` Dave Martin
2012-12-10 17:21               ` Marko Katić
2012-12-11  0:25                 ` Russell King - ARM Linux
2012-12-12 15:55                   ` Richard Purdie
2012-12-12 17:22                     ` Marko Katić
2012-12-12 17:59                     ` Russell King - ARM Linux [this message]
2012-12-12 22:29                       ` Richard Purdie
2012-11-30 12:17 ` Dave Martin
2012-11-30 12:28   ` Marko Katić
2012-11-30 12:40     ` Russell King - ARM Linux
2012-11-30 13:07       ` Marko Katić
2012-11-30 13:11         ` Marko Katić

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=20121212175925.GA14363@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).