On 08/07/2022 23:19, Jason Gunthorpe wrote:

On Fri, Jul 08, 2022 at 11:10:07PM +1000, Alexey Kardashevskiy wrote:

quoted

On 08/07/2022 21:55, Jason Gunthorpe wrote:

quoted

On Fri, Jul 08, 2022 at 04:34:55PM +1000, Alexey Kardashevskiy wrote:

quoted

For now I'll add a comment in spapr_tce_iommu_attach_dev() that it is fine
to do nothing as tce_iommu_take_ownership() and
tce_iommu_take_ownership_ddw() take care of not having active DMA mappings.

That will still cause a security problem because
tce_iommu_take_ownership()/etc are called too late. This is the moment
in the flow when the ownershift must change away from the DMA API that
power implements and to VFIO, not later.

It is getting better and better :)

On POWERNV, at the boot time the platforms sets up PHBs, enables bypass,
creates groups and attaches devices. As for now attaching devices to the
default domain (which is BLOCKED) fails the not-being-use check as enabled
bypass means "everything is mapped for DMA". So at this point the default
domain has to be IOMMU_DOMAIN_IDENTITY or IOMMU_DOMAIN_UNMANAGED so later on
VFIO can move devices to IOMMU_DOMAIN_BLOCKED. Am I missing something?

For power the default domain should be NULL

NULL means that the platform is using the group to provide its DMA
ops. IIRC this patch was already setup correctly to do this?

The transition from NULL to blocking must isolate the group so all DMA
is blocked. blocking to NULL should re-estbalish platform DMA API
control.

The default domain should be non-NULL when the normal dma-iommu stuff is
providing the DMA API.

So, I think it is already setup properly, it is just the question of
what to do when entering/leaving blocking mode.

Well, the patch calls iommu_probe_device() which calls 
iommu_alloc_default_domain() which creates IOMMU_DOMAIN_BLOCKED (==0) as 
nothing initialized iommu_def_domain_type. Need a different default type 
(and return NULL when IOMMU API tries creating this type)?

Jason

-- 
Alexey