> -----Original Message-----
> From: Xen-devel [mailto:[email protected]] On Behalf
> Of Paul Durrant
> Sent: 20 July 2018 10:05
> To: Wei Liu <[email protected]>
> Cc: Stefano Stabellini <[email protected]>; Wei Liu
> <[email protected]>; Andrew Cooper <[email protected]>; Tim
> (Xen.org) <[email protected]>; George Dunlap <[email protected]>;
> Julien Grall <[email protected]>; Jan Beulich <[email protected]>; Ian
> Jackson <[email protected]>; [email protected]
> Subject: Re: [Xen-devel] [PATCH v3 10/13] x86: add iommu_op to enable
> modification of IOMMU mappings
>
> > -----Original Message-----
> > From: Wei Liu [mailto:[email protected]]
> > Sent: 20 July 2018 09:59
> > To: Paul Durrant <[email protected]>
> > Cc: [email protected]; Jan Beulich <[email protected]>;
> > Andrew Cooper <[email protected]>; George Dunlap
> > <[email protected]>; Ian Jackson <[email protected]>;
> Julien
> > Grall <[email protected]>; Konrad Rzeszutek Wilk
> > <[email protected]>; Stefano Stabellini <[email protected]>;
> Tim
> > (Xen.org) <[email protected]>; Wei Liu <[email protected]>
> > Subject: Re: [PATCH v3 10/13] x86: add iommu_op to enable modification
> of
> > IOMMU mappings
> >
> > On Tue, Jul 17, 2018 at 02:38:13PM +0100, Paul Durrant wrote:
> > > +static int iommu_op_enable_modification(void)
> > > +{
> > > + struct domain *currd = current->domain;
> > > + struct domain_iommu *iommu = dom_iommu(currd);
> > > + const struct iommu_ops *ops = iommu->platform_ops;
> > > +
> > > + /* Has modification already been enabled? */
> > > + if ( iommu->iommu_op_ranges )
> > > + return 0;
> > > +
> > > + /*
> > > + * The IOMMU mappings cannot be modified if:
> > > + * - the IOMMU is not enabled or,
> > > + * - the current domain is dom0 and tranlsation is disabled or,
> > > + * - HAP is enabled and the IOMMU shares the mappings.
> > > + */
> > > + if ( !iommu_enabled ||
> > > + (is_hardware_domain(currd) && iommu_passthrough) ||
> > > + iommu_use_hap_pt(currd) )
> > > + return -EACCES;
> > > +
> > > + /*
> > > + * The IOMMU implementation must provide the lookup method if
> > > + * modification of the mappings is to be supported.
> > > + */
> > > + if ( !ops->lookup_page )
> > > + return -EOPNOTSUPP;
> > > +
> > > + iommu->iommu_op_ranges = rangeset_new(currd, NULL, 0);
> > > + if ( !iommu->iommu_op_ranges )
> > > + return -ENOMEM;
> > > +
> > > + currd->need_iommu = 0; /* Disable identity GFN mapping */
> >
> > I think this flag currently serves two purposes: one is to indicate
> > whether there are resources allocated within xen, the other it indicate
> > whether it is in use (as suggested in the comment).
> >
> > Suppose a domain gets all its devices deassigned after this call,
> > original iommu_teardown would have been called, but now it won't. The
> > iommu will still be torn down when it is destroyed, but this is a very
> > subtle change in behaviour.
>
> That is pretty subtle.
Actually, thinking a bit more, that is the semantic I want. The domain is
taking control of its own mappings. It doesn't want Xen to tear them down under
its feet.
>
> >
> > IMHO we should clearly separate these two cases if possible.
> >
>
> Agreed. I never liked the 'need_iommu' name anyway. I'll add a patch before
> this one to do the rename/split.
>
I still think this renaming/clarification is worthwhile so I'll still add
something into the series.
Paul
> Paul
>
> > > + return 0;
> > > +}
> > > +
> > [...]
> > > diff --git a/xen/drivers/passthrough/iommu.c
> > b/xen/drivers/passthrough/iommu.c
> > > index fb9d0e1848..c517428621 100644
> > > --- a/xen/drivers/passthrough/iommu.c
> > > +++ b/xen/drivers/passthrough/iommu.c
> > > @@ -26,7 +26,6 @@ static void iommu_dump_p2m_table(unsigned char
> > key);
> > >
> > > unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
> > > integer_param("iommu_dev_iotlb_timeout",
> iommu_dev_iotlb_timeout);
> > > -
> >
> > Please keep the blank line here.
> >
> > Wei.
>
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
