Re: [PATCH v2 1/3] libxl: Add 'permissive' option for PCI devices

From: Simon Gaiser <simon(a)invisiblethingslab.com&gt; By setting the permissive flag the Xen guest access to the PCI config space is not filtered. This might be a security risk, but it's required for some devices and the IOMMU and interrupt remapping should (mostly?) contain it. Because of it (and that the attribute is Xen only), in an example include "permissive='no'" - to not expose users copying from documentation to extra risk. Example usage: <devices> ... <hostdev mode='subsystem' type='pci' managed='yes' permissive='yes'>

<source> <address domain='0x0000' bus='0x06' slot='0x02' function='0x0'/> </source> </hostdev> </devices> Signed-off-by: Simon Gaiser <simon(a)invisiblethingslab.com&gt; Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com&gt; --- Changes in v2: - improve documentation (version info, example) - update schema - use virTristateBool - add a test - improve commit message - news entry --- docs/formatdomain.html.in | 7 +++++-- docs/news.xml | 11 +++++++++++-

docs/schemas/domaincommon.rng | 10 ++++++++++- src/conf/domain_conf.c | 19 +++++++++++++++++++- src/conf/domain_conf.h | 1 +- src/libxl/libxl_conf.c | 1 +- tests/libxlxml2domconfigdata/moredevs-hvm.json | 6 ++++++- tests/libxlxml2domconfigdata/moredevs-hvm.xml | 5 +++++- 8 files changed, 58 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c530573..0d1146e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4987,7 +4987,7 @@ <pre> ... &lt;devices&gt; - &lt;hostdev mode='subsystem' type='pci' managed='yes'&gt; + &lt;hostdev mode='subsystem' type='pci' managed='yes' permissive='no'&gt; &lt;source&gt; &lt;address domain='0x0000' bus='0x06' slot='0x02' function='0x0'/&gt; &lt;/source&gt; @@ -5082,7 +5082,10 @@ (or <code>virsh nodedev-detach</code> before starting the guest or hot-plugging the device and <code>virNodeDeviceReAttach</code> (or <code>virsh nodedev-reattach</code>) after hot-unplug or - stopping the guest. + stopping the guest. When <code>permissive</code> + (<span class="since">since 6.3.0, Xen only</span>) is "yes"

+ the pci config space access will not be filtered. This might be + a security issue. The default is "no". </dd> <dt><code>scsi</code></dt> <dd>For SCSI devices, user is responsible to make sure the device diff --git a/docs/news.xml b/docs/news.xml index 5835013..a8e992a 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -109,6 +109,17 @@ and/or fine tuned per individual host. </description> </change> + <change> + <summary> + xen: Add support for 'permissive' PCI device option + </summary> + <description> + <code>permissive</code> is a Xen-specific PCI device option that + disables config space write filtering. It is needed for proper + functioning of some devices using non-standard config space + registers. + </description> + </change> </section> <section title="Improvements"> </section> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7f18e5b..5a71222 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3065,6 +3065,11 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <attribute name="permissive"> + <ref name="virYesNo"/> + </attribute> + </optional> <interleave> <element name="source"> <optional> @@ -4899,6 +4904,11 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <attribute name="permissive"> + <ref name="virYesNo"/> + </attribute> + </optional> <choice> <ref name="hostdevsubsyspci"/> <ref name="hostdevsubsysusb"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89cd8c5..fe1a864 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8434,6 +8434,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = &def->source.subsys.u.scsi_host; virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev; g_autofree char *managed = NULL; + g_autofree char *permissive = NULL; g_autofree char *sgio = NULL; g_autofree char *rawio = NULL; g_autofree char *backendStr = NULL; @@ -8449,6 +8450,15 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, if ((managed = virXMLPropString(node, "managed")) != NULL) ignore_value(virStringParseYesNo(managed, &def->managed)); + if ((permissive = virXMLPropString(node, "permissive")) != NULL) { + if ((def->permissive = virTristateBoolTypeFromString(permissive)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown hostdev permissive setting '%s'"), + permissive); + return -1; + } + } + sgio = virXMLPropString(node, "sgio"); rawio = virXMLPropString(node, "rawio"); model = virXMLPropString(node, "model"); @@ -26091,6 +26101,9 @@ virDomainActualNetDefFormat(virBufferPtr buf, virDomainHostdevDefPtr hostdef = virDomainNetGetActualHostdev(def); if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); + if (hostdef && hostdef->permissive) + virBufferAsprintf(buf, " permissive='%s'", + virTristateBoolTypeToString(hostdef->permissive)); } if (def->trustGuestRxFilters) virBufferAsprintf(buf, " trustGuestRxFilters='%s'", @@ -26279,6 +26292,9 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<interface type='%s'", typeStr); if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); + if (hostdef && hostdef->permissive) + virBufferAsprintf(buf, " permissive='%s'", + virTristateBoolTypeToString(hostdef->permissive)); if (def->trustGuestRxFilters) virBufferAsprintf(buf, " trustGuestRxFilters='%s'", virTristateBoolTypeToString(def->trustGuestRxFilters)); @@ -28063,6 +28079,9 @@ virDomainHostdevDefFormat(virBufferPtr buf, if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { virBufferAsprintf(buf, " managed='%s'", def->managed ? "yes" : "no"); + if (def->permissive) + virBufferAsprintf(buf, " permissive='%s'", + virTristateBoolTypeToString(def->permissive)); if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && scsisrc->sgio) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ecb80ef..24ec03c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -345,6 +345,7 @@ struct _virDomainHostdevDef { bool missing; bool readonly; bool shareable; + virTristateBool permissive; union { virDomainHostdevSubsys subsys; virDomainHostdevCaps caps; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 458dfc2..23dd0e4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -2284,6 +2284,7 @@ libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev) pcidev->bus = pcisrc->addr.bus; pcidev->dev = pcisrc->addr.slot; pcidev->func = pcisrc->addr.function; + pcidev->permissive = hostdev->permissive == VIR_TRISTATE_BOOL_YES; return 0; } diff --git a/tests/libxlxml2domconfigdata/moredevs-hvm.json b/tests/libxlxml2domconfigdata/moredevs-hvm.json index 7bfd68b..474aa2c 100644 --- a/tests/libxlxml2domconfigdata/moredevs-hvm.json +++ b/tests/libxlxml2domconfigdata/moredevs-hvm.json @@ -88,6 +88,12 @@ "dev": 16, "bus": 10, "rdm_policy": "invalid" + }, + { + "dev": 8, + "bus": 10, + "permissive": true, + "rdm_policy": "invalid" } ], "vfbs": [ diff --git a/tests/libxlxml2domconfigdata/moredevs-hvm.xml b/tests/libxlxml2domconfigdata/moredevs-hvm.xml index f7eb09f..1b6b738 100644 --- a/tests/libxlxml2domconfigdata/moredevs-hvm.xml +++ b/tests/libxlxml2domconfigdata/moredevs-hvm.xml @@ -48,6 +48,11 @@ <address type='pci' domain='0x0000' bus='0x0a' slot='0x10' function='0x0'/> </source> </interface> + <hostdev mode='subsystem' type='pci' managed='yes' permissive='yes'> + <source> + <address domain='0x0000' bus='0x0a' slot='0x08' function='0x0'/> + </source> + </hostdev> <graphics type='vnc'/> <video> <model type='cirrus' vram='8192' heads='1' primary='yes'/> base-commit: c3ace7e234ccd43d5a008d93e122e6d47cd58e17