[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v4 3/6] i386/acpi-build: describe new pci domain i
From: |
Zihan Yang |
Subject: |
Re: [Qemu-devel] [RFC v4 3/6] i386/acpi-build: describe new pci domain in AML |
Date: |
Sun, 19 Aug 2018 10:00:16 +0800 |
Marcel Apfelbaum <address@hidden> 于2018年8月18日周六 上午1:49写道:
>
>
>
> 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?
In the last version, I got a misunderstanding about bus_nr, and you pointed me
out that we should start from bus 0 when in new domain. I can support this
start_bus in later patch, but I wonder when would we want to start a domain
from bus_nr > 0? I'm not quite familiar with the use case.
> > which is
> > equal to BBN.
> >
> > Signed-off-by: Zihan Yang <address@hidden>
> > ---
> > 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?
This is the same problem with above, the pci_bus_num will return bus_nr,
and I assumed the start bus is from 0.
>
> >
> > 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;
> >
>