Re: [Xen-devel] [PATCH 3/4] move XENMEM_add_to_physmap_range handling framework to common code

[Tim Deegan <tim@xxxxxxx>]

At 16:08 +0000 on 18 Dec (1387379324), Jan Beulich wrote:
> >>> On 18.12.13 at 16:59, Tim Deegan <tim@xxxxxxx> wrote:
> > At 14:35 +0000 on 18 Dec (1387373737), Jan Beulich wrote:
> >> There's really nothing really architecture specific here; the
> >> architecture specific handling is limited to
> >> xenmem_add_to_physmap_one().
> >> 
> >> Note the restriction of XENMAPSPACE_gmfn_range only being supported
> >> through XENMEM_add_to_physmap is being dropped as being pointless.
> > 
> > Hmmm.   With that restriction removed, this: 
> > 
> >> +        rc = xenmem_add_to_physmap_one(d, xatpr->space,
> >> +                                       xatpr->foreign_domid,
> >> +                                       idx, gpfn);
> >> +
> >> +        if ( unlikely(__copy_to_guest_offset(xatpr->errs, 0, &rc, 1)) )
> >> +        {
> >> +            rc = -EFAULT;
> >> +            goto out;
> >> +        }
> >> +
> >> +        guest_handle_add_offset(xatpr->idxs, 1);
> >> +        guest_handle_add_offset(xatpr->gpfns, 1);
> >> +        guest_handle_add_offset(xatpr->errs, 1);
> >> +        xatpr->size--;
> >> +
> >> +        /* Check for continuation if it's not the last interation. */
> >> +        if ( xatpr->size > 0 && hypercall_preempt_check() )
> >> +        {
> >> +            rc = -EAGAIN;
> >> +            goto out;
> >> +        }
> > 
> > will save the partial result of a preempted XENMAPSPACE_gmfn_range
> > sub-op into the errors array, and then the whole operation may or may
> > not return -EAGAIN.
> > 
> > I guess the caller now has enough information to figure out what
> > happened and restart, but given that the rest of this interface nicely
> > hides preemption from the caller, it's a bit of an ugly corner case.
> 
> I'm afraid I don't follow: XENMAPSPACE_gmfn_range and
> XENMAPSPACE_gmfn will be treated equally now - there simply
> is no range being specified for the individual
> add_to_physmap_range elements.

Oh of course.  Then it's just a slightly odd API, but I guess no
odder this way than before, and with slightly less code. :)  So
Reviewed-by: Tim Deegan <tim@xxxxxxx> for 3/4 and 4/4 as they stand.

Cheers,

Tim.


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