[PATCH v7 0/3] Expose HW APIC virtualization support to HVM guests

[PATCH v7 0/3] Expose HW APIC virtualization support to HVM guests

[Boris Ostrovsky]

Version 7:
* Enforce user-specified number of leaves between 1 and XEN_CPUID_MAX_NUM_LEAVES
* Merge Solaris patch reversal into the first patch

Version 6:
* Drop generic support for changing hypervisor leaves, only allow modifying max
  number of leaves, ignore all other bit change requests.

Version 5:
* Remember to deal with Viridian leaves in xc_cpuid_constrain()

Version 4:
* asm changes per Jan's suggestions
* Don't pass leaf index number to handlers, check for sub_idx==0
* Added (possibly somewhat overwrought) interface to constrain user-requested
  CPUID to what is allowed (specifically, for leaf 0x40000000 only EAX[7:0]
  can be ovverwritten).

Version 3:
* Removed sysctl for querying hypervisor leaf from libxc, replaced it with
  prefixed CPUID instruction
* Limited ability change hypervisor leaves to 0x40000000
* Fixed IS_HYPERVISOR_LEAF macro

Version 2:
* Added ability to specify hypervisor CPUID leaves in config file (this requires
  new sysctl)
* Use 2 bits to indicate what is supported --- one for APIC memory access and the
  other for x2APIC. Still not sure whether virtual interrupt delivery should be
  exposed as well.



HVM guests running on HW that supports HW APIC virtualization features
(APIC-register virtualization, virtual interrupt delivery, etc) may
want to use APIC instead of hvm_pirqs. Since we are not guaranteed to
have these features on VMX (for example, there is a boot option to
turn it off) and there is no such support on SVM we need to make the
guest aware that its APIC accesses may not be so bad.

CPUID seems to be a good way to provide this info to the guest.

Having a guest switch to APIC shows fairly good impact on number of
VMEXITs. For example, with a pass-through NIC, netperf sees almost
half as many. Here are results for 'xentrace -e 0x00083fff -c 2 -D -T 2'

(The guest here essentially turned off XENFEAT_hvm_pirqs but we may
want to use APIC for MSI interrupts only and leave pirqs for gsi).


[root@ovs105 virt]#  cat orig  |xentrace_format  ~/xen/tools/xentrace/formats | awk  '{print $5}' | sort  | uniq -c
     94 cpu_change
  13944 HLT
  26341 INJ_VIRQ
  12054 INTR
  30784 INTR_WINDOW
  10126 TRAP
 124783 VMENTRY
 124782 VMEXIT
  59217 VMMCALL
     35 wrap_buffer
 
 
[root@ovs105 virt]#  cat apicv  |xentrace_format  ~/xen/tools/xentrace/formats | awk  '{print $5}' | sort  | uniq -c
     49 cpu_change
  16157 HLT
     31 INJ_VIRQ
  10652 INTR
     38 INTR_WINDOW
     10 NPF
  10286 TRAP
  71269 VMENTRY
  71269 VMEXIT
  34129 VMMCALL
     15 wrap_buffer

The difference is even larger when the guest is busy.

These results are in line with what has been reported for KVM. For example
http://events.linuxfoundation.org/sites/events/files/cojp13_natapov.pdf
http://www.linuxplumbersconf.org/2012/wp-content/uploads/2012/09/2012-lpc-virt-intel-vt-feat-nakajima.pdf

I am also not sure whether (cpu_has_vmx_apic_reg_virt &
cpu_has_vmx_virtualize_x2apic_mode) is sufficient to declare full HW
APIC support to a guest. The tests show ~95K VMEXITs when virtual
interrupt delivery and posted interrupts are turned off so there
appears to still be some benefit. I suppose we can use another CPUID
bit for these two (although I am not particularly eager to do this).

Boris Ostrovsky (3):
  xen/libxc: Allow changing max number of hypervisor cpuid leaves
  x86/hvm: Add HVM-specific hypervisor CPUID leaf
  x86/hvm: Indicate avaliability of HW support of APIC virtualization
    to HVM guests

 tools/libxc/xc_cpuid_x86.c          |   11 +++++++++++
 xen/arch/x86/hvm/hvm.c              |    9 +++++++++
 xen/arch/x86/hvm/vmx/vmx.c          |   15 +++++++++++++++
 xen/arch/x86/traps.c                |   20 +++++++++++++-------
 xen/include/asm-x86/hvm/hvm.h       |    7 +++++++
 xen/include/public/arch-x86/cpuid.h |   11 +++++++++++
 6 files changed, 66 insertions(+), 7 deletions(-)

