On 14.11.2024 03:34, Stefano Stabellini wrote: > 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 :-)
No, please no deviations when we can avoid them. Since it feels like it's always (going to be?) me to give in when there is such disagreement, why don't I do so here as well: Go ahead. Jan
