On Wed, Jun 07, 2017 at 09:24:55PM +0200, Ján Tomko wrote: > <interface type='user'> > <mac address='52:54:56:5a:5c:5e'/> > <model type='virtio'/> > <driver iommu='on' ats='on'/> > </interface> > > https://bugzilla.redhat.com/show_bug.cgi?id=1283251 > --- > docs/formatdomain.html.in | 19 ++++ > docs/schemas/domaincommon.rng | 12 ++ > src/conf/domain_conf.c | 121 +++++++++++++++++++++ > src/conf/domain_conf.h | 10 ++ > .../qemuxml2argv-virtio-options.xml | 1 + > 5 files changed, 163 insertions(+) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index fa0edd1..e9ff73b 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -3451,6 +3451,19 @@ > </dd> > </dl> > > + <h4><a name="elementsVirtio">Virtio-related options</a></h4> > + > + <p> > + QEMU's virtio devices have some attributes related to the virtio transport under > + the <code>driver</code> element: > + The <code>iommu</code> attribute enables the use of emulated IOMMU > + by the device. The attribute <code>ats</code> controls the Address > + Translation Service support for PCIe devices. This is needed to make use > + of IOTLB support (see <a href="#elementsIommu">IOMMU device</a>). > + Possible values are <code>on</code> or <code>off</code>. > + <span class="since">Since 3.5.0</span> > + </p> > + > <h4><a name="elementsControllers">Controllers</a></h4> > > <p> > @@ -5142,6 +5155,12 @@ qemu-kvm -net nic,model=? /dev/null > <b>In general you should leave this option alone, unless you > are very certain you know what you are doing.</b> > </dd> > + <dt>virtio options</dt> > + <dd> > + For virtio interfaces, > + <a href="#elementsVirtio">Virtio-specific options</a> can also be > + set. (<span class="since">Since 3.5.0</span>) > + </dd> > </dl> > <p> > Offloading options for the host and guest can be configured using > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 5c542c7..15fd72f 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -2686,6 +2686,7 @@ > </optional> > </group> > </choice> > + <ref name="virtioOptions"/> > <interleave> > <optional> > <element name='host'> > @@ -5006,6 +5007,17 @@ > </element> > </define> > > + <define name="virtioOptions"> > + <optional> > + <attribute name="iommu"> > + <ref name="virOnOff"/> > + </attribute> > + <attribute name="ats"> > + <ref name="virOnOff"/> > + </attribute> > + </optional> > + </define> > + > <define name="usbmaster"> > <element name="master"> > <attribute name="startport"> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 710048c..22b8653 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -1112,6 +1112,46 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) > return &xmlopt->ns; > } > > +static int > +virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt, > + virDomainVirtioOptionsPtr *virtio) > +{ > + char *str = NULL; > + int ret = -1; > + int val; > + virDomainVirtioOptionsPtr res; > + > + if (VIR_ALLOC(*virtio) < 0) > + return -1; > + > + res = *virtio; > + > + if ((str = virXPathString("string(./driver/@iommu)", ctxt))) { > + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("invalid iommu value")); > + goto cleanup; > + } > + res->iommu = val; > + } > + VIR_FREE(str); > + > + if ((str = virXPathString("string(./driver/@ats)", ctxt))) { > + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("invalid ats value")); > + goto cleanup; > + } > + res->ats = val; > + } > + > + ret = 0; > + > + cleanup: > + VIR_FREE(str); > + return ret; > +} > + > > virSaveCookieCallbacksPtr > virDomainXMLOptionGetSaveCookie(virDomainXMLOptionPtr xmlopt) > @@ -1966,6 +2006,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) > VIR_FREE(def->ifname); > VIR_FREE(def->ifname_guest); > VIR_FREE(def->ifname_guest_actual); > + VIR_FREE(def->virtio); > > virNetDevIPInfoClear(&def->guestIP); > virNetDevIPInfoClear(&def->hostIP); > @@ -4340,6 +4381,28 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev, > > > static int > +virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio) > +{ > + if (!virtio) > + return 0; > + > + if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("iommu driver option is only supported " > + "for virtio devices")); > + return -1; > + } > + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("ats driver option is only supported " > + "for virtio devices")); > + return -1; > + } > + return 0; > +} > + > + > +static int > virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, > const virDomainDef *def, > virCapsPtr caps ATTRIBUTE_UNUSED, > @@ -4437,6 +4500,13 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, > } > } > > + if (dev->type == VIR_DOMAIN_DEVICE_NET) { > + virDomainNetDefPtr net = dev->data.net; > + if (STRNEQ_NULLABLE(net->model, "virtio") && > + virDomainCheckVirtioOptions(net->virtio) < 0) > + return -1; > + } > + > return 0; > } > > @@ -5235,6 +5305,24 @@ virDomainDefValidate(virDomainDefPtr def, > } > > > +static void > +virDomainVirtioOptionsFormat(virBufferPtr buf, > + virDomainVirtioOptionsPtr virtio) > +{ > + if (!virtio) > + return; > + > + if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { > + virBufferAsprintf(buf, " iommu='%s'", > + virTristateSwitchTypeToString(virtio->iommu)); > + } > + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { > + virBufferAsprintf(buf, " ats='%s'", > + virTristateSwitchTypeToString(virtio->ats)); > + } > +} > + > + > /* Generate a string representation of a device address > * @info address Device address to stringify > */ > @@ -10381,6 +10469,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, > goto error; > } > > + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) > + goto error; > + > cleanup: > ctxt->node = oldnode; > VIR_FREE(macaddr); > @@ -19005,6 +19096,30 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, > > > static bool > +virDomainVirtioOptionsCheckABIStability(virDomainVirtioOptionsPtr src, > + virDomainVirtioOptionsPtr dst) > +{ > + if (src->iommu != dst->iommu) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Target device iommu option '%s' does not " > + "match source '%s'"), > + virTristateSwitchTypeToString(dst->iommu), > + virTristateSwitchTypeToString(src->iommu)); > + return false; > + } > + if (src->ats != dst->ats) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Target device ats option '%s' does not " > + "match source '%s'"), > + virTristateSwitchTypeToString(dst->ats), > + virTristateSwitchTypeToString(src->ats)); > + return false; > + } > + return true; > +} > + > + > +static bool > virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, > virDomainDiskDefPtr dst) > { > @@ -19163,6 +19278,10 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, > return false; > } > > + if (src->virtio && dst->virtio && > + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) > + return false; Shouldn't we also check !!src->virtion == !!dst->virtio? The parser for virtio options always allocates @virtio so technically it shouldn't happen but throughout the code we check whether @virtio is allocated. I've missed the ABI stability check in the previous review, good catch. Pavel > + > if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) > return false; > > @@ -22118,6 +22237,8 @@ virDomainVirtioNetDriverFormat(char **outstr, > virBufferAsprintf(&buf, " rx_queue_size='%u'", > def->driver.virtio.rx_queue_size); > > + virDomainVirtioOptionsFormat(&buf, def->virtio); > + > if (virBufferCheckError(&buf) < 0) > return -1; > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 597fb22..b3e607b 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -156,6 +156,9 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; > typedef struct _virDomainIOMMUDef virDomainIOMMUDef; > typedef virDomainIOMMUDef *virDomainIOMMUDefPtr; > > +typedef struct _virDomainVirtioOptions virDomainVirtioOptions; > +typedef virDomainVirtioOptions *virDomainVirtioOptionsPtr; > + > /* Flags for the 'type' field in virDomainDeviceDef */ > typedef enum { > VIR_DOMAIN_DEVICE_NONE = 0, > @@ -1040,6 +1043,7 @@ struct _virDomainNetDef { > int linkstate; > unsigned int mtu; > virNetDevCoalescePtr coalesce; > + virDomainVirtioOptionsPtr virtio; > }; > > typedef enum { > @@ -2215,6 +2219,12 @@ struct _virDomainIOMMUDef { > virTristateSwitch eim; > virTristateSwitch iotlb; > }; > + > +struct _virDomainVirtioOptions { > + virTristateSwitch iommu; > + virTristateSwitch ats; > +}; > + > /* > * Guest VM main configuration > * > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml > index c88cf64..3357bc6 100644 > --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml > +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml > @@ -47,6 +47,7 @@ > <interface type='user'> > <mac address='52:54:56:58:5a:5c'/> > <model type='virtio'/> > + <driver iommu='on' ats='on'/> > <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> > </interface> > <input type='mouse' bus='virtio'> > -- > 2.10.2 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
