> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 29 August 2019 15:07
> To: Paul Durrant <paul.durr...@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Julien Grall <julien.gr...@arm.com>;
> Andrew Cooper
> <andrew.coop...@citrix.com>; Anthony Perard <anthony.per...@citrix.com>;
> Roger Pau Monne
> <roger....@citrix.com>; Volodymyr Babchuk <volodymyr_babc...@epam.com>;
> George Dunlap
> <george.dun...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; Stefano
> Stabellini
> <sstabell...@kernel.org>; Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; Tim
> (Xen.org) <t...@xen.org>;
> Wei Liu <w...@xen.org>
> 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 <paul.durr...@citrix.com>
>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
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
