From patchwork Fri Nov 20 23:42:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kurz X-Patchwork-Id: 11922917 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 819AEC2D0E4 for ; Fri, 20 Nov 2020 23:46:17 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 21FD2223B0 for ; Fri, 20 Nov 2020 23:46:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 21FD2223B0 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kaod.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:43598 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kgG6a-00027W-7C for qemu-devel@archiver.kernel.org; Fri, 20 Nov 2020 18:46:16 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:42726) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kgG2o-0005oh-Jg for qemu-devel@nongnu.org; Fri, 20 Nov 2020 18:42:22 -0500 Received: from us-smtp-delivery-44.mimecast.com ([205.139.111.44]:31624) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1kgG2l-0003lL-LU for qemu-devel@nongnu.org; Fri, 20 Nov 2020 18:42:22 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-386-3w8wJV-EPi2qEVOJ3GTMAA-1; Fri, 20 Nov 2020 18:42:13 -0500 X-MC-Unique: 3w8wJV-EPi2qEVOJ3GTMAA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C83B1107AD38; Fri, 20 Nov 2020 23:42:12 +0000 (UTC) Received: from bahia.redhat.com (ovpn-112-44.ams2.redhat.com [10.36.112.44]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9800260BFA; Fri, 20 Nov 2020 23:42:11 +0000 (UTC) From: Greg Kurz To: qemu-devel@nongnu.org Subject: [PATCH for-6.0 1/9] spapr: Do PCI device hotplug sanity checks at pre-plug only Date: Sat, 21 Nov 2020 00:42:00 +0100 Message-Id: <20201120234208.683521-2-groug@kaod.org> In-Reply-To: <20201120234208.683521-1-groug@kaod.org> References: <20201120234208.683521-1-groug@kaod.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: kaod.org Received-SPF: softfail client-ip=205.139.111.44; envelope-from=groug@kaod.org; helo=us-smtp-delivery-44.mimecast.com X-Spam_score_int: -11 X-Spam_score: -1.2 X-Spam_bar: - X-Spam_report: (-1.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Igor Mammedov , qemu-ppc@nongnu.org, Greg Kurz , David Gibson Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" The PHB acts as the hotplug handler for PCI devices. It does some sanity checks on DR enablement, PCI bridge chassis numbers and multifunction. These checks are currently performed at plug time, but they would best sit in a pre-plug handler in order to error out as early as possible. Create a spapr_pci_pre_plug() handler and move all the checking there. Add a check that the associated DRC doesn't already have an attached device. This is equivalent to the slot availability check performed by do_pci_register_device() upon realization of the PCI device. This allows to pass &error_abort to spapr_drc_attach() and to end up with a plug handler that doesn't need to report errors anymore. Signed-off-by: Greg Kurz --- hw/ppc/spapr_pci.c | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 88ce87f130a5..2829f298d9c1 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1532,8 +1532,8 @@ static bool bridge_has_valid_chassis_nr(Object *bridge, Error **errp) return true; } -static void spapr_pci_plug(HotplugHandler *plug_handler, - DeviceState *plugged_dev, Error **errp) +static void spapr_pci_pre_plug(HotplugHandler *plug_handler, + DeviceState *plugged_dev, Error **errp) { SpaprPhbState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler)); PCIDevice *pdev = PCI_DEVICE(plugged_dev); @@ -1542,9 +1542,6 @@ static void spapr_pci_plug(HotplugHandler *plug_handler, PCIBus *bus = PCI_BUS(qdev_get_parent_bus(DEVICE(pdev))); uint32_t slotnr = PCI_SLOT(pdev->devfn); - /* if DR is disabled we don't need to do anything in the case of - * hotplug or coldplug callbacks - */ if (!phb->dr_enabled) { /* if this is a hotplug operation initiated by the user * we need to let them know it's not enabled @@ -1552,17 +1549,14 @@ static void spapr_pci_plug(HotplugHandler *plug_handler, if (plugged_dev->hotplugged) { error_setg(errp, QERR_BUS_NO_HOTPLUG, object_get_typename(OBJECT(phb))); + return; } - return; } - g_assert(drc); - if (pc->is_bridge) { if (!bridge_has_valid_chassis_nr(OBJECT(plugged_dev), errp)) { return; } - spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev)); } /* Following the QEMU convention used for PCIe multifunction @@ -1574,13 +1568,41 @@ static void spapr_pci_plug(HotplugHandler *plug_handler, error_setg(errp, "PCI: slot %d function 0 already occupied by %s," " additional functions can no longer be exposed to guest.", slotnr, bus->devices[PCI_DEVFN(slotnr, 0)]->name); + } + + if (drc && drc->dev) { + error_setg(errp, "PCI: slot %d already occupied by %s", slotnr, + pci_get_function_0(PCI_DEVICE(drc->dev))->name); return; } +} + +static void spapr_pci_plug(HotplugHandler *plug_handler, + DeviceState *plugged_dev, Error **errp) +{ + SpaprPhbState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler)); + PCIDevice *pdev = PCI_DEVICE(plugged_dev); + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(plugged_dev); + SpaprDrc *drc = drc_from_dev(phb, pdev); + uint32_t slotnr = PCI_SLOT(pdev->devfn); - if (!spapr_drc_attach(drc, DEVICE(pdev), errp)) { + /* + * If DR is disabled we don't need to do anything in the case of + * hotplug or coldplug callbacks. + */ + if (!phb->dr_enabled) { return; } + g_assert(drc); + + if (pc->is_bridge) { + spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev)); + } + + /* spapr_pci_pre_plug() already checked the DRC is attachable */ + spapr_drc_attach(drc, DEVICE(pdev), &error_abort); + /* If this is function 0, signal hotplug for all the device functions. * Otherwise defer sending the hotplug event. */ @@ -2223,6 +2245,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data) /* Supported by TYPE_SPAPR_MACHINE */ dc->user_creatable = true; set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); + hp->pre_plug = spapr_pci_pre_plug; hp->plug = spapr_pci_plug; hp->unplug = spapr_pci_unplug; hp->unplug_request = spapr_pci_unplug_request;