[PATCH 1/2] lib: sbi: restore platform CSR when suspend

[Samuel Holland]

Hi Xiang,

On 2025-12-04 8:58 AM, Xiang W wrote:
> When HART enters suspend mode, some platforms require specific CSRs
> to be saved for recovery. This patch add interface for this.
> 
> Signed-off-by: Xiang W <wangxiang at iscas.ac.cn>
> ---
>  include/sbi/sbi_platform.h | 60 ++++++++++++++++++++++++++++++++++++++
>  lib/sbi/sbi_hsm.c          | 12 +++++++-
>  2 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index d75c12de..175ec1e2 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -41,6 +41,9 @@
>  #define SBI_PLATFORM_HART_INDEX2ID_OFFSET (0x60 + (__SIZEOF_POINTER__ * 2))
>  /** Offset of cbom_block_size in struct sbi_platform */
>  #define SBI_PLATFORM_CBOM_BLOCK_SIZE_OFFSET (0x60 + (__SIZEOF_POINTER__ * 3))
> +/** Offset of suspend_backup_csr_count in struct sbi_platform */
> +#define SBI_PLATFORM_SUSPEND_BACKUP_CSR_COUNT_OFFSET \
> +					    (0x60 + (__SIZEOF_POINTER__ * 4))
>  
>  #define SBI_PLATFORM_TLB_RANGE_FLUSH_LIMIT_DEFAULT		(1UL << 12)
>  
> @@ -146,6 +149,12 @@ struct sbi_platform_operations {
>  			unsigned long log2len);
>  	/** platform specific pmp disable on current HART */
>  	void (*pmp_disable)(unsigned int n);
> +
> +	/** Save platform specific CSR before suspend */
> +	void (*suspend_non_ret_save)(unsigned long *data);
> +
> +	/** Restore platform specific CSR after suspend */
> +	void (*suspend_non_ret_restore)(unsigned long *data);
>  };
>  
>  /** Platform default per-HART stack size for exception/interrupt handling */
> @@ -198,6 +207,13 @@ struct sbi_platform {
>  	const u32 *hart_index2id;
>  	/** Allocation alignment for Scratch */
>  	unsigned long cbom_block_size;
> +
> +	/**
> +	 * When HART enters suspend mode, some platforms require specific CSRs
> +	 * to be saved for recovery. This variable tracks how many CSRs need to
> +	 *  be backed up for this platform.

There's an extra space before "be".

> +	 */
> +	unsigned long suspend_backup_csr_count;
>  };
>  
>  /**
> @@ -216,6 +232,8 @@ assert_member_offset(struct sbi_platform, platform_ops_addr, SBI_PLATFORM_OPS_OF
>  assert_member_offset(struct sbi_platform, firmware_context, SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET);
>  assert_member_offset(struct sbi_platform, hart_index2id, SBI_PLATFORM_HART_INDEX2ID_OFFSET);
>  assert_member_offset(struct sbi_platform, cbom_block_size, SBI_PLATFORM_CBOM_BLOCK_SIZE_OFFSET);
> +assert_member_offset(struct sbi_platform, suspend_backup_csr_count, SBI_PLATFORM_SUSPEND_BACKUP_CSR_COUNT_OFFSET);
> +
>  
>  /** Get pointer to sbi_platform for sbi_scratch pointer */
>  #define sbi_platform_ptr(__s) \
> @@ -666,6 +684,48 @@ static inline void sbi_platform_pmp_disable(const struct sbi_platform *plat,
>  		sbi_platform_ops(plat)->pmp_disable(n);
>  }
>  
> +/**
> + * Platform specific CSR count to backup during suspend.
> + *
> + * @param plat pointer to struct sbi_platform
> + *
> + * @return csr count need backup
> + */
> +static inline unsigned long sbi_platform_suspend_backup_csr_count(
> +					    const struct sbi_platform *plat)
> +{
> +	if (plat)
> +		return plat->suspend_backup_csr_count;
> +	return 0;
> +}
> +
> +/**
> + * Save platform specific CSR before suspend
> + *
> + * @param plat pointer to struct sbi_platform
> + * @param data buffer used to save CSR
> + */
> +static inline void sbi_platform_suspend_non_ret_save(
> +					    const struct sbi_platform *plat,
> +					    unsigned long *data)
> +{
> +	if (plat && sbi_platform_ops(plat)->suspend_non_ret_save)
> +		sbi_platform_ops(plat)->suspend_non_ret_save(data);
> +}
> +
> +/**
> + * Restore platform specific CSR after suspend
> + * @param plat pointer to struct sbi_platform
> + * @param data buffer used to restore CSR
> + */
> +static inline void sbi_platform_suspend_non_ret_restore(
> +					    const struct sbi_platform *plat,
> +					    unsigned long *data)
> +{
> +	if (plat && sbi_platform_ops(plat)->suspend_non_ret_restore)
> +		sbi_platform_ops(plat)->suspend_non_ret_restore(data);
> +}
> +
>  #endif
>  
>  #endif
> diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
> index 0a355f9c..949f7aaf 100644
> --- a/lib/sbi/sbi_hsm.c
> +++ b/lib/sbi/sbi_hsm.c
> @@ -21,6 +21,7 @@
>  #include <sbi/sbi_hsm.h>
>  #include <sbi/sbi_init.h>
>  #include <sbi/sbi_ipi.h>
> +#include <sbi/sbi_platform.h>
>  #include <sbi/sbi_scratch.h>
>  #include <sbi/sbi_system.h>
>  #include <sbi/sbi_timer.h>
> @@ -50,6 +51,7 @@ struct sbi_hsm_data {
>  	unsigned long saved_mideleg;
>  	u64 saved_menvcfg;
>  	atomic_t start_ticket;
> +	unsigned long platform_saved[0];

Please use the standard flexible array member syntax (no zero) instead of the
GNU extension.

>  };
>  
>  bool sbi_hsm_hart_change_state(struct sbi_scratch *scratch, long oldstate,
> @@ -249,9 +251,13 @@ int sbi_hsm_init(struct sbi_scratch *scratch, bool cold_boot)
>  {
>  	struct sbi_scratch *rscratch;
>  	struct sbi_hsm_data *hdata;
> +	unsigned long n;
>  
>  	if (cold_boot) {
> -		hart_data_offset = sbi_scratch_alloc_offset(sizeof(*hdata));
> +		n = sbi_platform_suspend_backup_csr_count(
> +			sbi_platform_ptr(scratch));
> +		hart_data_offset = sbi_scratch_alloc_offset(sizeof(*hdata) +
> +							    n * sizeof(long));
>  		if (!hart_data_offset)
>  			return SBI_ENOMEM;
>  
> @@ -430,6 +436,8 @@ void __sbi_hsm_suspend_non_ret_save(struct sbi_scratch *scratch)
>  	hdata->saved_mideleg = csr_read(CSR_MIDELEG);
>  	if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_12)
>  		hdata->saved_menvcfg = csr_read64(CSR_MENVCFG);
> +
> +	sbi_platform_suspend_non_ret_save(sbi_platform_ptr(scratch), hdata->platform_saved);
>  }
>  
>  static void __sbi_hsm_suspend_non_ret_restore(struct sbi_scratch *scratch)
> @@ -443,6 +451,8 @@ static void __sbi_hsm_suspend_non_ret_restore(struct sbi_scratch *scratch)
>  	csr_write(CSR_MEDELEG, hdata->saved_medeleg);
>  	csr_write(CSR_MIE, hdata->saved_mie);
>  	csr_set(CSR_MIP, (hdata->saved_mip & (MIP_SSIP | MIP_STIP)));
> +
> +	sbi_platform_suspend_non_ret_restore(sbi_platform_ptr(scratch), hdata->platform_saved);

The suspend/restore functions operate in reverse/LIFO order. I would suggest to
maintain this property.

Regards,
Samuel

>  }
>  
>  void sbi_hsm_hart_resume_start(struct sbi_scratch *scratch)