[PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Lu Baolu]

The idxd driver attaches the default domain to a PASID of the device to
perform kernel DMA using that PASID. The domain is attached to the
device's PASID through iommu_attach_device_pasid(), which checks if the
domain->owner matches the iommu_ops retrieved from the device. If they
do not match, it returns a failure.

        if (ops != domain->owner || pasid == IOMMU_NO_PASID)
                return -EINVAL;

The static identity domain implemented by the intel iommu driver doesn't
specify the domain owner. Therefore, kernel DMA with PASID doesn't work
for the idxd driver if the device translation mode is set to passthrough.

Generally the owner field of static domains are not set because they are
already part of iommu ops. Add a helper domain_iommu_ops_compatible()
that checks if a domain is compatible with the device's iommu ops. This
helper explicitly allows the static blocked and identity domains associated
with the device's iommu_ops to be considered compatible.

Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220031
Cc: stable@vger.kernel.org
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/linux-iommu/20250422191554.GC1213339@ziepe.ca/
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/iommu.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Change log:
v3:
 - Convert all places checking domain->owner to the new helper.
v2: https://lore.kernel.org/linux-iommu/20250423021839.2189204-1-baolu.lu@linux.intel.com/
 - Make the solution generic for all static domains as suggested by
   Jason.
v1: https://lore.kernel.org/linux-iommu/20250422075422.2084548-1-baolu.lu@linux.intel.com/

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 4f91a740c15f..b26fc3ed9f01 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2204,6 +2204,19 @@ static void *iommu_make_pasid_array_entry(struct iommu_domain *domain,
 	return xa_tag_pointer(domain, IOMMU_PASID_ARRAY_DOMAIN);
 }
 
+static bool domain_iommu_ops_compatible(const struct iommu_ops *ops,
+					struct iommu_domain *domain)
+{
+	if (domain->owner == ops)
+		return true;
+
+	/* For static domains, owner isn't set. */
+	if (domain == ops->blocked_domain || domain == ops->identity_domain)
+		return true;
+
+	return false;
+}
+
 static int __iommu_attach_group(struct iommu_domain *domain,
 				struct iommu_group *group)
 {
@@ -2214,7 +2227,8 @@ static int __iommu_attach_group(struct iommu_domain *domain,
 		return -EBUSY;
 
 	dev = iommu_group_first_dev(group);
-	if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain->owner)
+	if (!dev_has_iommu(dev) ||
+	    !domain_iommu_ops_compatible(dev_iommu_ops(dev), domain))
 		return -EINVAL;
 
 	return __iommu_group_set_domain(group, domain);
@@ -3435,7 +3449,8 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
 	    !ops->blocked_domain->ops->set_dev_pasid)
 		return -EOPNOTSUPP;
 
-	if (ops != domain->owner || pasid == IOMMU_NO_PASID)
+	if (!domain_iommu_ops_compatible(ops, domain) ||
+	    pasid == IOMMU_NO_PASID)
 		return -EINVAL;
 
 	mutex_lock(&group->mutex);
@@ -3511,7 +3526,7 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
 	if (!domain->ops->set_dev_pasid)
 		return -EOPNOTSUPP;
 
-	if (dev_iommu_ops(dev) != domain->owner ||
+	if (!domain_iommu_ops_compatible(dev_iommu_ops(dev), domain) ||
 	    pasid == IOMMU_NO_PASID || !handle)
 		return -EINVAL;
 
-- 
2.43.0

RE: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Tian, Kevin]

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Thursday, April 24, 2025 11:41 AM
> 
> The idxd driver attaches the default domain to a PASID of the device to
> perform kernel DMA using that PASID. The domain is attached to the
> device's PASID through iommu_attach_device_pasid(), which checks if the
> domain->owner matches the iommu_ops retrieved from the device. If they
> do not match, it returns a failure.
> 
>         if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>                 return -EINVAL;
> 
> The static identity domain implemented by the intel iommu driver doesn't
> specify the domain owner. Therefore, kernel DMA with PASID doesn't work
> for the idxd driver if the device translation mode is set to passthrough.
> 
> Generally the owner field of static domains are not set because they are
> already part of iommu ops. Add a helper domain_iommu_ops_compatible()
> that checks if a domain is compatible with the device's iommu ops. This
> helper explicitly allows the static blocked and identity domains associated
> with the device's iommu_ops to be considered compatible.
> 
> Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220031
> Cc: stable@vger.kernel.org
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Link: https://lore.kernel.org/linux-
> iommu/20250422191554.GC1213339@ziepe.ca/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Re: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Vasant Hegde]

