[Qemu-devel] [PATCH v3 2/2] spapr_drc: don't allow 'empty' DRCs to be unisolated or allocated

[Michael Roth]

Logical resources start with allocation-state:UNUSABLE /
isolation-state:ISOLATED. During hotplug, guests will transition
them to allocation-state:USABLE, and then to
isolation-state:UNISOLATED.

For cases where we cannot transition to allocation-state:USABLE,
in this case due to no device/resource being association with
the logical DRC, we should return an error -3.

For physical DRCs, we default to allocation-state:USABLE and stay
there, so in this case we should report an error -3 when the guest
attempts to make the isolation-state:ISOLATED transition for a DRC
with no device associated.

These are as documented in PAPR 2.7, 13.5.3.4.

We also ensure allocation-state:USABLE when the guest attempts
transition to isolation-state:UNISOLATED to deal with misbehaving
guests attempting to bring online an unallocated logical resource.

This is as documented in PAPR 2.7, 13.7.

Currently we implement no such error logic. Fix this by handling
these error cases as PAPR defines.

Cc: address@hidden
Cc: David Gibson <address@hidden>
Cc: Bharata B Rao <address@hidden>
Signed-off-by: Michael Roth <address@hidden>
Signed-off-by: David Gibson <address@hidden>
---
v3:
 - added prior patch to report RTAS errors directly from DRC methods (David)
 - add error handling for invalid allocation state transitions (David)
 - error path will likely change for logical DR involving empty DRC, so
   dropped Tested-by.
v2:
 - actually include the full changeset in the patch
---
 hw/ppc/spapr_drc.c     | 21 +++++++++++++++++++++
 include/hw/ppc/spapr.h |  1 +
 2 files changed, 22 insertions(+)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index a1b428f..b7b9891 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -67,6 +67,16 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
 
     DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state);
 
+    if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
+        /* cannot unisolate a non-existant resource, and, or resources
+         * which are in an 'UNUSABLE' allocation state. (PAPR 2.7, 13.5.3.5)
+         */
+        if (!drc->dev ||
+            drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
+            return RTAS_OUT_NO_SUCH_INDICATOR;
+        }
+    }
+
     drc->isolation_state = state;
 
     if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
@@ -108,6 +118,17 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
 
     DPRINTFN("drc: %x, set_allocation_state: %x", get_index(drc), state);
 
+    if (state == SPAPR_DR_ALLOCATION_STATE_USABLE) {
+        /* if there's no resource/device associated with the DRC, there's
+         * no way for us to put it in an allocation state consistent with
+         * being 'USABLE'. PAPR 2.7, 13.5.3.4 documents that this should
+         * result in an RTAS return code of -3 / "no such indicator"
+         */
+        if (!drc->dev) {
+            return RTAS_OUT_NO_SUCH_INDICATOR;
+        }
+    }
+
     if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->allocation_state = state;
         if (drc->awaiting_release &&
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index c75cc5e..ffb108d 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -412,6 +412,7 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_OUT_BUSY               -2
 #define RTAS_OUT_PARAM_ERROR        -3
 #define RTAS_OUT_NOT_SUPPORTED      -3
+#define RTAS_OUT_NO_SUCH_INDICATOR  -3
 #define RTAS_OUT_NOT_AUTHORIZED     -9002
 
 /* RTAS tokens */
-- 
1.9.1