Re: [Xen-devel] [PATCH v8 6/6] introduce a 'passthrough' configuration option to xl.cfg...

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 9/2/19 3:50 PM, Paul Durrant wrote:
> ...and hence the ability to disable IOMMU mappings, and control EPT
> sharing.
>
> This patch introduces a new 'libxl_passthrough' enumeration into
> libxl_domain_create_info. The value will be set by xl either when it parses
> a new 'passthrough' option in xl.cfg, or implicitly if there is passthrough
> hardware specified for the domain.
>
> If the value of the passthrough configuration option is 'disabled' then
> the XEN_DOMCTL_CDF_iommu flag will be clear in the xen_domctl_createdomain
> flags, thus allowing the toolstack to control whether the domain gets
> IOMMU mappings or not (where previously they were globally set).
>
> If the value of the passthrough configuration option is 'sync_pt' then
> a new 'iommu_opts' field in xen_domctl_createdomain will be set with the
> value XEN_DOMCTL_IOMMU_no_sharept. This will override the global default
> set in iommu_hap_pt_share, thus allowing the toolstack to control whether
> EPT sharing is used for the domain.
>
> NOTE: The 'iommu_memkb' overhead in libxl_domain_build_info will only be
>        set to zero if passthrough is 'disabled'. It is not safe to set the
>        overhead to zero in the 'share_pt' case because the toolstack has no
>        means of knowing whether the hardware actually supports IOMMU page
>        table sharing.
>
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> Cc: Christian Lindig <christian.lindig@xxxxxxxxxx>
> Cc: David Scott <dave@xxxxxxxxxx>
> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
>
> Previously part of series 
> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02267.html
>
> v7:
>   - Added missing breaks
>   - Added missing ocaml binding changes
>
> v6:
>   - Remove the libxl_physinfo() call since it's usefulness is limited to x86
>
> v5:
>   - Expand xen_domctl_createdomain flags field and hence bump interface
>     version
>   - Fix spelling mistakes in context line
> ---
>   docs/man/xl.cfg.5.pod.in            |  52 +++++++++++
>   tools/libxl/libxl.h                 |   5 +
>   tools/libxl/libxl_create.c          |   9 ++
>   tools/libxl/libxl_types.idl         |   7 ++
>   tools/ocaml/libs/xc/xenctrl.ml      |   4 +
>   tools/ocaml/libs/xc/xenctrl.mli     |   4 +
>   tools/ocaml/libs/xc/xenctrl_stubs.c |  15 ++-
>   tools/xl/xl_parse.c                 | 140 ++++++++++++++++++----------
>   xen/arch/arm/domain.c               |  10 +-
>   xen/arch/x86/domain.c               |   2 +-
>   xen/common/domain.c                 |   7 ++
>   xen/common/domctl.c                 |  13 ---
>   xen/drivers/passthrough/iommu.c     |  13 ++-
>   xen/include/public/domctl.h         |   6 +-
>   xen/include/xen/iommu.h             |  19 ++--
>   15 files changed, 229 insertions(+), 77 deletions(-)
>
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index c99d40307e..fd35685e9e 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -605,6 +605,58 @@ option should only be used with a trusted device tree.
>   Note that the partial device tree should avoid using the phandle 65000
>   which is reserved by the toolstack.
>   
> +=item B<passthrough="STRING">
> +
> +Specify whether IOMMU mappings are enabled for the domain and hence whether
> +it will be enabled for passthrough hardware. Valid values for this option
> +are:
> +
> +=over 4
> +
> +=item B<disabled>
> +
> +IOMMU mappings are disabled for the domain and so hardware may not be
> +passed through.
> +
> +This option is the default if no passthrough hardware is specified
> +in the domain's configuration.
> +
> +=item B<sync_pt>
> +
> +This option means that IOMMU mappings will be synchronized with the
> +domain's P2M table as follows:
> +
> +For a PV domain, all writable pages assigned to the domain are identity
> +mapped by MFN in the IOMMU page table. Thus a device driver running in the
> +domain may program passthrough hardware for DMA using MFN values
> +(i.e. host/machine frame numbers) looked up in its P2M.
> +
> +For an HVM domain, all non-foreign RAM pages present in its P2M will be
> +mapped by GFN in the IOMMU page table. Thus a device driver running in the
> +domain may program passthrough hardware using GFN values (i.e. guest
> +physical frame numbers) without any further translation.
> +
> +This option is the default if the domain is PV and passthrough hardware
> +is specified in the configuration.
> +
> +This option is not available on Arm.
I don't particularly like the idea to allow the user (or any external 
toolstack) to rely on passthrough=share_pt for Arm. This may put us in a 
corner if we ever support IOMMU that can't use the CPU PT (I know a few 
of them).
It feels to me we want a "default" that can let the toolstack (or the 
hypervisor) to select what ever is the most suitable for the preferred way.
For now default, could be aliased to share_pt for Arm in the toolstack. 
The point here is any toolstack built on top of libxl will not rely on 
passthrough=share_pt.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel