* [Qemu-devel] [PATCH] spapr_drc: don't allow 'empty' DRCs to be unisolated
@ 2015-09-08 22:03 Michael Roth
2015-09-08 23:46 ` Michael Roth
0 siblings, 1 reply; 2+ messages in thread
From: Michael Roth @ 2015-09-08 22:03 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 allocate-state:USABLE, and then to isolate-state:UNISOLATED.
The former transition does not seem to have any failure path for
cases where a DRC does not have any resources associated with it to
allocate for guest, but instead relies on the subsequent
isolation-state:UNISOLATED transition to indicate failure in this
situation.
Currently DRC code does not implement this logic, but instead
tries to indicate failure by refusing the allocation-state:USABLE
transition. Unfortunately, since that's not a documented failure
path, guests continue undeterred, causing undefined behavior in
QEMU and guest code.
Fix this by handling things as PAPR defines (13.7 and 13.7.3.1).
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 | 12 ++++++++++++
include/hw/ppc/spapr_drc.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 9ce844a..c1f664f 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -66,6 +66,18 @@ static int 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. this generally
+ * happens for logical resources where transitions from
+ * allocation-state:UNUSABLE to allocation-state:USABLE are
+ * unguarded, but instead rely on a subsequent
+ * isolation-state:UNISOLATED transition to indicate failure
+ */
+ if (!drc->dev) {
+ return -1;
+ }
+ }
+
drc->isolation_state = state;
if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 28ffeae..3fbe9ea 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -109,6 +109,7 @@ typedef enum {
* unusable: device not currently available to OS
* exchange: (currently unused)
* recover: (currently unused)
+ * no_sensor: for logical DR only, returned when no resource available
*/
typedef enum {
SPAPR_DR_ENTITY_SENSE_EMPTY = 0,
@@ -116,6 +117,7 @@ typedef enum {
SPAPR_DR_ENTITY_SENSE_UNUSABLE = 2,
SPAPR_DR_ENTITY_SENSE_EXCHANGE = 3,
SPAPR_DR_ENTITY_SENSE_RECOVER = 4,
+ SPAPR_DR_ENTITY_SENSE_NO_SENSOR = -3,
} sPAPRDREntitySense;
typedef enum {
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr_drc: don't allow 'empty' DRCs to be unisolated
2015-09-08 22:03 [Qemu-devel] [PATCH] spapr_drc: don't allow 'empty' DRCs to be unisolated Michael Roth
@ 2015-09-08 23:46 ` Michael Roth
0 siblings, 0 replies; 2+ messages in thread
From: Michael Roth @ 2015-09-08 23:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Bharata B Rao, qemu-ppc, David Gibson
Quoting Michael Roth (2015-09-08 17:03:59)
> Logical resources start with allocation-state:UNUSABLE /
> isolation-state:ISOLATED. During hotplug, guests will transition
> them to allocate-state:USABLE, and then to isolate-state:UNISOLATED.
> The former transition does not seem to have any failure path for
> cases where a DRC does not have any resources associated with it to
> allocate for guest, but instead relies on the subsequent
> isolation-state:UNISOLATED transition to indicate failure in this
> situation.
>
> Currently DRC code does not implement this logic, but instead
> tries to indicate failure by refusing the allocation-state:USABLE
> transition. Unfortunately, since that's not a documented failure
> path, guests continue undeterred, causing undefined behavior in
> QEMU and guest code.
>
> Fix this by handling things as PAPR defines (13.7 and 13.7.3.1).
>
> 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>
Argh, please ignore. This patch is missing the full changeset. v2 sent.
> ---
> hw/ppc/spapr_drc.c | 12 ++++++++++++
> include/hw/ppc/spapr_drc.h | 2 ++
> 2 files changed, 14 insertions(+)
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 9ce844a..c1f664f 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -66,6 +66,18 @@ static int 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. this generally
> + * happens for logical resources where transitions from
> + * allocation-state:UNUSABLE to allocation-state:USABLE are
> + * unguarded, but instead rely on a subsequent
> + * isolation-state:UNISOLATED transition to indicate failure
> + */
> + if (!drc->dev) {
> + return -1;
> + }
> + }
> +
> drc->isolation_state = state;
>
> if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 28ffeae..3fbe9ea 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -109,6 +109,7 @@ typedef enum {
> * unusable: device not currently available to OS
> * exchange: (currently unused)
> * recover: (currently unused)
> + * no_sensor: for logical DR only, returned when no resource available
> */
> typedef enum {
> SPAPR_DR_ENTITY_SENSE_EMPTY = 0,
> @@ -116,6 +117,7 @@ typedef enum {
> SPAPR_DR_ENTITY_SENSE_UNUSABLE = 2,
> SPAPR_DR_ENTITY_SENSE_EXCHANGE = 3,
> SPAPR_DR_ENTITY_SENSE_RECOVER = 4,
> + SPAPR_DR_ENTITY_SENSE_NO_SENSOR = -3,
> } sPAPRDREntitySense;
>
> typedef enum {
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-09-08 23:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-08 22:03 [Qemu-devel] [PATCH] spapr_drc: don't allow 'empty' DRCs to be unisolated Michael Roth
2015-09-08 23:46 ` Michael Roth
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.