RE: [PATCH v3 02/11] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()

2022-07-06 Thread Tian, Kevin
> From: Baolu Lu 
> Sent: Sunday, July 3, 2022 12:34 PM
> 
> On 2022/7/1 15:58, Tian, Kevin wrote:
> >> From: Lu Baolu  Sent: Wednesday, June 29,
> >> 2022 3:47 PM
> >>
> >> The disable_dmar_iommu() is called when IOMMU initialization fails
> >> or the IOMMU is hot-removed from the system. In both cases, there
> >> is no need to clear the IOMMU translation data structures for
> >> devices.
> >>
> >> On the initialization path, the device probing only happens after
> >> the IOMMU is initialized successfully, hence there're no
> >> translation data structures.
> >>
> >> On the hot-remove path, there is no real use case where the IOMMU
> >> is hot-removed, but the devices that it manages are still alive in
> >> the system. The translation data structures were torn down during
> >> device release, hence there's no need to repeat it in IOMMU
> >> hot-remove path either. This removes the unnecessary code and only
> >> leaves a check.
> >>
> >> Signed-off-by: Lu Baolu 
> >
> > You probably overlooked my last comment on kexec:
> >
> >
> https://lore.kernel.org/lkml/BL1PR11MB52711A71AD9F11B7AE42694C8CAC9
> @BL1PR11MB5271.namprd11.prod.outlook.com/
> >
> >  I think my question is still not answered.
> 
> Sorry! I did overlook that comment. I can see your points now, though it
> seems to be irrelevant to the problems that this series tries to solve.
> 
> The failure path of copying table still needs some improvement. At least
> the pages allocated for root/context tables should be freed in the
> failure path. Even worse, the software occupied a bit of page table
> entry which is feasible for the old ECS, but not work for the new
> scalable mode anymore.
> 
> All these problems deserve a separate series. We could address your
> concerns there. Does this work for you?

Yes, this makes sense to me.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 02/11] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()

2022-07-02 Thread Baolu Lu

On 2022/7/1 15:58, Tian, Kevin wrote:

From: Lu Baolu  Sent: Wednesday, June 29,
2022 3:47 PM

The disable_dmar_iommu() is called when IOMMU initialization fails
or the IOMMU is hot-removed from the system. In both cases, there
is no need to clear the IOMMU translation data structures for
devices.

On the initialization path, the device probing only happens after
the IOMMU is initialized successfully, hence there're no
translation data structures.

On the hot-remove path, there is no real use case where the IOMMU
is hot-removed, but the devices that it manages are still alive in
the system. The translation data structures were torn down during
device release, hence there's no need to repeat it in IOMMU
hot-remove path either. This removes the unnecessary code and only
leaves a check.

Signed-off-by: Lu Baolu 


You probably overlooked my last comment on kexec:

https://lore.kernel.org/lkml/bl1pr11mb52711a71ad9f11b7ae42694c8c...@bl1pr11mb5271.namprd11.prod.outlook.com/

 I think my question is still not answered.


Sorry! I did overlook that comment. I can see your points now, though it
seems to be irrelevant to the problems that this series tries to solve.

The failure path of copying table still needs some improvement. At least
the pages allocated for root/context tables should be freed in the
failure path. Even worse, the software occupied a bit of page table
entry which is feasible for the old ECS, but not work for the new
scalable mode anymore.

All these problems deserve a separate series. We could address your
concerns there. Does this work for you?

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v3 02/11] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()

2022-07-01 Thread Tian, Kevin
> From: Lu Baolu 
> Sent: Wednesday, June 29, 2022 3:47 PM
> 
> The disable_dmar_iommu() is called when IOMMU initialization fails or
> the IOMMU is hot-removed from the system. In both cases, there is no
> need to clear the IOMMU translation data structures for devices.
> 
> On the initialization path, the device probing only happens after the
> IOMMU is initialized successfully, hence there're no translation data
> structures.
> 
> On the hot-remove path, there is no real use case where the IOMMU is
> hot-removed, but the devices that it manages are still alive in the
> system. The translation data structures were torn down during device
> release, hence there's no need to repeat it in IOMMU hot-remove path
> either. This removes the unnecessary code and only leaves a check.
> 
> Signed-off-by: Lu Baolu 

You probably overlooked my last comment on kexec:

https://lore.kernel.org/lkml/bl1pr11mb52711a71ad9f11b7ae42694c8c...@bl1pr11mb5271.namprd11.prod.outlook.com/

I think my question is still not answered.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v3 02/11] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()

2022-06-29 Thread Lu Baolu
The disable_dmar_iommu() is called when IOMMU initialization fails or
the IOMMU is hot-removed from the system. In both cases, there is no
need to clear the IOMMU translation data structures for devices.

On the initialization path, the device probing only happens after the
IOMMU is initialized successfully, hence there're no translation data
structures.

On the hot-remove path, there is no real use case where the IOMMU is
hot-removed, but the devices that it manages are still alive in the
system. The translation data structures were torn down during device
release, hence there's no need to repeat it in IOMMU hot-remove path
either. This removes the unnecessary code and only leaves a check.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel/pasid.h |  1 +
 drivers/iommu/intel/iommu.c | 21 +++--
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index bf5b937848b4..20c54e50f533 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -39,6 +39,7 @@
  * only and pass-through transfer modes.
  */
 #define FLPT_DEFAULT_DID   1
+#define NUM_RESERVED_DID   2
 
 /*
  * The SUPERVISOR_MODE flag indicates a first level translation which
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 3b6571681bdd..43aaec5bdd84 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1716,23 +1716,16 @@ static int iommu_init_domains(struct intel_iommu *iommu)
 
 static void disable_dmar_iommu(struct intel_iommu *iommu)
 {
-   struct device_domain_info *info, *tmp;
-   unsigned long flags;
-
if (!iommu->domain_ids)
return;
 
-   spin_lock_irqsave(_domain_lock, flags);
-   list_for_each_entry_safe(info, tmp, _domain_list, global) {
-   if (info->iommu != iommu)
-   continue;
-
-   if (!info->dev || !info->domain)
-   continue;
-
-   __dmar_remove_one_dev_info(info);
-   }
-   spin_unlock_irqrestore(_domain_lock, flags);
+   /*
+* All iommu domains must have been detached from the devices,
+* hence there should be no domain IDs in use.
+*/
+   if (WARN_ON(bitmap_weight(iommu->domain_ids, cap_ndoms(iommu->cap))
+   > NUM_RESERVED_DID))
+   return;
 
if (iommu->gcmd & DMA_GCMD_TE)
iommu_disable_translation(iommu);
-- 
2.25.1

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