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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 02.08.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
> On 02/08/16 13:50, Jan Beulich wrote:
>>>>> On 18.07.16 at 11:51, <andrew.cooper3@citrix.com> 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.

Jan


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