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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2023-01-25 at 17:15 +0000, Andrew Cooper wrote:
> On 25/01/2023 5:01 pm, 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.
> > And in future it will be changed to panic() so we won't need here
> > wfi()
> > any more.
> 
> WFI is permitted to be implemented as a NOP by hardware. 
> Furthermore,
> WFI with interrupts already disabled is a supported usecase, and will
> resume execution without taking the interrupt that became pending.
> 
> You need an infinite loop of WFI's for execution to halt here.
> 
Thanks a lot for clarification!
Then definitely it should changed to loop.
> ~Andrew