[PATCH 0/2] Add 'display' and 'ramfb' option for pci hostdevs

see https://issues.redhat.com/browse/RHEL-28808 Jonathon Jongsma (2): conf: allow display and ramfb for vfio pci hostdevs qemu: enable display/ramfb for vfio pci hostdevs docs/formatdomain.rst | 8 ++++ src/conf/domain_conf.c | 20 ++++++++- src/conf/domain_conf.h | 2 + src/conf/domain_validate.c | 21 +++++---- src/conf/schemas/domaincommon.rng | 25 ++++++----- src/qemu/qemu_command.c | 10 ++++- src/qemu/qemu_validate.c | 8 ++++ ...stdev-pci-display-ramfb.x86_64-latest.args | 33 ++++++++++++++ ...ostdev-pci-display-ramfb.x86_64-latest.xml | 44 +++++++++++++++++++ .../hostdev-pci-display-ramfb.xml | 33 ++++++++++++++ tests/qemuxmlconftest.c | 1 + 11 files changed, 185 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml -- 2.44.0

We already allow the user to specify display="on" and ramfb="on" for mdev host devices. But newer GPU models will no longer use the mdev framework, so we should enable this same functionality for other non-mdev passthrough PCI devices. Resolves: https://issues.redhat.com/browse/RHEL-28808 Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- docs/formatdomain.rst | 8 ++++++++ src/conf/domain_conf.c | 20 +++++++++++++++++++- src/conf/domain_conf.h | 2 ++ src/conf/domain_validate.c | 21 +++++++++++++-------- src/conf/schemas/domaincommon.rng | 25 +++++++++++++++---------- 5 files changed, 57 insertions(+), 19 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 2adc2ff968..ab44ed58a6 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -4390,6 +4390,14 @@ or: starting the guest or hot-plugging the device and ``virNodeDeviceReAttach`` (or ``virsh nodedev-reattach``) after hot-unplug or stopping the guest. + :since:`Since 10.2.0` an optional ``display`` attribute may be used to + enable using a vgpu device as a display device for the guest. Supported + values are either ``on`` or ``off`` (default). There is also an optional + ``ramfb`` attribute with values of either ``on`` or ``off`` (default). + When enabled, the ``ramfb`` attribute provides a memory framebuffer device + to the guest. This framebuffer allows the vgpu to be used as a boot display + before the gpu driver is loaded within the guest. ``ramfb`` requires the + ``display`` attribute to be set to ``on``. ``scsi`` For SCSI devices, user is responsible to make sure the device is not used by host. If supported by the hypervisor and OS, the optional ``sgio`` ( diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 770b5fbbff..11a0b0ecda 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6306,6 +6306,16 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, VIR_XML_PROP_NONE, &mdevsrc->ramfb) < 0) return -1; + } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + if (virXMLPropTristateSwitch(node, "display", + VIR_XML_PROP_NONE, + &pcisrc->display) < 0) + return -1; + + if (virXMLPropTristateSwitch(node, "ramfb", + VIR_XML_PROP_NONE, + &pcisrc->ramfb) < 0) + return -1; } switch (def->source.subsys.type) { @@ -26251,6 +26261,7 @@ virDomainHostdevDefFormat(virBuffer *buf, const char *mode = virDomainHostdevModeTypeToString(def->mode); virDomainHostdevSubsysSCSI *scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysMediatedDev *mdevsrc = &def->source.subsys.u.mdev; + virDomainHostdevSubsysPCI *pcisrc = &def->source.subsys.u.pci; virDomainHostdevSubsysSCSIVHost *scsihostsrc = &def->source.subsys.u.scsi_host; const char *type; @@ -26319,7 +26330,14 @@ virDomainHostdevDefFormat(virBuffer *buf, virBufferAsprintf(buf, " ramfb='%s'", virTristateSwitchTypeToString(mdevsrc->ramfb)); } - + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + if (pcisrc->display != VIR_TRISTATE_SWITCH_ABSENT) + virBufferAsprintf(buf, " display='%s'", + virTristateSwitchTypeToString(pcisrc->display)); + if (pcisrc->ramfb != VIR_TRISTATE_SWITCH_ABSENT) + virBufferAsprintf(buf, " ramfb='%s'", + virTristateSwitchTypeToString(pcisrc->ramfb)); + } } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 76251938b8..5925faaf1a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -236,6 +236,8 @@ struct _virDomainHostdevSubsysUSB { struct _virDomainHostdevSubsysPCI { virPCIDeviceAddress addr; /* host address */ virDeviceHostdevPCIDriverInfo driver; + virTristateSwitch display; + virTristateSwitch ramfb; virBitmap *origstates; }; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index faa7659f07..395e036e8f 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1291,15 +1291,20 @@ virDomainDefHostdevValidate(const virDomainDef *def) } } - if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV && - dev->source.subsys.u.mdev.ramfb == VIR_TRISTATE_SWITCH_ON) { - if (ramfbEnabled) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only one vgpu device can have 'ramfb' enabled")); - return -1; + if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { + virTristateSwitch *ramfbsetting = NULL; + if (dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) + ramfbsetting = &dev->source.subsys.u.mdev.ramfb; + else if (dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + ramfbsetting = &dev->source.subsys.u.pci.ramfb; + if (ramfbsetting && *ramfbsetting == VIR_TRISTATE_SWITCH_ON) { + if (ramfbEnabled) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only one vgpu device can have 'ramfb' enabled")); + return -1; + } + ramfbEnabled = true; } - ramfbEnabled = true; } } diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index c992956280..f386e46fae 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6267,6 +6267,7 @@ <ref name="pciaddress"/> </element> </element> + <ref name="hostdevsubsysvfiodisplay"/> </interleave> </define> @@ -6386,6 +6387,19 @@ </element> </define> + <define name="hostdevsubsysvfiodisplay"> + <optional> + <attribute name="ramfb"> + <ref name="virOnOff"/> + </attribute> + </optional> + <optional> + <attribute name="display"> + <ref name="virOnOff"/> + </attribute> + </optional> + </define> + <define name="hostdevsubsysmdev"> <attribute name="type"> <value>mdev</value> @@ -6397,16 +6411,7 @@ <value>vfio-ap</value> </choice> </attribute> - <optional> - <attribute name="ramfb"> - <ref name="virOnOff"/> - </attribute> - </optional> - <optional> - <attribute name="display"> - <ref name="virOnOff"/> - </attribute> - </optional> + <ref name="hostdevsubsysvfiodisplay"/> <element name="source"> <ref name="mdevaddress"/> </element> -- 2.44.0