On 4/24/2025 9:11 AM, Lu Baolu wrote:
> The idxd driver attaches the default domain to a PASID of the device to
> perform kernel DMA using that PASID. The domain is attached to the
> device's PASID through iommu_attach_device_pasid(), which checks if the
> domain->owner matches the iommu_ops retrieved from the device. If they
> do not match, it returns a failure.
> 
>         if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>                 return -EINVAL;
> 
> The static identity domain implemented by the intel iommu driver doesn't
> specify the domain owner. Therefore, kernel DMA with PASID doesn't work
> for the idxd driver if the device translation mode is set to passthrough.
> 
> Generally the owner field of static domains are not set because they are
> already part of iommu ops. Add a helper domain_iommu_ops_compatible()
> that checks if a domain is compatible with the device's iommu ops. This
> helper explicitly allows the static blocked and identity domains associated
> with the device's iommu_ops to be considered compatible.
> 
> Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220031
> Cc: stable@vger.kernel.org
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Link: https://lore.kernel.org/linux-iommu/20250422191554.GC1213339@ziepe.ca/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>


Thanks! We have static identity domain in AMD driver as well. Some day we may
hit similar issue :-)

Patch looks good to me.

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

W/ this change may be I should fix AMD driver like below. (No need to respin
patch. I can send topup patch later).


diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index be8761bbef0f..247b8fcb3a92 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2626,7 +2626,6 @@ void amd_iommu_init_identity_domain(void)

 	domain->type = IOMMU_DOMAIN_IDENTITY;
 	domain->ops = &identity_domain_ops;
-	domain->owner = &amd_iommu_ops;

 	identity_domain.id = pdom_id_alloc();


-Vasant

Re: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Baolu Lu]

On 4/24/25 14:49, Vasant Hegde wrote:
> On 4/24/2025 9:11 AM, Lu Baolu wrote:
>> The idxd driver attaches the default domain to a PASID of the device to
>> perform kernel DMA using that PASID. The domain is attached to the
>> device's PASID through iommu_attach_device_pasid(), which checks if the
>> domain->owner matches the iommu_ops retrieved from the device. If they
>> do not match, it returns a failure.
>>
>>          if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>>                  return -EINVAL;
>>
>> The static identity domain implemented by the intel iommu driver doesn't
>> specify the domain owner. Therefore, kernel DMA with PASID doesn't work
>> for the idxd driver if the device translation mode is set to passthrough.
>>
>> Generally the owner field of static domains are not set because they are
>> already part of iommu ops. Add a helper domain_iommu_ops_compatible()
>> that checks if a domain is compatible with the device's iommu ops. This
>> helper explicitly allows the static blocked and identity domains associated
>> with the device's iommu_ops to be considered compatible.
>>
>> Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
>> Closes:https://bugzilla.kernel.org/show_bug.cgi?id=220031
>> Cc:stable@vger.kernel.org
>> Suggested-by: Jason Gunthorpe<jgg@nvidia.com>
>> Link:https://lore.kernel.org/linux-iommu/20250422191554.GC1213339@ziepe.ca/
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>> Reviewed-by: Dave Jiang<dave.jiang@intel.com>
>> Reviewed-by: Robin Murphy<robin.murphy@arm.com>
> 
> Thanks! We have static identity domain in AMD driver as well. Some day we may
> hit similar issue 🙂
> 
> Patch looks good to me.
> 
> Reviewed-by: Vasant Hegde<vasant.hegde@amd.com>

Thank you!

> 
> W/ this change may be I should fix AMD driver like below. (No need to respin
> patch. I can send topup patch later).

I think that's a cleanup rather than a fix.

Thanks,
baolu

Re: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Jason Gunthorpe]

