Re: [PATCH v4 1/2] iommu/s390: Fix race with release_device ops

Jason Gunthorpe
2022-09-01T15:49:58Z

On Thu, Sep 01, 2022 at 04:03:36PM +0100, Robin Murphy wrote:
> On 2022-09-01 15:34, Jason Gunthorpe wrote:
> > On Thu, Sep 01, 2022 at 03:29:16PM +0100, Robin Murphy wrote:
> >
> > > Right, the next step would be to bridge that gap to iommu-dma to dump the
> > > flush queue when IOVA allocation failure implies we've reached the
> > > "rollover" point, and perhaps not use the timer at all. By that point a
> > > dedicated domain type, or at least some definite internal flag, for this
> > > alternate behaviour seems like the logical way to go.
> >
> > At least on this direction, I've been thinking it would be nice to
> > replace the domain type _FQ with a flag inside the domain, maybe the
> > ops, saying how the domain wants the common DMA API to operate. If it
> > wants FQ mode or other tuning parameters
>
> Compare the not-necessarily-obvious matrix of "strict" and "passthrough"
> command-line parameters with the nice understandable kconfig and sysfs
> controls for a reminder of why I moved things *from* that paradigm in the
> first place ;)

I'm looking at it from a code perspective, where the drivers don't
seem to actually care about DMA_FQ. eg search for it in the drivers
and you mostly see:

(type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_DMA_FQ))

The exception is domain_alloc which fails in cases where the domain
doesn't 'support' FQ.

But support FQ or not can be cast as a simple capability flag in the
domain. We don't need a whole type for the driver to communicate it.

The strictness level belongs completely in the core code, it shouldn't
leak into the driver.

The same general comment seems to be true of IOMMU_DOMAIN_DMA. All the
drivers implement this as an UNMANAGED domain. There are only two
places in the Intel driver that do anything special with DOMAIN_DMA vs
DOMAIN_UNMANAGED (and possibly that is just cruft). So the "does this
support DMA API" is also just a capability flag, and doesn't really
need a whole type.

This is what I mean, not going backwards to the driver specifying
strictness policy.

Jason