Re: [Xen-devel] [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection

2015-07-08 Thread Tian, Kevin
 From: Wu, Feng
 Sent: Wednesday, June 24, 2015 1:18 PM
 
 VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
 With VT-d Posted-Interrupts enabled, external interrupts from
 direct-assigned devices can be delivered to guests without VMM
 intervention when guest is running in non-root mode.
 
 This patch adds feature detection logic for VT-d posted-interrupt.
 
 Signed-off-by: Feng Wu feng...@intel.com
 ---
 v3:
 - Remove the if no intremap then no intpost logic in
   intel_vtd_setup(), it is covered in the iommu_setup().
 - Add if no intremap then no intpost logic in the end
   of init_vtd_hw() which is called by vtd_resume().
 
 So the logic exists in the following three places:
 - parse_iommu_param()
 - iommu_setup()
 - init_vtd_hw()
 
  xen/drivers/passthrough/vtd/iommu.c | 18 --
  xen/drivers/passthrough/vtd/iommu.h |  1 +
  2 files changed, 17 insertions(+), 2 deletions(-)
 
 diff --git a/xen/drivers/passthrough/vtd/iommu.c
 b/xen/drivers/passthrough/vtd/iommu.c
 index 9053a1f..4221185 100644
 --- a/xen/drivers/passthrough/vtd/iommu.c
 +++ b/xen/drivers/passthrough/vtd/iommu.c
 @@ -2071,6 +2071,9 @@ static int init_vtd_hw(void)
  disable_intremap(drhd-iommu);
  }
 
 +if ( !iommu_intremap )
 +iommu_intpost = 0;
 +
  /*
   * Set root entries for each VT-d engine.  After set root entry,
   * must globally invalidate context cache, and then globally
 @@ -2133,8 +2136,8 @@ int __init intel_vtd_setup(void)
  }
 
  /* We enable the following features only if they are supported by all 
 VT-d
 - * engines: Snoop Control, DMA passthrough, Queued Invalidation and
 - * Interrupt Remapping.
 + * engines: Snoop Control, DMA passthrough, Queued Invalidation, 
 Interrupt
 + * Remapping, and Posted Interrupt
   */
  for_each_drhd_unit ( drhd )
  {
 @@ -2162,6 +2165,15 @@ int __init intel_vtd_setup(void)
  if ( iommu_intremap  !ecap_intr_remap(iommu-ecap) )
  iommu_intremap = 0;
 
 +/*
 + * We cannot use posted interrupt if X86_FEATURE_CX16 is
 + * not supported, since we count on this feature to
 + * atomically update 16-byte IRTE in posted format.
 + */
 +if ( !iommu_intremap 
 + (!cap_intr_post(iommu-cap) || !cpu_has_cx16) )
 +iommu_intpost = 0;
 +

Looks a typo here. -||

Thanks
Kevin

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


Re: [Xen-devel] [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection

2015-07-08 Thread Wu, Feng


 -Original Message-
 From: Tian, Kevin
 Sent: Wednesday, July 08, 2015 3:32 PM
 To: Wu, Feng; xen-devel@lists.xen.org
 Cc: k...@xen.org; jbeul...@suse.com; andrew.coop...@citrix.com; Zhang,
 Yang Z; george.dun...@eu.citrix.com
 Subject: RE: [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection
 
  From: Wu, Feng
  Sent: Wednesday, June 24, 2015 1:18 PM
 
  VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
  With VT-d Posted-Interrupts enabled, external interrupts from
  direct-assigned devices can be delivered to guests without VMM
  intervention when guest is running in non-root mode.
 
  This patch adds feature detection logic for VT-d posted-interrupt.
 
  Signed-off-by: Feng Wu feng...@intel.com
  ---
  v3:
  - Remove the if no intremap then no intpost logic in
intel_vtd_setup(), it is covered in the iommu_setup().
  - Add if no intremap then no intpost logic in the end
of init_vtd_hw() which is called by vtd_resume().
 
  So the logic exists in the following three places:
  - parse_iommu_param()
  - iommu_setup()
  - init_vtd_hw()
 
   xen/drivers/passthrough/vtd/iommu.c | 18 --
   xen/drivers/passthrough/vtd/iommu.h |  1 +
   2 files changed, 17 insertions(+), 2 deletions(-)
 
  diff --git a/xen/drivers/passthrough/vtd/iommu.c
  b/xen/drivers/passthrough/vtd/iommu.c
  index 9053a1f..4221185 100644
  --- a/xen/drivers/passthrough/vtd/iommu.c
  +++ b/xen/drivers/passthrough/vtd/iommu.c
  @@ -2071,6 +2071,9 @@ static int init_vtd_hw(void)
   disable_intremap(drhd-iommu);
   }
 
  +if ( !iommu_intremap )
  +iommu_intpost = 0;
  +
   /*
* Set root entries for each VT-d engine.  After set root entry,
* must globally invalidate context cache, and then globally
  @@ -2133,8 +2136,8 @@ int __init intel_vtd_setup(void)
   }
 
   /* We enable the following features only if they are supported by all
 VT-d
  - * engines: Snoop Control, DMA passthrough, Queued Invalidation and
  - * Interrupt Remapping.
  + * engines: Snoop Control, DMA passthrough, Queued Invalidation,
 Interrupt
  + * Remapping, and Posted Interrupt
*/
   for_each_drhd_unit ( drhd )
   {
  @@ -2162,6 +2165,15 @@ int __init intel_vtd_setup(void)
   if ( iommu_intremap  !ecap_intr_remap(iommu-ecap) )
   iommu_intremap = 0;
 
  +/*
  + * We cannot use posted interrupt if X86_FEATURE_CX16 is
  + * not supported, since we count on this feature to
  + * atomically update 16-byte IRTE in posted format.
  + */
  +if ( !iommu_intremap 
  + (!cap_intr_post(iommu-cap) || !cpu_has_cx16) )
  +iommu_intpost = 0;
  +
 
 Looks a typo here. -||

Yes, this is a typo. Thanks for the review.

Thanks,
Feng
 
 Thanks
 Kevin

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


Re: [Xen-devel] [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection

2015-06-25 Thread Wu, Feng


 -Original Message-
 From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
 Sent: Thursday, June 25, 2015 6:21 PM
 To: Wu, Feng; xen-devel@lists.xen.org
 Cc: k...@xen.org; jbeul...@suse.com; Tian, Kevin; Zhang, Yang Z;
 george.dun...@eu.citrix.com
 Subject: Re: [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection
 
 On 24/06/15 06:18, Feng Wu wrote:
  VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
  With VT-d Posted-Interrupts enabled, external interrupts from
  direct-assigned devices can be delivered to guests without VMM
  intervention when guest is running in non-root mode.
 
  This patch adds feature detection logic for VT-d posted-interrupt.
 
  Signed-off-by: Feng Wu feng...@intel.com
  ---
  v3:
  - Remove the if no intremap then no intpost logic in
intel_vtd_setup(), it is covered in the iommu_setup().
  - Add if no intremap then no intpost logic in the end
of init_vtd_hw() which is called by vtd_resume().
 
  So the logic exists in the following three places:
  - parse_iommu_param()
  - iommu_setup()
  - init_vtd_hw()
 
   xen/drivers/passthrough/vtd/iommu.c | 18 --
   xen/drivers/passthrough/vtd/iommu.h |  1 +
   2 files changed, 17 insertions(+), 2 deletions(-)
 
  diff --git a/xen/drivers/passthrough/vtd/iommu.c
 b/xen/drivers/passthrough/vtd/iommu.c
  index 9053a1f..4221185 100644
  --- a/xen/drivers/passthrough/vtd/iommu.c
  +++ b/xen/drivers/passthrough/vtd/iommu.c
  @@ -2071,6 +2071,9 @@ static int init_vtd_hw(void)
   disable_intremap(drhd-iommu);
   }
 
  +if ( !iommu_intremap )
  +iommu_intpost = 0;
  +
 
 Yet again, this curious logic.
 
 There should be exactly 1 place which performs this dependency check,
 not scattered in multiple places.

Seems, it is a little hard to find the one right place to perform this check.
But I will think this more.

 
   /*
* Set root entries for each VT-d engine.  After set root entry,
* must globally invalidate context cache, and then globally
  @@ -2133,8 +2136,8 @@ int __init intel_vtd_setup(void)
   }
 
   /* We enable the following features only if they are supported by all
 VT-d
  - * engines: Snoop Control, DMA passthrough, Queued Invalidation and
  - * Interrupt Remapping.
  + * engines: Snoop Control, DMA passthrough, Queued Invalidation,
 Interrupt
  + * Remapping, and Posted Interrupt
*/
   for_each_drhd_unit ( drhd )
   {
  @@ -2162,6 +2165,15 @@ int __init intel_vtd_setup(void)
   if ( iommu_intremap  !ecap_intr_remap(iommu-ecap) )
   iommu_intremap = 0;
 
  +/*
  + * We cannot use posted interrupt if X86_FEATURE_CX16 is
  + * not supported, since we count on this feature to
  + * atomically update 16-byte IRTE in posted format.
  + */
  +if ( !iommu_intremap 
  + (!cap_intr_post(iommu-cap) || !cpu_has_cx16) )
  +iommu_intpost = 0;
 
 intremap is not relevant to CX16 being a prerequisite of intpost.
 

I think I made a typo here, it should be like this:

+if ( !iommu_intremap || !cap_intr_post(iommu-cap) || !cpu_has_cx16 )
+iommu_intpost = 0;

Thanks,
Feng

 ~Andrew
 
  +
   if ( !vtd_ept_page_compatible(iommu) )
   iommu_hap_pt_share = 0;
 
  @@ -2187,6 +2199,7 @@ int __init intel_vtd_setup(void)
   P(iommu_passthrough, Dom0 DMA Passthrough);
   P(iommu_qinval, Queued Invalidation);
   P(iommu_intremap, Interrupt Remapping);
  +P(iommu_intpost, Posted Interrupt);
   P(iommu_hap_pt_share, Shared EPT tables);
   #undef P
 
  @@ -2206,6 +2219,7 @@ int __init intel_vtd_setup(void)
   iommu_passthrough = 0;
   iommu_qinval = 0;
   iommu_intremap = 0;
  +iommu_intpost = 0;
   return ret;
   }
 
  diff --git a/xen/drivers/passthrough/vtd/iommu.h
 b/xen/drivers/passthrough/vtd/iommu.h
  index 80f8830..e807253 100644
  --- a/xen/drivers/passthrough/vtd/iommu.h
  +++ b/xen/drivers/passthrough/vtd/iommu.h
  @@ -69,6 +69,7 @@
   /*
* Decoding Capability Register
*/
  +#define cap_intr_post(c)   (((c)  59)  1)
   #define cap_read_drain(c)  (((c)  55)  1)
   #define cap_write_drain(c) (((c)  54)  1)
   #define cap_max_amask_val(c)   (((c)  48)  0x3f)


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


Re: [Xen-devel] [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection

2015-06-25 Thread Andrew Cooper
On 24/06/15 06:18, Feng Wu wrote:
 VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
 With VT-d Posted-Interrupts enabled, external interrupts from
 direct-assigned devices can be delivered to guests without VMM
 intervention when guest is running in non-root mode.

 This patch adds feature detection logic for VT-d posted-interrupt.

 Signed-off-by: Feng Wu feng...@intel.com
 ---
 v3:
 - Remove the if no intremap then no intpost logic in
   intel_vtd_setup(), it is covered in the iommu_setup().
 - Add if no intremap then no intpost logic in the end
   of init_vtd_hw() which is called by vtd_resume().

 So the logic exists in the following three places:
 - parse_iommu_param()
 - iommu_setup()
 - init_vtd_hw()

  xen/drivers/passthrough/vtd/iommu.c | 18 --
  xen/drivers/passthrough/vtd/iommu.h |  1 +
  2 files changed, 17 insertions(+), 2 deletions(-)

 diff --git a/xen/drivers/passthrough/vtd/iommu.c 
 b/xen/drivers/passthrough/vtd/iommu.c
 index 9053a1f..4221185 100644
 --- a/xen/drivers/passthrough/vtd/iommu.c
 +++ b/xen/drivers/passthrough/vtd/iommu.c
 @@ -2071,6 +2071,9 @@ static int init_vtd_hw(void)
  disable_intremap(drhd-iommu);
  }
  
 +if ( !iommu_intremap )
 +iommu_intpost = 0;
 +

Yet again, this curious logic.

There should be exactly 1 place which performs this dependency check,
not scattered in multiple places.

  /*
   * Set root entries for each VT-d engine.  After set root entry,
   * must globally invalidate context cache, and then globally
 @@ -2133,8 +2136,8 @@ int __init intel_vtd_setup(void)
  }
  
  /* We enable the following features only if they are supported by all 
 VT-d
 - * engines: Snoop Control, DMA passthrough, Queued Invalidation and
 - * Interrupt Remapping.
 + * engines: Snoop Control, DMA passthrough, Queued Invalidation, 
 Interrupt
 + * Remapping, and Posted Interrupt
   */
  for_each_drhd_unit ( drhd )
  {
 @@ -2162,6 +2165,15 @@ int __init intel_vtd_setup(void)
  if ( iommu_intremap  !ecap_intr_remap(iommu-ecap) )
  iommu_intremap = 0;
  
 +/*
 + * We cannot use posted interrupt if X86_FEATURE_CX16 is
 + * not supported, since we count on this feature to
 + * atomically update 16-byte IRTE in posted format.
 + */
 +if ( !iommu_intremap 
 + (!cap_intr_post(iommu-cap) || !cpu_has_cx16) )
 +iommu_intpost = 0;

intremap is not relevant to CX16 being a prerequisite of intpost.

~Andrew

 +
  if ( !vtd_ept_page_compatible(iommu) )
  iommu_hap_pt_share = 0;
  
 @@ -2187,6 +2199,7 @@ int __init intel_vtd_setup(void)
  P(iommu_passthrough, Dom0 DMA Passthrough);
  P(iommu_qinval, Queued Invalidation);
  P(iommu_intremap, Interrupt Remapping);
 +P(iommu_intpost, Posted Interrupt);
  P(iommu_hap_pt_share, Shared EPT tables);
  #undef P
  
 @@ -2206,6 +2219,7 @@ int __init intel_vtd_setup(void)
  iommu_passthrough = 0;
  iommu_qinval = 0;
  iommu_intremap = 0;
 +iommu_intpost = 0;
  return ret;
  }
  
 diff --git a/xen/drivers/passthrough/vtd/iommu.h 
 b/xen/drivers/passthrough/vtd/iommu.h
 index 80f8830..e807253 100644
 --- a/xen/drivers/passthrough/vtd/iommu.h
 +++ b/xen/drivers/passthrough/vtd/iommu.h
 @@ -69,6 +69,7 @@
  /*
   * Decoding Capability Register
   */
 +#define cap_intr_post(c)   (((c)  59)  1)
  #define cap_read_drain(c)  (((c)  55)  1)
  #define cap_write_drain(c) (((c)  54)  1)
  #define cap_max_amask_val(c)   (((c)  48)  0x3f)


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


[Xen-devel] [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection

2015-06-23 Thread Feng Wu
VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
With VT-d Posted-Interrupts enabled, external interrupts from
direct-assigned devices can be delivered to guests without VMM
intervention when guest is running in non-root mode.

This patch adds feature detection logic for VT-d posted-interrupt.

Signed-off-by: Feng Wu feng...@intel.com
---
v3:
- Remove the if no intremap then no intpost logic in
  intel_vtd_setup(), it is covered in the iommu_setup().
- Add if no intremap then no intpost logic in the end
  of init_vtd_hw() which is called by vtd_resume().

So the logic exists in the following three places:
- parse_iommu_param()
- iommu_setup()
- init_vtd_hw()

 xen/drivers/passthrough/vtd/iommu.c | 18 --
 xen/drivers/passthrough/vtd/iommu.h |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 9053a1f..4221185 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2071,6 +2071,9 @@ static int init_vtd_hw(void)
 disable_intremap(drhd-iommu);
 }
 
+if ( !iommu_intremap )
+iommu_intpost = 0;
+
 /*
  * Set root entries for each VT-d engine.  After set root entry,
  * must globally invalidate context cache, and then globally
@@ -2133,8 +2136,8 @@ int __init intel_vtd_setup(void)
 }
 
 /* We enable the following features only if they are supported by all VT-d
- * engines: Snoop Control, DMA passthrough, Queued Invalidation and
- * Interrupt Remapping.
+ * engines: Snoop Control, DMA passthrough, Queued Invalidation, Interrupt
+ * Remapping, and Posted Interrupt
  */
 for_each_drhd_unit ( drhd )
 {
@@ -2162,6 +2165,15 @@ int __init intel_vtd_setup(void)
 if ( iommu_intremap  !ecap_intr_remap(iommu-ecap) )
 iommu_intremap = 0;
 
+/*
+ * We cannot use posted interrupt if X86_FEATURE_CX16 is
+ * not supported, since we count on this feature to
+ * atomically update 16-byte IRTE in posted format.
+ */
+if ( !iommu_intremap 
+ (!cap_intr_post(iommu-cap) || !cpu_has_cx16) )
+iommu_intpost = 0;
+
 if ( !vtd_ept_page_compatible(iommu) )
 iommu_hap_pt_share = 0;
 
@@ -2187,6 +2199,7 @@ int __init intel_vtd_setup(void)
 P(iommu_passthrough, Dom0 DMA Passthrough);
 P(iommu_qinval, Queued Invalidation);
 P(iommu_intremap, Interrupt Remapping);
+P(iommu_intpost, Posted Interrupt);
 P(iommu_hap_pt_share, Shared EPT tables);
 #undef P
 
@@ -2206,6 +2219,7 @@ int __init intel_vtd_setup(void)
 iommu_passthrough = 0;
 iommu_qinval = 0;
 iommu_intremap = 0;
+iommu_intpost = 0;
 return ret;
 }
 
diff --git a/xen/drivers/passthrough/vtd/iommu.h 
b/xen/drivers/passthrough/vtd/iommu.h
index 80f8830..e807253 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -69,6 +69,7 @@
 /*
  * Decoding Capability Register
  */
+#define cap_intr_post(c)   (((c)  59)  1)
 #define cap_read_drain(c)  (((c)  55)  1)
 #define cap_write_drain(c) (((c)  54)  1)
 #define cap_max_amask_val(c)   (((c)  48)  0x3f)
-- 
2.1.0


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