On Thu, Apr 24, 2025 at 11:41:23AM +0800, Lu Baolu wrote:
> The idxd driver attaches the default domain to a PASID of the device to
> perform kernel DMA using that PASID. The domain is attached to the
> device's PASID through iommu_attach_device_pasid(), which checks if the
> domain->owner matches the iommu_ops retrieved from the device. If they
> do not match, it returns a failure.
> 
>         if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>                 return -EINVAL;
> 
> The static identity domain implemented by the intel iommu driver doesn't
> specify the domain owner. Therefore, kernel DMA with PASID doesn't work
> for the idxd driver if the device translation mode is set to passthrough.
> 
> Generally the owner field of static domains are not set because they are
> already part of iommu ops. Add a helper domain_iommu_ops_compatible()
> that checks if a domain is compatible with the device's iommu ops. This
> helper explicitly allows the static blocked and identity domains associated
> with the device's iommu_ops to be considered compatible.
> 
> Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220031
> Cc: stable@vger.kernel.org
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Link: https://lore.kernel.org/linux-iommu/20250422191554.GC1213339@ziepe.ca/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/iommu/iommu.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

Re: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Dave Jiang]

On 4/24/25 3:34 PM, Jack Vogel wrote:
> I am having test issues with this patch, test system is running OL9, basically RHEL 9.5, the kernel boots ok, and the dmesg is clean… but the tests in accel-config dont pass. Specifically the crypto tests, this is due to vfio_pci_core not loading.  Right now I’m not sure if any of this is my mistake, but at least it’s something I need to keep looking at.
> 
> Also, since I saw that issue on the latest I did a backport to our UEK8 kernel which is 6.12.23, and on that kernel it still exhibited  these messages on boot:
> 
> *idxd*0000:6a:01.0: enabling device (0144 -> 0146)
> 
> [   21.112733] *idxd*0000:6a:01.0: failed to attach device pasid 1, domain type 4
> 
> [   21.120770] *idxd*0000:6a:01.0: No in-kernel DMA with PASID. -95
> 
> 
> Again, maybe an issue in my backporting… however I’d like to be sure.

Can you verify against latest upstream kernel plus the patch and see if you still see the error?

DJ

> 
> Cheers,
> 
> Jack
> 
> 
>> On Apr 23, 2025, at 20:41, Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>
>> The idxd driver attaches the default domain to a PASID of the device to
>> perform kernel DMA using that PASID. The domain is attached to the
>> device's PASID through iommu_attach_device_pasid(), which checks if the
>> domain->owner matches the iommu_ops retrieved from the device. If they
>> do not match, it returns a failure.
>>
>>        if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>>                return -EINVAL;
>>
>> The static identity domain implemented by the intel iommu driver doesn't
>> specify the domain owner. Therefore, kernel DMA with PASID doesn't work
>> for the idxd driver if the device translation mode is set to passthrough.
>>
>> Generally the owner field of static domains are not set because they are
>> already part of iommu ops. Add a helper domain_iommu_ops_compatible()
>> that checks if a domain is compatible with the device's iommu ops. This
>> helper explicitly allows the static blocked and identity domains associated
>> with the device's iommu_ops to be considered compatible.
>>
>> Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220031
>> Cc: stable@vger.kernel.org
>> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
>> Link: https://lore.kernel.org/linux-iommu/20250422191554.GC1213339@ziepe.ca/
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>> drivers/iommu/iommu.c | 21 ++++++++++++++++++---
>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> Change log:
>> v3:
>> - Convert all places checking domain->owner to the new helper.
>> v2: https://lore.kernel.org/linux-iommu/20250423021839.2189204-1-baolu.lu@linux.intel.com/
>> - Make the solution generic for all static domains as suggested by
>>   Jason.
>> v1: https://lore.kernel.org/linux-iommu/20250422075422.2084548-1-baolu.lu@linux.intel.com/
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 4f91a740c15f..b26fc3ed9f01 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -2204,6 +2204,19 @@ static void *iommu_make_pasid_array_entry(struct iommu_domain *domain,
>> return xa_tag_pointer(domain, IOMMU_PASID_ARRAY_DOMAIN);
>> }
>>
>> +static bool domain_iommu_ops_compatible(const struct iommu_ops *ops,
>> +struct iommu_domain *domain)
>> +{
>> +if (domain->owner == ops)
>> +return true;
>> +
>> +/* For static domains, owner isn't set. */
>> +if (domain == ops->blocked_domain || domain == ops->identity_domain)
>> +return true;
>> +
>> +return false;
>> +}
>> +
>> static int __iommu_attach_group(struct iommu_domain *domain,
>> struct iommu_group *group)
>> {
>> @@ -2214,7 +2227,8 @@ static int __iommu_attach_group(struct iommu_domain *domain,
>> return -EBUSY;
>>
>> dev = iommu_group_first_dev(group);
>> -if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain->owner)
>> +if (!dev_has_iommu(dev) ||
>> +   !domain_iommu_ops_compatible(dev_iommu_ops(dev), domain))
>> return -EINVAL;
>>
>> return __iommu_group_set_domain(group, domain);
>> @@ -3435,7 +3449,8 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
>>    !ops->blocked_domain->ops->set_dev_pasid)
>> return -EOPNOTSUPP;
>>
>> -if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>> +if (!domain_iommu_ops_compatible(ops, domain) ||
>> +   pasid == IOMMU_NO_PASID)
>> return -EINVAL;
>>
>> mutex_lock(&group->mutex);
>> @@ -3511,7 +3526,7 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
>> if (!domain->ops->set_dev_pasid)
>> return -EOPNOTSUPP;
>>
>> -if (dev_iommu_ops(dev) != domain->owner ||
>> +if (!domain_iommu_ops_compatible(dev_iommu_ops(dev), domain) ||
>>    pasid == IOMMU_NO_PASID || !handle)
>> return -EINVAL;
>>
>> -- 
>> 2.43.0
>>
> 

Re: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Dave Jiang]

