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

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Tim Deegan <tim@xen.org> writes:

> At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
>> Tim Deegan <tim@xen.org> 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.

If we bring get_gfn_type_access() call to the top level it becomes
possible (and easy) but (if I'm not mistaken) we still need to walk
through all pages of the mapping checking that each of them has the
reqiuired count_info (so it is not mapped from other domain or xen
itself). In case we meet a 'bad' page we'll have to split the mapping
(and copy the page itself).

-- 
  Vitaly