Page MenuHomeFreeBSD
Differential D17531

bhyve: provide basic descriptions for VMX exit reason
ClosedPublic
Actions

Authored by yuripv on Oct 12 2018, 10:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 21, 8:53 AM
Unknown Object (File)
Mon, Oct 20, 10:46 AM
Unknown Object (File)
Mon, Oct 20, 10:46 AM
Unknown Object (File)
Mon, Oct 20, 10:46 AM
Unknown Object (File)
Mon, Oct 20, 10:46 AM
Unknown Object (File)
Mon, Oct 20, 10:46 AM
Unknown Object (File)
Sun, Oct 19, 8:20 PM
Unknown Object (File)
Sat, Oct 18, 10:09 PM
View All Files
Subscribers
0mp
araujo
imp
jhb
kib

Details

Reviewers
grehan
rgrimes
0mp
kib
jhb
Group Reviewers
manpages
bhyve
Commits
rS339826: Provide basic descriptions for VMX exit reason (from "Intel 64 and IA-32
Summary

Provide basic descriptions for VMX exit reason (from "Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3"). Add the document to SEE ALSO in bhyve.8 (and pet manlint here a bit).

It makes it a bit easier to understand the reason of exit, e.g.:

vm exit[0]
        reason          VMX
        rip             0x000000000000fff0
        inst_length     3
        status          0
        exit_reason     48 (EPT violation)
        qualification   0x0000000000000184
        inst_type               0
        inst_error              0
Abort trap

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

yuripv created this revision.Oct 12 2018, 10:43 AM
Herald added a reviewer: rgrimes. · View Herald TranscriptOct 12 2018, 10:43 AM
Herald added a reviewer: manpages. · View Herald Transcript
Herald added a subscriber: imp. · View Herald Transcript
0mp accepted this revision.Oct 12 2018, 10:56 AM
0mp added a subscriber: 0mp.

The manual page update seems fine from the mdoc perspective.

This revision is now accepted and ready to land.Oct 12 2018, 10:56 AM
yuripv updated this revision to Diff 49050.Oct 12 2018, 10:56 AM

copy/paste error

This revision now requires review to proceed.Oct 12 2018, 10:56 AM
kib added inline comments.Oct 12 2018, 10:57 AM
usr.sbin/bhyve/bhyverun.c
92

const char * const ? If it passes over other places.

93

Use designated initializers, even if the indexes do not have symbolic names. Put each initialization element on its own line.

548

I would create a static function which does the calculation of the string by number.

yuripv updated this revision to Diff 49052.Oct 12 2018, 11:48 AM

Address comments.

yuripv marked 4 inline comments as done.Oct 12 2018, 11:49 AM
yuripv added inline comments.
usr.sbin/bhyve/bhyverun.c
93

Done, I hope that including <amd64/vmm/intel/vmcs.h> is OK here.

yuripv marked an inline comment as done.Oct 12 2018, 12:02 PM
kib accepted this revision.Oct 12 2018, 12:20 PM
kib added inline comments.
usr.sbin/bhyve/bhyverun.c
93

This header is not installed, so bhyve(8) definitely requires the build from the full sources. I remember that it was the case already due to vmm_instructions_emul.c.

541

Blank line is needed after '{' if no locals are defined.

544

This blank line is excessive.

This revision is now accepted and ready to land.Oct 12 2018, 12:20 PM
yuripv updated this revision to Diff 49057.Oct 12 2018, 2:44 PM

style fixes

This revision now requires review to proceed.Oct 12 2018, 2:44 PM
yuripv marked 3 inline comments as done.Oct 12 2018, 2:45 PM
kib accepted this revision.Oct 12 2018, 3:03 PM
This revision is now accepted and ready to land.Oct 12 2018, 3:03 PM
rgrimes added a reviewer: bhyve.Oct 12 2018, 4:24 PM
yuripv added a comment.Oct 22 2018, 5:23 PM

Can I proceed with kib being the reviewer for this, or bhyve group will take a look?

rgrimes accepted this revision as: rgrimes.Oct 22 2018, 5:47 PM

The bhyve group hat was requested by Peter Grehan for "Maintainer" approval and awareness of bhyve bits, as far as I can see this review does not touch any bits that need bhyve maintainer approval, so you can commit with out an accept from them.

I am accepting this as me personally, I am not a person who can wear hat bhyve maintainer.

jhb accepted this revision.Oct 22 2018, 7:32 PM
jhb added a subscriber: jhb.
jhb added inline comments.
usr.sbin/bhyve/bhyve.8
503–505

I think the doc folks would want all of the formatting changes to this file be done as a separate commit from the actual content change. I think adding the reference to the Intel SDM manual is the only content change to this file?

yuripv marked 2 inline comments as done.Oct 22 2018, 7:35 PM
yuripv added inline comments.
usr.sbin/bhyve/bhyve.8
503–505

Yes, the only content change is added reference. Formatting changes are mostly pointed out by mandoc lint, and seem to be formally accepted by manpages (@0mp).

yuripv marked an inline comment as done.Oct 22 2018, 7:39 PM
araujo added a subscriber: araujo.Oct 27 2018, 3:48 AM

Hi @yuripv , is there anything pending for this patch to be committed?

yuripv added a comment.Oct 27 2018, 8:32 PM
In D17531#378698, @araujo wrote:

Hi @yuripv , is there anything pending for this patch to be committed?

Sorry for the delay, submitted to mentor for approval.

Closed by commit rS339826: Provide basic descriptions for VMX exit reason (from "Intel 64 and IA-32 (authored by yuripv). · Explain WhyOct 27 2018, 9:24 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
usr.sbin/
bhyve/
27 lines
25 lines

Diff 49050

View Options

usr.sbin/bhyve/bhyve.8

Loading...
View Options

usr.sbin/bhyve/bhyverun.c

Loading...