| Message ID | 3d32a952c7cc77fd759e211c3b60427485a75582.1725295716.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | RISCV device tree mapping | expand |
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
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
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c index 3285d18899..144f1250e1 100644 --- 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__); } void machine_halt(void)
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. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in v6: - new patch. --- xen/arch/riscv/stubs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)