Re: [PATCH 3/5] libxencall: introduce variant of xencall2() returning long

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/06/2021 11:24, Jan Beulich wrote:
> Some hypercalls, memory-op in particular, can return values requiring
> more than 31 bits to represent. Hence the underlying layers need to make
> sure they won't truncate such values.
>
> While adding the new function to the map file, I noticed the stray
> xencall6 there. I'm taking the liberty to remove it at this occasion. If
> such a function would ever appear, it shouldn't lane in version 1.0.

s/lane/land/ ?

I'm tempted to suggest spitting this out into a separate change anyway. 
I'm not sure of the implications on the ABI.

ABI-dumper appears not to have picked anything up, nor has readelf on
the object itself, so we're probably ok ABI wise.

That said, I would really have expected a compile/link error for a bad
symbol in a map file.

>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I wasn't sure whether euqivalents for the other xencall<N>() should also
> be introduced, and hence went for the minimal solution first. Otoh there
> is also xencall0() which has no users ...
>
> --- a/tools/include/xencall.h
> +++ b/tools/include/xencall.h
> @@ -113,6 +113,10 @@ int xencall5(xencall_handle *xcall, unsi
>               uint64_t arg1, uint64_t arg2, uint64_t arg3,
>               uint64_t arg4, uint64_t arg5);
>  
> +/* Variant(s) of the above, as needed, returning "long" instead of "int". */
> +long xencall2L(xencall_handle *xcall, unsigned int op,

If we're fixing ABIs, can we see about not truncate op on the way up?

> +               uint64_t arg1, uint64_t arg2);
> +
>  /*
>   * Allocate and free memory which is suitable for use as a pointer
>   * argument to a hypercall.
> --- a/tools/libs/call/core.c
> +++ b/tools/libs/call/core.c
> @@ -127,6 +127,17 @@ int xencall2(xencall_handle *xcall, unsi
>      return osdep_hypercall(xcall, &call);
>  }
>  
> +long xencall2L(xencall_handle *xcall, unsigned int op,
> +               uint64_t arg1, uint64_t arg2)
> +{
> +    privcmd_hypercall_t call = {
> +        .op = op,
> +        .arg = { arg1, arg2 },
> +    };
> +
> +    return osdep_hypercall(xcall, &call);

(If we're not changing op), I take it there are no alias tricks we can
play to reuse the same implementation?

~Andrew