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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Paul,

On 9/16/19 10:27 AM, 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.
>
> If the value of passthrough is 'enabled' then xl will choose an appropriate
> default according to the type of domain and hardware support.
>
> NOTE: The 'iommu_memkb' overhead in libxl_domain_build_info will now only
>        be set if passthrough is 'sync_pt' (or xl has chosen this mode as
>        a default).
>
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx>
> ---
> 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: David Scott <dave@xxxxxxxxxx>
> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
>
> Sending just this patch since the rest of the series is unchanged from
> v11(.1).
>
> Previously part of series 
> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02267.html
>
> v12:
>   - Set passthrough default in libxl
>
> v11:
>   - Fixed abi-check runes
>
> v10:
>   - Added abi-check runes
>
> v9:
>   - Added the passthrough='enabled' option to xl
>   - One cosmetic change in xen
>   - Assume Jan's R-b stands since non-cosmetic changes are only in the
>     toolstack
>
> 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            |  56 ++++++++++
>   tools/libxl/libxl.h                 |   9 ++
>   tools/libxl/libxl_create.c          |  27 +++--
>   tools/libxl/libxl_types.idl         |   7 ++
>   tools/ocaml/libs/xc/xenctrl.ml      |   4 +
>   tools/ocaml/libs/xc/xenctrl.mli     |   5 +
>   tools/ocaml/libs/xc/xenctrl_stubs.c |  17 ++-
>   tools/xl/xl_parse.c                 | 158 ++++++++++++++++++----------
>   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         |  10 +-
>   xen/include/xen/iommu.h             |  15 +--
>   15 files changed, 266 insertions(+), 87 deletions(-)
>
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index c99d40307e..154d847fb9 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -605,6 +605,62 @@ 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
Apologies to make the comment this late (it probably can be addressed in 
a follow-up if the patch is good to go).
From the user point of view, they have the choice between HVM and PVH.
The fact that PVH are based on HVM domain is a Xen internal (actually we 
were using PV for Àrm in the past).
So we may want to say HVM/PVH here and other instance below.

Other than that the Arm changes looks good to me:

Acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

--
Julien Grall

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