Skip to contentSkip to conversation

SBI system suspend (SUSP) extension

RFC PATCH New Andrew Jones 2 participants 26 messages Jan 6, 2023 – Jan 17, 2023

lore
Share & tools
lore

Andrew Jones

author
#0 lore raw

cover SBI system suspend (SUSP) extension

This series implements the SBI system suspend support in OpenSBI as-per the draft proposal for SBI system suspend which can be found at
https://github.com/jones-drew/riscv-sbi-doc/commit/d9e43e9a938fc3eb510e023c3f352462876f7785

The first half of the series are a few cleanups and fixes for HSM, as system suspend makes use of some of its code and patterns. The second half implements support for the extension and includes a patch which allows testing to be enabled, even when low-level firmware does not provide support. Regarding the test mode, it's currently enabled by adding a 'system-suspend-test' DT property, but Anup has suggested that FW_OPTION could also be used. I'm all ears for what people think the best way to control test functionality in OpenSBI is and am happy to switch it. To test with Linux the kernel must include the patch at
https://github.com/jones-drew/linux/commits/riscv/sbi-susp-rfc and be built with CONFIG_SUSPEND enabled.

The patches are based on v1.2 and "treewide: Replace TRUE/FALSE with true/false" and can also be found at
https://github.com/jones-drew/opensbi/commits/susp-rfc

Andrew Jones (11):
lib: sbi_hsm: Factor out invalid state detection
lib: sbi_hsm: Don't try to restore state on failed change
lib: sbi_hsm: Ensure errors are consistent with spec
lib: sbi_hsm: Move misplaced comment
lib: sbi_hsm: Remove unnecessary include
lib: sbi_hsm: Export some functions
lib: sbi: Add system suspend skeleton
lib: sbi: Add system_suspend_allowed domain property
lib: sbi: Implement system suspend
docs: Correct opensbi-domain property name
platform: generic: Add system suspend test

 docs/domain_support.md            |  15 +++-
 include/sbi/sbi_domain.h          |   2 +
 include/sbi/sbi_ecall_interface.h |   8 +++
 include/sbi/sbi_hsm.h             |   4 ++
 include/sbi/sbi_system.h          |  18 +++++
 lib/sbi/Kconfig                   |   4 ++
 lib/sbi/objects.mk                |   3 +
 lib/sbi/sbi_domain.c              |   4 ++
 lib/sbi/sbi_ecall_hsm.c           |   4 +-
 lib/sbi/sbi_ecall_susp.c          |  48 +++++++++++++
 lib/sbi/sbi_hsm.c                 |  92 +++++++++++--------------
 lib/sbi/sbi_system.c              | 111 ++++++++++++++++++++++++++++++
 lib/utils/fdt/fdt_domain.c        |   7 ++
 platform/generic/platform.c       |  20 +++++-
 14 files changed, 283 insertions(+), 57 deletions(-)
 create mode 100644 lib/sbi/sbi_ecall_susp.c
Signature · 1 line
2.39.0
author
#1 lore raw

RFC PATCH 1/11 lib: sbi_hsm: Factor out invalid state detection

Remove some redundant code by creating an invalid state detection macro.

No functional change intended.

Signed-off-by Andrew Jones <[email protected]>
---
 lib/sbi/sbi_hsm.c | 65 +++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)
lib/sbi/sbi_hsm.c +24 -41
index 836008fc92d7..1896f52c2ab2 100644
@@ -26,6 +26,15 @@
 #include <sbi/sbi_timer.h>
 #include <sbi/sbi_console.h>
 
+#define __sbi_hsm_hart_change_state(hdata, oldstate, newstate)		\
+({									\
+	long state = atomic_cmpxchg(&(hdata)->state, oldstate, newstate); \
+	if (state != (oldstate))					\
+		sbi_printf("%s: ERR: The hart is in invalid state [%lu]\n", \
+			   __func__, state);				\
+	state == (oldstate);						\
+})
+
 static const struct sbi_hsm_device *hsm_dev = NULL;
 static unsigned long hart_data_offset;
 

@@ -95,13 +104,11 @@ int sbi_hsm_hart_interruptible_mask(const struct sbi_domain *dom,
 
 void sbi_hsm_prepare_next_jump(struct sbi_scratch *scratch, u32 hartid)
 {
-	u32 oldstate;
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);
 
-	oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_START_PENDING,
-				  SBI_HSM_STATE_STARTED);
-	if (oldstate != SBI_HSM_STATE_START_PENDING)
+	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_START_PENDING,
+					 SBI_HSM_STATE_STARTED))
 		sbi_hart_hang();
 }
 

@@ -217,14 +224,12 @@ int sbi_hsm_init(struct sbi_scratch *scratch, u32 hartid, bool cold_boot)
 
 void __noreturn sbi_hsm_exit(struct sbi_scratch *scratch)
 {
-	u32 hstate;
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);
 	void (*jump_warmboot)(void) = (void (*)(void))scratch->warmboot_addr;
 
-	hstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_STOP_PENDING,
-				SBI_HSM_STATE_STOPPED);
-	if (hstate != SBI_HSM_STATE_STOP_PENDING)
+	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STOP_PENDING,
+					 SBI_HSM_STATE_STOPPED))
 		goto fail_exit;
 
 	if (hsm_device_has_hart_hotplug()) {

@@ -299,7 +304,6 @@ int sbi_hsm_hart_start(struct sbi_scratch *scratch,
 
 int sbi_hsm_hart_stop(struct sbi_scratch *scratch, bool exitnow)
 {
-	int oldstate;
 	const struct sbi_domain *dom = sbi_domain_thishart_ptr();
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);

@@ -307,13 +311,9 @@ int sbi_hsm_hart_stop(struct sbi_scratch *scratch, bool exitnow)
 	if (!dom)
 		return SBI_EFAIL;
 
-	oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_STARTED,
-				  SBI_HSM_STATE_STOP_PENDING);
-	if (oldstate != SBI_HSM_STATE_STARTED) {
-		sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-			   __func__, oldstate);
+	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
+					 SBI_HSM_STATE_STOP_PENDING))
 		return SBI_EFAIL;
-	}
 
 	if (exitnow)
 		sbi_exit(scratch);

@@ -363,36 +363,26 @@ static void __sbi_hsm_suspend_non_ret_restore(struct sbi_scratch *scratch)
 
 void sbi_hsm_hart_resume_start(struct sbi_scratch *scratch)
 {
-	int oldstate;
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);
 
 	/* If current HART was SUSPENDED then set RESUME_PENDING state */
-	oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_SUSPENDED,
-			SBI_HSM_STATE_RESUME_PENDING);
-	if (oldstate != SBI_HSM_STATE_SUSPENDED) {
-		sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-			   __func__, oldstate);
+	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_SUSPENDED,
+					 SBI_HSM_STATE_RESUME_PENDING))
 		sbi_hart_hang();
-	}
 
 	hsm_device_hart_resume();
 }
 
 void sbi_hsm_hart_resume_finish(struct sbi_scratch *scratch)
 {
-	u32 oldstate;
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);
 
 	/* If current HART was RESUME_PENDING then set STARTED state */
-	oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_RESUME_PENDING,
-				  SBI_HSM_STATE_STARTED);
-	if (oldstate != SBI_HSM_STATE_RESUME_PENDING) {
-		sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-			   __func__, oldstate);
+	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_RESUME_PENDING,
+					 SBI_HSM_STATE_STARTED))
 		sbi_hart_hang();
-	}
 
 	/*
 	 * Restore some of the M-mode CSRs which we are re-configured by

@@ -404,7 +394,7 @@ void sbi_hsm_hart_resume_finish(struct sbi_scratch *scratch)
 int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 			 ulong raddr, ulong rmode, ulong priv)
 {
-	int oldstate, ret;
+	int ret;
 	const struct sbi_domain *dom = sbi_domain_thishart_ptr();
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);

@@ -438,11 +428,8 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 	scratch->next_mode = rmode;
 
 	/* Directly move from STARTED to SUSPENDED state */
-	oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_STARTED,
-				  SBI_HSM_STATE_SUSPENDED);
-	if (oldstate != SBI_HSM_STATE_STARTED) {
-		sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-			   __func__, oldstate);
+	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
+					 SBI_HSM_STATE_SUSPENDED)) {
 		ret = SBI_EDENIED;
 		goto fail_restore_state;
 	}

@@ -485,13 +472,9 @@ fail_restore_state:
 	 * We might have successfully resumed from retentive suspend
 	 * or suspend failed. In both cases, we restore state of hart.
 	 */
-	oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_SUSPENDED,
-				  SBI_HSM_STATE_STARTED);
-	if (oldstate != SBI_HSM_STATE_SUSPENDED) {
-		sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-			   __func__, oldstate);
+	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_SUSPENDED,
+					 SBI_HSM_STATE_STARTED))
 		sbi_hart_hang();
-	}
 
 	return ret;
 }
Signature · 1 line
2.39.0
#2 lore raw

RFC PATCH 1/11 lib: sbi_hsm: Factor out invalid state detection

Quoted 7 lines · Andrew Jones

Remove some redundant code by creating an invalid state detection macro.

No functional change intended.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 169 lines
---
 lib/sbi/sbi_hsm.c | 65 +++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)
lib/sbi/sbi_hsm.c +24 -42
index 836008fc92d7..1896f52c2ab2 100644
@@ -26,6 +26,15 @@
 #include <sbi/sbi_timer.h>
 #include <sbi/sbi_console.h>
 
