From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60967) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVMsD-00064h-Hn for qemu-devel@nongnu.org; Tue, 10 Mar 2015 12:19:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVMsC-0002r8-Bt for qemu-devel@nongnu.org; Tue, 10 Mar 2015 12:19:13 -0400 Message-ID: <54FF195F.2040009@redhat.com> Date: Tue, 10 Mar 2015 18:18:39 +0200 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1426001534-7151-1-git-send-email-marcel@redhat.com> <1426001534-7151-27-git-send-email-marcel@redhat.com> <20150310163855-mutt-send-email-mst@redhat.com> In-Reply-To: <20150310163855-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 for-2.3 26/28] acpi: restrict the aml emission to PXB host bridges List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: kraxel@redhat.com, quintela@redhat.com, seabios@seabios.org, qemu-devel@nongnu.org, agraf@suse.de, alex.williamson@redhat.com, kevin@koconnor.net, qemu-ppc@nongnu.org, hare@suse.de, imammedo@redhat.com, amit.shah@redhat.com, pbonzini@redhat.com, leon.alrae@imgtec.com, aurelien@aurel32.net, rth@twiddle.net On 03/10/2015 05:41 PM, Michael S. Tsirkin wrote: > On Tue, Mar 10, 2015 at 05:32:12PM +0200, Marcel Apfelbaum wrote: >> Initial implementation assumed that the aml used for >> any extra root buses would be generic, however this >> is not always true. Restrict aml emission only to i440fx and >> PXB because is the only supported combination for now. >> >> Signed-off-by: Marcel Apfelbaum > > I was wondering about this too. > Please split this up and squash into appropriate patches. > I know it's more work but it's worth it. Sure, Thanks, Marcel > > >> --- >> hw/i386/acpi-build.c | 56 ++++++++++++++++++++----------------- >> hw/pci-bridge/pci_expander_bridge.c | 1 - >> include/hw/pci/pci_host.h | 2 ++ >> 3 files changed, 32 insertions(+), 27 deletions(-) >> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index ff18a07..2bc8a80 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -887,7 +887,7 @@ build_ssdt(GArray *table_data, GArray *linker, >> /* Reserve space for header */ >> acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader)); >> >> - { >> + if (find_i440fx()) { >> PciInfoList *info_list, *info; >> Error *err = NULL; >> >> @@ -901,37 +901,41 @@ build_ssdt(GArray *table_data, GArray *linker, >> PciInfo *bus_info = info->value; >> PCIHostState *host; >> >> - if (bus_info->bus == 0) { >> - continue; >> - } >> + HOST_BRIDGE_FOREACH(host) { >> + int numa_node; >> >> - if (bus_info->bus < root_bus_limit) { >> - root_bus_limit = bus_info->bus - 1; >> - } >> + if (!(pci_bus_num(host->bus) == bus_info->bus)) { >> + continue; >> + } >> >> - scope = aml_scope("\\_SB"); >> - dev = aml_device("PC%.02X", (uint8_t)bus_info->bus); >> - aml_append(dev, aml_name_decl("_UID", >> - aml_string("PC%.02X", (uint8_t)bus_info->bus))); >> - aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A03"))); >> - aml_append(dev, >> - aml_name_decl("_BBN", aml_int((uint8_t)bus_info->bus))); >> + if (!object_dynamic_cast(OBJECT(host), TYPE_PXB_HOST)) { >> + break; >> + } > > Or you can check the device/vendor id if that's easier. > >> >> - HOST_BRIDGE_FOREACH(host) { >> - if (pci_bus_num(host->bus) == bus_info->bus) { >> - int numa_node = pci_bus_numa_node(host->bus); >> - if (numa_node != NUMA_NODE_UNASSIGNED) { >> - aml_append(dev, >> + if (bus_info->bus < root_bus_limit) { >> + root_bus_limit = bus_info->bus - 1; >> + } >> + >> + scope = aml_scope("\\_SB"); >> + dev = aml_device("PC%.02X", (uint8_t)bus_info->bus); >> + aml_append(dev, aml_name_decl("_UID", >> + aml_string("PC%.02X", (uint8_t)bus_info->bus))); >> + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A03"))); >> + aml_append(dev, >> + aml_name_decl("_BBN", aml_int((uint8_t)bus_info->bus))); >> + >> + numa_node = pci_bus_numa_node(host->bus); >> + if (numa_node != NUMA_NODE_UNASSIGNED) { >> + aml_append(dev, >> aml_name_decl("_PXM", aml_int(numa_node))); >> - } >> } >> - } >> >> - aml_append(dev, build_prt()); >> - crs = build_crs(pci, bus_info, &io_ranges, &mem_ranges); >> - aml_append(dev, aml_name_decl("_CRS", crs)); >> - aml_append(scope, dev); >> - aml_append(ssdt, scope); >> + aml_append(dev, build_prt()); >> + crs = build_crs(pci, bus_info, &io_ranges, &mem_ranges); >> + aml_append(dev, aml_name_decl("_CRS", crs)); >> + aml_append(scope, dev); >> + aml_append(ssdt, scope); >> + } >> } >> >> qapi_free_PciInfoList(info_list); >> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c >> index 9329aab..8e6740e 100644 >> --- a/hw/pci-bridge/pci_expander_bridge.c >> +++ b/hw/pci-bridge/pci_expander_bridge.c >> @@ -41,7 +41,6 @@ typedef struct PXBDev { >> uint16_t numa_node; >> } PXBDev; >> >> -#define TYPE_PXB_HOST "pxb-host" >> >> static int pxb_bus_num(PCIBus *bus) >> { >> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h >> index ba5272f..9a35389 100644 >> --- a/include/hw/pci/pci_host.h >> +++ b/include/hw/pci/pci_host.h >> @@ -30,6 +30,8 @@ >> >> #include "hw/sysbus.h" >> >> +#define TYPE_PXB_HOST "pxb-host" >> + > > That's a wrong place for it I think. > >> /** >> * Marker interface for classes whose instances can >> * be main host bridges. It is intended to be used >> -- >> 2.1.0