linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] intel-iommu: Default to non-coherent for domains unattached to iommus
@ 2011-11-12  0:26 Alex Williamson
  2012-08-20 20:39 ` Craig Hada
  2012-09-18 11:58 ` Joerg Roedel
  0 siblings, 2 replies; 3+ messages in thread
From: Alex Williamson @ 2011-11-12  0:26 UTC (permalink / raw)
  To: dwmw2; +Cc: iommu, linux-pci, linux-kernel, chrisw, ddutile, alex.williamson

domain_update_iommu_coherency() currently defaults to setting domains
as coherent when the domain is not attached to any iommus.  This
allows for a window in domain_context_mapping_one() where such a
domain can update context entries non-coherently, and only after
update the domain capability to clear iommu_coherency.

This can be seen using KVM device assignment on VT-d systems that
do not support coherency in the ecap register.  When a device is
added to a guest, a domain is created (iommu_coherency = 0), the
device is attached, and ranges are mapped.  If we then hot unplug
the device, the coherency is updated and set to the default (1)
since no iommus are attached to the domain.  A subsequent attach
of a device makes use of the same dmar domain (now marked coherent)
updates context entries with coherency enabled, and only disables
coherency as the last step in the process.

To fix this, switch domain_update_iommu_coherency() to use the
safer, non-coherent default for domains not attached to iommus.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Donald Dutile <ddutile@redhat.com>
Acked-by: Donald Dutile <ddutile@redhat.com>
Acked-by: Chris Wright <chrisw@sous-sol.org>
---

v2: Fix spelling in commit log :-\

 drivers/iommu/intel-iommu.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c0c7820..6b9d8c1 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -560,7 +560,9 @@ static void domain_update_iommu_coherency(struct dmar_domain *domain)
 {
 	int i;
 
-	domain->iommu_coherency = 1;
+	i = find_first_bit(&domain->iommu_bmp, g_num_of_iommus);
+
+	domain->iommu_coherency = i < g_num_of_iommus ? 1 : 0;
 
 	for_each_set_bit(i, &domain->iommu_bmp, g_num_of_iommus) {
 		if (!ecap_coherent(g_iommus[i]->ecap)) {


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] intel-iommu: Default to non-coherent for domains unattached to iommus
  2011-11-12  0:26 [PATCH v2] intel-iommu: Default to non-coherent for domains unattached to iommus Alex Williamson
@ 2012-08-20 20:39 ` Craig Hada
  2012-09-18 11:58 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Craig Hada @ 2012-08-20 20:39 UTC (permalink / raw)
  To: linux-pci

Alex Williamson <alex.williamson <at> redhat.com> writes:

> 
> domain_update_iommu_coherency() currently defaults to setting domains
> as coherent when the domain is not attached to any iommus.  This
> allows for a window in domain_context_mapping_one() where such a
> domain can update context entries non-coherently, and only after
> update the domain capability to clear iommu_coherency.
> 
> This can be seen using KVM device assignment on VT-d systems that
> do not support coherency in the ecap register.  When a device is
> added to a guest, a domain is created (iommu_coherency = 0), the
> device is attached, and ranges are mapped.  If we then hot unplug
> the device, the coherency is updated and set to the default (1)
> since no iommus are attached to the domain.  A subsequent attach
> of a device makes use of the same dmar domain (now marked coherent)
> updates context entries with coherency enabled, and only disables
> coherency as the last step in the process.
> @@ -560,7 +560,9 @@ static void domain_update_iommu_coherency(struct 
dmar_domain *domain)
>  {
>  	int i;
> 
> -	domain->iommu_coherency = 1;
> +	i = find_first_bit(&domain->iommu_bmp, g_num_of_iommus);
> +
> +	domain->iommu_coherency = i < g_num_of_iommus ? 1 : 0;
> 
>  	for_each_set_bit(i, &domain->iommu_bmp, g_num_of_iommus) {
>  		if (!ecap_coherent(g_iommus[i]->ecap)) {
> 
> 

I'm running into this same problem on two HP systems while testing pass 
through mode. Both systems are running the iommu in non-coherent mode and it 
does not appear there is a way to change it in BIOS. I applied the above code 
change to my 3.5 kernel and the original problem has gone away but now I'm 
getting a fault 6 on a PTE entry.

DMAR:[DMA Read] Request device [00:1e.0] fault addr 1000
DMAR:[fault reason 06] PTE Read access is not set

Is anyone working on a fix for this problem?

-Craig


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] intel-iommu: Default to non-coherent for domains unattached to iommus
  2011-11-12  0:26 [PATCH v2] intel-iommu: Default to non-coherent for domains unattached to iommus Alex Williamson
  2012-08-20 20:39 ` Craig Hada
@ 2012-09-18 11:58 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Joerg Roedel @ 2012-09-18 11:58 UTC (permalink / raw)
  To: Alex Williamson; +Cc: dwmw2, linux-pci, linux-kernel, chrisw, iommu

On Fri, Nov 11, 2011 at 05:26:44PM -0700, Alex Williamson wrote:
> domain_update_iommu_coherency() currently defaults to setting domains
> as coherent when the domain is not attached to any iommus.  This
> allows for a window in domain_context_mapping_one() where such a
> domain can update context entries non-coherently, and only after
> update the domain capability to clear iommu_coherency.
> 
> This can be seen using KVM device assignment on VT-d systems that
> do not support coherency in the ecap register.  When a device is
> added to a guest, a domain is created (iommu_coherency = 0), the
> device is attached, and ranges are mapped.  If we then hot unplug
> the device, the coherency is updated and set to the default (1)
> since no iommus are attached to the domain.  A subsequent attach
> of a device makes use of the same dmar domain (now marked coherent)
> updates context entries with coherency enabled, and only disables
> coherency as the last step in the process.
> 
> To fix this, switch domain_update_iommu_coherency() to use the
> safer, non-coherent default for domains not attached to iommus.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Tested-by: Donald Dutile <ddutile@redhat.com>
> Acked-by: Donald Dutile <ddutile@redhat.com>
> Acked-by: Chris Wright <chrisw@sous-sol.org>

Applied, thanks.

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-09-18 11:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-12  0:26 [PATCH v2] intel-iommu: Default to non-coherent for domains unattached to iommus Alex Williamson
2012-08-20 20:39 ` Craig Hada
2012-09-18 11:58 ` Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).