-- 
1.7.10.4

[PATCH v7 1/3] xen/libxc: Allow changing max number of hypervisor cpuid leaves

[Boris Ostrovsky]

Add support for changing max number of hypervisor leaves from configuration
file.

This number can be specified using xl's standard 'cpuid' option. Only lowest
8 bits of leaf's 0x4000xx00 eax register are processed, all others are ignored.

The changes allow us to revert commit 80ecb40362365ba77e68fc609de8bd3b7208ae19
which is most likely no longer needed now anyway (Solaris bug that it addressed
has been fixed and backported to earlier releases) but leave possibility of
running unpatched version of Solaris by forcing number of leaves to 2 in the
configuration file.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 tools/libxc/xc_cpuid_x86.c          |   11 +++++++++++
 xen/arch/x86/traps.c                |   16 +++++++++-------
 xen/include/public/arch-x86/cpuid.h |    2 ++
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index bbbf9b8..45adcf0 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -555,6 +555,17 @@ static int xc_cpuid_policy(
 {
     xc_dominfo_t        info;
 
+    /*
+     * For hypervisor leaves (0x4000XXXX) only 0x4000xx00.EAX[7:0] bits (max
+     * number of leaves) can be set by user. Hypervisor will enforce this so
+     * all other bits are don't-care and we can set them to zero.
+     */
+    if ( (input[0] & 0xffff0000) == 0x40000000 )
+    {
+        regs[0] = regs[1] = regs[2] = regs[3] = 0;
+        return 0;
+    }
+
     if ( xc_domain_getinfo(xch, domid, 1, &info) == 0 )
         return -EINVAL;
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index c462317..e4dec4b 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -677,15 +677,17 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
     struct domain *d = current->domain;
     /* Optionally shift out of the way of Viridian architectural leaves. */
     uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
-    uint32_t limit;
+    uint32_t limit, dummy;
 
     idx -= base;
-
-    /*
-     * Some Solaris PV drivers fail if max > base + 2. Help them out by
-     * hiding the PVRDTSCP leaf if PVRDTSCP is disabled.
-     */
-    limit = (d->arch.tsc_mode < TSC_MODE_PVRDTSCP) ? 2 : 3;
+    if ( idx > XEN_CPUID_MAX_NUM_LEAVES )
+        return 0; /* Avoid unnecessary pass through domain_cpuid() */
+
+    /* Number of leaves may be user-specified */
+    domain_cpuid(d, base, 0, &limit, &dummy, &dummy, &dummy);
+    limit &= 0xff;
+    if ( (limit < 1) || (limit > XEN_CPUID_MAX_NUM_LEAVES) )
+        limit = XEN_CPUID_MAX_NUM_LEAVES;
 
     if ( idx > limit ) 
         return 0;
diff --git a/xen/include/public/arch-x86/cpuid.h b/xen/include/public/arch-x86/cpuid.h
index d9bd627..19fc9dd 100644
--- a/xen/include/public/arch-x86/cpuid.h
+++ b/xen/include/public/arch-x86/cpuid.h
@@ -65,4 +65,6 @@
 #define _XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD 0
 #define XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD  (1u<<0)
 
+#define XEN_CPUID_MAX_NUM_LEAVES 3
+
 #endif /* __XEN_PUBLIC_ARCH_X86_CPUID_H__ */
-- 
1.7.10.4

[PATCH v7 2/3] x86/hvm: Add HVM-specific hypervisor CPUID leaf

[Boris Ostrovsky]

CPUID leaf 0x40000004 is for HVM-specific features.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 xen/arch/x86/hvm/hvm.c              |    9 +++++++++
 xen/arch/x86/traps.c                |    4 ++++
 xen/include/asm-x86/hvm/hvm.h       |    7 +++++++
 xen/include/public/arch-x86/cpuid.h |    7 ++++++-
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ae24211..b07f11e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2980,6 +2980,15 @@ unsigned long copy_from_user_hvm(void *to, const void *from, unsigned len)
     return rc ? len : 0; /* fake a copy_from_user() return code */
 }
 
+void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
+                               uint32_t *eax, uint32_t *ebx,
+                               uint32_t *ecx, uint32_t *edx)
+{
+    *eax = *ebx = *ecx = *edx = 0;
+    if ( hvm_funcs.hypervisor_cpuid_leaf )
+        hvm_funcs.hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
+}
+
 void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
                                    unsigned int *ecx, unsigned int *edx)
 {
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e4dec4b..5c4ef4d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -724,6 +724,10 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
         cpuid_time_leaf( sub_idx, eax, ebx, ecx, edx );
         break;
 
+    case 4:
+        hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
+        break;
+
     default:
         BUG();
     }
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index dcc3483..a030ea4 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -200,6 +200,10 @@ struct hvm_function_table {
                                 paddr_t *L1_gpa, unsigned int *page_order,
                                 uint8_t *p2m_acc, bool_t access_r,
                                 bool_t access_w, bool_t access_x);