On 4/24/25 3:59 PM, Jack Vogel wrote:
> 
> 
>> On Apr 24, 2025, at 15:40, Dave Jiang <dave.jiang@intel.com> wrote:
>>
>>
>>
>> On 4/24/25 3:34 PM, Jack Vogel wrote:
>>> I am having test issues with this patch, test system is running OL9, basically RHEL 9.5, the kernel boots ok, and the dmesg is clean… but the tests in accel-config dont pass. Specifically the crypto tests, this is due to vfio_pci_core not loading.  Right now I’m not sure if any of this is my mistake, but at least it’s something I need to keep looking at.
>>>
>>> Also, since I saw that issue on the latest I did a backport to our UEK8 kernel which is 6.12.23, and on that kernel it still exhibited  these messages on boot:
>>>
>>> *idxd*0000:6a:01.0: enabling device (0144 -> 0146)
>>>
>>> [   21.112733] *idxd*0000:6a:01.0: failed to attach device pasid 1, domain type 4
>>>
>>> [   21.120770] *idxd*0000:6a:01.0: No in-kernel DMA with PASID. -95
>>>
>>>
>>> Again, maybe an issue in my backporting… however I’d like to be sure.
>>
>> Can you verify against latest upstream kernel plus the patch and see if you still see the error?
>>
>> DJ
> 
> Yes, the kernel was build from the tip this morning. Like I said, it got no messages booting up, all looked fine. But when running the actual test suite in the accel-config tarball specifically the iaa crypt tests, they failed and the dmesg was from vfio_pci_core failed to load with an unknown symbol.

I'm not sure what the test consists of (haven't worked on this device for almost 2 years). But usually the device is either bound to the idxd driver or the vfio_pci driver. Not both. And if the idxd driver didn't emit any errors while loading, then the test failure may be something else...

Another way to verify is to set CONFIG_IOMMU_DEFAULT_DMA_LAZY vs PASSTHROUGH. If the tests still fail then it's something else. 

DJ

