[PATCH] Support x-vga=on for PCI host passthrough devices

[Steven Newbury]

On Wed, 2020-10-07 at 14:20 +0100, Steven Newbury wrote:
> On Wed, 2020-10-07 at 15:07 +0200, Peter Krempa wrote:
> > On Wed, Oct 07, 2020 at 13:59:35 +0100, Steven Newbury wrote:
> > > 
> > >  7 files changed, 26 insertions(+)
> > > 
> > > diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> > > index 87bf32bbc6..02d226747e 100644
> > > --- a/src/conf/device_conf.c
> > > +++ b/src/conf/device_conf.c
> > > @@ -215,6 +215,7 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
> > >      g_autofree char *slot     = virXMLPropString(node, "slot");
> > >      g_autofree char *function = virXMLPropString(node,
> > > "function");
> > >      g_autofree char *multi    = virXMLPropString(node,
> > > "multifunction");
> > > +    g_autofree char *vga      = virXMLPropString(node, "vga");
> > >  
> > >      memset(addr, 0, sizeof(*addr));
> > >  
> > > @@ -253,6 +254,14 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
> > >                         multi);
> > >          return -1;
> > >  
> > > +    }
> > > +    if (vga &&
> > > +        ((addr->vga = virTristateSwitchTypeFromString(vga)) <=
> > > 0))
> > > {
> > > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > +                       _("Unknown value '%s' for <address> 'vga'
> > > attribute"),
> > > +                       vga);
> > > +        return -1;
> > > +
> > >      }
> > >      if (!virPCIDeviceAddressIsEmpty(addr) &&
> > > !virPCIDeviceAddressIsValid(addr, true))
> > >          return -1;
> > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > index c003b5c030..048b0f4028 100644
> > > --- a/src/conf/domain_conf.c
> > > +++ b/src/conf/domain_conf.c
> > > @@ -7587,6 +7587,10 @@ virDomainDeviceInfoFormat(virBufferPtr
> > > buf,
> > >              virBufferAsprintf(&attrBuf, " multifunction='%s'",
> > >                                virTristateSwitchTypeToString(info
> > > -
> > > > addr.pci.multi));
> > >          }
> > > +        if (info->addr.pci.vga) {
> > > +            virBufferAsprintf(&attrBuf, " vga='%s'",
> > > +                              virTristateSwitchTypeToString(info
> > > -
> > > > addr.pci.vga));
> > > +        }
> > >  
> > >          if (virZPCIDeviceAddressIsPresent(&info->addr.pci.zpci)) 
> > > {
> > >              virBufferAsprintf(&childBuf,
> > 
> > We also need validation that this isn't used with devices where it
> > doesn't make sense ... such as disk, or network card.

As I see it there are two distinct validation requirements:

1) It should only apply to hostdev PCI vfio devices

2) It is only relevant to PCI Class 3 (Display Controller), but libvirt
doesn't know about the PCI device class, does it?


1 is the most important probably.  Should it just be hard error if it's
defined for each devices type except for hostdev in domain_conf.c or
check for device type hostdev before setting the parameter and ignore
it otherwise?