From 0b739987779a9f197d558f9c3b31d557a564bb6d Mon Sep 17 00:00:00 2001

Message-Id: <0b739987779a9f197d558f9c3b31d557a564bb6d@dist-git>

From: Andrea Bolognani <abologna@redhat.com>

Date: Tue, 18 Jul 2017 12:10:05 +0200

Subject: [PATCH] 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.

This patch merely defines storage for the new information

we're going to need later on and makes sure it is passed from

the hypervisor driver (QEMU / bhyve) down to the generic PCI

address allocation code.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

Reviewed-by: Laine Stump <laine@laine.org>

(cherry picked from commit b8b6abbcd431d02ac106ee729204552d80762aae)

Conflicts:

  * src/conf/device_conf.h:

    caused by 54fa1b44afc8bfe4a36c7c4e3b2fe1fafde0506c, where

    virDomainDeviceInfo::loadparm is introduced, not being

    present in the tree;

  * src/conf/domain_conf.c:

    caused mostly by the absence of commit

    0c53382d820aac4767895a727c01de23397a6aaa and hence the

    virDomainSkipBackcompatConsole() function, but

    54fa1b44afc8bfe4a36c7c4e3b2fe1fafde0506c mentioned above

    also plays a role.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1280542

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 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/conf/domain_conf.c         |  2 ++

 src/qemu/qemu_domain_address.c | 35 ++++++++++++++++++-----------------

 6 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c

index fdfd512e10..03aa6c93bd 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 a20de853f8..6436a8cf3f 100644

--- a/src/conf/device_conf.h

+++ b/src/conf/device_conf.h

@@ -167,6 +167,16 @@ struct _virDomainDeviceInfo {

      * assignment, never saved and never reported.

      */

     int pciConnectFlags; /* enum virDomainPCIConnectFlags */

+

+    /* PCI devices will only be automatically placed on a PCI bus

+     * that shares the same isolation group */

+    unsigned 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 8990372ae0..a067493136 100644

--- a/src/conf/domain_addr.c

+++ b/src/conf/domain_addr.c

@@ -548,6 +548,7 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)

 virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,

                                        virPCIDeviceAddressPtr addr,

                                        virDomainPCIConnectFlags flags,

+                                       unsigned int isolationGroup ATTRIBUTE_UNUSED,

                                        bool fromConfig)

 {

     int ret = -1;

@@ -600,9 +601,11 @@ virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,

 int

 virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs,

                                virPCIDeviceAddressPtr addr,

-                               virDomainPCIConnectFlags flags)

+                               virDomainPCIConnectFlags flags,

+                               unsigned int isolationGroup)

 {

-    return virDomainPCIAddressReserveAddrInternal(addrs, addr, flags, true);

+    return virDomainPCIAddressReserveAddrInternal(addrs, addr, flags,

+                                                  isolationGroup, true);

 }

 int

@@ -638,7 +641,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);

     }

@@ -759,6 +763,7 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)

 virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs,

                                virPCIDeviceAddressPtr next_addr,

                                virDomainPCIConnectFlags flags,

+                               unsigned int isolationGroup ATTRIBUTE_UNUSED,

                                int function)

 {

     virPCIDeviceAddress a = { 0 };

@@ -839,10 +844,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 49f30332f0..01dbc5114f 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 */

+    unsigned int isolationGroup;

+

+    /* See virDomainDeviceInfo::isolationGroupLocked */

+    bool isolationGroupLocked;

 } virDomainPCIAddressBus;

 typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr;

@@ -155,7 +161,8 @@ int virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,

 int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs,

                                    virPCIDeviceAddressPtr addr,

-                                   virDomainPCIConnectFlags flags)

+                                   virDomainPCIConnectFlags flags,

+                                   unsigned int isolationGroup)

     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

 int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs,

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index f28f6aff63..5941a3a4c4 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -3608,6 +3608,8 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)

     memset(&info->addr, 0, sizeof(info->addr));

     info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;

     VIR_FREE(info->romfile);

+    info->isolationGroup = 0;

+    info->isolationGroupLocked = false;

 }

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c

index d19ffea7c9..02e214b8dd 100644

--- a/src/qemu/qemu_domain_address.c

+++ b/src/qemu/qemu_domain_address.c

@@ -1032,7 +1032,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,

     }

     if (virDomainPCIAddressReserveAddr(addrs, addr,

-                                       info->pciConnectFlags) < 0) {

+                                       info->pciConnectFlags,

+                                       info->isolationGroup) < 0) {

         goto cleanup;

     }

@@ -1077,6 +1078,10 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,

         if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], cont->model) < 0)

             goto error;

+        /* Forward the information about isolation groups */

+        addrs->buses[idx].isolationGroup = cont->info.isolationGroup;

+        addrs->buses[idx].isolationGroupLocked = cont->info.isolationGroupLocked;

+

         if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)

             hasPCIeRoot = true;

     }

@@ -1193,7 +1198,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;

     }

@@ -1228,7 +1233,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;

@@ -1253,7 +1258,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;

         }

     }

@@ -1329,10 +1334,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;

@@ -1354,10 +1357,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;

@@ -1380,12 +1381,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;

     }

@@ -1419,7 +1420,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;

@@ -1445,8 +1446,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;

         }

     }

@@ -1467,7 +1467,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;

@@ -1671,7 +1671,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;

                 }

-- 

2.13.3