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

From: "Jan Beulich" <JBeulich@suse.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH 1/9] x86/hypercall: Move some of the hvm hypercall infrastructure into hypercall.h
Date: Wed, 03 Aug 2016 02:53:49 -0600	[thread overview]
Message-ID: <57A1CD3D020000780010212C@prv-mh.provo.novell.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1608021140590.21415@sstabellini-ThinkPad-X260>

>>> On 02.08.16 at 20:43, <sstabellini@kernel.org> wrote:
> On Tue, 2 Aug 2016, Jan Beulich wrote:
>> >>> On 02.08.16 at 16:59, <andrew.cooper3@citrix.com> wrote:
>> > On 02/08/16 15:54, Jan Beulich wrote:
>> >>>>> On 02.08.16 at 16:26, <julien.grall@arm.com> wrote:
>> >>> On 02/08/16 15:17, Jan Beulich wrote:
>> >>>> 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.
>> >> arch-x86/xen-x86_32.h has
>> >>
>> >>  * Hypercall interface:
>> >>  *  Input:  %ebx, %ecx, %edx, %esi, %edi, %ebp (arguments 1-6)
>> >>  *  Output: %eax
>> >>
>> >> while arch-x86/xen-x86_64.h has
>> >>
>> >>  * Hypercall interface:
>> >>  *  Input:  %rdi, %rsi, %rdx, %r10, %r8, %r9 (arguments 1-6)
>> >>  *  Output: %rax
>> > 
>> > The only actual 6 argument hypercall is the v4v hypercall, better known
>> > as __HYPERVISOR_xc_reserved_op at index 39, but that isn't implemented
>> > anywhere upstream.
>> 
>> But it serves as an example what now wouldn't work on ARM.
> 
> At the time the arm hypercall ABI was published, it matched the x86
> hypercall ABI, which had only 5 hypercall arguments.
> 
> The issue is that the x86 hypercall ABI changed, and now is out of sync
> with ARM. The faulty commit being:

That's one way of viewing it, but I don't think an appropriate one.
6-argument hypercalls had always been possible on x86, just that
they might not have been documented in the public headers (but
instead only in the actual hypercall implementation).

Jan

> commit 4af64160c580b02f28c992c09d55957cb20a9b91
> Author: David Vrabel <david.vrabel@citrix.com>
> Date:   Wed May 30 09:25:11 2012 +0100
> 
>     x86: document register for 6th hypercall argument
>     
>     From: David Vrabel <david.vrabel@citrix.com>
>     
>     Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>     Committed-by: Keir Fraser <keir@xen.org>
> 
> 
> While the ARM ABI is from few months earlier:
> 
> commit 40f20c4bfcd5d25c90f9419250ca8a229bf4c1e5
> Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Date:   Tue Mar 13 16:04:05 2012 +0000
> 
>     arm: use r12 to pass the hypercall number
>     
>     ** This is a guest visible ABI change which requires an updated guest 
> kernel **
>     
>     Use r12 to pass the hypercall number and r0-r4 for the hypercall
>     arguments.
>     
>     Use the ISS to pass an hypervisor specific tag.
>     
>     Remove passing unused registers to arm_hypercall_table: we don't have 6
>     arguments hypercalls and we never use 64 bit values as hypercall
>     arguments, 64 bit values are only contained within structs passed as
>     arguments.
>     
>     Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>     [ use #ifndef NDEBUG, fix coding style, expand calling convention 
> comment
>       slightly and added a big fat note about ABI change - ijc ]
>  

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