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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 13 Nov 2024, Jan Beulich wrote:
> On 13.11.2024 04:15, Stefano Stabellini wrote:
> > It is challenging to create a solution that satisfies everyone for this
> > patch. However, we should add R8.3 to the clean list as soon as possible
> > to enable rule blocking in GitLab-CI. Failing to do so risks introducing
> > regressions, as recently occurred, undoing the significant efforts made
> > by Bugseng and the community over the past year.
> > 
> > Unless there is a specific counterproposal, let us proceed with
> > committing this patch.
> 
> Well, I find this odd. We leave things sit in limbo for months and then
> want to go ahead with a controversial solution? Rather than actually
> (and finally) sorting out the underlying disagreement (of which there
> are actually two sufficiently separate parts)? Plus ...

The reason is that several MISRA regressions were recently introduced.
These regressions could have been easily detected by GitLab CI if we had
marked the rules as clean. I believe we should expedite accepting the
fixes and marking the rules as clean. We can always adjust the fixes or
deviations later to better suit our preferences. In my opinion, we
should prioritize marking the rules as clean.


> > On Mon, 24 Jun 2024, Jan Beulich wrote:
> >> 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 question of mine effectively represents a concrete alternative
> proposal (or even two, if you like).
> 
> The two parts where there appears to be disagreement are:
> 1) When to (not) use fixed width types, as presently outlined in
>    ./CODING_STYLE.
> 2) How to type C function parameters called solely from assembly code (of
>    which the hypercall handlers are a subset).
> 
> And maybe
> 2b) How to best express such function parameters when they're (sometimes)
>     shared between native and compat handlers.
> 
> Of course 2) is affected by, as Andrew validly says, there not being a
> formally clean ABI definition.
> 
> My fear is that if this gets committed as is, it'll be used as a handle to
> force in further similarly questionable / controversial changes to other
> hypercall handlers. Which is why I think the controversy needs sorting out
> first (which admittedly is hard when the ABI is fuzzy).

While I appreciate your concern, as you know, aligning on the topics
above takes time. I do not believe it is in the interest of the
community, both contributors and reviewers, to delay marking this rule
as clean. Honestly, I do not mind how it gets marked as clean, as long
as we do it soon.

Additionally, please keep in mind that the Xen Project tends to have a
long memory. As a result, there is usually little risk of the so-called
"slippery slope" problem.

If you prefer a deviation I am OK with that too. I just want 8.3 as
clean :-)