[Qemu-devel] [PATCH 1/6] spapr: Start hotplugged PCI devices in ISOLATED state

David Gibson posted 6 patches 8 years, 5 months ago
[Qemu-devel] [PATCH 1/6] spapr: Start hotplugged PCI devices in ISOLATED state
Posted by David Gibson 8 years, 5 months ago
PCI DRCs, and only PCI DRCs, are immediately moved to UNISOLATED isolation
state once the device is attached.  This has been there from the initial
implementation, and it's not clear why.

The state diagram in PAPR 13.4 suggests PCI devices should start in
ISOLATED state until the guest moves them into UNISOLATED, and the code in
the guest-side drmgr tool seems to work that way too.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr_drc.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 15ef67d..6186f79 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -315,16 +315,6 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
     }
     g_assert(fdt || coldplug);
 
-    /* NOTE: setting initial isolation state to UNISOLATED means we can't
-     * detach unless guest has a userspace/kernel that moves this state
-     * back to ISOLATED in response to an unplug event, or this is done
-     * manually by the admin prior. if we force things while the guest
-     * may be accessing the device, we can easily crash the guest, so we
-     * we defer completion of removal in such cases to the reset() hook.
-     */
-    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
-        drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
-    }
     drc->dr_indicator = SPAPR_DR_INDICATOR_ACTIVE;
 
     drc->dev = d;
-- 
2.9.4


Re: [Qemu-devel] [PATCH 1/6] spapr: Start hotplugged PCI devices in ISOLATED state
Posted by Greg Kurz 8 years, 4 months ago
On Thu,  8 Jun 2017 15:09:25 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> PCI DRCs, and only PCI DRCs, are immediately moved to UNISOLATED isolation
> state once the device is attached.  This has been there from the initial
> implementation, and it's not clear why.
> 
> The state diagram in PAPR 13.4 suggests PCI devices should start in
> ISOLATED state until the guest moves them into UNISOLATED, and the code in
> the guest-side drmgr tool seems to work that way too.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_drc.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 15ef67d..6186f79 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -315,16 +315,6 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
>      }
>      g_assert(fdt || coldplug);
>  
> -    /* NOTE: setting initial isolation state to UNISOLATED means we can't
> -     * detach unless guest has a userspace/kernel that moves this state
> -     * back to ISOLATED in response to an unplug event, or this is done
> -     * manually by the admin prior. if we force things while the guest
> -     * may be accessing the device, we can easily crash the guest, so we
> -     * we defer completion of removal in such cases to the reset() hook.
> -     */
> -    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> -        drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
> -    }
>      drc->dr_indicator = SPAPR_DR_INDICATOR_ACTIVE;
>  
>      drc->dev = d;