[v9,3/5] xen/riscv: introduce trap_init()

Commit Message

Oleksii Kurochko July 2, 2024, 11:23 a.m. UTC

Signed-off-by: Oleksii Kurochko <[e-mail]>
Reviewed-by: Alistair Francis <[e-mail]>
---
 xen/arch/riscv/include/asm/traps.h |  1 +
 xen/arch/riscv/setup.c             |  3 +++
 xen/arch/riscv/traps.c             | 12 ++++++++++++
 3 files changed, 16 insertions(+)

Comments

Jan Beulich July 10, 2024, 9:50 a.m. UTC | #1

On 02.07.2024 13:23, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/traps.c
> +++ b/xen/arch/riscv/traps.c
> @@ -12,6 +12,18 @@
>  #include <asm/riscv_encoding.h>
>  #include <asm/traps.h>
>  
> +/*
> + * Initialize the trap handling.
> + *
> + * The function is called after MMU is enabled.
> + */
> +void trap_init(void)

It may be deliberate that this isn't __init, but for the present needs it
certainly could be. This is the kind of thing that imo want saying in the
description (which sadly once again is entirely empty).

Jan

Oleksii Kurochko July 11, 2024, 8:19 a.m. UTC | #2

On Wed, 2024-07-10 at 11:50 +0200, Jan Beulich wrote:
> On 02.07.2024 13:23, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/traps.c
> > +++ b/xen/arch/riscv/traps.c
> > @@ -12,6 +12,18 @@
> >  #include <asm/riscv_encoding.h>
> >  #include <asm/traps.h>
> >  
> > +/*
> > + * Initialize the trap handling.
> > + *
> > + * The function is called after MMU is enabled.
> > + */
> > +void trap_init(void)
> 
> It may be deliberate that this isn't __init, but for the present
> needs it
> certainly could be. This is the kind of thing that imo want saying in
> the
> description (which sadly once again is entirely empty).
I just missed to add __init so I have to added it in the next patch
version.

Thanks for finding that.

~ Oleksii

Andrew Cooper July 11, 2024, 8:28 a.m. UTC | #3

On 11/07/2024 9:19 am, Oleksii wrote:
> On Wed, 2024-07-10 at 11:50 +0200, Jan Beulich wrote:
>> On 02.07.2024 13:23, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/traps.c
>>> +++ b/xen/arch/riscv/traps.c
>>> @@ -12,6 +12,18 @@
>>>  #include <asm/riscv_encoding.h>
>>>  #include <asm/traps.h>
>>>  
>>> +/*
>>> + * Initialize the trap handling.
>>> + *
>>> + * The function is called after MMU is enabled.
>>> + */
>>> +void trap_init(void)
>> It may be deliberate that this isn't __init, but for the present
>> needs it
>> certainly could be. This is the kind of thing that imo want saying in
>> the
>> description (which sadly once again is entirely empty).
> I just missed to add __init so I have to added it in the next patch
> version.
>
> Thanks for finding that.

Don't be so hasty.  It will definitely need to not be __init as soon as
you add multi-CPU support to Xen.

Just say so in the commit message, and save yourself needing to undo the
__init in a later patch.

~Andrew

Patch

diff --git a/xen/arch/riscv/include/asm/traps.h b/xen/arch/riscv/include/asm/traps.h
index 3fef318478..c30118e095 100644
--- a/xen/arch/riscv/include/asm/traps.h
+++ b/xen/arch/riscv/include/asm/traps.h
@@ -9,6 +9,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 e3cb0866d5..a6a29a1508 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -8,6 +8,7 @@ 
 #include <public/version.h>
 
 #include <asm/early_printk.h>
+#include <asm/traps.h>
 
 void arch_get_xen_caps(xen_capabilities_info_t *info)
 {
@@ -23,6 +24,8 @@  void __init noreturn start_xen(unsigned long bootcpu_id,
 {
     remove_identity_mapping();
 
+    trap_init();
+
     printk("All set up\n");
 
     for ( ;; )
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index 37cec40dfa..cb18b30ff2 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -12,6 +12,18 @@ 
 #include <asm/riscv_encoding.h>
 #include <asm/traps.h>
 
+/*
+ * Initialize the trap handling.
+ *
+ * The function is called after MMU is enabled.
+ */
+void trap_init(void)
+{
+    unsigned long addr = (unsigned long)&handle_trap;
+
+    csr_write(CSR_STVEC, addr);
+}
+
 static const char *decode_trap_cause(unsigned long cause)
 {
     static const char *const trap_causes[] = {