Re: [Xen-devel] [PATCH v6 08/11] tools, libxl: parse optional start gfn from the iomem config option

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Mon, 2014-04-21 at 15:45 +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 <avanzini.arianna@xxxxxxxxx>
> Acked-by: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
> Cc: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Cc: Paolo Valente <paolo.valente@xxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> Cc: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
> Cc: Viktor Kleinik <viktor.kleinik@xxxxxxxxxxxxxxx>
> 
> ---
> 
>     v6:
>         - Document what happens if the new "gfn" parameter is omitted
>           while specifying the "iomem" option in the domain configuration.
>         - Add LIBXL_INVALID_GFN macro with value "~(uint64_t)0".
>         - Fix typo in the comment to the LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN
>           macro.
> 
>     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        | 15 +++++++++++----
>  tools/libxl/libxl.h          | 10 ++++++++++
>  tools/libxl/libxl_create.c   |  6 ++++++
>  tools/libxl/libxl_internal.h |  1 +
>  tools/libxl/libxl_types.idl  |  7 +++++--
>  tools/libxl/xl_cmdimpl.c     | 19 ++++++++++---------
>  6 files changed, 43 insertions(+), 15 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index a6663b9..da6e0bc 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -602,12 +602,19 @@ 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<iomem=[ "IOMEM_START,NUM_PAGES", "IOMEM_START,NUM_PAGES", ... ]>
> +=item B<iomem=[ "IOMEM_START,NUM_PAGES[@GFN]", 
> "IOMEM_START,NUM_PAGES[@GFN]", ... ]>
>  
>  Allow guest to access specific hardware I/O memory pages. B<IOMEM_START>
> -is a physical page number. B<NUM_PAGES> is the number
> -of pages beginning with B<START_PAGE> to allow access. Both values
> -must be given in hexadecimal.
> +is a physical page number. B<NUM_PAGES> is the number of pages beginning
> +with B<START_PAGE> to allow access. B<GFN> specifies the guest frame number
> +where the mapping will start in the domU's address space. If B<GFN> is not
> +given, the mapping will be performed using B<IOMEM_START> as a start in the
> +domU's address space.

Please can you make some allusion to this being a 1:1 mapping in the
final sentence.

> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index c2b73c4..2dba844 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -91,6 +91,7 @@
>  #define LIBXL_PV_EXTRA_MEMORY 1024
>  #define LIBXL_HVM_EXTRA_MEMORY 2048
>  #define LIBXL_MIN_DOM0_MEM (128*1024)
> +#define LIBXL_INVALID_GFN (~(uint64_t)0)
>  /* use 0 as the domid of the toolstack domain for now */
>  #define LIBXL_TOOLSTACK_DOMID 0
>  #define QEMU_SIGNATURE "DeviceModelRecord0002"
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 0f7bbf8..f3aaa46 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -170,8 +170,11 @@ libxl_ioport_range = Struct("ioport_range", [
>      ])
>  
>  libxl_iomem_range = Struct("iomem_range", [
> -    ("start", uint64),
> -    ("number", uint64),
> +    ("start", uint64),                 # start host frame number to be mapped
> +                                       # to the guest
> +    ("number", uint64),                # number of frames to be mapped
> +    ("gfn", uint64, {'init_val': -1}), # guest frame number used as a start

ITYM to use LIBXL_INVALID_GFN here.

Ian.


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