Re: [PATCH kernel] powerpc/iommu: Add iommu_ops to report capabilities and allow blocking domains

On 10/07/2022 22:32, Alexey Kardashevskiy wrote:
> 
> 
> On 10/07/2022 16:29, Jason Gunthorpe wrote:
>> On Sat, Jul 09, 2022 at 12:58:00PM +1000, Alexey Kardashevskiy wrote:
>>> driver->ops->attach_group on POWER attaches a group so VFIO claims 
>>> ownership
>>> over a group, not devices. Underlying API 
>>> (pnv_ioda2_take_ownership()) does
>>> not need to keep track of the state, it is one group, one ownership
>>> transfer, easy.
>>
>> It should not change, I think you can just map the attach_dev to the 
>> group?
> 
> There are multiple devices in a group, cannot just map 1:1.
> 
> 
>>> What is exactly the reason why iommu_group_claim_dma_owner() cannot stay
>>> inside Type1 (sorry if it was explained, I could have missed)?
>>
>> It has nothing to do with type1 - the ownership system is designed to
>> exclude other in-kernel drivers from using the group at the same time
>> vfio is using the group. power still needs this protection regardless
>> of if is using the formal iommu api or not.
> 
> POWER deals with it in vfio_iommu_driver_ops::attach_group.


I really think that for 5.19 we should really move this blocked domain 
business to Type1 like this:

https://github.com/aik/linux/commit/96f80c8db03b181398ad355f6f90e574c3ada4bf

Thanks,


>>> Also, from another mail, you said iommu_alloc_default_domain() should 
>>> fail
>>> on power but at least IOMMU_DOMAIN_BLOCKED must be supported, or the 
>>> whole
>>> iommu_group_claim_dma_owner() thing falls apart.
>>
>> Yes
>>
>>> And iommu_ops::domain_alloc() is not told if it is asked to create a 
>>> default
>>> domain, it only takes a type.
>>
>> "default domain" refers to the default type pased to domain_alloc(),
>> it will never be blocking, so it will always fail on power.
>> "default domain" is better understood as the domain used by the DMA
>> API
> 
> The DMA API on POWER does not use iommu_ops, it is dma_iommu_ops from 
> arch/powerpc/kernel/dma-iommu.c from before 2005. so the default domain 
> is type = 0 where 0 = BLOCKED.
> 

-- 
Alexey