Project

General

Profile

Actions

Bug #12746

closed

x86_emulate_cpuid() should clear upper 32 bits

Added by Patrick Mooney 12 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Category:
kernel
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
bhyve
Gerrit CR:

Description

When investigating a gcc9 gripe in the bhyve codebase, I noticed something that looked amiss:

1516                 handled = x86_emulate_cpuid(svm_sc->vm, vcpu,                    
1517                     (uint32_t *)&state->rax,                                     
1518                     (uint32_t *)&ctx->sctx_rbx,                                  
1519                     (uint32_t *)&ctx->sctx_rcx,                                  
1520                     (uint32_t *)&ctx->sctx_rdx); 

While gcc9 was complaining about the alignment on the rax-rdx stores, it looked like the upper bits in those registers wouldn't be properly cleared. Indeed the SDM notes that the upper 32-bits of those registers is zeroed by CPUID calls. A quick test under bhyve showed that garbage left in the upper bits remained there after a CPUID call.

While no guest has complained about this to date, bhyve should probably do the right thing and clear those bits. (And in the process, address gcc9's gripe.)

SmartOS issue: OS-8168
FreeBSD upstreaming CR: D24727


Related issues

Related to illumos gate - Bug #12749: gcc10 complains about vmcb_state struct packingClosedPatrick Mooney

Actions
Actions #1

Updated by Patrick Mooney 12 months ago

  • Related to Bug #12749: gcc10 complains about vmcb_state struct packing added
Actions #2

Updated by Patrick Mooney 12 months ago

Here's a silly little test program I've used to check if high bits in a CPUID operation are cleared:

#include <stdio.h>

int main()
{
        unsigned long a, b, c, d;
        unsigned long level = 0x00000002;

        asm ("movq $0xffffffffffffffff, %%rbx\n\tcpuid\n\t" 
        : "=a" (a), "=b" (b), "=c" (c), "=d" (d)
        : "0" (level));

        printf("cpuid %lx: a:%lx b:%lx c:%lx d:%lx\n", level, a, b, c, d);
        return (0);
}

Actions #3

Updated by Patrick Mooney 12 months ago

Using the above script, bhyve would behave as noted in the issue report, leaving any garbage in the upper bits untouched. With the patch, the upper 32-bits of those registers is cleared by the CPUID emulation.

Actions #4

Updated by Electric Monk 12 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 3c5f2a9de9c6554ce899ad4ebf7978ea7293994a

commit  3c5f2a9de9c6554ce899ad4ebf7978ea7293994a
Author: Patrick Mooney <pmooney@pfmooney.com>
Date:   2020-05-22T02:56:09.000Z

    12746 x86_emulate_cpuid() should clear upper 32 bits
    Reviewed by: John Levon <john.levon@joyent.com>
    Reviewed by: Mike Zeller <mike.zeller@joyent.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions #5

Updated by Patrick Mooney 8 months ago

Integrated upstream as ff41c89

Actions

Also available in: Atom PDF