Re: [PATCH v1 3/8] iommu: Extend iommu_at[de]tach_device() for multi-device groups

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Feb 14, 2022 at 12:39:36PM +0100, Joerg Roedel wrote:

> This extends iommu_attach_device() to behave as iommu_attach_group(),
> changing the domain for the whole group. 

Of course, the only action to take is to change the domain of a
group..

> Wouldn't it be better to scrap the iommu_attach_device() interface
> instead and only rely on iommu_attach_group()? This way it is clear
> that a call changes the whole group.

From an API design perspective drivers should never touch groups -
they have struct devices, they should have a clean struct device based
API.

Groups should disappear into an internal implementation detail, not be
so prominent in the API.

> IIUC this work is heading towards allowing multiple domains in one group
> as long as the group is owned by one entity.

No, it isn't. This work is only about properly arbitrating which
single domain is attached to an entire group.

> 	1) Introduce a concept of a sub-group (or whatever we want to
> 	   call it), which groups devices together which must be in the
> 	   same domain because they use the same request ID and thus
> 	   look all the same to the IOMMU.
>
> 	2) Keep todays IOMMU groups to group devices together which can
> 	   bypass the IOMMU when talking to each other, like
> 	   multi-function devices and devices behind a no-ACS bridge.

We've talked about all these details before and nobody has thought
they are important enough to implement. This distinction is not the
goal of this series.

I think if someone did want to do this there is room in the API to
allow the distinction between 1 (must share) and 2 (sharing is
insecure). eg by checking owner and blocking mixing user/kernel. 

This is another reason to stick with the device centric API as if we
did someday want multi-domain groups then the device input is still
the correct input and the iommu code can figure out what sub-groups or
whatever transparently.

Jason