+
+    void (*hypervisor_cpuid_leaf)(uint32_t sub_idx,
+                                  uint32_t *eax, uint32_t *ebx,
+                                  uint32_t *ecx, uint32_t *edx);
 };
 
 extern struct hvm_function_table hvm_funcs;
@@ -336,6 +340,9 @@ static inline unsigned long hvm_get_shadow_gs_base(struct vcpu *v)
 #define is_viridian_domain(_d)                                             \
  (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN]))
 
+void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
+                               uint32_t *eax, uint32_t *ebx,
+                               uint32_t *ecx, uint32_t *edx);
 void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
                                    unsigned int *ecx, unsigned int *edx);
 void hvm_migrate_timers(struct vcpu *v);
diff --git a/xen/include/public/arch-x86/cpuid.h b/xen/include/public/arch-x86/cpuid.h
index 19fc9dd..fff972a 100644
--- a/xen/include/public/arch-x86/cpuid.h
+++ b/xen/include/public/arch-x86/cpuid.h
@@ -65,6 +65,11 @@
 #define _XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD 0
 #define XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD  (1u<<0)
 
-#define XEN_CPUID_MAX_NUM_LEAVES 3
+/*
+ * Leaf 5 (0x40000004)
+ * HVM-specific features
+ */
+
+#define XEN_CPUID_MAX_NUM_LEAVES 4
 
 #endif /* __XEN_PUBLIC_ARCH_X86_CPUID_H__ */
-- 
1.7.10.4

[PATCH v7 3/3] x86/hvm: Indicate avaliability of HW support of APIC virtualization to HVM guests

[Boris Ostrovsky]

Set bits in hypervisor CPUID leaf indicating that HW provides (and the
hypervisor enables) HW support for APIC and x2APIC virtualization.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 xen/arch/x86/hvm/vmx/vmx.c          |   15 +++++++++++++++
 xen/include/public/arch-x86/cpuid.h |    4 ++++
 2 files changed, 19 insertions(+)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 8395e86..a59bac6 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -57,6 +57,7 @@
 #include <asm/apic.h>
 #include <asm/hvm/nestedhvm.h>
 #include <asm/event.h>
+#include <public/arch-x86/cpuid.h>
 
 enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
 
@@ -1646,6 +1647,19 @@ static void vmx_handle_eoi(u8 vector)
     __vmwrite(GUEST_INTR_STATUS, status);
 }
 
