Re: [Qemu-devel] [PATCH] spapr_drc: don't allow 'empty' DRCs to be unisolated

[Michael Roth]

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: address@hidden
> Cc: David Gibson <address@hidden>
> Cc: Bharata B Rao <address@hidden>
> Signed-off-by: Michael Roth <address@hidden>

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
>