[PATCH] VT-d PI: disable VT-d PI when APICv is disabled

[PATCH] VT-d PI: disable VT-d PI when APICv is disabled

[Chao Gao]

From the context calling pi_desc_init(), we can conclude the current
implementation of VT-d PI depends on CPU-side PI. If we disable APICv
but enable VT-d PI explicitly in xen boot command line, we would get
an assertion failure.

This patch disables VT-d PI when APICv is disabled and adds some
related description to docs.
---
 docs/misc/xen-command-line.markdown | 3 ++-
 xen/arch/x86/hvm/vmx/vmcs.c         | 2 +-
 xen/drivers/passthrough/vtd/iommu.c | 6 +++++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 44d9985..29c2411 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1001,7 +1001,8 @@ debug hypervisor only).
 > Default: `false`
 
 >> Control the use of interrupt posting, which depends on the availability of
->> interrupt remapping.
+>> interrupt remapping and CPU-side interrupt posting which requires APIC
+>> Virtualization Extension is enabled.
 
 > `qinval` (VT-d)
 
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 8103b20..11bf03a 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -47,7 +47,7 @@ boolean_param("vpid", opt_vpid_enabled);
 static bool_t __read_mostly opt_unrestricted_guest_enabled = 1;
 boolean_param("unrestricted_guest", opt_unrestricted_guest_enabled);
 
-static bool_t __read_mostly opt_apicv_enabled = 1;
+bool_t __read_mostly opt_apicv_enabled = 1;
 boolean_param("apicv", opt_apicv_enabled);
 
 /*
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 19328f6..01d1470 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -41,6 +41,8 @@
 #include "vtd.h"
 #include "../ats.h"
 
+extern bool_t opt_apicv_enabled;
+
 struct mapped_rmrr {
     struct list_head list;
     u64 base, end;
@@ -2266,8 +2268,10 @@ int __init intel_vtd_setup(void)
          * 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.
+         * VT-d PI implementation relies on VMX PI. Thus, disable
+         * VT-d PI when VMX PI is disabled.
          */
-        if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
+        if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 || !opt_apicv_enabled )
             iommu_intpost = 0;
 
         if ( !vtd_ept_page_compatible(iommu) )
-- 
1.8.3.1


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

Re: [PATCH] VT-d PI: disable VT-d PI when APICv is disabled

[Jan Beulich]

>>> On 01.06.17 at 18:00, <chao.gao@intel.com> wrote:
> From the context calling pi_desc_init(), we can conclude the current
> implementation of VT-d PI depends on CPU-side PI. If we disable APICv
> but enable VT-d PI explicitly in xen boot command line, we would get
> an assertion failure.
> 
> This patch disables VT-d PI when APICv is disabled and adds some
> related description to docs.

There's no S-o-b here.

> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1001,7 +1001,8 @@ debug hypervisor only).
>  > Default: `false`
>  
>  >> Control the use of interrupt posting, which depends on the availability of
> ->> interrupt remapping.
> +>> interrupt remapping and CPU-side interrupt posting which requires APIC
> +>> Virtualization Extension is enabled.

"... interrupt posting, which in turn requires ..."?

Also APIC-V is default on, so maybe it would be better to say "is
not disabled" and perhaps even cross reference the other command
line option.

> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -47,7 +47,7 @@ boolean_param("vpid", opt_vpid_enabled);
>  static bool_t __read_mostly opt_unrestricted_guest_enabled = 1;
>  boolean_param("unrestricted_guest", opt_unrestricted_guest_enabled);
>  
> -static bool_t __read_mostly opt_apicv_enabled = 1;
> +bool_t __read_mostly opt_apicv_enabled = 1;

If you touch such code, please switch to bool/true/false at once.

> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -41,6 +41,8 @@
>  #include "vtd.h"
>  #include "../ats.h"
>  
> +extern bool_t opt_apicv_enabled;

No. Declarations need to go into a header which is being included
by both producer and consumer(s).

> @@ -2266,8 +2268,10 @@ int __init intel_vtd_setup(void)
>           * 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.
> +         * VT-d PI implementation relies on VMX PI. Thus, disable
> +         * VT-d PI when VMX PI is disabled.
>           */
> -        if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
> +        if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 || !opt_apicv_enabled )
>              iommu_intpost = 0;

Comment and code seem out of sync (talking about PI but
checking APIC-V).

Jan


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