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?
next prev 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).