+#define __sbi_hsm_hart_change_state(hdata, oldstate, newstate)         \
+({                                                                     \
+       long state = atomic_cmpxchg(&(hdata)->state, oldstate, newstate); \
+       if (state != (oldstate))                                        \
+               sbi_printf("%s: ERR: The hart is in invalid state [%lu]\n", \
+                          __func__, state);                            \
+       state == (oldstate);                                            \
+})
+
 static const struct sbi_hsm_device *hsm_dev = NULL;
 static unsigned long hart_data_offset;
 

@@ -95,13 +104,11 @@ int sbi_hsm_hart_interruptible_mask(const struct sbi_domain *dom,
 
 void sbi_hsm_prepare_next_jump(struct sbi_scratch *scratch, u32 hartid)
 {
-       u32 oldstate;
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);
 
-       oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_START_PENDING,
-                                 SBI_HSM_STATE_STARTED);
-       if (oldstate != SBI_HSM_STATE_START_PENDING)
+       if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_START_PENDING,
+                                        SBI_HSM_STATE_STARTED))
                sbi_hart_hang();
 }
 

@@ -217,14 +224,12 @@ int sbi_hsm_init(struct sbi_scratch *scratch, u32 hartid, bool cold_boot)
 
 void __noreturn sbi_hsm_exit(struct sbi_scratch *scratch)
 {
-       u32 hstate;
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);
        void (*jump_warmboot)(void) = (void (*)(void))scratch->warmboot_addr;
 
-       hstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_STOP_PENDING,
-                               SBI_HSM_STATE_STOPPED);
-       if (hstate != SBI_HSM_STATE_STOP_PENDING)
+       if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STOP_PENDING,
+                                        SBI_HSM_STATE_STOPPED))
                goto fail_exit;
 
        if (hsm_device_has_hart_hotplug()) {

@@ -299,7 +304,6 @@ int sbi_hsm_hart_start(struct sbi_scratch *scratch,
 
 int sbi_hsm_hart_stop(struct sbi_scratch *scratch, bool exitnow)
 {
-       int oldstate;
        const struct sbi_domain *dom = sbi_domain_thishart_ptr();
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);

@@ -307,13 +311,9 @@ int sbi_hsm_hart_stop(struct sbi_scratch *scratch, bool exitnow)
        if (!dom)
                return SBI_EFAIL;
 
-       oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_STARTED,
-                                 SBI_HSM_STATE_STOP_PENDING);
-       if (oldstate != SBI_HSM_STATE_STARTED) {
-               sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-                          __func__, oldstate);
+       if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
+                                        SBI_HSM_STATE_STOP_PENDING))
                return SBI_EFAIL;
-       }
 
        if (exitnow)
                sbi_exit(scratch);

@@ -363,36 +363,26 @@ static void __sbi_hsm_suspend_non_ret_restore(struct sbi_scratch *scratch)
 
 void sbi_hsm_hart_resume_start(struct sbi_scratch *scratch)
 {
-       int oldstate;
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);
 
        /* If current HART was SUSPENDED then set RESUME_PENDING state */
-       oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_SUSPENDED,
-                       SBI_HSM_STATE_RESUME_PENDING);
-       if (oldstate != SBI_HSM_STATE_SUSPENDED) {
-               sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-                          __func__, oldstate);
+       if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_SUSPENDED,
+                                        SBI_HSM_STATE_RESUME_PENDING))
                sbi_hart_hang();
-       }
 
        hsm_device_hart_resume();
 }
 
 void sbi_hsm_hart_resume_finish(struct sbi_scratch *scratch)
 {
-       u32 oldstate;
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);
 
        /* If current HART was RESUME_PENDING then set STARTED state */
-       oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_RESUME_PENDING,
-                                 SBI_HSM_STATE_STARTED);
-       if (oldstate != SBI_HSM_STATE_RESUME_PENDING) {
-               sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-                          __func__, oldstate);
+       if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_RESUME_PENDING,
+                                        SBI_HSM_STATE_STARTED))
                sbi_hart_hang();
-       }
 
        /*
         * Restore some of the M-mode CSRs which we are re-configured by

@@ -404,7 +394,7 @@ void sbi_hsm_hart_resume_finish(struct sbi_scratch *scratch)
 int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
                         ulong raddr, ulong rmode, ulong priv)
 {
-       int oldstate, ret;
+       int ret;
        const struct sbi_domain *dom = sbi_domain_thishart_ptr();
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);

@@ -438,11 +428,8 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
        scratch->next_mode = rmode;
 
        /* Directly move from STARTED to SUSPENDED state */
-       oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_STARTED,
-                                 SBI_HSM_STATE_SUSPENDED);
-       if (oldstate != SBI_HSM_STATE_STARTED) {
-               sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-                          __func__, oldstate);
+       if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
+                                        SBI_HSM_STATE_SUSPENDED)) {
                ret = SBI_EDENIED;
                goto fail_restore_state;
        }

@@ -485,13 +472,9 @@ fail_restore_state:
         * We might have successfully resumed from retentive suspend
         * or suspend failed. In both cases, we restore state of hart.
         */
-       oldstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_SUSPENDED,
-                                 SBI_HSM_STATE_STARTED);
-       if (oldstate != SBI_HSM_STATE_SUSPENDED) {
-               sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
-                          __func__, oldstate);
+       if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_SUSPENDED,
+                                        SBI_HSM_STATE_STARTED))
                sbi_hart_hang();
-       }
 
        return ret;
 }
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#3 lore raw

RFC PATCH 2/11 lib: sbi_hsm: Don't try to restore state on failed change

In reply to Andrew Jones

When a state change fails there's no need to restore the original state as it remains the same.

Signed-off-by Andrew Jones <[email protected]>
---
 lib/sbi/sbi_hsm.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
lib/sbi/sbi_hsm.c +2 -5
index 1896f52c2ab2..6ef6c5bdd4a7 100644
@@ -429,10 +429,8 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 
 	/* Directly move from STARTED to SUSPENDED state */
 	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
-					 SBI_HSM_STATE_SUSPENDED)) {
-		ret = SBI_EDENIED;
-		goto fail_restore_state;
-	}
+					 SBI_HSM_STATE_SUSPENDED))
+		return SBI_EDENIED;
 
 	/* Save the suspend type */
 	hdata->suspend_type = suspend_type;

@@ -467,7 +465,6 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 		jump_warmboot();
 	}
 
-fail_restore_state:
 	/*
 	 * We might have successfully resumed from retentive suspend
 	 * or suspend failed. In both cases, we restore state of hart.
Signature · 1 line
2.39.0
#4 lore raw

RFC PATCH 2/11 lib: sbi_hsm: Don't try to restore state on failed change

Quoted 5 lines · Andrew Jones

When a state change fails there's no need to restore the original state as it remains the same.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 37 lines
---
 lib/sbi/sbi_hsm.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
lib/sbi/sbi_hsm.c +2 -6
index 1896f52c2ab2..6ef6c5bdd4a7 100644
@@ -429,10 +429,8 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 
        /* Directly move from STARTED to SUSPENDED state */
        if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
-                                        SBI_HSM_STATE_SUSPENDED)) {
-               ret = SBI_EDENIED;
-               goto fail_restore_state;
-       }
+                                        SBI_HSM_STATE_SUSPENDED))
+               return SBI_EDENIED;
 
        /* Save the suspend type */
        hdata->suspend_type = suspend_type;

@@ -467,7 +465,6 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
                jump_warmboot();
        }
 
-fail_restore_state:
        /*
         * We might have successfully resumed from retentive suspend
         * or suspend failed. In both cases, we restore state of hart.
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#5 lore raw

RFC PATCH 3/11 lib: sbi_hsm: Ensure errors are consistent with spec

In reply to Andrew Jones

HSM functions define when SBI_ERR_INVALID_PARAM should be returned. Ensure it's not used for reasons that don't meet the definitions by using the catch-all code, SBI_ERR_FAILED, for those reasons instead. Also, in one case sbi_hart_suspend() may have returned SBI_ERR_DENIED, which isn't defined for that function at all. Use SBI_ERR_FAILED for that case too.

Signed-off-by Andrew Jones <[email protected]>
---
 lib/sbi/sbi_hsm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
lib/sbi/sbi_hsm.c +3 -3
index 6ef6c5bdd4a7..3635f968e020 100644
@@ -403,7 +403,7 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 
 	/* Sanity check on domain assigned to current HART */
 	if (!dom)
-		return SBI_EINVAL;
+		return SBI_EFAIL;
 
 	/* Sanity check on suspend type */
 	if (SBI_HSM_SUSPEND_RET_DEFAULT < suspend_type &&

@@ -416,7 +416,7 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 	/* Additional sanity check for non-retentive suspend */
 	if (suspend_type & SBI_HSM_SUSP_NON_RET_BIT) {
 		if (rmode != PRV_S && rmode != PRV_U)
-			return SBI_EINVAL;
+			return SBI_EFAIL;
 		if (dom && !sbi_domain_check_addr(dom, raddr, rmode,
 						  SBI_DOMAIN_EXECUTE))
 			return SBI_EINVALID_ADDR;

@@ -430,7 +430,7 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 	/* Directly move from STARTED to SUSPENDED state */
 	if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
 					 SBI_HSM_STATE_SUSPENDED))
-		return SBI_EDENIED;
+		return SBI_EFAIL;
 
 	/* Save the suspend type */
 	hdata->suspend_type = suspend_type;
Signature · 1 line
2.39.0
#6 lore raw

RFC PATCH 3/11 lib: sbi_hsm: Ensure errors are consistent with spec

Quoted 9 lines · Andrew Jones

