Re: [RFC PATCH 1/3] iommu: introduce IOMMU_DOMAIN_HYP domain type for hypervisor allocation

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

On Wed, Jul 02, 2014 at 04:04:15PM +0100, Alex Williamson wrote:
> On Wed, 2014-07-02 at 14:57 +0100, Will Deacon wrote:
> > On Wed, Jul 02, 2014 at 11:49:02AM +0100, Will Deacon wrote:
> > > On Tue, Jul 01, 2014 at 08:28:11PM +0100, Alex Williamson wrote:
> > > > If we go the route of defining it at allocation time, I'd probably think
> > > > about adding a new interface, like:
> > > > 
> > > > iommu_domain_alloc_with_features(struct bus_type *bus, u32 features)
> > > > 
> > > > Where features is a bitmap
> > > > 
> > > > #define IOMMU_DOMAIN_FEATURE_NESTING (1U << 0)
> > > > 
> > > > iommu_domain_alloc(struct bus_type *bus) would then just be a wrapper
> > > > with features = 0.  If an IOMMU driver doesn't support a requested
> > > > feature, it should fail so we don't have cases like KVM requesting a
> > > > "HYP" domain when it doesn't need it and the IOMMU drivers don't support
> > > > it.
> > > > 
> > > > It would be a lot less intrusive if we could use iommu_domain_set_attr()
> > > > to enable nesting after allocation though.  This could return error if
> > > > called after the point at which it can be easily enabled.
> > > 
> > > I'll take another look at set_attr, since I agree that it would be cleaner.
> > 
> > Ok, we quickly end up with a bootstrap problem here:
> > 
> >   - We need to know whether the domain is stage-1 or stage-2 before we've
> >     attached any devices to it (since we can have DMA traffic once a
> >     device is attached).
> > 
> >   - Until a device is attached, we don't know the specific SMMU instance
> >     backing the domain.
> > 
> >   - Until we know the SMMU, we don't know whether or not we can do
> >     nesting, and therefore can't handle set_attr calls.
> > 
> > So the two places we can provide this information are either at domain_alloc
> > time or at device_attach time. I think the former makes more sense, so I'll
> > look at adding a new alloc function as you suggest above.
> 
> I don't really see how using set_attr to set a domain parameter is any
> different than specifying it at domain_alloc.  "Create a domain, set it
> to require nesting, add devices" versus "Create a domain and set it to
> require nesting, add devices".  Thanks,

There's a difference in where you detect the error, but actually, that's all
under the same VFIO ioctl so I think you're right.

I'll put together a patch.

Will