On 6/3/20 6:40 PM, Agarwal, Anchal wrote:
> 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;
With your explanation now making this clearer, is this check really
necessary? From what I see we are in XEN_SUSPEND mode when
lock_system_sleep() lock is taken, meaning that we can't initialize
hibernation.
> > +
> > + 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?
You should call it nevertheless. It will decide whether or not anything
needs to be done.
-boris
