[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v8 6/6] introduce a 'passthrough' configuration option to xl.cfg...
- To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Julien Grall <julien.grall@xxxxxxx>
- Date: Fri, 6 Sep 2019 10:06:24 +0100
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, "Tim \(Xen.org\)" <tim@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>, Christian Lindig <christian.lindig@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, David Scott <dave@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>
- Delivery-date: Fri, 06 Sep 2019 09:06:39 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
Hi Paul,
On 9/6/19 9:08 AM, Paul Durrant wrote:
-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 05 September 2019 21:28
To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Jan Beulich <jbeulich@xxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu
<wl@xxxxxxx>;
Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
<George.Dunlap@xxxxxxxxxx>; Konrad Rzeszutek
Wilk <konrad.wilk@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Tim
(Xen.org)
<tim@xxxxxxx>; Anthony Perard <anthony.perard@xxxxxxxxxx>; Christian Lindig
<christian.lindig@xxxxxxxxxx>; David Scott <dave@xxxxxxxxxx>; Volodymyr Babchuk
<Volodymyr_Babchuk@xxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>
Subject: Re: [PATCH v8 6/6] introduce a 'passthrough' configuration option to
xl.cfg...
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).
I could just say 'not currently available' and, if ARM gains a non-shared
implementation then the manpage could be changed. I don't think xl.cfg files
need to be considered a stable interface, do they?
I am not too worried about the xl.cfg files. My worry is regarding the
libxl_types.idl which is definitely stable.
If you say the field is currently not available for Arm, the field will
still be fill up by a given value. That given value will have to be
handled differently when ARM gains a non-shared implementation.
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.
I don't really like that. The 'default' option is chosen by not putting any option in the cfg, I don't see why it needs to be explicit for this option.r
Well, here you impose the user to know how to configure the IOMMU (i.e.
shared vs non-shared). He/She may not know about it and therefore he/she
will have to try different value until Xen accepts it.
For the benefits of the user (and make it easy to extend in the future)
having an option to say "let Xen chose" allow an external toolstack to
not replicate the exact code you wrote in xl_parse.c.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
