From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48766) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YmqA0-0008WN-7D for qemu-devel@nongnu.org; Mon, 27 Apr 2015 17:01:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ymq32-0003xl-E0 for qemu-devel@nongnu.org; Mon, 27 Apr 2015 16:54:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45127) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ymq32-0003xF-65 for qemu-devel@nongnu.org; Mon, 27 Apr 2015 16:54:36 -0400 Date: Mon, 27 Apr 2015 22:54:28 +0200 From: "Michael S. Tsirkin" Message-ID: <20150427225152-mutt-send-email-mst@redhat.com> References: <1426791181-23831-1-git-send-email-marcel@redhat.com> <1426791181-23831-13-git-send-email-marcel@redhat.com> <20150427132205-mutt-send-email-mst@redhat.com> <553E2BC9.3080207@redhat.com> <20150427144732-mutt-send-email-mst@redhat.com> <553E33E6.5070005@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <553E33E6.5070005@redhat.com> Subject: Re: [Qemu-devel] [PATCH V6 for-2.3 12/26] hw/pci: introduce TYPE_PCI_MAIN_HOST_BRIDGE interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: kraxel@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, alex.williamson@redhat.com, kevin@koconnor.net, hare@suse.de, imammedo@redhat.com, amit.shah@redhat.com, pbonzini@redhat.com, leon.alrae@imgtec.com, aurelien@aurel32.net, rth@twiddle.net On Mon, Apr 27, 2015 at 04:04:38PM +0300, Marcel Apfelbaum wrote: > On 04/27/2015 03:48 PM, Michael S. Tsirkin wrote: > >On Mon, Apr 27, 2015 at 03:30:01PM +0300, Marcel Apfelbaum wrote: > >>On 04/27/2015 02:24 PM, Michael S. Tsirkin wrote: > >>>On Thu, Mar 19, 2015 at 08:52:47PM +0200, Marcel Apfelbaum wrote: > >>>>From: Marcel Apfelbaum > >>>> > >>>>This is a marker interface used to differentiate the > >>>>"default" host bridge on a system with multiple host bridges. > >>>>This differentiation is required only for pc machines for now > >>>>by the ACPI subsystem. > >>>> > >>>>Signed-off-by: Marcel Apfelbaum > >>> > >>>Fixing hotplug for pxb almost for sure means we'll need > >>>to drop this later. > >>Not really, I am not sure how it is related. I'll go over the pxb pci devices > >>for this hotplug. I have no intention to drop this later. > >>This interface denotes the main/default/bus 0 host bridge. > > > >It's related since you will want to describe all host bridges in acpi. > When I'll submit the pxb hotplug support it will be more clear, > Basically, I'll go over the pxbs leaving the default host bridge alone. So instead, just go over all roots. > > > >> > >> How about an interface to > >>>iterate over objects by type? > >>>You can then blacklist pxbs for now. > >>Less elegant in my opinion, > > > >Right but it will be needed in the future anyway. > I think we should leave this for when we'll need it, this series > is already long enough without adding a new QOM iteration type. > > Thanks, > Marcel Do you have hotplug working? Maybe wait until it's there, then we can discuss. > > > >>> > >>>>--- > >>>> hw/i386/acpi-build.c | 9 ++++++--- > >>>> hw/pci-host/piix.c | 5 +++++ > >>>> hw/pci-host/q35.c | 4 ++++ > >>>> hw/pci/pci_host.c | 6 ++++++ > >>>> include/hw/pci/pci_host.h | 7 +++++++ > >>>> 5 files changed, 28 insertions(+), 3 deletions(-) > >>>> > >>>>diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >>>>index d0a5c85..86e474a 100644 > >>>>--- a/hw/i386/acpi-build.c > >>>>+++ b/hw/i386/acpi-build.c > >>>>@@ -249,7 +249,8 @@ static void acpi_get_pci_info(PcPciInfo *info) > >>>> Object *pci_host; > >>>> bool ambiguous; > >>>> > >>>>- pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, &ambiguous); > >>>>+ pci_host = object_resolve_path_type("", TYPE_PCI_MAIN_HOST_BRIDGE, > >>>>+ &ambiguous); > >>>> g_assert(!ambiguous); > >>>> g_assert(pci_host); > >>>> > >>>>@@ -993,7 +994,8 @@ build_ssdt(GArray *table_data, GArray *linker, > >>>> PCIBus *bus = NULL; > >>>> bool ambiguous; > >>>> > >>>>- pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, &ambiguous); > >>>>+ pci_host = object_resolve_path_type("", TYPE_PCI_MAIN_HOST_BRIDGE, > >>>>+ &ambiguous); > >>>> if (!ambiguous && pci_host) { > >>>> bus = PCI_HOST_BRIDGE(pci_host)->bus; > >>>> } > >>>>@@ -1338,7 +1340,8 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg) > >>>> QObject *o; > >>>> bool ambiguous; > >>>> > >>>>- pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, &ambiguous); > >>>>+ pci_host = object_resolve_path_type("", TYPE_PCI_MAIN_HOST_BRIDGE, > >>>>+ &ambiguous); > >>>> g_assert(!ambiguous); > >>>> g_assert(pci_host); > >>>> > >>>>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > >>>>index 292b6e9..0033ab4 100644 > >>>>--- a/hw/pci-host/piix.c > >>>>+++ b/hw/pci-host/piix.c > >>>>@@ -766,6 +766,11 @@ static const TypeInfo i440fx_pcihost_info = { > >>>> .instance_size = sizeof(I440FXState), > >>>> .instance_init = i440fx_pcihost_initfn, > >>>> .class_init = i440fx_pcihost_class_init, > >>>>+ .interfaces = (InterfaceInfo[]) { > >>>>+ { TYPE_PCI_MAIN_HOST_BRIDGE }, > >>>>+ { } > >>>>+ } > >>>>+ > >>>> }; > >>>> > >>>> static void i440fx_register_types(void) > >>>>diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > >>>>index 5dd559e..e35b3a2 100644 > >>>>--- a/hw/pci-host/q35.c > >>>>+++ b/hw/pci-host/q35.c > >>>>@@ -193,6 +193,10 @@ static const TypeInfo q35_host_info = { > >>>> .instance_size = sizeof(Q35PCIHost), > >>>> .instance_init = q35_host_initfn, > >>>> .class_init = q35_host_class_init, > >>>>+ .interfaces = (InterfaceInfo[]) { > >>>>+ { TYPE_PCI_MAIN_HOST_BRIDGE }, > >>>>+ { } > >>>>+ } > >>>> }; > >>>> > >>>> /**************************************************************************** > >>>>diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > >>>>index 3e26f92..87180c8 100644 > >>>>--- a/hw/pci/pci_host.c > >>>>+++ b/hw/pci/pci_host.c > >>>>@@ -175,6 +175,11 @@ const MemoryRegionOps pci_host_data_be_ops = { > >>>> .endianness = DEVICE_BIG_ENDIAN, > >>>> }; > >>>> > >>>>+static const TypeInfo pci_main_host_interface_info = { > >>>>+ .name = TYPE_PCI_MAIN_HOST_BRIDGE, > >>>>+ .parent = TYPE_INTERFACE, > >>>>+}; > >>>>+ > >>>> static const TypeInfo pci_host_type_info = { > >>>> .name = TYPE_PCI_HOST_BRIDGE, > >>>> .parent = TYPE_SYS_BUS_DEVICE, > >>>>@@ -185,6 +190,7 @@ static const TypeInfo pci_host_type_info = { > >>>> > >>>> static void pci_host_register_types(void) > >>>> { > >>>>+ type_register_static(&pci_main_host_interface_info); > >>>> type_register_static(&pci_host_type_info); > >>>> } > >>>> > >>>>diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > >>>>index ba31595..3c72e26 100644 > >>>>--- a/include/hw/pci/pci_host.h > >>>>+++ b/include/hw/pci/pci_host.h > >>>>@@ -30,6 +30,13 @@ > >>>> > >>>> #include "hw/sysbus.h" > >>>> > >>>>+/** > >>>>+ * Marker interface for classes whose instances can > >>>>+ * be main host bridges. It is intended to be used > >>>>+ * when the QOM tree includes multiple host bridges. > >>>>+ */ > >>>>+#define TYPE_PCI_MAIN_HOST_BRIDGE "pci-main-host-bridge" > >>>>+ > >>>> #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge" > >>>> #define PCI_HOST_BRIDGE(obj) \ > >>>> OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST_BRIDGE) > >>>>-- > >>>>2.1.0