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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e2073aa4-2410-4630-fee6-4e4abc172876@oracle.com>
Date:   Fri, 5 Jun 2020 17:24:37 -0400
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     "Agarwal, Anchal" <anchalag@...zon.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>, "hpa@...or.com" <hpa@...or.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "jgross@...e.com" <jgross@...e.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "Kamata, Munehisa" <kamatam@...zon.com>,
        "sstabellini@...nel.org" <sstabellini@...nel.org>,
        "konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
        "roger.pau@...rix.com" <roger.pau@...rix.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>,
        "len.brown@...el.com" <len.brown@...el.com>,
        "pavel@....cz" <pavel@....cz>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "Valentin, Eduardo" <eduval@...zon.com>,
        "Singh, Balbir" <sblbir@...zon.com>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Woodhouse, David" <dwmw@...zon.co.uk>,
        "benh@...nel.crashing.org" <benh@...nel.crashing.org>
Subject: Re: [PATCH 04/12] x86/xen: add system core suspend and resume
 callbacks

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@...zon.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