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

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2012-02-27 at 17:53 +0000, Dave Martin wrote:
> On Thu, Feb 23, 2012 at 05:48:22PM +0000, Stefano Stabellini wrote:
> > We need a register to pass the hypercall number because we might not
> > know it at compile time and HVC only takes an immediate argument.
> > 
> > Among the available registers r12 seems to be the best choice because it
> > is defined as "intra-procedure call scratch register".
> 
> This would be massively simplified if you didn't try to inline the HVC.
> Does it really need to be inline?
>
> > +#define __HYPERCALL ".word 0xe1400070 + " __HVC_IMM(XEN_HYPERCALL_TAG)
> 
> Please, do not do this.  It won't work in Thumb, where the encodings are
> different.
> 
> It is reasonable to expect anyone building Xen to have reasonably new
> tools, you you can justifiably use
> 
> AFLAGS_thisfile.o := -Wa,-march=armv7-a+virt
> 
> in the Makefile and just use the hvc instruction directly.

Our aim is for guest kernel binaries not to be specific to Xen -- i.e.
they should be able to run on baremetal and other hypervisors as well.
The differences should only be in the device-tree passed to the kernel.

> Of course, this is only practical if the HVC invocation is not inlined.

I suppose we could make the stub functions out of line, we just copied
what Xen does on x86.

The only thing which springs to mind is that 5 argument hypercalls will
end up pushing the fifth argument to the stack only to pop it back into
r4 for the hypercall and IIRC it also needs to preserve r4 (callee saved
reg) which is going to involve some small amount of code to move stuff
around too.

So by inlining the functions we avoid some thunking because the compiler
would know exactly what was happening at the hypercall site.

We don't currently have any 6 argument hypercalls but the same would
extend there.

> If we can't avoid macro-ising HVC, we should do it globally, not locally
> to the Xen code.  That way we at least keep all the horror in one place.

That sounds like a good idea to me.

Given that Stefano is proposing to make the ISS a (per-hypervisor)
constant we could consider just defining the Thumb and non-Thumb
constants instead of doing all the construction with the __HVC_IMM stuff
-- that would remove a big bit of the macroization.

Ian.