* [RFC PATCH 01/11] lib: sbi_hsm: Factor out invalid state detection
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
@ 2023-01-06 11:21 ` Andrew Jones
2023-01-17 3:36 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 02/11] lib: sbi_hsm: Don't try to restore state on failed change Andrew Jones
` (11 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:21 UTC (permalink / raw)
To: opensbi
Remove some redundant code by creating an invalid state detection
macro.
No functional change intended.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
lib/sbi/sbi_hsm.c | 65 +++++++++++++++++------------------------------
1 file changed, 24 insertions(+), 41 deletions(-)
diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
index 836008fc92d7..1896f52c2ab2 100644
--- a/lib/sbi/sbi_hsm.c
+++ b/lib/sbi/sbi_hsm.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 01/11] lib: sbi_hsm: Factor out invalid state detection
2023-01-06 11:21 ` [RFC PATCH 01/11] lib: sbi_hsm: Factor out invalid state detection Andrew Jones
@ 2023-01-17 3:36 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:36 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Remove some redundant code by creating an invalid state detection
> macro.
>
> No functional change intended.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> lib/sbi/sbi_hsm.c | 65 +++++++++++++++++------------------------------
> 1 file changed, 24 insertions(+), 41 deletions(-)
>
> diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
> index 836008fc92d7..1896f52c2ab2 100644
> --- a/lib/sbi/sbi_hsm.c
> +++ b/lib/sbi/sbi_hsm.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 02/11] lib: sbi_hsm: Don't try to restore state on failed change
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
2023-01-06 11:21 ` [RFC PATCH 01/11] lib: sbi_hsm: Factor out invalid state detection Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:36 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 03/11] lib: sbi_hsm: Ensure errors are consistent with spec Andrew Jones
` (10 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
When a state change fails there's no need to restore the original
state as it remains the same.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
lib/sbi/sbi_hsm.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
index 1896f52c2ab2..6ef6c5bdd4a7 100644
--- a/lib/sbi/sbi_hsm.c
+++ b/lib/sbi/sbi_hsm.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 02/11] lib: sbi_hsm: Don't try to restore state on failed change
2023-01-06 11:22 ` [RFC PATCH 02/11] lib: sbi_hsm: Don't try to restore state on failed change Andrew Jones
@ 2023-01-17 3:36 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:36 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> When a state change fails there's no need to restore the original
> state as it remains the same.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> lib/sbi/sbi_hsm.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
> index 1896f52c2ab2..6ef6c5bdd4a7 100644
> --- a/lib/sbi/sbi_hsm.c
> +++ b/lib/sbi/sbi_hsm.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 03/11] lib: sbi_hsm: Ensure errors are consistent with spec
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
2023-01-06 11:21 ` [RFC PATCH 01/11] lib: sbi_hsm: Factor out invalid state detection Andrew Jones
2023-01-06 11:22 ` [RFC PATCH 02/11] lib: sbi_hsm: Don't try to restore state on failed change Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:37 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 04/11] lib: sbi_hsm: Move misplaced comment Andrew Jones
` (9 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
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 <ajones@ventanamicro.com>
---
lib/sbi/sbi_hsm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
index 6ef6c5bdd4a7..3635f968e020 100644
--- a/lib/sbi/sbi_hsm.c
+++ b/lib/sbi/sbi_hsm.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 03/11] lib: sbi_hsm: Ensure errors are consistent with spec
2023-01-06 11:22 ` [RFC PATCH 03/11] lib: sbi_hsm: Ensure errors are consistent with spec Andrew Jones
@ 2023-01-17 3:37 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:37 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> 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 <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> lib/sbi/sbi_hsm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
> index 6ef6c5bdd4a7..3635f968e020 100644
> --- a/lib/sbi/sbi_hsm.c
> +++ b/lib/sbi/sbi_hsm.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 04/11] lib: sbi_hsm: Move misplaced comment
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (2 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 03/11] lib: sbi_hsm: Ensure errors are consistent with spec Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:39 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 05/11] lib: sbi_hsm: Remove unnecessary include Andrew Jones
` (8 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
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 <ajones@ventanamicro.com>
---
lib/sbi/sbi_hsm.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
index 3635f968e020..cce4b07bbb36 100644
--- a/lib/sbi/sbi_hsm.c
+++ b/lib/sbi/sbi_hsm.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 04/11] lib: sbi_hsm: Move misplaced comment
2023-01-06 11:22 ` [RFC PATCH 04/11] lib: sbi_hsm: Move misplaced comment Andrew Jones
@ 2023-01-17 3:39 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:39 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> 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 <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> lib/sbi/sbi_hsm.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
> index 3635f968e020..cce4b07bbb36 100644
> --- a/lib/sbi/sbi_hsm.c
> +++ b/lib/sbi/sbi_hsm.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 05/11] lib: sbi_hsm: Remove unnecessary include
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (3 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 04/11] lib: sbi_hsm: Move misplaced comment Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:39 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 06/11] lib: sbi_hsm: Export some functions Andrew Jones
` (7 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
Also remove a superfluous semicolon and add a blank line.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
lib/sbi/sbi_ecall_hsm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/sbi/sbi_ecall_hsm.c b/lib/sbi/sbi_ecall_hsm.c
index 703a824493b4..f1b41d073b22 100644
--- a/lib/sbi/sbi_ecall_hsm.c
+++ b/lib/sbi/sbi_ecall_hsm.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 05/11] lib: sbi_hsm: Remove unnecessary include
2023-01-06 11:22 ` [RFC PATCH 05/11] lib: sbi_hsm: Remove unnecessary include Andrew Jones
@ 2023-01-17 3:39 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:39 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Also remove a superfluous semicolon and add a blank line.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> lib/sbi/sbi_ecall_hsm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/sbi/sbi_ecall_hsm.c b/lib/sbi/sbi_ecall_hsm.c
> index 703a824493b4..f1b41d073b22 100644
> --- a/lib/sbi/sbi_ecall_hsm.c
> +++ b/lib/sbi/sbi_ecall_hsm.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 06/11] lib: sbi_hsm: Export some functions
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (4 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 05/11] lib: sbi_hsm: Remove unnecessary include Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:40 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 07/11] lib: sbi: Add system suspend skeleton Andrew Jones
` (6 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
A coming patch can make use of a few internal hsm functions if
we export them.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
include/sbi/sbi_hsm.h | 4 ++++
lib/sbi/sbi_hsm.c | 12 ++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/sbi/sbi_hsm.h b/include/sbi/sbi_hsm.h
index d6cc468d0528..00adfe1a4ef1 100644
--- a/include/sbi/sbi_hsm.h
+++ b/include/sbi/sbi_hsm.h
@@ -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
diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
index cce4b07bbb36..8eb9643e6e72 100644
--- a/lib/sbi/sbi_hsm.c
+++ b/lib/sbi/sbi_hsm.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 06/11] lib: sbi_hsm: Export some functions
2023-01-06 11:22 ` [RFC PATCH 06/11] lib: sbi_hsm: Export some functions Andrew Jones
@ 2023-01-17 3:40 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:40 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> A coming patch can make use of a few internal hsm functions if
> we export them.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> include/sbi/sbi_hsm.h | 4 ++++
> lib/sbi/sbi_hsm.c | 12 ++++++++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/include/sbi/sbi_hsm.h b/include/sbi/sbi_hsm.h
> index d6cc468d0528..00adfe1a4ef1 100644
> --- a/include/sbi/sbi_hsm.h
> +++ b/include/sbi/sbi_hsm.h
> @@ -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
> diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
> index cce4b07bbb36..8eb9643e6e72 100644
> --- a/lib/sbi/sbi_hsm.c
> +++ b/lib/sbi/sbi_hsm.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 07/11] lib: sbi: Add system suspend skeleton
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (5 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 06/11] lib: sbi_hsm: Export some functions Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:46 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 08/11] lib: sbi: Add system_suspend_allowed domain property Andrew Jones
` (5 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
Add the SUSP extension probe and ecall support, but for now the
system suspend function is just a stub.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
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
diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
index a3f2bf4bdf69..98a426a74ec4 100644
--- a/include/sbi/sbi_ecall_interface.h
+++ b/include/sbi/sbi_ecall_interface.h
@@ -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
diff --git a/include/sbi/sbi_system.h b/include/sbi/sbi_system.h
index 84c281383d8c..65ea3d36d6cf 100644
--- a/include/sbi/sbi_system.h
+++ b/include/sbi/sbi_system.h
@@ -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
diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
index df74bba38540..7e139017fec7 100644
--- a/lib/sbi/Kconfig
+++ b/lib/sbi/Kconfig
@@ -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
diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
index c774ebbcd142..c4f3d0991aa2 100644
--- a/lib/sbi/objects.mk
+++ b/lib/sbi/objects.mk
@@ -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
diff --git a/lib/sbi/sbi_ecall_susp.c b/lib/sbi/sbi_ecall_susp.c
new file mode 100644
index 000000000000..f20126c49a60
--- /dev/null
+++ b/lib/sbi/sbi_ecall_susp.c
@@ -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,
+};
diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
index f37c811d0bf3..5c123a6c9d8d 100644
--- a/lib/sbi/sbi_system.c
+++ b/lib/sbi/sbi_system.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 07/11] lib: sbi: Add system suspend skeleton
2023-01-06 11:22 ` [RFC PATCH 07/11] lib: sbi: Add system suspend skeleton Andrew Jones
@ 2023-01-17 3:46 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:46 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Add the SUSP extension probe and ecall support, but for now the
> system suspend function is just a stub.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> 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
>
> diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
> index a3f2bf4bdf69..98a426a74ec4 100644
> --- a/include/sbi/sbi_ecall_interface.h
> +++ b/include/sbi/sbi_ecall_interface.h
> @@ -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
> diff --git a/include/sbi/sbi_system.h b/include/sbi/sbi_system.h
> index 84c281383d8c..65ea3d36d6cf 100644
> --- a/include/sbi/sbi_system.h
> +++ b/include/sbi/sbi_system.h
> @@ -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
> diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
> index df74bba38540..7e139017fec7 100644
> --- a/lib/sbi/Kconfig
> +++ b/lib/sbi/Kconfig
> @@ -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
> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> index c774ebbcd142..c4f3d0991aa2 100644
> --- a/lib/sbi/objects.mk
> +++ b/lib/sbi/objects.mk
> @@ -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
>
> diff --git a/lib/sbi/sbi_ecall_susp.c b/lib/sbi/sbi_ecall_susp.c
> new file mode 100644
> index 000000000000..f20126c49a60
> --- /dev/null
> +++ b/lib/sbi/sbi_ecall_susp.c
> @@ -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,
> +};
> diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
> index f37c811d0bf3..5c123a6c9d8d 100644
> --- a/lib/sbi/sbi_system.c
> +++ b/lib/sbi/sbi_system.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 08/11] lib: sbi: Add system_suspend_allowed domain property
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (6 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 07/11] lib: sbi: Add system suspend skeleton Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:47 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 09/11] lib: sbi: Implement system suspend Andrew Jones
` (4 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
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 <ajones@ventanamicro.com>
---
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(+)
diff --git a/docs/domain_support.md b/docs/domain_support.md
index 8963b57e3787..ac8c73d40b3e 100644
--- a/docs/domain_support.md
+++ b/docs/domain_support.md
@@ -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 {
diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index f0d9289ec7cc..6f51b3f6324c 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -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 */
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 19e2029e6c4c..38684cdc6527 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -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)
diff --git a/lib/utils/fdt/fdt_domain.c b/lib/utils/fdt/fdt_domain.c
index 35462a2008aa..32be6a5ab7e4 100644
--- a/lib/utils/fdt/fdt_domain.c
+++ b/lib/utils/fdt/fdt_domain.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 08/11] lib: sbi: Add system_suspend_allowed domain property
2023-01-06 11:22 ` [RFC PATCH 08/11] lib: sbi: Add system_suspend_allowed domain property Andrew Jones
@ 2023-01-17 3:47 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:47 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> 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 <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> 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(+)
>
> diff --git a/docs/domain_support.md b/docs/domain_support.md
> index 8963b57e3787..ac8c73d40b3e 100644
> --- a/docs/domain_support.md
> +++ b/docs/domain_support.md
> @@ -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 {
> diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
> index f0d9289ec7cc..6f51b3f6324c 100644
> --- a/include/sbi/sbi_domain.h
> +++ b/include/sbi/sbi_domain.h
> @@ -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 */
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index 19e2029e6c4c..38684cdc6527 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -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)
> diff --git a/lib/utils/fdt/fdt_domain.c b/lib/utils/fdt/fdt_domain.c
> index 35462a2008aa..32be6a5ab7e4 100644
> --- a/lib/utils/fdt/fdt_domain.c
> +++ b/lib/utils/fdt/fdt_domain.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 09/11] lib: sbi: Implement system suspend
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (7 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 08/11] lib: sbi: Add system_suspend_allowed domain property Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:49 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 10/11] docs: Correct opensbi-domain property name Andrew Jones
` (3 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
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 <ajones@ventanamicro.com>
---
lib/sbi/sbi_system.c | 57 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
index 5c123a6c9d8d..c18562cdcfab 100644
--- a/lib/sbi/sbi_system.c
+++ b/lib/sbi/sbi_system.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 09/11] lib: sbi: Implement system suspend
2023-01-06 11:22 ` [RFC PATCH 09/11] lib: sbi: Implement system suspend Andrew Jones
@ 2023-01-17 3:49 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:49 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> 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 <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> lib/sbi/sbi_system.c | 57 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
> index 5c123a6c9d8d..c18562cdcfab 100644
> --- a/lib/sbi/sbi_system.c
> +++ b/lib/sbi/sbi_system.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 10/11] docs: Correct opensbi-domain property name
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (8 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 09/11] lib: sbi: Implement system suspend Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:51 ` Anup Patel
2023-01-06 11:22 ` [RFC PATCH 11/11] platform: generic: Add system suspend test Andrew Jones
` (2 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
Replace the commas with dashes to correct the name.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
docs/domain_support.md | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/docs/domain_support.md b/docs/domain_support.md
index ac8c73d40b3e..b2c1023008cc 100644
--- a/docs/domain_support.md
+++ b/docs/domain_support.md
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 10/11] docs: Correct opensbi-domain property name
2023-01-06 11:22 ` [RFC PATCH 10/11] docs: Correct opensbi-domain property name Andrew Jones
@ 2023-01-17 3:51 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:51 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Replace the commas with dashes to correct the name.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> docs/domain_support.md | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/docs/domain_support.md b/docs/domain_support.md
> index ac8c73d40b3e..b2c1023008cc 100644
> --- a/docs/domain_support.md
> +++ b/docs/domain_support.md
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 11/11] platform: generic: Add system suspend test
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (9 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 10/11] docs: Correct opensbi-domain property name Andrew Jones
@ 2023-01-06 11:22 ` Andrew Jones
2023-01-17 3:45 ` Anup Patel
2023-01-06 11:34 ` [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
2023-01-17 3:54 ` Anup Patel
12 siblings, 1 reply; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:22 UTC (permalink / raw)
To: opensbi
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 <ajones@ventanamicro.com>
---
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(-)
diff --git a/docs/domain_support.md b/docs/domain_support.md
index b2c1023008cc..d86628049b62 100644
--- a/docs/domain_support.md
+++ b/docs/domain_support.md
@@ -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";
diff --git a/include/sbi/sbi_system.h b/include/sbi/sbi_system.h
index 65ea3d36d6cf..7d0867f276aa 100644
--- a/include/sbi/sbi_system.h
+++ b/include/sbi/sbi_system.h
@@ -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);
diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
index c18562cdcfab..3242b21699cb 100644
--- a/lib/sbi/sbi_system.c
+++ b/lib/sbi/sbi_system.c
@@ -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 &&
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index bfe15f0d6b87..b3a3351c0439 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 11/11] platform: generic: Add system suspend test
2023-01-06 11:22 ` [RFC PATCH 11/11] platform: generic: Add system suspend test Andrew Jones
@ 2023-01-17 3:45 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:45 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> 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 <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> 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(-)
>
> diff --git a/docs/domain_support.md b/docs/domain_support.md
> index b2c1023008cc..d86628049b62 100644
> --- a/docs/domain_support.md
> +++ b/docs/domain_support.md
> @@ -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";
> diff --git a/include/sbi/sbi_system.h b/include/sbi/sbi_system.h
> index 65ea3d36d6cf..7d0867f276aa 100644
> --- a/include/sbi/sbi_system.h
> +++ b/include/sbi/sbi_system.h
> @@ -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);
>
> diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
> index c18562cdcfab..3242b21699cb 100644
> --- a/lib/sbi/sbi_system.c
> +++ b/lib/sbi/sbi_system.c
> @@ -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 &&
> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> index bfe15f0d6b87..b3a3351c0439 100644
> --- a/platform/generic/platform.c
> +++ b/platform/generic/platform.c
> @@ -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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 00/11] SBI system suspend (SUSP) extension
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (10 preceding siblings ...)
2023-01-06 11:22 ` [RFC PATCH 11/11] platform: generic: Add system suspend test Andrew Jones
@ 2023-01-06 11:34 ` Andrew Jones
2023-01-17 3:54 ` Anup Patel
12 siblings, 0 replies; 26+ messages in thread
From: Andrew Jones @ 2023-01-06 11:34 UTC (permalink / raw)
To: opensbi
On Fri, Jan 06, 2023 at 12:21:58PM +0100, Andrew Jones wrote:
> 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
>
> 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/
>
> 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
>
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [RFC PATCH 00/11] SBI system suspend (SUSP) extension
2023-01-06 11:21 [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
` (11 preceding siblings ...)
2023-01-06 11:34 ` [RFC PATCH 00/11] SBI system suspend (SUSP) extension Andrew Jones
@ 2023-01-17 3:54 ` Anup Patel
2023-01-17 9:33 ` Andrew Jones
12 siblings, 1 reply; 26+ messages in thread
From: Anup Patel @ 2023-01-17 3:54 UTC (permalink / raw)
To: opensbi
On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> 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
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
>
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread* [RFC PATCH 00/11] SBI system suspend (SUSP) extension
2023-01-17 3:54 ` Anup Patel
@ 2023-01-17 9:33 ` Andrew Jones
0 siblings, 0 replies; 26+ messages in thread
From: Andrew Jones @ 2023-01-17 9:33 UTC (permalink / raw)
To: opensbi
On Tue, Jan 17, 2023 at 09:24:11AM +0530, Anup Patel wrote:
> On Fri, Jan 6, 2023 at 4:52 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > 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
>
> I must say that this is pretty neatly done. Great work !!!
Thank you, Anup!
>
> Could you please add a boot print for suspend device in
> sbi_boot_print_general() ?
Will do.
Thanks,
drew
>
> Regards,
> Anup
>
> >
> > 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
^ permalink raw reply [flat|nested] 26+ messages in thread