[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)
- To: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
- From: Julien Grall <julien.grall@xxxxxxx>
- Date: Thu, 15 Nov 2018 20:25:01 +0000
- Cc: Tim Deegan <tim@xxxxxxx>, Xen Devel <xen-devel@xxxxxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, Davorin Mista <dm@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andre Przywara <andre.przywara@xxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
- Delivery-date: Thu, 15 Nov 2018 20:25:14 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
Hi Mirela,
On 11/15/18 11:42 AM, Mirela Simonovic wrote:
Hi Julien,
On Thu, Nov 15, 2018 at 12:38 PM Julien Grall <julien.grall@xxxxxxx> wrote:
Hi,
On 11/15/18 11:10 AM, Mirela Simonovic wrote:
Hi Julien,
On Thu, Nov 15, 2018 at 11:59 AM Julien Grall <julien.grall@xxxxxxx> wrote:
Hi Mirela,
On 11/15/18 10:33 AM, Mirela Simonovic wrote:
On Thu, Nov 15, 2018 at 11:26 AM Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
On 15/11/2018 10:13, Julien Grall wrote:
(+ Andre)
On 11/15/18 12:47 AM, Andrew Cooper wrote:
On 14/11/2018 12:49, Julien Grall wrote:
Hi Mirela,
On 14/11/2018 12:08, Mirela Simonovic wrote:
On 11/13/2018 09:32 AM, Andrew Cooper wrote:
On 12/11/2018 19:56, Julien Grall wrote:
Hi Andrew,
On 11/12/18 4:41 PM, Andrew Cooper wrote:
On 12/11/18 16:35, Mirela Simonovic wrote:
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. 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.
Would domain pause/unpause be a better solution?
Actually yes - that sounds like a very neat solution.
I believe domain pause will not work here - a domain cannot pause
itself, i.e. current domain cannot be paused. This functionality
seems to assume that a domain is pausing another domain. Or I missed
the point.
Yes domain pause/unpause will not work. However, you can introduce a
boolean to tell you whether the domain was suspend.
I actually quite like how suspend work for x86 HVM. This is based on
pause/unpause. Have a look at hvm_s3_{suspend/resume}.
That only exists because the ACPI controller is/was implemented in
QEMU. I wouldn't recommend copying it.
May I ask why? I don't think the properties are very different from
Arm here.
If you observe, that can only be actioned by a hypercall from qemu. It
can't be actioned by an ACPI controller emulated by Xen.
Having spent some more time thinking about this problem, what properties
does the PSCI call have?
I gather from other parts of this thread that there may be a partial
reset of state? Beyond that, what else?
I ask, because conceptually, domU suspend to RAM is literally just
"de-schedule until we want to wake up", and in this case, it appears to
be "wake up on any of the still-active interrupts". We've already got a
scheduler API for that, and its called vcpu_block(). This already
exists with "wait until a new event occurs" semantics.
Is there anything else I've missed?
That's correct, and I agree. BTW that is what's implemented in this series.
All vCPUs but the vCPU calling SYSTEM_SUSPEND should be off. Also,
only events on that vCPU can trigger a resume. All the other event
should not be taken into account.
What other events are talking about here?
vcpu_unblock is not only called when you receive interrupts. It can be
called in other place when you receive an events.
From the Arm Arm, an event can be anything. So do we really want to
wake-up on any events?
My worry with vcpu_block() is we don't prevent the other vCPUs to run.
Technically they should be off, but I would like some safety to avoid
any potential corner case (i.e other way to turn a vCPU on).
Other vCPUs are hot-unplugged (offlined) by the guest. If that is not
the case, SYSTEM_SUSPEND will return an error.
Could you please clarify what a potential corner case would be?
PSCI CPU_ON is not the only way to online a vCPU. I merely want to
prevent other path to play with the vCPU when it is not necessary.
[...]
If instead of waiting for any event, you need to wait for a specific
event, there is also vcpu_poll() which is a related scheduler API.
~Andrew
Some content disappeared, so I'll write here to avoid thread branching.
The semantic of vCPU block and domain_pause is not the same. When
guest suspends the domain is not (and should not be) paused, instead
its last online vCPU is blocked waiting on an interrupt. That's it.
Well no, you will block until you receive an event. Interrupts are one
of them.
Do we want to consider all events as wakeup event?
I think we need to assume that events are not triggered via toolstack,
Andrew made a good point.
I don't think we are discussing the same thing. The discussion was
around other vCPUs, not the vCPU calling SYSTEM_SUSPEND.
Most likely in the future, we would want to allow the toolstack to
request resuming the domain. This can be considered as an event.
Yes, such an event will unblock the vcpu and cause the domain to
resume. So from this perspective it's not only an interrupt targeted
to the guest.
Given the assumption, my understanding is that Xen itself will not
unblock vCPU, except due to an interrupt targeted to the guest.
Am I missing something? An example would be appreciated.
At least on Arm, the current semantics of vcpu_block/vcpu_unblock is to
block until you receive an events.
I don't much want to restrict the definition of events to only
interrupts. To clarify my point, if you want to wake-up for any events
then fine. But this needs to be understood that it may not be only
interrupts.
In the context described above this is fine - events are not only interrupts.
So I guess we have a way forward here. This could be implemented with
vcpu_block(). This would need to be done in combination with as tasklet
as discussed in patch #5.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
