Re: [PATCH v1 09/14] xen/riscv: introduce do_unexpected_trap()
On Sat, Jan 21, 2023 at 1:00 AM Oleksii Kurochko
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
> ---
> 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
>
>
Re: [PATCH v1 09/14] xen/riscv: introduce do_unexpected_trap()
On Mon, 2023-01-23 at 09:39 +1000, Alistair Francis wrote:
> On Sat, Jan 21, 2023 at 1:00 AM Oleksii Kurochko
> 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
> > ---
> > 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
Re: [PATCH v1 09/14] xen/riscv: introduce do_unexpected_trap()
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
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
---
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,
--
Julien Grall
Re: [PATCH v1 09/14] xen/riscv: introduce do_unexpected_trap()
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
>> 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
>>> ---
>>> 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
Re: [PATCH v1 09/14] xen/riscv: introduce do_unexpected_trap()
On Wed, 2023-01-25 at 17:15 +, 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
> > > 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
> > > > ---
> > > > 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
