Re: [PATCH v5 4/7] xen/riscv: introduce decode_cause() stuff

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2023-03-22 at 14:46 +0100, Jan Beulich wrote:
> On 22.03.2023 14:32, Oleksii wrote:
> > On Wed, 2023-03-22 at 13:26 +0100, Jan Beulich wrote:
> > > But as said before - I'm unconvinced this approach will scale,
> > > because
> > > you'll need to apply the wrapper to anything which can be reached
> > > prior
> > > to you enabling the MMU. Whether you can contain this to RISC-V-
> > > only
> > > code is unclear; I don't think it'll be acceptable to change any
> > > part
> > > of common code to meet your special needs.
> > But it looks like it is only two places where it should be done:
> > 1. As you mentioned LINK_TO_LOAD() should be applied for
> > trap_causes.
> > 2. And it should be applied inside do_bug_frame() for getting an
> > start/end address of bug_frame. I want to note that do_bug_frame()
> > will
> > be removed after RISC-V is ready to switch to generic bug
> > implementation.
> > 
> > The next step after the current patch series is merged is to enable
> > MMU, so it shouldn't be new use cases where it is needed to use
> > LINK_TO_LOAD().
> 
> I'm not convinced. You can't stick to using earlyprintk only beyond
> very short term. Yet I expect you also don't want to use
> 
>     if ( early )
>         earlyprintk();
>     else
>         printk();
> 
> everywhere in exception handling code (and anywhere else in code
> which is reachable before the MMU is turned on). This extends to
> uses of BUG() and alike in such early code, which - when they
> trigger - want to use printk() (only). Whether printk(), somehow,
> involves an array access similar to the ones you're presently
> aware of you simply shouldn't depend on (it's an implementation
> detail in a separate subsystem).
I planned to changed all earlyprintk() to printk() in traps.c after
printk will be ready.

I would like to remind that xen/common code isn't compiled now for
RISC-V.
> 
> > If it is not acceptable to change any part of common code ( as I
> > understand in this case it is do_unexpected_trap() and all that is
> > called inside it ) have I to introduce two types of function
> > do_unexpected_trap() for when MMU is enabled and not?
> 
> By "common code" I mean code outside of arch/riscv/. And I
> sincerely hope you / we can get away without duplicated functions.
Got it.

Than it might be an issue with do_bug_frame() as RISCV should be
switched to it at the end.

Then it looks like enabling of MMU should go first but in that case
this not clear what to do with BUG(), WARN() etc as for them it is
needed excpetion handling functionality and MMU related code uses the
macros.

~ Oleksii