lkml.org 
[lkml]   [2020]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 04/12] x86/xen: add system core suspend and resume callbacks
    From
    Date
    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@amazon.com>
    > >
    > > 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


    \
     
     \ /
      Last update: 2020-06-05 23:26    [W:3.365 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site