[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 05/18] xen/arm: Trigger Xen suspend when Dom0 completes suspend
Hi,
On 14/11/2018 17:35, Mirela Simonovic wrote:
On Wed, Nov 14, 2018 at 6:10 PM Julien Grall <julien.grall@xxxxxxx> wrote:
On 14/11/2018 15:40, Mirela Simonovic wrote:
On Wed, Nov 14, 2018 at 4:07 PM Julien Grall <julien.grall@xxxxxxx> wrote:
On 12/11/2018 11:30, Mirela Simonovic wrote:
When Dom0 finalizes its suspend procedure the suspend of Xen is
triggered by calling system_suspend(). Dom0 finalizes the suspend from
its boot core (VCPU#0), which could be mapped to any physical CPU,
i.e. the system_suspend() function could be executed by any physical
CPU. Since Xen suspend procedure has to be run by the boot CPU
(non-boot CPUs will be disabled at some point in suspend procedure),
system_suspend() execution has to continue on CPU#0.
When the system_suspend() returns 0, it means that the system was
suspended and it is coming out of the resume procedure. Regardless
of the system_suspend() return value, after this function returns
Xen is fully functional, and its state, including all devices and data
structures, matches the state prior to calling system_suspend().
The status is returned by system_suspend() for debugging/logging
purposes and function prototype compatibility.
Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>
---
xen/arch/arm/suspend.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
index f2338e41db..21b45f8248 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -112,11 +112,20 @@ static void vcpu_suspend(register_t epoint, register_t
cid)
_arch_set_info_guest(v, &ctxt);
}
+/* Xen suspend. Note: data is not used (suspend is the suspend to RAM) */
+static long system_suspend(void *data)
+{
+ BUG_ON(system_state != SYS_STATE_active);
+
+ return -ENOSYS;
+}
+
int32_t domain_suspend(register_t epoint, register_t cid)
{
struct vcpu *v;
struct domain *d = current->domain;
bool is_thumb = epoint & 1;
+ int status;
dprintk(XENLOG_DEBUG,
"Dom%d suspend: epoint=0x%"PRIregister",
cid=0x%"PRIregister"\n",
@@ -156,6 +165,31 @@ int32_t domain_suspend(register_t epoint, register_t cid)
*/
vcpu_block_unless_event_pending(current);
+ /* If this was dom0 the whole system should suspend: trigger Xen suspend */
+ if ( is_hardware_domain(d) )
+ {
+ /*
+ * system_suspend should be called when Dom0 finalizes the suspend
+ * procedure from its boot core (VCPU#0). However, Dom0's VCPU#0 could
+ * be mapped to any PCPU (this function could be executed by any PCPU).
+ * The suspend procedure has to be finalized by the PCPU#0 (non-boot
+ * PCPUs will be disabled during the suspend).
+ */
+ status = continue_hypercall_on_cpu(0, system_suspend, NULL);
Based on my comment in patch #2, I don't think this will do the correct thing on
Dom0. x0 will not contain cid but PSCI_SUCCESS has it is overriden in
do_vpsci_0_2_call.
Could you please explain? I can't follow
General purpose (e.g xN, pc) registers live at the bottom of the vCPU stack. The
function vcpu_suspend will reset all of them to 0 but x0 (Context ID) and pc
(entry point).
You rely on those registers to be untouched in the return path. However, this is
not the case. If you look at do_vpsci_0_2_call, x0 will be set to whatever is
the return value of domain_suspend (e.g PSCI_SUSPEND). So x0 will not contain
anymore the Context ID as expected by the guest.
This is expected, the system should behave that way. If the guest
managed to suspend, i.e. domain_suspend has returned PSCI_SUCCESS, the
return value is meaningless to the guest because it won't return to
it. The guest has suspended, just the fact the it will start from an
another entry point implicitly carries the information that the
suspend was successful.
However, if the return value from domain_suspend is an error then the
error will be returned to the guest because the suspend has failed.
Then the x0 register should contain error code, not the context ID.
The PC should be untouched, i.e. it should not contain the resume
entry point, but whatever was in there once the hvc/system_suspend was
issued by the guest. Guests handle errors right below the code from
which they tried to suspend.
I think you misunderstood my comment. I am not speaking about the failure case
but the success case where the domain is actually suspended for some time.
When the domain is resuming, the "boot" vCPU should see Context ID and not
PSCI_SUCCESS.
However, this is not the case because of the following path:
-> do_vpsci_0_2_call
-> domain_suspend
-> vcpu_suspend
-> x0 = Context ID
-> return PSCI_SUCCESS
-> x0 = PSCI_SUCCESS
At some point in the future your guest will resume. Instead of seen Context ID,
it will actually see PSCI_SUCCESS. You can easily test that by modifying the
SYSTEM_SUSPEND code in Linux to use a different context ID and add hvc #0xffe0
to dump register x0.
You probably haven't noticed it because Linux is currently using 0 for the
context ID. This is the same value as PSCI_SUCCESS.
In the case of Dom0, this is a bit different to what I explained in my previous
e-mail because I got confused. The function continue_hypercall_on_cpu is not
doing what you expect. The function will pause the vCPU, schedule the tasklet
and then return directly.
At some point the tasklet will get scheduled on CPU#0 and system_suspend will be
called. The return value of that function will be copied to x0. The vCPU will
then get unpaused and continue to run.
So x0 will not contain the Context ID but whatever system_suspend return.
I agree with all you described above, but that is intended - the
system should behave that way. I believe the cause of misunderstanding
could be in how the return value versus context ID is handled. Those
are different paths, and only one of the following executes: 1) either
the suspend is successful and nothing will be returned to the guest
because it is suspended; or 2) the suspend of the guest has failed so
context ID is useless.
You missed my point here, your guest will resume at some point. As you reset the
vCPU and set x0 in vcpu_suspend. Then anything after can overwrite the registers
you set in vcpu_suspend.
Please explain why you think your code behave differently that I wrote when
resuming.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
