diff mbox

[RFC,1/8] x86/hvm: set initial apicid to vcpu_id

Message ID 1456174934-22973-2-git-send-email-joao.m.martins@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joao Martins Feb. 22, 2016, 9:02 p.m. UTC 
Currently the initial_apicid is set vcpu_id * 2 which makes it difficult
for the toolstack to manage how is the topology seen by the guest.
Instead of forcing procpkg and proccount to be VCPUID * 2, instead we
set it to max vcpuid on proccount to max_vcpu_id + 1 (logical number of
logical cores) and procpkg to max_vcpu_id (max cores minus 1)

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_cpuid_x86.c | 18 +++++++++++++-----
 xen/arch/x86/hvm/hvm.c     |  2 +-
 2 files changed, 14 insertions(+), 6 deletions(-)

Comments

Jan Beulich Feb. 25, 2016, 5:03 p.m. UTC | #1 
>>> On 22.02.16 at 22:02, <joao.m.martins@oracle.com> wrote:
> Currently the initial_apicid is set vcpu_id * 2 which makes it difficult
> for the toolstack to manage how is the topology seen by the guest.
> Instead of forcing procpkg and proccount to be VCPUID * 2, instead we
> set it to max vcpuid on proccount to max_vcpu_id + 1 (logical number of
> logical cores) and procpkg to max_vcpu_id (max cores minus 1)

I'm afraid it takes more than this to explain why the change is
needed or at least desirable. In particular I'd like to suggest that
you do some archeology to understand why things are the way
they are.

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4633,7 +4633,7 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>      case 0x1:
>          /* Fix up VLAPIC details. */
>          *ebx &= 0x00FFFFFFu;
> -        *ebx |= (v->vcpu_id * 2) << 24;
> +        *ebx |= (v->vcpu_id) << 24;
>          if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
>              __clear_bit(X86_FEATURE_APIC & 31, edx);

In no case is this sufficient as adjustment to the hypervisor side,
as it gets things out of sync with e.g. hvm/vlapic.c.

Jan
diff mbox

Patch

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index c142595..aa1d679 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -45,6 +45,7 @@  struct cpuid_domain_info
     bool hvm;
     bool pvh;
     uint64_t xfeature_mask;
+    uint32_t max_vcpu_id;
 
     /* PV-only information. */
     bool pv64;
@@ -102,6 +103,7 @@  static int get_cpuid_domain_info(xc_interface *xch, domid_t domid,
 
     info->hvm = di.hvm;
     info->pvh = di.pvh;
+    info->max_vcpu_id = di.max_vcpu_id;
 
     /* Get xstate information. */
     domctl.cmd = XEN_DOMCTL_getvcpuextstate;
@@ -245,10 +247,16 @@  static void intel_xc_cpuid_policy(xc_interface *xch,
     case 0x00000004:
         /*
          * EAX[31:26] is Maximum Cores Per Package (minus one).
-         * Update to reflect vLAPIC_ID = vCPU_ID * 2.
          */
-        regs[0] = (((regs[0] & 0x7c000000u) << 1) | 0x04000000u |
-                   (regs[0] & 0x3ffu));
+	{
+                unsigned int procpkg = 0;
+                if ( info->hvm )
+                        procpkg = info->max_vcpu_id << 26;
+                else
+                        procpkg = (regs[0] & 0x7c000000u) << 1;
+
+                regs[0] = (procpkg | 0x04000000u | (regs[0] & 0x3ffu));
+        }
         regs[3] &= 0x3ffu;
         break;
 
@@ -353,9 +361,9 @@  static void xc_cpuid_hvm_policy(xc_interface *xch,
     case 0x00000001:
         /*
          * EBX[23:16] is Maximum Logical Processors Per Package.
-         * Update to reflect vLAPIC_ID = vCPU_ID * 2.
          */
-        regs[1] = (regs[1] & 0x0000ffffu) | ((regs[1] & 0x007f0000u) << 1);
+        regs[1] = (regs[1] & 0x0000ffffu) |
+                   (((info->max_vcpu_id + 1) << 16));
 
         regs[2] &= (bitmaskof(X86_FEATURE_XMM3) |
                     bitmaskof(X86_FEATURE_PCLMULQDQ) |
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index fe382ce..d45e7a9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4633,7 +4633,7 @@  void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
     case 0x1:
         /* Fix up VLAPIC details. */
         *ebx &= 0x00FFFFFFu;
-        *ebx |= (v->vcpu_id * 2) << 24;
+        *ebx |= (v->vcpu_id) << 24;
         if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
             __clear_bit(X86_FEATURE_APIC & 31, edx);