HSM functions define when SBI_ERR_INVALID_PARAM should be returned. Ensure it's not used for reasons that don't meet the definitions by using the catch-all code, SBI_ERR_FAILED, for those reasons instead. Also, in one case sbi_hart_suspend() may have returned SBI_ERR_DENIED, which isn't defined for that function at all. Use SBI_ERR_FAILED for that case too.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 43 lines
---
 lib/sbi/sbi_hsm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
lib/sbi/sbi_hsm.c +3 -4
index 6ef6c5bdd4a7..3635f968e020 100644
@@ -403,7 +403,7 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 
        /* Sanity check on domain assigned to current HART */
        if (!dom)
-               return SBI_EINVAL;
+               return SBI_EFAIL;
 
        /* Sanity check on suspend type */
        if (SBI_HSM_SUSPEND_RET_DEFAULT < suspend_type &&

@@ -416,7 +416,7 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
        /* Additional sanity check for non-retentive suspend */
        if (suspend_type & SBI_HSM_SUSP_NON_RET_BIT) {
                if (rmode != PRV_S && rmode != PRV_U)
-                       return SBI_EINVAL;
+                       return SBI_EFAIL;
                if (dom && !sbi_domain_check_addr(dom, raddr, rmode,
                                                  SBI_DOMAIN_EXECUTE))
                        return SBI_EINVALID_ADDR;

@@ -430,7 +430,7 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
        /* Directly move from STARTED to SUSPENDED state */
        if (!__sbi_hsm_hart_change_state(hdata, SBI_HSM_STATE_STARTED,
                                         SBI_HSM_STATE_SUSPENDED))
-               return SBI_EDENIED;
+               return SBI_EFAIL;
 
        /* Save the suspend type */
        hdata->suspend_type = suspend_type;
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#7 lore raw

RFC PATCH 4/11 lib: sbi_hsm: Move misplaced comment

In reply to Andrew Jones

While non-retentive suspend is not allowed for M-mode, the comment at the top of sbi_hsm_hart_suspend() implied suspend wasn't allowed for M-mode at all. Move the comment above the mode check which is inside a suspend type is non-retentive check.

Signed-off-by Andrew Jones <[email protected]>
---
 lib/sbi/sbi_hsm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
lib/sbi/sbi_hsm.c +4 -2
index 3635f968e020..cce4b07bbb36 100644
@@ -399,8 +399,6 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);
 
-	/* For now, we only allow suspend from S-mode or U-mode. */
-
 	/* Sanity check on domain assigned to current HART */
 	if (!dom)
 		return SBI_EFAIL;

@@ -415,6 +413,10 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 
 	/* Additional sanity check for non-retentive suspend */
 	if (suspend_type & SBI_HSM_SUSP_NON_RET_BIT) {
+		/*
+		 * For now, we only allow non-retentive suspend from
+		 * S-mode or U-mode.
+		 */
 		if (rmode != PRV_S && rmode != PRV_U)
 			return SBI_EFAIL;
 		if (dom && !sbi_domain_check_addr(dom, raddr, rmode,
Signature · 1 line
2.39.0
#8 lore raw

RFC PATCH 4/11 lib: sbi_hsm: Move misplaced comment

Quoted 7 lines · Andrew Jones

While non-retentive suspend is not allowed for M-mode, the comment at the top of sbi_hsm_hart_suspend() implied suspend wasn't allowed for M-mode at all. Move the comment above the mode check which is inside a suspend type is non-retentive check.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 36 lines
---
 lib/sbi/sbi_hsm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
lib/sbi/sbi_hsm.c +4 -3
index 3635f968e020..cce4b07bbb36 100644
@@ -399,8 +399,6 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);
 
-       /* For now, we only allow suspend from S-mode or U-mode. */
-
        /* Sanity check on domain assigned to current HART */
        if (!dom)
                return SBI_EFAIL;

@@ -415,6 +413,10 @@ int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 
        /* Additional sanity check for non-retentive suspend */
        if (suspend_type & SBI_HSM_SUSP_NON_RET_BIT) {
+               /*
+                * For now, we only allow non-retentive suspend from
+                * S-mode or U-mode.
+                */
                if (rmode != PRV_S && rmode != PRV_U)
                        return SBI_EFAIL;
                if (dom && !sbi_domain_check_addr(dom, raddr, rmode,
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#9 lore raw

RFC PATCH 5/11 lib: sbi_hsm: Remove unnecessary include

In reply to Andrew Jones

Also remove a superfluous semicolon and add a blank line.

Signed-off-by Andrew Jones <[email protected]>
---
 lib/sbi/sbi_ecall_hsm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
lib/sbi/sbi_ecall_hsm.c +2 -2
index 703a824493b4..f1b41d073b22 100644
@@ -12,7 +12,6 @@
 #include <sbi/sbi_ecall_interface.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_trap.h>
-#include <sbi/sbi_version.h>
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/riscv_asm.h>

@@ -45,7 +44,8 @@ static int sbi_ecall_hsm_handler(unsigned long extid, unsigned long funcid,
 		break;
 	default:
 		ret = SBI_ENOTSUPP;
-	};
+	}
+
 	if (ret >= 0) {
 		*out_val = ret;
 		ret = 0;
Signature · 1 line
2.39.0
#10 lore raw

RFC PATCH 5/11 lib: sbi_hsm: Remove unnecessary include

Quoted 4 lines · Andrew Jones

Also remove a superfluous semicolon and add a blank line.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 34 lines
---
 lib/sbi/sbi_ecall_hsm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
lib/sbi/sbi_ecall_hsm.c +2 -3
index 703a824493b4..f1b41d073b22 100644
@@ -12,7 +12,6 @@
 #include <sbi/sbi_ecall_interface.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_trap.h>
-#include <sbi/sbi_version.h>
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/riscv_asm.h>

@@ -45,7 +44,8 @@ static int sbi_ecall_hsm_handler(unsigned long extid, unsigned long funcid,
                break;
        default:
                ret = SBI_ENOTSUPP;
-       };
+       }
+
        if (ret >= 0) {
                *out_val = ret;
                ret = 0;
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#11 lore raw

RFC PATCH 6/11 lib: sbi_hsm: Export some functions

In reply to Andrew Jones

A coming patch can make use of a few internal hsm functions if we export them.

Signed-off-by Andrew Jones <[email protected]>
---
 include/sbi/sbi_hsm.h |  4 ++++
 lib/sbi/sbi_hsm.c     | 12 ++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)
include/sbi/sbi_hsm.h +4 -0
index d6cc468d0528..00adfe1a4ef1 100644
@@ -65,9 +65,13 @@ void sbi_hsm_hart_resume_start(struct sbi_scratch *scratch);
 void sbi_hsm_hart_resume_finish(struct sbi_scratch *scratch);
 int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
 			 ulong raddr, ulong rmode, ulong priv);
+bool sbi_hsm_hart_change_state(struct sbi_scratch *scratch, long oldstate,
+			       long newstate);
+int __sbi_hsm_hart_get_state(u32 hartid);
 int sbi_hsm_hart_get_state(const struct sbi_domain *dom, u32 hartid);
 int sbi_hsm_hart_interruptible_mask(const struct sbi_domain *dom,
 				    ulong hbase, ulong *out_hmask);
+void __sbi_hsm_suspend_non_ret_save(struct sbi_scratch *scratch);
 void sbi_hsm_prepare_next_jump(struct sbi_scratch *scratch, u32 hartid);
 
 #endif
lib/sbi/sbi_hsm.c +10 -2
index cce4b07bbb36..8eb9643e6e72 100644
@@ -46,7 +46,15 @@ struct sbi_hsm_data {
 	unsigned long saved_mip;
 };
 
-static inline int __sbi_hsm_hart_get_state(u32 hartid)
+bool sbi_hsm_hart_change_state(struct sbi_scratch *scratch, long oldstate,
+			       long newstate)
+{
+	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
+							    hart_data_offset);
+	return __sbi_hsm_hart_change_state(hdata, oldstate, newstate);
+}
+
+int __sbi_hsm_hart_get_state(u32 hartid)
 {
 	struct sbi_hsm_data *hdata;
 	struct sbi_scratch *scratch;

@@ -329,7 +337,7 @@ static int __sbi_hsm_suspend_default(struct sbi_scratch *scratch)
 	return 0;
 }
 
-static void __sbi_hsm_suspend_non_ret_save(struct sbi_scratch *scratch)
+void __sbi_hsm_suspend_non_ret_save(struct sbi_scratch *scratch)
 {
 	struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
 							    hart_data_offset);
Signature · 1 line
2.39.0
#12 lore raw

RFC PATCH 6/11 lib: sbi_hsm: Export some functions

Quoted 5 lines · Andrew Jones

A coming patch can make use of a few internal hsm functions if we export them.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 61 lines
---
 include/sbi/sbi_hsm.h |  4 ++++
 lib/sbi/sbi_hsm.c     | 12 ++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)
include/sbi/sbi_hsm.h +4 -0
index d6cc468d0528..00adfe1a4ef1 100644
@@ -65,9 +65,13 @@ void sbi_hsm_hart_resume_start(struct sbi_scratch *scratch);
 void sbi_hsm_hart_resume_finish(struct sbi_scratch *scratch);
 int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
                         ulong raddr, ulong rmode, ulong priv);
+bool sbi_hsm_hart_change_state(struct sbi_scratch *scratch, long oldstate,
+                              long newstate);
+int __sbi_hsm_hart_get_state(u32 hartid);
 int sbi_hsm_hart_get_state(const struct sbi_domain *dom, u32 hartid);
 int sbi_hsm_hart_interruptible_mask(const struct sbi_domain *dom,
                                    ulong hbase, ulong *out_hmask);
+void __sbi_hsm_suspend_non_ret_save(struct sbi_scratch *scratch);
 void sbi_hsm_prepare_next_jump(struct sbi_scratch *scratch, u32 hartid);
 
 #endif
lib/sbi/sbi_hsm.c +10 -3
index cce4b07bbb36..8eb9643e6e72 100644
@@ -46,7 +46,15 @@ struct sbi_hsm_data {
        unsigned long saved_mip;
 };
 
-static inline int __sbi_hsm_hart_get_state(u32 hartid)
+bool sbi_hsm_hart_change_state(struct sbi_scratch *scratch, long oldstate,
+                              long newstate)
+{
+       struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
+                                                           hart_data_offset);
+       return __sbi_hsm_hart_change_state(hdata, oldstate, newstate);
+}
+
+int __sbi_hsm_hart_get_state(u32 hartid)
 {
        struct sbi_hsm_data *hdata;
        struct sbi_scratch *scratch;

@@ -329,7 +337,7 @@ static int __sbi_hsm_suspend_default(struct sbi_scratch *scratch)
        return 0;
 }
 
