linux-kernel - Re: [PATCH] MTD: Fix Orion NAND driver compilation with ARM OABI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 20 Mar 2010 11:58:06 +0000
From:	David Woodhouse <dwmw2@...radead.org>
To:	Paulius Zaleckas <paulius.zaleckas@...il.com>, nico@....org,
	rth@...ddle.net
Cc:	nico@...xnic.net, akpm@...ux-foundation.org,
	u.kleine-koenig@...gutronix.de, simon.kagstrom@...insight.net,
	linux-mtd@...ts.infradead.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] MTD: Fix Orion NAND driver compilation with ARM OABI

On Sat, 2010-03-20 at 13:20 +0200, Paulius Zaleckas wrote:
> On Sat, Mar 20, 2010 at 11:41 AM, David Woodhouse <dwmw2@...radead.org> wrote:
> > On Sat, 2010-03-20 at 10:55 +0200, Paulius Zaleckas wrote:
> >> -             uint64_t x;
> >> +             /*
> >> +              * force x variable to r2/r3 registers since ldrd instruction
> >> +              * requires first register to be even.
> >> +              */
> >> +             register uint64_t x asm ("r2");
> >> +
> >>               asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
> >
> > Hm, isn't there an asm constraint which will force it into an
> > appropriate register pair?
> 
> Not that I know of...
> 
> > Failing that, "=&r2,r4,r6,r8" ought to work.
> 
> No, fails with error: matching constraint not valid in output operand

Hm, crap -- GCC on ARM doesn't let you give specific registers, so that
trick doesn't work.

Strictly speaking, I think your version is wrong -- although you force
the variable 'x' to be stored in r2/r3, you don't actually force GCC to
use r2/r3 as the output registers for the asm statement -- it could
happily use other registers, then move their contents into r2/r3
afterwards.

Obviously it _won't_ do that most of the time, but it _could_. GCC PR
#15089 was filed for the fact that sometimes it does, but I think Nico
was missing the point -- GCC is _allowed_ to do that, and if it makes
you sad then you should be asking for better assembly constraints which
would allow you to tell it not to.

See the __asmeq() macro in <asm/system.h> for a dirty hack which will
check which registers are used and abort at compile time, although your
compilation is going to fail anyway so I'm not sure it makes much of a
difference in this particular case.

The real fix here is to add an asm constraint to GCC which allows you to
specify "any even GPR" (or whatever's most suitable for the ldrd
instruction). Being able to give specific registers, like you can on
other architectures, would be useful too.

Please file a PR, then resubmit your patch with a comment explaining
that the code is known to be broken because GCC doesn't allow you to do
any better, and containing a reference to your PR. If people copy your
code, I want them to at least know that they're propagating a bug.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@...el.com                              Intel Corporation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/