Page MenuHomeFreeBSD

bhyve: virtio shares definitions between sys/dev/virtio
ClosedPublic

Authored by khng on Mar 5 2021, 3:33 PM.

Details

Summary

Definitions inside usr.sbin/bhyve/virtio.h are thrown away.
Definitions in sys/dev/virtio are used instead.

This reduces code duplication.

Submitted by: Ka Ho Ng <khng@freebsdfoundation.org>
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

khng requested review of this revision.Mar 5 2021, 3:34 PM

VRING_AVAIL_F_NO_INTERRUPT and VRING_USED_F_NO_NOTIFY can also be removed

Replace VTCFG_R with VIRTIO_PCI_/VIRTIO_MSI_ from sys/dev/virtio/pci/virtio_pci_legacy_var.h

take my suggestions/questions with a grain of salt as I'm not familiar with this area of the code..

fundamentally though, this looks like a good change.

sys/dev/virtio/virtio.h
33–34

I got this error when compiling bhyve:

...[ clipped ] ...
-Wno-switch -Wno-switch-enum -Wno-knr-promoted-parameter -Qunused-arguments -c /usr/src/usr.sbin/bhyve/pci_virtio_rnd.c -o pci_virtio_rnd.o
In file included from /usr/src/usr.sbin/bhyve/pci_virtio_rnd.c:63:
In file included from /usr/src/usr.sbin/bhyve/virtio.h:36:
In file included from /usr/src/sys/dev/virtio/virtio.h:34:
/usr/src/sys/dev/virtio/virtio_endian.h:42:15: error: unknown type name 'bool'
static inline bool
[... clipped ...]
(9 errors generated)

fixed the error: https://reviews.freebsd.org/differential/diff/85257/

I provided the diff for context since virtio_endian.h isn't in the review

usr.sbin/bhyve/virtio.h
132–159

I wonder why these were __packed, but their corresponding structs in virtio_ring.h aren't?

Only a question, not a suggestion for change.

178–179

vring_size() returns an int

khng marked 2 inline comments as done.
  • Fix rew@'s compilation error
  • vring_size_aligned()'s return type should match vring_size()'s
usr.sbin/bhyve/virtio.h
132–159

These __packed does seem unnecessary to me. They belonged to 366f60834ff8e which was the first commit introducing usr.sbin/bhyve/virtio.h.

Reuse ISR status flags from sys/dev/virtio

grehan added a subscriber: grehan.

Thanks for doing this; a long overdue change.

This revision is now accepted and ready to land.Mar 6 2021, 11:10 AM

D29084?id=85260 was accidentally reverted.

This revision now requires review to proceed.Mar 6 2021, 4:40 PM
This revision is now accepted and ready to land.Mar 6 2021, 8:57 PM
khng planned changes to this revision.Mar 7 2021, 7:04 PM

vring_size has to be further aligned. Do it in vring_size_aligned now.

See 2.6.2 Legacy Interfaces: A Note on Virtqueue Layout

In D29084#652080, @khng300_gmail.com wrote:

vring_size has to be further aligned. Do it in vring_size_aligned now.

See 2.6.2 Legacy Interfaces: A Note on Virtqueue Layout

Thanks rew@ for pointing that out.

This revision is now accepted and ready to land.Mar 7 2021, 11:05 PM

Approved by: philip (mentor)