+void vmx_hypervisor_cpuid_leaf(uint32_t sub_idx,
+                               uint32_t *eax, uint32_t *ebx,
+                               uint32_t *ecx, uint32_t *edx)
+{
+    if ( sub_idx != 0 )
+        return;
+
+    if ( cpu_has_vmx_apic_reg_virt )
+        *eax |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
+    if ( cpu_has_vmx_virtualize_x2apic_mode )
+        *eax |= XEN_HVM_CPUID_X2APIC_VIRT;
+}
+
 static struct hvm_function_table __initdata vmx_function_table = {
     .name                 = "VMX",
     .cpu_up_prepare       = vmx_cpu_up_prepare,
@@ -1703,6 +1717,7 @@ static struct hvm_function_table __initdata vmx_function_table = {
     .sync_pir_to_irr      = vmx_sync_pir_to_irr,
     .handle_eoi           = vmx_handle_eoi,
     .nhvm_hap_walk_L1_p2m = nvmx_hap_walk_L1_p2m,
+    .hypervisor_cpuid_leaf= vmx_hypervisor_cpuid_leaf,
 };
 
 const struct hvm_function_table * __init start_vmx(void)
diff --git a/xen/include/public/arch-x86/cpuid.h b/xen/include/public/arch-x86/cpuid.h
index fff972a..7135d54 100644
--- a/xen/include/public/arch-x86/cpuid.h
+++ b/xen/include/public/arch-x86/cpuid.h
@@ -70,6 +70,10 @@
  * HVM-specific features
  */
 
+/* EAX Features */
+#define XEN_HVM_CPUID_APIC_ACCESS_VIRT (1u << 0) /* Virtualized APIC registers */
+#define XEN_HVM_CPUID_X2APIC_VIRT      (1u << 1) /* Virtualized x2APIC accesses */
+
 #define XEN_CPUID_MAX_NUM_LEAVES 4
 
 #endif /* __XEN_PUBLIC_ARCH_X86_CPUID_H__ */
-- 
1.7.10.4

Re: [PATCH v7 1/3] xen/libxc: Allow changing max number of hypervisor cpuid leaves

[Jan Beulich]

>>> On 25.03.14 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> Add support for changing max number of hypervisor leaves from configuration
> file.
> 
> This number can be specified using xl's standard 'cpuid' option. Only lowest
> 8 bits of leaf's 0x4000xx00 eax register are processed, all others are 
> ignored.
> 
> The changes allow us to revert commit 
> 80ecb40362365ba77e68fc609de8bd3b7208ae19
> which is most likely no longer needed now anyway (Solaris bug that it 
> addressed
> has been fixed and backported to earlier releases) but leave possibility of
> running unpatched version of Solaris by forcing number of leaves to 2 in the
> configuration file.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-off-by: Jan Beulich <jbeulich@suse.com>

> ---
>  tools/libxc/xc_cpuid_x86.c          |   11 +++++++++++
>  xen/arch/x86/traps.c                |   16 +++++++++-------
>  xen/include/public/arch-x86/cpuid.h |    2 ++
>  3 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index bbbf9b8..45adcf0 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -555,6 +555,17 @@ static int xc_cpuid_policy(
>  {
>      xc_dominfo_t        info;
>  
> +    /*
> +     * For hypervisor leaves (0x4000XXXX) only 0x4000xx00.EAX[7:0] bits 
> (max
> +     * number of leaves) can be set by user. Hypervisor will enforce this 
> so
> +     * all other bits are don't-care and we can set them to zero.
> +     */
> +    if ( (input[0] & 0xffff0000) == 0x40000000 )
> +    {
> +        regs[0] = regs[1] = regs[2] = regs[3] = 0;
> +        return 0;
> +    }
> +
>      if ( xc_domain_getinfo(xch, domid, 1, &info) == 0 )
>          return -EINVAL;
>  
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index c462317..e4dec4b 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -677,15 +677,17 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t 
> sub_idx,
>      struct domain *d = current->domain;
>      /* Optionally shift out of the way of Viridian architectural leaves. */
>      uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
> -    uint32_t limit;
> +    uint32_t limit, dummy;
>  
>      idx -= base;
> -
> -    /*
> -     * Some Solaris PV drivers fail if max > base + 2. Help them out by
> -     * hiding the PVRDTSCP leaf if PVRDTSCP is disabled.
> -     */
> -    limit = (d->arch.tsc_mode < TSC_MODE_PVRDTSCP) ? 2 : 3;
> +    if ( idx > XEN_CPUID_MAX_NUM_LEAVES )
> +        return 0; /* Avoid unnecessary pass through domain_cpuid() */
> +
> +    /* Number of leaves may be user-specified */
> +    domain_cpuid(d, base, 0, &limit, &dummy, &dummy, &dummy);
> +    limit &= 0xff;
> +    if ( (limit < 1) || (limit > XEN_CPUID_MAX_NUM_LEAVES) )
> +        limit = XEN_CPUID_MAX_NUM_LEAVES;
>  
>      if ( idx > limit ) 
>          return 0;
> diff --git a/xen/include/public/arch-x86/cpuid.h 
> b/xen/include/public/arch-x86/cpuid.h
> index d9bd627..19fc9dd 100644
> --- a/xen/include/public/arch-x86/cpuid.h
> +++ b/xen/include/public/arch-x86/cpuid.h
> @@ -65,4 +65,6 @@
>  #define _XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD 0
>  #define XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD  (1u<<0)
>  
> +#define XEN_CPUID_MAX_NUM_LEAVES 3
> +
>  #endif /* __XEN_PUBLIC_ARCH_X86_CPUID_H__ */
> -- 
> 1.7.10.4

Re: [PATCH v7 2/3] x86/hvm: Add HVM-specific hypervisor CPUID leaf

[Jan Beulich]

>>> On 25.03.14 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> CPUID leaf 0x40000004 is for HVM-specific features.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-off-by: Jan Beulich <jbeulich@suse.com>

> ---
>  xen/arch/x86/hvm/hvm.c              |    9 +++++++++
>  xen/arch/x86/traps.c                |    4 ++++
>  xen/include/asm-x86/hvm/hvm.h       |    7 +++++++
>  xen/include/public/arch-x86/cpuid.h |    7 ++++++-
>  4 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ae24211..b07f11e 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2980,6 +2980,15 @@ unsigned long copy_from_user_hvm(void *to, const void 
> *from, unsigned len)
>      return rc ? len : 0; /* fake a copy_from_user() return code */
>  }
>  
> +void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
> +                               uint32_t *eax, uint32_t *ebx,
> +                               uint32_t *ecx, uint32_t *edx)
> +{
> +    *eax = *ebx = *ecx = *edx = 0;
> +    if ( hvm_funcs.hypervisor_cpuid_leaf )
> +        hvm_funcs.hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
> +}
> +
>  void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>                                     unsigned int *ecx, unsigned int *edx)
>  {
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index e4dec4b..5c4ef4d 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -724,6 +724,10 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t 
> sub_idx,
>          cpuid_time_leaf( sub_idx, eax, ebx, ecx, edx );
>          break;
>  
> +    case 4:
> +        hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
> +        break;
> +
>      default:
>          BUG();
>      }
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index dcc3483..a030ea4 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -200,6 +200,10 @@ struct hvm_function_table {
>                                  paddr_t *L1_gpa, unsigned int *page_order,
>                                  uint8_t *p2m_acc, bool_t access_r,
>                                  bool_t access_w, bool_t access_x);
> +
> +    void (*hypervisor_cpuid_leaf)(uint32_t sub_idx,
> +                                  uint32_t *eax, uint32_t *ebx,
> +                                  uint32_t *ecx, uint32_t *edx);
>  };
>  
>  extern struct hvm_function_table hvm_funcs;
> @@ -336,6 +340,9 @@ static inline unsigned long hvm_get_shadow_gs_base(struct 
> vcpu *v)
>  #define is_viridian_domain(_d)                                             
> \
>   (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN]))
>  
> +void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
> +                               uint32_t *eax, uint32_t *ebx,
> +                               uint32_t *ecx, uint32_t *edx);
>  void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>                                     unsigned int *ecx, unsigned int *edx);
>  void hvm_migrate_timers(struct vcpu *v);
> diff --git a/xen/include/public/arch-x86/cpuid.h 
> b/xen/include/public/arch-x86/cpuid.h
> index 19fc9dd..fff972a 100644
> --- a/xen/include/public/arch-x86/cpuid.h
> +++ b/xen/include/public/arch-x86/cpuid.h
> @@ -65,6 +65,11 @@
>  #define _XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD 0
>  #define XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD  (1u<<0)
>  
> -#define XEN_CPUID_MAX_NUM_LEAVES 3
> +/*
> + * Leaf 5 (0x40000004)
> + * HVM-specific features
> + */
> +
> +#define XEN_CPUID_MAX_NUM_LEAVES 4
>  
>  #endif /* __XEN_PUBLIC_ARCH_X86_CPUID_H__ */
> -- 
> 1.7.10.4

