Re: [PATCH v1 09/14] xen/riscv: introduce do_unexpected_trap()

[Julien Grall <julien@xxxxxxx>]

Hi,

On 25/01/2023 17:01, Oleksii wrote:
> On Mon, 2023-01-23 at 09:39 +1000, Alistair Francis wrote:
> > On Sat, Jan 21, 2023 at 1:00 AM Oleksii Kurochko
> > <oleksii.kurochko@xxxxxxxxx> wrote:
> > > The patch introduces the function the purpose of which is to print
> > > a cause of an exception and call "wfi" instruction.
> > >
> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > > ---
> > >   xen/arch/riscv/traps.c | 14 +++++++++++++-
> > >   1 file changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
> > > index dd64f053a5..fc25138a4b 100644
> > > --- a/xen/arch/riscv/traps.c
> > > +++ b/xen/arch/riscv/traps.c
> > > @@ -95,7 +95,19 @@ const char *decode_cause(unsigned long cause)
> > >       return decode_trap_cause(cause);
> > >   }
> > >
> > > -void __handle_exception(struct cpu_user_regs *cpu_regs)
> > > +static void do_unexpected_trap(const struct cpu_user_regs *regs)
> > >   {
> > > +    unsigned long cause = csr_read(CSR_SCAUSE);
> > > +
> > > +    early_printk("Unhandled exception: ");
> > > +    early_printk(decode_cause(cause));
> > > +    early_printk("\n");
> > > +
> > > +    // kind of die...
> > >       wait_for_interrupt();
> > We could put this in a loop, to ensure we never progress
> I think that right now there is no big difference how to stop
> because we have only 1 CPU, interrupts are disabled and we are in
> exception so it looks like anything can interrupt us.
From my understanding of the specification, WFI is an hint. So it could 
be implemented as a NOP.
Therefore it would sound better to wrap in a loop. That said...

> And in future it will be changed to panic() so we won't need here wfi()
> any more.
... ideally using panic() right now would be the best.

Cheers,

--
Julien Grall