[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 02/18] xen/arm: Implement PSCI system suspend call (virtual interface)



Hi Stefano,

On 13/11/2018 01:53, Stefano Stabellini wrote:
On Mon, 12 Nov 2018, Andrew Cooper wrote:
On 12/11/18 16:35, Mirela Simonovic wrote:
Hi Julien,

Thanks for your feedback, I'll need to answer in iterations.

On Mon, Nov 12, 2018 at 4:27 PM Julien Grall <julien.grall@xxxxxxx> wrote:
Hi Mirela,

On 11/12/18 11:30 AM, Mirela Simonovic wrote:
The implementation consists of:
-Adding PSCI system suspend call as new PSCI function
-Trapping PSCI system_suspend HVC
-Implementing PSCI system suspend call (virtual interface that allows
   guests to suspend themselves)

The PSCI system suspend should be called by a guest from its boot
VCPU. Non-boot VCPUs of the guest should be hot-unplugged using PSCI
CPU_OFF call prior to issuing PSCI system suspend. Interrupts that
are left enabled by the guest are assumed to be its wake-up interrupts.
Therefore, a wake-up interrupt triggers the resume of the guest. Guest
should resume regardless of the state of Xen (suspended or not).

When a guest calls PSCI system suspend the respective domain will be
suspended if the following conditions are met:
1) Given resume entry point is not invalid
2) Other (if any) VCPUs of the calling guest are hot-unplugged

If the conditions above are met the calling domain is labeled as
suspended and the calling VCPU is blocked. If nothing else wouldn't
be done the suspended domain would resume from the place where it
called PSCI system suspend. This is expected if processing of the PSCI
system suspend call fails. However, in the case of success the calling
guest should resume (continue execution after the wake-up) from the entry
point which is given as the first argument of the PSCI system suspend
call. In addition to the entry point, the guest expects to start within
the environment whose state matches the state after reset. This means
that the guest should find reset register values, MMU disabled, etc.
Thereby, the context of VCPU should be 'reset' (as if the system is
comming out of reset), the program counter should contain entry point,
which is 1st argument, and r0/x0 should contain context ID which is 2nd
argument of PSCI system suspend call. The context of VCPU is set
accordingly when the PSCI system suspend is processed, so that nothing
needs to be done on resume/wake-up path. However, in order to ensure that
this context doesn't get overwritten by the scheduler when scheduling out
this VCPU (would normally happen after the calling CPU is blocked), we need
to check whether to return early from ctxt_switch_from().

There are variables in domain structure to keep track of domain shutdown.
One of existing shutdown reason is 'suspend' which this patch is using to
track the suspend state of a domain. Those variables are used to determine
whether to early return from ctxt_switch_from() or not.

A suspended domain will resume after the Xen receives an interrupt which is
targeted to the domain, unblocks the domain's VCPU, and schedules it in.
When the VCPU is scheduled in, the VCPU context is already reset, and
contains the right resume entry point in program counter that will be
restored in ctxt_switch_to(). The only thing that needs to be done at this
point is to clear the variables that marked the domain state as suspended.

Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>

---
Changes in v2:

-Fix print to compile for arm32 and to align with Xen coding style
---
   xen/arch/arm/Makefile            |   1 +
   xen/arch/arm/domain.c            |  13 +++
   xen/arch/arm/suspend.c           | 166 
+++++++++++++++++++++++++++++++++++++++
   xen/arch/arm/vpsci.c             |  19 +++++
   xen/include/asm-arm/perfc_defn.h |   1 +
   xen/include/asm-arm/psci.h       |   2 +
   xen/include/asm-arm/suspend.h    |  16 ++++
   xen/include/xen/sched.h          |   1 +
   8 files changed, 219 insertions(+)
   create mode 100644 xen/arch/arm/suspend.c
   create mode 100644 xen/include/asm-arm/suspend.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 23c5d9adbc..744b1a4dc8 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -43,6 +43,7 @@ obj-y += setup.o
   obj-y += shutdown.o
   obj-y += smp.o
   obj-y += smpboot.o
+obj-y += suspend.o
   obj-y += sysctl.o
   obj-y += time.o
   obj-y += traps.o
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index e594b48d81..7f8105465c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -97,6 +97,11 @@ static void ctxt_switch_from(struct vcpu *p)
       if ( is_idle_vcpu(p) )
           return;

+    /* VCPU's context should not be saved if its domain is suspended */
+    if ( p->domain->is_shut_down &&
+        (p->domain->shutdown_code == SHUTDOWN_suspend) )
+        return;
SHUTDOWN_suspend is used in Xen for other purpose (see
SCHEDOP_shutdown). The other user of that code relies on all the state
to be saved on suspend.

We just need a flag to mark a domain as suspended, and I do believe
SHUTDOWN_suspend is not used anywhere else.
Let's come back on this.

SHUTDOWN_suspend is used for migration.

That is true, but it is not only used for migration. It is also used
for suspending a guest and saving its state to file with the intention
of resuming it later from file.

Which is some sort of migration at the end. However, they don't have the same semantics as suspend/resume regarding the state of the vCPU.



Grep for it through the Xen
tree and you'll find several pieces of documentation, including the
description of what this shutdown code means.

What you are introducing here is not a shutdown - it is a suspend with
the intent to resume executing later.  As such, it shouldn't use Xen's
shutdown infrastructure, which exists mainly to communicate with the
toolstack.

Future work will need toolstack support for suspending/resuming guests.
SHUTDOWN_suspend is the most natural fit today; we don't want to hijack
domain pause, because if we do, then we can't normally pause a domain
anymore.

Why? suspend/resume is like pausing the domain but will be resumed on event (e.g interrupts) rather than user request.

From the Xen side of thing, there isn't a huge difference
between saving the state of a domain and writing it to file, or saving
the state of a domain in memory. However, I agree that there is a
difference.

If we don't want to reuse SHUTDOWN_suspend, then the only other option I
can think of is to introduce a new ARM specific suspend code for this
(and new xl commands and hypercalls in the future).

Why would you need a specific xl commands and hypercalls for it?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.