Re: [libvirt] [PATCH v3 23/26] conf: Introduce isolation groups

Isolation groups will eventually allow us to make sure certain devices, eg. PCI hostdevs, are assigned to guest PCI buses in a way that guarantees improved isolation, error detection and recovery for machine types and hypervisors that support it, eg. pSeries guest on QEMU.

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt;

--- src/bhyve/bhyve_device.c | 4 ++-- src/conf/device_conf.h | 10 ++++++++++ src/conf/domain_addr.c | 17 ++++++++++++----- src/conf/domain_addr.h | 9 ++++++++- src/qemu/qemu_domain_address.c | 35 ++++++++++++++++++----------------- 5 files changed, 50 insertions(+), 25 deletions(-) diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index fdfd512..03aa6c9 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -57,7 +57,7 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, } if (virDomainPCIAddressReserveAddr(addrs, addr, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) { + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { goto cleanup; } @@ -100,7 +100,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, lpc_addr.slot = 0x1; if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) { + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { goto error; } diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 14be2e3..b9beafd 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -164,6 +164,16 @@ struct _virDomainDeviceInfo { */ int pciConnectFlags; /* enum virDomainPCIConnectFlags */ char *loadparm; + + /* PCI devices will only be automatically placed on a PCI bus + * that shares the same isolation group */ + int isolationGroup; + + /* Usually, PCI buses will take on the same isolation group + * as the first device that is plugged into them, but in some + * cases we might want to prevent that from happening by + * locking the isolation group */ + bool isolationGroupLocked; }; diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index b9e9145..48af1f5 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -534,6 +534,7 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags, + int isolationGroup ATTRIBUTE_UNUSED, bool fromConfig) { int ret = -1; @@ -586,9 +587,11 @@ virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs, int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, - virDomainPCIConnectFlags flags) + virDomainPCIConnectFlags flags, + int isolationGroup) { - return virDomainPCIAddressReserveAddrInternal(addrs, addr, flags, true); + return virDomainPCIAddressReserveAddrInternal(addrs, addr, flags, + isolationGroup, true); } int @@ -624,7 +627,8 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, goto cleanup; ret = virDomainPCIAddressReserveAddrInternal(addrs, &dev->addr.pci, - flags, true); + flags, dev->isolationGroup, + true); } else { ret = virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1); } @@ -745,6 +749,7 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr next_addr, virDomainPCIConnectFlags flags, + int isolationGroup ATTRIBUTE_UNUSED, int function) { virPCIDeviceAddress a = { 0 }; @@ -825,10 +830,12 @@ virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, { virPCIDeviceAddress addr; - if (virDomainPCIAddressGetNextAddr(addrs, &addr, flags, function) < 0) + if (virDomainPCIAddressGetNextAddr(addrs, &addr, flags, + dev->isolationGroup, function) < 0) return -1; - if (virDomainPCIAddressReserveAddrInternal(addrs, &addr, flags, false) < 0) + if (virDomainPCIAddressReserveAddrInternal(addrs, &addr, flags, + dev->isolationGroup, false) < 0) return -1; if (!addrs->dryRun) { diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 9cd1b29..f8d63df 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -100,6 +100,12 @@ typedef struct { * bit is set, that function is in use by a device. */ virDomainPCIAddressSlot slot[VIR_PCI_ADDRESS_SLOT_LAST + 1]; + + /* See virDomainDeviceInfo::isolationGroup */ + int isolationGroup; + + /* See virDomainDeviceInfo::isolationGroupLocked */ + bool isolationGroupLocked; } virDomainPCIAddressBus; typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr; @@ -139,7 +145,8 @@ bool virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, - virDomainPCIConnectFlags flags) + virDomainPCIConnectFlags flags, + int isolationGroup) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 50e41a4..3d56900 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1037,7 +1037,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, } if (virDomainPCIAddressReserveAddr(addrs, addr, - info->pciConnectFlags) < 0) { + info->pciConnectFlags, + info->isolationGroup) < 0) { goto cleanup; } @@ -1082,6 +1083,10 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], cont->model) < 0) goto error; + /* Forward the information about whether or not this bus should + * be allowed to change isolation groups */ + addrs->buses[idx].isolationGroupLocked = cont->info.isolationGroupLocked; + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) hasPCIeRoot = true; } @@ -1198,7 +1203,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, if (addrs->nbuses) { memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 1; - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; } @@ -1233,7 +1238,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, goto cleanup; } } else { - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; primaryVideo->info.addr.pci = tmp_addr; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1258,7 +1263,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video" " device will not be possible without manual" " intervention"); - } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) { + } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) { goto cleanup; } } @@ -1334,10 +1339,8 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, assign = true; } if (assign) { - if (virDomainPCIAddressReserveAddr(addrs, - &tmp_addr, flags) < 0) { + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; - } cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; @@ -1359,10 +1362,8 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { - if (virDomainPCIAddressReserveAddr(addrs, - &tmp_addr, flags) < 0) { + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; - } cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; @@ -1385,12 +1386,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1F; tmp_addr.function = 0; tmp_addr.multi = VIR_TRISTATE_SWITCH_ON; - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; tmp_addr.function = 3; tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT; - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; } @@ -1424,7 +1425,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, goto cleanup; } } else { - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; primaryVideo->info.addr.pci = tmp_addr; @@ -1450,8 +1451,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, " device will not be possible without manual" " intervention"); virResetLastError(); - } else if (virDomainPCIAddressReserveAddr(addrs, - &tmp_addr, flags) < 0) { + } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) { goto cleanup; } } @@ -1472,7 +1472,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&sound->info)) { continue; } - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) goto cleanup; sound->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1689,7 +1689,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (foundAddr) { /* Reserve this function on the slot we found */ if (virDomainPCIAddressReserveAddr(addrs, &addr, - cont->info.pciConnectFlags) < 0) { + cont->info.pciConnectFlags, + cont->info.isolationGroup) < 0) { goto error; }