Re: [PATCH] consolidate do_bug_frame() / bug_fn_t

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.01.2024 13:10, Andrew Cooper wrote:
> Answering out of order:
> 
> On 25/01/2024 8:15 am, Jan Beulich wrote:
>> All fine. Still I wonder whether in the meantime this patch isn't an
>> improvement on its own, and hence whether the const couldn't sensibly
>> be added subsequently.
> 
> We have a while until 4.19 goes out.  I would prefer to try and get this
> untangled properly, because half of your patch is in the opposite
> direction for getting the const-ness working.
> 
> If we start to hit rc1 and the const-ness isn't complete, we can revisit.
> 
> 
> Regarding the removal of gdb-stub.  I'd like to get that done, and
> rebase the remainder of the IRQ series over it, because it will reduce
> the churn in your IRQ series.  I'm happy to do the rebase if you want.

Shouldn't be overly difficult, so I guess I can as well do it before
(eventually) re-submitting.

>> On 25.01.2024 02:14, Andrew Cooper wrote:
>>> 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.
>> Hmm, I'm not sure I see how reverting that would end up working. However,
>> aiui you need to revert primarily for the non-const-ness of the pointers
>> involved in [gs]et_irq_regs(). I wonder whether, if we followed your
>> underlying thought here, those shouldn't be const-ified then anyway.
>>
>>> The _$X_poll() functions are used in timer context, which means there's
>>> an outer regs context already latched, and that's arguably a better
>>> context to use anyway for 'd'.
>> If the timer happens to run on an idle vCPU, what "outer regs context"
>> would we have there?
> 
> The only reason the serial infrastructure was setting up a fake IRQ
> context was because it was using run_in_exception_handler().
> 
> But I (think I have) removed that fully (and it simplifies things more
> than I was hoping).
> 
> With '%' deleted, it's only 'd' that cares, isn't it?

Yes.

>  And that's "dump the current regs", rather than wanting something else.

Question remains though: What are "current regs" when on an idle vCPU?
But perhaps that will clarify itself once I see how you "removed that
fully".

Jan