[Qemu-devel] [PATCH 6/7] spapr: Clean up handling of DR-indicator

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

There are 3 types of "indicator" associated with hotplug in the PAPR spec
the "allocation state", "isolation state" and "DR-indicator".  The first
two are intimately tied to the various state transitions associated with
hotplug.  The DR-indicator, however, is different and simpler.

It's basically just a guest controlled variable which can be used by the
guest to flag state or problems associated with a device.  The idea is that
the hypervisor can use it to present information back on management
consoles (on some machines with PowerVM it may even control physical LEDs
on the machine case associated with the relevant device).

For that reason, there's only ever likely to be a single update
implementation so the set_indicator_state method isn't useful.  Replace it
with a direct function call.

While we're there, make some small associated cleanups:
  * PAPR doesn't use the term "indicator state", just "DR-indicator" and
the allocation state and isolation state are also considered "indicators".
Rename things to be less confusing
  * Fold set_indicator_state() and rtas_set_indicator_state() into a single
rtas_set_dr_indicator() function.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_drc.c         | 25 ++++++++-----------------
 hw/ppc/trace-events        |  2 +-
 include/hw/ppc/spapr_drc.h | 16 ++++++++--------
 3 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 6c2fa93..a4ece2e 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -116,14 +116,6 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
     return RTAS_OUT_SUCCESS;
 }
 
-static uint32_t set_indicator_state(sPAPRDRConnector *drc,
-                                    sPAPRDRIndicatorState state)
-{
-    trace_spapr_drc_set_indicator_state(spapr_drc_index(drc), state);
-    drc->indicator_state = state;
-    return RTAS_OUT_SUCCESS;
-}
-
 static uint32_t set_allocation_state(sPAPRDRConnector *drc,
                                      sPAPRDRAllocationState state)
 {
@@ -313,7 +305,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
     if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
     }
-    drc->indicator_state = SPAPR_DR_INDICATOR_STATE_ACTIVE;
+    drc->dr_indicator = SPAPR_DR_INDICATOR_ACTIVE;
 
     drc->dev = d;
     drc->fdt = fdt;
@@ -386,7 +378,7 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
         }
     }
 
-    drc->indicator_state = SPAPR_DR_INDICATOR_STATE_INACTIVE;
+    drc->dr_indicator = SPAPR_DR_INDICATOR_INACTIVE;
 
     /* Calling release callbacks based on spapr_drc_type(drc). */
     switch (spapr_drc_type(drc)) {
@@ -499,7 +491,7 @@ static const VMStateDescription vmstate_spapr_drc = {
     .fields  = (VMStateField []) {
         VMSTATE_UINT32(isolation_state, sPAPRDRConnector),
         VMSTATE_UINT32(allocation_state, sPAPRDRConnector),
-        VMSTATE_UINT32(indicator_state, sPAPRDRConnector),
+        VMSTATE_UINT32(dr_indicator, sPAPRDRConnector),
         VMSTATE_BOOL(configured, sPAPRDRConnector),
         VMSTATE_BOOL(awaiting_release, sPAPRDRConnector),
         VMSTATE_BOOL(awaiting_allocation, sPAPRDRConnector),
@@ -614,7 +606,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     dk->realize = realize;
     dk->unrealize = unrealize;
     drck->set_isolation_state = set_isolation_state;
-    drck->set_indicator_state = set_indicator_state;
     drck->set_allocation_state = set_allocation_state;
     drck->attach = attach;
     drck->detach = detach;
@@ -895,17 +886,17 @@ static uint32_t rtas_set_allocation_state(uint32_t idx, uint32_t state)
     return drck->set_allocation_state(drc, state);
 }
 
-static uint32_t rtas_set_indicator_state(uint32_t idx, uint32_t state)
+static uint32_t rtas_set_dr_indicator(uint32_t idx, uint32_t state)
 {
     sPAPRDRConnector *drc = spapr_drc_by_index(idx);
-    sPAPRDRConnectorClass *drck;
 
     if (!drc) {
         return RTAS_OUT_PARAM_ERROR;
     }
 
-    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    return drck->set_indicator_state(drc, state);
+    trace_spapr_drc_set_dr_indicator(idx, state);
+    drc->dr_indicator = state;
+    return RTAS_OUT_SUCCESS;
 }
 
 static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
@@ -930,7 +921,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         ret = rtas_set_isolation_state(idx, state);
         break;
     case RTAS_SENSOR_TYPE_DR:
-        ret = rtas_set_indicator_state(idx, state);
+        ret = rtas_set_dr_indicator(idx, state);
         break;
     case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
         ret = rtas_set_allocation_state(idx, state);
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 581fa85..3e8e3cf 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -39,7 +39,7 @@ spapr_iommu_ddw_reset(uint64_t buid, uint32_t cfgaddr) "buid=%"PRIx64" addr=%"PR
 spapr_drc_set_isolation_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: %"PRIx32
 spapr_drc_set_isolation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32
 spapr_drc_set_isolation_state_deferring(uint32_t index) "drc: 0x%"PRIx32
-spapr_drc_set_indicator_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
+spapr_drc_set_dr_indicator(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
 spapr_drc_set_allocation_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
 spapr_drc_set_allocation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32
 spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index d437e0a..802c708 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -125,7 +125,7 @@ typedef enum {
 } sPAPRDRAllocationState;
 
 /*
- * LED/visual indicator state
+ * DR-indicator (LED/visual indicator)
  *
  * set via set-indicator RTAS calls
  * as documented by PAPR+ 2.7 13.5.3.4, Table 177,
@@ -137,10 +137,10 @@ typedef enum {
  * action: (currently unused)
  */
 typedef enum {
-    SPAPR_DR_INDICATOR_STATE_INACTIVE   = 0,
-    SPAPR_DR_INDICATOR_STATE_ACTIVE     = 1,
-    SPAPR_DR_INDICATOR_STATE_IDENTIFY   = 2,
-    SPAPR_DR_INDICATOR_STATE_ACTION     = 3,
+    SPAPR_DR_INDICATOR_INACTIVE   = 0,
+    SPAPR_DR_INDICATOR_ACTIVE     = 1,
+    SPAPR_DR_INDICATOR_IDENTIFY   = 2,
+    SPAPR_DR_INDICATOR_ACTION     = 3,
 } sPAPRDRIndicatorState;
 
 /*
@@ -186,10 +186,12 @@ typedef struct sPAPRDRConnector {
     Object *owner;
     char *name;
 
+    /* DR-indicator */
+    uint32_t dr_indicator;
+
     /* sensor/indicator states */
     uint32_t isolation_state;
     uint32_t allocation_state;
-    uint32_t indicator_state;
 
     /* configure-connector state */
     void *fdt;
@@ -219,8 +221,6 @@ typedef struct sPAPRDRConnectorClass {
     /* accessors for guest-visible (generally via RTAS) DR state */
     uint32_t (*set_isolation_state)(sPAPRDRConnector *drc,
                                     sPAPRDRIsolationState state);
-    uint32_t (*set_indicator_state)(sPAPRDRConnector *drc,
-                                    sPAPRDRIndicatorState state);
     uint32_t (*set_allocation_state)(sPAPRDRConnector *drc,
                                      sPAPRDRAllocationState state);
 
-- 
2.9.4