From patchwork Tue Apr 5 02:17:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gibson X-Patchwork-Id: 8746441 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id BDC65C0553 for ; Tue, 5 Apr 2016 02:16:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CB1A92024F for ; Tue, 5 Apr 2016 02:16:54 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AE1FC20204 for ; Tue, 5 Apr 2016 02:16:53 +0000 (UTC) Received: from localhost ([::1]:33910 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anGY1-0008MQ-2T for patchwork-qemu-devel@patchwork.kernel.org; Mon, 04 Apr 2016 22:16:53 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43302) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anGXi-0007y7-5E for qemu-devel@nongnu.org; Mon, 04 Apr 2016 22:16:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anGXg-0002J1-Ks for qemu-devel@nongnu.org; Mon, 04 Apr 2016 22:16:34 -0400 Received: from ozlabs.org ([2401:3900:2:1::2]:46675) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anGXg-0002AV-1c; Mon, 04 Apr 2016 22:16:32 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3qfCBw4xJpz9sD9; Tue, 5 Apr 2016 12:16:08 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1459822568; bh=poNpwMRurOKiwnw2qAsr06Ze5glGkNaynHwN6gaULzQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=m06A/VXQ3eh8kCuYvvNzogTDZi9cxXWaNwDhJBTDwUYplTbTDNMogaaz4lIqgnEkb k2RZZLdIjcUlQ40ydT/E2nLaUsUPAYU1t3ybsN7GoegKNZShxqWRJ+CAIM7fmJxHOC VvdJmRXfuJ4OvJXgtZVOHHwcCv0tS9oY4+SRfriM= From: David Gibson To: peter.maydell@linaro.org Date: Tue, 5 Apr 2016 12:17:22 +1000 Message-Id: <1459822643-4770-3-git-send-email-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1459822643-4770-1-git-send-email-david@gibson.dropbear.id.au> References: <1459822643-4770-1-git-send-email-david@gibson.dropbear.id.au> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2401:3900:2:1::2 Cc: qemu-devel@nongnu.org, aik@ozlabs.ru, sbhat@linux.vnet.ibm.com, agraf@suse.de, mdroth@linux.vnet.ibm.com, clg@fr.ibm.com, qemu-ppc@nongnu.org, bharata@linux.vnet.ibm.com, pbonzini@redhat.com, david@gibson.dropbear.id.au Subject: [Qemu-devel] [PULL 2/3] spapr_drc: enable immediate detach for unsignalled devices X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Michael Roth Currently spapr doesn't support "aborting" hotplug of PCI devices by allowing device_del to immediately remove the device if we haven't signalled the presence of the device to the guest. In the past this wasn't an issue, since we always immediately signalled device attach and simply relied on full guest-aware add->remove path for device removal. However, as of 788d259, we now defer signalling for PCI functions until function 0 is attached, so now we need to deal with these "abort" operations for cases where a user hotplugs a non-0 function, then opts to remove it prior hotplugging function 0. Currently they'd have to reboot before the unplug completed. PCIe multifunction hotplug does not have this requirement however, so from a management implementation perspective it would be good to address this within the same release as 788d259. We accomplish this by simply adding a 'signalled' flag to track whether a device hotplug event has been sent to the guest. If it hasn't, we allow immediate removal under the assumption that the guest will not be using the device. Devices present at boot/reset time are also assumed to be 'signalled'. For CPU/memory/etc, signalling will still happen immediately as part of device_add, so only PCI functions should be affected. Cc: bharata@linux.vnet.ibm.com Cc: david@gibson.dropbear.id.au Cc: sbhat@linux.vnet.ibm.com Cc: qemu-ppc@nongnu.org Signed-off-by: Michael Roth [dwg: This fixes a regression where an incorrect hot-add of a non-zero function can no longer be backed out until function 0 is added] Signed-off-by: David Gibson --- hw/ppc/spapr_drc.c | 34 ++++++++++++++++++++++++++++++++++ hw/ppc/spapr_events.c | 11 +++++++++++ include/hw/ppc/spapr_drc.h | 2 ++ 3 files changed, 47 insertions(+) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index e6eedf8..3173940 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -176,6 +176,12 @@ static void set_configured(sPAPRDRConnector *drc) drc->configured = true; } +/* has the guest been notified of device attachment? */ +static void set_signalled(sPAPRDRConnector *drc) +{ + drc->signalled = true; +} + /* * dr-entity-sense sensor value * returned via get-sensor-state RTAS calls @@ -358,6 +364,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, drc->fdt = fdt; drc->fdt_start_offset = fdt_start_offset; drc->configured = coldplug; + drc->signalled = coldplug; object_property_add_link(OBJECT(drc), "device", object_get_typename(OBJECT(drc->dev)), @@ -374,6 +381,26 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, drc->detach_cb = detach_cb; drc->detach_cb_opaque = detach_cb_opaque; + /* if we've signalled device presence to the guest, or if the guest + * has gone ahead and configured the device (via manually-executed + * device add via drmgr in guest, namely), we need to wait + * for the guest to quiesce the device before completing detach. + * Otherwise, we can assume the guest hasn't seen it and complete the + * detach immediately. Note that there is a small race window + * just before, or during, configuration, which is this context + * refers mainly to fetching the device tree via RTAS. + * During this window the device access will be arbitrated by + * associated DRC, which will simply fail the RTAS calls as invalid. + * This is recoverable within guest and current implementations of + * drmgr should be able to cope. + */ + if (!drc->signalled && !drc->configured) { + /* if the guest hasn't seen the device we can't rely on it to + * set it back to an isolated state via RTAS, so do it here manually + */ + drc->isolation_state = SPAPR_DR_ISOLATION_STATE_ISOLATED; + } + if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) { DPRINTFN("awaiting transition to isolated state before removal"); drc->awaiting_release = true; @@ -412,6 +439,7 @@ static void reset(DeviceState *d) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d); sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); + sPAPRDREntitySense state; DPRINTFN("drc reset: %x", drck->get_index(drc)); /* immediately upon reset we can safely assume DRCs whose devices @@ -439,6 +467,11 @@ static void reset(DeviceState *d) drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_UNUSABLE); } } + + drck->entity_sense(drc, &state); + if (state == SPAPR_DR_ENTITY_SENSE_PRESENT) { + drck->set_signalled(drc); + } } static void realize(DeviceState *d, Error **errp) @@ -597,6 +630,7 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) drck->attach = attach; drck->detach = detach; drck->release_pending = release_pending; + drck->set_signalled = set_signalled; /* * Reason: it crashes FIXME find and document the real reason */ diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index 1abec27..269ab7e 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -389,6 +389,13 @@ static void spapr_powerdown_req(Notifier *n, void *opaque) qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq)); } +static void spapr_hotplug_set_signalled(uint32_t drc_index) +{ + sPAPRDRConnector *drc = spapr_dr_connector_by_index(drc_index); + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); + drck->set_signalled(drc); +} + static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action, sPAPRDRConnectorType drc_type, uint32_t drc) @@ -455,6 +462,10 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action, rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true); + if (hp->hotplug_action == RTAS_LOG_V6_HP_ACTION_ADD) { + spapr_hotplug_set_signalled(drc); + } + qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq)); } diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 7e56347..fa21ba0 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -151,6 +151,7 @@ typedef struct sPAPRDRConnector { bool configured; bool awaiting_release; + bool signalled; /* device pointer, via link property */ DeviceState *dev; @@ -188,6 +189,7 @@ typedef struct sPAPRDRConnectorClass { spapr_drc_detach_cb *detach_cb, void *detach_cb_opaque, Error **errp); bool (*release_pending)(sPAPRDRConnector *drc); + void (*set_signalled)(sPAPRDRConnector *drc); } sPAPRDRConnectorClass; sPAPRDRConnector *spapr_dr_connector_new(Object *owner,