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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Feb 26, 2020 at 9:10 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>
> 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.
>

Of course. I will finish up the PV timer parts and copying the shared
info page, but will leave the rest as TODO. Hopefully this will get at
least a default Linux HVM fork working the same way as Windows does,
but if not then I'll unfortunately have to leave it as-is for now.

Tamas

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