| Message ID | 74ca10d9be1dfc3aed4b3b21a79eae88c9df26a4.1674226563.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RISCV basic exception handling implementation | expand |
On Sat, Jan 21, 2023 at 1:00 AM Oleksii Kurochko <oleksii.kurochko@gmail.com> 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@gmail.com> > --- > 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 Alistair > } > + > +void __handle_exception(struct cpu_user_regs *cpu_regs) > +{ > + do_unexpected_trap(cpu_regs); > +} > -- > 2.39.0 > >
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@gmail.com> 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@gmail.com> > > --- > > 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. > Oleksii
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@gmail.com> 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@gmail.com> >>> --- >>> 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,
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@gmail.com> 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@gmail.com> >>> --- >>> 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. ~Andrew
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@gmail.com> 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@gmail.com> > > > > --- > > > > 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
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(); } + +void __handle_exception(struct cpu_user_regs *cpu_regs) +{ + do_unexpected_trap(cpu_regs); +}
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@gmail.com> --- xen/arch/riscv/traps.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)