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: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.

> > > +static int fork(struct domain *d, struct domain *cd)
> > > +{
> > > +    int rc = -EBUSY;
> > > +
> > > +    if ( !cd->controller_pause_count )
> > > +        return rc;
> > > +
> > > +    /*
> > > +     * We only want to get and pause the parent once, not each time this
> > > +     * operation is restarted due to preemption.
> > > +     */
> > > +    if ( !cd->parent_paused )
> > > +    {
> > > +        if ( !get_domain(d) )
> > > +        {
> > > +            ASSERT_UNREACHABLE();
> > > +            return -EBUSY;
> > > +        }
> > > +
> > > +        domain_pause(d);
> > > +        cd->parent_paused = true;
> > > +        cd->max_pages = d->max_pages;
> > > +        cd->max_vcpus = d->max_vcpus;
> > > +    }
> > > +
> > > +    /* this is preemptible so it's the first to get done */
> > > +    if ( (rc = fork_hap_allocation(cd, d)) )
> > > +        goto done;
> > > +
> > > +    if ( (rc = bring_up_vcpus(cd, d)) )
> > > +        goto done;
> > > +
> > > +    if ( (rc = hvm_copy_context_and_params(cd, d)) )
> > > +        goto done;
> > > +
> > > +    if ( (rc = populate_special_pages(cd)) )
> > > +        goto done;
> > > +
> > > +    fork_tsc(cd, d);
> >
> > I think you need to copy the contents of the shared info page from the
> > parent into the child, or else you are discarding any pending event
> > channels. You should also map such shared info page into the same gfn
> > as the parent.
> >
> 
> I'll look into it, thanks!

Oh, and the PV timer state should also be copied over, so that PV
timer interrupts are not lost.

Thanks, Roger.

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