bhyve: Implement virtio 1.0

[aokblast]

Summary

Virtio 1.0 mostly reuses functionality from the previous version. It
adds a new custom BAR with four capabilities: common configuration,
notification, interrupt, and device configuration.

The common configuration mostly exposes information already available in
the previous version. One difference is that device feature negotiation
is extended from 32 bits to 128 bits. We currently support only 64 bits,
as the upper bits are reserved for future use and are not used by any
existing devices. As suggested by the virtio specification, unsupported
bits are ignored. The negotiation process now has a clearly defined
status model. Another change is that the virtqueue descriptor, available
ring, and used ring are configured using separate guest addresses. This
is provided through six different commands (three addresses, each split
into low and high 32-bit parts). Finally, the device may reduce the
queue size during negotiation. As a result, we record the maximum queue
size from the initial configuration and restore it when rolling back.

The notification capability serves as the doorbell register, equivalent
to VIRTIO_PCI_QUEUE_NOTIFY in the previous PCI transport
specification.

The interrupt capability serves as the interrupt status and
acknowledgment mechanism, similar to VIRTIO_PCI_ISR in the previous
specification.

The device configuration capability provides access to device-specific
configuration data, replacing reads from VIRTIO_PCI_CONFIG_OFF in the
previous version.

[aokblast]

Renaming

[aokblast]

Goto correct branch

[aokblast]

I try to make bhyve 1.0 only (non-transitional device) but it seems that edk2 doesn't like it So we supports them at the same time.

[bnovkov]

usr.sbin/bhyve/virtio.c

179–186

This initialization block appears multiple times throughout the function, would it make sense to split it off into a separate function?
It would definitely help improve the readability IMO.

257

Where does this constant number come from?

309

Can this value ever be set to something other than 0 or 1? The comment in struct virtio_softc seems to imply so, but I want to double-check since I'm not that familiar with virtio.
Looking at the code it seems that it's possible for a buggy device driver to set vs_drv_feature_sel to an arbitrary value, would it make sense to turn these if-else checks into an explicit assertion here (and in other blocks that use vs_drv_feature_sel)?

636–639

I think that these else-if statements would benefit from curly braces since they are a bit dense and slightly less readable (especially this empty block with a comment).

[aokblast]

Fix snapshot and minor fixes

[aokblast]

Thansk for your feedback.

usr.sbin/bhyve/virtio.c

257

They are the same from the legacy version.

Virtio spec 1.2 section 2.7.2:

#define ALIGN(x) (((x) + qalign) & "qalign) 
static inline unsigned virtq_size(unsigned int qsz) 
{ 
     return ALIGN(sizeof(struct virtq_desc)*qsz + sizeof(u16)*(3 + qsz)) 
          + ALIGN(sizeof(u16)*3 + sizeof(struct virtq_used_elem)*qsz); 
}

309

Virtio Spec ver 1.2 section 2.2
The preferred way for handling reserved and unexpected features is that the driver ignores them.

[aokblast]

Fix snapshot again

[andy_omniosce.org]

If you haven't already seen it, transitional device support was added in illumos bhyve at the start of this year. It hasn't been upstreamed back to FreeBSD yet but that was planned for some point. Packed queues weren't part of that.

https://github.com/illumos/illumos-gate/commit/c3b97060722accbd08cd9eb3f18cc189b2c07b5e

[aokblast]

In D57364#1314419, @andy_omniosce.org wrote:

If you haven't already seen it, transitional device support was added in illumos bhyve at the start of this year. It hasn't been upstreamed back to FreeBSD yet but that was planned for some point. Packed queues weren't part of that.

https://github.com/illumos/illumos-gate/commit/c3b97060722accbd08cd9eb3f18cc189b2c07b5e

Hi. I didn't notice that. Thanks for the remind! Packed queues is on my WIP tree but if you have that. I can try to rebase on your work. I think I will leave the patch here but with no further update as some Linux distros have already required 1.0 spec.

[aokblast]

In D57364#1314419, @andy_omniosce.org wrote:

If you haven't already seen it, transitional device support was added in illumos bhyve at the start of this year. It hasn't been upstreamed back to FreeBSD yet but that was planned for some point. Packed queues weren't part of that.

https://github.com/illumos/illumos-gate/commit/c3b97060722accbd08cd9eb3f18cc189b2c07b5e

I have this line in my original code. And edk2 refused to load the device while we have revid = 1. I delete this line in here.

pci_set_cfgdata16(pi, PCIR_REVID, 1);