Project

General

Profile

Actions

Bug #13139

closed

bhyve bungles math for VMX ins/outs size

Added by Patrick Mooney 11 months ago. Updated 11 months ago.

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

100%

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

Description

A user on the smartos-discuss mailing list reported a blown assertion:

panic[cpu1]/thread=fffffe16ef6df0a0:
assertion failed: inout->addrsize == 2 || inout->addrsize == 4 || inout->addrsize == 8, file: ../../i86pc/io/vmm/intel/vmx.c, line: 1953

This would be from code added/modified by #12989. Taking a closer look at the code in question, the problem is apparent:

                inst_info = vmcs_read(VMCS_EXIT_INSTRUCTION_INFO);

                /*
                 * Bits 7-9 encode the address size of ins/outs operations where
                 * the 0/1/2 values correspond to 16/32/64 bit sizes.
                 */
                inout->addrsize = 2 << (1 + ((inst_info >> 7) & 0x3));
                VERIFY(inout->addrsize == 2 || inout->addrsize == 4 ||
                    inout->addrsize == 8);

That Intel reserved 3 bits is deceiving. The only expected values from that field (once shifted) are 0/1/2. Considering the result is 2 << 0 or 1 or 2, the erroneous 1 + should be eliminated, as it results in output values of 4/8/16, blowing the assert in the high case.


Related issues

Related to illumos gate - Bug #12989: improve interface boundary for bhyve MMIOClosedPatrick Mooney

Actions
Related to illumos gate - Bug #13147: bhyve kernel could make more use of BITX()New

Actions
Actions #1

Updated by Patrick Mooney 11 months ago

  • Related to Bug #12989: improve interface boundary for bhyve MMIO added
Actions #2

Updated by Electric Monk 11 months ago

  • Gerrit CR set to 917
Actions #3

Updated by Patrick Mooney 11 months ago

  • Related to Bug #13147: bhyve kernel could make more use of BITX() added
Actions #4

Updated by Patrick Mooney 11 months ago

  • % Done changed from 0 to 90
Actions #5

Updated by Patrick Mooney 11 months ago

I did a few things to test this. Firstly, I copied the addrsize derivation logic out of the VMX code and into an independent C file where I could feed it the three anticipated inputs. The results there were as expected (which would meet the VERIFY criteria). After that, I forced a guest VM to perform outs operations again a io port to confirm that it does function properly end-to-end. A rep outs to 0x2f8 resulted in the expected string appearing on the ttyb virtual serial port.

Actions #6

Updated by Patrick Mooney 11 months ago

There was some suspicion about an ubuntu guest tripping over this as well. I installed an ubuntu server 20.04 guest, but did not observe any string-style ioport usage.

Actions #7

Updated by Electric Monk 11 months ago

  • Status changed from New to Closed
  • % Done changed from 90 to 100

git commit d92a2ce76311170b6aa244c3a22f3a3754dad613

commit  d92a2ce76311170b6aa244c3a22f3a3754dad613
Author: Patrick Mooney <pmooney@pfmooney.com>
Date:   2020-09-14T22:30:32.000Z

    13139 bhyve bungles math for VMX ins/outs size
    Reviewed by: Jason King <jason.king@joyent.com>
    Reviewed by: Joshua M. Clulow <josh@sysmgr.org>
    Reviewed by: Andy Fiddaman <andy@omniosce.org>
    Approved by: Robert Mustacchi <rm@fingolfin.org>

Actions

Also available in: Atom PDF