Author: jhb
Date: Thu Oct  1 16:45:11 2020
New Revision: 366328
URL: https://svnweb.freebsd.org/changeset/base/366328

Log:
  Clear the upper 32-bits of registers in x86_emulate_cpuid().
  
  Per the Intel manuals, CPUID is supposed to unconditionally zero the
  upper 32 bits of the involved (rax/rbx/rcx/rdx) registers.
  Previously, the emulation would cast pointers to the 64-bit register
  values down to `uint32_t`, which while properly manipulating the lower
  bits, would leave any garbage in the upper bits uncleared.  While no
  existing guest OSes seem to stumble over this in practice, the bhyve
  emulation should match x86 expectations.
  
  This was discovered through alignment warnings emitted by gcc9, while
  testing it against SmartOS/bhyve.
  
  SmartOS bug:  https://smartos.org/bugview/OS-8168
  Submitted by: Patrick Mooney
  Reviewed by:  rgrimes
  Differential Revision:        https://reviews.freebsd.org/D24727

Modified:
  head/sys/amd64/vmm/amd/svm.c
  head/sys/amd64/vmm/intel/vmx.c
  head/sys/amd64/vmm/x86.c
  head/sys/amd64/vmm/x86.h

Modified: head/sys/amd64/vmm/amd/svm.c
==============================================================================
--- head/sys/amd64/vmm/amd/svm.c        Thu Oct  1 16:37:49 2020        
(r366327)
+++ head/sys/amd64/vmm/amd/svm.c        Thu Oct  1 16:45:11 2020        
(r366328)
@@ -1496,11 +1496,8 @@ svm_vmexit(struct svm_softc *svm_sc, int vcpu, struct 
                break;
        case VMCB_EXIT_CPUID:
                vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_CPUID, 1);
-               handled = x86_emulate_cpuid(svm_sc->vm, vcpu,
-                   (uint32_t *)&state->rax,
-                   (uint32_t *)&ctx->sctx_rbx,
-                   (uint32_t *)&ctx->sctx_rcx,
-                   (uint32_t *)&ctx->sctx_rdx);
+               handled = x86_emulate_cpuid(svm_sc->vm, vcpu, &state->rax,
+                   &ctx->sctx_rbx, &ctx->sctx_rcx, &ctx->sctx_rdx);
                break;
        case VMCB_EXIT_HLT:
                vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_HLT, 1);

Modified: head/sys/amd64/vmm/intel/vmx.c
==============================================================================
--- head/sys/amd64/vmm/intel/vmx.c      Thu Oct  1 16:37:49 2020        
(r366327)
+++ head/sys/amd64/vmm/intel/vmx.c      Thu Oct  1 16:45:11 2020        
(r366328)
@@ -1193,15 +1193,11 @@ vmx_vminit(struct vm *vm, pmap_t pmap)
 static int
 vmx_handle_cpuid(struct vm *vm, int vcpu, struct vmxctx *vmxctx)
 {
-       int handled, func;
+       int handled;
 
-       func = vmxctx->guest_rax;
-
-       handled = x86_emulate_cpuid(vm, vcpu,
-                                   (uint32_t*)(&vmxctx->guest_rax),
-                                   (uint32_t*)(&vmxctx->guest_rbx),
-                                   (uint32_t*)(&vmxctx->guest_rcx),
-                                   (uint32_t*)(&vmxctx->guest_rdx));
+       handled = x86_emulate_cpuid(vm, vcpu, (uint64_t *)&vmxctx->guest_rax,
+           (uint64_t *)&vmxctx->guest_rbx, (uint64_t *)&vmxctx->guest_rcx,
+           (uint64_t *)&vmxctx->guest_rdx);
        return (handled);
 }
 

Modified: head/sys/amd64/vmm/x86.c
==============================================================================
--- head/sys/amd64/vmm/x86.c    Thu Oct  1 16:37:49 2020        (r366327)
+++ head/sys/amd64/vmm/x86.c    Thu Oct  1 16:45:11 2020        (r366328)
@@ -87,35 +87,40 @@ log2(u_int x)
 }
 
 int