> 
> This sounds like the module was wrong, but i would think it was installed with the v6.15 kernel….. 
> 
> Jack
> 
>>
>>>
>>> Cheers,
>>>
>>> Jack
>>>
>>>
>>>> On Apr 23, 2025, at 20:41, Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>>>
>>>> The idxd driver attaches the default domain to a PASID of the device to
>>>> perform kernel DMA using that PASID. The domain is attached to the
>>>> device's PASID through iommu_attach_device_pasid(), which checks if the
>>>> domain->owner matches the iommu_ops retrieved from the device. If they
>>>> do not match, it returns a failure.
>>>>
>>>>        if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>>>>                return -EINVAL;
>>>>
>>>> The static identity domain implemented by the intel iommu driver doesn't
>>>> specify the domain owner. Therefore, kernel DMA with PASID doesn't work
>>>> for the idxd driver if the device translation mode is set to passthrough.
>>>>
>>>> Generally the owner field of static domains are not set because they are
>>>> already part of iommu ops. Add a helper domain_iommu_ops_compatible()
>>>> that checks if a domain is compatible with the device's iommu ops. This
>>>> helper explicitly allows the static blocked and identity domains associated
>>>> with the device's iommu_ops to be considered compatible.
>>>>
>>>> Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220031
>>>> Cc: stable@vger.kernel.org
>>>> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
>>>> Link: https://lore.kernel.org/linux-iommu/20250422191554.GC1213339@ziepe.ca/
>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>>> ---
>>>> drivers/iommu/iommu.c | 21 ++++++++++++++++++---
>>>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>>>
>>>> Change log:
>>>> v3:
>>>> - Convert all places checking domain->owner to the new helper.
>>>> v2: https://lore.kernel.org/linux-iommu/20250423021839.2189204-1-baolu.lu@linux.intel.com/
>>>> - Make the solution generic for all static domains as suggested by
>>>>   Jason.
>>>> v1: https://lore.kernel.org/linux-iommu/20250422075422.2084548-1-baolu.lu@linux.intel.com/
>>>>
>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>>> index 4f91a740c15f..b26fc3ed9f01 100644
>>>> --- a/drivers/iommu/iommu.c
>>>> +++ b/drivers/iommu/iommu.c
>>>> @@ -2204,6 +2204,19 @@ static void *iommu_make_pasid_array_entry(struct iommu_domain *domain,
>>>> return xa_tag_pointer(domain, IOMMU_PASID_ARRAY_DOMAIN);
>>>> }
>>>>
>>>> +static bool domain_iommu_ops_compatible(const struct iommu_ops *ops,
>>>> +struct iommu_domain *domain)
>>>> +{
>>>> +if (domain->owner == ops)
>>>> +return true;
>>>> +
>>>> +/* For static domains, owner isn't set. */
>>>> +if (domain == ops->blocked_domain || domain == ops->identity_domain)
>>>> +return true;
>>>> +
>>>> +return false;
>>>> +}
>>>> +
>>>> static int __iommu_attach_group(struct iommu_domain *domain,
>>>> struct iommu_group *group)
>>>> {
>>>> @@ -2214,7 +2227,8 @@ static int __iommu_attach_group(struct iommu_domain *domain,
>>>> return -EBUSY;
>>>>
>>>> dev = iommu_group_first_dev(group);
>>>> -if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain->owner)
>>>> +if (!dev_has_iommu(dev) ||
>>>> +   !domain_iommu_ops_compatible(dev_iommu_ops(dev), domain))
>>>> return -EINVAL;
>>>>
>>>> return __iommu_group_set_domain(group, domain);
>>>> @@ -3435,7 +3449,8 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
>>>>    !ops->blocked_domain->ops->set_dev_pasid)
>>>> return -EOPNOTSUPP;
>>>>
>>>> -if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>>>> +if (!domain_iommu_ops_compatible(ops, domain) ||
>>>> +   pasid == IOMMU_NO_PASID)
>>>> return -EINVAL;
>>>>
>>>> mutex_lock(&group->mutex);
>>>> @@ -3511,7 +3526,7 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
>>>> if (!domain->ops->set_dev_pasid)
>>>> return -EOPNOTSUPP;
>>>>
>>>> -if (dev_iommu_ops(dev) != domain->owner ||
>>>> +if (!domain_iommu_ops_compatible(dev_iommu_ops(dev), domain) ||
>>>>    pasid == IOMMU_NO_PASID || !handle)
>>>> return -EINVAL;
>>>>
>>>> -- 
>>>> 2.43.0
> 

Re: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Dave Jiang]

