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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan