2:45 a.m.
On Thu, May 19, 2016 at 11:59 PM, Laine Stump <laine(a)laine.org> wrote:
On 05/18/2016 05:32 PM, Shivaprasad G Bhat wrote:
> This patch assigns multifunction pci addresses to devices in the devlist.
> The pciaddrs passed in the argument is not altered so that the actual
> call to
> reserve the address using virDomainPCIAddressEnsureAddr() passes. The
> function
> focuses on user given address validation and also the auto-assign of
> addresses.
> The address auto-assignment works well for PPC64 and x86-i440fx machines.
>
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.
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.
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.
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).
> The q35 machines needs some level of logic here to get the correct next
> valid
> slot so that the hotplug go through fine.
>
Can you explain that? There should be no difference.
> Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
> ---
> src/conf/domain_addr.c | 199
> ++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_addr.h | 4 +
> src/libvirt_private.syms | 1
> 3 files changed, 204 insertions(+)
>
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 7ea9e4d..7c52f84 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -454,6 +454,205 @@
> virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs,
> return virDomainPCIAddressReserveAddr(addrs, addr, flags, true,
> false);
> }
> +static int
> +virDomainPCIAddressGetNextFunctionAddr(uint8_t *slot,
> + virPCIDeviceAddressPtr addr)
> +{
> + size_t i = 0;
> +
> + for (i = 0; i < 8; i++) {
> + if (!(*slot & 1 << i)) {
> + addr->function = i;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int
> +virDomainPCIAddressAssignFunctionAddr(virDomainPCIAddressSetPtr addrs,
> + uint8_t *slot,
> + virPCIDeviceAddressPtr addr,
> + virDomainPCIConnectFlags flags,
> + bool fromConfig)
> +{
> + int ret = -1;
> + char *addrStr = NULL;
> + virErrorNumber errType = (fromConfig
> + ? VIR_ERR_XML_ERROR :
> VIR_ERR_INTERNAL_ERROR);
> +
> + if (!(addrStr = virDomainPCIAddressAsString(addr)))
> + goto cleanup;
> +
> + /* Check that the requested bus exists, is the correct type, and we
> + * are asking for a valid slot and function
> + */
> + if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags,
> fromConfig))
> + goto cleanup;
> +
> + if (*slot & (1 << addr->function)) {
> + virReportError(errType,
> + _("Attempted double use of PCI Address %s"),
> + addrStr);
> + goto cleanup;
> + }
> + *slot |= (1 << addr->function);
> + if (addr->function == 0)
> + addr->multi = VIR_TRISTATE_SWITCH_ON;
> + VIR_DEBUG("Reserving PCI address %s", addrStr);
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(addrStr);
> + return ret;
> +}
> +
> +
> +static int
> +virDomainPCIAddressAssignSlotNextFunction(virDomainPCIAddressSetPtr
> addrs,
> + uint8_t *slot,
> + virDomainDeviceInfoPtr dev,
> + virDomainPCIConnectFlags flags)
> +{
> + if (virDomainPCIAddressGetNextFunctionAddr(slot, &dev->addr.pci) < 0)
> + return -1;
> +
> + if (virDomainPCIAddressAssignFunctionAddr(addrs, slot,
> &dev->addr.pci, flags, false) < 0)
> + return -1;
> +
> + dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> +
> + return 0;
> +}
> +
> +
> +static int
> +virDomainPCIAddressAssignSlotAddr(virDomainPCIAddressSetPtr addrs,
> + uint8_t *slot,
> + virDomainDeviceInfoPtr dev)
> +{
> + int ret = -1;
> + char *addrStr = NULL;
> + virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
> + VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
> +
> + if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci)))
> + goto cleanup;
> +
> + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> + if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi ==
> VIR_TRISTATE_SWITCH_ON)) ||
> + dev->addr.pci.function != 0) {
> +
> + if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,
> + addrStr, flags, true))
> + goto cleanup;
> +
> + ret = virDomainPCIAddressAssignFunctionAddr(addrs, slot,
> &dev->addr.pci, flags, true);
> + } else {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Not a multifunction device address %s"),
> addrStr);
> + }
> + } else {
> + ret = virDomainPCIAddressAssignSlotNextFunction(addrs, slot,
> dev, flags);
> + }
> +
> + cleanup:
> + VIR_FREE(addrStr);
> + return ret;
> +}
> +
> +int
> +virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr
> addrs, virDomainDeviceDefListPtr devlist)
> +{
> + size_t i;
> + virPCIDeviceAddress addr;
> + virDomainPCIAddressBusPtr bus;
> + uint8_t slot;
> + virDomainDeviceInfoPtr info = NULL, privinfo = NULL;
> + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK;
> +
> + if (virDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0)
> + return -1;
> +
> + bus = &addrs->buses[addr.bus];
> + slot = bus->slots[addr.slot];
> +
> + for (i = 0; i < devlist->count; i++) {
> + virDomainDeviceDefPtr dev = devlist->devs[devlist->count - i -
> 1];
> + switch ((virDomainDeviceType) dev->type) {
> + case VIR_DOMAIN_DEVICE_DISK:
> + info = &dev->data.disk->info;
> + break;
> + case VIR_DOMAIN_DEVICE_NET:
> + info = &dev->data.net->info;
> + break;
> + case VIR_DOMAIN_DEVICE_RNG:
> + info = &dev->data.rng->info;
> + break;
> + case VIR_DOMAIN_DEVICE_HOSTDEV:
> + info = dev->data.hostdev->info;
> + break;
> + case VIR_DOMAIN_DEVICE_CONTROLLER:
> + info = &dev->data.controller->info;
> + break;
> + case VIR_DOMAIN_DEVICE_CHR:
> + info = &dev->data.chr->info;
> + break;
> + case VIR_DOMAIN_DEVICE_FS:
> + case VIR_DOMAIN_DEVICE_INPUT:
> + case VIR_DOMAIN_DEVICE_SOUND:
> + case VIR_DOMAIN_DEVICE_VIDEO:
> + case VIR_DOMAIN_DEVICE_WATCHDOG:
> + case VIR_DOMAIN_DEVICE_HUB:
> + case VIR_DOMAIN_DEVICE_SMARTCARD:
> + case VIR_DOMAIN_DEVICE_MEMBALLOON:
> + case VIR_DOMAIN_DEVICE_NVRAM:
> + case VIR_DOMAIN_DEVICE_SHMEM:
> + case VIR_DOMAIN_DEVICE_LEASE:
> + case VIR_DOMAIN_DEVICE_REDIRDEV:
> + case VIR_DOMAIN_DEVICE_MEMORY:
> + case VIR_DOMAIN_DEVICE_NONE:
> + case VIR_DOMAIN_DEVICE_TPM:
> + case VIR_DOMAIN_DEVICE_PANIC:
> + case VIR_DOMAIN_DEVICE_GRAPHICS:
> + case VIR_DOMAIN_DEVICE_LAST:
> + break;
> + }
> +
> + if (i == 0 && info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
{
> + /* User has given an address in xml */
> + bus = &addrs->buses[info->addr.pci.bus];
> + slot = bus->slots[info->addr.pci.slot];
> + }
> +
> + if (privinfo) {
> + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> + if (privinfo->addr.pci.bus != info->addr.pci.bus ||
> + privinfo->addr.pci.slot != info->addr.pci.slot) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Multifunction PCI device "
> + "cant be split across different guest
> PCI slots"));
> + return -1;
> + }
> + }
> + }
> +
> + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> + info->addr.pci.bus = addr.bus;
> + info->addr.pci.slot = addr.slot;
> + info->addr.pci.domain = addr.domain;
> + }
> +
> + if (virDomainPCIAddressAssignSlotAddr(addrs, &slot, info) < 0)
> + return -1;
> + privinfo = info;
> + }
> +
> + return 0;
> +}
> +
> +
> int
> virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
> virDomainDeviceInfoPtr dev)
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index f3eda89..9eb6b9d 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -157,6 +157,10 @@ int
> virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs,
> virDomainPCIConnectFlags flags)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +int
> +virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr
> addrs,
> + virDomainDeviceDefListPtr
> devlist);
> +
> struct _virDomainCCWAddressSet {
> virHashTablePtr defined;
> virDomainDeviceCCWAddress next;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d6a60b5..d72dc63 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow;
> virDomainPCIAddressSlotInUse;
> virDomainPCIAddressValidate;
> virDomainPCIControllerModelToConnectType;
> +virDomainPCIMultifunctionDeviceAddressAssign;
> virDomainPCIMultifunctionDeviceDefParseNode;
> virDomainVirtioSerialAddrAssign;
> virDomainVirtioSerialAddrAutoAssign;
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
>