Messages in this thread | | | Date | Fri, 10 Mar 2023 11:55:07 -0400 | From | Jason Gunthorpe <> | Subject | Re: [PATCH v1 01/14] iommu: Add iommu_get_unmanaged_domain helper |
| |
On Fri, Mar 10, 2023 at 09:41:01AM +0100, Eric Auger wrote:
> I tend to agree with Robin here. This was first introduced by > > [PATCH v7 21/22] iommu/dma: Add support for mapping MSIs <https://lore.kernel.org/all/2273af20d844bd618c6a90b57e639700328ebf7f.1473695704.git.robin.murphy@arm.com/#r> > https://lore.kernel.org/all/2273af20d844bd618c6a90b57e639700328ebf7f.1473695704.git.robin.murphy@arm.com/
Presumably it had to use the iommu_get_domain_for_dev() instead of iommu_get_dma_domain() to support ARM 32 arm-iommu. Ie it is poking into the arm-iommu owned domain as well. VFIO just ended being the same flow
> even before the support un VFIO use case which came later on. So > using the "unmanaged" terminology sounds improper to me, at least. > Couldn't we use a parent/child terminology as used in the past in
No objection to a better name...
Actually how about if we write it like this? Robin would you be happier? I think it much more clearly explains why this function is special within our single domain attachment model.
"get_unmanaged_msi_domain" seems like a much more narrowly specific to the purpose name.
int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr) { struct device *dev = msi_desc_to_dev(desc); struct iommu_domain *domain = iommu_get_domain_for_dev(dev); struct iommu_dma_msi_page *msi_page; static DEFINE_MUTEX(msi_prepare_lock); /* see below */
desc->iommu_cookie = NULL;
/* * This probably shouldn't happen as the ARM32 systems should only have * NULL if arm-iommu has been disconnected during setup/destruction. * Assume it is an identity domain. */ if (!domain) return 0;
/* Caller is expected to use msi_addr for the page */ if (domain->type == IOMMU_DOMAIN_IDENTITY) return 0;
/* * The current domain is some driver opaque thing. We assume the * driver/user knows what it is doing regarding ARM ITS MSI pages and we * want to try to install the page into some kind of kernel owned * unmanaged domain. Eg for nesting this will install the ITS page into * the S2 domain and then we assume that the S1 domain has independently * made it mapped at the same address. */ // FIXME wrap in a function if (domain->type != IOMMU_DOMAIN_UNMANAGED && domain->ops->get_unmanged_msi_domain) domain = domain->ops->get_unmanged_msi_domain(domain);
if (!domain || domain->type != IOMMU_DOMAIN_UNMANAGED) return -EINVAL;
// ??? if (!domain->iova_cookie) return 0;
/* * In fact the whole prepare operation should already be serialised by * irq_domain_mutex further up the callchain, but that's pretty subtle * on its own, so consider this locking as failsafe documentation... */ mutex_lock(&msi_prepare_lock); msi_page = iommu_dma_get_msi_page(dev, msi_addr, domain); mutex_unlock(&msi_prepare_lock);
msi_desc_set_iommu_cookie(desc, msi_page);
if (!msi_page) return -ENOMEM; return 0; }
Jason
| |