Re: [PATCH] RISCV/shutdown: Implement machine_{halt,restart}()

[oleksii.kurochko@xxxxxxxxx]

On Tue, 2024-09-03 at 15:19 +0100, Andrew Cooper wrote:
> SBI has an API for shutdown so wire it up.  However, the spec does
> allow the
> call not to be implemented, so we have to cope with the call return
> returning.
> 
> There is a reboot-capable SBI extention, but in the short term route
> route
> machine_restart() into machine_halt().
> 
> Then, use use machine_halt() rather than an infinite loop at the end
> of
> start_xen().  This avoids the Qemu smoke test needing to wait for the
> full
> timeout in order to succeed.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
LGTM:
 Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Thanks for the patch.

~ Oleksii

> 
> As per commit e44f33ccddc2 ("ppc/shutdown: Implement
> machine_{halt,restart}()")
> 
> Simply replacing BUG() with a printk() is just swapping one problem
> for
> another.
> ---
>  xen/arch/riscv/Makefile          |  1 +
>  xen/arch/riscv/include/asm/sbi.h |  3 +++
>  xen/arch/riscv/sbi.c             |  5 +++++
>  xen/arch/riscv/setup.c           |  6 ++----
>  xen/arch/riscv/shutdown.c        | 25 +++++++++++++++++++++++++
>  xen/arch/riscv/stubs.c           | 12 ------------
>  6 files changed, 36 insertions(+), 16 deletions(-)
>  create mode 100644 xen/arch/riscv/shutdown.c
> 
> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> index 81b77b13d652..d192be7b552a 100644
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -4,6 +4,7 @@ obj-y += mm.o
>  obj-$(CONFIG_RISCV_64) += riscv64/
>  obj-y += sbi.o
>  obj-y += setup.o
> +obj-y += shutdown.o
>  obj-y += stubs.o
>  obj-y += traps.o
>  obj-y += vm_event.o
> diff --git a/xen/arch/riscv/include/asm/sbi.h
> b/xen/arch/riscv/include/asm/sbi.h
> index 0e6820a4eda3..4d72a2295e72 100644
> --- a/xen/arch/riscv/include/asm/sbi.h
> +++ b/xen/arch/riscv/include/asm/sbi.h
> @@ -13,6 +13,7 @@
>  #define __ASM_RISCV_SBI_H__
>  
>  #define SBI_EXT_0_1_CONSOLE_PUTCHAR          0x1
> +#define SBI_EXT_0_1_SHUTDOWN                 0x8
>  
>  struct sbiret {
>      long error;
> @@ -31,4 +32,6 @@ struct sbiret sbi_ecall(unsigned long ext, unsigned
> long fid,
>   */
>  void sbi_console_putchar(int ch);
>  
> +void sbi_shutdown(void);
> +
>  #endif /* __ASM_RISCV_SBI_H__ */
> diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c
> index 0ae166c8610e..c7984344bc6b 100644
> --- a/xen/arch/riscv/sbi.c
> +++ b/xen/arch/riscv/sbi.c
> @@ -42,3 +42,8 @@ void sbi_console_putchar(int ch)
>  {
>      sbi_ecall(SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, ch, 0, 0, 0, 0, 0);
>  }
> +
> +void sbi_shutdown(void)
> +{
> +    sbi_ecall(SBI_EXT_0_1_SHUTDOWN, 0, 0, 0, 0, 0, 0, 0);
> +}
> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> index a6a29a150869..bf9078f36aff 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -4,6 +4,7 @@
>  #include <xen/compile.h>
>  #include <xen/init.h>
>  #include <xen/mm.h>
> +#include <xen/shutdown.h>
>  
>  #include <public/version.h>
>  
> @@ -28,8 +29,5 @@ void __init noreturn start_xen(unsigned long
> bootcpu_id,
>  
>      printk("All set up\n");
>  
> -    for ( ;; )
> -        asm volatile ("wfi");
> -
> -    unreachable();
> +    machine_halt();
>  }
> diff --git a/xen/arch/riscv/shutdown.c b/xen/arch/riscv/shutdown.c
> new file mode 100644
> index 000000000000..270bb26b68a6
> --- /dev/null
> +++ b/xen/arch/riscv/shutdown.c
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#include <xen/shutdown.h>
> +
> +#include <asm/sbi.h>
> +
> +void machine_halt(void)
> +{
> +    sbi_shutdown();
> +
> +    for ( ;; )
> +        asm volatile ("wfi");
> +
> +    unreachable();
> +}
> +
> +void machine_restart(unsigned int delay_millisecs)
> +{
> +    /*
> +     * TODO: mdelay(delay_millisecs)
> +     * TODO: Probe for #SRST support, where sbi_system_reset() has a
> +     *       shutdown/reboot parameter.
> +     */
> +
> +    machine_halt();
> +}
> diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> index 3285d1889940..2aa245f272b5 100644
> --- a/xen/arch/riscv/stubs.c
> +++ b/xen/arch/riscv/stubs.c
> @@ -49,18 +49,6 @@ void domain_set_time_offset(struct domain *d,
> int64_t time_offset_seconds)
>      BUG_ON("unimplemented");
>  }
>  
> -/* shutdown.c */
> -
> -void machine_restart(unsigned int delay_millisecs)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
> -void machine_halt(void)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
>  /* domctl.c */
>  
>  long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
> 
> base-commit: 1e6bb29b03680a9d0e12f14c4d406a0d67317ea7