On Thu, 13 Feb 2025 14:50:50 +0100
Cédric Le Goater <c...@redhat.com> wrote:
> Investigate the git history to uncover when and why the VFIO
> properties were introduced and update the models. This is mostly
> targeting vfio-pci device, since vfio-plateform, vfio-ap and vfio-ccw
> devices are simpler.
>
> Organize the vfio-pci properties in topics. It would be great to have
> a way to do the same for the output.
>
> Cc: Tony Krowiak <akrow...@linux.ibm.com>
> Cc: Eric Farman <far...@linux.ibm.com>
> Cc: Eric Auger <eric.au...@redhat.com>
> Signed-off-by: Cédric Le Goater <c...@redhat.com>
> ---
>
> Changes in v2:
>
> - Fixed version numbers
> - Fixed #ifdef in vfio/ccw.c
> - Addressed vfio-pci-nohotplug
> - Organize the vfio-pci properties in topics
>
> hw/vfio/ap.c | 9 +++
> hw/vfio/ccw.c | 15 +++++
> hw/vfio/pci.c | 134 +++++++++++++++++++++++++++++++++++++++++++++
> hw/vfio/platform.c | 23 ++++++++
> 4 files changed, 181 insertions(+)
>
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index
> 30b08ad375d5ecae886c5000fbaa364799fe76d0..ec1150e5d627fce83a5a6319af471fd0aa45ae9b
> 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -257,6 +257,15 @@ static void vfio_ap_class_init(ObjectClass *klass, void
> *data)
> dc->hotpluggable = true;
> device_class_set_legacy_reset(dc, vfio_ap_reset);
> dc->bus_type = TYPE_AP_BUS;
> +
> + object_class_property_set_description(klass, /* 3.1 */
> + "sysfsdev",
> + "Host sysfs path of assigned
> device");
> +#ifdef CONFIG_IOMMUFD
> + object_class_property_set_description(klass, /* 9.0 */
> + "iommufd",
> + "Set host IOMMUFD backend device
> ");
> +#endif
> }
>
> static const TypeInfo vfio_ap_info = {
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index
> 67bc137f9be6d43c5970c6271f3cdbfffd8a32de..242dc39660fcd028722093d637b7b64440649863
> 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -717,6 +717,21 @@ static void vfio_ccw_class_init(ObjectClass *klass, void
> *data)
> cdc->handle_halt = vfio_ccw_handle_halt;
> cdc->handle_clear = vfio_ccw_handle_clear;
> cdc->handle_store = vfio_ccw_handle_store;
> +
> + object_class_property_set_description(klass, /* 2.10 */
> + "sysfsdev",
> + "Host sysfs path of assigned
> device");
> + object_class_property_set_description(klass, /* 3.0 */
> + "force-orb-pfch",
> + "Force unlimited prefetch");
> +#ifdef CONFIG_IOMMUFD
> + object_class_property_set_description(klass, /* 9.0 */
> + "iommufd",
> + "Set host IOMMUFD backend device
> ");
> +#endif
> + object_class_property_set_description(klass, /* 9.2 */
> + "loadparm",
> + "Define which devices that can be
> used for booting");
> }
>
> static const TypeInfo vfio_ccw_info = {
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index
> 9a55e7b77324babf7295132b08e3ba23b482a291..fbd8cf566b1cfd508ccb0042a395e3b79ba781c0
> 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3433,6 +3433,133 @@ static void vfio_pci_dev_class_init(ObjectClass
> *klass, void *data)
> pdc->exit = vfio_exitfn;
> pdc->config_read = vfio_pci_read_config;
> pdc->config_write = vfio_pci_write_config;
> +
> + object_class_property_set_description(klass, /* 1.3 */
> + "host",
> + "Host PCI address
> [domain:]<bus:slot.function> of assigned device");
> + object_class_property_set_description(klass, /* 2.6 */
> + "sysfsdev",
> + "Host sysfs path of assigned
> device");
> + /*
> + * Display
> + */
> +
> + object_class_property_set_description(klass, /* 1.5 */
> + "x-vga",
> + "Add support for VGA MMIO and I/O
> port access");
"Expose VGA address spaces for device"
> + object_class_property_set_description(klass, /* 2.12 */
> + "display",
> + "Add display support");
"Enable display support for device, ex. vGPU"
> + object_class_property_set_description(klass, /* 3.2 */
> + "xres",
> + "Set X display resolution the vgpu
> should use");
Maybe capitalize for consistency, vGPU.
> + object_class_property_set_description(klass, /* 3.2 */
> + "yres",
> + "Set Y display resolution the vgpu
> should use");
> +
> + /*
> + * IGD
> + */
> +
> + object_class_property_set_description(klass, /* 2.7 */
> + "x-igd-opregion",
> + "Add IGD OpRegion support for
> (headless system)");
[Cc Tomita and Corvin have more recent understanding of IGD options]
Not necessarily for headless systems, unless others have better
suggestions, maybe just "Expose host IGD OpRegion table to guest".
> + object_class_property_set_description(klass, /* 2.7 (See c4c45e943e51) */
> + "x-igd-gms",
> + "Add Intel graphics legacy mode
> device assignment support. "
> + "Assign 00:02.0 from the host to
> 00:02.0 in the VM");
Not really. Tomita added a useful comment and commit log in
37f05a59e869. Perhaps:
"Override DVMT Pre-Allocated value for IGD stolen memory. (32MiB units)"
> +
> + /*
> + * NVIDIA
> + */
> + object_class_property_set_description(klass, /* 2.12 */
> + "x-no-geforce-quirks",
> + "Disable GeForce quirks (for
> NVIDIA Quadro/GRID/Tesla). Improves performance");
> + object_class_property_set_description(klass, /* 3.0 */
> + "x-no-kvm-ioeventfd",
> + "Disable ioeventfd quirk
> (NVIDIA)");
> + object_class_property_set_description(klass, /* 3.0 */
> + "x-no-vfio-ioeventfd",
> + "Enable ioeventfd quirks to be
> handled by VFIO directly. Improves performance");
The above two should be under debugging, they're only used by NVIDIA
quirks but they exist for the purpose of validating behavior with and
without. The first disables registering an ioeventfd with KVM and the
latter disables directly "wiring" it through to vfio. Maybe
(respectively):
"Disable registration of ioeventfds with KVM"
"Disable linking of KVM ioeventfds to VFIO ioeventfds"
NB. setting the latter does not improve performance.
> + object_class_property_set_description(klass, /* 2.11 */
> + "x-nv-gpudirect-clique",
> + "Add NVIDIA GPUDirect Cliques
> support");
"Add NVIDIA GPUDirect capability indicating P2P DMA clique for device. (0~15)"
(maybe this is already specified as a uint4 given the PropertyInfo?)
> +
> + /*
> + * Migration support
> + */
> + object_class_property_set_description(klass, /* 5.2 */
> + "x-pre-copy-dirty-page-tracking",
> + "Disable dirty pages tracking
> during iterative phase");
> + object_class_property_set_description(klass, /* 9.1 */
> + "x-device-dirty-page-tracking",
> + "Disable device dirty page
> tracking and use container-based dirty page tracking");
These are really debug as well, right? They just happen to be
migration related debug.
> + object_class_property_set_description(klass, /* 5.2, 8.0 non-experimetal
> */
> + "enable-migration",
> + "Enale device migration. Also
> requires a host VFIO PCI variant "
> + "driver with migration support
> enabled");
> + object_class_property_set_description(klass, /* 9.1 */
> + "migration-events",
> + "Emit VFIO migration QAPI event
> when a VFIO device changes its migration "
> + "state. For management
> applications");
> + object_class_property_set_description(klass, /* 9.1 */
> + "skip-vsc-check",
> + "Skip config space check for
> Vendor Specific Capability. Useful for "
> + "NVIDIA vGPU driver migration");
The vsc check is really for debug too since it's enabled by default and
mostly only exists as a visible option for manipulation by the machine
type.
> +
> + /*
> + * debug, tracing
> + */
> + object_class_property_set_description(klass, /* 2.4 and 2.5 */
> + "x-no-mmap",
> + "Disable MMAP for device. Allows
> to trace MMIO accesses");
> + object_class_property_set_description(klass, /* 2.5 */
> + "x-no-kvm-intx",
> + "Bypass INTx interrupts. Allows
> interrupt tracing");
"Disable direct VFIO->KVM INTx injection. Allows to trace INTx interrupts"
s/INTx/MSI/
s/INTx/MSIx/
for the next.
> + object_class_property_set_description(klass, /* 2.5 */
> + "x-no-kvm-msi",
> + "Bypass MSI interrupts. Allows
> interrupt tracing");
> + object_class_property_set_description(klass, /* 2.5 */
> + "x-no-kvm-msix",
> + "Bypass MSIx interrupts. Allows
> interrupt tracing");
> + object_class_property_set_description(klass, /* 2.5 */
> + "x-pci-vendor-id",
> + "Set emulated PCI Vendor ID.
> Allows testing quirks");
> + object_class_property_set_description(klass, /* 2.5 */
> + "x-pci-device-id",
> + "Set emulated PCI device ID.
> Allows testing quirks");
"Override PCI Vendor ID with provided value"
s/Vendor/Device/
There are also "sub" versions of these for subsystem Vendor and Device
IDs.
None of these are really for quirks, they're more for making devices
appear as other devices. For reasons.
> +
> + /*
> + * other
> + */
> + object_class_property_set_description(klass, /* 8.1 */
> + "vf-token",
> + "Add support for VF token among PF
> and VFs (Linux 5.7+)");
"Specify UUID VF token. Required for VF when PF is owned by another VFIO
driver"
> + object_class_property_set_description(klass, /* 1.3 */
> + "x-intx-mmap-timeout-ms",
> + "Timeout value in milliseconds to
> re-enable BAR mapping when under "
> + "INTx interrupts. Improves
> performance");
Changes performance perhaps. This should also be under debug.
"When EOI is not provided by KVM/QEMU, wait time to re-enable device direct
access after INTx"
> + object_class_property_set_description(klass, /* 2.3 */
> + "x-req",
> + "Add device request notification
> support (Linux 4.0+)");
Also debug.
"Disable device request notification support"
> + object_class_property_set_description(klass, /* 3.1 */
> + "x-balloon-allowed",
> + "Allow devices to opt-in for
> ballooning");
Debug.
"Override allowing ballooning with device. (DANGER)"
> + object_class_property_set_description(klass, /* 2.5 */
> + "x-pci-sub-vendor-id",
> + "Set emulated PCI Sub-vendor ID");
> + object_class_property_set_description(klass, /* 2.5 */
> + "x-pci-sub-device-id",
> + "Set emulated PCI Sub-device ID");
Ah, here they are. Grouping with non-subsystem would make sense to me.
> + object_class_property_set_description(klass, /* 2.12 */
> + "x-msix-relocation",
> + "Allow relocating MSI-X MMIO on
> systems which page size is larger "
> + "than the PCI spec recommendation.
> Mostly for sPAPR");
Yes, we added it because of systems with >4K pages, but there do exist
devices that do not follow the PCI spec recommendation and place
non-MSI related registers too close the vector table and pba.
"Specify MSI-X MMIO relocation to the end of specified existing BAR or
new BAR to avoid virtualization overhead due to adjacent device
registers"
> +#ifdef CONFIG_IOMMUFD
> + object_class_property_set_description(klass, /* 9.0 */
> + "iommufd",
> + "Set host IOMMUFD backend device
> ");
> +#endif
> }
>
> static const TypeInfo vfio_pci_dev_info = {
> @@ -3461,6 +3588,13 @@ static void
> vfio_pci_nohotplug_dev_class_init(ObjectClass *klass, void *data)
>
> device_class_set_props(dc, vfio_pci_dev_nohotplug_properties);
> dc->hotpluggable = false;
> + object_class_property_set_description(klass, /* 3.1 */
> + "ramfb",
> + "Add ramfb support");
"Enable ramfb to provide pre-boot graphics for devices enabling display option"
> + object_class_property_set_description(klass, /* 8.2 */
> + "x-ramfb-migrate",
> + "Add ramfb migration support");
Debug.
"Override default migration support for ramfb support"
> +
> }
>
> static const TypeInfo vfio_pci_nohotplug_dev_info = {
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index
> 1070a2113a17edb9ebafb5066e51ee2bc52a767d..8e646e543692221e86b16fecd8bf40316f064a7d
> 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -674,6 +674,29 @@ static void vfio_platform_class_init(ObjectClass *klass,
> void *data)
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> /* Supported by TYPE_VIRT_MACHINE */
> dc->user_creatable = true;
> +
> + object_class_property_set_description(klass, /* 2.4 */
> + "host",
> + "Host device name of assigned
> device");
> + object_class_property_set_description(klass, /* 2.6 */
> + "sysfsdev",
> + "Host sysfs path of assigned
> device");
> + object_class_property_set_description(klass, /* 2.4 and 2.5 */
> + "x-no-mmap",
> + "Disable MMAP for device. Allows
> to trace MMIO accesses");
> + object_class_property_set_description(klass, /* 2.4 */
> + "mmap-timeout-ms",
> + "Timeout value in milliseconds to
> re-enable BAR mapping");
This is essentially the same as the vfio-pci INTx version, only used
when we don't have a mechanism for EOI.
> + object_class_property_set_description(klass, /* 2.4 */
> + "x-irqfd",
> + "Use irqfd for IRQ handling");
Debug. Allow disabling irqfd support.
Maybe rather than trying to group debug options together they should
just be explicitly labeled in the description, ex. "[DEBUG]"
Thanks,
Alex
> +
> +#ifdef CONFIG_IOMMUFD
> + object_class_property_set_description(klass, /* 9.0 */
> + "iommufd",
> + "Set host IOMMUFD backend device
> ");
> +#endif
> +
> }
>
> static const TypeInfo vfio_platform_dev_info = {
