linux-kernel - Re: [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number to the hypervisor

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20120301102702.GD2034@linaro.org>
Date:	Thu, 1 Mar 2012 10:27:02 +0000
From:	Dave Martin <dave.martin@...aro.org>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Ian Campbell <Ian.Campbell@...rix.com>,
	Peter Maydell <peter.maydell@...aro.org>,
	"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
	"linaro-dev@...ts.linaro.org" <linaro-dev@...ts.linaro.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"arnd@...db.de" <arnd@...db.de>,
	Stefano Stabellini <Stefano.Stabellini@...citrix.com>,
	"catalin.marinas@....com" <catalin.marinas@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	David Vrabel <david.vrabel@...rix.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number
 to the hypervisor

On Thu, Mar 01, 2012 at 10:10:29AM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 29, 2012 at 12:58:26PM +0000, Dave Martin wrote:
> > On Wed, Feb 29, 2012 at 09:56:02AM +0000, Ian Campbell wrote:
> > > On Wed, 2012-02-29 at 09:34 +0000, Dave Martin wrote:
> > > > On Tue, Feb 28, 2012 at 12:28:29PM +0000, Stefano Stabellini wrote:
> > > 
> > > > > I don't have a very strong opinion on which register we should use, but
> > > > > I would like to avoid r7 if it is already actively used by gcc.
> > > > 
> > > > But there is no framepointer for Thumb-2 code (?)
> > > 
> > > Peter Maydell suggested there was:
> > > > r7 is (used by gcc as) the Thumb frame pointer; I don't know if this
> > > > makes it worth avoiding in this context.
> > > 
> > > Sounds like it might be a gcc-ism, possibly a non-default option?
> > > 
> > > Anyway, I think r12 will be fine for our purposes so the point is rather
> > > moot.
> > 
> > Just had a chat with some tools guys -- apparently, when passing register
> > arguments to gcc inline asms there really isn't a guarantee that those
> > variables will be in the expected registers on entry to the inline asm.
> 
> The best you can do is:
> 
> 	register unsigned int foo asm("r7") = value;
> 
> 	asm("blah %0" : : "r" (foo));
> 
> and ensure that your assembly checks that %0 _is_ r7 and produces a build
> error if not.  See the __asmeq() macro in asm/system.h to find out how to
> do that.
> 
> This feature has been missing from ARM GCC for quite a long time - it's
> used extensively on x86 GCC, where they have one register class per
> register, so they can do stuff like:
> 
> 	asm("blah %0" : : "a" (value));
> 
> and be guaranteed that %0 will be eax.
> 
> > If you need a specific register, this means that you must set up that
> > register explicitly inside the asm if you want a guarantee that the
> > code will work:
> > 
> > 	asm volatile (
> > 		"movw	r12, %[hvc_num]\n\t"
> > 		...
> > 		"hvc	#0"
> > 		:: [hvc_num] "i" (NUMBER) : "r12"
> > 	);
> > 
> > Of course, if you need to set up more than about 5 or 6 registers in
> > this way, the doubled register footprint means that the compiler will
> > have to start spilling stuff to the stack.
> 
> No it won't - it will barf instead - think about it.  If you're clobbering
> r0 - r5, but need to pass in six values in registers, gcc can't use r0-r5
> for that, so it must use the remaining registers.  It gets rather unhappy
> with that, and starts erroring out (iirc 'too many reloads' or some similar
> error.)  I've been there.

You're right about that -- I didn't pursue my line of thought to the end,
there.  I have see the behaviour you describe.

> If you want to do it that way, your only option is to store them to memory
> and pass the address of the block into the assembly, and reload them there.
> Which is extremely sucky and inefficient.
> 
> Practically, the register variable plus asm() does seem to work, and seems
> to work for virtually all gcc versions out there (there have been the odd
> buggy version, but those bugs appear to get fixed.)

That is inconvenient for us, but it's a not a bug.  The ability for asm
contraints to be able to gcc to put things in specific registers (as with
the gcc "abcd" constraints for i386) would be nice, but as you point out,
this capability is simply not supported by gcc right now for ARM -- the
compiler guys seem to be pretty opposed to it, so I can't say I anticiapte
this being supported in the near future.

So, where there's a compelling reason to inline these things, we can use
the existing techniques if we're alert to the risks.  But in cases where
there isn't a compelling reason, aren't we just inviting fragility
unnecessarily?

Cheers
---Dave
--
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/