-static void __sbi_hsm_suspend_non_ret_save(struct sbi_scratch *scratch)
+void __sbi_hsm_suspend_non_ret_save(struct sbi_scratch *scratch)
 {
        struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
                                                            hart_data_offset);
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#13 lore raw

RFC PATCH 7/11 lib: sbi: Add system suspend skeleton

In reply to Andrew Jones

Add the SUSP extension probe and ecall support, but for now the system suspend function is just a stub.

Signed-off-by Andrew Jones <[email protected]>
---
 include/sbi/sbi_ecall_interface.h |  8 ++++++
 include/sbi/sbi_system.h          | 17 +++++++++++
 lib/sbi/Kconfig                   |  4 +++
 lib/sbi/objects.mk                |  3 ++
 lib/sbi/sbi_ecall_susp.c          | 48 +++++++++++++++++++++++++++++++
 lib/sbi/sbi_system.c              | 26 +++++++++++++++++
 6 files changed, 106 insertions(+)
 create mode 100644 lib/sbi/sbi_ecall_susp.c
include/sbi/sbi_ecall_interface.h +8 -0
index a3f2bf4bdf69..98a426a74ec4 100644
@@ -29,6 +29,7 @@
 #define SBI_EXT_HSM				0x48534D
 #define SBI_EXT_SRST				0x53525354
 #define SBI_EXT_PMU				0x504D55
+#define SBI_EXT_SUSP				0x53555350
 
 /* SBI function IDs for BASE extension*/
 #define SBI_EXT_BASE_GET_SPEC_VERSION		0x0

@@ -230,6 +231,13 @@ enum sbi_pmu_ctr_type {
 /* Flags defined for counter stop function */
 #define SBI_PMU_STOP_FLAG_RESET (1 << 0)
 
+/* SBI function IDs for SUSP extension */
+#define SBI_EXT_SUSP_SUSPEND			0x0
+
+#define SBI_SUSP_SLEEP_TYPE_SUSPEND		0x0
+#define SBI_SUSP_SLEEP_TYPE_LAST		SBI_SUSP_SLEEP_TYPE_SUSPEND
+#define SBI_SUSP_PLATFORM_SLEEP_START		0x80000000
+
 /* SBI base specification related macros */
 #define SBI_SPEC_VERSION_MAJOR_OFFSET		24
 #define SBI_SPEC_VERSION_MAJOR_MASK		0x7f
include/sbi/sbi_system.h +17 -0
index 84c281383d8c..65ea3d36d6cf 100644
@@ -43,4 +43,21 @@ bool sbi_system_reset_supported(u32 reset_type, u32 reset_reason);
 
 void __noreturn sbi_system_reset(u32 reset_type, u32 reset_reason);
 
+/** System suspend device */
+struct sbi_system_suspend_device {
+	/** Name of the system suspend device */
+	char name[32];
+
+	/* Check whether sleep type is supported by the device */
+	int (*system_suspend_check)(u32 sleep_type);
+
+	/** Suspend the system */
+	int (*system_suspend)(u32 sleep_type);
+};
+
+const struct sbi_system_suspend_device *sbi_system_suspend_get_device(void);
+void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev);
+bool sbi_system_suspend_supported(u32 sleep_type);
+int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque);
+
 #endif
lib/sbi/Kconfig +4 -0
index df74bba38540..7e139017fec7 100644
@@ -22,6 +22,10 @@ config SBI_ECALL_SRST
 	bool "System Reset extension"
 	default y
 
+config SBI_ECALL_SUSP
+	bool "System Suspend extension"
+	default y
+
 config SBI_ECALL_PMU
 	bool "Performance Monitoring Unit extension"
 	default y
lib/sbi/objects.mk +3 -0
index c774ebbcd142..c4f3d0991aa2 100644
@@ -34,6 +34,9 @@ libsbi-objs-$(CONFIG_SBI_ECALL_HSM) += sbi_ecall_hsm.o
 carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_SRST) += ecall_srst
 libsbi-objs-$(CONFIG_SBI_ECALL_SRST) += sbi_ecall_srst.o
 
+carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_SUSP) += ecall_susp
+libsbi-objs-$(CONFIG_SBI_ECALL_SUSP) += sbi_ecall_susp.o
+
 carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_PMU) += ecall_pmu
 libsbi-objs-$(CONFIG_SBI_ECALL_PMU) += sbi_ecall_pmu.o
lib/sbi/sbi_ecall_susp.c new file +48 -0

48 lines not on this page · raw message

lib/sbi/sbi_system.c +26 -0
index f37c811d0bf3..5c123a6c9d8d 100644
@@ -92,3 +92,29 @@ void __noreturn sbi_system_reset(u32 reset_type, u32 reset_reason)
 	/* If platform specific reset did not work then do sbi_exit() */
 	sbi_exit(scratch);
 }
+
+static const struct sbi_system_suspend_device *suspend_dev = NULL;
+
+const struct sbi_system_suspend_device *sbi_system_suspend_get_device(void)
+{
+	return suspend_dev;
+}
+
+void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev)
+{
+	if (!dev || suspend_dev)
+		return;
+
+	suspend_dev = dev;
+}
+
+bool sbi_system_suspend_supported(u32 sleep_type)
+{
+	return suspend_dev && suspend_dev->system_suspend_check &&
+	       suspend_dev->system_suspend_check(sleep_type);
+}
+
+int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque)
+{
+	return 0;
+}
Signature · 1 line
2.39.0
#14 lore raw

RFC PATCH 7/11 lib: sbi: Add system suspend skeleton

Quoted 5 lines · Andrew Jones

Add the SUSP extension probe and ecall support, but for now the system suspend function is just a stub.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 187 lines
---
 include/sbi/sbi_ecall_interface.h |  8 ++++++
 include/sbi/sbi_system.h          | 17 +++++++++++
 lib/sbi/Kconfig                   |  4 +++
 lib/sbi/objects.mk                |  3 ++
 lib/sbi/sbi_ecall_susp.c          | 48 +++++++++++++++++++++++++++++++
 lib/sbi/sbi_system.c              | 26 +++++++++++++++++
 6 files changed, 106 insertions(+)
 create mode 100644 lib/sbi/sbi_ecall_susp.c
include/sbi/sbi_ecall_interface.h +8 -0
index a3f2bf4bdf69..98a426a74ec4 100644
@@ -29,6 +29,7 @@
 #define SBI_EXT_HSM                            0x48534D
 #define SBI_EXT_SRST                           0x53525354
 #define SBI_EXT_PMU                            0x504D55
+#define SBI_EXT_SUSP                           0x53555350
 
 /* SBI function IDs for BASE extension*/
 #define SBI_EXT_BASE_GET_SPEC_VERSION          0x0

@@ -230,6 +231,13 @@ enum sbi_pmu_ctr_type {
 /* Flags defined for counter stop function */
 #define SBI_PMU_STOP_FLAG_RESET (1 << 0)
 
+/* SBI function IDs for SUSP extension */
+#define SBI_EXT_SUSP_SUSPEND                   0x0
+
+#define SBI_SUSP_SLEEP_TYPE_SUSPEND            0x0
+#define SBI_SUSP_SLEEP_TYPE_LAST               SBI_SUSP_SLEEP_TYPE_SUSPEND
+#define SBI_SUSP_PLATFORM_SLEEP_START          0x80000000
+
 /* SBI base specification related macros */
 #define SBI_SPEC_VERSION_MAJOR_OFFSET          24
 #define SBI_SPEC_VERSION_MAJOR_MASK            0x7f
include/sbi/sbi_system.h +17 -0
index 84c281383d8c..65ea3d36d6cf 100644
@@ -43,4 +43,21 @@ bool sbi_system_reset_supported(u32 reset_type, u32 reset_reason);
 
 void __noreturn sbi_system_reset(u32 reset_type, u32 reset_reason);
 
+/** System suspend device */
+struct sbi_system_suspend_device {
+       /** Name of the system suspend device */
+       char name[32];
+
+       /* Check whether sleep type is supported by the device */
+       int (*system_suspend_check)(u32 sleep_type);
+
+       /** Suspend the system */
+       int (*system_suspend)(u32 sleep_type);
+};
+
+const struct sbi_system_suspend_device *sbi_system_suspend_get_device(void);
+void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev);
+bool sbi_system_suspend_supported(u32 sleep_type);
+int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque);
+
 #endif
lib/sbi/Kconfig +4 -0
index df74bba38540..7e139017fec7 100644
@@ -22,6 +22,10 @@ config SBI_ECALL_SRST
        bool "System Reset extension"
        default y
 