Re: [PATCH v7 3/3] x86/hvm: Indicate avaliability of HW support of APIC virtualization to HVM guests

[Jan Beulich]

>>> On 25.03.14 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> +void vmx_hypervisor_cpuid_leaf(uint32_t sub_idx,
> +                               uint32_t *eax, uint32_t *ebx,
> +                               uint32_t *ecx, uint32_t *edx)
> +{
> +    if ( sub_idx != 0 )
> +        return;
> +
> +    if ( cpu_has_vmx_apic_reg_virt )
> +        *eax |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
> +    if ( cpu_has_vmx_virtualize_x2apic_mode )
> +        *eax |= XEN_HVM_CPUID_X2APIC_VIRT;
> +}

So did the two of you then settle on (a) needing to expose two bits
rather than just one and (b) these being the two relevant features
to expose?

Jan

Re: [PATCH v7 3/3] x86/hvm: Indicate avaliability of HW support of APIC virtualization to HVM guests

[Boris Ostrovsky]

On 03/25/2014 05:45 AM, Jan Beulich wrote:
>>>> On 25.03.14 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>> +void vmx_hypervisor_cpuid_leaf(uint32_t sub_idx,
>> +                               uint32_t *eax, uint32_t *ebx,
>> +                               uint32_t *ecx, uint32_t *edx)
>> +{
>> +    if ( sub_idx != 0 )
>> +        return;
>> +
>> +    if ( cpu_has_vmx_apic_reg_virt )
>> +        *eax |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
>> +    if ( cpu_has_vmx_virtualize_x2apic_mode )
>> +        *eax |= XEN_HVM_CPUID_X2APIC_VIRT;
>> +}
> So did the two of you then settle on (a) needing to expose two bits
> rather than just one and (b) these being the two relevant features
> to expose?

