From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v5 6/8] tools, libxl: parse optional start gfn from the iomem config option Date: Wed, 9 Apr 2014 15:10:24 +0100 Message-ID: <1397052624.6275.79.camel@kazak.uk.xensource.com> References: <1396827120-30617-1-git-send-email-avanzini.arianna@gmail.com> <1396827120-30617-7-git-send-email-avanzini.arianna@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1396827120-30617-7-git-send-email-avanzini.arianna@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Arianna Avanzini Cc: paolo.valente@unimore.it, keir@xen.org, stefano.stabellini@eu.citrix.com, Ian.Jackson@eu.citrix.com, dario.faggioli@citrix.com, tim@xen.org, xen-devel@lists.xen.org, julien.grall@citrix.com, etrudeau@broadcom.com, JBeulich@suse.com, viktor.kleinik@globallogic.com List-Id: xen-devel@lists.xenproject.org On Mon, 2014-04-07 at 01:31 +0200, Arianna Avanzini wrote: > Currently, the "iomem" domU config option allows to specify a machine > address range to be mapped to the domU. However, there is no way to > specify the guest address range used for the mapping. This commit > extends the iomem option handling code to parse an additional, optional > parameter: this parameter, if given, specifies the start guest address > used for the mapping; if no start guest address is given, a 1:1 mapping > is performed as default. > > Signed-off-by: Arianna Avanzini > Cc: Dario Faggioli > Cc: Paolo Valente > Cc: Stefano Stabellini > Cc: Julien Grall > Cc: Ian Campbell > Cc: Jan Beulich > Cc: Keir Fraser > Cc: Tim Deegan > Cc: Ian Jackson > Cc: Eric Trudeau > Cc: Viktor Kleinik > > --- > > v5: > - Initialize the gfn field of the iomem structure with the > value "(uint64_t)-1". > - Defer the assignment of the value of "start" to "gfn", if > "gfn" has been initialized to the default value, in libxl. > - Use a local variable to keep the return value of sscanf() > for better code readability. > > v4: > - Add definition of a LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN macro > to indicate the availability of the new option. > - Simplify the code parsing the new optional parameter by using > the switch construct on the return value of the sscanf() function > instead of calling the function twice. > - Add a short paragraph to the manpage about the use of the iomem > option to passthrough devices protected by an IOMMU. > - Add comments to the fields of the "iomem" structure. > > --- > docs/man/xl.cfg.pod.5 | 13 +++++++++---- > tools/libxl/libxl.h | 10 ++++++++++ > tools/libxl/libxl_create.c | 6 ++++++ > tools/libxl/libxl_types.idl | 7 +++++-- > tools/libxl/xl_cmdimpl.c | 19 ++++++++++--------- > 5 files changed, 40 insertions(+), 15 deletions(-) > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > index a6663b9..d54f601 100644 > --- a/docs/man/xl.cfg.pod.5 > +++ b/docs/man/xl.cfg.pod.5 > @@ -602,12 +602,17 @@ is given in hexadecimal and may either a span e.g. C<2f8-2ff> > It is recommended to use this option only for trusted VMs under > administrator control. > > -=item B > +=item B > > Allow guest to access specific hardware I/O memory pages. B > -is a physical page number. B is the number > -of pages beginning with B to allow access. Both values > -must be given in hexadecimal. > +is a physical page number. B is the number of pages beginning > +with B to allow access. B specifies the guest frame number > +where the mapping will start in the domU's address space. "If B is not given then ..." > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index b2c3015..e4a1128 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -95,6 +95,16 @@ > #define LIBXL_HAVE_BUILDINFO_EVENT_CHANNELS 1 > > /* > + * LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN indicated that it is possible s/indicated/indicates/ > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index d015cf4..369cd77 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > + if (b_info->iomem[i].gfn == (uint64_t)-1) Please can you #define something like LIBXL_INVALID_GFN and use it both here and in the idl. The value should probably be "~(uint64_t)0" rather than -1. Other than those small nits: Acked-by: Ian Campbell