From e9f468d3f9147349453009cdc789faaaec509d67 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 22 Dec 2009 10:53:51 +0100 Subject: [PATCH] Make Xen VT-d PCI attach/detach work The Xen code for making HVM VT-d PCI passthrough attach and detach wasn't working properly: 1) In xenDaemonAttachDevice(), we were always trying to reconfigure a PCI passthrough device, even the first time we added it. This was because the code in virDomainXMLDevID() was not checking xenstore for the existence of the device, and always returning 0 (meaning that the device already existed). 2) In xenDaemonDetachDevice(), we were trying to use "device_destroy" to detach a PCI device. While you would think that is the right method to call, it's actually wrong for PCI devices. In particular, in upstream Xen (and soon in RHEL-5 Xen), device_configure is actually used to destroy a PCI device. To fix the attach problem I add a lookup into xenstore to see if the device we are trying to attach already exists. To fix the detach problem I change it so that for PCI detach (only), we use device_configure with the appropriate sxpr to do the detachment. * src/xen/xend_internal.c: don't use device_destroy for PCI devices and fix the other issues. * src/xen/xs_internal.c src/xen/xs_internal.h: add xenStoreDomainGetPCIID() --- src/xen/xend_internal.c | 70 ++++++++++++++++++++++++++++++++++++----- src/xen/xs_internal.c | 55 ++++++++++++++++++++++++++++++++ src/xen/xs_internal.h | 3 ++ 3 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index d0142d1e52..827aac4b76 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -88,7 +88,8 @@ xenDaemonFormatSxprNet(virConnectPtr conn ATTRIBUTE_UNUSED, static int xenDaemonFormatSxprOnePCI(virConnectPtr conn, virDomainHostdevDefPtr def, - virBufferPtr buf); + virBufferPtr buf, + int detach); static int virDomainXMLDevID(virDomainPtr domain, @@ -4165,7 +4166,7 @@ xenDaemonAttachDevice(virDomainPtr domain, const char *xml) dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { if (xenDaemonFormatSxprOnePCI(domain->conn, dev->data.hostdev, - &buf) < 0) + &buf, 0) < 0) goto cleanup; } else { virXendError(domain->conn, VIR_ERR_NO_SUPPORT, "%s", @@ -4217,6 +4218,8 @@ xenDaemonDetachDevice(virDomainPtr domain, const char *xml) virDomainDeviceDefPtr dev = NULL; virDomainDefPtr def = NULL; int ret = -1; + char *xendev = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, @@ -4247,8 +4250,28 @@ xenDaemonDetachDevice(virDomainPtr domain, const char *xml) if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) goto cleanup; - ret = xend_op(domain->conn, domain->name, "op", "device_destroy", - "type", class, "dev", ref, "force", "0", "rm_cfg", "1", NULL); + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { + if (dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + if (xenDaemonFormatSxprOnePCI(domain->conn, + dev->data.hostdev, + &buf, 1) < 0) + goto cleanup; + } else { + virXendError(domain->conn, VIR_ERR_NO_SUPPORT, "%s", + _("unsupported device type")); + goto cleanup; + } + xendev = virBufferContentAndReset(&buf); + ret = xend_op(domain->conn, domain->name, "op", "device_configure", + "config", xendev, "dev", ref, NULL); + VIR_FREE(xendev); + } + else { + ret = xend_op(domain->conn, domain->name, "op", "device_destroy", + "type", class, "dev", ref, "force", "0", "rm_cfg", "1", + NULL); + } cleanup: virDomainDefFree(def); @@ -5567,7 +5590,8 @@ xenDaemonFormatSxprPCI(virDomainHostdevDefPtr def, static int xenDaemonFormatSxprOnePCI(virConnectPtr conn, virDomainHostdevDefPtr def, - virBufferPtr buf) + virBufferPtr buf, + int detach) { if (def->managed) { virXendError(conn, VIR_ERR_NO_SUPPORT, "%s", @@ -5577,6 +5601,10 @@ xenDaemonFormatSxprOnePCI(virConnectPtr conn, virBufferAddLit(buf, "(pci "); xenDaemonFormatSxprPCI(def, buf); + if (detach) + virBufferAddLit(buf, "(state 'Closing')"); + else + virBufferAddLit(buf, "(state 'Initialising')"); virBufferAddLit(buf, ")"); return 0; @@ -5608,9 +5636,8 @@ xenDaemonFormatSxprAllPCI(virConnectPtr conn, * ) * ) * - * Normally there is one (device ...) block per device, but in - * wierd world of Xen PCI, once (device ...) covers multiple - * devices. + * Normally there is one (device ...) block per device, but in the + * weird world of Xen PCI, one (device ...) covers multiple devices. */ virBufferAddLit(buf, "(device (pci "); @@ -5949,6 +5976,8 @@ error: * - if disk, copy in ref the target name from description * - if network, get MAC address from description, scan XenStore and * copy in ref the corresponding vif number. + * - if pci, get BDF from description, scan XenStore and + * copy in ref the corresponding dev number. * * Returns 0 in case of success, -1 in case of failure. */ @@ -6006,6 +6035,31 @@ virDomainXMLDevID(virDomainPtr domain, } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + char *bdf; + virDomainHostdevDefPtr def = dev->data.hostdev; + + if (virAsprintf(&bdf, "%04x:%02x:%02x.%0x", + def->source.subsys.u.pci.domain, + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function) < 0) { + virReportOOMError(NULL); + return -1; + } + + strcpy(class, "pci"); + + xenUnifiedLock(priv); + xref = xenStoreDomainGetPCIID(domain->conn, domain->id, bdf); + xenUnifiedUnlock(priv); + VIR_FREE(bdf); + if (xref == NULL) + return -1; + + tmp = virStrcpy(ref, xref, ref_len); + VIR_FREE(xref); + if (tmp == NULL) + return -1; } else { virXendError(NULL, VIR_ERR_NO_SUPPORT, "%s", _("hotplug of device type not supported")); diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 10b1f8e614..8a64d4e77b 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -1055,6 +1055,61 @@ xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) { return (NULL); } +/* + * xenStoreDomainGetPCIID: + * @conn: pointer to the connection. + * @id: the domain id + * @bdf: the PCI BDF + * + * Get the reference (i.e. the string number) for the device on that domain + * which uses the given PCI address + * + * The caller must hold the lock on the privateData + * associated with the 'conn' parameter. + * + * Returns the new string or NULL in case of error, the string must be + * freed by the caller. + */ +char * +xenStoreDomainGetPCIID(virConnectPtr conn, int id, const char *bdf) +{ + char dir[80], path[128], **list = NULL, *val = NULL; + unsigned int len, i, num; + char *ret = NULL; + xenUnifiedPrivatePtr priv; + + if (id < 0) + return(NULL); + + priv = (xenUnifiedPrivatePtr) conn->privateData; + if (priv->xshandle == NULL) + return (NULL); + if (bdf == NULL) + return (NULL); + + snprintf(dir, sizeof(dir), "/local/domain/0/backend/pci/%d", id); + list = xs_directory(priv->xshandle, 0, dir, &num); + if (list == NULL) + return(NULL); + for (i = 0; i < num; i++) { + snprintf(path, sizeof(path), "%s/%s/%s", dir, list[i], "dev-0"); + if ((val = xs_read(priv->xshandle, 0, path, &len)) == NULL) + break; + + bool match = STREQ(val, bdf); + + VIR_FREE(val); + + if (match) { + ret = strdup(list[i]); + break; + } + } + + VIR_FREE(list); + return(ret); +} + /* * The caller must hold the lock on the privateData * associated with the 'conn' parameter. diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index 29e680fb07..6e0f40dc1d 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -50,6 +50,9 @@ char * xenStoreDomainGetNetworkID(virConnectPtr conn, char * xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev); +char * xenStoreDomainGetPCIID(virConnectPtr conn, + int domid, + const char *bdf); char * xenStoreDomainGetName(virConnectPtr conn, int id); int xenStoreDomainGetUUID(virConnectPtr conn, -- 2.47.2