* [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
@ 2024-08-15 12:48 Lu Baolu
2024-08-15 14:04 ` Alex Williamson
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Lu Baolu @ 2024-08-15 12:48 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Alex Williamson,
Jason Gunthorpe, Kevin Tian
Cc: iommu, linux-kernel, Lu Baolu
The helper intel_context_flush_present() is designed to flush all related
caches when a context entry with the present bit set is modified. It
currently retrieves the domain ID from the context entry and uses it to
flush the IOTLB and context caches. This is incorrect when the context
entry transitions from present to non-present, as the domain ID field is
cleared before calling the helper.
Fix it by passing the domain ID programmed in the context entry before the
change to intel_context_flush_present(). This ensures that the correct
domain ID is used for cache invalidation.
Fixes: f90584f4beb8 ("iommu/vt-d: Add helper to flush caches for context change")
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Closes: https://lore.kernel.org/linux-iommu/20240814162726.5efe1a6e.alex.williamson@redhat.com/
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/iommu.h | 2 +-
drivers/iommu/intel/iommu.c | 8 ++++++--
drivers/iommu/intel/pasid.c | 7 ++++---
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index b67c14da1240..a969be2258b1 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -1154,7 +1154,7 @@ void cache_tag_flush_range_np(struct dmar_domain *domain, unsigned long start,
void intel_context_flush_present(struct device_domain_info *info,
struct context_entry *context,
- bool affect_domains);
+ u16 did, bool affect_domains);
#ifdef CONFIG_INTEL_IOMMU_SVM
void intel_svm_check(struct intel_iommu *iommu);
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9ff8b83c19a3..4aa070cf56e7 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1944,6 +1944,7 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8
{
struct intel_iommu *iommu = info->iommu;
struct context_entry *context;
+ u16 did;
spin_lock(&iommu->lock);
context = iommu_context_addr(iommu, bus, devfn, 0);
@@ -1952,10 +1953,11 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8
return;
}
+ did = context_domain_id(context);
context_clear_entry(context);
__iommu_flush_cache(iommu, context, sizeof(*context));
spin_unlock(&iommu->lock);
- intel_context_flush_present(info, context, true);
+ intel_context_flush_present(info, context, did, true);
}
static int domain_setup_first_level(struct intel_iommu *iommu,
@@ -4249,6 +4251,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
struct intel_iommu *iommu = info->iommu;
u8 bus = info->bus, devfn = info->devfn;
struct context_entry *context;
+ u16 did;
spin_lock(&iommu->lock);
if (context_copied(iommu, bus, devfn)) {
@@ -4261,6 +4264,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
spin_unlock(&iommu->lock);
return -ENODEV;
}
+ did = context_domain_id(context);
if (enable)
context_set_sm_pre(context);
@@ -4269,7 +4273,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
if (!ecap_coherent(iommu->ecap))
clflush_cache_range(context, sizeof(*context));
- intel_context_flush_present(info, context, true);
+ intel_context_flush_present(info, context, did, true);
spin_unlock(&iommu->lock);
return 0;
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 5792c817cefa..b51fc268dc84 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -683,6 +683,7 @@ static void device_pasid_table_teardown(struct device *dev, u8 bus, u8 devfn)
struct device_domain_info *info = dev_iommu_priv_get(dev);
struct intel_iommu *iommu = info->iommu;
struct context_entry *context;
+ u16 did;
spin_lock(&iommu->lock);
context = iommu_context_addr(iommu, bus, devfn, false);
@@ -691,10 +692,11 @@ static void device_pasid_table_teardown(struct device *dev, u8 bus, u8 devfn)
return;
}
+ did = context_domain_id(context);
context_clear_entry(context);
__iommu_flush_cache(iommu, context, sizeof(*context));
spin_unlock(&iommu->lock);
- intel_context_flush_present(info, context, false);
+ intel_context_flush_present(info, context, did, false);
}
static int pci_pasid_table_teardown(struct pci_dev *pdev, u16 alias, void *data)
@@ -885,10 +887,9 @@ static void __context_flush_dev_iotlb(struct device_domain_info *info)
*/
void intel_context_flush_present(struct device_domain_info *info,
struct context_entry *context,
- bool flush_domains)
+ u16 did, bool flush_domains)
{
struct intel_iommu *iommu = info->iommu;
- u16 did = context_domain_id(context);
struct pasid_entry *pte;
int i;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-15 12:48 [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper Lu Baolu
@ 2024-08-15 14:04 ` Alex Williamson
2024-08-15 15:50 ` Jerry Snitselaar
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Alex Williamson @ 2024-08-15 14:04 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, iommu, linux-kernel
On Thu, 15 Aug 2024 20:48:57 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:
> The helper intel_context_flush_present() is designed to flush all related
> caches when a context entry with the present bit set is modified. It
> currently retrieves the domain ID from the context entry and uses it to
> flush the IOTLB and context caches. This is incorrect when the context
> entry transitions from present to non-present, as the domain ID field is
> cleared before calling the helper.
>
> Fix it by passing the domain ID programmed in the context entry before the
> change to intel_context_flush_present(). This ensures that the correct
> domain ID is used for cache invalidation.
>
> Fixes: f90584f4beb8 ("iommu/vt-d: Add helper to flush caches for context change")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Closes: https://lore.kernel.org/linux-iommu/20240814162726.5efe1a6e.alex.williamson@redhat.com/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/iommu.h | 2 +-
> drivers/iommu/intel/iommu.c | 8 ++++++--
> drivers/iommu/intel/pasid.c | 7 ++++---
> 3 files changed, 11 insertions(+), 6 deletions(-)
Works for me. Thanks for the fix.
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index b67c14da1240..a969be2258b1 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -1154,7 +1154,7 @@ void cache_tag_flush_range_np(struct dmar_domain *domain, unsigned long start,
>
> void intel_context_flush_present(struct device_domain_info *info,
> struct context_entry *context,
> - bool affect_domains);
> + u16 did, bool affect_domains);
>
> #ifdef CONFIG_INTEL_IOMMU_SVM
> void intel_svm_check(struct intel_iommu *iommu);
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 9ff8b83c19a3..4aa070cf56e7 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1944,6 +1944,7 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8
> {
> struct intel_iommu *iommu = info->iommu;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> context = iommu_context_addr(iommu, bus, devfn, 0);
> @@ -1952,10 +1953,11 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8
> return;
> }
>
> + did = context_domain_id(context);
> context_clear_entry(context);
> __iommu_flush_cache(iommu, context, sizeof(*context));
> spin_unlock(&iommu->lock);
> - intel_context_flush_present(info, context, true);
> + intel_context_flush_present(info, context, did, true);
> }
>
> static int domain_setup_first_level(struct intel_iommu *iommu,
> @@ -4249,6 +4251,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
> struct intel_iommu *iommu = info->iommu;
> u8 bus = info->bus, devfn = info->devfn;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> if (context_copied(iommu, bus, devfn)) {
> @@ -4261,6 +4264,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
> spin_unlock(&iommu->lock);
> return -ENODEV;
> }
> + did = context_domain_id(context);
>
> if (enable)
> context_set_sm_pre(context);
> @@ -4269,7 +4273,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
>
> if (!ecap_coherent(iommu->ecap))
> clflush_cache_range(context, sizeof(*context));
> - intel_context_flush_present(info, context, true);
> + intel_context_flush_present(info, context, did, true);
> spin_unlock(&iommu->lock);
>
> return 0;
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index 5792c817cefa..b51fc268dc84 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -683,6 +683,7 @@ static void device_pasid_table_teardown(struct device *dev, u8 bus, u8 devfn)
> struct device_domain_info *info = dev_iommu_priv_get(dev);
> struct intel_iommu *iommu = info->iommu;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> context = iommu_context_addr(iommu, bus, devfn, false);
> @@ -691,10 +692,11 @@ static void device_pasid_table_teardown(struct device *dev, u8 bus, u8 devfn)
> return;
> }
>
> + did = context_domain_id(context);
> context_clear_entry(context);
> __iommu_flush_cache(iommu, context, sizeof(*context));
> spin_unlock(&iommu->lock);
> - intel_context_flush_present(info, context, false);
> + intel_context_flush_present(info, context, did, false);
> }
>
> static int pci_pasid_table_teardown(struct pci_dev *pdev, u16 alias, void *data)
> @@ -885,10 +887,9 @@ static void __context_flush_dev_iotlb(struct device_domain_info *info)
> */
> void intel_context_flush_present(struct device_domain_info *info,
> struct context_entry *context,
> - bool flush_domains)
> + u16 did, bool flush_domains)
> {
> struct intel_iommu *iommu = info->iommu;
> - u16 did = context_domain_id(context);
> struct pasid_entry *pte;
> int i;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-15 12:48 [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper Lu Baolu
2024-08-15 14:04 ` Alex Williamson
@ 2024-08-15 15:50 ` Jerry Snitselaar
2024-08-15 16:05 ` Jacob Pan
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Jerry Snitselaar @ 2024-08-15 15:50 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Alex Williamson,
Jason Gunthorpe, Kevin Tian, iommu, linux-kernel
On Thu, Aug 15, 2024 at 08:48:57PM GMT, Lu Baolu wrote:
> The helper intel_context_flush_present() is designed to flush all related
> caches when a context entry with the present bit set is modified. It
> currently retrieves the domain ID from the context entry and uses it to
> flush the IOTLB and context caches. This is incorrect when the context
> entry transitions from present to non-present, as the domain ID field is
> cleared before calling the helper.
>
> Fix it by passing the domain ID programmed in the context entry before the
> change to intel_context_flush_present(). This ensures that the correct
> domain ID is used for cache invalidation.
>
> Fixes: f90584f4beb8 ("iommu/vt-d: Add helper to flush caches for context change")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Closes: https://lore.kernel.org/linux-iommu/20240814162726.5efe1a6e.alex.williamson@redhat.com/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-15 12:48 [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper Lu Baolu
2024-08-15 14:04 ` Alex Williamson
2024-08-15 15:50 ` Jerry Snitselaar
@ 2024-08-15 16:05 ` Jacob Pan
2024-08-25 12:44 ` Alex Williamson
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Jacob Pan @ 2024-08-15 16:05 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Alex Williamson,
Jason Gunthorpe, Kevin Tian, iommu, linux-kernel
On Thu, 15 Aug 2024 20:48:57 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:
> The helper intel_context_flush_present() is designed to flush all
> related caches when a context entry with the present bit set is
> modified. It currently retrieves the domain ID from the context entry
> and uses it to flush the IOTLB and context caches. This is incorrect
> when the context entry transitions from present to non-present, as
> the domain ID field is cleared before calling the helper.
>
> Fix it by passing the domain ID programmed in the context entry
> before the change to intel_context_flush_present(). This ensures that
> the correct domain ID is used for cache invalidation.
>
> Fixes: f90584f4beb8 ("iommu/vt-d: Add helper to flush caches for
> context change") Reported-by: Alex Williamson
> <alex.williamson@redhat.com> Closes:
> https://lore.kernel.org/linux-iommu/20240814162726.5efe1a6e.alex.williamson@redhat.com/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> ---
> drivers/iommu/intel/iommu.h | 2 +-
> drivers/iommu/intel/iommu.c | 8 ++++++--
> drivers/iommu/intel/pasid.c | 7 ++++---
> 3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index b67c14da1240..a969be2258b1 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -1154,7 +1154,7 @@ void cache_tag_flush_range_np(struct
> dmar_domain *domain, unsigned long start,
> void intel_context_flush_present(struct device_domain_info *info,
> struct context_entry *context,
> - bool affect_domains);
> + u16 did, bool affect_domains);
>
> #ifdef CONFIG_INTEL_IOMMU_SVM
> void intel_svm_check(struct intel_iommu *iommu);
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 9ff8b83c19a3..4aa070cf56e7 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1944,6 +1944,7 @@ static void domain_context_clear_one(struct
> device_domain_info *info, u8 bus, u8 {
> struct intel_iommu *iommu = info->iommu;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> context = iommu_context_addr(iommu, bus, devfn, 0);
> @@ -1952,10 +1953,11 @@ static void domain_context_clear_one(struct
> device_domain_info *info, u8 bus, u8 return;
> }
>
> + did = context_domain_id(context);
> context_clear_entry(context);
> __iommu_flush_cache(iommu, context, sizeof(*context));
> spin_unlock(&iommu->lock);
> - intel_context_flush_present(info, context, true);
> + intel_context_flush_present(info, context, did, true);
> }
>
> static int domain_setup_first_level(struct intel_iommu *iommu,
> @@ -4249,6 +4251,7 @@ static int context_flip_pri(struct
> device_domain_info *info, bool enable) struct intel_iommu *iommu =
> info->iommu; u8 bus = info->bus, devfn = info->devfn;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> if (context_copied(iommu, bus, devfn)) {
> @@ -4261,6 +4264,7 @@ static int context_flip_pri(struct
> device_domain_info *info, bool enable) spin_unlock(&iommu->lock);
> return -ENODEV;
> }
> + did = context_domain_id(context);
>
> if (enable)
> context_set_sm_pre(context);
> @@ -4269,7 +4273,7 @@ static int context_flip_pri(struct
> device_domain_info *info, bool enable)
> if (!ecap_coherent(iommu->ecap))
> clflush_cache_range(context, sizeof(*context));
> - intel_context_flush_present(info, context, true);
> + intel_context_flush_present(info, context, did, true);
> spin_unlock(&iommu->lock);
>
> return 0;
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index 5792c817cefa..b51fc268dc84 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -683,6 +683,7 @@ static void device_pasid_table_teardown(struct
> device *dev, u8 bus, u8 devfn) struct device_domain_info *info =
> dev_iommu_priv_get(dev); struct intel_iommu *iommu = info->iommu;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> context = iommu_context_addr(iommu, bus, devfn, false);
> @@ -691,10 +692,11 @@ static void device_pasid_table_teardown(struct
> device *dev, u8 bus, u8 devfn) return;
> }
>
> + did = context_domain_id(context);
> context_clear_entry(context);
> __iommu_flush_cache(iommu, context, sizeof(*context));
> spin_unlock(&iommu->lock);
> - intel_context_flush_present(info, context, false);
> + intel_context_flush_present(info, context, did, false);
> }
>
> static int pci_pasid_table_teardown(struct pci_dev *pdev, u16 alias,
> void *data) @@ -885,10 +887,9 @@ static void
> __context_flush_dev_iotlb(struct device_domain_info *info) */
> void intel_context_flush_present(struct device_domain_info *info,
> struct context_entry *context,
> - bool flush_domains)
> + u16 did, bool flush_domains)
> {
> struct intel_iommu *iommu = info->iommu;
> - u16 did = context_domain_id(context);
> struct pasid_entry *pte;
> int i;
>
Reviewed-by: Jacob Pan <jacob.pan@linux.microsoft.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-15 12:48 [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper Lu Baolu
` (2 preceding siblings ...)
2024-08-15 16:05 ` Jacob Pan
@ 2024-08-25 12:44 ` Alex Williamson
2024-08-26 7:14 ` Joerg Roedel
2024-08-26 16:15 ` Jerry Snitselaar
5 siblings, 0 replies; 9+ messages in thread
From: Alex Williamson @ 2024-08-25 12:44 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, iommu, linux-kernel
Hi,
Can we please get this merged for v6.11? This is a significant
regression in VT-d, effectively breaking all vfio use cases and since
we're effectively no longer flushing the VT-d cache for these use
cases, I imagine there's a security issue as well. Thanks,
Alex
On Thu, 15 Aug 2024 20:48:57 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:
> The helper intel_context_flush_present() is designed to flush all related
> caches when a context entry with the present bit set is modified. It
> currently retrieves the domain ID from the context entry and uses it to
> flush the IOTLB and context caches. This is incorrect when the context
> entry transitions from present to non-present, as the domain ID field is
> cleared before calling the helper.
>
> Fix it by passing the domain ID programmed in the context entry before the
> change to intel_context_flush_present(). This ensures that the correct
> domain ID is used for cache invalidation.
>
> Fixes: f90584f4beb8 ("iommu/vt-d: Add helper to flush caches for context change")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Closes: https://lore.kernel.org/linux-iommu/20240814162726.5efe1a6e.alex.williamson@redhat.com/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/iommu.h | 2 +-
> drivers/iommu/intel/iommu.c | 8 ++++++--
> drivers/iommu/intel/pasid.c | 7 ++++---
> 3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index b67c14da1240..a969be2258b1 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -1154,7 +1154,7 @@ void cache_tag_flush_range_np(struct dmar_domain *domain, unsigned long start,
>
> void intel_context_flush_present(struct device_domain_info *info,
> struct context_entry *context,
> - bool affect_domains);
> + u16 did, bool affect_domains);
>
> #ifdef CONFIG_INTEL_IOMMU_SVM
> void intel_svm_check(struct intel_iommu *iommu);
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 9ff8b83c19a3..4aa070cf56e7 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1944,6 +1944,7 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8
> {
> struct intel_iommu *iommu = info->iommu;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> context = iommu_context_addr(iommu, bus, devfn, 0);
> @@ -1952,10 +1953,11 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8
> return;
> }
>
> + did = context_domain_id(context);
> context_clear_entry(context);
> __iommu_flush_cache(iommu, context, sizeof(*context));
> spin_unlock(&iommu->lock);
> - intel_context_flush_present(info, context, true);
> + intel_context_flush_present(info, context, did, true);
> }
>
> static int domain_setup_first_level(struct intel_iommu *iommu,
> @@ -4249,6 +4251,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
> struct intel_iommu *iommu = info->iommu;
> u8 bus = info->bus, devfn = info->devfn;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> if (context_copied(iommu, bus, devfn)) {
> @@ -4261,6 +4264,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
> spin_unlock(&iommu->lock);
> return -ENODEV;
> }
> + did = context_domain_id(context);
>
> if (enable)
> context_set_sm_pre(context);
> @@ -4269,7 +4273,7 @@ static int context_flip_pri(struct device_domain_info *info, bool enable)
>
> if (!ecap_coherent(iommu->ecap))
> clflush_cache_range(context, sizeof(*context));
> - intel_context_flush_present(info, context, true);
> + intel_context_flush_present(info, context, did, true);
> spin_unlock(&iommu->lock);
>
> return 0;
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index 5792c817cefa..b51fc268dc84 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -683,6 +683,7 @@ static void device_pasid_table_teardown(struct device *dev, u8 bus, u8 devfn)
> struct device_domain_info *info = dev_iommu_priv_get(dev);
> struct intel_iommu *iommu = info->iommu;
> struct context_entry *context;
> + u16 did;
>
> spin_lock(&iommu->lock);
> context = iommu_context_addr(iommu, bus, devfn, false);
> @@ -691,10 +692,11 @@ static void device_pasid_table_teardown(struct device *dev, u8 bus, u8 devfn)
> return;
> }
>
> + did = context_domain_id(context);
> context_clear_entry(context);
> __iommu_flush_cache(iommu, context, sizeof(*context));
> spin_unlock(&iommu->lock);
> - intel_context_flush_present(info, context, false);
> + intel_context_flush_present(info, context, did, false);
> }
>
> static int pci_pasid_table_teardown(struct pci_dev *pdev, u16 alias, void *data)
> @@ -885,10 +887,9 @@ static void __context_flush_dev_iotlb(struct device_domain_info *info)
> */
> void intel_context_flush_present(struct device_domain_info *info,
> struct context_entry *context,
> - bool flush_domains)
> + u16 did, bool flush_domains)
> {
> struct intel_iommu *iommu = info->iommu;
> - u16 did = context_domain_id(context);
> struct pasid_entry *pte;
> int i;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-15 12:48 [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper Lu Baolu
` (3 preceding siblings ...)
2024-08-25 12:44 ` Alex Williamson
@ 2024-08-26 7:14 ` Joerg Roedel
2024-08-26 7:20 ` Baolu Lu
2024-08-26 16:15 ` Jerry Snitselaar
5 siblings, 1 reply; 9+ messages in thread
From: Joerg Roedel @ 2024-08-26 7:14 UTC (permalink / raw)
To: Lu Baolu
Cc: Will Deacon, Robin Murphy, Alex Williamson, Jason Gunthorpe,
Kevin Tian, iommu, linux-kernel
On Thu, Aug 15, 2024 at 08:48:57PM +0800, Lu Baolu wrote:
> drivers/iommu/intel/iommu.h | 2 +-
> drivers/iommu/intel/iommu.c | 8 ++++++--
> drivers/iommu/intel/pasid.c | 7 ++++---
> 3 files changed, 11 insertions(+), 6 deletions(-)
Applied for -rc, thanks.
Side note: Please send critical fixes as a pull request next time, as I
usually do not look deeply into patches for drivers which are actively
maintained.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-26 7:14 ` Joerg Roedel
@ 2024-08-26 7:20 ` Baolu Lu
0 siblings, 0 replies; 9+ messages in thread
From: Baolu Lu @ 2024-08-26 7:20 UTC (permalink / raw)
To: Joerg Roedel
Cc: baolu.lu, Will Deacon, Robin Murphy, Alex Williamson,
Jason Gunthorpe, Kevin Tian, iommu, linux-kernel
On 2024/8/26 15:14, Joerg Roedel wrote:
> On Thu, Aug 15, 2024 at 08:48:57PM +0800, Lu Baolu wrote:
>> drivers/iommu/intel/iommu.h | 2 +-
>> drivers/iommu/intel/iommu.c | 8 ++++++--
>> drivers/iommu/intel/pasid.c | 7 ++++---
>> 3 files changed, 11 insertions(+), 6 deletions(-)
> Applied for -rc, thanks.
>
> Side note: Please send critical fixes as a pull request next time, as I
> usually do not look deeply into patches for drivers which are actively
> maintained.
Yes, sure.
Thanks,
baolu
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-15 12:48 [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper Lu Baolu
` (4 preceding siblings ...)
2024-08-26 7:14 ` Joerg Roedel
@ 2024-08-26 16:15 ` Jerry Snitselaar
2024-08-27 7:07 ` Joerg Roedel
5 siblings, 1 reply; 9+ messages in thread
From: Jerry Snitselaar @ 2024-08-26 16:15 UTC (permalink / raw)
To: Joerg Roedel
Cc: Lu Baolu, Will Deacon, Robin Murphy, Alex Williamson,
Jason Gunthorpe, Kevin Tian, iommu, linux-kernel
On Thu, Aug 15, 2024 at 08:48:57PM GMT, Lu Baolu wrote:
> The helper intel_context_flush_present() is designed to flush all related
> caches when a context entry with the present bit set is modified. It
> currently retrieves the domain ID from the context entry and uses it to
> flush the IOTLB and context caches. This is incorrect when the context
> entry transitions from present to non-present, as the domain ID field is
> cleared before calling the helper.
>
> Fix it by passing the domain ID programmed in the context entry before the
> change to intel_context_flush_present(). This ensures that the correct
> domain ID is used for cache invalidation.
>
> Fixes: f90584f4beb8 ("iommu/vt-d: Add helper to flush caches for context change")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Closes: https://lore.kernel.org/linux-iommu/20240814162726.5efe1a6e.alex.williamson@redhat.com/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Joerg,
Can this go into a fixes pr for 6.11?
Regards,
Jerry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper
2024-08-26 16:15 ` Jerry Snitselaar
@ 2024-08-27 7:07 ` Joerg Roedel
0 siblings, 0 replies; 9+ messages in thread
From: Joerg Roedel @ 2024-08-27 7:07 UTC (permalink / raw)
To: Jerry Snitselaar
Cc: Lu Baolu, Will Deacon, Robin Murphy, Alex Williamson,
Jason Gunthorpe, Kevin Tian, iommu, linux-kernel
On Mon, Aug 26, 2024 at 09:15:06AM -0700, Jerry Snitselaar wrote:
> Can this go into a fixes pr for 6.11?
It is already queued for 6.11, will send the PR in the next days.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-08-27 7:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-15 12:48 [PATCH 1/1] iommu/vt-d: Fix incorrect domain ID in context flush helper Lu Baolu
2024-08-15 14:04 ` Alex Williamson
2024-08-15 15:50 ` Jerry Snitselaar
2024-08-15 16:05 ` Jacob Pan
2024-08-25 12:44 ` Alex Williamson
2024-08-26 7:14 ` Joerg Roedel
2024-08-26 7:20 ` Baolu Lu
2024-08-26 16:15 ` Jerry Snitselaar
2024-08-27 7:07 ` Joerg Roedel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox