* [PATCH for-4.20 1/3] RISCV/boot: Run constructors during setup
2025-02-07 22:01 [PATCH for-4.20 0/3] RISCV: Bugfixes and UBSAN Andrew Cooper
@ 2025-02-07 22:01 ` Andrew Cooper
2025-02-10 8:49 ` Oleksii Kurochko
2025-02-07 22:01 ` [PATCH for-4.20 2/3] RISCV/asm: Use CALL rather than JAL Andrew Cooper
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2025-02-07 22:01 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Oleksii Kurochko, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini
Without this, RISC-V isn't running boot time selftests when they're compiled
in.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1660821676
For-4.20. Boot selftests are new in 4.20, and work in each other
archtiecture.
---
xen/arch/riscv/setup.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 38ca4f3baa1b..f2b6e684ac69 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -109,6 +109,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
*/
system_state = SYS_STATE_boot;
+ init_constructors();
+
if ( acpi_disabled )
{
printk("Booting using Device Tree\n");
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH for-4.20 1/3] RISCV/boot: Run constructors during setup
2025-02-07 22:01 ` [PATCH for-4.20 1/3] RISCV/boot: Run constructors during setup Andrew Cooper
@ 2025-02-10 8:49 ` Oleksii Kurochko
0 siblings, 0 replies; 10+ messages in thread
From: Oleksii Kurochko @ 2025-02-10 8:49 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
Roger Pau Monné, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]
On 2/7/25 11:01 PM, Andrew Cooper wrote:
> Without this, RISC-V isn't running boot time selftests when they're compiled
> in.
>
> Signed-off-by: Andrew Cooper<andrew.cooper3@citrix.com>
> ---
> CC: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> CC: Anthony PERARD<anthony.perard@vates.tech>
> CC: Michal Orzel<michal.orzel@amd.com>
> CC: Jan Beulich<jbeulich@suse.com>
> CC: Julien Grall<julien@xen.org>
> CC: Roger Pau Monné<roger.pau@citrix.com>
> CC: Stefano Stabellini<sstabellini@kernel.org>
>
> https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1660821676
>
> For-4.20. Boot selftests are new in 4.20, and work in each other
> archtiecture.
LGTM:
Reviewed-By: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Thanks.
~ Oleksii
> ---
> xen/arch/riscv/setup.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> index 38ca4f3baa1b..f2b6e684ac69 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -109,6 +109,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
> */
> system_state = SYS_STATE_boot;
>
> + init_constructors();
> +
> if ( acpi_disabled )
> {
> printk("Booting using Device Tree\n");
[-- Attachment #2: Type: text/html, Size: 2584 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH for-4.20 2/3] RISCV/asm: Use CALL rather than JAL
2025-02-07 22:01 [PATCH for-4.20 0/3] RISCV: Bugfixes and UBSAN Andrew Cooper
2025-02-07 22:01 ` [PATCH for-4.20 1/3] RISCV/boot: Run constructors during setup Andrew Cooper
@ 2025-02-07 22:01 ` Andrew Cooper
2025-02-10 8:57 ` Oleksii Kurochko
2025-02-07 22:01 ` [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing Andrew Cooper
2025-02-10 9:05 ` [PATCH for-4.20 0/3] RISCV: Bugfixes and UBSAN Oleksii Kurochko
3 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2025-02-07 22:01 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Oleksii Kurochko, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini
JAL has a maximium displacement of 2M. To branch further, it needs pairing
with an AUIPC instruction. CALL is a pseudo-op which allows the linker to
pick the appropriate sequence while processing relaxations.
This avoids a build failure of the form:
prelink.o: in function `start':
xen/xen/arch/riscv/riscv64/head.S:28:(.text.header+0x2c):
relocation truncated to fit: R_RISCV_JAL against symbol `calc_phys_offset' defined in .init.text section in prelink.o
make[3]: *** [arch/riscv/Makefile:18: xen-syms] Error 1
when Xen gets large enough, e.g. with CONFIG_UBSAN enabled.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
---
xen/arch/riscv/entry.S | 2 +-
xen/arch/riscv/riscv64/head.S | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/xen/arch/riscv/entry.S b/xen/arch/riscv/entry.S
index bf974655f8b3..4db818ba8d24 100644
--- a/xen/arch/riscv/entry.S
+++ b/xen/arch/riscv/entry.S
@@ -49,7 +49,7 @@ save_to_stack:
REG_S t0, CPU_USER_REGS_SSTATUS(sp)
mv a0, sp
- jal do_trap
+ call do_trap
restore_registers:
/* Restore stack_cpu_regs */
diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
index 2a1b3dad9191..9c40512e612e 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -28,7 +28,7 @@ FUNC(start)
add t3, t3, __SIZEOF_POINTER__
bltu t3, t4, .L_clear_bss
- jal reset_stack
+ call reset_stack
/*
* save hart_id ( bootcpu_id ) and dtb_base as a0 and a1 register can
@@ -37,16 +37,16 @@ FUNC(start)
mv s0, a0
mv s1, a1
- jal calc_phys_offset
+ call calc_phys_offset
mv s2, a0
- jal setup_initial_pagetables
+ call setup_initial_pagetables
/* Calculate proper VA after jump from 1:1 mapping */
la a0, .L_primary_switched
sub a0, a0, s2
- jal turn_on_mmu
+ call turn_on_mmu
.L_primary_switched:
/*
@@ -54,11 +54,11 @@ FUNC(start)
* recalculated after jump from 1:1 mapping world as 1:1 mapping
* will be removed soon in start_xen().
*/
- jal reset_stack
+ call reset_stack
/* Xen's boot cpu id is equal to 0 so setup TP register for it */
li a0, 0
- jal setup_tp
+ call setup_tp
/* restore hart_id ( bootcpu_id ) and dtb address */
mv a0, s0
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH for-4.20 2/3] RISCV/asm: Use CALL rather than JAL
2025-02-07 22:01 ` [PATCH for-4.20 2/3] RISCV/asm: Use CALL rather than JAL Andrew Cooper
@ 2025-02-10 8:57 ` Oleksii Kurochko
0 siblings, 0 replies; 10+ messages in thread
From: Oleksii Kurochko @ 2025-02-10 8:57 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
Roger Pau Monné, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]
On 2/7/25 11:01 PM, Andrew Cooper wrote:
> JAL has a maximium displacement of 2M. To branch further, it needs pairing
> with an AUIPC instruction. CALL is a pseudo-op which allows the linker to
> pick the appropriate sequence while processing relaxations.
>
> This avoids a build failure of the form:
>
> prelink.o: in function `start':
> xen/xen/arch/riscv/riscv64/head.S:28:(.text.header+0x2c):
> relocation truncated to fit: R_RISCV_JAL against symbol `calc_phys_offset' defined in .init.text section in prelink.o
> make[3]: *** [arch/riscv/Makefile:18: xen-syms] Error 1
>
> when Xen gets large enough, e.g. with CONFIG_UBSAN enabled.
>
> Signed-off-by: Andrew Cooper<andrew.cooper3@citrix.com>
LGTM: Reviewed-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Thanks.
~ Oleksii
> ---
> CC: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> CC: Anthony PERARD<anthony.perard@vates.tech>
> CC: Michal Orzel<michal.orzel@amd.com>
> CC: Jan Beulich<jbeulich@suse.com>
> CC: Julien Grall<julien@xen.org>
> CC: Roger Pau Monné<roger.pau@citrix.com>
> CC: Stefano Stabellini<sstabellini@kernel.org>
> ---
> xen/arch/riscv/entry.S | 2 +-
> xen/arch/riscv/riscv64/head.S | 12 ++++++------
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/riscv/entry.S b/xen/arch/riscv/entry.S
> index bf974655f8b3..4db818ba8d24 100644
> --- a/xen/arch/riscv/entry.S
> +++ b/xen/arch/riscv/entry.S
> @@ -49,7 +49,7 @@ save_to_stack:
> REG_S t0, CPU_USER_REGS_SSTATUS(sp)
>
> mv a0, sp
> - jal do_trap
> + call do_trap
>
> restore_registers:
> /* Restore stack_cpu_regs */
> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
> index 2a1b3dad9191..9c40512e612e 100644
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -28,7 +28,7 @@ FUNC(start)
> add t3, t3, __SIZEOF_POINTER__
> bltu t3, t4, .L_clear_bss
>
> - jal reset_stack
> + call reset_stack
>
> /*
> * save hart_id ( bootcpu_id ) and dtb_base as a0 and a1 register can
> @@ -37,16 +37,16 @@ FUNC(start)
> mv s0, a0
> mv s1, a1
>
> - jal calc_phys_offset
> + call calc_phys_offset
> mv s2, a0
>
> - jal setup_initial_pagetables
> + call setup_initial_pagetables
>
> /* Calculate proper VA after jump from 1:1 mapping */
> la a0, .L_primary_switched
> sub a0, a0, s2
>
> - jal turn_on_mmu
> + call turn_on_mmu
>
> .L_primary_switched:
> /*
> @@ -54,11 +54,11 @@ FUNC(start)
> * recalculated after jump from 1:1 mapping world as 1:1 mapping
> * will be removed soon in start_xen().
> */
> - jal reset_stack
> + call reset_stack
>
> /* Xen's boot cpu id is equal to 0 so setup TP register for it */
> li a0, 0
> - jal setup_tp
> + call setup_tp
>
> /* restore hart_id ( bootcpu_id ) and dtb address */
> mv a0, s0
[-- Attachment #2: Type: text/html, Size: 4273 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing
2025-02-07 22:01 [PATCH for-4.20 0/3] RISCV: Bugfixes and UBSAN Andrew Cooper
2025-02-07 22:01 ` [PATCH for-4.20 1/3] RISCV/boot: Run constructors during setup Andrew Cooper
2025-02-07 22:01 ` [PATCH for-4.20 2/3] RISCV/asm: Use CALL rather than JAL Andrew Cooper
@ 2025-02-07 22:01 ` Andrew Cooper
2025-02-08 2:39 ` Stefano Stabellini
2025-02-10 9:03 ` Oleksii Kurochko
2025-02-10 9:05 ` [PATCH for-4.20 0/3] RISCV: Bugfixes and UBSAN Oleksii Kurochko
3 siblings, 2 replies; 10+ messages in thread
From: Andrew Cooper @ 2025-02-07 22:01 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Oleksii Kurochko, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini
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.cooper3@citrix.com>
---
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@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>
#define __noreturn noreturn
#define pr_err(...) printk(XENLOG_ERR __VA_ARGS__)
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing
2025-02-07 22:01 ` [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing Andrew Cooper
@ 2025-02-08 2:39 ` Stefano Stabellini
2025-02-10 9:03 ` Oleksii Kurochko
1 sibling, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2025-02-08 2:39 UTC (permalink / raw)
To: Andrew Cooper
Cc: Xen-devel, Oleksii Kurochko, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]
On Fri, 7 Feb 2025, 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.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> CC: Anthony PERARD <anthony.perard@vates.tech>
> CC: Michal Orzel <michal.orzel@amd.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Julien Grall <julien@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@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>
>
> #define __noreturn noreturn
> #define pr_err(...) printk(XENLOG_ERR __VA_ARGS__)
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing
2025-02-07 22:01 ` [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing Andrew Cooper
2025-02-08 2:39 ` Stefano Stabellini
@ 2025-02-10 9:03 ` Oleksii Kurochko
2025-02-10 23:39 ` Andrew Cooper
1 sibling, 1 reply; 10+ messages in thread
From: Oleksii Kurochko @ 2025-02-10 9:03 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
Roger Pau Monné, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 3852 bytes --]
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.cooper3@citrix.com>
> ---
> CC: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> CC: Anthony PERARD<anthony.perard@vates.tech>
> CC: Michal Orzel<michal.orzel@amd.com>
> CC: Jan Beulich<jbeulich@suse.com>
> CC: Julien Grall<julien@xen.org>
> CC: Roger Pau Monné<roger.pau@citrix.com>
> CC: Stefano Stabellini<sstabellini@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.kurochko@gmail.com>
Thanks.
~ Oleksii
>
> #define __noreturn noreturn
> #define pr_err(...) printk(XENLOG_ERR __VA_ARGS__)
[-- Attachment #2: Type: text/html, Size: 5154 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing
2025-02-10 9:03 ` Oleksii Kurochko
@ 2025-02-10 23:39 ` Andrew Cooper
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2025-02-10 23:39 UTC (permalink / raw)
To: Oleksii Kurochko, Xen-devel
Cc: Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
Roger Pau Monné, Stefano Stabellini
On 10/02/2025 9:03 am, Oleksii Kurochko wrote:
>
>
> 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.cooper3@citrix.com>
>> ---
>> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> CC: Anthony PERARD <anthony.perard@vates.tech>
>> CC: Michal Orzel <michal.orzel@amd.com>
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Julien Grall <julien@xen.org>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Stefano Stabellini <sstabellini@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.
They are a direct consequence of RISC-V having less complicated (== less
entwined) headers.
>
> Anyway, changes look good to me, so:
> Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Thanks.
~Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.20 0/3] RISCV: Bugfixes and UBSAN
2025-02-07 22:01 [PATCH for-4.20 0/3] RISCV: Bugfixes and UBSAN Andrew Cooper
` (2 preceding siblings ...)
2025-02-07 22:01 ` [PATCH for-4.20 3/3] RISCV: Activate UBSAN in testing Andrew Cooper
@ 2025-02-10 9:05 ` Oleksii Kurochko
3 siblings, 0 replies; 10+ messages in thread
From: Oleksii Kurochko @ 2025-02-10 9:05 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
Roger Pau Monné, Stefano Stabellini, Volodymyr Babchuk,
Bertrand Marquis, Shawn Anastasio
[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]
On 2/7/25 11:01 PM, Andrew Cooper wrote:
> One bugfix, and two minor patches to get UBSAN working with RISCV. They
> should be considered for 4.20 at this juncture.
Considering that RISC-V port isn't really usable and changes are quite straightforward
and low risk:
Release-Acked-By: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Thanks.
~ Oleksii
>
> I tried to get this working everywhere, but:
>
> 1) ARM32 has some problem with dump_execution_state() and dies with an
> undefined instruction error.
>
> 2) PPC doesn't get any console output, and also appears to have no exception
> handling either. Also, when it doesn't succeed, it takes ages to fail.
>
> Andrew Cooper (3):
> RISCV/boot: Run constructors during setup
> RISCV/asm: Use CALL rather than JAL
> RISCV: Activate UBSAN in testing
>
> automation/gitlab-ci/build.yaml | 3 +++
> xen/arch/riscv/Kconfig | 1 +
> xen/arch/riscv/entry.S | 2 +-
> xen/arch/riscv/include/asm/processor.h | 2 ++
> xen/arch/riscv/riscv64/head.S | 12 ++++++------
> xen/arch/riscv/setup.c | 2 ++
> xen/arch/riscv/traps.c | 2 +-
> xen/common/ubsan/ubsan.c | 5 ++++-
> 8 files changed, 20 insertions(+), 9 deletions(-)
>
[-- Attachment #2: Type: text/html, Size: 1877 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread