Re: [PATCH v4] x86/p2m: use large pages for MMIO mappings

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2016-01-25 at 06:54 -0700, Jan Beulich wrote:
> > > > On 25.01.16 at 13:16, <ian.campbell@citrix.com> wrote:
> > On Fri, 2016-01-22 at 08:42 -0700, Jan Beulich wrote:
> > > +#define MAP_MMIO_MAX_ITER 64 /* pretty arbitrary */
> > > +
> > 
> > I suppose no existing in-tree code exceeds that (or there'd be more
> > patch
> > here).
> 
> There simply is no in-tree user other than the domctl on x86.


Right, I meant callers of the domctl (via libxc)

>  It's
> only ARM which has numerous other users (complicating the fixing
> of the issue there).
> 
> > > --- a/xen/include/public/domctl.h
> > > +++ b/xen/include/public/domctl.h
> > > @@ -542,8 +542,14 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_bind_
> > >  
> > >  
> > >  /* Bind machine I/O address range -> HVM address range. */
> > > -/* If this returns -E2BIG lower nr_mfns value. */
> > >  /* XEN_DOMCTL_memory_mapping */
> > > +/* Returns
> > > +   - zero     (success, everything done)
> > > +   - -E2BIG   (passed in nr_mfns value too large for the
> > > implementation)
> > > +   - positive (partial success, this many [less than nr_mfns] done,
> > 
> > Is the successful region contiguous, i.e. 0..return val, or does the
> > caller
> > need to figure it somehow? (I think based on libxc changes the former,
> > but
> > it should be spelt out here I think).
> 
> Yes, it is contiguous, but I'm at a loss how to spell out the (seemingly
> obvious) fact here: "partial success, this many [less than nr_mfns]
> initial iterations done" doesn't sound much better to me. Everything
> else I can think of would require a full second sentence, which I
> wouldn't like here.

I'd perhaps write "[0, result) completed successfully" or something along
those lines.

Maybe I'm just over thinking it having the MMAP_BATCH semantics (with a
full err array) on my mind.

> > > +               requiring re-invocation by the caller with updated
> > > inputs)
> > > +   - negative (error)
> > 
> > This is a more general case of -E2BIG, you might fix that by saying
> > "other
> > error" or by moving -E2BIG to be a subclause.
> 
> "other error" would seem okay, or how about "negative (error; other
> than -E2BIG)"?

Both ok IMHO.
> 
> Jan
> 

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