lkml.org 
[lkml]   [2020]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 03/12] x86/xen: Introduce new function to map HYPERVISOR_shared_info on Resume
    Date
    From
    On Fri, Jun 05, 2020 at 05:39:54PM -0400, Boris Ostrovsky 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 6/4/20 7:03 PM, Anchal Agarwal wrote:
    > > On Sat, May 30, 2020 at 07:02:01PM -0400, Boris Ostrovsky 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:25 PM, Anchal Agarwal wrote:
    > >>> Introduce a small function which re-uses shared page's PA allocated
    > >>> during guest initialization time in reserve_shared_info() and not
    > >>> allocate new page during resume flow.
    > >>> It also does the mapping of shared_info_page by calling
    > >>> xen_hvm_init_shared_info() to use the function.
    > >>>
    > >>> Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
    > >>> ---
    > >>> arch/x86/xen/enlighten_hvm.c | 7 +++++++
    > >>> arch/x86/xen/xen-ops.h | 1 +
    > >>> 2 files changed, 8 insertions(+)
    > >>>
    > >>> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
    > >>> index e138f7de52d2..75b1ec7a0fcd 100644
    > >>> --- a/arch/x86/xen/enlighten_hvm.c
    > >>> +++ b/arch/x86/xen/enlighten_hvm.c
    > >>> @@ -27,6 +27,13 @@
    > >>>
    > >>> static unsigned long shared_info_pfn;
    > >>>
    > >>> +void xen_hvm_map_shared_info(void)
    > >>> +{
    > >>> + xen_hvm_init_shared_info();
    > >>> + if (shared_info_pfn)
    > >>> + HYPERVISOR_shared_info = __va(PFN_PHYS(shared_info_pfn));
    > >>> +}
    > >>> +
    > >>
    > >> AFAICT it is only called once so I don't see a need for new routine.
    > >>
    > >>
    > > HYPERVISOR_shared_info can only be mapped in this scope without refactoring
    > > much of the code.
    >
    >
    > Refactoring what? All am suggesting is
    >
    shared_info_pfn does not seem to be in scope here, it's scope is limited
    to enlighten_hvm.c. That's the reason I introduced a new function there.

    > --- a/arch/x86/xen/suspend.c
    > +++ b/arch/x86/xen/suspend.c
    > @@ -124,7 +124,9 @@ static void xen_syscore_resume(void)
    > return;
    >
    > /* No need to setup vcpu_info as it's already moved off */
    > - xen_hvm_map_shared_info();
    > + xen_hvm_init_shared_info();
    > + if (shared_info_pfn)
    > + HYPERVISOR_shared_info = __va(PFN_PHYS(shared_info_pfn));
    >
    > pvclock_resume();
    >
    > >> And is it possible for shared_info_pfn to be NULL in resume path (which
    > >> is where this is called)?
    > >>
    > >>
    > > I don't think it should be, still a sanity check but I don't think its needed there
    > > because hibernation will fail in any case if thats the case.
    >
    >
    > If shared_info_pfn is NULL you'd have problems long before hibernation
    > started. We set it in xen_hvm_guest_init() and never touch again.
    >
    >
    > In fact, I'd argue that it should be __ro_after_init.
    >
    >
    I agree, and I should have mentioned that I will remove that check and its not
    necessary as this gets mapped way early in the boot process.
    > > However, HYPERVISOR_shared_info does needs to be re-mapped on resume as its been
    > > marked to dummy address on suspend. Its also safe in case va changes.
    > > Does the answer your question?
    >
    >
    > I wasn't arguing whether HYPERVISOR_shared_info needs to be set, I was
    > only saying that shared_info_pfn doesn't need to be tested.
    >
    Got it. :)
    >
    > -boris
    >
    Thanks,
    Anchal
    >

    \
     
     \ /
      Last update: 2020-06-08 18:53    [W:4.652 / U:0.576 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site