On Fri, 14 Feb 2025 15:34:15 +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-platform, vfio-ap and vfio-ccw
> devices are simpler.
> 
> Sort the properties based on the QEMU version in which they were
> introduced.
> 
> 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>
> ---
> 
>  Should we introduce documentation for properties like the kernel has
>  in Documentation/ABI/*/sysfs-* ?
> 
>  Changes in v3:
> 
>  - Re-organized the vfio-pci properties based on the QEMU version in
>    which they were introduced
>  - Added property labels
>  - Improved description as suggested by Alex, Tomita and Corvin
> 
>  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      | 122 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/vfio/platform.c |  25 ++++++++++
>  4 files changed, 171 insertions(+)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 
> 30b08ad375d5ecae886c5000fbaa364799fe76d0..c7ab4ff57ada0ed0e5a76f52b5a05c86ca4fe0b4
>  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..ea766ae26c744c05515e1cc80a486a3462d82834
>  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..cdf4cdcf88c143ff145427cae3fd64239a2d1e74
>  100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3433,6 +3433,119 @@ 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, /* 1.3 */
> +                                          "x-intx-mmap-timeout-ms",
> +                                          "When EOI is not provided by 
> KVM/QEMU, wait time "
> +                                          "(milliseconds) to re-enable 
> device direct access "
> +                                          "after INTx (DEBUG)");
> +    object_class_property_set_description(klass, /* 1.5 */
> +                                          "x-vga",
> +                                          "Expose VGA address spaces for 
> device");
> +    object_class_property_set_description(klass, /* 2.3 */
> +                                          "x-req",
> +                                          "Disable device request 
> notification support (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.4 and 2.5 */
> +                                          "x-no-mmap",
> +                                          "Disable MMAP for device. Allows 
> to trace MMIO "
> +                                          "accesses (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.5 */
> +                                          "x-no-kvm-intx",
> +                                          "Disable direct VFIO->KVM INTx 
> injection. Allows to "
> +                                          "trace INTx interrupts (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.5 */
> +                                          "x-no-kvm-msi",
> +                                          "Disable direct VFIO->KVM MSI 
> injection. Allows to "
> +                                          "trace MSI interrupts (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.5 */
> +                                          "x-no-kvm-msix",
> +                                          "Disable direct VFIO->KVM MSIx 
> injection. Allows to "
> +                                          "trace MSIx interrupts (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.5 */
> +                                          "x-pci-vendor-id",
> +                                          "Override PCI Vendor ID with 
> provided value (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.5 */
> +                                          "x-pci-device-id",
> +                                          "Override PCI device ID with 
> provided value (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.5 */
> +                                          "x-pci-sub-vendor-id",
> +                                          "Override PCI Sub Vendor ID with 
> provided value (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.5 */
> +                                          "x-pci-sub-device-id",
> +                                          "Override PCI Sub Device ID with 
> provided value (DEBUG)");

Nit, we don't seem to have a character limit, so I'd go ahead and
s/Sub/Subsystem/.


> +    object_class_property_set_description(klass, /* 2.6 */
> +                                          "sysfsdev",
> +                                          "Host sysfs path of assigned 
> device");
> +    object_class_property_set_description(klass, /* 2.7 */
> +                                          "x-igd-opregion",
> +                                          "Expose host IGD OpRegion to 
> guest");
> +    object_class_property_set_description(klass, /* 2.7 (See c4c45e943e51) */
> +                                          "x-igd-gms",
> +                                          "Override IGD data stolen memory 
> size (32MiB units). "
> +                                          "Assign 00:02.0 from the host to 
> 00:02.0 in the VM");

Maybe it's a good general hint to assign IGD identity mapped to the
same PCI address in the guest as on the host, but it doesn't seem
particularly relevant to this specific option.

> +    object_class_property_set_description(klass, /* 2.11 */
> +                                          "x-nv-gpudirect-clique",
> +                                          "Add NVIDIA GPUDirect capability 
> indicating P2P DMA "
> +                                          "clique for device [0-15]");
> +    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, /* 2.12 */
> +                                          "display",
> +                                          "Enable display support for 
> device, ex. vGPU");
> +    object_class_property_set_description(klass, /* 2.12 */
> +                                          "x-msix-relocation",
> +                                          "Specify MSI-X MMIO relocation to 
> the end of specified "
> +                                          "existing BAR or new BAR to avoid 
> virtualization overhead "
> +                                          "due to adjacent device registers 
> (sPAPR)");

I'd drop sPAPR reference, there are other reasons it can be useful.

> +    object_class_property_set_description(klass, /* 3.0 */
> +                                          "x-no-kvm-ioeventfd",
> +                                          "Disable registration of 
> ioeventfds with KVM (DEBUG)");
> +    object_class_property_set_description(klass, /* 3.0 */
> +                                          "x-no-vfio-ioeventfd",
> +                                          "Disable linking of KVM ioeventfds 
> to VFIO ioeventfds"
> +                                          " (NVIDIA, DEBUG)");

Likewise dropping NVIDIA here.  Only currently used by an NVIDIA quirk,
but that could change.

> +    object_class_property_set_description(klass, /* 3.1 */
> +                                          "x-balloon-allowed",
> +                                          "Override allowing ballooning with 
> device (DEBUG, DANGER)");
> +    object_class_property_set_description(klass, /* 3.2 */
> +                                          "xres",
> +                                          "Set X display resolution the vGPU 
> should use");
> +    object_class_property_set_description(klass, /* 3.2 */
> +                                          "yres",
> +                                          "Set Y display resolution the vGPU 
> should use");
> +    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, /* 5.2, 8.0 non-experimetal 
> */
> +                                          "enable-migration",
> +                                          "Enale device migration. Also 
> requires a host VFIO PCI "
> +                                          "variant driver with migration 
> support enabled");

Variant or mdev.

> +    object_class_property_set_description(klass, /* 8.1 */
> +                                          "vf-token",
> +                                          "Specify UUID VF token. Required 
> for VF when PF is owned "
> +                                          "by another VFIO driver");
> +#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.1 */
> +                                          "x-device-dirty-page-tracking",
> +                                          "Disable device dirty page 
> tracking and use "
> +                                          "container-based dirty page 
> tracking");

This sounds like debug to me.

> +    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 "
> +                                          "(NVIDIA, DEBUG)");

Required for a specific migration use case with NVIDIA, but the option
is generic.  Given the polarity is by default true for the current
machine type, we might also indicate this provides an option for strict
VSC content enforcement.

>  }
>  
>  static const TypeInfo vfio_pci_dev_info = {
> @@ -3461,6 +3574,15 @@ 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",
> +                                          "Enable ramfb to provide pre-boot 
> graphics for devices "
> +                                          "enabling display option");
> +    object_class_property_set_description(klass, /* 8.2 */
> +                                          "x-ramfb-migrate",
> +                                          "Override default migration 
> support for ramfb support "
> +                                          "(DEBUG)");
>  }
>  
>  static const TypeInfo vfio_pci_nohotplug_dev_info = {
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 
> 1070a2113a17edb9ebafb5066e51ee2bc52a767d..61b8b856fac26e26a4805dc2d5e65266b907ef58
>  100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -674,6 +674,31 @@ 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.4 and 2.5 */
> +                                          "x-no-mmap",
> +                                          "Disable MMAP for device. Allows 
> to trace MMIO "
> +                                          "accesses (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.4 */
> +                                          "mmap-timeout-ms",
> +                                          "When EOI is not provided by 
> KVM/QEMU, wait time "
> +                                          "(milliseconds) to re-enable 
> device direct access "
> +                                          "after INTx (DEBUG)");

INTx is a PCI-ism, maybe we should just call it level interrupts.

Thanks!
Alex

> +    object_class_property_set_description(klass, /* 2.4 */
> +                                          "x-irqfd",
> +                                          "Allow disabling irqfd support 
> (DEBUG)");
> +    object_class_property_set_description(klass, /* 2.6 */
> +                                          "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_platform_dev_info = {


Reply via email to