Re: [PATCH 04/12] x86/xen: add system core suspend and resume callbacks

From: Agarwal, Anchal
Date: Wed Jun 03 2020 - 18:40:25 EST


CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



On 5/19/20 7:26 PM, Anchal Agarwal wrote:
> From: Munehisa Kamata <kamatam@xxxxxxxxxx>
>
> Add Xen PVHVM specific system core callbacks for PM suspend and
> hibernation support. The callbacks suspend and resume Xen
> primitives,like shared_info, pvclock and grant table. Note that
> Xen suspend can handle them in a different manner, but system
> core callbacks are called from the context.


I don't think I understand that last sentence.

Looks like it may have cryptic meaning of stating that xen_suspend calls syscore_suspend from xen_suspend
So, if these syscore ops gets called during xen_suspend do not do anything. Check if the mode is in xen suspend
and return from there. These syscore_ops are specifically for domU hibernation.
I must admit, I may have overlooked lack of explanation of some implicit details in the original commit msg.

> So if the callbacks
> are called from Xen suspend context, return immediately.
>


> +
> +static int xen_syscore_suspend(void)
> +{
> + struct xen_remove_from_physmap xrfp;
> + int ret;
> +
> + /* Xen suspend does similar stuffs in its own logic */
> + if (xen_suspend_mode_is_xen_suspend())
> + return 0;
> +
> + xrfp.domid = DOMID_SELF;
> + xrfp.gpfn = __pa(HYPERVISOR_shared_info) >> PAGE_SHIFT;
> +
> + ret = HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrfp);
> + if (!ret)
> + HYPERVISOR_shared_info = &xen_dummy_shared_info;
> +
> + return ret;
> +}
> +
> +static void xen_syscore_resume(void)
> +{
> + /* Xen suspend does similar stuffs in its own logic */
> + if (xen_suspend_mode_is_xen_suspend())
> + return;
> +
> + /* No need to setup vcpu_info as it's already moved off */
> + xen_hvm_map_shared_info();
> +
> + pvclock_resume();
> +
> + gnttab_resume();


Do you call gnttab_suspend() in pm suspend path?
No, since it does nothing for HVM guests. The unmap_frames is only applicable for PV guests right?

> +}
> +
> +/*
> + * These callbacks will be called with interrupts disabled and when having only
> + * one CPU online.
> + */
> +static struct syscore_ops xen_hvm_syscore_ops = {
> + .suspend = xen_syscore_suspend,
> + .resume = xen_syscore_resume
> +};
> +
> +void __init xen_setup_syscore_ops(void)
> +{
> + if (xen_hvm_domain())


Have you tested this (the whole feature, not just this patch) with PVH
guest BTW? And PVH dom0 for that matter?

No I haven't. The whole series is just tested with hvm/pvhvm guests.

-boris
Thanks,
Anchal

> + register_syscore_ops(&xen_hvm_syscore_ops);
> +}