Re: [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing

[Oleksii Kurochko]

On 2/7/25 11:01 PM, Andrew Cooper wrote:

RISC-V has less complicated headers, so update ubsan.c to pull in everything
it needs.  Provide dump_execution_state(), and update the printk() message to
make it more obvious that it's an outstanding task.

As with commit 8ef2ac727e21 ("automation: enable UBSAN for debug tests"),
enable UBSAN in RISC-V testing too.

No functional change.

Signed-off-by: Andrew Cooper<andrew.coop...@citrix.com>
---
CC: Oleksii Kurochko<oleksii.kuroc...@gmail.com>
CC: Anthony PERARD<anthony.per...@vates.tech>
CC: Michal Orzel<michal.or...@amd.com>
CC: Jan Beulich<jbeul...@suse.com>
CC: Julien Grall<jul...@xen.org>
CC: Roger Pau Monné<roger....@citrix.com>
CC: Stefano Stabellini<sstabell...@kernel.org>

Testing of this series:
   https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9078817715

Sample run with an intentional UBSAN failure:
   https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9078570135
---
  automation/gitlab-ci/build.yaml        | 3 +++
  xen/arch/riscv/Kconfig                 | 1 +
  xen/arch/riscv/include/asm/processor.h | 2 ++
  xen/arch/riscv/traps.c                 | 2 +-
  xen/common/ubsan/ubsan.c               | 5 ++++-
  5 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index fb55d4ce5568..35e224366f62 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -359,6 +359,9 @@ debian-12-riscv64-gcc-debug:
      CONTAINER: debian:12-riscv64
      KBUILD_DEFCONFIG: tiny64_defconfig
      HYPERVISOR_ONLY: y
+    EXTRA_XEN_CONFIG: |
+      CONFIG_UBSAN=y
+      CONFIG_UBSAN_FATAL=y

# Arm32 cross-build diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig

index 00f329054c94..fa95cd0a4213 100644
--- a/xen/arch/riscv/Kconfig
+++ b/xen/arch/riscv/Kconfig
@@ -4,6 +4,7 @@ config RISCV
        select GENERIC_BUG_FRAME
        select HAS_DEVICE_TREE
        select HAS_PMAP
+       select HAS_UBSAN
        select HAS_VMAP

config RISCV_64

diff --git a/xen/arch/riscv/include/asm/processor.h 
b/xen/arch/riscv/include/asm/processor.h
index 90b800956303..39696fb58dc6 100644
--- a/xen/arch/riscv/include/asm/processor.h
+++ b/xen/arch/riscv/include/asm/processor.h
@@ -91,6 +91,8 @@ static inline void sfence_vma(void)
      asm volatile ( "sfence.vma" ::: "memory" );
  }

+#define dump_execution_state() run_in_exception_handler(show_execution_state)

+
  #endif /* __ASSEMBLY__ */

#endif /* ASM__RISCV__PROCESSOR_H */

diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index d55a4a827b8c..ea3638a54fed 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -140,7 +140,7 @@ void vcpu_show_execution_state(struct vcpu *v)

void show_execution_state(const struct cpu_user_regs *regs)

  {
-    printk("implement show_execution_state(regs)\n");
+    printk("TODO: Implement show_execution_state(regs)\n");
  }

void arch_hypercall_tasklet_result(struct vcpu *v, long res)

diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
index 7f73f94759db..e99370322b44 100644
--- a/xen/common/ubsan/ubsan.c
+++ b/xen/common/ubsan/ubsan.c
@@ -10,8 +10,11 @@
   *
   */

-#include <xen/spinlock.h>

+#include <xen/bitops.h>
+#include <xen/kernel.h>
+#include <xen/lib.h>
  #include <xen/percpu.h>
+#include <xen/spinlock.h>

I am not insisting on to have these changes in a separate patch, but they don't 
really
look as RISC-V specific.

Anyway, changes look good to me, so:
 Reviewed-by: Oleksii Kurochko<oleksii.kuroc...@gmail.com>

Thanks.

~ Oleksii

#define __noreturn noreturn

  #define pr_err(...) printk(XENLOG_ERR __VA_ARGS__)