[PATCH 1/2] conf: allow display and ramfb for vfio pci hostdevs

Jonathon Jongsma posted 2 patches 1 year, 10 months ago
[PATCH 1/2] conf: allow display and ramfb for vfio pci hostdevs
Posted by Jonathon Jongsma 1 year, 10 months ago
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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH 1/2] conf: allow display and ramfb for vfio pci hostdevs
Posted by Daniel P. Berrangé 1 year, 10 months ago
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.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-28808

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 :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH 1/2] conf: allow display and ramfb for vfio pci hostdevs
Posted by Jonathon Jongsma 1 year, 10 months ago
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.
>>
>> Resolves: https://issues.redhat.com/browse/RHEL-28808
> 
> 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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org