On Sat, Feb 08 2025 at 01:02, Nicolin Chen wrote:

From: Jason Gunthorpe <jgg@nvidia.com>

All the iommu cases simply want to override the MSI page's address with
the IOVA that was mapped through the iommu. This doesn't need a cookie
pointer, we just need to store the IOVA and its page size in the
msi_desc.

We need to do nothing :)

Instead provide msi_desc_set_iommu_msi_iova() which allows the IOMMU side
to specify the IOVA that the MSI page is placed during
iommu_dma_prepare_msi(). This is stored in the msi_desc and then
iommu_dma_compose_msi_msg() is a simple inline that sets address_hi/lo.

The next patch will correct the naming.

This is done because we cannot correctly lock access to group->domain
in

Now this gets really confusing. How is the next patch which corrects the
naming related the locking problem?

the atomic context that iommu_dma_compose_msi_msg() is called under. Today
the locking miss is tolerable because dma_iommu.c operates under an
assumption that the domain does not change while a driver is probed.

However iommufd now permits the domain to change while the driver is
probed and VFIO userspace can create races with IRQ changes calling
iommu_dma_prepare_msi/compose_msi_msg() and changing/freeing the
iommu_domain.

Removing the pointer, and critically, the call to
iommu_get_domain_for_dev() during compose resolves this race.

So this change log really fails to follow the basic structure:

   The context, the problem and the solution

Something like this:

   The IOMMU translation for MSI message addresses is a two stage
   process:

     1) A cookie containing the IOVA address is stored in the MSI
        descriptor, when a MSI interrupt is allocated

     2) The compose callback uses this cookie to apply the translation
        to the message address.

   This worked so far because .......

   With the iommufd rework this becomes racy, because ...

   Fix this by storing ... instead of ... ....

Hmm?

        tglx