xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] VT-d: don't suppress invalidation address write when it is zero
@ 2015-09-28 14:32 Jan Beulich
  2015-09-28 16:01 ` Andrew Cooper
  2015-10-10  5:35 ` Zhang, Yang Z
  0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2015-09-28 14:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Yang Z Zhang, Kevin Tian

[-- Attachment #1: Type: text/plain, Size: 1891 bytes --]

GFN zero is a valid address, and hence may need invalidation done for
it just like for any other GFN.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
The comment right before the respective dmar_writeq() confuses me: What
"first" TLB reg does it talk about? Is it simply stale (albeit it has
been there already in 3.2.x, i.e. it would then likely have been stale
already at the time that code got added)?

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -414,7 +414,7 @@ static int flush_iotlb_reg(void *_iommu,
 {
     struct iommu *iommu = (struct iommu *) _iommu;
     int tlb_offset = ecap_iotlb_offset(iommu->ecap);
-    u64 val = 0, val_iva = 0;
+    u64 val = 0;
     unsigned long flags;
 
     /*
@@ -435,7 +435,6 @@ static int flush_iotlb_reg(void *_iommu,
     switch ( type )
     {
     case DMA_TLB_GLOBAL_FLUSH:
-        /* global flush doesn't need set IVA_REG */
         val = DMA_TLB_GLOBAL_FLUSH|DMA_TLB_IVT;
         break;
     case DMA_TLB_DSI_FLUSH:
@@ -443,8 +442,6 @@ static int flush_iotlb_reg(void *_iommu,
         break;
     case DMA_TLB_PSI_FLUSH:
         val = DMA_TLB_PSI_FLUSH|DMA_TLB_IVT|DMA_TLB_DID(did);
-        /* Note: always flush non-leaf currently */
-        val_iva = size_order | addr;
         break;
     default:
         BUG();
@@ -457,8 +454,11 @@ static int flush_iotlb_reg(void *_iommu,
 
     spin_lock_irqsave(&iommu->register_lock, flags);
     /* Note: Only uses first TLB reg currently */
-    if ( val_iva )
-        dmar_writeq(iommu->reg, tlb_offset, val_iva);
+    if ( type == DMA_TLB_PSI_FLUSH )
+    {
+        /* Note: always flush non-leaf currently. */
+        dmar_writeq(iommu->reg, tlb_offset, size_order | addr);
+    }
     dmar_writeq(iommu->reg, tlb_offset + 8, val);
 
     /* Make sure hardware complete it */




[-- Attachment #2: VT-d-flush-IOTLB-gfn-0.patch --]
[-- Type: text/plain, Size: 1952 bytes --]

VT-d: don't suppress invalidation address write when it is zero

GFN zero is a valid address, and hence may need invalidation done for
it just like for any other GFN.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
The comment right before the respective dmar_writeq() confuses me: What
"first" TLB reg does it talk about? Is it simply stale (albeit it has
been there already in 3.2.x, i.e. it would then likely have been stale
already at the time that code got added)?

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -414,7 +414,7 @@ static int flush_iotlb_reg(void *_iommu,
 {
     struct iommu *iommu = (struct iommu *) _iommu;
     int tlb_offset = ecap_iotlb_offset(iommu->ecap);
-    u64 val = 0, val_iva = 0;
+    u64 val = 0;
     unsigned long flags;
 
     /*
@@ -435,7 +435,6 @@ static int flush_iotlb_reg(void *_iommu,
     switch ( type )
     {
     case DMA_TLB_GLOBAL_FLUSH:
-        /* global flush doesn't need set IVA_REG */
         val = DMA_TLB_GLOBAL_FLUSH|DMA_TLB_IVT;
         break;
     case DMA_TLB_DSI_FLUSH:
@@ -443,8 +442,6 @@ static int flush_iotlb_reg(void *_iommu,
         break;
     case DMA_TLB_PSI_FLUSH:
         val = DMA_TLB_PSI_FLUSH|DMA_TLB_IVT|DMA_TLB_DID(did);
-        /* Note: always flush non-leaf currently */
-        val_iva = size_order | addr;
         break;
     default:
         BUG();
@@ -457,8 +454,11 @@ static int flush_iotlb_reg(void *_iommu,
 
     spin_lock_irqsave(&iommu->register_lock, flags);
     /* Note: Only uses first TLB reg currently */
-    if ( val_iva )
-        dmar_writeq(iommu->reg, tlb_offset, val_iva);
+    if ( type == DMA_TLB_PSI_FLUSH )
+    {
+        /* Note: always flush non-leaf currently. */
+        dmar_writeq(iommu->reg, tlb_offset, size_order | addr);
+    }
     dmar_writeq(iommu->reg, tlb_offset + 8, val);
 
     /* Make sure hardware complete it */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: [PATCH] VT-d: don't suppress invalidation address write when it is zero
  2015-09-28 14:32 [PATCH] VT-d: don't suppress invalidation address write when it is zero Jan Beulich
@ 2015-09-28 16:01 ` Andrew Cooper
  2015-10-10  5:35 ` Zhang, Yang Z
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2015-09-28 16:01 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Yang Z Zhang, Kevin Tian

On 28/09/15 15:32, Jan Beulich wrote:
> GFN zero is a valid address, and hence may need invalidation done for
> it just like for any other GFN.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

* Re: [PATCH] VT-d: don't suppress invalidation address write when it is zero
  2015-09-28 14:32 [PATCH] VT-d: don't suppress invalidation address write when it is zero Jan Beulich
  2015-09-28 16:01 ` Andrew Cooper
@ 2015-10-10  5:35 ` Zhang, Yang Z
  1 sibling, 0 replies; 3+ messages in thread
From: Zhang, Yang Z @ 2015-10-10  5:35 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Tian, Kevin

Jan Beulich wrote on 2015-09-28:
> GFN zero is a valid address, and hence may need invalidation done for 
> it just like for any other GFN.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Yang Zhang <yang.z.zhang@intel.com>

> ---
> The comment right before the respective dmar_writeq() confuses me:
> What "first" TLB reg does it talk about? Is it simply stale (albeit it 
> has been there already in 3.2.x, i.e. it would then likely have been 
> stale already at the time that code got added)?

I have no idea about the 'first TLB' and didn't find any clue from spec.

> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -414,7 +414,7 @@ static int flush_iotlb_reg(void *_iommu,  {
>      struct iommu *iommu = (struct iommu *) _iommu;
>      int tlb_offset = ecap_iotlb_offset(iommu->ecap);
> -    u64 val = 0, val_iva = 0;
> +    u64 val = 0;
>      unsigned long flags;
>      
>      /* @@ -435,7 +435,6 @@ static int flush_iotlb_reg(void *_iommu,
>      switch ( type ) { case DMA_TLB_GLOBAL_FLUSH:
> -        /* global flush doesn't need set IVA_REG */
>          val = DMA_TLB_GLOBAL_FLUSH|DMA_TLB_IVT;
>          break;
>      case DMA_TLB_DSI_FLUSH:
> @@ -443,8 +442,6 @@ static int flush_iotlb_reg(void *_iommu,
>          break; case DMA_TLB_PSI_FLUSH: val =
>          DMA_TLB_PSI_FLUSH|DMA_TLB_IVT|DMA_TLB_DID(did);
> -        /* Note: always flush non-leaf currently */
> -        val_iva = size_order | addr;
>          break; default: BUG();
> @@ -457,8 +454,11 @@ static int flush_iotlb_reg(void *_iommu,
> 
>      spin_lock_irqsave(&iommu->register_lock, flags);
>      /* Note: Only uses first TLB reg currently */
> -    if ( val_iva )
> -        dmar_writeq(iommu->reg, tlb_offset, val_iva);
> +    if ( type == DMA_TLB_PSI_FLUSH )
> +    {
> +        /* Note: always flush non-leaf currently. */
> +        dmar_writeq(iommu->reg, tlb_offset, size_order | addr);
> +    }
>      dmar_writeq(iommu->reg, tlb_offset + 8, val);
>      
>      /* Make sure hardware complete it */
>


Best regards,
Yang

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

end of thread, other threads:[~2015-10-10  5:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-28 14:32 [PATCH] VT-d: don't suppress invalidation address write when it is zero Jan Beulich
2015-09-28 16:01 ` Andrew Cooper
2015-10-10  5:35 ` Zhang, Yang Z

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).