Re: [PATCH 3/3] tools: introduce parameter max_ranges.

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 19 January 2016 11:54
> To: Yu Zhang
> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Stefano Stabellini;
> Andrew Cooper; Paul Durrant; zhiyuan.lv@intel.com; jbeulich@suse.com;
> Wei Liu
> Subject: Re: [Xen-devel] [PATCH 3/3] tools: introduce parameter
> max_ranges.
> 
> On Tue, Jan 19, 2016 at 05:27:58PM +0800, Yu Zhang wrote:
> > A new parameter - max_ranges is added to set the upper limit of ranges
> > to be tracked inside one ioreq server rangeset.
> >
> > Ioreq server uses a group of rangesets to track the I/O or memory
> > resources to be emulated. The default value of this limit is set to
> > 256. Yet there are circumstances under which the limit should exceed
> > the default one. E.g. in XenGT, when tracking the per-process graphic
> > translation tables on intel broadwell platforms, the number of page
> > tables concerned will be several thousand(normally in this case, 8192
> > could be a big enough value). Users who set his item explicitly are
> > supposed to know the specific scenarios that necessitate this
> > configuration.
> >
> > Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> > ---
> >  docs/man/xl.cfg.pod.5           | 17 +++++++++++++++++
> >  tools/libxl/libxl_dom.c         |  3 +++
> >  tools/libxl/libxl_types.idl     |  1 +
> >  tools/libxl/xl_cmdimpl.c        |  4 ++++
> >  xen/arch/x86/hvm/hvm.c          |  7 ++++++-
> >  xen/include/public/hvm/params.h |  5 ++++-
> >  6 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index 8899f75..562563d 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -962,6 +962,23 @@ FIFO-based event channel ABI support up to
> 131,071 event channels.
> >  Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
> >  x86).
> >
> > +=item B<max_ranges=N>
> > +
> 
> This name is too generic. I don't have better suggestion though.
> 

I think the increased limit for XenGT need only be applied to wp mem ranges so perhaps the parameter name could be 'max_wp_memory_ranges'?

  Paul

> > +Limit the maximum ranges that can be tracked inside one ioreq server
> > +rangeset.
> > +
> > +Ioreq server uses a group of rangesets to track the I/O or memory
> > +resources to be emulated. By default, this item is not set. Not
> > +configuring this item, or setting its value to 0 will result in the
> > +upper limit set to its default value - 256. Yet there are circumstances
> 
> No need to say 256 because this might change in the future in
> hypervisor.
> 
> > +under which the upper limit inside one rangeset should exceed the
> > +default one. E.g. in XenGT, when tracking the per-process graphic
> > +translation tables on intel broadwell platforms, the number of page
> > +tables concerned will be several thousand(normally in this case, 8192
> > +could be a big enough value). Users who set his item explicitly are
> > +supposed to know the specific scenarios that necessitate this
> > +configuration.
> > +
> >  =back
> >
> >  =head2 Paravirtualised (PV) Guest Specific Options
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index 47971a9..607b0c4 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -288,6 +288,9 @@ static void hvm_set_conf_params(xc_interface
> *handle, uint32_t domid,
> >                      libxl_defbool_val(info->u.hvm.nested_hvm));
> >      xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M,
> >                      libxl_defbool_val(info->u.hvm.altp2m));
> > +    if (info->u.hvm.max_ranges > 0)
> > +        xc_hvm_param_set(handle, domid, HVM_PARAM_MAX_RANGES,
> > +                        info->u.hvm.max_ranges);
> >  }
> >
> >  int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 9ad7eba..c936265 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -518,6 +518,7 @@ libxl_domain_build_info =
> Struct("domain_build_info",[
> >                                         ("serial_list",      libxl_string_list),
> >                                         ("rdm", libxl_rdm_reserve),
> >                                         ("rdm_mem_boundary_memkb", MemKB),
> > +                                       ("max_ranges", uint32),
> 
> This also needs a better name.
> 
> Another thing is that you need to define LIBXL_HAVE_XXX in libxl.h to
> indicate we introduce a new field.
> 
> Wei.