Re: [Xen-devel] [PATCH 2/4] fix XENMEM_add_to_physmap preemption handling

[Tim Deegan <tim@xxxxxxx>]

At 16:03 +0000 on 18 Dec (1387378993), Jan Beulich wrote:
> >>> On 18.12.13 at 16:48, Tim Deegan <tim@xxxxxxx> wrote:
> > At 14:35 +0000 on 18 Dec (1387373707), Jan Beulich wrote:
> >> Just like for all other hypercalls we shouldn't be modifying the input
> >> structure - all of the fields are, even if not explicitly documented,
> >> just inputs.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> >> @@ -543,22 +543,32 @@ static long memory_exchange(XEN_GUEST_HA
> >>  }
> >>  
> >>  static int xenmem_add_to_physmap(struct domain *d,
> >> -                                 struct xen_add_to_physmap *xatp)
> >> +                                 struct xen_add_to_physmap *xatp,
> >> +                                 unsigned int start)
> >>  {
> >> -    struct xen_add_to_physmap start_xatp;
> >> -    int rc = 0;
> >> +    unsigned int done = 0;
> >> +    long rc = 0;
> >>  
> >>      if ( xatp->space != XENMAPSPACE_gmfn_range )
> >> +    {
> >> +        ASSERT(!start);
> > 
> > I don't think you've enforced this in the caller; you only check that
> > the guest hasn't supplied an over-sized start-extent.  I think it's
> > fine just to ignore start for singleton operations anyway.
> 
> Right, if at all I should be returning an error here. But that should
> perhaps either done uniformly at once for all mem ops, or not at
> all.
> 
> I'll just drop that change - makes the patch smaller :-)

Grand so. :)  With that dropped, 1/4 and 2/4 are
Reviewed-by: Tim Deegan <tim@xxxxxxx>

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