Re: [Xen-devel] [PATCH v6 10/10] introduce a 'passthrough' configuration option to xl.cfg...
> -Original Message-
> From: Jan Beulich
> Sent: 29 August 2019 15:07
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Julien Grall ;
> Andrew Cooper
> ; Anthony Perard ;
> Roger Pau Monne
> ; Volodymyr Babchuk ;
> George Dunlap
> ; Ian Jackson ; Stefano
> Stabellini
> ; Konrad Rzeszutek Wilk ; Tim
> (Xen.org) ;
> Wei Liu
> Subject: Re: [PATCH v6 10/10] introduce a 'passthrough' configuration option
> to xl.cfg...
>
> On 16.08.2019 19:20, 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.
> >
> > Signed-off-by: Paul Durrant
>
> Reviewed-by: Jan Beulich
Thanks.
> with a question/suggestion and a nit:
>
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -308,6 +308,13 @@ static int sanitise_domain_config(struct
> > xen_domctl_createdomain *config)
> > return -EINVAL;
> > }
> >
> > +if ( !(config->flags & XEN_DOMCTL_CDF_iommu) && config->iommu_opts )
> > +{
> > +dprintk(XENLOG_INFO,
> > +"IOMMU options specified but IOMMU not enabled\n");
> > +return -EINVAL;
> > +}
>
> Seeing this I wonder whether XEN_DOMCTL_CDF_iommu wouldn't better be
> bit 0 of iommu_opts.
>
I debated this with myself and I prefer to stick with separate flag and options.
> > @@ -70,6 +70,10 @@ struct xen_domctl_createdomain {
> >
> > uint32_t flags;
> >
> > +#define _XEN_DOMCTL_IOMMU_no_sharept 0
> > +#define XEN_DOMCTL_IOMMU_no_sharept (1U<<_XEN_DOMCTL_IOMMU_no_sharept)
>
> Please can you add the missing blanks around << ?
>
Sure.
Paul
> Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 10/10] introduce a 'passthrough' configuration option to xl.cfg...
> -Original Message-
> From: Roger Pau Monne
> Sent: 23 August 2019 15:17
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Ian Jackson ; Wei
> Liu ; Andrew
> Cooper ; George Dunlap ;
> Jan Beulich
> ; Julien Grall ; Konrad Rzeszutek
> Wilk
> ; Stefano Stabellini ; Tim
> (Xen.org) ;
> Anthony Perard ; Volodymyr Babchuk
>
> Subject: Re: [PATCH v6 10/10] introduce a 'passthrough' configuration option
> to xl.cfg...
>
> On Fri, Aug 16, 2019 at 06:20:01PM +0100, 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.
> >
> > Signed-off-by: Paul Durrant
> > ---
> > Cc: Ian Jackson
> > Cc: Wei Liu
> > Cc: Andrew Cooper
> > Cc: George Dunlap
> > Cc: Jan Beulich
> > Cc: Julien Grall
> > Cc: Konrad Rzeszutek Wilk
> > Cc: Stefano Stabellini
> > Cc: Tim Deegan
> > Cc: Anthony PERARD
> > Cc: Volodymyr Babchuk
> > Cc: "Roger Pau Monné"
> >
> > Previously part of series
> > https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02267.html
> >
> > 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/xl/xl_parse.c | 38
> > xen/arch/arm/domain.c | 10 ++-
> > xen/arch/x86/domain.c | 2 +-
> > xen/common/domain.c | 7 +
> > xen/drivers/passthrough/iommu.c | 13 -
> > xen/include/public/domctl.h | 6 +++-
> > xen/include/xen/iommu.h | 19
> > 11 files changed, 158 insertions(+), 10 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
> > +
> > +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
> > +
> > +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
>
> I would maybe name this exclusive_pt, but historically it's been named
> sync_pt.
>
Yes, I prefer to stick with the incumbent name.
[snip]
> > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> > index e105bda2bb..c904604008 100644
> > --- a/tools/xl/xl_parse.c
> > +++ b/tools/xl/xl_parse.c
> > @@ -2326,6 +2326,44 @@ skip_vfb:
> > }
> > }
> >
> > +if (!xlu_cfg_get_string(config, "passthrough", , 0)) {
> > +libxl_passthrough o;
> > +
> > +e = libxl_passthrough_from_string(buf, );
> > +if (e) {
> > +fprintf(stderr,
> > +"ERROR: unknown passthrough option '%s'\n",
> > +buf);
> > +exit(-ERROR_FAIL);
> > +}
> > +
> > +switch (o) {
> > +case LIBXL_PASSTHROUGH_DISABLED:
> > +if (d_config->num_pcidevs || d_config->num_dtdevs) {
> > +fprintf(stderr,
> > +"ERROR: passthrough disabled but devices are
> > specified\n");
> > +exit(-ERROR_FAIL);
> > +}
>
> Don't you need a break here?
>
> > +case LIBXL_PASSTHROUGH_SHARE_PT:
> > +if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
> > +fprintf(stderr,
> > +"ERROR:
Re: [Xen-devel] [PATCH v6 10/10] introduce a 'passthrough' configuration option to xl.cfg...
On 16.08.2019 19:20, 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.
>
> Signed-off-by: Paul Durrant
Reviewed-by: Jan Beulich
with a question/suggestion and a nit:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -308,6 +308,13 @@ static int sanitise_domain_config(struct
> xen_domctl_createdomain *config)
> return -EINVAL;
> }
>
> +if ( !(config->flags & XEN_DOMCTL_CDF_iommu) && config->iommu_opts )
> +{
> +dprintk(XENLOG_INFO,
> +"IOMMU options specified but IOMMU not enabled\n");
> +return -EINVAL;
> +}
Seeing this I wonder whether XEN_DOMCTL_CDF_iommu wouldn't better be
bit 0 of iommu_opts.
> @@ -70,6 +70,10 @@ struct xen_domctl_createdomain {
>
> uint32_t flags;
>
> +#define _XEN_DOMCTL_IOMMU_no_sharept 0
> +#define XEN_DOMCTL_IOMMU_no_sharept (1U<<_XEN_DOMCTL_IOMMU_no_sharept)
Please can you add the missing blanks around << ?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 10/10] introduce a 'passthrough' configuration option to xl.cfg...
On Fri, Aug 16, 2019 at 06:20:01PM +0100, 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. > > Signed-off-by: Paul Durrant > --- > Cc: Ian Jackson > Cc: Wei Liu > Cc: Andrew Cooper > Cc: George Dunlap > Cc: Jan Beulich > Cc: Julien Grall > Cc: Konrad Rzeszutek Wilk > Cc: Stefano Stabellini > Cc: Tim Deegan > Cc: Anthony PERARD > Cc: Volodymyr Babchuk > Cc: "Roger Pau Monné" > > Previously part of series > https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02267.html > > 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/xl/xl_parse.c | 38 > xen/arch/arm/domain.c | 10 ++- > xen/arch/x86/domain.c | 2 +- > xen/common/domain.c | 7 + > xen/drivers/passthrough/iommu.c | 13 - > xen/include/public/domctl.h | 6 +++- > xen/include/xen/iommu.h | 19 > 11 files changed, 158 insertions(+), 10 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 > + > +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 > + > +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 I would maybe name this exclusive_pt, but historically it's been named 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. > + > +=item B > + > +This option is unavailable for a PV domain. For an HVM domain, this option > +means that the IOMMU will be programmed to directly reference the domain's > +P2M table as its page table. From the point of view of a device driver > +running in the domain this is functionally equivalent to B but > +places less load on the hypervisor and so should generally be selected in > +preference. However, the availability of this option is hardware specific > +and thus, if it is specified for a domain running on hardware that does > +not allow it (e.g. AMD), B will be used instead. > + > +This option is the default if the domain is HVM and passthrough hardware > +is specified in the configuration. > + > +=back > + > =back > > =head2 Devices > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 9bacfb97f0..5de7c07a41 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -394,6 +394,11 @@ > */ > #define LIBXL_HAVE_EXTENDED_VKB 1
[Xen-devel] [PATCH v6 10/10] introduce a 'passthrough' configuration option to xl.cfg...
...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. Signed-off-by: Paul Durrant --- Cc: Ian Jackson Cc: Wei Liu Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Anthony PERARD Cc: Volodymyr Babchuk Cc: "Roger Pau Monné" Previously part of series https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02267.html 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/xl/xl_parse.c | 38 xen/arch/arm/domain.c | 10 ++- xen/arch/x86/domain.c | 2 +- xen/common/domain.c | 7 + xen/drivers/passthrough/iommu.c | 13 - xen/include/public/domctl.h | 6 +++- xen/include/xen/iommu.h | 19 11 files changed, 158 insertions(+), 10 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 + +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 + +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 + +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. + +=item B + +This option is unavailable for a PV domain. For an HVM domain, this option +means that the IOMMU will be programmed to directly reference the domain's +P2M table as its page table. From the point of view of a device driver +running in the domain this is functionally equivalent to B but +places less load on the hypervisor and so should generally be selected in +preference. However, the availability of this option is hardware specific +and thus, if it is specified for a domain running on hardware that does +not allow it (e.g. AMD), B will be used instead. + +This option is the default if the domain is HVM and passthrough hardware +is specified in the configuration. + +=back + =back =head2 Devices diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 9bacfb97f0..5de7c07a41 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -394,6 +394,11 @@ */ #define LIBXL_HAVE_EXTENDED_VKB 1 +/* + * libxl_domain_create_info has libxl_passthrough enumeration. + */ +#define LIBXL_HAVE_CREATEINFO_PASSTHROUGH 1 + /* * libxl ABI compatibility * diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 03ce166f4f..f288e13dc1 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -564,6 +564,15 @@ int libxl__domain_make(libxl__gc *gc,
