Messages in this thread | | | Date | Fri, 8 Dec 2023 09:42:02 +0800 | Subject | Re: [PATCH 1/1] iommu: Set owner token to sva and nested domains | From | Baolu Lu <> |
| |
On 12/7/23 9:36 PM, Jason Gunthorpe wrote: > On Thu, Dec 07, 2023 at 09:56:10AM +0000, Robin Murphy wrote: >> On 2023-12-07 2:19 am, Lu Baolu wrote: >>> Commit a9c362db3920 ("iommu: Validate that devices match domains") added >>> an owner token to an iommu_domain. This token is checked during domain >>> attachment to RID or PASID through the generic iommu interfaces. >>> >>> The sva and nested domains are attached to device or PASID through the >>> generic iommu interfaces. Therefore, they require the owner token to be >>> set during allocation. Otherwise, they fail to attach. >> Oops, I missed that iommu_sva_domain_alloc() is a thing - when did we get >> such a confusing proliferation of domain allocation paths? Sigh... > We have alot of different kinds of domains now, APIs that are giant > multiplexers are not good. > > What I've been wanting to do for a while is to have the drivers call a > helper to allocate their domain struct and the helper would initialize > the common iommu_domain instead of doing this after the op > returns. This is more typical kernel pattern and avoids some of the > confusion about when struct members are valid or not (notice some of > driver code needs iommu_domain stuff set earlier and we confusingly > initialize things twice :() > >> I think we should set the owner generically there, since presumably it's >> being missed for SMMUv3/AMD/etc. SVA domains as well. Nested domains are >> supposed to be OK since both ->domain_alloc_user callsites are covered, or >> is there some other sneaky path I've also missed? > Indeed, I also think the first hunk is not needed, the second hunk was > missed.
Oh, yes! I overlooked that iommufd has already done that for nested domain. I will update it with a v2.
Best regards, baolu
| |