My argument is that we can't know which APIC model a guest uses and so 
both are needed. For PVHVM we default to APIC (MMIO accesses), I can't 
remember what unenlightened HVM Linux would do. And then there are other 
OSs.

For (b) having either (or both) of these two seems to be sufficient to 
bring down the number of VMEXITs when switching from pirqs to APIC. It's 
more important to agree on (a) since for (b) we can always add another bit.

-boris

Re: [PATCH v7 3/3] x86/hvm: Indicate avaliability of HW support of APIC virtualization to HVM guests

[Zhang, Yang Z]

Boris Ostrovsky wrote on 2014-03-25:
> On 03/25/2014 05:45 AM, Jan Beulich wrote:
>>>>> On 25.03.14 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>> +void vmx_hypervisor_cpuid_leaf(uint32_t sub_idx,
>>> +                               uint32_t *eax, uint32_t *ebx,
>>> +                               uint32_t *ecx, uint32_t *edx) {
>>> +    if ( sub_idx != 0 )
>>> +        return;
>>> +
>>> +    if ( cpu_has_vmx_apic_reg_virt )
>>> +        *eax |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
>>> +    if ( cpu_has_vmx_virtualize_x2apic_mode )
>>> +        *eax |= XEN_HVM_CPUID_X2APIC_VIRT; }
>> So did the two of you then settle on (a) needing to expose two bits
>> rather than just one and (b) these being the two relevant features
>> to expose?
> 
> My argument is that we can't know which APIC model a guest uses and so
> both are needed. For PVHVM we default to APIC (MMIO accesses), I can't
> remember what unenlightened HVM Linux would do. And then there are
> other OSs.
> 
> For (b) having either (or both) of these two seems to be sufficient to
> bring down the number of VMEXITs when switching from pirqs to APIC.
> It's more important to agree on (a) since for (b) we can always add another bit.

In currently Xen, virtualize_x2apic_mode takes effect only when APICv is enabled. Without APICv, virtualize_x2apic_mode is never set. 
Per your request, you only want to drop the pirqs if guest is using x2apic. So, just check it inside guest OS is enough. NB: to use x2apic for guest doesn't require the virtualize_x2apic_mode been set. 
 
> 
> -boris


Best regards,
Yang

Re: [PATCH v7 1/3] xen/libxc: Allow changing max number of hypervisor cpuid leaves

[Ian Campbell]

On Mon, 2014-03-24 at 19:18 -0400, Boris Ostrovsky wrote:
> Add support for changing max number of hypervisor leaves from configuration
> file.
> 
> This number can be specified using xl's standard 'cpuid' option. Only lowest
> 8 bits of leaf's 0x4000xx00 eax register are processed, all others are ignored.

This sounds like it would need a docs update to go along with it, both
to address this specific constraint and perhaps to give an example of
the syntax (unless it is already obvious?).

Speaking of obvious syntaxes, does the list in libxl_cpuid_parse_config
want updating so people can use maxhvleaf=2 instead of whatever the more
obscure syntax would be?

Re: [PATCH v7 1/3] xen/libxc: Allow changing max number of hypervisor cpuid leaves

[Boris Ostrovsky]

On 03/27/2014 09:13 AM, Ian Campbell wrote:
> On Mon, 2014-03-24 at 19:18 -0400, Boris Ostrovsky wrote:
>> Add support for changing max number of hypervisor leaves from configuration
>> file.
>>
>> This number can be specified using xl's standard 'cpuid' option. Only lowest
>> 8 bits of leaf's 0x4000xx00 eax register are processed, all others are ignored.
> This sounds like it would need a docs update to go along with it, both
> to address this specific constraint and perhaps to give an example of
> the syntax (unless it is already obvious?).

This could to xl.cfg.5[.txt|.html], something in the cpuid option 
description. E.g.

   Note: when specifying CPUID for hypervisor leaves (0x4000xxxx major
   group) only the lowest 8 bits of leaf's 0x4000xx00 EAX register that 
signify
   maximum number of hypervisor leaves are processed, the rest are ignored.

As for example, this uses the same syntax as all other leaves so I am 
not sure
anything more is needed.


> Speaking of obvious syntaxes, does the list in libxl_cpuid_parse_config
> want updating so people can use maxhvleaf=2 instead of whatever the more
> obscure syntax would be?

