[v12] RISCV basic exception handling implementation

[PATCH v12 3/3] xen/riscv: refactor decode_trap_cause()

Posted by Oleksii Kurochko 3 months, 3 weeks ago

Use array_access_nospec() to prevent guest speculation.

Avoid double access of trap_causes[cause].

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V12:
 - New patch.
---
 xen/arch/riscv/traps.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index 72ffdcd79e..51f6e45ccc 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -7,6 +7,7 @@
 
 #include <xen/bug.h>
 #include <xen/lib.h>
+#include <xen/nospec.h>
 #include <xen/sched.h>
 
 #include <asm/processor.h>
@@ -48,9 +49,10 @@ static const char *decode_trap_cause(unsigned long cause)
         [CAUSE_STORE_GUEST_PAGE_FAULT] = "Guest Store/AMO Page Fault",
     };
 
-    if ( cause < ARRAY_SIZE(trap_causes) && trap_causes[cause] )
-        return trap_causes[cause];
-    return "UNKNOWN";
+    const char *res = cause < ARRAY_SIZE(trap_causes) ? array_access_nospec(trap_causes, cause)
+                                                      : NULL;
+
+    return res ?: "UNKNOWN";
 }
 
 static const char *decode_reserved_interrupt_cause(unsigned long irq_cause)
-- 
2.45.2

Re: [PATCH v12 3/3] xen/riscv: refactor decode_trap_cause()

Posted by Jan Beulich 3 months, 2 weeks ago

On 02.08.2024 15:54, Oleksii Kurochko wrote:
> Use array_access_nospec() to prevent guest speculation.
> 
> Avoid double access of trap_causes[cause].
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with ...

> @@ -48,9 +49,10 @@ static const char *decode_trap_cause(unsigned long cause)
>          [CAUSE_STORE_GUEST_PAGE_FAULT] = "Guest Store/AMO Page Fault",
>      };
>  
> -    if ( cause < ARRAY_SIZE(trap_causes) && trap_causes[cause] )
> -        return trap_causes[cause];
> -    return "UNKNOWN";
> +    const char *res = cause < ARRAY_SIZE(trap_causes) ? array_access_nospec(trap_causes, cause)

... the overly long line here suitably wrapped; commonly we'd do this
as ...

> +                                                      : NULL;

    const char *res = cause < ARRAY_SIZE(trap_causes)
                      ? array_access_nospec(trap_causes, cause)
                      : NULL;

I guess I'll adjust this while committing.

Jan

> +    return res ?: "UNKNOWN";
>  }
>  
>  static const char *decode_reserved_interrupt_cause(unsigned long irq_cause)

Re: [PATCH v12 3/3] xen/riscv: refactor decode_trap_cause()

Posted by oleksii.kurochko@gmail.com 3 months, 2 weeks ago

On Mon, 2024-08-05 at 08:20 +0200, Jan Beulich wrote:
> On 02.08.2024 15:54, Oleksii Kurochko wrote:
> > Use array_access_nospec() to prevent guest speculation.
> > 
> > Avoid double access of trap_causes[cause].
> > 
> > Suggested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with ...
> 
> > @@ -48,9 +49,10 @@ static const char *decode_trap_cause(unsigned
> > long cause)
> >          [CAUSE_STORE_GUEST_PAGE_FAULT] = "Guest Store/AMO Page
> > Fault",
> >      };
> >  
> > -    if ( cause < ARRAY_SIZE(trap_causes) && trap_causes[cause] )
> > -        return trap_causes[cause];
> > -    return "UNKNOWN";
> > +    const char *res = cause < ARRAY_SIZE(trap_causes) ?
> > array_access_nospec(trap_causes, cause)
> 
> ... the overly long line here suitably wrapped; commonly we'd do this
> as ...
> 
> > +                                                      : NULL;
> 
>     const char *res = cause < ARRAY_SIZE(trap_causes)
>                       ? array_access_nospec(trap_causes, cause)
>                       : NULL;
> 
> I guess I'll adjust this while committing.
I will be happy with that. Thanks!

~ Oleksii
> 
> Jan
> 
> > +    return res ?: "UNKNOWN";
> >  }
> >  
> >  static const char *decode_reserved_interrupt_cause(unsigned long
> > irq_cause)
>