[PATCH V3 0/3] Xen: Support PCI permissive setting with writeFiltering attribute
This is V3 of Marek's series to support the xl.cfg(5) permissive setting on PCI devices. V2 can be found here https://www.redhat.com/archives/libvir-list/2020-April/msg01230.html Changes from V2: Instead of using a permisssive attribute on the <hostdev> element, use a writeFiltering attribute on the <hostdev>'s <source> element. Rational being that the filtering of writes to the PCI config space is done at the source. Jim Fehlig (3): Xen: Add writeFiltering option for PCI devices Xen: Add support for writeFiltering in config converter News: Advertise support for writeFiltering attribute of PCI hostdevs NEWS.rst | 7 +++ docs/formatdomain.rst | 7 ++- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 14 +++++ src/conf/domain_conf.h | 1 + src/libxl/libxl_conf.c | 1 + src/libxl/xen_common.c | 52 ++++++++++++++++-- .../libxlxml2domconfigdata/moredevs-hvm.json | 6 +++ tests/libxlxml2domconfigdata/moredevs-hvm.xml | 5 ++ tests/xlconfigdata/test-fullvirt-pci.cfg | 25 +++++++++ tests/xlconfigdata/test-fullvirt-pci.xml | 53 +++++++++++++++++++ tests/xlconfigtest.c | 1 + 12 files changed, 171 insertions(+), 6 deletions(-) create mode 100644 tests/xlconfigdata/test-fullvirt-pci.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-pci.xml -- 2.28.0
By default Xen only allows guests to write "known safe" values into PCI configuration space, yet many devices require writes to other areas of the configuration space in order to operate properly. To allow writing any values Xen supports the 'permissive' setting, see xl.cfg(5) man page. This change models Xen's permissive setting by adding a writeFiltering attribute on the <source> element of a PCI hostdev. When writeFiltering is set to 'no', the Xen permissive setting will be enabled and guests will be able to write any values into the device's configuration space. The permissive setting remains disabled in the absense of the writeFiltering attribute, of if it is explicitly set to 'yes'. Signed-off-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- docs/formatdomain.rst | 7 ++++++- docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 14 ++++++++++++++ src/conf/domain_conf.h | 1 + src/libxl/libxl_conf.c | 1 + tests/libxlxml2domconfigdata/moredevs-hvm.json | 6 ++++++ tests/libxlxml2domconfigdata/moredevs-hvm.xml | 5 +++++ 7 files changed, 38 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 218f0c1718..e1bd35fd53 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3733,7 +3733,7 @@ or: ... <devices> <hostdev mode='subsystem' type='pci' managed='yes'> - <source> + <source writeFiltering='no'> <address domain='0x0000' bus='0x06' slot='0x02' function='0x0'/> </source> <boot order='1'/> @@ -3899,6 +3899,11 @@ or: ``pci`` PCI devices can only be described by their ``address``. + :since:`Since 6.7.0 (Xen only)` , the ``source`` element of a PCI device + may contain the ``writeFiltering`` attribute to control write access to + the PCI configuration space. By default Xen only allows writes of known + safe values to the configuration space. Setting ``writeFiltering='no'`` + will allow all writes to the device's PCI configuration space. ``scsi`` SCSI devices are described by both the ``adapter`` and ``address`` elements. The ``address`` element includes a ``bus`` attribute (a 2-digit diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0d0dcbc5ce..b4eb7486b9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4982,6 +4982,11 @@ <optional> <ref name="startupPolicy"/> </optional> + <optional> + <attribute name="writeFiltering"> + <ref name="virYesNo"/> + </attribute> + </optional> <element name="address"> <ref name="pciaddress"/> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8e7981bf25..bba31cfddb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8095,8 +8095,18 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, virDomainHostdevDefPtr def, unsigned int flags) { + g_autofree char *filtering = NULL; xmlNodePtr cur; + if ((filtering = virXMLPropString(node, "writeFiltering"))) { + if ((def->writeFiltering = virTristateBoolTypeFromString(filtering)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown pci writeFiltering setting '%s'"), + filtering); + return -1; + } + } + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { @@ -26119,6 +26129,10 @@ virDomainHostdevDefFormatSubsysPCI(virBufferPtr buf, g_auto(virBuffer) origstatesChildBuf = VIR_BUFFER_INIT_CHILD(&sourceChildBuf); virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; + if (def->writeFiltering != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(&sourceAttrBuf, " writeFiltering='%s'", + virTristateBoolTypeToString(def->writeFiltering)); + if (pcisrc->backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { const char *backend = virDomainHostdevSubsysPCIBackendTypeToString(pcisrc->backend); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 68be32614c..2f17053198 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 writeFiltering; union { virDomainHostdevSubsys subsys; virDomainHostdevCaps caps; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7c2c015015..0056f6fe66 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -2279,6 +2279,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->writeFiltering == VIR_TRISTATE_BOOL_NO; return 0; } diff --git a/tests/libxlxml2domconfigdata/moredevs-hvm.json b/tests/libxlxml2domconfigdata/moredevs-hvm.json index 7bfd68bd67..474aa2cef6 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 f7eb09fa3b..89ad80631d 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'> + <source writeFiltering='no'> + <address domain='0x0000' bus='0x0a' slot='0x08' function='0x0'/> + </source> + </hostdev> <graphics type='vnc'/> <video> <model type='cirrus' vram='8192' heads='1' primary='yes'/> -- 2.28.0
Add support for the writeFiltering attribute in the domXML to native config converter. Also include a test. Signed-off-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- src/libxl/xen_common.c | 52 ++++++++++++++++++++--- tests/xlconfigdata/test-fullvirt-pci.cfg | 25 +++++++++++ tests/xlconfigdata/test-fullvirt-pci.xml | 53 ++++++++++++++++++++++++ tests/xlconfigtest.c | 1 + 4 files changed, 126 insertions(+), 5 deletions(-) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 75fe7e0644..a15a3c6007 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -381,10 +381,11 @@ xenParsePCI(char *entry) int busID; int slotID; int funcID; + virTristateBool filtered = VIR_TRISTATE_BOOL_ABSENT; domain[0] = bus[0] = slot[0] = func[0] = '\0'; - /* pci=['0000:00:1b.0','0000:00:13.0'] */ + /* pci=['0000:00:1b.0','0000:00:13.0,permissive=1'] */ if (!(key = entry)) return NULL; if (!(nextkey = strchr(key, ':'))) @@ -414,14 +415,38 @@ xenParsePCI(char *entry) } key = nextkey + 1; - if (strlen(key) != 1) + if (!(nextkey = strchrnul(key, ','))) return NULL; - if (virStrncpy(func, key, 1, sizeof(func)) < 0) { + if (virStrncpy(func, key, (nextkey - key), sizeof(func)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Function %s too big for destination"), key); return NULL; } + /* options */ + while (*nextkey != '\0') { + char *data; + key = nextkey + 1; + if (!(data = strchr(key, '='))) + return NULL; + data++; + if (!(nextkey = strchrnul(key, ','))) + return NULL; + if (STRPREFIX(key, "permissive=")) { + char valuestr[5]; + int valueint; + if (virStrncpy(valuestr, data, (nextkey - data), sizeof(valuestr)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Permissive %s too big for destination"), data); + return NULL; + } + /* xl.cfg(5) specifies false as 0 and true as any other numeric value */ + if (virStrToLong_i(valuestr, NULL, 10, &valueint) < 0) + return NULL; + filtered = valueint ? VIR_TRISTATE_BOOL_NO : VIR_TRISTATE_BOOL_YES; + } + } + if (virStrToLong_i(domain, NULL, 16, &domainID) < 0) return NULL; if (virStrToLong_i(bus, NULL, 16, &busID) < 0) @@ -435,6 +460,7 @@ xenParsePCI(char *entry) return NULL; hostdev->managed = false; + hostdev->writeFiltering = filtered; hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; hostdev->source.subsys.u.pci.addr.domain = domainID; hostdev->source.subsys.u.pci.addr.bus = busID; @@ -1849,12 +1875,28 @@ xenFormatPCI(virConfPtr conf, virDomainDefPtr def) def->hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { virConfValuePtr val, tmp; char *buf; + const char *permissive_str = NULL; - buf = g_strdup_printf("%04x:%02x:%02x.%x", + switch (def->hostdevs[i]->writeFiltering) { + case VIR_TRISTATE_BOOL_YES: + permissive_str = ",permissive=0"; + break; + case VIR_TRISTATE_BOOL_NO: + permissive_str = ",permissive=1"; + break; + case VIR_TRISTATE_BOOL_ABSENT: + case VIR_TRISTATE_BOOL_LAST: + permissive_str = ""; + break; + } + + buf = g_strdup_printf("%04x:%02x:%02x.%x%s", def->hostdevs[i]->source.subsys.u.pci.addr.domain, def->hostdevs[i]->source.subsys.u.pci.addr.bus, def->hostdevs[i]->source.subsys.u.pci.addr.slot, - def->hostdevs[i]->source.subsys.u.pci.addr.function); + def->hostdevs[i]->source.subsys.u.pci.addr.function, + permissive_str); + if (VIR_ALLOC(val) < 0) { VIR_FREE(buf); diff --git a/tests/xlconfigdata/test-fullvirt-pci.cfg b/tests/xlconfigdata/test-fullvirt-pci.cfg new file mode 100644 index 0000000000..5a3f572e25 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-pci.cfg @@ -0,0 +1,25 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +pci = [ "0000:01:1a.1", "0000:02:00.0,permissive=1" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-fullvirt-pci.xml b/tests/xlconfigdata/test-fullvirt-pci.xml new file mode 100644 index 0000000000..75aa6ac25b --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-pci.xml @@ -0,0 +1,53 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>592896</memory> + <currentMemory unit='KiB'>403456</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='xenbus' index='0'/> + <controller type='ide' index='0'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='xen'/> + <source> + <address domain='0x0000' bus='0x01' slot='0x1a' function='0x1'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='xen'/> + <source writeFiltering='no'> + <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </source> + </hostdev> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index b2e045dfa5..35f437bde1 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -260,6 +260,7 @@ mymain(void) DO_TEST("fullvirt-nestedhvm-disabled"); DO_TEST("fullvirt-cpuid"); DO_TEST("fullvirt-acpi-slic"); + DO_TEST("fullvirt-pci"); #ifdef LIBXL_HAVE_VNUMA DO_TEST("fullvirt-vnuma"); DO_TEST_PARSE("fullvirt-vnuma-autocomplete", false); -- 2.28.0
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- NEWS.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 63ca689b43..eb9739618f 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,13 @@ v6.7.0 (unreleased) * **New features** + * xen: Add ``writeFiltering`` attribute for PCI devices + + By default Xen filters guest writes to the PCI configuration space of a + PCI hostdev, which may cause problems for some devices. The ``writeFiltering`` + attribute of the device's ``<source>`` element can be used to disable the + filtering and allow all guest writes to the configuration space. + * **Improvements** * **Bug fixes** -- 2.28.0
participants (1)
-
Jim Fehlig