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

[Julien Grall <julien.grall@arm.com>]

On 03/08/16 19:20, Stefano Stabellini wrote:
> On Wed, 3 Aug 2016, Julien Grall wrote:
>> Hi Jan,
>>
>> On 03/08/16 09:53, Jan Beulich wrote:
>>>>>> 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).
>>
>> I would tend to say that anything not documented in the public header is not
>> part of the ABI regardless how it has been implemented before hand.
>
> I agree. What is documented (or not documented) in the public headers
> is the golden standard.
>
>
>> Anyway, I looked at the hypercall implementation on ARM and it seems that we
>> half support the 6th argument. For instance hypercall_create_continuation is
>> clobbering r5/x5 which is not part of the ABI.
>>
>> However do_trap_hypercall is only supporting up to 5 argument.
>>
>> I don't think it would be an issue to support 6 arguments on ARM. Stefano,
>> what do you think?
>
> I am OK with supporting 6 arguments hypercalls and it would be good to
> have both architectures match. The 32bit guest-side implementation needs
> special caring, but it should be OK (see HYPERCALL5 in Linux).

It seems that Linux only handle 5 arguments for both x86 (see 
arch/x86/asm/xen/hypercall.h) and ARM.

The funny part is the x86 code has:

"The result certainly isn't pretty, and it really shows up cpp's
weakness as as macro language.  Sorry.  (But let's just give thanks
there aren't more than 5 arguments...)"

Anyway, I will add this in my todo list, I don't think it is really 
important to have this change right now.

> Fortunately xen/include/public/arch-arm.h states that only arguments
> used by an hypercall can be clobbered.

Regards,

-- 
Julien Grall

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