Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops

[Baolu Lu]

Hi Jason,

On 2022/5/16 21:57, Jason Gunthorpe wrote:
> On Mon, May 16, 2022 at 12:22:08PM +0100, Robin Murphy wrote:
> > On 2022-05-16 02:57, Lu Baolu wrote:
> > > Each IOMMU driver must provide a blocking domain ops. If the hardware
> > > supports detaching domain from device, setting blocking domain equals
> > > detaching the existing domain from the deivce. Otherwise, an UNMANAGED
> > > domain without any mapping will be used instead.
> > Unfortunately that's backwards - most of the implementations of .detach_dev
> > are disabling translation entirely, meaning the device ends up effectively
> > in passthrough rather than blocked.
> Ideally we'd convert the detach_dev of every driver into either
> a blocking or identity domain. The trick is knowing which is which..

I am still a bit puzzled about how the blocking_domain should be used 
when it is extended to support ->set_dev_pasid.

If it's a blocking domain, the IOMMU driver knows that setting the
blocking domain to device pasid means detaching the existing one.

But if it's an identity domain, how could the IOMMU driver choose
between:

 - setting the input domain to the pasid on device; or,
 - detaching the existing domain.

I've ever thought about below solutions:

- Checking the domain types and dispatching them to different
  operations.
- Using different blocking domains for different types of domains.

But both look rough.

> Guessing going down the list:
>   apple dart - blocking, detach_dev calls apple_dart_hw_disable_dma() same as
>                IOMMU_DOMAIN_BLOCKED
> 	      [I wonder if this drive ris wrong in other ways though because
>                 I dont see a remove_streams in attach_dev]
>   exynos - this seems to disable the 'sysmmu' so I'm guessing this is
>            identity
>   iommu-vmsa - Comment says 'disable mmu translaction' so I'm guessing
>                this is idenity
>   mkt_v1 - Code looks similar to mkt, which is probably identity.
>   rkt - No idea
>   sprd - No idea
>   sun50i - This driver confusingly treats identity the same as
>            unmanaged, seems wrong, no idea.
>   amd - Not sure, clear_dte_entry() seems to set translation on but points
>         the PTE to 0 ? Based on the spec table 8 I would have expected
>         TV to be clear which would be blocking. Maybe a bug??
>   arm smmu qcomm - not sure
>   intel - blocking
>
> These doesn't support default domains, so detach_dev should return
> back to DMA API ownership, which is either identity or something weird:
>   fsl_pamu - identity due to the PPC use of dma direct
>   msm
>   mkt
>   omap
>   s390 - platform DMA ops
>   terga-gart - Usually something called a GART would be 0 length once
>                disabled, guessing blocking?
>   tegra-smmu
>
> So, the approach here should be to go driver by driver and convert
> detach_dev to either identity, blocking or just delete it entirely,
> excluding the above 7 that don't support default domains. And get acks
> from the driver owners.

Agreed. There seems to be a long way to go. I am wondering if we could
decouple this refactoring from my new SVA API work? We can easily switch
.detach_dev_pasid to using blocking domain later.

Best regards,
baolu