Re: [Xen-devel] [PATCH v6 4/4] xen: rework paging_log_dirty_op to work with hvm guests

[Tim Deegan <tim@xxxxxxx>]

At 16:25 +0200 on 11 May (1431361528), Roger Pau Monne wrote:
> When the caller of paging_log_dirty_op is a hvm guest Xen would choke when
> trying to copy the dirty bitmap to the guest because the paging lock is
> already held.
> 
> Fix this by independently mapping each page of the guest bitmap as needed
> without the paging lock held.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

This looks mostly correct, but OTOH we now seem to have two nested
stop-and-retry mechsnisms in one function.  I think this would be
cleaner if here:

> +                    if ( pages >> (3 + PAGE_SHIFT) !=
> +                         index_mapped >> (3 + PAGE_SHIFT) )
>                      {
> -                        rv = -EFAULT;
> -                        goto out;
> +                        /* We need to map next page */
> +                        d->arch.paging.preempt.log_dirty.i4 = i4;
> +                        d->arch.paging.preempt.log_dirty.i3 = i3;
> +                        d->arch.paging.preempt.log_dirty.i2 = i2;
> +                        d->arch.paging.preempt.log_dirty.done = pages;
> +                        paging_unlock(d);
> +                        unmap_dirty_bitmap(dirty_bitmap, page);

we set the rest of the preempt state, like so:

        d->arch.paging.preempt.dom = current->domain;
        d->arch.paging.preempt.op = sc->op;
        resuming = 1;

and bailed to the very top of the function.  I think we might also
need a hypercall_preempt_check() here as well, so that this restart
path doesn't stop us ever reaching the preempt_checks below.

Cheers,

Tim.

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