They both provide the same bus for child devices, the only difference is
how they implement the underlying transport, but this should not be
relevant to devices.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 36071 Build 32960: arc lint + arc unit
Event Timeline
Comment Actions
@bryanv Does this seem sensible to you? It's a cleanup/simplification I've been meaning to do for a while as the current situation is a bit silly, unless there's a reason you can think of why this is a bad idea?
| sys/dev/virtio/balloon/virtio_balloon.c | ||
|---|---|---|
| 160–162 ↗ | (On Diff #81856) | These could then be combined into a single macro as a follow-up commit. |
Comment Actions
This would change the device names in places like devinfo, devctl, vmstat, etc, correct? Frankly, I prefer to keep some indication of the transport used in the device name. I have some WIP man pages changes to add a virtio_pci(4) page for modern-specific tunables, and add missing MLINK's to improve discoverability between device and kernel module names.
This has made me rethink D27929 as one step too far, and I'll effectively drop that patch and move them both under the virtio_pci bus.
I'm fine with folding the PNP into a single macro, and perhaps adding a VIRTIO_DRIVER_MODULE as well.
Comment Actions
Of the transport, yes. The leaf device attached to that doesn't include the transport in it regardless. devinfo will still print the hierarchy so vtpci_driver instances have a pcibX above/next to them. The description of the transport device still includes the transport in it.
Frankly, I prefer to keep some indication of the transport used in the device name. I have some WIP man pages changes to add a virtio_pci(4) page for modern-specific tunables, and add missing MLINK's to improve discoverability between device and kernel module names.
Yeah, I'm personally torn. I do think cosmetically it's a nice to have, but it does still seem a bit silly to have two separate buses for what is the same interface.
This has made me rethink D27929 as one step too far, and I'll effectively drop that patch and move them both under the virtio_pci bus.
Ack, that's what reminded me that I wanted to do this (and I was otherwise going to suggest you do that).
I'm fine with folding the PNP into a single macro, and perhaps adding a VIRTIO_DRIVER_MODULE as well.
Should I open a review to do that change instead (with VIRTIO_SIMPLE_PNPINFO defining the table and containing both of the MODULE_PNP_INFO calls) and then leave this one as just a cosmetic change that can be bikeshed with less importance (and then only needs to touch the transports and virtio.h, not the device drivers themselves)?
Comment Actions
For reference, this is what an AArch64 QEMU virt machine shows in devinfo when using PCI:
root@:/ # devinfo
nexus0
acpi0
cpu0
uart0
virtio0
virtio1
virtio2
virtio3
virtio4
virtio5
virtio6
virtio7
virtio8
virtio9
virtio10
virtio11
virtio12
virtio13
virtio14
virtio15
virtio16
virtio17
virtio18
virtio19
virtio20
virtio21
virtio22
virtio23
virtio24
virtio25
virtio26
virtio27
virtio28
virtio29
virtio30
virtio31
pcib0
pci0
hostb0
virtio32
vtblk0
virtio33
vtnet0
virtio34
pci_link0
pci_link1
pci_link2
pci_link3
acpi_sysresource0
acpi_button0
psci0
gic0
generic_timer0
cryptosoft0
efirtc0
root@:/ # devinfo -p vtblk0
vtblk0 virtio32 pci0 pcib0 acpi0 nexus0
root@:/ # devinfo -p vtblk0 -v
vtblk0 pnpinfo vendor=0x00001af4 device=0x1001 subvendor=0x1af4 device_type=0x00000002
virtio32 pnpinfo vendor=0x1af4 device=0x1001 subvendor=0x1af4 subdevice=0x0002 class=0x010000 at slot=1 function=0 dbsf=pci0:0:1:0
pci0
pcib0 pnpinfo _HID=PNP0A08 _UID=0 _CID=PNP0A03 at handle=\_SB_.PCI0
acpi0
nexus0 pnpinfo nexusAs a separate thing I would like to update virtio_mmio to return ENXIO when probing instances with no device rather than creating pointless empty device instances as it's not particularly useful (and the AArch64 QEMU virt machine has a particularly large number of MMIO devices available for whatever reason which leads to rather silly device numbers).
Comment Actions
Not all commands have the heirachy at hand, and I'd like to keep/improve alignment between man pages and device/module names.
Frankly, I prefer to keep some indication of the transport used in the device name. I have some WIP man pages changes to add a virtio_pci(4) page for modern-specific tunables, and add missing MLINK's to improve discoverability between device and kernel module names.
Yeah, I'm personally torn. I do think cosmetically it's a nice to have, but it does still seem a bit silly to have two separate buses for what is the same interface.
It has been this way for quite awhile and I think there is enough inertia to keep it. After more than a decade, the spec has 3 transports, and I don't forsee S/390 coming to FreeBSD anytime soon.
This has made me rethink D27929 as one step too far, and I'll effectively drop that patch and move them both under the virtio_pci bus.
Ack, that's what reminded me that I wanted to do this (and I was otherwise going to suggest you do that).
I'm fine with folding the PNP into a single macro, and perhaps adding a VIRTIO_DRIVER_MODULE as well.
Should I open a review to do that change instead (with VIRTIO_SIMPLE_PNPINFO defining the table and containing both of the MODULE_PNP_INFO calls) and then leave this one as just a cosmetic change that can be bikeshed with less importance (and then only needs to touch the transports and virtio.h, not the device drivers themselves)?
A new review for the PNP changes is fine
Comment Actions
I think it is useful that a transport driver has been attached. Also my recollection when I original did it this way for PCI, is that this is needed for if the device (network, block, etc) module is later loaded. In the ensuing decade, devctl has appeared, but for driver development it is nice to just be able to load/unload the module for hack/test/hack.
Comment Actions
Oh I didn't mean if a driver wasn't available, I meant if the device ID is 0. If it's non-zero but no driver's available that would still attach ready for if the driver gets loaded later, either automatically by devd or manually by the user.
Comment Actions
ENXIO is fine for MMIO DeviceId of 0x0. Currently the MMIO driver fails to conform to section 4.2.3.1.1 w.r.t. to this.
Comment Actions
I'd like to keep the current distinction - virito_pci and virtio_mmio - so it is clear as to what is being attached. After https://reviews.freebsd.org/D28073 there is much less boilerplate, and not likely another transport is forthcoming.