[Qemu-devel] [RFC PATCH 11/14] spapr_pci: Allow EEH on spapr-pci-host-bridge

[David Gibson <david@gibson.dropbear.id.au>]

The pseries machine type has two variants of the PCI Host Bridge device:
spapr-pci-host-bridge and spapr-pci-vfio-host-bridge.  Originally, only the
latter could support VFIO devices, but we've now extended the VFIO code so
that they both can.

However, it's still only spapr-pci-vfio-host-bridge which supports the PAPR
Enhanced Error Handling (EEH) interfaces.  The reason is that we don't yet
have a way to determine the correct VFIOGroup for EEH operations on the
"plain" host bridge.

Handling this in general is problematic due to some limitations in the
current code, and some bugs in the kernel EEH interfaces.  However, it's
easy enough to do for the unambiguous case: where there is only one VFIO
group used on the whole host bridge i.e. one Partitionable Endpoint (PE).

This re-implements spapr_phb_check_vfio_group() in terms of the host
bridge's Partitionable Endpoints, allowing EEH on any host bridge in the
unambiguous case.  This is enough to make spapr-pci-host-bridge support
EEH as well as spapr-pci-vfio-host-bridge, since the latter only supported
devices from one IOMMU group anyway (although it didn't properly
enforce that).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_pci.c          | 36 ++++++++++++++++++++++++++++++++++++
 hw/ppc/spapr_pci_vfio.c     | 18 ------------------
 include/hw/pci-host/spapr.h |  1 -
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 81ad3ae..5614b45 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -41,6 +41,8 @@
 #include "hw/ppc/spapr_drc.h"
 #include "sysemu/device_tree.h"
 
+#include "hw/vfio/vfio-common.h"
+
 /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
 #define RTAS_QUERY_FN           0
 #define RTAS_CHANGE_FN          1
@@ -446,6 +448,40 @@ static sPAPRPHBGuestPE *spapr_phb_pe_by_device(sPAPRPHBState *phb,
     return pe;
 }
 
+static int spapr_phb_check_vfio_group(sPAPRPHBState *phb, VFIOGroup **gpp)
+{
+    sPAPRPHBGuestPE *pe;
+
+    if (QLIST_EMPTY(&phb->pe_list)) {
+        /* No EEH capable devices on this PHB */
+        return RTAS_OUT_PARAM_ERROR;
+    }
+
+    /* Limitations in both qemu and the kernel mean that, for now, EEH
+     * won't work if there are devices from more than one PE
+     * (i.e. IOMMU group) on the same PHB */
+    pe = QLIST_FIRST(&phb->pe_list);
+    if (QLIST_NEXT(pe, list)) {
+        error_report("spapr-pci-host-bridge: EEH attempted on PHB with multiple"
+                     " IOMMU groups");
+        return RTAS_OUT_HW_ERROR;
+    }
+
+    if (!object_dynamic_cast(OBJECT(phb), "spapr-pci-vfio-host-bridge")) {
+        sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(phb);
+        /* FIXME: this is an abstraction violation */
+        if (pe->group->groupid != svphb->iommugroupid) {
+            error_report("spapr-pci-vfio-host-bridge: Bad IOMMU group");
+            return RTAS_OUT_HW_ERROR;
+        }
+    }
+
+    if (gpp) {
+        *gpp = pe->group;
+    }
+    return RTAS_OUT_SUCCESS;
+}
+
 static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
                                     sPAPRMachineState *spapr,
                                     uint32_t token, uint32_t nargs,
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index b61923c..a08292a 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -24,29 +24,11 @@
 #include "hw/vfio/vfio.h"
 #include "hw/vfio/vfio-eeh.h"
 
-#include "hw/vfio/vfio-common.h"
-
 static Property spapr_phb_vfio_properties[] = {
     DEFINE_PROP_INT32("iommu", sPAPRPHBVFIOState, iommugroupid, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-int spapr_phb_check_vfio_group(sPAPRPHBState *phb, VFIOGroup **gpp)
-{
-    VFIOGroup *group;
-
-    if (!object_dynamic_cast(OBJECT(phb), "spapr-pci-vfio-host-bridge")) {
-        return RTAS_OUT_PARAM_ERROR;
-    }
-
-    /* FIXME: this is an abstraction violation */
-    group = vfio_get_group(SPAPR_PCI_VFIO_HOST_BRIDGE(phb)->iommugroupid,
-                           &phb->iommu_as);
-    if (gpp)
-        *gpp = group;
-    return RTAS_OUT_SUCCESS;
-}
-
 static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
 {
     sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 535e5ef..8c8d187 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -138,7 +138,6 @@ void spapr_pci_msi_init(sPAPRMachineState *spapr, hwaddr addr);
 
 void spapr_pci_rtas_init(void);
 
-int spapr_phb_check_vfio_group(sPAPRPHBState *phb, VFIOGroup **gpp);
 sPAPRPHBState *spapr_pci_find_phb(sPAPRMachineState *spapr, uint64_t buid);
 PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
                               uint32_t config_addr);
-- 
2.4.3