Yes, I should add this.

Thanks.
-boris

Re: [PATCH v7 1/3] xen/libxc: Allow changing max number of hypervisor cpuid leaves

[Ian Campbell]

On Thu, 2014-03-27 at 10:04 -0400, Boris Ostrovsky wrote:
> On 03/27/2014 09:13 AM, Ian Campbell wrote:
> > On Mon, 2014-03-24 at 19:18 -0400, Boris Ostrovsky wrote:
> >> Add support for changing max number of hypervisor leaves from configuration
> >> file.
> >>
> >> This number can be specified using xl's standard 'cpuid' option. Only lowest
> >> 8 bits of leaf's 0x4000xx00 eax register are processed, all others are ignored.
> > This sounds like it would need a docs update to go along with it, both
> > to address this specific constraint and perhaps to give an example of
> > the syntax (unless it is already obvious?).
> 
> This could to xl.cfg.5[.txt|.html], something in the cpuid option 
> description. E.g.
> 
>    Note: when specifying CPUID for hypervisor leaves (0x4000xxxx major
>    group) only the lowest 8 bits of leaf's 0x4000xx00 EAX register that 
> signify
>    maximum number of hypervisor leaves are processed, the rest are ignored.
> 
> As for example, this uses the same syntax as all other leaves so I am 
> not sure
> anything more is needed.

OK.

> > Speaking of obvious syntaxes, does the list in libxl_cpuid_parse_config
> > want updating so people can use maxhvleaf=2 instead of whatever the more
> > obscure syntax would be?
> 
> Yes, I should add this.

Thanks.

Ian.

Re: [PATCH v7 3/3] x86/hvm: Indicate avaliability of HW support of APIC virtualization to HVM guests

[Boris Ostrovsky]

On 03/25/2014 09:03 PM, Zhang, Yang Z wrote:
> Boris Ostrovsky wrote on 2014-03-25:
>> On 03/25/2014 05:45 AM, Jan Beulich wrote:
>>>>>> On 25.03.14 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>>> +void vmx_hypervisor_cpuid_leaf(uint32_t sub_idx,
>>>> +                               uint32_t *eax, uint32_t *ebx,
>>>> +                               uint32_t *ecx, uint32_t *edx) {
>>>> +    if ( sub_idx != 0 )
>>>> +        return;
>>>> +
>>>> +    if ( cpu_has_vmx_apic_reg_virt )
>>>> +        *eax |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
>>>> +    if ( cpu_has_vmx_virtualize_x2apic_mode )
>>>> +        *eax |= XEN_HVM_CPUID_X2APIC_VIRT; }
>>> So did the two of you then settle on (a) needing to expose two bits
>>> rather than just one and (b) these being the two relevant features
>>> to expose?
>> My argument is that we can't know which APIC model a guest uses and so
>> both are needed. For PVHVM we default to APIC (MMIO accesses), I can't
>> remember what unenlightened HVM Linux would do. And then there are
>> other OSs.
>>
>> For (b) having either (or both) of these two seems to be sufficient to
>> bring down the number of VMEXITs when switching from pirqs to APIC.
>> It's more important to agree on (a) since for (b) we can always add another bit.
> In currently Xen, virtualize_x2apic_mode takes effect only when APICv is enabled. Without APICv, virtualize_x2apic_mode is never set.
> Per your request, you only want to drop the pirqs if guest is using x2apic. So, just check it inside guest OS is enough. NB: to use x2apic for guest doesn't require the virtualize_x2apic_mode been set.

Yang and I talked a bit off-list and I don't think there was an 
agreement on this.

Here is a simple experiment to demonstrate why exposing 
virtualize_x2apic_mode is important (with one correction to what I said 
earlier: PVHVM guest will actualy default to x2apic, at least on Intel 
CPUs):

With existing code (using pirqs, i.e. no APIC/x2apic accesses), VMEXIT 
stats look as follows:

14397 HLT
   22420 INJ_VIRQ
    8551 INTR
   29849 INTR_WINDOW
       4 MMIO_READ
       2 MMIO_WRITE
     628 TRAP
       2 unknown
   78157 VMENTRY
   78157 VMEXIT
   29299 VMMCALL

Without pirqs (i.e. guest using x2APIC), and with virtualized x2apic, 
virtualized APIC register accesses:

   15572 HLT
     164 INJ_VIRQ
    4218 INTR
     184 INTR_WINDOW
     624 TRAP
   23199 VMENTRY
   23198 VMEXIT
    2607 VMMCALL

