Re: [PATCH 2/2] xen/multicall: Change nr_calls to uniformly be unsigned long

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.06.2024 22:58, Andrew Cooper wrote:
> Right now, the non-compat declaration and definition of do_multicall()
> differing types for the nr_calls parameter.
> 
> This is a MISRA rule 8.3 violation, but it's also time-bomb waiting for the
> first 128bit architecture (RISC-V looks as if it might get there first).
> 
> Worse, the type chosen here has a side effect of truncating the guest
> parameter, because Xen still doesn't have a clean hypercall ABI definition.
> 
> Switch uniformly to using unsigned long.

And re-raising all the same question again: Why not uniformly unsigned int?
Or uint32_t?

> This addresses the MISRA violation, and while it is a guest-visible ABI
> change, it's only in the corner case where the guest kernel passed a
> bogus-but-correct-when-truncated value.  I can't find any any users of
> mutilcall which pass a bad size to begin with, so this should have no
> practical effect on guests.
> 
> In fact, this brings the behaviour of multicalls more in line with the header
> description of how it behaves.

Which description? If you mean ...

> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -623,7 +623,7 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
>  /*
>   * ` enum neg_errnoval
>   * ` HYPERVISOR_multicall(multicall_entry_t call_list[],
> - * `                      uint32_t nr_calls);
> + * `                      unsigned long nr_calls);
>   *
>   * NB. The fields are logically the natural register size for this
>   * architecture. In cases where xen_ulong_t is larger than this then

... this comment here, note how is says "fields", i.e. talks about the
subsequent struct.

What you're doing is effectively an ABI change: All of the sudden the
upper bits of the incoming argument would be respected. Yes, it is
overwhelmingly likely that no-one would ever pass such a value. Yet
iirc on other similar hypercall handler adjustments in the past you
did raise a similar concern.

Jan