Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Tim Deegan <tim@xxxxxxx>]

At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
> Tim Deegan <tim@xxxxxxx> writes:
> >> +    last_gmfn = domain_get_maximum_gpfn(source_d);
> >> +    gmfn = *gmfn_start;
> >> +    while ( gmfn <= last_gmfn )
> >> +    {
> >> +        page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
> >
> > In order to be safe against p2m changes, you should use
> > get_gfn_type_access() _here_, and put_gfn() when you're finished with the
> > gfn.  You'll also need to get_page() the returned MFN, of course, to
> > make sure that it isn't freed before you reassign it.
> 
> The only problem I see is the fact that get_gfn_type_access() is
> x86-specific. Actually, I don't see why we can't have
> get_gfn_type_access() for ARM: it locks the whole p2m with gfn_lock
> (which is just p2m_lock() on x86) and then resolves the gfn. I'm not
> sure what should be the right approach for this series: make this
> hypercall x86-specific (temporary before we have all the required bits
> in ARM) or try to make something up for ARM...

I think it would be best to add this call to ARM.

> >> +        while ( next_page && !is_xen_heap_page(next_page) &&
> >> +                page_to_mfn(next_page) == mfn + count )
> >
> > What's the purpose of this second loop?  It doesn't seem to be doing
> > anything that the outer loop couldn't.
> 
> True. This second loops searches for a continuous region to preserve the
> order of mappings (when possible)

Ah; I think this, like the PoD case, should the more detailed p2m
lookup to get the actual order of the current mapping.  That should be
shorter and more readable, and probably more correct.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel