From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54256) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2FqM-0000HI-MJ for qemu-devel@nongnu.org; Thu, 03 Nov 2016 07:06:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2FqH-0003L2-Ld for qemu-devel@nongnu.org; Thu, 03 Nov 2016 07:06:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41588) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2FqH-0003KF-DX for qemu-devel@nongnu.org; Thu, 03 Nov 2016 07:05:57 -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 107EEC04B939 for ; Thu, 3 Nov 2016 11:05:56 +0000 (UTC) References: <1478099802-14188-1-git-send-email-marcel@redhat.com> <20161103061711-mutt-send-email-mst@kernel.org> From: Marcel Apfelbaum Message-ID: <8400224e-073e-eebc-bbb6-43bae96f3155@redhat.com> Date: Thu, 3 Nov 2016 13:05:44 +0200 MIME-Version: 1.0 In-Reply-To: <20161103061711-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" Cc: qemu-devel@nongnu.org, kraxel@redhat.com, laine@redhat.com, Markus Armbruster 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? 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. 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 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