Re: [PATCH v6 1/9] xen/riscv: prevent recursion when ASSERT(), BUG*(), or panic() are called

[oleksii.kurochko@gmail.com]

On Tue, 2024-09-10 at 11:42 +0200, Jan Beulich wrote:
> On 02.09.2024 19:01, Oleksii Kurochko wrote:
> > Implement machine_restart() using printk() to prevent recursion
> > that
> > occurs when ASSERT(), BUG*(), or panic() are invoked.
> > All these macros (except panic() which could be called directly)
> > eventually call panic(), which then calls machine_restart(),
> > leading to a recursive loop.
> 
> Right, that pretty likely was an oversight. Yet then ...
> 
> > --- a/xen/arch/riscv/stubs.c
> > +++ b/xen/arch/riscv/stubs.c
> > @@ -53,7 +53,7 @@ void domain_set_time_offset(struct domain *d,
> > int64_t time_offset_seconds)
> >  
> >  void machine_restart(unsigned int delay_millisecs)
> >  {
> > -    BUG_ON("unimplemented");
> > +    printk("%s: unimplemented\n", __func__);
> >  }
> 
> ... you still want to halt execution here, by (re?)adding a for()
> loop
> of the kind you at least had in a few places earlier on. The function
> is declared noreturn after all, which you're now violating. I'm
> actually surprised the compiler didn't complain to you.
> 
> The same is also going to be needed for machine_halt(), btw: As soon
> as you get far enough to parse the command line, "noreboot" on the
> command line would have crashes end up there, not here.

I will drop this patch in the next version as Andrew C. provides the
patch:
https://gitlab.com/xen-project/people/olkur/xen/-/commit/ea6d5a148970a7f8066e51e64fe67a9bd51e3084


~ Oleksii