All of lore.kernel.org
 help / color / mirror / Atom feed
From: dave.martin@linaro.org (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number to the hypervisor
Date: Tue, 28 Feb 2012 09:46:16 +0000	[thread overview]
Message-ID: <20120228094616.GA2063@linaro.org> (raw)
In-Reply-To: <1330372125.10008.47.camel@dagon.hellion.org.uk>

On Mon, Feb 27, 2012 at 07:48:45PM +0000, Ian Campbell wrote:
> 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.

True ...

> 
> 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.

It's not quite as simple as that -- emitting instructions using data
directives is not endianness safe, and even in the cases where .long gives
the right result for ARM, it gives the wrong result for 32-bit Thumb
instructions if the opcode is given in human-readable order.

I was trying to solve the same problem for the kvm guys with some global
macros -- I'm aiming to get a patch posted soon, so I'll make sure
you're on CC.

Cheers
---Dave

WARNING: multiple messages have this Message-ID (diff)
From: Dave Martin <dave.martin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Ian Campbell <Ian.Campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
Cc: "xen-devel-GuqFBffKawuULHF6PoxzQEEOCMrvLtNR@public.gmane.org"
	<xen-devel-GuqFBffKawuULHF6PoxzQEEOCMrvLtNR@public.gmane.org>,
	"linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org"
	<linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org>,
	"kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"arnd-r2nGTMty4D4@public.gmane.org"
	<arnd-r2nGTMty4D4@public.gmane.org>,
	"catalin.marinas-5wv7dgnIgG8@public.gmane.org"
	<catalin.marinas-5wv7dgnIgG8@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	David Vrabel
	<david.vrabel-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number to the hypervisor
Date: Tue, 28 Feb 2012 09:46:16 +0000	[thread overview]
Message-ID: <20120228094616.GA2063@linaro.org> (raw)
In-Reply-To: <1330372125.10008.47.camel-ztPmHsLffjjnO4AKDKe2m+kiAK3p4hvP@public.gmane.org>

On Mon, Feb 27, 2012 at 07:48:45PM +0000, Ian Campbell wrote:
> 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.

True ...

> 
> 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.

It's not quite as simple as that -- emitting instructions using data
directives is not endianness safe, and even in the cases where .long gives
the right result for ARM, it gives the wrong result for 32-bit Thumb
instructions if the opcode is given in human-readable order.

I was trying to solve the same problem for the kvm guys with some global
macros -- I'm aiming to get a patch posted soon, so I'll make sure
you're on CC.

Cheers
---Dave

WARNING: multiple messages have this Message-ID (diff)
From: Dave Martin <dave.martin@linaro.org>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	"linaro-dev@lists.linaro.org" <linaro-dev@lists.linaro.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	David Vrabel <david.vrabel@citrix.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number to the hypervisor
Date: Tue, 28 Feb 2012 09:46:16 +0000	[thread overview]
Message-ID: <20120228094616.GA2063@linaro.org> (raw)
In-Reply-To: <1330372125.10008.47.camel@dagon.hellion.org.uk>

On Mon, Feb 27, 2012 at 07:48:45PM +0000, Ian Campbell wrote:
> 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.

True ...

> 
> 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.

It's not quite as simple as that -- emitting instructions using data
directives is not endianness safe, and even in the cases where .long gives
the right result for ARM, it gives the wrong result for 32-bit Thumb
instructions if the opcode is given in human-readable order.

I was trying to solve the same problem for the kvm guys with some global
macros -- I'm aiming to get a patch posted soon, so I'll make sure
you're on CC.

Cheers
---Dave

  reply	other threads:[~2012-02-28  9:46 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-23 17:47 [PATCH-WIP 00/13] xen/arm: receive Xen events and initialize xenbus Stefano Stabellini
2012-02-23 17:47 ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number to the hypervisor Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-27 16:27   ` Ian Campbell
2012-02-27 16:27     ` Ian Campbell
2012-02-27 18:03     ` Dave Martin
2012-02-27 18:03       ` Dave Martin
2012-02-27 18:03       ` Dave Martin
2012-02-27 19:33       ` Ian Campbell
2012-02-27 19:33         ` Ian Campbell
2012-02-28 10:20         ` Dave Martin
2012-02-28 10:20           ` Dave Martin
2012-02-28 10:20           ` Dave Martin
2012-02-28 10:48           ` Ian Campbell
2012-02-28 10:48             ` Ian Campbell
2012-02-28 12:28             ` Stefano Stabellini
2012-02-28 12:28               ` Stefano Stabellini
2012-02-28 12:28               ` Stefano Stabellini
2012-02-29  9:34               ` Dave Martin
2012-02-29  9:34                 ` Dave Martin
2012-02-29  9:34                 ` Dave Martin
2012-02-29  9:56                 ` Ian Campbell
2012-02-29  9:56                   ` Ian Campbell
2012-02-29  9:56                   ` Ian Campbell
2012-02-29 11:47                   ` Dave Martin
2012-02-29 11:47                     ` Dave Martin
2012-02-29 12:58                   ` Dave Martin
2012-02-29 12:58                     ` Dave Martin
2012-02-29 12:58                     ` Dave Martin
2012-02-29 14:44                     ` Ian Campbell
2012-02-29 14:44                       ` Ian Campbell
2012-03-01  9:35                       ` Dave Martin
2012-03-01  9:35                         ` Dave Martin
2012-03-01  9:35                         ` Dave Martin
2012-03-01 10:12                       ` Russell King - ARM Linux
2012-03-01 10:12                         ` Russell King - ARM Linux
2012-03-02 21:19                       ` Nicolas Pitre
2012-03-02 21:19                         ` Nicolas Pitre
2012-02-29 14:52                     ` Stefano Stabellini
2012-02-29 14:52                       ` Stefano Stabellini
2012-03-01  9:51                       ` Dave Martin
2012-03-01  9:51                         ` Dave Martin
2012-03-01  9:51                         ` Dave Martin
2012-03-01 10:10                     ` Russell King - ARM Linux
2012-03-01 10:10                       ` Russell King - ARM Linux
2012-03-01 10:27                       ` Dave Martin
2012-03-01 10:27                         ` Dave Martin
2012-03-01 10:27                         ` Dave Martin
2012-03-01 10:35                         ` Russell King - ARM Linux
2012-03-01 10:35                           ` Russell King - ARM Linux
2012-03-01 12:12                           ` Stefano Stabellini
2012-03-01 12:12                             ` Stefano Stabellini
2012-03-01 12:12                             ` Stefano Stabellini
2012-03-02 21:15                     ` Nicolas Pitre
2012-03-02 21:15                       ` Nicolas Pitre
2012-03-02 21:15                       ` Nicolas Pitre
2012-03-08  9:58                       ` Richard Earnshaw
2012-03-08  9:58                         ` Richard Earnshaw
2012-03-08 12:17                         ` Dave Martin
2012-03-08 12:17                           ` Dave Martin
2012-03-08 17:21                         ` Nicolas Pitre
2012-03-08 17:21                           ` Nicolas Pitre
2012-03-08 18:47                           ` Richard Earnshaw
2012-03-08 18:47                             ` Richard Earnshaw
2012-03-08 18:47                             ` Richard Earnshaw
2012-03-09 15:58                             ` Dave Martin
2012-03-09 15:58                               ` Dave Martin
2012-03-09 15:58                               ` Dave Martin
2012-03-09 16:20                               ` Nicolas Pitre
2012-03-09 16:20                                 ` Nicolas Pitre
2012-03-09 17:38                                 ` Richard Earnshaw
2012-03-09 17:38                                   ` Richard Earnshaw
2012-03-09 17:38                                   ` Richard Earnshaw
2012-02-27 21:05     ` Peter Maydell
2012-02-27 21:05       ` Peter Maydell
2012-02-27 21:05       ` Peter Maydell
2012-02-28 10:12       ` Ian Campbell
2012-02-28 10:12         ` Ian Campbell
2012-02-27 17:53   ` Dave Martin
2012-02-27 17:53     ` Dave Martin
2012-02-27 17:53     ` Dave Martin
2012-02-27 19:48     ` Ian Campbell
2012-02-27 19:48       ` Ian Campbell
2012-02-28  9:46       ` Dave Martin [this message]
2012-02-28  9:46         ` Dave Martin
2012-02-28  9:46         ` Dave Martin
2012-02-28 10:07         ` Ian Campbell
2012-02-28 10:07           ` Ian Campbell
2012-02-28 12:21         ` Stefano Stabellini
2012-02-28 12:21           ` Stefano Stabellini
2012-02-28 12:21           ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 02/13] xen/arm: introduce privcmp, physdev_op and memory_op hypercalls Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 03/13] xen/arm: mmu.h and page.h related definitions Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 04/13] xen/arm: sync_bitops Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 05/13] xen/arm: empty implementation of grant_table arch specific functions Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 06/13] xen/arm: missing includes Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 07/13] xen/arm: receive xen events on arm Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-24 11:12   ` David Vrabel
2012-02-24 11:12     ` David Vrabel
2012-02-24 12:23     ` Stefano Stabellini
2012-02-24 12:23       ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 08/13] xen/arm: fix arm xen guest handle definitions Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 09/13] xen/arm: shared_info and start_info Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 10/13] xen/arm: empty implementation of xen_remap_domain_mfn_range Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 11/13] xen/arm: Introduce xen_pfn_t for pfn and mfn types Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 12/13] xen/arm: compile and run xenbus Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 13/13] xen/arm: compile grant-table features events and xenbus, do not compile pci Stefano Stabellini
2012-02-23 17:48   ` Stefano Stabellini

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=20120228094616.GA2063@linaro.org \
    --to=dave.martin@linaro.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.