<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 19, 2016 at 11:59 PM, Laine Stump <span dir="ltr"><<a href="mailto:laine@laine.org" target="_blank">laine@laine.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 05/18/2016 05:32 PM, Shivaprasad G Bhat wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This patch assigns multifunction pci addresses to devices in the devlist.<br>
The pciaddrs passed in the argument is not altered so that the actual call to<br>
reserve the address using virDomainPCIAddressEnsureAddr() passes. The function<br>
focuses on user given address validation and also the auto-assign of addresses.<br>
The address auto-assignment works well for PPC64 and x86-i440fx machines.<br>
</blockquote>
<br></span>
Since you know that after hotplugging these devices into this slot, you won't be able to add any new devices to it, I think it's unnecessary to keep track of exactly which functions of the slot are occupied and which aren't. Effectively, they *all* are.<br>
<br>
So instead of copy-pasting the huge chunk of code and allocating one function at a time, how about just marking the entire slot in use at a higher level rather than trying to mark individual functions? (unless you're looking at these individual bits later for something *other* than just deciding which ones to free.<br></blockquote><div> </div><div>Missed to answer to this point. I am using the function numbers later with the device_add. I need the function numbers to be passed along. So, arriving at it is important here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Note that you'll need to determine the CONNECT_TYPE at the higher level too, and be sure to choose PCI_DEVICE or PCIE_DEVICE appropriately (and if there is any attempt to mix the two, I would say you should refuse to auto-assign an address (but allow it if they manually specify the address).<span class=""><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The q35 machines needs some level of logic here to get the correct next valid<br>
slot so that the hotplug go through fine.<br>
</blockquote>
<br></span>
Can you explain that? There should be no difference.<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Signed-off-by: Shivaprasad G Bhat <<a href="mailto:sbhat@linux.vnet.ibm.com" target="_blank">sbhat@linux.vnet.ibm.com</a>><br>
---<br>
src/conf/domain_addr.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++<br>
src/conf/domain_addr.h | 4 +<br>
src/libvirt_private.syms | 1<br>
3 files changed, 204 insertions(+)<br>
<br>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c<br>
index 7ea9e4d..7c52f84 100644<br>
--- a/src/conf/domain_addr.c<br>
+++ b/src/conf/domain_addr.c<br>
@@ -454,6 +454,205 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs,<br>
return virDomainPCIAddressReserveAddr(addrs, addr, flags, true, false);<br>
}<br>
+static int<br>
+virDomainPCIAddressGetNextFunctionAddr(uint8_t *slot,<br>
+ virPCIDeviceAddressPtr addr)<br>
+{<br>
+ size_t i = 0;<br>
+<br>
+ for (i = 0; i < 8; i++) {<br>
+ if (!(*slot & 1 << i)) {<br>
+ addr->function = i;<br>
+ break;<br>
+ }<br>
+ }<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+static int<br>
+virDomainPCIAddressAssignFunctionAddr(virDomainPCIAddressSetPtr addrs,<br>
+ uint8_t *slot,<br>
+ virPCIDeviceAddressPtr addr,<br>
+ virDomainPCIConnectFlags flags,<br>
+ bool fromConfig)<br>
+{<br>
+ int ret = -1;<br>
+ char *addrStr = NULL;<br>
+ virErrorNumber errType = (fromConfig<br>
+ ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);<br>
+<br>
+ if (!(addrStr = virDomainPCIAddressAsString(addr)))<br>
+ goto cleanup;<br>
+<br>
+ /* Check that the requested bus exists, is the correct type, and we<br>
+ * are asking for a valid slot and function<br>
+ */<br>
+ if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, fromConfig))<br>
+ goto cleanup;<br>
+<br>
+ if (*slot & (1 << addr->function)) {<br>
+ virReportError(errType,<br>
+ _("Attempted double use of PCI Address %s"),<br>
+ addrStr);<br>
+ goto cleanup;<br>
+ }<br>
+ *slot |= (1 << addr->function);<br>
+ if (addr->function == 0)<br>
+ addr->multi = VIR_TRISTATE_SWITCH_ON;<br>
+ VIR_DEBUG("Reserving PCI address %s", addrStr);<br>
+<br>
+ ret = 0;<br>
+ cleanup:<br>
+ VIR_FREE(addrStr);<br>
+ return ret;<br>
+}<br>
+<br>
+<br>
+static int<br>
+virDomainPCIAddressAssignSlotNextFunction(virDomainPCIAddressSetPtr addrs,<br>
+ uint8_t *slot,<br>
+ virDomainDeviceInfoPtr dev,<br>
+ virDomainPCIConnectFlags flags)<br>
+{<br>
+ if (virDomainPCIAddressGetNextFunctionAddr(slot, &dev->addr.pci) < 0)<br>
+ return -1;<br>
+<br>
+ if (virDomainPCIAddressAssignFunctionAddr(addrs, slot, &dev->addr.pci, flags, false) < 0)<br>
+ return -1;<br>
+<br>
+ dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+<br>
+static int<br>
+virDomainPCIAddressAssignSlotAddr(virDomainPCIAddressSetPtr addrs,<br>
+ uint8_t *slot,<br>
+ virDomainDeviceInfoPtr dev)<br>
+{<br>
+ int ret = -1;<br>
+ char *addrStr = NULL;<br>
+ virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |<br>
+ VIR_PCI_CONNECT_TYPE_PCI_DEVICE);<br>
+<br>
+ if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci)))<br>
+ goto cleanup;<br>
+<br>
+ if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {<br>
+ if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)) ||<br>
+ dev->addr.pci.function != 0) {<br>
+<br>
+ if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,<br>
+ addrStr, flags, true))<br>
+ goto cleanup;<br>
+<br>
+ ret = virDomainPCIAddressAssignFunctionAddr(addrs, slot, &dev->addr.pci, flags, true);<br>
+ } else {<br>
+ virReportError(VIR_ERR_XML_ERROR,<br>
+ _("Not a multifunction device address %s"), addrStr);<br>
+ }<br>
+ } else {<br>
+ ret = virDomainPCIAddressAssignSlotNextFunction(addrs, slot, dev, flags);<br>
+ }<br>
+<br>
+ cleanup:<br>
+ VIR_FREE(addrStr);<br>
+ return ret;<br>
+}<br>
+<br>
+int<br>
+virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr addrs, virDomainDeviceDefListPtr devlist)<br>
+{<br>
+ size_t i;<br>
+ virPCIDeviceAddress addr;<br>
+ virDomainPCIAddressBusPtr bus;<br>
+ uint8_t slot;<br>
+ virDomainDeviceInfoPtr info = NULL, privinfo = NULL;<br>
+ virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK;<br>
+<br>
+ if (virDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0)<br>
+ return -1;<br>
+<br>
+ bus = &addrs->buses[addr.bus];<br>
+ slot = bus->slots[addr.slot];<br>
+<br>
+ for (i = 0; i < devlist->count; i++) {<br>
+ virDomainDeviceDefPtr dev = devlist->devs[devlist->count - i - 1];<br>
+ switch ((virDomainDeviceType) dev->type) {<br>
+ case VIR_DOMAIN_DEVICE_DISK:<br>
+ info = &dev->data.disk->info;<br>
+ break;<br>
+ case VIR_DOMAIN_DEVICE_NET:<br>
+ info = &dev->data.net->info;<br>
+ break;<br>
+ case VIR_DOMAIN_DEVICE_RNG:<br>
+ info = &dev->data.rng->info;<br>
+ break;<br>
+ case VIR_DOMAIN_DEVICE_HOSTDEV:<br>
+ info = dev->data.hostdev->info;<br>
+ break;<br>
+ case VIR_DOMAIN_DEVICE_CONTROLLER:<br>
+ info = &dev->data.controller->info;<br>
+ break;<br>
+ case VIR_DOMAIN_DEVICE_CHR:<br>
+ info = &dev->data.chr->info;<br>
+ break;<br>
+ case VIR_DOMAIN_DEVICE_FS:<br>
+ case VIR_DOMAIN_DEVICE_INPUT:<br>
+ case VIR_DOMAIN_DEVICE_SOUND:<br>
+ case VIR_DOMAIN_DEVICE_VIDEO:<br>
+ case VIR_DOMAIN_DEVICE_WATCHDOG:<br>
+ case VIR_DOMAIN_DEVICE_HUB:<br>
+ case VIR_DOMAIN_DEVICE_SMARTCARD:<br>
+ case VIR_DOMAIN_DEVICE_MEMBALLOON:<br>
+ case VIR_DOMAIN_DEVICE_NVRAM:<br>
+ case VIR_DOMAIN_DEVICE_SHMEM:<br>
+ case VIR_DOMAIN_DEVICE_LEASE:<br>
+ case VIR_DOMAIN_DEVICE_REDIRDEV:<br>
+ case VIR_DOMAIN_DEVICE_MEMORY:<br>
+ case VIR_DOMAIN_DEVICE_NONE:<br>
+ case VIR_DOMAIN_DEVICE_TPM:<br>
+ case VIR_DOMAIN_DEVICE_PANIC:<br>
+ case VIR_DOMAIN_DEVICE_GRAPHICS:<br>
+ case VIR_DOMAIN_DEVICE_LAST:<br>
+ break;<br>
+ }<br>
+<br>
+ if (i == 0 && info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {<br>
+ /* User has given an address in xml */<br>
+ bus = &addrs->buses[info->addr.pci.bus];<br>
+ slot = bus->slots[info->addr.pci.slot];<br>
+ }<br>
+<br>
+ if (privinfo) {<br>
+ if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {<br>
+ if (privinfo->addr.pci.bus != info->addr.pci.bus ||<br>
+ privinfo->addr.pci.slot != info->addr.pci.slot) {<br>
+ virReportError(VIR_ERR_XML_ERROR, "%s",<br>
+ _("Multifunction PCI device "<br>
+ "cant be split across different guest PCI slots"));<br>
+ return -1;<br>
+ }<br>
+ }<br>
+ }<br>
+<br>
+ if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {<br>
+ info->addr.pci.bus = addr.bus;<br>
+ info->addr.pci.slot = addr.slot;<br>
+ info->addr.pci.domain = addr.domain;<br>
+ }<br>
+<br>
+ if (virDomainPCIAddressAssignSlotAddr(addrs, &slot, info) < 0)<br>
+ return -1;<br>
+ privinfo = info;<br>
+ }<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+<br>
int<br>
virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,<br>
virDomainDeviceInfoPtr dev)<br>
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h<br>
index f3eda89..9eb6b9d 100644<br>
--- a/src/conf/domain_addr.h<br>
+++ b/src/conf/domain_addr.h<br>
@@ -157,6 +157,10 @@ int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs,<br>
virDomainPCIConnectFlags flags)<br>
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);<br>
+int<br>
+virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr addrs,<br>
+ virDomainDeviceDefListPtr devlist);<br>
+<br>
struct _virDomainCCWAddressSet {<br>
virHashTablePtr defined;<br>
virDomainDeviceCCWAddress next;<br>
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms<br>
index d6a60b5..d72dc63 100644<br>
--- a/src/libvirt_private.syms<br>
+++ b/src/libvirt_private.syms<br>
@@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow;<br>
virDomainPCIAddressSlotInUse;<br>
virDomainPCIAddressValidate;<br>
virDomainPCIControllerModelToConnectType;<br>
+virDomainPCIMultifunctionDeviceAddressAssign;<br>
virDomainPCIMultifunctionDeviceDefParseNode;<br>
virDomainVirtioSerialAddrAssign;<br>
virDomainVirtioSerialAddrAutoAssign;<br>
<br>
--<br>
libvir-list mailing list<br>
<a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a><br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div></div>