Re: [PATCH WIP] xen/public: move incomplete type definitions to xen.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.09.2023 17:42, Elliott Mitchell wrote:
> On Fri, Sep 22, 2023 at 10:21:21AM +0200, Jan Beulich wrote:
>> On 21.09.2023 18:18, Elliott Mitchell wrote:
>>>  As such these incomplete definitions should be
>>> in xen.h next to their hypercalls, rather than spread all over.
>>
>> Perhaps s/incomplete definitions/forward declarations/.
>>
>> There's a downside to the movement, though: You now introduce items
>> into the namespace which may be entirely unused. The two contradicting
>> goals need weighing as to their usefulness.
> 
> For the case which this is part of, they're not 100% unused.
> 
>>> trap_info_t is particularly notable since even though the hypercall is
>>> x86-only, the wrapper is likely to be visible to generic source code.
>>
>> Why would it be?
> 
> Related to converting ARM to using inline assembly-language wrappers
> instead of the current declarations+small assembly wrapper function.
> 
> The first step is you split the Linux header
> arch/x86/include/asm/xen/hypercall.h.  The upper portion (the x86
> inline assembly language) remains in arch/x86/include, all the
> HYPERVISOR_*() wrappers go into include/xen/$somewhere.  Several months
> ago I sent a candidate header to implement _hypercall#() for ARM.
> 
> Problem is:
> static inline int
> HYPERVISOR_set_trap_table(struct trap_info *table)
> {
>         return _hypercall1(int, set_trap_table, table);
> }
> Without without `struct trap_info;` somewhere, this fails.
> 
> Now, this isn't used on ARM, but this is tricky to guess.  Someone
> setting this up won't know whether any given function is absent due to
> being legacy and unlikely to ever be on non-x86.  Versus simply not /yet/
> being available on non-x86 (vPCI).
> 
> Perhaps xen/include/public/xen.h should only conditionally #define some
> of the __HYPERVISOR_* constants.  Likely there should be a way to force
> all the hypercall numbers to be available (for linting).  Yet as the
> current Linux header hints, perhaps there should be a way to disable the
> PV constants even on x86.

Downstream consumers of the public headers are free to adjust them to their
needs. The upstream form wants to remain sufficiently generic, which to me
includes not exposing types which aren't relevant for a particular arch.

Jan