bhyve: document fbuf and xhci devices in the manpage
ClosedPublic

Authored by novel on Mar 15 2017, 3:32 PM.

Details

Summary
  • document fbuf and xhci devices and their configuration
  • add example for an UEFI virtual machine

PS I think the 'bootrom' description should be extended too. Currently, the manpage only shows that it's a possible choice for '-l', but doesn't provide configuration details, i.e. that a path to the firmware file needs to be provided. I'm wondering if the '-l lpcdev[,conf]' bit should be structured in the same way as the '-s slot,emulation[,conf]' block with a little more detailed description of the 'bootrom' option? Feedback on that is welcome.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
novel updated this revision to Diff 26277.Mar 15 2017, 4:03 PM

Added a note that external loaders are not needed when using boot ROM.

grehan edited edge metadata.Mar 16 2017, 5:16 AM

Many thanks for doing this. I'll get some detailed comments shortly.

bcr added a subscriber: bcr.Mar 16 2017, 6:01 PM

I have two suggestions.

bhyve/bhyve.8
321 ↗(On Diff #26277)

You need a comma between "height" and "respectively".

429 ↗(On Diff #26277)

I would add "pixels" after the 600.

novel updated this revision to Diff 26392.Mar 18 2017, 5:36 PM

Added missing comma and "pixels".

novel marked 2 inline comments as done.Mar 18 2017, 5:36 PM
grehan added inline comments.Mar 20 2017, 4:27 AM
bhyve/bhyve.8
52 ↗(On Diff #26392)

using a boot ROM

312 ↗(On Diff #26392)

"tcp=" will be deprected. bhyve currenctly accepts "rfb" for the VNC protocol so that one should be documented.

312 ↗(On Diff #26392)

The width, height and wait parameters are optional. Width/height defaults to 1024x768, with a minimum of 640x480.

Another optional parameter is "vga=[on|io|off]", which controls whether the frame buffer exports standard VGA registers. Windows 7/2k8 guests require "io", which is the default. "on" should be used when using graphics with BIOS guests, and "off" is required for OpenBSD UEFI guests to prevent the early probe from assuming a VGA framebuffer is present.

385 ↗(On Diff #26392)

using a boot ROM

390 ↗(On Diff #26392)

Otherwise, the boot loader is not needed.

novel added a comment.Mar 22 2017, 4:07 PM

Thanks for feedback! Some questions inline.

bhyve/bhyve.8
312 ↗(On Diff #26392)

Is there probably some general description of the "io" option? Also, are there any rules regarding mentioning Windows in the manual pages, i.e. should we put stuff like (tm) or (r) or anything like that? The first thing that came to my mind is wine(1) and it just goes with "Windows $Ver", though it's not a part of the FreeBSD base.

312 ↗(On Diff #26392)

Oh, it's unfortunate that 'tcp=' is getting deprecated as I'm using that in the libvirt driver already.

Is there a way to detect that in runtime? I can see that bhyve *can* report unsupported configuration for the fbuf device, but that happens pretty late in the VM creation process, i.e. I have to run in this way:

/usr/sbin/bhyve -m 2048 -s 4:0,fbuf,tcp=127.0.0.1:5904,oops test

It does not work if I omit "-m" and guest name. Also, it creates /dev/vmm/test, so I have to call destroy on it, which makes such kind of a probe a little expensive.

PS In a message "Invalid fbuf emulation "oops" fbuf: {wait,}{vga=on|io|off,}rfb=<ip>:port" do commas have to go after the closing brace to follow common style?

grehan added inline comments.Mar 23 2017, 12:13 AM
bhyve/bhyve.8
312 ↗(On Diff #26392)

PCI graphics cards have a dual personality in that they are standard PCI devices with BAR addressing, but may also implicitly decode legacy VGA i/o space (0x3c0-3df) and memory space (64KB at 0xA0000).

A true non-legacy UEFI-only graphics adapter doesn't require any of the legacy decodes. However, Windows 7 and 2k8 still attempt to issue BIOS calls which result in i/o port queries. If the i/o decode is disabled, the Windows guest will fail to boot. This corresponds to the "vga=io" mode.

Regarding explicit references to Windows - that could be changed to "some guests" and the FreeBSD wiki used to demonstrate options for particular guests.

I can just leave "tcp=" in the code. But, best not to document it.

I know we've chatted about ways to try and determine what features are available at run-time. Let me think a bit more about that.

When using the CSM BIOS compatibility in UEFI to boot traditional BIOS guests, the complete i/o and memory decode needs to be present ("vga=on").

OpenBSD in UEFI mode will detect i/o ports and assume a VGA adapter is present. To prevent this, the "vga=off" mode is provided. Note that picking a default will prevent either Windows or OpenBSD from functioning. Since Win7 seems very prevalent, the default was selected to allow that.

novel updated this revision to Diff 26606.Mar 23 2017, 3:56 PM

Added a description of the "vga=" configuration options.

novel marked 3 inline comments as done.Mar 23 2017, 4:07 PM
novel added inline comments.
bhyve/bhyve.8
312 ↗(On Diff #26392)

Thanks for a great explanation! I've tried to summarize it up using the powerful copy/paste technique. Hopefully I didn't mess up with with the rest.

bcr added a comment.Apr 9 2017, 10:50 AM

One small nit remains, then it is ready to go from my side.

bhyve/bhyve.8
390 ↗(On Diff #26392)

This does not seem to be "done" yet.

novel updated this revision to Diff 27266.Apr 9 2017, 2:54 PM
novel marked an inline comment as done.
novel added inline comments.
bhyve/bhyve.8
390 ↗(On Diff #26392)

Oops, I've added the comma after "otherwise", but didn't pay attention to the missing "the". Should be fine now, thanks for noting.

novel marked an inline comment as done.Apr 9 2017, 3:00 PM
bcr accepted this revision.Apr 9 2017, 3:00 PM

OK from manpages.

This revision is now accepted and ready to land.Apr 9 2017, 3:00 PM
grehan accepted this revision.Apr 9 2017, 5:53 PM

Fine to commit after the max fbuf res added.

Once again - many thanks for doing this.

bhyve/bhyve.8
322 ↗(On Diff #27266)

Probably worth documenting the current max of 1920 x 1200

bjk added a subscriber: bjk.Apr 9 2017, 10:33 PM
bjk added inline comments.
bhyve/bhyve.8
331 ↗(On Diff #27266)

It is very strange to see zero-width-space+'.' on its own line like this.
".Dq off ." all on one line should be fine (and put the period outside the quotes), and there's always the Do and Dc macros if finer control is needed.

338 ↗(On Diff #27266)

ditto here.
(And the new sentence should start on a new line regardless.)

340 ↗(On Diff #27266)

s/the guests/guests

342 ↗(On Diff #27266)

mdoc.9 says "This macro should not be used" about .br; I *think* .Pp will work fine within the list environment.
(many appearances)

347 ↗(On Diff #27266)

I don't think I understand this sentence. Is there another word that can go with "memory decode is present" to clarify what exactly is present -- the decoded memory itself, or the ability to decode memory, or ...?

360 ↗(On Diff #27266)

Could use .Lk for the link.

469 ↗(On Diff #27266)

These things are always somewhat contentious, but I would pronounce "UEFI" as "You ee eff aye", which corresponds to just "a UEFI".

novel updated this revision to Diff 27302.Apr 10 2017, 3:40 PM
novel edited edge metadata.
This revision now requires review to proceed.Apr 10 2017, 3:40 PM
novel updated this revision to Diff 27303.Apr 10 2017, 3:42 PM
novel updated this revision to Diff 27304.
novel marked 6 inline comments as done.Apr 10 2017, 3:52 PM
novel added inline comments.
bhyve/bhyve.8
342 ↗(On Diff #27266)

Yeah, and it even looks nicer with .Pp instead of .br.

347 ↗(On Diff #27266)

I *think* that's "the ability to decode ...". Though I hope Peter could suggest something on that as he's clearly more familiar with the topic.

novel added inline comments.Apr 27 2017, 2:59 PM
bhyve/bhyve.8
347 ↗(On Diff #27266)

@grehan could you please help with this one?

grehan added inline comments.Apr 27 2017, 4:34 PM
bhyve/bhyve.8
347 ↗(On Diff #27266)

How about "to boot traditional BIOS guests that require the legacy VGA I/O and memory regions to be available"

bjk added inline comments.Apr 28 2017, 1:58 AM
bhyve/bhyve.8
347 ↗(On Diff #27266)

That sounds good to me, thanks.

novel abandoned this revision.Apr 28 2017, 4:26 PM

Abandon because a similar change was committed already: https://svnweb.freebsd.org/changeset/base/317545

novel reclaimed this revision.Apr 28 2017, 6:42 PM
novel updated this revision to Diff 27838.
glebius accepted this revision.Apr 29 2017, 2:10 AM
glebius added a subscriber: glebius.

I'm very sorry for committing in front of you. Do you want me to commit your updates on top or would you prefer for someone else to do it?

This revision is now accepted and ready to land.Apr 29 2017, 2:10 AM
novel added a comment.Apr 29 2017, 5:30 AM

I'm very sorry for committing in front of you. Do you want me to commit your updates on top or would you prefer for someone else to do it?

I'd love if you could commit it, thanks.

glebius requested changes to this revision.May 1 2017, 8:29 PM

I think that some parts of my version gave more precise description of options. I am not sure about the mdoc formatting, though.

Here is your options string:

rfb=host:port[,w=width,h=height][,vga=vgaconf][,wait]

Here is mine:

[wait][,vga=<on|io|off>][,rfb=[IP:]port][,w=w][,h=h]

So, what's different.

  1. In your desription the rfb= is mandatory, while indeed it is not. It is not required. After my recent changes it defaults to 127.0.0.1:5900, before them it defaulted to 0.0.0.0:random_port.
  2. If you specify rfb= you may omit the IP address, but port must be present. Your description doesn't note that. Also my documentation focuses on that this is IPv4 address, not host. bhyve doesn't resolve neither supports IPv6, your version deletes that.
  3. Your description implies setting w and h together, while indeed you can set either of them, leaving the other on default value.

I like very much your description of the vga= option.

Can you please check that and merge our versions better?

bhyve/bhyve.8
195 ↗(On Diff #27838)

I think my description was better. Without details on VNC the description is too bare. What is our side of framebuffer device? May be it is just memory region to emulate VGA, that does nothing instead of providing VGA device to the guest?

This revision now requires changes to proceed.May 1 2017, 8:29 PM
novel updated this revision to Diff 27918.May 2 2017, 1:42 PM
novel edited edge metadata.
novel added a comment.May 2 2017, 1:48 PM

I think that some parts of my version gave more precise description of options. I am not sure about the mdoc formatting, though.

Here is your options string:

rfb=host:port[,w=width,h=height][,vga=vgaconf][,wait]

Here is mine:

[wait][,vga=<on|io|off>][,rfb=[IP:]port][,w=w][,h=h]

So, what's different.

  1. In your desription the rfb= is mandatory, while indeed it is not. It is not required. After my recent changes it defaults to 127.0.0.1:5900, before them it defaulted to 0.0.0.0:random_port.
  2. If you specify rfb= you may omit the IP address, but port must be present. Your description doesn't note that. Also my documentation focuses on that this is IPv4 address, not host. bhyve doesn't resolve neither supports IPv6, your version deletes that.

For 1) and 2) I've updated the option usage string and pulled in some bits from your original description.

  1. Your description implies setting w and h together, while indeed you can set either of them, leaving the other on default value.

I've accounted that in the update option usage string as well, though decided not to explicitly mention that in text because it does not seem to be a very common case.

Also, I took courage to replace "A IP" -> "An IP" as the former looks more familiar to me (e.g. wikipedia seems to use that), though I'm not 100% sure it's correct.

I like very much your description of the vga= option.

Can you please check that and merge our versions better?

Done.

bhyve/bhyve.8
195 ↗(On Diff #27838)

Agreed, will roll back to your version.

I missed one thing yesterday. The default for vga is "on", not "io". Lines 363-364 of pci_fbuf.c.

Do you want me to fix it before commit, or would you prefer to update the phab?

grehan added a comment.May 2 2017, 6:26 PM

I missed one thing yesterday. The default for vga is "on", not "io". Lines 363-364 of pci_fbuf.c.

The default is "io" - enabled/full set to 1/0 means io-only. See the combinations in pci_fbuf_parse_opts() when parsing "vga="

Sorry, I was wrong. Thanks.

This revision was automatically updated to reflect the committed changes.