Re: [Xen-devel] [PATCH 1/9] x86/hypercall: Move some of the hvm hypercall infrastructure into hypercall.h

[Julien Grall <julien.grall@xxxxxxx>]

On 02/08/16 15:17, Jan Beulich wrote:
> > > > On 02.08.16 at 16:04, <julien.grall@xxxxxxx> wrote:
> > On 02/08/16 14:28, Jan Beulich wrote:
> > > > > > On 02.08.16 at 15:14, <andrew.cooper3@xxxxxxxxxx> wrote:
> > > > On 02/08/16 13:50, Jan Beulich wrote:
> > > > > > > > On 18.07.16 at 11:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> > > > > > --- a/xen/include/asm-x86/hypercall.h
> > > > > > +++ b/xen/include/asm-x86/hypercall.h
> > > > > > @@ -5,9 +5,21 @@
> > > > > >  #ifndef __ASM_X86_HYPERCALL_H__
> > > > > >  #define __ASM_X86_HYPERCALL_H__
> > > > > >
> > > > > > +#include <xen/types.h>
> > > > > >  #include <public/physdev.h>
> > > > > > +#include <public/event_channel.h>
> > > > > Why?
> > > > You snipped the commit message, which justifies why.  This header file
> > > > cannot currently be included in isolation, and I need it to be.
> > > Ah, that sentence there also relates to this addition.
> > >
> > > > > >  #include <public/arch-x86/xen-mca.h> /* for do_mca */
> > > > > > -#include <xen/types.h>
> > > > > > +
> > > > > > +typedef unsigned long hypercall_fn_t(
> > > > > > +    unsigned long, unsigned long, unsigned long,
> > > > > > +    unsigned long, unsigned long, unsigned long);
> > > > > Wouldn't this better go into xen/hypercall.h?
> > > > It is architecture specific.
> > > >
> > > > ARM's version is
> > > >
> > > > typedef register_t (*arm_hypercall_fn_t)(
> > > >     register_t, register_t, register_t, register_t, register_t);
> > > Which is bogus - they're lucky we so far don't have any 6-argument
> > > hypercalls. Or the other way around - we could limit hypercalls to
> > > just five arguments (for now) on x86 too, allowing things to get
> > > unified. Anyway - that probably goes too far right now, so feel free
> > > to add my ack to the patch.
> > I am not sure why you think we are lucky on ARM. The hypercall has been
> > defined on ARM to support up to 5 arguments (public/arch-arm.h):
> >
> > "A hypercall can take up to 5 arguments. These are passed in
> > registers, the first argument in x0/r0 (for arm64/arm32 guests
> > respectively irrespective of whether the underlying hypervisor is
> > 32- or 64-bit), the second argument in x1/r1, the third in x2/r2,
> > the forth in x3/r3 and the fifth in x4/r4."
> >
> > So the prototype matches the ABI.
> Well, I find it quite odd for hypercall argument counts to differ
> between arches. I.e. I'd conclude the ABI was mis-specified.
Is it documented somewhere for the x86 code? Looking at Linux, the 
privcmd call is only passing 5 arguments on both ARM and x86.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel