Re: [PATCH] consolidate do_bug_frame() / bug_fn_t

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.01.2024 02:14, Andrew Cooper wrote:
> On 24/01/2024 7:28 am, Jan Beulich wrote:
>> On 24.01.2024 02:34, Stefano Stabellini wrote:
>>> I managed to get back to read the mailing list and noticed this patch.
>>>
>>> Is it still relevant and needs to be reviewed?
>>>
>>> Are there any outstanding disagreements between maintainers on the
>>> approach to take here?  Or should I just go ahead and review it?
>> It is still relevant from my pov, and everything that may be controversial
>> is said ...
> 
> BUGFRAME_* cannot legitimately modify the interrupted context.  Two are
> fatal paths, and other two are side-effect-less as far as C can tell.
> 
> So the infrastructure ought to take a const pointer.
> 
> The reason why this pointer is non-const is to do with the interaction
> of the serial and keyhandler infrastructures.  Because we're adjusting
> that for other reasons, I was hoping it would subsequently be easy to
> switch Xen to being properly const in this regard.
> 
> Turns out it is:
> 
>  
> https://gitlab.com/xen-project/people/andyhhp/xen/-/commit/4f857075005da1d28632e4f9198c2e7d0f404b9a
> 
> with a couple of caveats.  (Only the buster-gcc-ibt run failed, so I've
> got some cf_check-ing to adjust, but all the other builds worked fine).
> 
> 
> To make the serial code compile, I ended up having to revert patch 2 of
> the regs series, which I believe is safe to do following patch 3-5 which
> un-plumb the regs pointer deeper in the call chain.  If this is turns
> out to be true, then the patch ought to be added and reverted in the
> same series so it isn't left hanging about after the fact.

Looking further into this, I can't see how reverting ought to be possible,
even less so specifically after patch 5. Patches 4 and 5 merely eliminate
now unused parameters. Hence imo if it could be reverted after 5, it also
ought to be fine to revert after 3. Which in turn it would mean it's not
needed at all. Which I simply cannot see (yet?).

Jan