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

[Julien Grall <julien@xen.org>]

Hi Oleksii,

On 16/03/2023 14:39, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> 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.

Depending on the answer, the next would be whether the value would still 
be valid after the MMU is turned on?

> +
> +    csr_write(CSR_STVEC, addr);
> +}
> +
>   static const char *decode_trap_cause(unsigned long cause)
>   {
>       static const char *const trap_causes[] = {

Cheers,

-- 
Julien Grall