* [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS
@ 2015-09-10 21:11 Michael Roth
2015-09-10 21:11 ` [Qemu-devel] [PATCH v3 2/2] spapr_drc: don't allow 'empty' DRCs to be unisolated or allocated Michael Roth
2015-09-11 0:39 ` [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS David Gibson
0 siblings, 2 replies; 3+ messages in thread
From: Michael Roth @ 2015-09-10 21:11 UTC (permalink / raw
To: qemu-devel; +Cc: Bharata B Rao, qemu-ppc, Michael Roth, David Gibson
Certain methods in sPAPRDRConnector objects are only ever called by
RTAS and in many cases are responsible for the logic that determines
the RTAS return codes.
Rather than having a level of indirection requiring RTAS code to
re-interpret return values from such methods to determine the
appropriate return code, just pass them through directly.
This requires changing method return types to uint32_t to match the
type of values currently passed to RTAS helpers.
In the case of read accesses like drc->entity_sense() where we weren't
previously reporting any errors, just the read value, we modify the
function to return RTAS return code, and pass the read value back via
reference.
Suggested-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/ppc/spapr_drc.c | 37 +++++++++++++++++++------------------
hw/ppc/spapr_rtas.c | 40 ++++++++++++++++++++++------------------
include/hw/ppc/spapr_drc.h | 14 +++++++-------
3 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 9ce844a..a1b428f 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -15,6 +15,7 @@
#include "hw/qdev.h"
#include "qapi/visitor.h"
#include "qemu/error-report.h"
+#include "hw/ppc/spapr.h" /* for RTAS return codes */
/* #define DEBUG_SPAPR_DRC */
@@ -59,8 +60,8 @@ static uint32_t get_index(sPAPRDRConnector *drc)
(drc->id & DRC_INDEX_ID_MASK);
}
-static int set_isolation_state(sPAPRDRConnector *drc,
- sPAPRDRIsolationState state)
+static uint32_t set_isolation_state(sPAPRDRConnector *drc,
+ sPAPRDRIsolationState state)
{
sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -89,19 +90,19 @@ static int set_isolation_state(sPAPRDRConnector *drc,
drc->configured = false;
}
- return 0;
+ return RTAS_OUT_SUCCESS;
}
-static int set_indicator_state(sPAPRDRConnector *drc,
- sPAPRDRIndicatorState state)
+static uint32_t set_indicator_state(sPAPRDRConnector *drc,
+ sPAPRDRIndicatorState state)
{
DPRINTFN("drc: %x, set_indicator_state: %x", get_index(drc), state);
drc->indicator_state = state;
- return 0;
+ return RTAS_OUT_SUCCESS;
}
-static int set_allocation_state(sPAPRDRConnector *drc,
- sPAPRDRAllocationState state)
+static uint32_t set_allocation_state(sPAPRDRConnector *drc,
+ sPAPRDRAllocationState state)
{
sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -116,7 +117,7 @@ static int set_allocation_state(sPAPRDRConnector *drc,
drc->detach_cb_opaque, NULL);
}
}
- return 0;
+ return RTAS_OUT_SUCCESS;
}
static uint32_t get_type(sPAPRDRConnector *drc)
@@ -157,10 +158,8 @@ static void set_configured(sPAPRDRConnector *drc)
* based on the current allocation/indicator/power states
* for the DR connector.
*/
-static sPAPRDREntitySense entity_sense(sPAPRDRConnector *drc)
+static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
{
- sPAPRDREntitySense state;
-
if (drc->dev) {
if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
@@ -169,7 +168,7 @@ static sPAPRDREntitySense entity_sense(sPAPRDRConnector *drc)
* Otherwise, report the state as USABLE/PRESENT,
* as we would for PCI.
*/
- state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
+ *state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
} else {
/* this assumes all PCI devices are assigned to
* a 'live insertion' power domain, where QEMU
@@ -177,21 +176,21 @@ static sPAPRDREntitySense entity_sense(sPAPRDRConnector *drc)
* to the guest. present, non-PCI resources are
* unaffected by power state.
*/
- state = SPAPR_DR_ENTITY_SENSE_PRESENT;
+ *state = SPAPR_DR_ENTITY_SENSE_PRESENT;
}
} else {
if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
/* PCI devices, and only PCI devices, use EMPTY
* in cases where we'd otherwise use UNUSABLE
*/
- state = SPAPR_DR_ENTITY_SENSE_EMPTY;
+ *state = SPAPR_DR_ENTITY_SENSE_EMPTY;
} else {
- state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
+ *state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
}
}
DPRINTFN("drc: %x, entity_sense: %x", get_index(drc), state);
- return state;
+ return RTAS_OUT_SUCCESS;
}
static void prop_get_index(Object *obj, Visitor *v, void *opaque,
@@ -224,7 +223,9 @@ static void prop_get_entity_sense(Object *obj, Visitor *v, void *opaque,
{
sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
- uint32_t value = (uint32_t)drck->entity_sense(drc);
+ uint32_t value;
+
+ drck->entity_sense(drc, &value);
visit_type_uint32(v, &value, name, errp);
}
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 3b7b20b..ecbf599 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -372,12 +372,13 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
uint32_t sensor_type;
uint32_t sensor_index;
uint32_t sensor_state;
+ uint32_t ret = RTAS_OUT_SUCCESS;
sPAPRDRConnector *drc;
sPAPRDRConnectorClass *drck;
if (nargs != 3 || nret != 1) {
- rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
- return;
+ ret = RTAS_OUT_PARAM_ERROR;
+ goto out;
}
sensor_type = rtas_ld(args, 0);
@@ -393,8 +394,8 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
if (!drc) {
DPRINTF("rtas_set_indicator: invalid sensor/DRC index: %xh\n",
sensor_index);
- rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
- return;
+ ret = RTAS_OUT_PARAM_ERROR;
+ goto out;
}
drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -413,19 +414,20 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
spapr_ccs_remove(spapr, ccs);
}
}
- drck->set_isolation_state(drc, sensor_state);
+ ret = drck->set_isolation_state(drc, sensor_state);
break;
case RTAS_SENSOR_TYPE_DR:
- drck->set_indicator_state(drc, sensor_state);
+ ret = drck->set_indicator_state(drc, sensor_state);
break;
case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
- drck->set_allocation_state(drc, sensor_state);
+ ret = drck->set_allocation_state(drc, sensor_state);
break;
default:
goto out_unimplemented;
}
- rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+out:
+ rtas_st(rets, 0, ret);
return;
out_unimplemented:
@@ -442,13 +444,14 @@ static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr,
{
uint32_t sensor_type;
uint32_t sensor_index;
+ uint32_t sensor_state = 0;
sPAPRDRConnector *drc;
sPAPRDRConnectorClass *drck;
- uint32_t entity_sense;
+ uint32_t ret = RTAS_OUT_SUCCESS;
if (nargs != 2 || nret != 2) {
- rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
- return;
+ ret = RTAS_OUT_PARAM_ERROR;
+ goto out;
}
sensor_type = rtas_ld(args, 0);
@@ -458,22 +461,23 @@ static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr,
/* currently only DR-related sensors are implemented */
DPRINTF("rtas_get_sensor_state: sensor/indicator not implemented: %d\n",
sensor_type);
- rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
- return;
+ ret = RTAS_OUT_NOT_SUPPORTED;
+ goto out;
}
drc = spapr_dr_connector_by_index(sensor_index);
if (!drc) {
DPRINTF("rtas_get_sensor_state: invalid sensor/DRC index: %xh\n",
sensor_index);
- rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
- return;
+ ret = RTAS_OUT_PARAM_ERROR;
+ goto out;
}
drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
- entity_sense = drck->entity_sense(drc);
+ ret = drck->entity_sense(drc, &sensor_state);
- rtas_st(rets, 0, RTAS_OUT_SUCCESS);
- rtas_st(rets, 1, entity_sense);
+out:
+ rtas_st(rets, 0, ret);
+ rtas_st(rets, 1, sensor_state);
}
/* configure-connector work area offsets, int32_t units for field
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 28ffeae..7e56347 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -165,17 +165,17 @@ typedef struct sPAPRDRConnectorClass {
/*< public >*/
/* accessors for guest-visible (generally via RTAS) DR state */
- int (*set_isolation_state)(sPAPRDRConnector *drc,
- sPAPRDRIsolationState state);
- int (*set_indicator_state)(sPAPRDRConnector *drc,
- sPAPRDRIndicatorState state);
- int (*set_allocation_state)(sPAPRDRConnector *drc,
- sPAPRDRAllocationState 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);
uint32_t (*get_index)(sPAPRDRConnector *drc);
uint32_t (*get_type)(sPAPRDRConnector *drc);
const char *(*get_name)(sPAPRDRConnector *drc);
- sPAPRDREntitySense (*entity_sense)(sPAPRDRConnector *drc);
+ uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
/* QEMU interfaces for managing FDT/configure-connector */
const void *(*get_fdt)(sPAPRDRConnector *drc, int *fdt_start_offset);
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] spapr_drc: don't allow 'empty' DRCs to be unisolated or allocated
2015-09-10 21:11 [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS Michael Roth
@ 2015-09-10 21:11 ` Michael Roth
2015-09-11 0:39 ` [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: Michael Roth @ 2015-09-10 21:11 UTC (permalink / raw
To: qemu-devel; +Cc: Bharata B Rao, qemu-ppc, Michael Roth, David Gibson
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: qemu-ppc@nongnu.org
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS
2015-09-10 21:11 [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS Michael Roth
2015-09-10 21:11 ` [Qemu-devel] [PATCH v3 2/2] spapr_drc: don't allow 'empty' DRCs to be unisolated or allocated Michael Roth
@ 2015-09-11 0:39 ` David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2015-09-11 0:39 UTC (permalink / raw
To: Michael Roth; +Cc: qemu-ppc, qemu-devel, Bharata B Rao
[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]
On Thu, Sep 10, 2015 at 04:11:02PM -0500, Michael Roth wrote:
> Certain methods in sPAPRDRConnector objects are only ever called by
> RTAS and in many cases are responsible for the logic that determines
> the RTAS return codes.
>
> Rather than having a level of indirection requiring RTAS code to
> re-interpret return values from such methods to determine the
> appropriate return code, just pass them through directly.
>
> This requires changing method return types to uint32_t to match the
> type of values currently passed to RTAS helpers.
>
> In the case of read accesses like drc->entity_sense() where we weren't
> previously reporting any errors, just the read value, we modify the
> function to return RTAS return code, and pass the read value back via
> reference.
>
> Suggested-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-ppc@nongnu.org
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Series applied to spapr-dev, thanks.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-09-11 0:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-10 21:11 [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS Michael Roth
2015-09-10 21:11 ` [Qemu-devel] [PATCH v3 2/2] spapr_drc: don't allow 'empty' DRCs to be unisolated or allocated Michael Roth
2015-09-11 0:39 ` [Qemu-devel] [PATCH v3 1/2] spapr_drc: use RTAS return codes for methods called by RTAS David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.