On 4/24/25 5:55 PM, Jack Vogel wrote:
> 
> 
>> On Apr 24, 2025, at 16:15, Dave Jiang <dave.jiang@intel.com> wrote:
>>
>>
>>
>> On 4/24/25 3:59 PM, Jack Vogel wrote:
>>>
>>>
>>>> On Apr 24, 2025, at 15:40, Dave Jiang <dave.jiang@intel.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/24/25 3:34 PM, Jack Vogel wrote:
>>>>> I am having test issues with this patch, test system is running OL9, basically RHEL 9.5, the kernel boots ok, and the dmesg is clean… but the tests in accel-config dont pass. Specifically the crypto tests, this is due to vfio_pci_core not loading.  Right now I’m not sure if any of this is my mistake, but at least it’s something I need to keep looking at.
>>>>>
>>>>> Also, since I saw that issue on the latest I did a backport to our UEK8 kernel which is 6.12.23, and on that kernel it still exhibited  these messages on boot:
>>>>>
>>>>> *idxd*0000:6a:01.0: enabling device (0144 -> 0146)
>>>>>
>>>>> [   21.112733] *idxd*0000:6a:01.0: failed to attach device pasid 1, domain type 4
>>>>>
>>>>> [   21.120770] *idxd*0000:6a:01.0: No in-kernel DMA with PASID. -95
>>>>>
>>>>>
>>>>> Again, maybe an issue in my backporting… however I’d like to be sure.
>>>>
>>>> Can you verify against latest upstream kernel plus the patch and see if you still see the error?
>>>>
>>>> DJ
>>>
>>> Yes, the kernel was build from the tip this morning. Like I said, it got no messages booting up, all looked fine. But when running the actual test suite in the accel-config tarball specifically the iaa crypt tests, they failed and the dmesg was from vfio_pci_core failed to load with an unknown symbol.
>>
>> I'm not sure what the test consists of (haven't worked on this device for almost 2 years). But usually the device is either bound to the idxd driver or the vfio_pci driver. Not both. And if the idxd driver didn't emit any errors while loading, then the test failure may be something else...
>>
>> Another way to verify is to set CONFIG_IOMMU_DEFAULT_DMA_LAZY vs PASSTHROUGH. If the tests still fail then it's something else. 
>>
>> DJ
> 
> There isn’t a lot of ways to test this driver, yes DPDK will use it, but apart from that? So, the tests that are part of your (Intel) accel-config package are the only convenient way that I’ve found to do so. It is also convenient, there is a “make check” target in the top Makefile that will invoke both set of DMA tests, and some crypto (IAA) tests. I have been planning to give this to our QA group as a verification suite. Do you have an alternative to this?

This should be the right test package. Let me talk to our QA people and see if there are any issues. We can resolve this off list. If there's any issues that end up pointing to the original bug, we can raise that then. 

DJ

