Re: [PATCH] iommu: check for the deferred attach when attaching a device

2021-01-18 Thread lijiang
在 2021年01月15日 23:15, Robin Murphy 写道:
> On 2021-01-15 14:26, lijiang wrote:
>> Hi, Robin
>>
>> Thank you for the comment.
>>
>> 在 2021年01月13日 01:29, Robin Murphy 写道:
>>> On 2021-01-05 07:52, lijiang wrote:
 在 2021年01月05日 11:55, lijiang 写道:
> Hi,
>
> Also add Joerg to cc list.
>

 Also add more people to cc list, Jerry Snitselaar and Tom Lendacky.

 Thanks.

> Thanks.
> Lianbo
> 在 2020年12月26日 13:39, Lianbo Jiang 写道:
>> Currently, because domain attach allows to be deferred from iommu
>> driver to device driver, and when iommu initializes, the devices
>> on the bus will be scanned and the default groups will be allocated.
>>
>> Due to the above changes, some devices could be added to the same
>> group as below:
>>
>> [    3.859417] pci :01:00.0: Adding to iommu group 16
>> [    3.864572] pci :01:00.1: Adding to iommu group 16
>> [    3.869738] pci :02:00.0: Adding to iommu group 17
>> [    3.874892] pci :02:00.1: Adding to iommu group 17
>>
>> But when attaching these devices, it doesn't allow that a group has
>> more than one device, otherwise it will return an error. This conflicts
>> with the deferred attaching. Unfortunately, it has two devices in the
>> same group for my side, for example:
>>
>> [    9.627014] iommu_group_device_count(): device name[0]::01:00.0
>> [    9.633545] iommu_group_device_count(): device name[1]::01:00.1
>> ...
>> [   10.255609] iommu_group_device_count(): device name[0]::02:00.0
>> [   10.262144] iommu_group_device_count(): device name[1]::02:00.1
>>
>> Finally, which caused the failure of tg3 driver when tg3 driver calls
>> the dma_alloc_coherent() to allocate coherent memory in the 
>> tg3_test_dma().
>>
>> [    9.660310] tg3 :01:00.0: DMA engine test failed, aborting
>> [    9.754085] tg3: probe of :01:00.0 failed with error -12
>> [    9.997512] tg3 :01:00.1: DMA engine test failed, aborting
>> [   10.043053] tg3: probe of :01:00.1 failed with error -12
>> [   10.288905] tg3 :02:00.0: DMA engine test failed, aborting
>> [   10.334070] tg3: probe of :02:00.0 failed with error -12
>> [   10.578303] tg3 :02:00.1: DMA engine test failed, aborting
>> [   10.622629] tg3: probe of :02:00.1 failed with error -12
>>
>> In addition, the similar situations also occur in other drivers such
>> as the bnxt_en driver. That can be reproduced easily in kdump kernel
>> when SME is active.
>>
>> Add a check for the deferred attach in the iommu_attach_device() and
>> allow to attach the deferred device regardless of how many devices
>> are in a group.
>>>
>>> Is this iommu_attach_device() call is coming from iommu-dma? (if not, then 
>>> whoever's calling it probably shouldn't be)
>>>
>>
>> Yes, you are right, the iommu_attach_device call is coming from iommu-dma.
>>  
>>> Assuming so, then probably what should happen is to move the handling 
>>> currently in iommu_dma_deferred_attach() into the core so that it can call 
>>> __iommu_attach_device() directly - the intent is just to replay that exact 
>>> call skipped in iommu_group_add_device(), so the legacy external 
>>> iommu_attach_device() interface isn't really the right tool for the job
>>
>> Sounds good. I will check if this can work in various cases. If it's OK, I 
>> will post again.
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index f0305e6aac1b..5e7da902ac36 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -23,7 +23,6 @@
>>   #include 
>>   #include 
>>   #include 
>> -#include 
>>   #include 
>>     struct iommu_dma_msi_page {
>> @@ -378,21 +377,6 @@ static int iommu_dma_init_domain(struct iommu_domain 
>> *domain, dma_addr_t base,
>>   return iova_reserve_iommu_regions(dev, domain);
>>   }
>>   -static int iommu_dma_deferred_attach(struct device *dev,
>> -    struct iommu_domain *domain)
>> -{
>> -    const struct iommu_ops *ops = domain->ops;
>> -
>> -    if (!is_kdump_kernel())
>> -    return 0;
>> -
>> -    if (unlikely(ops->is_attach_deferred &&
>> -    ops->is_attach_deferred(domain, dev)))
>> -    return iommu_attach_device(domain, dev);
>> -
>> -    return 0;
>> -}
>> -
>>   /**
>>    * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU 
>> API
>>    *    page flags.
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index ffeebda8d6de..4fed1567b498 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -23,6 +23,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>     static struct kset *iommu_group_kset;
>> @@ -1952,6 +1953,21 @@ static int __iommu_attach_device(struct iommu_domain 
>> *domain,
>>   return ret;
>>   }
>>   +int 

Re: [PATCH] iommu: check for the deferred attach when attaching a device

2021-01-15 Thread Robin Murphy

On 2021-01-15 14:26, lijiang wrote:

Hi, Robin

Thank you for the comment.

在 2021年01月13日 01:29, Robin Murphy 写道:

On 2021-01-05 07:52, lijiang wrote:

在 2021年01月05日 11:55, lijiang 写道:

Hi,

Also add Joerg to cc list.



Also add more people to cc list, Jerry Snitselaar and Tom Lendacky.

Thanks.


Thanks.
Lianbo
在 2020年12月26日 13:39, Lianbo Jiang 写道:

Currently, because domain attach allows to be deferred from iommu
driver to device driver, and when iommu initializes, the devices
on the bus will be scanned and the default groups will be allocated.

Due to the above changes, some devices could be added to the same
group as below:

[    3.859417] pci :01:00.0: Adding to iommu group 16
[    3.864572] pci :01:00.1: Adding to iommu group 16
[    3.869738] pci :02:00.0: Adding to iommu group 17
[    3.874892] pci :02:00.1: Adding to iommu group 17

But when attaching these devices, it doesn't allow that a group has
more than one device, otherwise it will return an error. This conflicts
with the deferred attaching. Unfortunately, it has two devices in the
same group for my side, for example:

[    9.627014] iommu_group_device_count(): device name[0]::01:00.0
[    9.633545] iommu_group_device_count(): device name[1]::01:00.1
...
[   10.255609] iommu_group_device_count(): device name[0]::02:00.0
[   10.262144] iommu_group_device_count(): device name[1]::02:00.1

Finally, which caused the failure of tg3 driver when tg3 driver calls
the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma().

[    9.660310] tg3 :01:00.0: DMA engine test failed, aborting
[    9.754085] tg3: probe of :01:00.0 failed with error -12
[    9.997512] tg3 :01:00.1: DMA engine test failed, aborting
[   10.043053] tg3: probe of :01:00.1 failed with error -12
[   10.288905] tg3 :02:00.0: DMA engine test failed, aborting
[   10.334070] tg3: probe of :02:00.0 failed with error -12
[   10.578303] tg3 :02:00.1: DMA engine test failed, aborting
[   10.622629] tg3: probe of :02:00.1 failed with error -12

In addition, the similar situations also occur in other drivers such
as the bnxt_en driver. That can be reproduced easily in kdump kernel
when SME is active.

Add a check for the deferred attach in the iommu_attach_device() and
allow to attach the deferred device regardless of how many devices
are in a group.


Is this iommu_attach_device() call is coming from iommu-dma? (if not, then 
whoever's calling it probably shouldn't be)



Yes, you are right, the iommu_attach_device call is coming from iommu-dma.
  

Assuming so, then probably what should happen is to move the handling currently 
in iommu_dma_deferred_attach() into the core so that it can call 
__iommu_attach_device() directly - the intent is just to replay that exact call 
skipped in iommu_group_add_device(), so the legacy external 
iommu_attach_device() interface isn't really the right tool for the job


Sounds good. I will check if this can work in various cases. If it's OK, I will 
post again.

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f0305e6aac1b..5e7da902ac36 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -23,7 +23,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  
  struct iommu_dma_msi_page {

@@ -378,21 +377,6 @@ static int iommu_dma_init_domain(struct iommu_domain 
*domain, dma_addr_t base,
return iova_reserve_iommu_regions(dev, domain);
  }
  
-static int iommu_dma_deferred_attach(struct device *dev,

-   struct iommu_domain *domain)
-{
-   const struct iommu_ops *ops = domain->ops;
-
-   if (!is_kdump_kernel())
-   return 0;
-
-   if (unlikely(ops->is_attach_deferred &&
-   ops->is_attach_deferred(domain, dev)))
-   return iommu_attach_device(domain, dev);
-
-   return 0;
-}
-
  /**
   * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
   *page flags.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ffeebda8d6de..4fed1567b498 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -23,6 +23,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  
  static struct kset *iommu_group_kset;

@@ -1952,6 +1953,21 @@ static int __iommu_attach_device(struct iommu_domain 
*domain,
return ret;
  }
  
+int iommu_dma_deferred_attach(struct device *dev,

+struct iommu_domain *domain)
+{
+const struct iommu_ops *ops = domain->ops;
+
+if (!is_kdump_kernel())
+return 0;
+
+if (unlikely(ops->is_attach_deferred &&
+ops->is_attach_deferred(domain, dev)))
+return __iommu_attach_device(domain, dev);
+
+return 0;
+}
+
  int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
  {
struct iommu_group *group;
diff --git a/include/linux/iommu.h 

Re: [PATCH] iommu: check for the deferred attach when attaching a device

2021-01-15 Thread lijiang
Hi, Robin

Thank you for the comment.

在 2021年01月13日 01:29, Robin Murphy 写道:
> On 2021-01-05 07:52, lijiang wrote:
>> 在 2021年01月05日 11:55, lijiang 写道:
>>> Hi,
>>>
>>> Also add Joerg to cc list.
>>>
>>
>> Also add more people to cc list, Jerry Snitselaar and Tom Lendacky.
>>
>> Thanks.
>>
>>> Thanks.
>>> Lianbo
>>> 在 2020年12月26日 13:39, Lianbo Jiang 写道:
 Currently, because domain attach allows to be deferred from iommu
 driver to device driver, and when iommu initializes, the devices
 on the bus will be scanned and the default groups will be allocated.

 Due to the above changes, some devices could be added to the same
 group as below:

 [    3.859417] pci :01:00.0: Adding to iommu group 16
 [    3.864572] pci :01:00.1: Adding to iommu group 16
 [    3.869738] pci :02:00.0: Adding to iommu group 17
 [    3.874892] pci :02:00.1: Adding to iommu group 17

 But when attaching these devices, it doesn't allow that a group has
 more than one device, otherwise it will return an error. This conflicts
 with the deferred attaching. Unfortunately, it has two devices in the
 same group for my side, for example:

 [    9.627014] iommu_group_device_count(): device name[0]::01:00.0
 [    9.633545] iommu_group_device_count(): device name[1]::01:00.1
 ...
 [   10.255609] iommu_group_device_count(): device name[0]::02:00.0
 [   10.262144] iommu_group_device_count(): device name[1]::02:00.1

 Finally, which caused the failure of tg3 driver when tg3 driver calls
 the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma().

 [    9.660310] tg3 :01:00.0: DMA engine test failed, aborting
 [    9.754085] tg3: probe of :01:00.0 failed with error -12
 [    9.997512] tg3 :01:00.1: DMA engine test failed, aborting
 [   10.043053] tg3: probe of :01:00.1 failed with error -12
 [   10.288905] tg3 :02:00.0: DMA engine test failed, aborting
 [   10.334070] tg3: probe of :02:00.0 failed with error -12
 [   10.578303] tg3 :02:00.1: DMA engine test failed, aborting
 [   10.622629] tg3: probe of :02:00.1 failed with error -12

 In addition, the similar situations also occur in other drivers such
 as the bnxt_en driver. That can be reproduced easily in kdump kernel
 when SME is active.

 Add a check for the deferred attach in the iommu_attach_device() and
 allow to attach the deferred device regardless of how many devices
 are in a group.
> 
> Is this iommu_attach_device() call is coming from iommu-dma? (if not, then 
> whoever's calling it probably shouldn't be)
> 

Yes, you are right, the iommu_attach_device call is coming from iommu-dma.
 
> Assuming so, then probably what should happen is to move the handling 
> currently in iommu_dma_deferred_attach() into the core so that it can call 
> __iommu_attach_device() directly - the intent is just to replay that exact 
> call skipped in iommu_group_add_device(), so the legacy external 
> iommu_attach_device() interface isn't really the right tool for the job 

Sounds good. I will check if this can work in various cases. If it's OK, I will 
post again.

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f0305e6aac1b..5e7da902ac36 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 struct iommu_dma_msi_page {
@@ -378,21 +377,6 @@ static int iommu_dma_init_domain(struct iommu_domain 
*domain, dma_addr_t base,
return iova_reserve_iommu_regions(dev, domain);
 }
 
-static int iommu_dma_deferred_attach(struct device *dev,
-   struct iommu_domain *domain)
-{
-   const struct iommu_ops *ops = domain->ops;
-
-   if (!is_kdump_kernel())
-   return 0;
-
-   if (unlikely(ops->is_attach_deferred &&
-   ops->is_attach_deferred(domain, dev)))
-   return iommu_attach_device(domain, dev);
-
-   return 0;
-}
-
 /**
  * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
  *page flags.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ffeebda8d6de..4fed1567b498 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static struct kset *iommu_group_kset;
@@ -1952,6 +1953,21 @@ static int __iommu_attach_device(struct iommu_domain 
*domain,
return ret;
 }
 
+int iommu_dma_deferred_attach(struct device *dev,
+struct iommu_domain *domain)
+{
+const struct iommu_ops *ops = domain->ops;
+
+if (!is_kdump_kernel())
+return 0;
+
+if (unlikely(ops->is_attach_deferred &&
+ops->is_attach_deferred(domain, dev)))
+return __iommu_attach_device(domain, 

Re: [PATCH] iommu: check for the deferred attach when attaching a device

2021-01-12 Thread Robin Murphy

On 2021-01-05 07:52, lijiang wrote:

在 2021年01月05日 11:55, lijiang 写道:

Hi,

Also add Joerg to cc list.



Also add more people to cc list, Jerry Snitselaar and Tom Lendacky.

Thanks.


Thanks.
Lianbo
在 2020年12月26日 13:39, Lianbo Jiang 写道:

Currently, because domain attach allows to be deferred from iommu
driver to device driver, and when iommu initializes, the devices
on the bus will be scanned and the default groups will be allocated.

Due to the above changes, some devices could be added to the same
group as below:

[3.859417] pci :01:00.0: Adding to iommu group 16
[3.864572] pci :01:00.1: Adding to iommu group 16
[3.869738] pci :02:00.0: Adding to iommu group 17
[3.874892] pci :02:00.1: Adding to iommu group 17

But when attaching these devices, it doesn't allow that a group has
more than one device, otherwise it will return an error. This conflicts
with the deferred attaching. Unfortunately, it has two devices in the
same group for my side, for example:

[9.627014] iommu_group_device_count(): device name[0]::01:00.0
[9.633545] iommu_group_device_count(): device name[1]::01:00.1
...
[   10.255609] iommu_group_device_count(): device name[0]::02:00.0
[   10.262144] iommu_group_device_count(): device name[1]::02:00.1

Finally, which caused the failure of tg3 driver when tg3 driver calls
the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma().

[9.660310] tg3 :01:00.0: DMA engine test failed, aborting
[9.754085] tg3: probe of :01:00.0 failed with error -12
[9.997512] tg3 :01:00.1: DMA engine test failed, aborting
[   10.043053] tg3: probe of :01:00.1 failed with error -12
[   10.288905] tg3 :02:00.0: DMA engine test failed, aborting
[   10.334070] tg3: probe of :02:00.0 failed with error -12
[   10.578303] tg3 :02:00.1: DMA engine test failed, aborting
[   10.622629] tg3: probe of :02:00.1 failed with error -12

In addition, the similar situations also occur in other drivers such
as the bnxt_en driver. That can be reproduced easily in kdump kernel
when SME is active.

Add a check for the deferred attach in the iommu_attach_device() and
allow to attach the deferred device regardless of how many devices
are in a group.


Is this iommu_attach_device() call is coming from iommu-dma? (if not, 
then whoever's calling it probably shouldn't be)


Assuming so, then probably what should happen is to move the handling 
currently in iommu_dma_deferred_attach() into the core so that it can 
call __iommu_attach_device() directly - the intent is just to replay 
that exact call skipped in iommu_group_add_device(), so the legacy 
external iommu_attach_device() interface isn't really the right tool for 
the job anyway. That's just slightly awkward since ideally it wants to 
be done in a way that doesn't result in a redundant out-of-line call for 
!kdump.


Alternatively I suppose it *could* just call ops->attach_dev directly, 
but then we miss out on the tracepoint, and deferred attach is arguably 
one of the cases where that's most useful :/


Robin.



Signed-off-by: Lianbo Jiang 
---
  drivers/iommu/iommu.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ffeebda8d6de..dccab7b133fb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1967,8 +1967,11 @@ int iommu_attach_device(struct iommu_domain *domain, 
struct device *dev)
 */
mutex_lock(>mutex);
ret = -EINVAL;
-   if (iommu_group_device_count(group) != 1)
+   if (!iommu_is_attach_deferred(domain, dev) &&
+   iommu_group_device_count(group) != 1) {
+   dev_err_ratelimited(dev, "Group has more than one device\n");
goto out_unlock;
+   }
  
  	ret = __iommu_attach_group(domain, group);
  



___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu



Re: [PATCH] iommu: check for the deferred attach when attaching a device

2021-01-04 Thread lijiang
在 2021年01月05日 11:55, lijiang 写道:
> Hi,
> 
> Also add Joerg to cc list.
> 

Also add more people to cc list, Jerry Snitselaar and Tom Lendacky.

Thanks.

> Thanks.
> Lianbo
> 在 2020年12月26日 13:39, Lianbo Jiang 写道:
>> Currently, because domain attach allows to be deferred from iommu
>> driver to device driver, and when iommu initializes, the devices
>> on the bus will be scanned and the default groups will be allocated.
>>
>> Due to the above changes, some devices could be added to the same
>> group as below:
>>
>> [3.859417] pci :01:00.0: Adding to iommu group 16
>> [3.864572] pci :01:00.1: Adding to iommu group 16
>> [3.869738] pci :02:00.0: Adding to iommu group 17
>> [3.874892] pci :02:00.1: Adding to iommu group 17
>>
>> But when attaching these devices, it doesn't allow that a group has
>> more than one device, otherwise it will return an error. This conflicts
>> with the deferred attaching. Unfortunately, it has two devices in the
>> same group for my side, for example:
>>
>> [9.627014] iommu_group_device_count(): device name[0]::01:00.0
>> [9.633545] iommu_group_device_count(): device name[1]::01:00.1
>> ...
>> [   10.255609] iommu_group_device_count(): device name[0]::02:00.0
>> [   10.262144] iommu_group_device_count(): device name[1]::02:00.1
>>
>> Finally, which caused the failure of tg3 driver when tg3 driver calls
>> the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma().
>>
>> [9.660310] tg3 :01:00.0: DMA engine test failed, aborting
>> [9.754085] tg3: probe of :01:00.0 failed with error -12
>> [9.997512] tg3 :01:00.1: DMA engine test failed, aborting
>> [   10.043053] tg3: probe of :01:00.1 failed with error -12
>> [   10.288905] tg3 :02:00.0: DMA engine test failed, aborting
>> [   10.334070] tg3: probe of :02:00.0 failed with error -12
>> [   10.578303] tg3 :02:00.1: DMA engine test failed, aborting
>> [   10.622629] tg3: probe of :02:00.1 failed with error -12
>>
>> In addition, the similar situations also occur in other drivers such
>> as the bnxt_en driver. That can be reproduced easily in kdump kernel
>> when SME is active.
>>
>> Add a check for the deferred attach in the iommu_attach_device() and
>> allow to attach the deferred device regardless of how many devices
>> are in a group.
>>
>> Signed-off-by: Lianbo Jiang 
>> ---
>>  drivers/iommu/iommu.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index ffeebda8d6de..dccab7b133fb 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1967,8 +1967,11 @@ int iommu_attach_device(struct iommu_domain *domain, 
>> struct device *dev)
>>   */
>>  mutex_lock(>mutex);
>>  ret = -EINVAL;
>> -if (iommu_group_device_count(group) != 1)
>> +if (!iommu_is_attach_deferred(domain, dev) &&
>> +iommu_group_device_count(group) != 1) {
>> +dev_err_ratelimited(dev, "Group has more than one device\n");
>>  goto out_unlock;
>> +}
>>  
>>  ret = __iommu_attach_group(domain, group);
>>  
>>



Re: [PATCH] iommu: check for the deferred attach when attaching a device

2021-01-04 Thread lijiang
Hi,

Also add Joerg to cc list.

Thanks.
Lianbo
在 2020年12月26日 13:39, Lianbo Jiang 写道:
> Currently, because domain attach allows to be deferred from iommu
> driver to device driver, and when iommu initializes, the devices
> on the bus will be scanned and the default groups will be allocated.
> 
> Due to the above changes, some devices could be added to the same
> group as below:
> 
> [3.859417] pci :01:00.0: Adding to iommu group 16
> [3.864572] pci :01:00.1: Adding to iommu group 16
> [3.869738] pci :02:00.0: Adding to iommu group 17
> [3.874892] pci :02:00.1: Adding to iommu group 17
> 
> But when attaching these devices, it doesn't allow that a group has
> more than one device, otherwise it will return an error. This conflicts
> with the deferred attaching. Unfortunately, it has two devices in the
> same group for my side, for example:
> 
> [9.627014] iommu_group_device_count(): device name[0]::01:00.0
> [9.633545] iommu_group_device_count(): device name[1]::01:00.1
> ...
> [   10.255609] iommu_group_device_count(): device name[0]::02:00.0
> [   10.262144] iommu_group_device_count(): device name[1]::02:00.1
> 
> Finally, which caused the failure of tg3 driver when tg3 driver calls
> the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma().
> 
> [9.660310] tg3 :01:00.0: DMA engine test failed, aborting
> [9.754085] tg3: probe of :01:00.0 failed with error -12
> [9.997512] tg3 :01:00.1: DMA engine test failed, aborting
> [   10.043053] tg3: probe of :01:00.1 failed with error -12
> [   10.288905] tg3 :02:00.0: DMA engine test failed, aborting
> [   10.334070] tg3: probe of :02:00.0 failed with error -12
> [   10.578303] tg3 :02:00.1: DMA engine test failed, aborting
> [   10.622629] tg3: probe of :02:00.1 failed with error -12
> 
> In addition, the similar situations also occur in other drivers such
> as the bnxt_en driver. That can be reproduced easily in kdump kernel
> when SME is active.
> 
> Add a check for the deferred attach in the iommu_attach_device() and
> allow to attach the deferred device regardless of how many devices
> are in a group.
> 
> Signed-off-by: Lianbo Jiang 
> ---
>  drivers/iommu/iommu.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index ffeebda8d6de..dccab7b133fb 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1967,8 +1967,11 @@ int iommu_attach_device(struct iommu_domain *domain, 
> struct device *dev)
>*/
>   mutex_lock(>mutex);
>   ret = -EINVAL;
> - if (iommu_group_device_count(group) != 1)
> + if (!iommu_is_attach_deferred(domain, dev) &&
> + iommu_group_device_count(group) != 1) {
> + dev_err_ratelimited(dev, "Group has more than one device\n");
>   goto out_unlock;
> + }
>  
>   ret = __iommu_attach_group(domain, group);
>  
> 



[PATCH] iommu: check for the deferred attach when attaching a device

2020-12-25 Thread Lianbo Jiang
Currently, because domain attach allows to be deferred from iommu
driver to device driver, and when iommu initializes, the devices
on the bus will be scanned and the default groups will be allocated.

Due to the above changes, some devices could be added to the same
group as below:

[3.859417] pci :01:00.0: Adding to iommu group 16
[3.864572] pci :01:00.1: Adding to iommu group 16
[3.869738] pci :02:00.0: Adding to iommu group 17
[3.874892] pci :02:00.1: Adding to iommu group 17

But when attaching these devices, it doesn't allow that a group has
more than one device, otherwise it will return an error. This conflicts
with the deferred attaching. Unfortunately, it has two devices in the
same group for my side, for example:

[9.627014] iommu_group_device_count(): device name[0]::01:00.0
[9.633545] iommu_group_device_count(): device name[1]::01:00.1
...
[   10.255609] iommu_group_device_count(): device name[0]::02:00.0
[   10.262144] iommu_group_device_count(): device name[1]::02:00.1

Finally, which caused the failure of tg3 driver when tg3 driver calls
the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma().

[9.660310] tg3 :01:00.0: DMA engine test failed, aborting
[9.754085] tg3: probe of :01:00.0 failed with error -12
[9.997512] tg3 :01:00.1: DMA engine test failed, aborting
[   10.043053] tg3: probe of :01:00.1 failed with error -12
[   10.288905] tg3 :02:00.0: DMA engine test failed, aborting
[   10.334070] tg3: probe of :02:00.0 failed with error -12
[   10.578303] tg3 :02:00.1: DMA engine test failed, aborting
[   10.622629] tg3: probe of :02:00.1 failed with error -12

In addition, the similar situations also occur in other drivers such
as the bnxt_en driver. That can be reproduced easily in kdump kernel
when SME is active.

Add a check for the deferred attach in the iommu_attach_device() and
allow to attach the deferred device regardless of how many devices
are in a group.

Signed-off-by: Lianbo Jiang 
---
 drivers/iommu/iommu.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ffeebda8d6de..dccab7b133fb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1967,8 +1967,11 @@ int iommu_attach_device(struct iommu_domain *domain, 
struct device *dev)
 */
mutex_lock(>mutex);
ret = -EINVAL;
-   if (iommu_group_device_count(group) != 1)
+   if (!iommu_is_attach_deferred(domain, dev) &&
+   iommu_group_device_count(group) != 1) {
+   dev_err_ratelimited(dev, "Group has more than one device\n");
goto out_unlock;
+   }
 
ret = __iommu_attach_group(domain, group);
 
-- 
2.17.1