-x86_emulate_cpuid(struct vm *vm, int vcpu_id,
-                 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
+x86_emulate_cpuid(struct vm *vm, int vcpu_id, uint64_t *rax, uint64_t *rbx,
+    uint64_t *rcx, uint64_t *rdx)
 {
        const struct xsave_limits *limits;
        uint64_t cr4;
        int error, enable_invpcid, enable_rdpid, enable_rdtscp, level,
            width, x2apic_id;
-       unsigned int func, regs[4], logical_cpus;
+       unsigned int func, regs[4], logical_cpus, param;
        enum x2apic_state x2apic_state;
        uint16_t cores, maxcpus, sockets, threads;
 
-       VCPU_CTR2(vm, vcpu_id, "cpuid %#x,%#x", *eax, *ecx);
+       /*
+        * The function of CPUID is controlled through the provided value of
+        * %eax (and secondarily %ecx, for certain leaf data).
+        */
+       func = (uint32_t)*rax;
+       param = (uint32_t)*rcx;
 
+       VCPU_CTR2(vm, vcpu_id, "cpuid %#x,%#x", func, param);
+
        /*
         * Requests for invalid CPUID levels should map to the highest
         * available level instead.
         */
-       if (cpu_exthigh != 0 && *eax >= 0x80000000) {
-               if (*eax > cpu_exthigh)
-                       *eax = cpu_exthigh;
-       } else if (*eax >= 0x40000000) {
-               if (*eax > CPUID_VM_HIGH)
-                       *eax = CPUID_VM_HIGH;
-       } else if (*eax > cpu_high) {
-               *eax = cpu_high;
+       if (cpu_exthigh != 0 && func >= 0x80000000) {
+               if (func > cpu_exthigh)
+                       func = cpu_exthigh;
+       } else if (func >= 0x40000000) {
+               if (func > CPUID_VM_HIGH)
+                       func = CPUID_VM_HIGH;
+       } else if (func > cpu_high) {
+               func = cpu_high;
        }
 
-       func = *eax;
-
        /*
         * In general the approach used for CPU topology is to
         * advertise a flat topology where all CPUs are packages with
@@ -133,10 +138,10 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                case CPUID_8000_0003:
                case CPUID_8000_0004:
                case CPUID_8000_0006:
-                       cpuid_count(*eax, *ecx, regs);
+                       cpuid_count(func, param, regs);
                        break;
                case CPUID_8000_0008:
-                       cpuid_count(*eax, *ecx, regs);
+                       cpuid_count(func, param, regs);
                        if (vmm_is_svm()) {
                                /*
                                 * As on Intel (0000_0007:0, EDX), mask out
@@ -167,7 +172,7 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                        break;
 
                case CPUID_8000_0001:
-                       cpuid_count(*eax, *ecx, regs);
+                       cpuid_count(func, param, regs);
 
                        /*
                         * Hide SVM from guest.
@@ -248,7 +253,7 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                         */
                        vm_get_topology(vm, &sockets, &cores, &threads,
                            &maxcpus);
-                       switch (*ecx) {
+                       switch (param) {
                        case 0:
                                logical_cpus = threads;
                                level = 1;
@@ -396,7 +401,7 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                        break;
 
                case CPUID_0000_0004:
-                       cpuid_count(*eax, *ecx, regs);
+                       cpuid_count(func, param, regs);
 
                        if (regs[0] || regs[1] || regs[2] || regs[3]) {
                                vm_get_topology(vm, &sockets, &cores, &threads,
@@ -425,8 +430,8 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                        regs[3] = 0;
 
                        /* leaf 0 */
-                       if (*ecx == 0) {
-                               cpuid_count(*eax, *ecx, regs);
+                       if (param == 0) {
+                               cpuid_count(func, param, regs);
 
                                /* Only leaf 0 is supported */
                                regs[0] = 0;
@@ -485,21 +490,21 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                        if (vmm_is_intel()) {
                                vm_get_topology(vm, &sockets, &cores, &threads,
                                    &maxcpus);
-                               if (*ecx == 0) {
+                               if (param == 0) {
                                        logical_cpus = threads;
                                        width = log2(logical_cpus);
                                        level = CPUID_TYPE_SMT;
                                        x2apic_id = vcpu_id;
                                }
 
-                               if (*ecx == 1) {
+                               if (param == 1) {
                                        logical_cpus = threads * cores;
                                        width = log2(logical_cpus);
                                        level = CPUID_TYPE_CORE;
                                        x2apic_id = vcpu_id;
                                }
 
-                               if (!cpuid_leaf_b || *ecx >= 2) {
+                               if (!cpuid_leaf_b || param >= 2) {
                                        width = 0;
                                        logical_cpus = 0;
                                        level = 0;
@@ -508,7 +513,7 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
 
                                regs[0] = width & 0x1f;
                                regs[1] = logical_cpus & 0xffff;
-                               regs[2] = (level << 8) | (*ecx & 0xff);
+                               regs[2] = (level << 8) | (param & 0xff);
                                regs[3] = x2apic_id;
                        } else {
                                regs[0] = 0;
@@ -528,8 +533,8 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                                break;
                        }
 
-                       cpuid_count(*eax, *ecx, regs);
-                       switch (*ecx) {
+                       cpuid_count(func, param, regs);
+                       switch (param) {
                        case 0:
                                /*
                                 * Only permit the guest to use bits
@@ -559,7 +564,7 @@ x86_emulate_cpuid(struct vm *vm, int vcpu_id,
                                 * pass through as-is, otherwise return
                                 * all zeroes.
                                 */
-                               if (!(limits->xcr0_allowed & (1ul << *ecx))) {
+                               if (!(limits->xcr0_allowed & (1ul << param))) {
                                        regs[0] = 0;
                                        regs[1] = 0;
                                        regs[2] = 0;
@@ -596,14 +601,17 @@ default_leaf:
                         * how many unhandled leaf values have been seen.
                         */
                        atomic_add_long(&bhyve_xcpuids, 1);
-                       cpuid_count(*eax, *ecx, regs);
+                       cpuid_count(func, param, regs);
                        break;
        }
 
-       *eax = regs[0];
-       *ebx = regs[1];
-       *ecx = regs[2];
-       *edx = regs[3];
+       /*
+        * CPUID clears the upper 32-bits of the long-mode registers.
+        */
+       *rax = regs[0];
+       *rbx = regs[1];
+       *rcx = regs[2];
+       *rdx = regs[3];
 
        return (1);
 }

Modified: head/sys/amd64/vmm/x86.h
==============================================================================
--- head/sys/amd64/vmm/x86.h    Thu Oct  1 16:37:49 2020        (r366327)
+++ head/sys/amd64/vmm/x86.h    Thu Oct  1 16:45:11 2020        (r366328)
@@ -64,8 +64,8 @@
  */
 #define CPUID_0000_0001_FEAT0_VMX      (1<<5)
 
-int x86_emulate_cpuid(struct vm *vm, int vcpu_id, uint32_t *eax, uint32_t *ebx,
-                     uint32_t *ecx, uint32_t *edx);
+int x86_emulate_cpuid(struct vm *vm, int vcpu_id, uint64_t *rax, uint64_t *rbx,
+    uint64_t *rcx, uint64_t *rdx);
 
 enum vm_cpuid_capability {
        VCC_NONE,
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to