Without pirqs (again, guest uses x2APIC), without virtualized x2apic 
support but with virtualized APIC register access (which can be 
simulated by having msr_high of MSR_IA32_VMX_PROCBASED_CTLS2 clear bit 4):

      53 cpu_change
   18674 HLT
     226 INJ_VIRQ
   11702 INTR
     294 INTR_WINDOW
   35186 MSR_WRITE
     791 TRAP
   70441 VMENTRY
   70440 VMEXIT
3823 VMMCALL

In other words, if the guest is unaware of the fact that x2apic is not 
virtualized, it will disable pirqs for no good reason.

-boris

Re: [PATCH v7 3/3] x86/hvm: Indicate avaliability of HW support of APIC virtualization to HVM guests

[Zhang, Yang Z]

Boris Ostrovsky wrote on 2014-04-01:
> On 03/25/2014 09:03 PM, Zhang, Yang Z wrote:
>> Boris Ostrovsky wrote on 2014-03-25:
>>> On 03/25/2014 05:45 AM, Jan Beulich wrote:
>>>>>>> On 25.03.14 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>>>> +void vmx_hypervisor_cpuid_leaf(uint32_t sub_idx,
>>>>> +                               uint32_t *eax, uint32_t *ebx,
>>>>> +                               uint32_t *ecx, uint32_t *edx) {
>>>>> +    if ( sub_idx != 0 )
>>>>> +        return;
>>>>> +
>>>>> +    if ( cpu_has_vmx_apic_reg_virt )
>>>>> +        *eax |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
>>>>> +    if ( cpu_has_vmx_virtualize_x2apic_mode )
>>>>> +        *eax |= XEN_HVM_CPUID_X2APIC_VIRT; }
>>>> So did the two of you then settle on (a) needing to expose two
>>>> bits rather than just one and (b) these being the two relevant
>>>> features to expose?
>>> My argument is that we can't know which APIC model a guest uses and
>>> so both are needed. For PVHVM we default to APIC (MMIO accesses), I
>>> can't remember what unenlightened HVM Linux would do. And then
>>> there are other OSs.
>>> 
>>> For (b) having either (or both) of these two seems to be sufficient
>>> to bring down the number of VMEXITs when switching from pirqs to APIC.
>>> It's more important to agree on (a) since for (b) we can always add
>>> another
> bit.
>> In currently Xen, virtualize_x2apic_mode takes effect only when APICv
>> is enabled. Without APICv, virtualize_x2apic_mode is never set. Per
>> your request, you only want to drop the pirqs if guest is using x2apic.
>> So,
> just check it inside guest OS is enough. NB: to use x2apic for guest
> doesn't require the virtualize_x2apic_mode been set.
> 
> Yang and I talked a bit off-list and I don't think there was an agreement on this.
> 
> Here is a simple experiment to demonstrate why exposing
> virtualize_x2apic_mode is important (with one correction to what I said
> earlier: PVHVM guest will actualy default to x2apic, at least on Intel
> CPUs):
> 
> With existing code (using pirqs, i.e. no APIC/x2apic accesses), VMEXIT
> stats look as follows:
> 
> 14397 HLT
>    22420 INJ_VIRQ
>     8551 INTR
>    29849 INTR_WINDOW
>        4 MMIO_READ 2 MMIO_WRITE 628 TRAP 2 unknown
>    78157 VMENTRY
>    78157 VMEXIT
>    29299 VMMCALL
> Without pirqs (i.e. guest using x2APIC), and with virtualized x2apic,
> virtualized APIC register accesses:
> 
>    15572 HLT
>      164 INJ_VIRQ 4218 INTR 184 INTR_WINDOW 624 TRAP
>    23199 VMENTRY
>    23198 VMEXIT
>     2607 VMMCALL
> Without pirqs (again, guest uses x2APIC), without virtualized x2apic
> support but with virtualized APIC register access (which can be
> simulated by having msr_high of MSR_IA32_VMX_PROCBASED_CTLS2 clear bit 4):
> 
>       53 cpu_change
>    18674 HLT
>      226 INJ_VIRQ 11702 INTR 294 INTR_WINDOW 35186 MSR_WRITE 791 TRAP
>    70441 VMENTRY
>    70440 VMEXIT
> 3823 VMMCALL
> 
> In other words, if the guest is unaware of the fact that x2apic is not
> virtualized, it will disable pirqs for no good reason.

You don't have the data that guest is using x2apic and virtulized_x2apic is set but no apicv. But this scenario is not support in Xen. 

> 
> -boris


Best regards,
Yang