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
This commit is contained in:
John Baldwin 2020-10-01 16:45:11 +00:00
parent c9d175ea90
commit a3f2a9c57e
4 changed files with 49 additions and 48 deletions

View File

@ -1496,11 +1496,8 @@ svm_vmexit(struct svm_softc *svm_sc, int vcpu, struct vm_exit *vmexit)
break; break;
case VMCB_EXIT_CPUID: case VMCB_EXIT_CPUID:
vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_CPUID, 1); vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_CPUID, 1);
handled = x86_emulate_cpuid(svm_sc->vm, vcpu, handled = x86_emulate_cpuid(svm_sc->vm, vcpu, &state->rax,
(uint32_t *)&state->rax, &ctx->sctx_rbx, &ctx->sctx_rcx, &ctx->sctx_rdx);
(uint32_t *)&ctx->sctx_rbx,
(uint32_t *)&ctx->sctx_rcx,
(uint32_t *)&ctx->sctx_rdx);
break; break;
case VMCB_EXIT_HLT: case VMCB_EXIT_HLT:
vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_HLT, 1); vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_HLT, 1);

View File

@ -1193,15 +1193,11 @@ vmx_vminit(struct vm *vm, pmap_t pmap)
static int static int
vmx_handle_cpuid(struct vm *vm, int vcpu, struct vmxctx *vmxctx) 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, (uint64_t *)&vmxctx->guest_rax,
(uint64_t *)&vmxctx->guest_rbx, (uint64_t *)&vmxctx->guest_rcx,
handled = x86_emulate_cpuid(vm, vcpu, (uint64_t *)&vmxctx->guest_rdx);
(uint32_t*)(&vmxctx->guest_rax),
(uint32_t*)(&vmxctx->guest_rbx),
(uint32_t*)(&vmxctx->guest_rcx),
(uint32_t*)(&vmxctx->guest_rdx));
return (handled); return (handled);
} }

View File

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

View File

@ -64,8 +64,8 @@
*/ */
#define CPUID_0000_0001_FEAT0_VMX (1<<5) #define CPUID_0000_0001_FEAT0_VMX (1<<5)
int x86_emulate_cpuid(struct vm *vm, int vcpu_id, uint32_t *eax, uint32_t *ebx, int x86_emulate_cpuid(struct vm *vm, int vcpu_id, uint64_t *rax, uint64_t *rbx,
uint32_t *ecx, uint32_t *edx); uint64_t *rcx, uint64_t *rdx);
enum vm_cpuid_capability { enum vm_cpuid_capability {
VCC_NONE, VCC_NONE,