> -----Original Message-----
> From: Jan Beulich <[email protected]>
> Sent: 29 August 2019 15:07
> To: Paul Durrant <[email protected]>
> Cc: [email protected]; Julien Grall <[email protected]>;
> Andrew Cooper
> <[email protected]>; Anthony Perard <[email protected]>;
> Roger Pau Monne
> <[email protected]>; Volodymyr Babchuk <[email protected]>;
> George Dunlap
> <[email protected]>; Ian Jackson <[email protected]>; Stefano
> Stabellini
> <[email protected]>; Konrad Rzeszutek Wilk <[email protected]>; Tim
> (Xen.org) <[email protected]>;
> Wei Liu <[email protected]>
> 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 <[email protected]>
>
> Reviewed-by: Jan Beulich <[email protected]>
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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
