Message ID | 1441749839-30540-1-git-send-email-mdroth@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
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 >
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 {
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(+)