From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2NsT-0000OJ-Le for qemu-devel@nongnu.org; Thu, 03 Nov 2016 15:40:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2NsR-0004Dr-24 for qemu-devel@nongnu.org; Thu, 03 Nov 2016 15:40:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40072) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2NsQ-0004Dj-Qx for qemu-devel@nongnu.org; Thu, 03 Nov 2016 15:40:42 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C078E335F97 for ; Thu, 3 Nov 2016 19:40:41 +0000 (UTC) Date: Thu, 3 Nov 2016 21:40:39 +0200 From: "Michael S. Tsirkin" Message-ID: <20161103213514-mutt-send-email-mst@kernel.org> References: <1478099802-14188-1-git-send-email-marcel@redhat.com> <20161103061711-mutt-send-email-mst@kernel.org> <8400224e-073e-eebc-bbb6-43bae96f3155@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8400224e-073e-eebc-bbb6-43bae96f3155@redhat.com> Subject: Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: qemu-devel@nongnu.org, kraxel@redhat.com, laine@redhat.com, Markus Armbruster On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote: > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote: > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, Marcel Apfelbaum wrote: > > > The shpc component is optional while ACPI hotplug is used > > > for hot-plugging PCI devices into a PCI-PCI bridge. > > > Disabling the shpc by default will make slot 0 usable at boot time > > Hi Michael > > > > > at the cost of breaking all hotplug for all non-acpi users. > > > > Do we have a non-acpi user that is able to use the shpc component as-is today? power and some arm systems I guess? > I remember we need to even tweak QEMU before it can be used, but I might be wrong. > > And we don't touch the current machines < 2.8 . > > > > and not only for hot-plug, without loosing any functionality. > > > Older machines will have shpc enabled for compatibility reasons. > > > > > > Signed-off-by: Marcel Apfelbaum > > > > Is an extra slot such a big deal? You can always add more bridges ... > > > > It is not only about the slot itself, but more about the usage model. > The PCIe Upstream ports/DMI-PCI devices are also pci-bridges, > but for them slot 0 is allowed. The reason is that these devices are not themselves hotpluggable. Isn't there a flag that allows adding a non hotpluggable device? Allowing these would be one solution. > And what about the hotplug? Slot 0 is not usable at boot, but then is > usable again (for ACPI users) making people wondering: > https://bugzilla.redhat.com/show_bug.cgi?id=1175113 Let's just disallow that then for consistency? > My point is - can shpc be used as-is today? Even so, I suspect there are much (much) > less users using SHPC than ACPI based hotplug. If this is the case, why bother the > majority of the users? And for the shpc users, they can keep the prev machines > or change the command line, I think changes like this happens over the time. > > Adding Markus for his opinion on command line changes. > > Thanks, > Marcel > > > --- > > > hw/pci-bridge/pci_bridge_dev.c | 2 +- > > > include/hw/compat.h | 4 ++++ > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > > > index 5dbd933..647ad80 100644 > > > --- a/hw/pci-bridge/pci_bridge_dev.c > > > +++ b/hw/pci-bridge/pci_bridge_dev.c > > > @@ -163,7 +163,7 @@ static Property pci_bridge_dev_properties[] = { > > > DEFINE_PROP_ON_OFF_AUTO(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev, msi, > > > ON_OFF_AUTO_AUTO), > > > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, > > > - PCI_BRIDGE_DEV_F_SHPC_REQ, true), > > > + PCI_BRIDGE_DEV_F_SHPC_REQ, false), > > > DEFINE_PROP_END_OF_LIST(), > > > }; > > > > > > diff --git a/include/hw/compat.h b/include/hw/compat.h > > > index 0f06e11..388b7ec 100644 > > > --- a/include/hw/compat.h > > > +++ b/include/hw/compat.h > > > @@ -18,6 +18,10 @@ > > > .driver = "intel-iommu",\ > > > .property = "x-buggy-eim",\ > > > .value = "true",\ > > > + },{\ > > > + .driver = "pci-bridge",\ > > > + .property = "shpc",\ > > > + .value = "on",\ > > > }, > > > > > > #define HW_COMPAT_2_6 \ > > > -- > > > 2.5.5