Re: [Xen-devel] [PATCH v6 04/10] xen: Introduce XENMEM_soft_reset operation

[Tim Deegan <tim@xxxxxxx>]

At 17:26 +0100 on 22 May (1432315574), Jan Beulich wrote:
> >>> On 22.05.15 at 17:36, <vkuznets@xxxxxxxxxx> wrote:
> >>>>> On 13.05.15 at 11:49, <vkuznets@xxxxxxxxxx> wrote:
> >>> +    if ( !source_d->is_dying )
> >>> +    {
> >>> +        /*
> >>> +         * Make sure no allocation/remapping for the source domain is 
> >>> ongoing
> >>> +         * and set is_dying flag to prevent such actions in future.
> >>> +         */
> >>> +        spin_lock(&source_d->page_alloc_lock);
> >>> +        source_d->is_dying = DOMDYING_locked;
> >>
> >> Furthermore I don't see how this prevents there being further
> >> changes to the GFN <-> MFN mappings for the guest (yet you rely
> >> on them to be stable below afaict).
> >>
> > 
> > As you suggested below we can complement that by pausing both source and
> > destination domains here. In that case these domains won't be changing
> > their mappings by themselves but it would still be possible for the
> > hypervisor to change something. We do have !d->is_dying check in many
> > places though ... In theory we could have taken the p2m_lock() for both
> > domains but I'm not sure all stuff I need here will cope with p2m_lock()
> > already being held, this lock is x86-specific and I'm not sure we want
> > it in the first place. I'd be very grateful for some additional ideas on
> > how to make it safer.
> 
> For whether p2m_lock() might be needed here I'd like to defer to
> Tim.

I don't think that will work.  Given that you have to make this
preemptable you can't hope to hold the p2m lock for the entire
operation.

If you want to make sure that the p2m mapping doesn't change
underfoot, you can use get_gfn()/put_gfn() around each individual
operation.  If you use get_gfn_type_access() it will also report
superpage mappings, so you can drop the loop that attempts
to detect them in the PoD case.

Incidentally, counting from 0-->max_mapped_gfn is unfortunate (though I
know this is not the first code to do that).  It would be better
to introduce an iterator over the p2m itself, either some sort of
for_each_gfn(dom, callback_fn, callback_arg) or a get_next_gfn(dom,
base_gfn, &found_gfn) that skips unmapped areas.

Tim.

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