From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47902) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c347D-00065Q-Mm for qemu-devel@nongnu.org; Sat, 05 Nov 2016 12:46:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c3478-0002fS-Qe for qemu-devel@nongnu.org; Sat, 05 Nov 2016 12:46:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34070) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c3478-0002e7-JM for qemu-devel@nongnu.org; Sat, 05 Nov 2016 12:46:42 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 D6D7F31B310 for ; Sat, 5 Nov 2016 16:46:40 +0000 (UTC) References: <1478099802-14188-1-git-send-email-marcel@redhat.com> <20161103061711-mutt-send-email-mst@kernel.org> <8400224e-073e-eebc-bbb6-43bae96f3155@redhat.com> <20161103213514-mutt-send-email-mst@kernel.org> From: Marcel Apfelbaum Message-ID: Date: Sat, 5 Nov 2016 18:46:34 +0200 MIME-Version: 1.0 In-Reply-To: <20161103213514-mutt-send-email-mst@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: "Michael S. Tsirkin" , Andrew Jones Cc: qemu-devel@nongnu.org, Gerd Hoffmann , laine@redhat.com, Markus Armbruster On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote: > 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? > Adding Andrew , maybe he can give us an answer. Anybody else can help answering this? >> 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? > I suppose we can do that... not sure if it worth it. Thanks, Marcel > >> 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