Re: [PATCH] iommu: intel: Flush the IOTLB to get rid of the initial kdump mappings

2017-05-17 Thread Joerg Roedel
On Fri, May 05, 2017 at 11:39:59AM -0700, KarimAllah Ahmed wrote:
> Ever since commit 091d42e43d ("iommu/vt-d: Copy translation tables from
> old kernel") 

Please put the commit-information into a Fixes: tag next time. I'll
added a Fixes tag to the commit this time.

> the kdump kernel copies the IOMMU context tables from the
> previous kernel. Each device mappings will be destroyed once the driver
> for the respective device takes over.
> 
> This unfortunately breaks the workflow of mapping and unmapping a new
> context to the IOMMU. The mapping function assumes that either:
> 
> 1) Unmapping did the proper IOMMU flushing and it only ever flush if the
>IOMMU unit supports caching invalid entries.
> 2) The system just booted and the initialization code took care of
>flushing all IOMMU caches.
> 
> This assumption is not true for the kdump kernel since the context
> tables have been copied from the previous kernel and translations could
> have been cached ever since. So make sure to flush the IOTLB as well
> when we destroy these old copied mappings.
> 
> Cc: Joerg Roedel 
> Cc: David Woodhouse 
> Cc: David Woodhouse 
> Cc: Anthony Liguori 
> Signed-off-by: KarimAllah Ahmed 

Applied to iommu/fixes, thanks.

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


Re: [PATCH] iommu: intel: Flush the IOTLB to get rid of the initial kdump mappings

2017-05-05 Thread David Woodhouse
On Fri, 2017-05-05 at 11:39 -0700, KarimAllah Ahmed wrote:
> Ever since commit 091d42e43d ("iommu/vt-d: Copy translation tables from
> old kernel") the kdump kernel copies the IOMMU context tables from the
> previous kernel. Each device mappings will be destroyed once the driver
> for the respective device takes over.
> 
> This unfortunately breaks the workflow of mapping and unmapping a new
> context to the IOMMU. The mapping function assumes that either:
> 
> 1) Unmapping did the proper IOMMU flushing and it only ever flush if the
>    IOMMU unit supports caching invalid entries.
> 2) The system just booted and the initialization code took care of
>    flushing all IOMMU caches.
> 
> This assumption is not true for the kdump kernel since the context
> tables have been copied from the previous kernel and translations could
> have been cached ever since. So make sure to flush the IOTLB as well
> when we destroy these old copied mappings.
> 
> Cc: Joerg Roedel 
> Cc: David Woodhouse 
> Cc: David Woodhouse 
> Cc: Anthony Liguori 
> Signed-off-by: KarimAllah Ahmed 

Acked-by: David Woodhouse 
Cc: sta...@vger.kernel.org  v4.2+

I'm still moderately unhappy about the whole "preserve existing
mappings during kdump" thing, and wanted to have a PCI quirk for the
known-broken-can't-be-reset-after-fault devices, and trigger this
behaviour only then. Although I have a vague recollection of there
being a slightly saner justification for it... perhaps this should be
documented, if there is one?

smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu: intel: Flush the IOTLB to get rid of the initial kdump mappings

2017-05-05 Thread KarimAllah Ahmed via iommu
Ever since commit 091d42e43d ("iommu/vt-d: Copy translation tables from
old kernel") the kdump kernel copies the IOMMU context tables from the
previous kernel. Each device mappings will be destroyed once the driver
for the respective device takes over.

This unfortunately breaks the workflow of mapping and unmapping a new
context to the IOMMU. The mapping function assumes that either:

1) Unmapping did the proper IOMMU flushing and it only ever flush if the
   IOMMU unit supports caching invalid entries.
2) The system just booted and the initialization code took care of
   flushing all IOMMU caches.

This assumption is not true for the kdump kernel since the context
tables have been copied from the previous kernel and translations could
have been cached ever since. So make sure to flush the IOTLB as well
when we destroy these old copied mappings.

Cc: Joerg Roedel 
Cc: David Woodhouse 
Cc: David Woodhouse 
Cc: Anthony Liguori 
Signed-off-by: KarimAllah Ahmed 
---
 drivers/iommu/intel-iommu.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d412a31..478130d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2050,11 +2050,14 @@ static int domain_context_mapping_one(struct 
dmar_domain *domain,
if (context_copied(context)) {
u16 did_old = context_domain_id(context);
 
-   if (did_old >= 0 && did_old < cap_ndoms(iommu->cap))
+   if (did_old >= 0 && did_old < cap_ndoms(iommu->cap)) {
iommu->flush.flush_context(iommu, did_old,
   (((u16)bus) << 8) | devfn,
   DMA_CCMD_MASK_NOBIT,
   DMA_CCMD_DEVICE_INVL);
+   iommu->flush.flush_iotlb(iommu, did_old, 0, 0,
+DMA_TLB_DSI_FLUSH);
+   }
}
 
pgd = domain->pgd;
-- 
2.7.4

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