> 
> Jack
> 
>>
>>>
>>> This sounds like the module was wrong, but i would think it was installed with the v6.15 kernel….. 
>>>
>>> Jack
>>>
>>>>
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Jack
>>>>>
>>>>>
>>>>>> On Apr 23, 2025, at 20:41, Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>>>>>
>>>>>> The idxd driver attaches the default domain to a PASID of the device to
>>>>>> perform kernel DMA using that PASID. The domain is attached to the
>>>>>> device's PASID through iommu_attach_device_pasid(), which checks if the
>>>>>> domain->owner matches the iommu_ops retrieved from the device. If they
>>>>>> do not match, it returns a failure.
>>>>>>
>>>>>>        if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>>>>>>                return -EINVAL;
>>>>>>
>>>>>> The static identity domain implemented by the intel iommu driver doesn't
>>>>>> specify the domain owner. Therefore, kernel DMA with PASID doesn't work
>>>>>> for the idxd driver if the device translation mode is set to passthrough.
>>>>>>
>>>>>> Generally the owner field of static domains are not set because they are
>>>>>> already part of iommu ops. Add a helper domain_iommu_ops_compatible()
>>>>>> that checks if a domain is compatible with the device's iommu ops. This
>>>>>> helper explicitly allows the static blocked and identity domains associated
>>>>>> with the device's iommu_ops to be considered compatible.
>>>>>>
>>>>>> Fixes: 2031c469f816 ("iommu/vt-d: Add support for static identity domain")
>>>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220031
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
>>>>>> Link: https://lore.kernel.org/linux-iommu/20250422191554.GC1213339@ziepe.ca/
>>>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>>>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>>>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>>>>> ---
>>>>>> drivers/iommu/iommu.c | 21 ++++++++++++++++++---
>>>>>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> Change log:
>>>>>> v3:
>>>>>> - Convert all places checking domain->owner to the new helper.
>>>>>> v2: https://lore.kernel.org/linux-iommu/20250423021839.2189204-1-baolu.lu@linux.intel.com/
>>>>>> - Make the solution generic for all static domains as suggested by
>>>>>>   Jason.
>>>>>> v1: https://lore.kernel.org/linux-iommu/20250422075422.2084548-1-baolu.lu@linux.intel.com/
>>>>>>
>>>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>>>>> index 4f91a740c15f..b26fc3ed9f01 100644
>>>>>> --- a/drivers/iommu/iommu.c
>>>>>> +++ b/drivers/iommu/iommu.c
>>>>>> @@ -2204,6 +2204,19 @@ static void *iommu_make_pasid_array_entry(struct iommu_domain *domain,
>>>>>> return xa_tag_pointer(domain, IOMMU_PASID_ARRAY_DOMAIN);
>>>>>> }
>>>>>>
>>>>>> +static bool domain_iommu_ops_compatible(const struct iommu_ops *ops,
>>>>>> +struct iommu_domain *domain)
>>>>>> +{
>>>>>> +if (domain->owner == ops)
>>>>>> +return true;
>>>>>> +
>>>>>> +/* For static domains, owner isn't set. */
>>>>>> +if (domain == ops->blocked_domain || domain == ops->identity_domain)
>>>>>> +return true;
>>>>>> +
>>>>>> +return false;
>>>>>> +}
>>>>>> +
>>>>>> static int __iommu_attach_group(struct iommu_domain *domain,
>>>>>> struct iommu_group *group)
>>>>>> {
>>>>>> @@ -2214,7 +2227,8 @@ static int __iommu_attach_group(struct iommu_domain *domain,
>>>>>> return -EBUSY;
>>>>>>
>>>>>> dev = iommu_group_first_dev(group);
>>>>>> -if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain->owner)
>>>>>> +if (!dev_has_iommu(dev) ||
>>>>>> +   !domain_iommu_ops_compatible(dev_iommu_ops(dev), domain))
>>>>>> return -EINVAL;
>>>>>>
>>>>>> return __iommu_group_set_domain(group, domain);
>>>>>> @@ -3435,7 +3449,8 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
>>>>>>    !ops->blocked_domain->ops->set_dev_pasid)
>>>>>> return -EOPNOTSUPP;
>>>>>>
>>>>>> -if (ops != domain->owner || pasid == IOMMU_NO_PASID)
>>>>>> +if (!domain_iommu_ops_compatible(ops, domain) ||
>>>>>> +   pasid == IOMMU_NO_PASID)
>>>>>> return -EINVAL;
>>>>>>
>>>>>> mutex_lock(&group->mutex);
>>>>>> @@ -3511,7 +3526,7 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
>>>>>> if (!domain->ops->set_dev_pasid)
>>>>>> return -EOPNOTSUPP;
>>>>>>
>>>>>> -if (dev_iommu_ops(dev) != domain->owner ||
>>>>>> +if (!domain_iommu_ops_compatible(dev_iommu_ops(dev), domain) ||
>>>>>>    pasid == IOMMU_NO_PASID || !handle)
>>>>>> return -EINVAL;
>>>>>>
>>>>>> -- 
>>>>>> 2.43.0
> 

Re: [PATCH v3 1/1] iommu: Allow attaching static domains in iommu_attach_device_pasid()

[Joerg Roedel]

On Thu, Apr 24, 2025 at 11:41:23AM +0800, Lu Baolu wrote:
>  drivers/iommu/iommu.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)

Applied, thanks.