Re: [PATCH 5/6] iommu/vt-d: Cleanup after delegating DMA domain to generic iommu
Hi,
On 6/11/19 3:25 AM, Sai Praneeth Prakhya wrote:
On Mon, 2019-06-10 at 11:45 -0700, Mehta, Sohil wrote:
On Sun, 2019-06-09 at 10:38 +0800, Lu Baolu wrote:
static int __init si_domain_init(int hw)
@@ -3306,14 +3252,13 @@ static int __init init_dmars(void)
if (pasid_supported(iommu))
intel_svm_init(iommu);
#endif
- }
- /*
-* Now that qi is enabled on all iommus, set the root entry
and flush
-* caches. This is required on some Intel X58 chipsets,
otherwise the
-* flush_context function will loop forever and the boot
hangs.
-*/
- for_each_active_iommu(iommu, drhd) {
+ /*
+* Now that qi is enabled on all iommus, set the root
entry and
+* flush caches. This is required on some Intel X58
chipsets,
+* otherwise the flush_context function will loop
forever and
+* the boot hangs.
+*/
iommu_flush_write_buffer(iommu);
iommu_set_root_entry(iommu);
iommu->flush.flush_context(iommu, 0, 0, 0,
DMA_CCMD_GLOBAL_INVL);
This changes the intent of the original code. As the comment says
enable QI on all IOMMUs, then flush the caches and set the root entry.
The order of setting the root entries has changed now.
Refer:
Commit a4c34ff1c029 ('iommu/vt-d: Enable QI on all IOMMUs before
setting root entry')
Thanks Sohil! for catching the bug.
Will send a V2 to Lu Baolu fixing this.
Okay, I will submit a v2 of this series later.
Regards,
Sai
Best regards,
Baolu
Re: [PATCH 5/6] iommu/vt-d: Cleanup after delegating DMA domain to generic iommu
On Mon, 2019-06-10 at 11:45 -0700, Mehta, Sohil wrote:
> On Sun, 2019-06-09 at 10:38 +0800, Lu Baolu wrote:
> > static int __init si_domain_init(int hw)
> > @@ -3306,14 +3252,13 @@ static int __init init_dmars(void)
> > if (pasid_supported(iommu))
> > intel_svm_init(iommu);
> > #endif
> > - }
> >
> > - /*
> > -* Now that qi is enabled on all iommus, set the root entry
> > and flush
> > -* caches. This is required on some Intel X58 chipsets,
> > otherwise the
> > -* flush_context function will loop forever and the boot
> > hangs.
> > -*/
> > - for_each_active_iommu(iommu, drhd) {
> > + /*
> > +* Now that qi is enabled on all iommus, set the root
> > entry and
> > +* flush caches. This is required on some Intel X58
> > chipsets,
> > +* otherwise the flush_context function will loop
> > forever and
> > +* the boot hangs.
> > +*/
> > iommu_flush_write_buffer(iommu);
> > iommu_set_root_entry(iommu);
> > iommu->flush.flush_context(iommu, 0, 0, 0,
> > DMA_CCMD_GLOBAL_INVL);
>
> This changes the intent of the original code. As the comment says
> enable QI on all IOMMUs, then flush the caches and set the root entry.
> The order of setting the root entries has changed now.
>
> Refer:
> Commit a4c34ff1c029 ('iommu/vt-d: Enable QI on all IOMMUs before
> setting root entry')
Thanks Sohil! for catching the bug.
Will send a V2 to Lu Baolu fixing this.
Regards,
Sai
Re: [PATCH 5/6] iommu/vt-d: Cleanup after delegating DMA domain to generic iommu
On Sun, 2019-06-09 at 10:38 +0800, Lu Baolu wrote:
> static int __init si_domain_init(int hw)
> @@ -3306,14 +3252,13 @@ static int __init init_dmars(void)
> if (pasid_supported(iommu))
> intel_svm_init(iommu);
> #endif
> - }
>
> - /*
> - * Now that qi is enabled on all iommus, set the root entry
> and flush
> - * caches. This is required on some Intel X58 chipsets,
> otherwise the
> - * flush_context function will loop forever and the boot
> hangs.
> - */
> - for_each_active_iommu(iommu, drhd) {
> + /*
> + * Now that qi is enabled on all iommus, set the root
> entry and
> + * flush caches. This is required on some Intel X58
> chipsets,
> + * otherwise the flush_context function will loop
> forever and
> + * the boot hangs.
> + */
> iommu_flush_write_buffer(iommu);
> iommu_set_root_entry(iommu);
> iommu->flush.flush_context(iommu, 0, 0, 0,
> DMA_CCMD_GLOBAL_INVL);
This changes the intent of the original code. As the comment says
enable QI on all IOMMUs, then flush the caches and set the root entry.
The order of setting the root entries has changed now.
Refer:
Commit a4c34ff1c029 ('iommu/vt-d: Enable QI on all IOMMUs before
setting root entry')
--Sohil
[PATCH 5/6] iommu/vt-d: Cleanup after delegating DMA domain to generic iommu
From: Sai Praneeth Prakhya
[No functional changes]
1. Starting with commit df4f3c603aeb ("iommu/vt-d: Remove static identity
map code") there are no callers for iommu_prepare_rmrr_dev() but the
implementation of the function still exists, so remove it. Also, as a
ripple effect remove get_domain_for_dev() and iommu_prepare_identity_map()
because they aren't being used either.
2. init_dmars() runs the same loop (for_each_active_iommu(iommu, drhd))
over different functions back to back. So, remove the duplicate loop to
make it a single for_each_active_iommu().
3. Cleanup few double spaces.
Cc: Joerg Roedel
Cc: Ashok Raj
Cc: Lu Baolu
Cc: Sohil Mehta
Cc: Jacob Pan
Signed-off-by: Sai Praneeth Prakhya
---
drivers/iommu/intel-iommu.c | 68 -
1 file changed, 6 insertions(+), 62 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d1cd7e449161..d60cf0fd9500 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -909,7 +909,6 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain
*domain,
return pte;
}
-
/* return address's pte at specific level */
static struct dma_pte *dma_pfn_level_pte(struct dmar_domain *domain,
unsigned long pfn,
@@ -1578,7 +1577,6 @@ static void iommu_disable_translation(struct intel_iommu
*iommu)
raw_spin_unlock_irqrestore(>register_lock, flag);
}
-
static int iommu_init_domains(struct intel_iommu *iommu)
{
u32 ndomains, nlongs;
@@ -1616,8 +1614,6 @@ static int iommu_init_domains(struct intel_iommu *iommu)
return -ENOMEM;
}
-
-
/*
* If Caching mode is set, then invalid translations are tagged
* with domain-id 0, hence we need to pre-allocate it. We also
@@ -2649,29 +2645,6 @@ static struct dmar_domain *set_domain_for_dev(struct
device *dev,
return domain;
}
-static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
-{
- struct dmar_domain *domain, *tmp;
-
- domain = find_domain(dev);
- if (domain)
- goto out;
-
- domain = find_or_alloc_domain(dev, gaw);
- if (!domain)
- goto out;
-
- tmp = set_domain_for_dev(dev, domain);
- if (!tmp || domain != tmp) {
- domain_exit(domain);
- domain = tmp;
- }
-
-out:
-
- return domain;
-}
-
static int iommu_domain_identity_map(struct dmar_domain *domain,
unsigned long long start,
unsigned long long end)
@@ -2736,33 +2709,6 @@ static int domain_prepare_identity_map(struct device
*dev,
return iommu_domain_identity_map(domain, start, end);
}
-static int iommu_prepare_identity_map(struct device *dev,
- unsigned long long start,
- unsigned long long end)
-{
- struct dmar_domain *domain;
- int ret;
-
- domain = get_domain_for_dev(dev, DEFAULT_DOMAIN_ADDRESS_WIDTH);
- if (!domain)
- return -ENOMEM;
-
- ret = domain_prepare_identity_map(dev, domain, start, end);
- if (ret)
- domain_exit(domain);
-
- return ret;
-}
-
-static inline int iommu_prepare_rmrr_dev(struct dmar_rmrr_unit *rmrr,
-struct device *dev)
-{
- if (dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO)
- return 0;
- return iommu_prepare_identity_map(dev, rmrr->base_address,
- rmrr->end_address);
-}
-
static int md_domain_init(struct dmar_domain *domain, int guest_width);
static int __init si_domain_init(int hw)
@@ -3306,14 +3252,13 @@ static int __init init_dmars(void)
if (pasid_supported(iommu))
intel_svm_init(iommu);
#endif
- }
- /*
-* Now that qi is enabled on all iommus, set the root entry and flush
-* caches. This is required on some Intel X58 chipsets, otherwise the
-* flush_context function will loop forever and the boot hangs.
-*/
- for_each_active_iommu(iommu, drhd) {
+ /*
+* Now that qi is enabled on all iommus, set the root entry and
+* flush caches. This is required on some Intel X58 chipsets,
+* otherwise the flush_context function will loop forever and
+* the boot hangs.
+*/
iommu_flush_write_buffer(iommu);
iommu_set_root_entry(iommu);
iommu->flush.flush_context(iommu, 0, 0, 0,
DMA_CCMD_GLOBAL_INVL);
@@ -4058,7 +4003,6 @@ static void __init init_iommu_pm_ops(void)
static inline void init_iommu_pm_ops(void) {}
#endif /* CONFIG_PM */
-
int __init dmar_parse_one_rmrr(struct acpi_dmar_header *header, void *arg)
{
struct
