Re: [Xen-devel] [PATCH v10 1/3] xen/mem_sharing: VM forking

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Feb 26, 2020 at 08:58:05AM -0700, Tamas K Lengyel wrote:
> On Wed, Feb 26, 2020 at 8:36 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> >
> > On Wed, Feb 26, 2020 at 08:20:30AM -0700, Tamas K Lengyel wrote:
> > > > > +static int populate_special_pages(struct domain *cd)
> > > > > +{
> > > > > +    struct p2m_domain *p2m = p2m_get_hostp2m(cd);
> > > > > +    static const unsigned int params[] =
> > > > > +    {
> > > > > +        HVM_PARAM_STORE_PFN,
> > > > > +        HVM_PARAM_IOREQ_PFN,
> > > > > +        HVM_PARAM_BUFIOREQ_PFN,
> > > > > +        HVM_PARAM_CONSOLE_PFN
> > > > > +    };
> > > > > +    unsigned int i;
> > > > > +
> > > > > +    for ( i=0; i<4; i++ )
> > > >
> > > > Nit: can you please add some spaces around the operators?
> > >
> > > Sure.
> > >
> > > >
> > > > > +    {
> > > > > +        uint64_t value = 0;
> > > > > +        mfn_t new_mfn;
> > > > > +        struct page_info *page;
> > > > > +
> > > > > +        if ( hvm_get_param(cd, params[i], &value) || !value )
> > > > > +            continue;
> > > > > +
> > > > > +        if ( !(page = alloc_domheap_page(cd, 0)) )
> > > > > +            return -ENOMEM;
> > > > > +
> > > > > +        new_mfn = page_to_mfn(page);
> > > > > +        set_gpfn_from_mfn(mfn_x(new_mfn), value);
> > > > > +
> > > > > +        return p2m->set_entry(p2m, _gfn(value), new_mfn, 
> > > > > PAGE_ORDER_4K,
> > > > > +                              p2m_ram_rw, p2m->default_access, -1);
> > > >
> > > > I think you also need to copy the contents from the parent page here.
> > >
> > > The toolstack simply clears these pages during restore so I'm not sure
> > > (see 
> > > https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxc/xc_sr_restore_x86_hvm.c;h=3f78248f32fec239db77b0e483b0195211e6b974;hb=HEAD#l61).
> > > I don't see why you would have to clear the pages first if they get
> > > overwritten by saved versions later. Or these pages are expected to be
> > > torn-down by the save/restore aware guests?
> >
> > Guests using those pages know they are torn down during suspend/resume
> > and expect to find a clean state when resuming. That's not the case with
> > forking however, as the guest is completely unaware of the fork
> > happening.
> >
> > One thing I'm not sure of is whether the backends (xenstored,
> > xenconsoled) will cope with those pages being already populated on
> > guest creation.
> >
> > AFAICT another issue is that xenstore watches are not copied over from
> > the parent, so any watches the parent might have set will not fire on
> > the child. That would require some kind of interaction with xenstored
> > in order to request a guest state to be copied over to another guest.
> 
> Sounds like it most likely would need to be handled if the guest uses
> them. I'm not sure if a default Linux HVM guest uses them though.

Linux PVHVM does use xenstore watches to monitor backend state
changes, but it would require a non-trivial amount of work to clone
the state of PV devices, so I guess it's something to be left as a
TODO item.

> A
> Windows HVM guest without the PV drivers is certainly not Xen aware so
> there things already work just fine and that is our primary target for
> our use-case. PVHVM/PVH Linux guests are not. So that's really outside
> the scope of what I can contribute at the moment.

Sure.

Can you please add a TODO item here to note that the contents of those
special pages likely need to be copied over, and that the state of PV
devices and interfaces that rely on backends running in userspace is
not handled at all.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel