Re: [PATCH v5 5/7] xen/riscv: introduce trap_init()

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

Hi Julien,

On Tue, 2023-03-21 at 17:42 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 16/03/2023 14:39, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > Reviewed-by: Alistair Francis <alistair.francis@xxxxxxx>
> > ---
> > Changes in V5:
> >    - Nothing changed
> > ---
> > Changes in V4:
> >    - Nothing changed
> > ---
> > Changes in V3:
> >    - Nothing changed
> > ---
> > Changes in V2:
> >    - Rename setup_trap_handler() to trap_init().
> >    - Add Reviewed-by to the commit message.
> > ---
> >   xen/arch/riscv/include/asm/traps.h | 1 +
> >   xen/arch/riscv/setup.c             | 5 +++++
> >   xen/arch/riscv/traps.c             | 7 +++++++
> >   3 files changed, 13 insertions(+)
> > 
> > diff --git a/xen/arch/riscv/include/asm/traps.h
> > b/xen/arch/riscv/include/asm/traps.h
> > index f3fb6b25d1..f1879294ef 100644
> > --- a/xen/arch/riscv/include/asm/traps.h
> > +++ b/xen/arch/riscv/include/asm/traps.h
> > @@ -7,6 +7,7 @@
> >   
> >   void do_trap(struct cpu_user_regs *cpu_regs);
> >   void handle_trap(void);
> > +void trap_init(void);
> >   
> >   #endif /* __ASSEMBLY__ */
> >   
> > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> > index 36556eb779..b44d105b5f 100644
> > --- a/xen/arch/riscv/setup.c
> > +++ b/xen/arch/riscv/setup.c
> > @@ -3,7 +3,9 @@
> >   #include <xen/kernel.h>
> >   
> >   #include <asm/boot-info.h>
> > +#include <asm/csr.h>
> >   #include <asm/early_printk.h>
> > +#include <asm/traps.h>
> >   
> >   /* Xen stack for bringing up the first CPU. */
> >   unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
> > @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long
> > bootcpu_id,
> >   
> >       fill_boot_info();
> >   
> > +    trap_init();
> > +
> >       early_printk("All set up\n");
> > +
> >       for ( ;; )
> >           asm volatile ("wfi");
> >   
> > diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
> > index 8a1529e0c5..581f34efbc 100644
> > --- a/xen/arch/riscv/traps.c
> > +++ b/xen/arch/riscv/traps.c
> > @@ -13,6 +13,13 @@
> >   #include <asm/processor.h>
> >   #include <asm/traps.h>
> >   
> > +void trap_init(void)
> > +{
> > +    unsigned long addr = (unsigned long)&handle_trap;
> 
> It is not super clear to me whether this is going to store the
> virtual 
> or physical address.
Actually it is going to store both the virtual and physical address.
Depending on if MMU is enabled or not.
> 
> Depending on the answer, the next would be whether the value would
> still 
> be valid after the MMU is turned on?
It would still be valid because for addr will be generated PC-relative
address.
> 
> > +
> > +    csr_write(CSR_STVEC, addr);
> > +}
> > +
> >   static const char *decode_trap_cause(unsigned long cause)
> >   {
> >       static const char *const trap_causes[] = {
> 
> Cheers,
> 
> ~ Oleksii