Re: [Xen-devel] [PATCH 13/18] xen/arm: Implement PSCI SYSTEM_SUSPEND call (physical interface)
Hi,
On 11/12/18 11:30 AM, Mirela Simonovic wrote:
PSCI system suspend function shall be invoked to finalize Xen suspend
procedure. Resume entry point, which needs to be passed via 1st argument
of PSCI system suspend call to the EL3, is hyp_resume. For now, hyp_resume
is just a placeholder that will be implemented in assembly. Context ID,
which is 2nd argument of system suspend PSCI call, is unused, as in Linux.
Signed-off-by: Mirela Simonovic
Signed-off-by: Saeed Nowshadi
---
Changes in v2:
-The commit message was stale - referring to the do_suspend function
that has been renamed long time ago. Fixed commit message
---
xen/arch/arm/arm64/entry.S| 3 +++
xen/arch/arm/psci.c | 16
xen/arch/arm/suspend.c| 4
xen/include/asm-arm/psci.h| 1 +
xen/include/asm-arm/suspend.h | 1 +
5 files changed, 25 insertions(+)
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 97b05f53ea..dbc4717903 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -533,6 +533,9 @@ ENTRY(__context_switch)
mov sp, x9
ret
+ENTRY(hyp_resume)
+b .
+
/*
* Local variables:
* mode: ASM
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index a93121f43b..b100bd8ad2 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
/*
* While a 64-bit OS can make calls with SMC32 calling conventions, for
@@ -67,6 +68,21 @@ void call_psci_cpu_off(void)
}
}
+int call_psci_system_suspend(void)
+{
SYSTEM_SUSPEND was introduced by PSCI 1.0 and optional. So you need to
check the PSCI version and use PSCI_FEATURES to check if it was implemented.
+#ifdef CONFIG_ARM_64
+struct arm_smccc_res res;
+
+/* 2nd argument (context ID) is not used */
It still needs to be defined to some known values rather than whatever
is in x2 at that time.
But I would suggest to make good use of it to catch implementation not
doing the right thing. We could define it to 0xdeadbeef and shout at
anyone not preserving the value.
+arm_smccc_smc(PSCI_1_0_FN64_SYSTEM_SUSPEND, __pa(hyp_resume), );
+
+return PSCI_RET(res);
+#else
+/* not supported */
+return 1;
+#endif
+}
+
void call_psci_system_off(void)
{
if ( psci_ver > PSCI_VERSION(0, 1) )
diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
index d1b48c339a..37926374bc 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -141,6 +141,10 @@ static long system_suspend(void *data)
goto resume_irqs;
}
+status = call_psci_system_suspend();
Some platform may not support PSCI at all. So this need to be check.
+if ( status )
+dprintk(XENLOG_ERR, "PSCI system suspend failed, err=%d\n", status);
+
system_state = SYS_STATE_resume;
gic_resume();
diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
index 26462d0c47..9f6116a224 100644
--- a/xen/include/asm-arm/psci.h
+++ b/xen/include/asm-arm/psci.h
@@ -20,6 +20,7 @@ extern uint32_t psci_ver;
int psci_init(void);
int call_psci_cpu_on(int cpu);
+int call_psci_system_suspend(void);
void call_psci_cpu_off(void);
void call_psci_system_off(void);
void call_psci_system_reset(void);
diff --git a/xen/include/asm-arm/suspend.h b/xen/include/asm-arm/suspend.h
index de787d296a..7604e2e2e2 100644
--- a/xen/include/asm-arm/suspend.h
+++ b/xen/include/asm-arm/suspend.h
@@ -2,6 +2,7 @@
#define __ASM_ARM_SUSPEND_H__
int32_t domain_suspend(register_t epoint, register_t cid);
+void hyp_resume(void);
#endif
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 13/18] xen/arm: Implement PSCI SYSTEM_SUSPEND call (physical interface)
On Wed, Nov 14, 2018 at 1:14 AM Stefano Stabellini
wrote:
>
> On Mon, 12 Nov 2018, Mirela Simonovic wrote:
> > PSCI system suspend function shall be invoked to finalize Xen suspend
> > procedure. Resume entry point, which needs to be passed via 1st argument
> > of PSCI system suspend call to the EL3, is hyp_resume. For now, hyp_resume
> > is just a placeholder that will be implemented in assembly. Context ID,
> > which is 2nd argument of system suspend PSCI call, is unused, as in Linux.
> >
> > Signed-off-by: Mirela Simonovic
> > Signed-off-by: Saeed Nowshadi
> >
> > ---
> > Changes in v2:
> >
> > -The commit message was stale - referring to the do_suspend function
> > that has been renamed long time ago. Fixed commit message
> > ---
> > xen/arch/arm/arm64/entry.S| 3 +++
> > xen/arch/arm/psci.c | 16
> > xen/arch/arm/suspend.c| 4
> > xen/include/asm-arm/psci.h| 1 +
> > xen/include/asm-arm/suspend.h | 1 +
> > 5 files changed, 25 insertions(+)
> >
> > diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> > index 97b05f53ea..dbc4717903 100644
> > --- a/xen/arch/arm/arm64/entry.S
> > +++ b/xen/arch/arm/arm64/entry.S
> > @@ -533,6 +533,9 @@ ENTRY(__context_switch)
> > mov sp, x9
> > ret
> >
> > +ENTRY(hyp_resume)
> > +b .
> > +
> > /*
> > * Local variables:
> > * mode: ASM
> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> > index a93121f43b..b100bd8ad2 100644
> > --- a/xen/arch/arm/psci.c
> > +++ b/xen/arch/arm/psci.c
> > @@ -24,6 +24,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > /*
> > * While a 64-bit OS can make calls with SMC32 calling conventions, for
> > @@ -67,6 +68,21 @@ void call_psci_cpu_off(void)
> > }
> > }
> >
> > +int call_psci_system_suspend(void)
> > +{
> > +#ifdef CONFIG_ARM_64
> > +struct arm_smccc_res res;
> > +
> > +/* 2nd argument (context ID) is not used */
> > +arm_smccc_smc(PSCI_1_0_FN64_SYSTEM_SUSPEND, __pa(hyp_resume), );
> > +
> > +return PSCI_RET(res);
> > +#else
> > +/* not supported */
> > +return 1;
> > +#endif
> > +}
>
> Given that suspend is unimplemented on arm32, the #ifdef is OK. But
> in that case return PSCI_NOT_SUPPORTED for arm32.
>
> Otherwise you should be able to remove this #ifdef by introducing
> something similar to PSCI_0_2_FN_NATIVE, but for PSCI_1_0 calls.
>
>
> > void call_psci_system_off(void)
> > {
> > if ( psci_ver > PSCI_VERSION(0, 1) )
> > diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
> > index d1b48c339a..37926374bc 100644
> > --- a/xen/arch/arm/suspend.c
> > +++ b/xen/arch/arm/suspend.c
> > @@ -141,6 +141,10 @@ static long system_suspend(void *data)
> > goto resume_irqs;
> > }
> >
> > +status = call_psci_system_suspend();
> > +if ( status )
> > +dprintk(XENLOG_ERR, "PSCI system suspend failed, err=%d\n",
> > status);
> > +
> > system_state = SYS_STATE_resume;
> >
> > gic_resume();
> > diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
> > index 26462d0c47..9f6116a224 100644
> > --- a/xen/include/asm-arm/psci.h
> > +++ b/xen/include/asm-arm/psci.h
> > @@ -20,6 +20,7 @@ extern uint32_t psci_ver;
> >
> > int psci_init(void);
> > int call_psci_cpu_on(int cpu);
> > +int call_psci_system_suspend(void);
> > void call_psci_cpu_off(void);
> > void call_psci_system_off(void);
> > void call_psci_system_reset(void);
> > diff --git a/xen/include/asm-arm/suspend.h b/xen/include/asm-arm/suspend.h
> > index de787d296a..7604e2e2e2 100644
> > --- a/xen/include/asm-arm/suspend.h
> > +++ b/xen/include/asm-arm/suspend.h
> > @@ -2,6 +2,7 @@
> > #define __ASM_ARM_SUSPEND_H__
> >
> > int32_t domain_suspend(register_t epoint, register_t cid);
> > +void hyp_resume(void);
>
> I think it would be better to separate physical suspend from virtual
> suspend, like Julien suggested. As you separate the C implementation,
> please also introduce separate header files (for instance vsuspend.h and
> suspend.h).
>
AFAIU Julien came back with Andrew's feedback suggesting that this
should rather not be done. Please see discussion "[PATCH 02/18]
xen/arm: Implement PSCI system suspend call (virtual interface)"
>
> > #endif
> >
> > --
> > 2.13.0
> >
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 13/18] xen/arm: Implement PSCI SYSTEM_SUSPEND call (physical interface)
On Mon, 12 Nov 2018, Mirela Simonovic wrote:
> PSCI system suspend function shall be invoked to finalize Xen suspend
> procedure. Resume entry point, which needs to be passed via 1st argument
> of PSCI system suspend call to the EL3, is hyp_resume. For now, hyp_resume
> is just a placeholder that will be implemented in assembly. Context ID,
> which is 2nd argument of system suspend PSCI call, is unused, as in Linux.
>
> Signed-off-by: Mirela Simonovic
> Signed-off-by: Saeed Nowshadi
>
> ---
> Changes in v2:
>
> -The commit message was stale - referring to the do_suspend function
> that has been renamed long time ago. Fixed commit message
> ---
> xen/arch/arm/arm64/entry.S| 3 +++
> xen/arch/arm/psci.c | 16
> xen/arch/arm/suspend.c| 4
> xen/include/asm-arm/psci.h| 1 +
> xen/include/asm-arm/suspend.h | 1 +
> 5 files changed, 25 insertions(+)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 97b05f53ea..dbc4717903 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -533,6 +533,9 @@ ENTRY(__context_switch)
> mov sp, x9
> ret
>
> +ENTRY(hyp_resume)
> +b .
> +
> /*
> * Local variables:
> * mode: ASM
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index a93121f43b..b100bd8ad2 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * While a 64-bit OS can make calls with SMC32 calling conventions, for
> @@ -67,6 +68,21 @@ void call_psci_cpu_off(void)
> }
> }
>
> +int call_psci_system_suspend(void)
> +{
> +#ifdef CONFIG_ARM_64
> +struct arm_smccc_res res;
> +
> +/* 2nd argument (context ID) is not used */
> +arm_smccc_smc(PSCI_1_0_FN64_SYSTEM_SUSPEND, __pa(hyp_resume), );
> +
> +return PSCI_RET(res);
> +#else
> +/* not supported */
> +return 1;
> +#endif
> +}
Given that suspend is unimplemented on arm32, the #ifdef is OK. But
in that case return PSCI_NOT_SUPPORTED for arm32.
Otherwise you should be able to remove this #ifdef by introducing
something similar to PSCI_0_2_FN_NATIVE, but for PSCI_1_0 calls.
> void call_psci_system_off(void)
> {
> if ( psci_ver > PSCI_VERSION(0, 1) )
> diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
> index d1b48c339a..37926374bc 100644
> --- a/xen/arch/arm/suspend.c
> +++ b/xen/arch/arm/suspend.c
> @@ -141,6 +141,10 @@ static long system_suspend(void *data)
> goto resume_irqs;
> }
>
> +status = call_psci_system_suspend();
> +if ( status )
> +dprintk(XENLOG_ERR, "PSCI system suspend failed, err=%d\n", status);
> +
> system_state = SYS_STATE_resume;
>
> gic_resume();
> diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
> index 26462d0c47..9f6116a224 100644
> --- a/xen/include/asm-arm/psci.h
> +++ b/xen/include/asm-arm/psci.h
> @@ -20,6 +20,7 @@ extern uint32_t psci_ver;
>
> int psci_init(void);
> int call_psci_cpu_on(int cpu);
> +int call_psci_system_suspend(void);
> void call_psci_cpu_off(void);
> void call_psci_system_off(void);
> void call_psci_system_reset(void);
> diff --git a/xen/include/asm-arm/suspend.h b/xen/include/asm-arm/suspend.h
> index de787d296a..7604e2e2e2 100644
> --- a/xen/include/asm-arm/suspend.h
> +++ b/xen/include/asm-arm/suspend.h
> @@ -2,6 +2,7 @@
> #define __ASM_ARM_SUSPEND_H__
>
> int32_t domain_suspend(register_t epoint, register_t cid);
> +void hyp_resume(void);
I think it would be better to separate physical suspend from virtual
suspend, like Julien suggested. As you separate the C implementation,
please also introduce separate header files (for instance vsuspend.h and
suspend.h).
> #endif
>
> --
> 2.13.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 13/18] xen/arm: Implement PSCI SYSTEM_SUSPEND call (physical interface)
PSCI system suspend function shall be invoked to finalize Xen suspend
procedure. Resume entry point, which needs to be passed via 1st argument
of PSCI system suspend call to the EL3, is hyp_resume. For now, hyp_resume
is just a placeholder that will be implemented in assembly. Context ID,
which is 2nd argument of system suspend PSCI call, is unused, as in Linux.
Signed-off-by: Mirela Simonovic
Signed-off-by: Saeed Nowshadi
---
Changes in v2:
-The commit message was stale - referring to the do_suspend function
that has been renamed long time ago. Fixed commit message
---
xen/arch/arm/arm64/entry.S| 3 +++
xen/arch/arm/psci.c | 16
xen/arch/arm/suspend.c| 4
xen/include/asm-arm/psci.h| 1 +
xen/include/asm-arm/suspend.h | 1 +
5 files changed, 25 insertions(+)
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 97b05f53ea..dbc4717903 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -533,6 +533,9 @@ ENTRY(__context_switch)
mov sp, x9
ret
+ENTRY(hyp_resume)
+b .
+
/*
* Local variables:
* mode: ASM
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index a93121f43b..b100bd8ad2 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
/*
* While a 64-bit OS can make calls with SMC32 calling conventions, for
@@ -67,6 +68,21 @@ void call_psci_cpu_off(void)
}
}
+int call_psci_system_suspend(void)
+{
+#ifdef CONFIG_ARM_64
+struct arm_smccc_res res;
+
+/* 2nd argument (context ID) is not used */
+arm_smccc_smc(PSCI_1_0_FN64_SYSTEM_SUSPEND, __pa(hyp_resume), );
+
+return PSCI_RET(res);
+#else
+/* not supported */
+return 1;
+#endif
+}
+
void call_psci_system_off(void)
{
if ( psci_ver > PSCI_VERSION(0, 1) )
diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
index d1b48c339a..37926374bc 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -141,6 +141,10 @@ static long system_suspend(void *data)
goto resume_irqs;
}
+status = call_psci_system_suspend();
+if ( status )
+dprintk(XENLOG_ERR, "PSCI system suspend failed, err=%d\n", status);
+
system_state = SYS_STATE_resume;
gic_resume();
diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
index 26462d0c47..9f6116a224 100644
--- a/xen/include/asm-arm/psci.h
+++ b/xen/include/asm-arm/psci.h
@@ -20,6 +20,7 @@ extern uint32_t psci_ver;
int psci_init(void);
int call_psci_cpu_on(int cpu);
+int call_psci_system_suspend(void);
void call_psci_cpu_off(void);
void call_psci_system_off(void);
void call_psci_system_reset(void);
diff --git a/xen/include/asm-arm/suspend.h b/xen/include/asm-arm/suspend.h
index de787d296a..7604e2e2e2 100644
--- a/xen/include/asm-arm/suspend.h
+++ b/xen/include/asm-arm/suspend.h
@@ -2,6 +2,7 @@
#define __ASM_ARM_SUSPEND_H__
int32_t domain_suspend(register_t epoint, register_t cid);
+void hyp_resume(void);
#endif
--
2.13.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