+config SBI_ECALL_SUSP
+       bool "System Suspend extension"
+       default y
+
 config SBI_ECALL_PMU
        bool "Performance Monitoring Unit extension"
        default y
lib/sbi/objects.mk +3 -0
index c774ebbcd142..c4f3d0991aa2 100644
@@ -34,6 +34,9 @@ libsbi-objs-$(CONFIG_SBI_ECALL_HSM) += sbi_ecall_hsm.o
 carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_SRST) += ecall_srst
 libsbi-objs-$(CONFIG_SBI_ECALL_SRST) += sbi_ecall_srst.o
 
+carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_SUSP) += ecall_susp
+libsbi-objs-$(CONFIG_SBI_ECALL_SUSP) += sbi_ecall_susp.o
+
 carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_PMU) += ecall_pmu
 libsbi-objs-$(CONFIG_SBI_ECALL_PMU) += sbi_ecall_pmu.o
lib/sbi/sbi_ecall_susp.c new file +48 -0
new file mode 100644
index 000000000000..f20126c49a60
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: BSD-2-Clause
+#include <sbi/sbi_ecall.h>
+#include <sbi/sbi_ecall_interface.h>
+#include <sbi/sbi_error.h>
+#include <sbi/sbi_trap.h>
+#include <sbi/sbi_system.h>
+
+static int sbi_ecall_susp_handler(unsigned long extid, unsigned long funcid,
+                                 const struct sbi_trap_regs *regs,
+                                 unsigned long *out_val,
+                                 struct sbi_trap_info *out_trap)
+{
+       int ret = SBI_ENOTSUPP;
+
+       if (funcid == SBI_EXT_SUSP_SUSPEND)
+               ret = sbi_system_suspend(regs->a0, regs->a1, regs->a2);
+
+       if (ret >= 0) {
+               *out_val = ret;
+               ret = 0;
+       }
+
+       return ret;
+}
+
+static int sbi_ecall_susp_probe(unsigned long extid, unsigned long *out_val)
+{
+       u32 type, count = 0;
+
+       /*
+        * At least one suspend type should be supported by the
+        * platform for the SBI SUSP extension to be usable.
+        */
+       for (type = 0; type <= SBI_SUSP_SLEEP_TYPE_LAST; type++) {
+               if (sbi_system_suspend_supported(type))
+                       count++;
+       }
+
+       *out_val = count ? 1 : 0;
+       return 0;
+}
+
+struct sbi_ecall_extension ecall_susp = {
+       .extid_start = SBI_EXT_SUSP,
+       .extid_end = SBI_EXT_SUSP,
+       .handle = sbi_ecall_susp_handler,
+       .probe = sbi_ecall_susp_probe,
+};
lib/sbi/sbi_system.c +26 -1
index f37c811d0bf3..5c123a6c9d8d 100644
@@ -92,3 +92,29 @@ void __noreturn sbi_system_reset(u32 reset_type, u32 reset_reason)
        /* If platform specific reset did not work then do sbi_exit() */
        sbi_exit(scratch);
 }
+
+static const struct sbi_system_suspend_device *suspend_dev = NULL;
+
+const struct sbi_system_suspend_device *sbi_system_suspend_get_device(void)
+{
+       return suspend_dev;
+}
+
+void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev)
+{
+       if (!dev || suspend_dev)
+               return;
+
+       suspend_dev = dev;
+}
+
+bool sbi_system_suspend_supported(u32 sleep_type)
+{
+       return suspend_dev && suspend_dev->system_suspend_check &&
+              suspend_dev->system_suspend_check(sleep_type);
+}
+
+int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque)
+{
+       return 0;
+}
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#15 lore raw

RFC PATCH 8/11 lib: sbi: Add system_suspend_allowed domain property

In reply to Andrew Jones

Only privileged domains should be allowed to suspend the entire system. Give the root domain this property by default and allow other domains to be given the property by specifying it in the DT.

Signed-off-by Andrew Jones <[email protected]>
---
 docs/domain_support.md     | 5 +++++
 include/sbi/sbi_domain.h   | 2 ++
 lib/sbi/sbi_domain.c       | 4 ++++
 lib/utils/fdt/fdt_domain.c | 7 +++++++
 4 files changed, 18 insertions(+)
docs/domain_support.md +5 -0
index 8963b57e3787..ac8c73d40b3e 100644
@@ -52,6 +52,7 @@ has following details:
 * **next_mode** - Privilege mode of the next booting stage for this
   domain. This can be either S-mode or U-mode.
 * **system_reset_allowed** - Is domain allowed to reset the system?
+* **system_suspend_allowed** - Is domain allowed to suspend the system?
 
 The memory regions represented by **regions** in **struct sbi_domain** have
 following additional constraints to align with RISC-V PMP requirements:

@@ -91,6 +92,7 @@ following manner:
 * **next_mode** - Next booting stage mode in coldboot HART scratch space
   is the next mode for the ROOT domain
 * **system_reset_allowed** - The ROOT domain is allowed to reset the system
+* **system_suspend_allowed** - The ROOT domain is allowed to suspend the system
 
 Domain Effects
 --------------

@@ -187,6 +189,8 @@ The DT properties of a domain instance DT node are as follows:
   stage mode of coldboot HART** is used as default value.
 * **system-reset-allowed** (Optional) - A boolean flag representing
   whether the domain instance is allowed to do system reset.
+* **system-suspend-allowed** (Optional) - A boolean flag representing
+  whether the domain instance is allowed to do system suspend.
 
 ### Assigning HART To Domain Instance
 

@@ -252,6 +256,7 @@ be done:
                 next-addr = <0x0 0x80100000>;
                 next-mode = <0x0>;
                 system-reset-allowed;
+                system-suspend-allowed;
             };
 
             udomain: untrusted-domain {
include/sbi/sbi_domain.h +2 -0
index f0d9289ec7cc..6f51b3f6324c 100644
@@ -78,6 +78,8 @@ struct sbi_domain {
 	unsigned long next_mode;
 	/** Is domain allowed to reset the system */
 	bool system_reset_allowed;
+	/** Is domain allowed to suspend the system */
+	bool system_suspend_allowed;
 };
 
 /** The root domain instance */
lib/sbi/sbi_domain.c +4 -0
index 19e2029e6c4c..38684cdc6527 100644
@@ -34,6 +34,7 @@ struct sbi_domain root = {
 	.possible_harts = &root_hmask,
 	.regions = root_memregs,
 	.system_reset_allowed = true,
+	.system_suspend_allowed = true,
 };
 
 bool sbi_domain_is_assigned_hart(const struct sbi_domain *dom, u32 hartid)

@@ -374,6 +375,9 @@ void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix)
 
 	sbi_printf("Domain%d SysReset    %s: %s\n",
 		   dom->index, suffix, (dom->system_reset_allowed) ? "yes" : "no");
+
+	sbi_printf("Domain%d SysSuspend  %s: %s\n",
+		   dom->index, suffix, (dom->system_suspend_allowed) ? "yes" : "no");
 }
 
 void sbi_domain_dump_all(const char *suffix)
lib/utils/fdt/fdt_domain.c +7 -0
index 35462a2008aa..32be6a5ab7e4 100644
@@ -405,6 +405,13 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 	else
 		dom->system_reset_allowed = false;
 
+	/* Read "system-suspend-allowed" DT property */
+	if (fdt_get_property(fdt, domain_offset,
+			     "system-suspend-allowed", NULL))
+		dom->system_suspend_allowed = true;
+	else
+		dom->system_suspend_allowed = false;
+
 	/* Find /cpus DT node */
 	cpus_offset = fdt_path_offset(fdt, "/cpus");
 	if (cpus_offset < 0)
Signature · 1 line
2.39.0
#16 lore raw

RFC PATCH 8/11 lib: sbi: Add system_suspend_allowed domain property

Quoted 7 lines · Andrew Jones

Only privileged domains should be allowed to suspend the entire system. Give the root domain this property by default and allow other domains to be given the property by specifying it in the DT.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 105 lines
---
 docs/domain_support.md     | 5 +++++
 include/sbi/sbi_domain.h   | 2 ++
 lib/sbi/sbi_domain.c       | 4 ++++
 lib/utils/fdt/fdt_domain.c | 7 +++++++
 4 files changed, 18 insertions(+)
docs/domain_support.md +5 -0
index 8963b57e3787..ac8c73d40b3e 100644
@@ -52,6 +52,7 @@ has following details:
 * **next_mode** - Privilege mode of the next booting stage for this
   domain. This can be either S-mode or U-mode.
 * **system_reset_allowed** - Is domain allowed to reset the system?
+* **system_suspend_allowed** - Is domain allowed to suspend the system?
 
 The memory regions represented by **regions** in **struct sbi_domain** have
 following additional constraints to align with RISC-V PMP requirements:

@@ -91,6 +92,7 @@ following manner:
 * **next_mode** - Next booting stage mode in coldboot HART scratch space
   is the next mode for the ROOT domain
 * **system_reset_allowed** - The ROOT domain is allowed to reset the system
+* **system_suspend_allowed** - The ROOT domain is allowed to suspend the system
 
 Domain Effects
 --------------

@@ -187,6 +189,8 @@ The DT properties of a domain instance DT node are as follows:
   stage mode of coldboot HART** is used as default value.
 * **system-reset-allowed** (Optional) - A boolean flag representing
   whether the domain instance is allowed to do system reset.
+* **system-suspend-allowed** (Optional) - A boolean flag representing
+  whether the domain instance is allowed to do system suspend.
 
 ### Assigning HART To Domain Instance
 