On Tue, Mar 19, 2024 at 05:25:54PM -0500, Jonathon Jongsma wrote:
We already allow the user to specify display="on" and ramfb="on" for mdev host devices. But newer GPU models will no longer use the mdev framework, so we should enable this same functionality for other non-mdev passthrough PCI devices.
This bug is private, so please describe the background more fully With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 3/20/24 3:08 AM, Daniel P. Berrangé wrote:
On Tue, Mar 19, 2024 at 05:25:54PM -0500, Jonathon Jongsma wrote:
We already allow the user to specify display="on" and ramfb="on" for mdev host devices. But newer GPU models will no longer use the mdev framework, so we should enable this same functionality for other non-mdev passthrough PCI devices.
This bug is private, so please describe the background more fully
With regards, Daniel
Apologies. The context is that we currently support enabling display and ramfb support for hostdev vGPUs, but only for those using the mdev subsystem. However, going forward, some graphics cards will no longer use mdev for their vGPUs. In order to maintain the same capability for these devices, we can also enable it for other PCI passthrough devices. Apparently this is something that's used heavily by layered products like OSP. Cheers, Jonathon

Implement display="on" and ramfb="on" for vfio PCI host devices in qemu. This enables passthrough PCI devices for display just like we did for mdevs. Resolves: https://issues.redhat.com/browse/RHEL-28808 Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_command.c | 10 ++++- src/qemu/qemu_validate.c | 8 ++++ ...stdev-pci-display-ramfb.x86_64-latest.args | 33 ++++++++++++++ ...ostdev-pci-display-ramfb.x86_64-latest.xml | 44 +++++++++++++++++++ .../hostdev-pci-display-ramfb.xml | 33 ++++++++++++++ tests/qemuxmlconftest.c | 1 + 6 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2bb1b6a0e7..62df609884 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4735,10 +4735,16 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, virDomainNetTeamingInfo *teaming; g_autofree char *host = virPCIDeviceAddressAsString(&pcisrc->addr); const char *failover_pair_id = NULL; + const char *driver = NULL; /* caller has to assign proper passthrough driver name */ switch (pcisrc->driver.name) { case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO: + /* ramfb support requires the nohotplug variant */ + if (pcisrc->ramfb == VIR_TRISTATE_SWITCH_ON) + driver = "vfio-pci-nohotplug"; + else + driver = "vfio-pci"; break; case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_KVM: @@ -4762,11 +4768,13 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, failover_pair_id = teaming->persistent; if (virJSONValueObjectAdd(&props, - "s:driver", "vfio-pci", + "s:driver", driver, "s:host", host, "s:id", dev->info->alias, "p:bootindex", dev->info->effectiveBootIndex, "S:failover_pair_id", failover_pair_id, + "S:display", qemuOnOffAuto(pcisrc->display), + "B:ramfb", pcisrc->ramfb, NULL) < 0) return NULL; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 6a73d02050..edc7169a30 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2477,6 +2477,14 @@ qemuValidateDomainDeviceDefHostdev(const virDomainHostdevDef *hostdev, _("Write filtering of PCI device configuration space is not supported by qemu")); return -1; } + + if (hostdev->source.subsys.u.pci.display == VIR_TRISTATE_SWITCH_ON) { + if (def->ngraphics == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("graphics device is needed for attribute value 'display=on' in <hostdev>")); + return -1; + } + } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: diff --git a/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args new file mode 100644 index 0000000000..6a3bfbe6fb --- /dev/null +++ b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args @@ -0,0 +1,33 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest2,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest2/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-vnc 127.0.0.1:0,audiodev=audio1 \ +-device '{"driver":"vfio-pci-nohotplug","host":"0000:06:12.5","id":"hostdev0","display":"on","ramfb":true,"bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml new file mode 100644 index 0000000000..16e8a1dee2 --- /dev/null +++ b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml @@ -0,0 +1,44 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> + </graphics> + <audio id='1' type='none'/> + <video> + <model type='none'/> + </video> + <hostdev mode='subsystem' type='pci' managed='no' display='on' ramfb='on'> + <source> + <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </hostdev> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml new file mode 100644 index 0000000000..39c84da7e1 --- /dev/null +++ b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'> + </controller> + <graphics type='vnc'/> + <hostdev mode='subsystem' type='pci' display='on' ramfb='on'> + <source> + <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/> + </source> + </hostdev> + <video> + <model type='none'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 71b59136da..c3fbb4fbca 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2069,6 +2069,7 @@ mymain(void) DO_TEST_CAPS_LATEST("hostdev-pci-address"); DO_TEST_CAPS_LATEST("hostdev-pci-address-device"); DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-pci-duplicate"); + DO_TEST_CAPS_LATEST("hostdev-pci-display-ramfb"); DO_TEST_CAPS_LATEST("hostdev-vfio"); DO_TEST_CAPS_LATEST("hostdev-vfio-multidomain"); DO_TEST_CAPS_LATEST("hostdev-mdev-precreated"); -- 2.44.0

On 3/19/24 23:25, Jonathon Jongsma wrote:
see https://issues.redhat.com/browse/RHEL-28808
Jonathon Jongsma (2): conf: allow display and ramfb for vfio pci hostdevs qemu: enable display/ramfb for vfio pci hostdevs
docs/formatdomain.rst | 8 ++++ src/conf/domain_conf.c | 20 ++++++++- src/conf/domain_conf.h | 2 + src/conf/domain_validate.c | 21 +++++---- src/conf/schemas/domaincommon.rng | 25 ++++++----- src/qemu/qemu_command.c | 10 ++++- src/qemu/qemu_validate.c | 8 ++++ ...stdev-pci-display-ramfb.x86_64-latest.args | 33 ++++++++++++++ ...ostdev-pci-display-ramfb.x86_64-latest.xml | 44 +++++++++++++++++++ .../hostdev-pci-display-ramfb.xml | 33 ++++++++++++++ tests/qemuxmlconftest.c | 1 + 11 files changed, 185 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Daniel P. Berrangé
-
Jonathon Jongsma
-
Michal Prívozník