From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44790) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fqisp-00008T-8P for qemu-devel@nongnu.org; Fri, 17 Aug 2018 13:50:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fqisk-0004Fp-62 for qemu-devel@nongnu.org; Fri, 17 Aug 2018 13:49:59 -0400 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:40102) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fqisj-0004FQ-UV for qemu-devel@nongnu.org; Fri, 17 Aug 2018 13:49:54 -0400 Received: by mail-wm0-x244.google.com with SMTP id y9-v6so8114811wma.5 for ; Fri, 17 Aug 2018 10:49:53 -0700 (PDT) References: <1533796458-22306-1-git-send-email-whois.zihan.yang@gmail.com> From: Marcel Apfelbaum Message-ID: Date: Fri, 17 Aug 2018 20:49:49 +0300 MIME-Version: 1.0 In-Reply-To: <1533796458-22306-1-git-send-email-whois.zihan.yang@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [Qemu-devel] [RFC v4 3/6] i386/acpi-build: describe new pci domain in AML List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zihan Yang , qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" , Igor Mammedov , Paolo Bonzini , Richard Henderson , Eduardo Habkost On 08/09/2018 09:34 AM, Zihan Yang wrote: > Describe new pci segments of host bridges in AML as new pci devices, > with _SEG and _BBN to let them be in DSDT. > > Besides, bus_nr indicates the bus number of pxb-pcie under pcie.0 bus, > but since we put it into separate domain, it should be zero, Why should it be 0? Isn't possible we start another domain from bus_nr> 0? > which is > equal to BBN. > > Signed-off-by: Zihan Yang > --- > hw/i386/acpi-build.c | 63 ++++++++++++++++++++----------------- > hw/pci-bridge/pci_expander_bridge.c | 8 +++++ > hw/pci/pci.c | 5 +++ > include/hw/pci/pci.h | 1 + > include/hw/pci/pci_bus.h | 1 + > 5 files changed, 50 insertions(+), 28 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index c0fc2b4..a7d9af2 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -828,7 +828,7 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set) > Aml *crs = aml_resource_template(); > CrsRangeSet temp_range_set; > CrsRangeEntry *entry; > - uint8_t max_bus = pci_bus_num(host->bus); > + uint8_t max_bus = 0, host_bus = 0; > uint8_t type; > int devfn; > int i; > @@ -967,10 +967,10 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set) > aml_append(crs, > aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE, > 0, > - pci_bus_num(host->bus), > + host_bus, > max_bus, > 0, > - max_bus - pci_bus_num(host->bus) + 1)); > + max_bus - host_bus + 1)); I don't understand the above, can you please explain? > > return crs; > } > @@ -1832,6 +1832,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > aml_append(dev, build_q35_osc_method()); > aml_append(sb_scope, dev); > + No need for this line Thanks, Marcel > aml_append(dsdt, sb_scope); > > build_hpet_aml(dsdt); > @@ -1875,10 +1876,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > crs_range_set_init(&crs_range_set); > bus = PC_MACHINE(machine)->bus; > + i = 1; // PCI0 is q35 host, pxb starts from 1 > if (bus) { > QLIST_FOREACH(bus, &bus->child, sibling) { > uint8_t bus_num = pci_bus_num(bus); > uint8_t numa_node = pci_bus_numa_node(bus); > + int domain_nr = pci_domain_num(bus); > > /* look only for expander root buses */ > if (!pci_bus_is_root(bus)) { > @@ -1890,10 +1893,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > } > > scope = aml_scope("\\_SB"); > - dev = aml_device("PC%.02X", bus_num); > + dev = aml_device("PCI%d", i++); > aml_append(dev, aml_name_decl("_UID", aml_int(bus_num))); > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > - aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num))); > + aml_append(dev, aml_name_decl("_ADR", aml_int(bus_num))); > + aml_append(dev, aml_name_decl("_SEG", aml_int(domain_nr))); > + aml_append(dev, aml_name_decl("_BBN", aml_int(0))); > if (pci_bus_is_express(bus)) { > aml_append(dev, build_q35_osc_method()); > } > @@ -2126,35 +2131,37 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > { > Object *pci_host; > PCIBus *bus = NULL; > + int index = 0; > > pci_host = acpi_get_i386_pci_host(); > - if (pci_host) { > + while (pci_host) { > bus = PCI_HOST_BRIDGE(pci_host)->bus; > - } > + if (bus) { > + Aml *scope = aml_scope("PCI%d", index); > + /* Scan all PCI buses. Generate tables to support hotplug. */ > + build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en); > > - if (bus) { > - Aml *scope = aml_scope("PCI0"); > - /* Scan all PCI buses. Generate tables to support hotplug. */ > - build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en); > + /* Only add TPM once */ > + if (index++ == 0 && TPM_IS_TIS(tpm_find())) { > + dev = aml_device("ISA.TPM"); > + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31"))); > + aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > + crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE, > + TPM_TIS_ADDR_SIZE, AML_READ_WRITE)); > + /* > + FIXME: TPM_TIS_IRQ=5 conflicts with PNP0C0F irqs, > + Rewrite to take IRQ from TPM device model and > + fix default IRQ value there to use some unused IRQ > + */ > + /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > + } > > - if (TPM_IS_TIS(tpm_find())) { > - dev = aml_device("ISA.TPM"); > - aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31"))); > - aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > - crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE, > - TPM_TIS_ADDR_SIZE, AML_READ_WRITE)); > - /* > - FIXME: TPM_TIS_IRQ=5 conflicts with PNP0C0F irqs, > - Rewrite to take IRQ from TPM device model and > - fix default IRQ value there to use some unused IRQ > - */ > - /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > + aml_append(sb_scope, scope); > } > - > - aml_append(sb_scope, scope); > + pci_host = OBJECT(QTAILQ_NEXT(PCI_HOST_BRIDGE(pci_host), next)); > } > } > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > index f50938f..d28a64c 100644 > --- a/hw/pci-bridge/pci_expander_bridge.c > +++ b/hw/pci-bridge/pci_expander_bridge.c > @@ -87,6 +87,13 @@ static int pxb_bus_num(PCIBus *bus) > return pxb->bus_nr; > } > > +static int pxb_domain_num(PCIBus *bus) > +{ > + PXBDev *pxb = convert_to_pxb(bus->parent_dev); > + > + return pxb->domain_nr; > +} > + > static bool pxb_is_root(PCIBus *bus) > { > return true; /* by definition */ > @@ -104,6 +111,7 @@ static void pxb_bus_class_init(ObjectClass *class, void *data) > PCIBusClass *pbc = PCI_BUS_CLASS(class); > > pbc->bus_num = pxb_bus_num; > + pbc->domain_num = pxb_domain_num; > pbc->is_root = pxb_is_root; > pbc->numa_node = pxb_bus_numa_node; > } > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index ddc27ba..e72887f 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -445,6 +445,11 @@ int pci_bus_num(PCIBus *s) > return PCI_BUS_GET_CLASS(s)->bus_num(s); > } > > +int pci_domain_num(PCIBus *s) > +{ > + return PCI_BUS_GET_CLASS(s)->domain_num(s); > +} > + > int pci_bus_numa_node(PCIBus *bus) > { > return PCI_BUS_GET_CLASS(bus)->numa_node(bus); > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index 990d6fc..de514d3 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -436,6 +436,7 @@ static inline PCIBus *pci_get_bus(const PCIDevice *dev) > return PCI_BUS(qdev_get_parent_bus(DEVICE(dev))); > } > int pci_bus_num(PCIBus *s); > +int pci_domain_num(PCIBus *s); > static inline int pci_dev_bus_num(const PCIDevice *dev) > { > return pci_bus_num(pci_get_bus(dev)); > diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h > index b7da8f5..415da90 100644 > --- a/include/hw/pci/pci_bus.h > +++ b/include/hw/pci/pci_bus.h > @@ -15,6 +15,7 @@ typedef struct PCIBusClass { > > bool (*is_root)(PCIBus *bus); > int (*bus_num)(PCIBus *bus); > + int (*domain_num)(PCIBus *bus); > uint16_t (*numa_node)(PCIBus *bus); > } PCIBusClass; >