@@ -252,6 +256,7 @@ be done:
                 next-addr = <0x0 0x80100000>;
                 next-mode = <0x0>;
                 system-reset-allowed;
+                system-suspend-allowed;
             };
 
             udomain: untrusted-domain {
include/sbi/sbi_domain.h +2 -0
index f0d9289ec7cc..6f51b3f6324c 100644
@@ -78,6 +78,8 @@ struct sbi_domain {
        unsigned long next_mode;
        /** Is domain allowed to reset the system */
        bool system_reset_allowed;
+       /** Is domain allowed to suspend the system */
+       bool system_suspend_allowed;
 };
 
 /** The root domain instance */
lib/sbi/sbi_domain.c +4 -0
index 19e2029e6c4c..38684cdc6527 100644
@@ -34,6 +34,7 @@ struct sbi_domain root = {
        .possible_harts = &root_hmask,
        .regions = root_memregs,
        .system_reset_allowed = true,
+       .system_suspend_allowed = true,
 };
 
 bool sbi_domain_is_assigned_hart(const struct sbi_domain *dom, u32 hartid)

@@ -374,6 +375,9 @@ void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix)
 
        sbi_printf("Domain%d SysReset    %s: %s\n",
                   dom->index, suffix, (dom->system_reset_allowed) ? "yes" : "no");
+
+       sbi_printf("Domain%d SysSuspend  %s: %s\n",
+                  dom->index, suffix, (dom->system_suspend_allowed) ? "yes" : "no");
 }
 
 void sbi_domain_dump_all(const char *suffix)
lib/utils/fdt/fdt_domain.c +7 -1
index 35462a2008aa..32be6a5ab7e4 100644
@@ -405,6 +405,13 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
        else
                dom->system_reset_allowed = false;
 
+       /* Read "system-suspend-allowed" DT property */
+       if (fdt_get_property(fdt, domain_offset,
+                            "system-suspend-allowed", NULL))
+               dom->system_suspend_allowed = true;
+       else
+               dom->system_suspend_allowed = false;
+
        /* Find /cpus DT node */
        cpus_offset = fdt_path_offset(fdt, "/cpus");
        if (cpus_offset < 0)
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#17 lore raw

RFC PATCH 9/11 lib: sbi: Implement system suspend

In reply to Andrew Jones

Fill the implementation of the system suspend ecall. A platform implementation of the suspend callbacks is still required for this to do anything.

Signed-off-by Andrew Jones <[email protected]>
---
 lib/sbi/sbi_system.c | 57 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)
lib/sbi/sbi_system.c +56 -1
index 5c123a6c9d8d..c18562cdcfab 100644
@@ -116,5 +116,60 @@ bool sbi_system_suspend_supported(u32 sleep_type)
 
 int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque)
 {
-	return 0;
+	int ret = SBI_ENOTSUPP;
+	const struct sbi_domain *dom = sbi_domain_thishart_ptr();
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+	void (*jump_warmboot)(void) = (void (*)(void))scratch->warmboot_addr;
+	unsigned int hartid = current_hartid();
+	unsigned long prev_mode;
+	unsigned long i;
+
+	if (!dom || !dom->system_suspend_allowed)
+		return SBI_EFAIL;
+
+	if (!suspend_dev || !suspend_dev->system_suspend)
+		return SBI_EFAIL;
+
+	if (!sbi_system_suspend_supported(sleep_type))
+		return SBI_ENOTSUPP;
+
+	prev_mode = (csr_read(CSR_MSTATUS) & MSTATUS_MPP) >> MSTATUS_MPP_SHIFT;
+	if (prev_mode != PRV_S && prev_mode != PRV_U)
+		return SBI_EFAIL;
+
+	sbi_hartmask_for_each_hart(i, &dom->assigned_harts) {
+		if (i == hartid)
+			continue;
+		if (__sbi_hsm_hart_get_state(i) != SBI_HSM_STATE_STOPPED)
+			return SBI_EFAIL;
+	}
+
+	if (!sbi_domain_check_addr(dom, resume_addr, prev_mode,
+				   SBI_DOMAIN_EXECUTE))
+		return SBI_EINVALID_ADDR;
+
+	if (!sbi_hsm_hart_change_state(scratch, SBI_HSM_STATE_STARTED,
+				       SBI_HSM_STATE_SUSPENDED))
+		return SBI_EFAIL;
+
+	/* Prepare for resume */
+	scratch->next_mode = prev_mode;
+	scratch->next_addr = resume_addr;
+	scratch->next_arg1 = opaque;
+
+	__sbi_hsm_suspend_non_ret_save(scratch);
+
+	/* Suspend */
+	ret = suspend_dev->system_suspend(sleep_type);
+	if (ret != SBI_OK) {
+		if (!sbi_hsm_hart_change_state(scratch, SBI_HSM_STATE_SUSPENDED,
+					       SBI_HSM_STATE_STARTED))
+			sbi_hart_hang();
+		return ret;
+	}
+
+	/* Resume */
+	jump_warmboot();
+
+	__builtin_unreachable();
 }
Signature · 1 line
2.39.0
#18 lore raw

RFC PATCH 9/11 lib: sbi: Implement system suspend

Quoted 6 lines · Andrew Jones

Fill the implementation of the system suspend ecall. A platform implementation of the suspend callbacks is still required for this to do anything.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 78 lines
---
 lib/sbi/sbi_system.c | 57 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)
lib/sbi/sbi_system.c +56 -2
index 5c123a6c9d8d..c18562cdcfab 100644
@@ -116,5 +116,60 @@ bool sbi_system_suspend_supported(u32 sleep_type)
 
 int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque)
 {
-       return 0;
+       int ret = SBI_ENOTSUPP;
+       const struct sbi_domain *dom = sbi_domain_thishart_ptr();
+       struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+       void (*jump_warmboot)(void) = (void (*)(void))scratch->warmboot_addr;
+       unsigned int hartid = current_hartid();
+       unsigned long prev_mode;
+       unsigned long i;
+
+       if (!dom || !dom->system_suspend_allowed)
+               return SBI_EFAIL;
+
+       if (!suspend_dev || !suspend_dev->system_suspend)
+               return SBI_EFAIL;
+
+       if (!sbi_system_suspend_supported(sleep_type))
+               return SBI_ENOTSUPP;
+
+       prev_mode = (csr_read(CSR_MSTATUS) & MSTATUS_MPP) >> MSTATUS_MPP_SHIFT;
+       if (prev_mode != PRV_S && prev_mode != PRV_U)
+               return SBI_EFAIL;
+
+       sbi_hartmask_for_each_hart(i, &dom->assigned_harts) {
+               if (i == hartid)
+                       continue;
+               if (__sbi_hsm_hart_get_state(i) != SBI_HSM_STATE_STOPPED)
+                       return SBI_EFAIL;
+       }
+
+       if (!sbi_domain_check_addr(dom, resume_addr, prev_mode,
+                                  SBI_DOMAIN_EXECUTE))
+               return SBI_EINVALID_ADDR;
+
+       if (!sbi_hsm_hart_change_state(scratch, SBI_HSM_STATE_STARTED,
+                                      SBI_HSM_STATE_SUSPENDED))
+               return SBI_EFAIL;
+
+       /* Prepare for resume */
+       scratch->next_mode = prev_mode;
+       scratch->next_addr = resume_addr;
+       scratch->next_arg1 = opaque;
+
+       __sbi_hsm_suspend_non_ret_save(scratch);
+
+       /* Suspend */
+       ret = suspend_dev->system_suspend(sleep_type);
+       if (ret != SBI_OK) {
+               if (!sbi_hsm_hart_change_state(scratch, SBI_HSM_STATE_SUSPENDED,
+                                              SBI_HSM_STATE_STARTED))
+                       sbi_hart_hang();
+               return ret;
+       }
+
+       /* Resume */
+       jump_warmboot();
+
+       __builtin_unreachable();
 }
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#19 lore raw

RFC PATCH 10/11 docs: Correct opensbi-domain property name

In reply to Andrew Jones

Replace the commas with dashes to correct the name.

Signed-off-by Andrew Jones <[email protected]>
---
 docs/domain_support.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
docs/domain_support.md +3 -3
index ac8c73d40b3e..b2c1023008cc 100644
@@ -199,9 +199,9 @@ platform support can provide the HART to domain instance assignment using
 platform specific callback.
 
 The HART to domain instance assignment can be parsed from the device tree
-using optional DT property **opensbi,domain** in each CPU DT node. The
-value of DT property **opensbi,domain** is the DT phandle of the domain
-instance DT node. If **opensbi,domain** DT property is not specified then
+using optional DT property **opensbi-domain** in each CPU DT node. The
+value of DT property **opensbi-domain** is the DT phandle of the domain
+instance DT node. If **opensbi-domain** DT property is not specified then
 corresponding HART is assigned to **the ROOT domain**.
 
 ### Domain Configuration Only Accessible to OpenSBI
Signature · 1 line
2.39.0
#20 lore raw

RFC PATCH 10/11 docs: Correct opensbi-domain property name

Quoted 4 lines · Andrew Jones

Replace the commas with dashes to correct the name.

Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 29 lines
---
 docs/domain_support.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
docs/domain_support.md +3 -4
index ac8c73d40b3e..b2c1023008cc 100644
@@ -199,9 +199,9 @@ platform support can provide the HART to domain instance assignment using
 platform specific callback.
 
 The HART to domain instance assignment can be parsed from the device tree
