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
