On Thu, Jan 23, 2025 at 06:10:47PM +0100, Eric Auger wrote:
Hi,
On 1/11/25 4:32 AM, Nicolin Chen wrote:
From: Jason Gunthorpe jgg@nvidia.com
SW_MSI supports IOMMU to translate an MSI message before the MSI message is delivered to the interrupt controller. On such systems the iommu_domain must have a translation for the MSI message for interrupts to work.
The IRQ subsystem will call into IOMMU to request that a physical page be setup to receive MSI message, and the IOMMU then sets an IOVA that maps to that physical page. Ultimately the IOVA is programmed into the device via the msi_msg.
Generalize this to allow the iommu_domain owner to provide its own implementation of this mapping. Add a function pointer to struct iommu_domain to allow the domain owner to provide an implementation.
Have dma-iommu supply its implementation for IOMMU_DOMAIN_DMA types during the iommu_get_dma_cookie() path. For IOMMU_DOMAIN_UNMANAGED types used by VFIO (and iommufd for now), have the same iommu_dma_sw_msi set as well in the iommu_get_msi_cookie() path.
Hold the group mutex while in iommu_dma_prepare_msi() to ensure the domain doesn't change or become freed while running. Races with IRQ operations from VFIO and domain changes from iommufd are possible here.
this was my question in previous comments
Ah, well there is the answer :)
Rreplace the msi_prepare_lock with a lockdep assertion for the group mutex
Replace
as documentation. For the dma_iommu.c each iommu_domain unique to a
is?
group.
Yes
Replace the msi_prepare_lock with a lockdep assertion for the group mutex as documentation. For the dmau_iommu.c each iommu_domain is unique to a group.
@@ -443,6 +449,9 @@ void iommu_put_dma_cookie(struct iommu_domain *domain) struct iommu_dma_cookie *cookie = domain->iova_cookie; struct iommu_dma_msi_page *msi, *tmp;
- if (domain->sw_msi != iommu_dma_sw_msi)
return;I don't get the above check.
It is because of this:
void iommu_domain_free(struct iommu_domain *domain) { if (domain->type == IOMMU_DOMAIN_SVA) mmdrop(domain->mm); iommu_put_dma_cookie(domain);
iommufd may be using domain->sw_msi so iommu_put_dma_cookie() needs to be a NOP. Also, later we move cookie into a union so it is not reliably NULL anymore.
The comment says this is also called for a cookie prepared with iommu_get_dma_cookie(). Don't you need to do some cleanup for this latter?
That seems seems OK, only two places set domain->iova_cookie:
int iommu_get_dma_cookie(struct iommu_domain *domain) { domain->iova_cookie = cookie_alloc(IOMMU_DMA_IOVA_COOKIE); iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
and
int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base) { domain->iova_cookie = cookie; iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
So (domain->sw_msi == iommu_dma_sw_msi) in iommu_put_dma_cookie() for both cases..
Jason