-using optional DT property **opensbi,domain** in each CPU DT node. The
-value of DT property **opensbi,domain** is the DT phandle of the domain
-instance DT node. If **opensbi,domain** DT property is not specified then
+using optional DT property **opensbi-domain** in each CPU DT node. The
+value of DT property **opensbi-domain** is the DT phandle of the domain
+instance DT node. If **opensbi-domain** DT property is not specified then
 corresponding HART is assigned to **the ROOT domain**.
 
 ### Domain Configuration Only Accessible to OpenSBI
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#21 lore raw

RFC PATCH 11/11 platform: generic: Add system suspend test

In reply to Andrew Jones

When the system-suspend-test property is present in the domain config node as shown below, implement system suspend with a simple 5 second delay followed by a WFI. This allows testing system suspend when the low-level firmware doesn't support it.

  / {
    chosen {
      opensbi-domains {
          compatible = "opensbi,domain,config";
          system-suspend-test;
      };
Signed-off-by Andrew Jones <[email protected]>
---
 docs/domain_support.md      |  4 ++++
 include/sbi/sbi_system.h    |  1 +
 lib/sbi/sbi_system.c        | 30 ++++++++++++++++++++++++++++++
 platform/generic/platform.c | 20 +++++++++++++++++++-
 4 files changed, 54 insertions(+), 1 deletion(-)
docs/domain_support.md +4 -0
index b2c1023008cc..d86628049b62 100644
@@ -126,6 +126,9 @@ The DT properties of a domain configuration DT node are as follows:
 * **compatible** (Mandatory) - The compatible string of the domain
   configuration. This DT property should have value *"opensbi,domain,config"*
 
+* **system-suspend-test** (Optional) - When present, enable a system
+  suspend test implementation which simply waits five seconds and issues a WFI.
+
 ### Domain Memory Region Node
 
 The domain memory region DT node describes details of a memory region and

@@ -226,6 +229,7 @@ be done:
     chosen {
         opensbi-domains {
             compatible = "opensbi,domain,config";
+            system-suspend-test;
 
             tmem: tmem {
                 compatible = "opensbi,domain,memregion";
include/sbi/sbi_system.h +1 -0
index 65ea3d36d6cf..7d0867f276aa 100644
@@ -57,6 +57,7 @@ struct sbi_system_suspend_device {
 
 const struct sbi_system_suspend_device *sbi_system_suspend_get_device(void);
 void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev);
+void sbi_system_suspend_test_enable(void);
 bool sbi_system_suspend_supported(u32 sleep_type);
 int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque);
lib/sbi/sbi_system.c +30 -0
index c18562cdcfab..3242b21699cb 100644
@@ -17,6 +17,7 @@
 #include <sbi/sbi_system.h>
 #include <sbi/sbi_ipi.h>
 #include <sbi/sbi_init.h>
+#include <sbi/sbi_timer.h>
 
 static SBI_LIST_HEAD(reset_devices_list);
 

@@ -108,6 +109,35 @@ void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev)
 	suspend_dev = dev;
 }
 
+static int sbi_system_suspend_test_check(u32 sleep_type)
+{
+	return sleep_type == SBI_SUSP_SLEEP_TYPE_SUSPEND;
+}
+
+static int sbi_system_suspend_test_suspend(u32 sleep_type)
+{
+	if (sleep_type != SBI_SUSP_SLEEP_TYPE_SUSPEND)
+		return SBI_EINVAL;
+
+	sbi_timer_mdelay(5000);
+
+	/* Wait for interrupt */
+	wfi();
+
+	return SBI_OK;
+}
+
+static struct sbi_system_suspend_device sbi_system_suspend_test = {
+	.name = "system-suspend-test",
+	.system_suspend_check = sbi_system_suspend_test_check,
+	.system_suspend = sbi_system_suspend_test_suspend,
+};
+
+void sbi_system_suspend_test_enable(void)
+{
+	sbi_system_suspend_set_device(&sbi_system_suspend_test);
+}
+
 bool sbi_system_suspend_supported(u32 sleep_type)
 {
 	return suspend_dev && suspend_dev->system_suspend_check &&
platform/generic/platform.c +19 -1
index bfe15f0d6b87..b3a3351c0439 100644
@@ -13,6 +13,7 @@
 #include <sbi/sbi_hartmask.h>
 #include <sbi/sbi_platform.h>
 #include <sbi/sbi_string.h>
+#include <sbi/sbi_system.h>
 #include <sbi_utils/fdt/fdt_domain.h>
 #include <sbi_utils/fdt/fdt_fixup.h>
 #include <sbi_utils/fdt/fdt_helper.h>

@@ -215,7 +216,24 @@ static int generic_extensions_init(struct sbi_hart_features *hfeatures)
 
 static int generic_domains_init(void)
 {
-	return fdt_domains_populate(fdt_get_address());
+	void *fdt = fdt_get_address();
+	int offset, ret;
+
+	ret = fdt_domains_populate(fdt);
+	if (ret < 0)
+		return ret;
+
+	offset = fdt_path_offset(fdt, "/chosen");
+
+	if (offset >= 0) {
+		offset = fdt_node_offset_by_compatible(fdt, offset,
+						       "opensbi,domain,config");
+		if (offset >= 0 &&
+		    fdt_get_property(fdt, offset, "system-suspend-test", NULL))
+			sbi_system_suspend_test_enable();
+	}
+
+	return 0;
 }
 
 static u64 generic_tlbr_flush_limit(void)
Signature · 1 line
2.39.0
#22 lore raw

RFC PATCH 11/11 platform: generic: Add system suspend test

Quoted 6 lines · Andrew Jones

When the system-suspend-test property is present in the domain config node as shown below, implement system suspend with a simple 5 second delay followed by a WFI. This allows testing system suspend when the low-level firmware doesn't support it.

Quoted 8 lines
  / {
    chosen {
      opensbi-domains {
          compatible = "opensbi,domain,config";
          system-suspend-test;
      };
Signed-off-by Andrew Jones <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 135 lines
---
 docs/domain_support.md      |  4 ++++
 include/sbi/sbi_system.h    |  1 +
 lib/sbi/sbi_system.c        | 30 ++++++++++++++++++++++++++++++
 platform/generic/platform.c | 20 +++++++++++++++++++-
 4 files changed, 54 insertions(+), 1 deletion(-)
docs/domain_support.md +4 -0
index b2c1023008cc..d86628049b62 100644
@@ -126,6 +126,9 @@ The DT properties of a domain configuration DT node are as follows:
 * **compatible** (Mandatory) - The compatible string of the domain
   configuration. This DT property should have value *"opensbi,domain,config"*
 
+* **system-suspend-test** (Optional) - When present, enable a system
+  suspend test implementation which simply waits five seconds and issues a WFI.
+
 ### Domain Memory Region Node
 
 The domain memory region DT node describes details of a memory region and

@@ -226,6 +229,7 @@ be done:
     chosen {
         opensbi-domains {
             compatible = "opensbi,domain,config";
+            system-suspend-test;
 
             tmem: tmem {
                 compatible = "opensbi,domain,memregion";
include/sbi/sbi_system.h +1 -0
index 65ea3d36d6cf..7d0867f276aa 100644
@@ -57,6 +57,7 @@ struct sbi_system_suspend_device {
 
 const struct sbi_system_suspend_device *sbi_system_suspend_get_device(void);
 void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev);
+void sbi_system_suspend_test_enable(void);
 bool sbi_system_suspend_supported(u32 sleep_type);
 int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque);
lib/sbi/sbi_system.c +30 -0
index c18562cdcfab..3242b21699cb 100644
@@ -17,6 +17,7 @@
 #include <sbi/sbi_system.h>
 #include <sbi/sbi_ipi.h>
 #include <sbi/sbi_init.h>
+#include <sbi/sbi_timer.h>
 
 static SBI_LIST_HEAD(reset_devices_list);
 

@@ -108,6 +109,35 @@ void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev)
        suspend_dev = dev;
 }
 
+static int sbi_system_suspend_test_check(u32 sleep_type)
+{
+       return sleep_type == SBI_SUSP_SLEEP_TYPE_SUSPEND;
+}
+
+static int sbi_system_suspend_test_suspend(u32 sleep_type)
+{
+       if (sleep_type != SBI_SUSP_SLEEP_TYPE_SUSPEND)
+               return SBI_EINVAL;
+
+       sbi_timer_mdelay(5000);
+
+       /* Wait for interrupt */
+       wfi();
+
+       return SBI_OK;
+}
+
+static struct sbi_system_suspend_device sbi_system_suspend_test = {
+       .name = "system-suspend-test",
+       .system_suspend_check = sbi_system_suspend_test_check,
+       .system_suspend = sbi_system_suspend_test_suspend,
+};
+
+void sbi_system_suspend_test_enable(void)
+{
+       sbi_system_suspend_set_device(&sbi_system_suspend_test);
+}
+
 bool sbi_system_suspend_supported(u32 sleep_type)
 {
        return suspend_dev && suspend_dev->system_suspend_check &&
platform/generic/platform.c +19 -2
index bfe15f0d6b87..b3a3351c0439 100644
@@ -13,6 +13,7 @@
 #include <sbi/sbi_hartmask.h>
 #include <sbi/sbi_platform.h>
 #include <sbi/sbi_string.h>
+#include <sbi/sbi_system.h>
 #include <sbi_utils/fdt/fdt_domain.h>
 #include <sbi_utils/fdt/fdt_fixup.h>
 #include <sbi_utils/fdt/fdt_helper.h>

@@ -215,7 +216,24 @@ static int generic_extensions_init(struct sbi_hart_features *hfeatures)
 
 static int generic_domains_init(void)
 {
-       return fdt_domains_populate(fdt_get_address());
+       void *fdt = fdt_get_address();
+       int offset, ret;
+
+       ret = fdt_domains_populate(fdt);
+       if (ret < 0)
+               return ret;
+
+       offset = fdt_path_offset(fdt, "/chosen");
+
+       if (offset >= 0) {
+               offset = fdt_node_offset_by_compatible(fdt, offset,
+                                                      "opensbi,domain,config");
+               if (offset >= 0 &&
+                   fdt_get_property(fdt, offset, "system-suspend-test", NULL))
+                       sbi_system_suspend_test_enable();
+       }
+
+       return 0;
 }
 
 static u64 generic_tlbr_flush_limit(void)
--

2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#23 lore raw

cover SBI system suspend (SUSP) extension

In reply to Andrew Jones
Quoted 3 lines · Andrew Jones

This series implements the SBI system suspend support in OpenSBI as-per the draft proposal for SBI system suspend which can be found at
https://github.com/jones-drew/riscv-sbi-doc/commit/d9e43e9a938fc3eb510e023c3f352462876f7785

The extension proposal has been posted to tech-prs,
https://lists.riscv.org/g/tech-prs/message/75

Quoted 12 lines

The first half of the series are a few cleanups and fixes for HSM, as system suspend makes use of some of its code and patterns. The second half implements support for the extension and includes a patch which allows testing to be enabled, even when low-level firmware does not provide support. Regarding the test mode, it's currently enabled by adding a 'system-suspend-test' DT property, but Anup has suggested that FW_OPTION could also be used. I'm all ears for what people think the best way to control test functionality in OpenSBI is and am happy to switch it. To test with Linux the kernel must include the patch at
https://github.com/jones-drew/linux/commits/riscv/sbi-susp-rfc and be built with CONFIG_SUSPEND enabled.

The linux patch has been posted,
https://lore.kernel.org/all/20230106113216.443057-1-ajones at ventanamicro.com/

Quoted 38 lines

The patches are based on v1.2 and "treewide: Replace TRUE/FALSE with true/false" and can also be found at
https://github.com/jones-drew/opensbi/commits/susp-rfc

Andrew Jones (11):
lib: sbi_hsm: Factor out invalid state detection
lib: sbi_hsm: Don't try to restore state on failed change
lib: sbi_hsm: Ensure errors are consistent with spec
lib: sbi_hsm: Move misplaced comment
lib: sbi_hsm: Remove unnecessary include
lib: sbi_hsm: Export some functions
lib: sbi: Add system suspend skeleton
lib: sbi: Add system_suspend_allowed domain property
lib: sbi: Implement system suspend
docs: Correct opensbi-domain property name
platform: generic: Add system suspend test

 docs/domain_support.md            |  15 +++-
 include/sbi/sbi_domain.h          |   2 +
 include/sbi/sbi_ecall_interface.h |   8 +++
 include/sbi/sbi_hsm.h             |   4 ++
 include/sbi/sbi_system.h          |  18 +++++
 lib/sbi/Kconfig                   |   4 ++
 lib/sbi/objects.mk                |   3 +
 lib/sbi/sbi_domain.c              |   4 ++
 lib/sbi/sbi_ecall_hsm.c           |   4 +-
 lib/sbi/sbi_ecall_susp.c          |  48 +++++++++++++
 lib/sbi/sbi_hsm.c                 |  92 +++++++++++--------------
 lib/sbi/sbi_system.c              | 111 ++++++++++++++++++++++++++++++
 lib/utils/fdt/fdt_domain.c        |   7 ++
 platform/generic/platform.c       |  20 +++++-
 14 files changed, 283 insertions(+), 57 deletions(-)
 create mode 100644 lib/sbi/sbi_ecall_susp.c
Signature · 1 line
2.39.0
#24 lore raw

cover SBI system suspend (SUSP) extension

In reply to Andrew Jones
Quoted 21 lines · Andrew Jones

This series implements the SBI system suspend support in OpenSBI as-per the draft proposal for SBI system suspend which can be found at
https://github.com/jones-drew/riscv-sbi-doc/commit/d9e43e9a938fc3eb510e023c3f352462876f7785

The first half of the series are a few cleanups and fixes for HSM, as system suspend makes use of some of its code and patterns. The second half implements support for the extension and includes a patch which allows testing to be enabled, even when low-level firmware does not provide support. Regarding the test mode, it's currently enabled by adding a 'system-suspend-test' DT property, but Anup has suggested that FW_OPTION could also be used. I'm all ears for what people think the best way to control test functionality in OpenSBI is and am happy to switch it. To test with Linux the kernel must include the patch at
https://github.com/jones-drew/linux/commits/riscv/sbi-susp-rfc and be built with CONFIG_SUSPEND enabled.

The patches are based on v1.2 and "treewide: Replace TRUE/FALSE with true/false" and can also be found at
https://github.com/jones-drew/opensbi/commits/susp-rfc

Quoted 12 lines

Andrew Jones (11):
lib: sbi_hsm: Factor out invalid state detection
lib: sbi_hsm: Don't try to restore state on failed change
lib: sbi_hsm: Ensure errors are consistent with spec
lib: sbi_hsm: Move misplaced comment
lib: sbi_hsm: Remove unnecessary include
lib: sbi_hsm: Export some functions
lib: sbi: Add system suspend skeleton
lib: sbi: Add system_suspend_allowed domain property
lib: sbi: Implement system suspend
docs: Correct opensbi-domain property name
platform: generic: Add system suspend test

I must say that this is pretty neatly done. Great work !!!

Could you please add a boot print for suspend device in sbi_boot_print_general() ?

Regards,
Anup

Quoted 26 lines
 docs/domain_support.md            |  15 +++-
 include/sbi/sbi_domain.h          |   2 +
 include/sbi/sbi_ecall_interface.h |   8 +++
 include/sbi/sbi_hsm.h             |   4 ++
 include/sbi/sbi_system.h          |  18 +++++
 lib/sbi/Kconfig                   |   4 ++
 lib/sbi/objects.mk                |   3 +
 lib/sbi/sbi_domain.c              |   4 ++
 lib/sbi/sbi_ecall_hsm.c           |   4 +-
 lib/sbi/sbi_ecall_susp.c          |  48 +++++++++++++
 lib/sbi/sbi_hsm.c                 |  92 +++++++++++--------------
 lib/sbi/sbi_system.c              | 111 ++++++++++++++++++++++++++++++
 lib/utils/fdt/fdt_domain.c        |   7 ++
 platform/generic/platform.c       |  20 +++++-
 14 files changed, 283 insertions(+), 57 deletions(-)
 create mode 100644 lib/sbi/sbi_ecall_susp.c

--
2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

author
#25 lore raw

cover SBI system suspend (SUSP) extension

Quoted 35 lines · Anup Patel
Quoted 21 lines · Andrew Jones

This series implements the SBI system suspend support in OpenSBI as-per the draft proposal for SBI system suspend which can be found at
https://github.com/jones-drew/riscv-sbi-doc/commit/d9e43e9a938fc3eb510e023c3f352462876f7785

The first half of the series are a few cleanups and fixes for HSM, as system suspend makes use of some of its code and patterns. The second half implements support for the extension and includes a patch which allows testing to be enabled, even when low-level firmware does not provide support. Regarding the test mode, it's currently enabled by adding a 'system-suspend-test' DT property, but Anup has suggested that FW_OPTION could also be used. I'm all ears for what people think the best way to control test functionality in OpenSBI is and am happy to switch it. To test with Linux the kernel must include the patch at
https://github.com/jones-drew/linux/commits/riscv/sbi-susp-rfc and be built with CONFIG_SUSPEND enabled.

The patches are based on v1.2 and "treewide: Replace TRUE/FALSE with true/false" and can also be found at
https://github.com/jones-drew/opensbi/commits/susp-rfc

Quoted 12 lines

Andrew Jones (11):
lib: sbi_hsm: Factor out invalid state detection
lib: sbi_hsm: Don't try to restore state on failed change
lib: sbi_hsm: Ensure errors are consistent with spec
lib: sbi_hsm: Move misplaced comment
lib: sbi_hsm: Remove unnecessary include
lib: sbi_hsm: Export some functions
lib: sbi: Add system suspend skeleton
lib: sbi: Add system_suspend_allowed domain property
lib: sbi: Implement system suspend
docs: Correct opensbi-domain property name
platform: generic: Add system suspend test

Quoted 1 line

I must say that this is pretty neatly done. Great work !!!

Thank you, Anup!

Quoted 3 lines

Could you please add a boot print for suspend device in sbi_boot_print_general() ?

Will do.

Thanks,
drew

Quoted 30 lines

Regards,
Anup

Quoted 26 lines
 docs/domain_support.md            |  15 +++-
 include/sbi/sbi_domain.h          |   2 +
 include/sbi/sbi_ecall_interface.h |   8 +++
 include/sbi/sbi_hsm.h             |   4 ++
 include/sbi/sbi_system.h          |  18 +++++
 lib/sbi/Kconfig                   |   4 ++
 lib/sbi/objects.mk                |   3 +
 lib/sbi/sbi_domain.c              |   4 ++
 lib/sbi/sbi_ecall_hsm.c           |   4 +-
 lib/sbi/sbi_ecall_susp.c          |  48 +++++++++++++
 lib/sbi/sbi_hsm.c                 |  92 +++++++++++--------------
 lib/sbi/sbi_system.c              | 111 ++++++++++++++++++++++++++++++
 lib/utils/fdt/fdt_domain.c        |   7 ++
 platform/generic/platform.c       |  20 +++++-
 14 files changed, 283 insertions(+), 57 deletions(-)
 create mode 100644 lib/sbi/sbi_ecall_susp.c

--
2.39.0

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi