diff mbox series

[v4,2/3] acpi:pci-expender-bus: Add pxb support for arm

Message ID 20200225015026.940-3-miaoyubo@huawei.com (mailing list archive)
State New, archived
Headers show
Series pci_expander_brdige:acpi:Support pxb-pcie for ARM | expand

Commit Message

Yubo Miao Feb. 25, 2020, 1:50 a.m. UTC
From: miaoyubo <miaoyubo@huawei.com>

Currently virt machine is not supported by pxb-pcie,
and only one main host bridge described in ACPI tables.
In this patch,PXB-PCIE is supproted by arm and certain
resource is allocated for each pxb-pcie in acpi table.
The resource for the main host bridge is also reallocated.

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 hw/arm/virt-acpi-build.c | 115 ++++++++++++++++++++++++++++++++++++---
 hw/arm/virt.c            |   3 +
 include/hw/arm/virt.h    |   7 +++
 3 files changed, 118 insertions(+), 7 deletions(-)

Comments

Philippe Mathieu-Daudé Feb. 25, 2020, 9:47 a.m. UTC | #1
On 2/25/20 2:50 AM, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
> 
> Currently virt machine is not supported by pxb-pcie,
> and only one main host bridge described in ACPI tables.
> In this patch,PXB-PCIE is supproted by arm and certain

Typos: "expander" in subject and "supported" here.

> resource is allocated for each pxb-pcie in acpi table.
> The resource for the main host bridge is also reallocated.
> 
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> ---
>   hw/arm/virt-acpi-build.c | 115 ++++++++++++++++++++++++++++++++++++---
>   hw/arm/virt.c            |   3 +
>   include/hw/arm/virt.h    |   7 +++
>   3 files changed, 118 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 37c34748a6..be1986c60d 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -49,6 +49,8 @@
>   #include "kvm_arm.h"
>   #include "migration/vmstate.h"
>   
> +#include "hw/arm/virt.h"
> +#include "hw/pci/pci_bus.h"
>   #define ARM_SPI_BASE 32
>   
>   static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> @@ -266,19 +268,116 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, Aml *scope)
>   }
>   
>   static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> -                              uint32_t irq, bool use_highmem, bool highmem_ecam)
> +                              uint32_t irq, bool use_highmem, bool highmem_ecam,
> +                              VirtMachineState *vms)
>   {
>       int ecam_id = VIRT_ECAM_ID(highmem_ecam);
> -    Aml *method, *crs;
> +    Aml *method, *crs, *dev;
> +    int count = 0;
>       hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base;
>       hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size;
>       hwaddr base_pio = memmap[VIRT_PCIE_PIO].base;
>       hwaddr size_pio = memmap[VIRT_PCIE_PIO].size;
>       hwaddr base_ecam = memmap[ecam_id].base;
>       hwaddr size_ecam = memmap[ecam_id].size;
> +    /*
> +     * 0x600000 would be enough for pxb device
> +     * if it is too small, there is no enough space
> +     * for a pcie device plugged in a pcie-root port
> +     */
> +    hwaddr size_addr = 0x600000;
> +    hwaddr size_io = 0x4000;
>       int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> +    PCIBus *bus = VIRT_MACHINE(vms)->bus;
> +
> +    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);
> +
> +            if (!pci_bus_is_root(bus)) {
> +                continue;
> +            }
> +            /*
> +             * Coded up the MIN of the busNr defined for pxb-pcie,
> +             * the MIN - 1 would be the MAX bus number for the main
> +             * host bridge.
> +             */
> +            if (bus_num < nr_pcie_buses) {
> +                nr_pcie_buses = bus_num;
> +            }
> +            count++;
> +            dev = aml_device("PC%.02X", bus_num);
> +            aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> +            aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> +            aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> +            aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +            aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> +            aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
> +            aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> +            aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device")));
> +            if (numa_node != NUMA_NODE_UNASSIGNED) {
> +                method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
> +                aml_append(method, aml_return(aml_int(numa_node)));
> +                aml_append(dev, method);
> +            }
> +
> +            acpi_dsdt_add_pci_route_table(dev, scope, irq);
> +
> +            method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
> +            aml_append(method, aml_return(aml_int(base_ecam)));
> +            aml_append(dev, method);
> +
> +            method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
> +            Aml *rbuf = aml_resource_template();
> +            aml_append(rbuf,
> +                       aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> +                                           AML_POS_DECODE, 0x0000,
> +                                           bus_num, bus_num + 1, 0x0000,
> +                                           2));
> +            aml_append(rbuf,
> +                       aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> +                                        AML_READ_WRITE, 0x0000,
> +                                        base_mmio + size_mmio -
> +                                        size_addr * count,
> +                                        base_mmio + size_mmio - 1 -
> +                                        size_addr * (count - 1),
> +                                        0x0000, size_addr));
> +            aml_append(rbuf,
> +                       aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> +                       AML_POS_DECODE, AML_ENTIRE_RANGE,
> +                       0x0000, size_pio - size_io * count,
> +                       size_pio - 1 - size_io * (count - 1),
> +                       base_pio, size_io));
> +
> +            if (use_highmem) {
> +                hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> +                hwaddr size_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].size;
> +
> +                aml_append(rbuf,
> +                       aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> +                                        AML_READ_WRITE, 0x0000,
> +                                        base_mmio_high + size_mmio_high -
> +                                        size_addr * count,
> +                                        base_mmio_high + size_mmio_high -
> +                                        1 - size_addr * (count - 1),
> +                                        0x0000, size_addr));
> +            }
> +
> +            aml_append(method, aml_name_decl("RBUF", rbuf));
> +            aml_append(method, aml_return(rbuf));
> +            aml_append(dev, method);
> +
> +            acpi_dsdt_add_pci_osc(dev, scope);
> +
> +            aml_append(scope, dev);
> +
> +        }
> +    }
>   
> -    Aml *dev = aml_device("%s", "PCI0");
> +    dev = aml_device("%s", "PCI0");
>       aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
>       aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
>       aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> @@ -302,11 +401,13 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>       aml_append(rbuf,
>           aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
>                            AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, base_mmio,
> -                         base_mmio + size_mmio - 1, 0x0000, size_mmio));
> +                         base_mmio + size_mmio - 1 - size_addr * count,
> +                         0x0000, size_mmio - size_addr * count));
>       aml_append(rbuf,
>           aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
> -                     AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
> -                     size_pio));
> +                     AML_ENTIRE_RANGE, 0x0000, 0x0000,
> +                     size_pio - 1 - size_io * count, base_pio,
> +                     size_pio - size_io * count));
>   
>       if (use_highmem) {
>           hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> @@ -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>       acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>                       (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
>       acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
> -                      vms->highmem, vms->highmem_ecam);
> +                      vms->highmem, vms->highmem_ecam, vms);
>       if (vms->acpi_dev) {
>           build_ged_aml(scope, "\\_SB."GED_DEVICE,
>                         HOTPLUG_HANDLER(vms->acpi_dev),
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f788fe27d6..6314928671 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState *vms)
>       }
>   
>       pci = PCI_HOST_BRIDGE(dev);
> +
> +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> +
>       if (pci->bus) {
>           for (i = 0; i < nb_nics; i++) {
>               NICInfo *nd = &nd_table[i];
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 71508bf40c..90f10a1e46 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -140,6 +140,13 @@ typedef struct {
>       DeviceState *gic;
>       DeviceState *acpi_dev;
>       Notifier powerdown_notifier;
> +    /*
> +     * pointer to devices and objects
> +     * Via going through the bus, all
> +     * pci devices and related objectes

Typo "objects", but I don't understand the comment well.

> +     * could be gained.
> +     */
> +    PCIBus *bus;
>   } VirtMachineState;
>   
>   #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
>
Michael S. Tsirkin Feb. 25, 2020, 11:20 a.m. UTC | #2
On Tue, Feb 25, 2020 at 10:47:47AM +0100, Philippe Mathieu-Daudé wrote:
> On 2/25/20 2:50 AM, Yubo Miao wrote:
> > From: miaoyubo <miaoyubo@huawei.com>
> > 
> > Currently virt machine is not supported by pxb-pcie,
> > and only one main host bridge described in ACPI tables.
> > In this patch,PXB-PCIE is supproted by arm and certain
> 
> Typos: "expander" in subject and "supported" here.
> 
> > resource is allocated for each pxb-pcie in acpi table.
> > The resource for the main host bridge is also reallocated.
> > 
> > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > ---
> >   hw/arm/virt-acpi-build.c | 115 ++++++++++++++++++++++++++++++++++++---
> >   hw/arm/virt.c            |   3 +
> >   include/hw/arm/virt.h    |   7 +++
> >   3 files changed, 118 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 37c34748a6..be1986c60d 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -49,6 +49,8 @@
> >   #include "kvm_arm.h"
> >   #include "migration/vmstate.h"
> > +#include "hw/arm/virt.h"
> > +#include "hw/pci/pci_bus.h"
> >   #define ARM_SPI_BASE 32
> >   static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> > @@ -266,19 +268,116 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, Aml *scope)
> >   }
> >   static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> > -                              uint32_t irq, bool use_highmem, bool highmem_ecam)
> > +                              uint32_t irq, bool use_highmem, bool highmem_ecam,
> > +                              VirtMachineState *vms)
> >   {
> >       int ecam_id = VIRT_ECAM_ID(highmem_ecam);
> > -    Aml *method, *crs;
> > +    Aml *method, *crs, *dev;
> > +    int count = 0;
> >       hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base;
> >       hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size;
> >       hwaddr base_pio = memmap[VIRT_PCIE_PIO].base;
> >       hwaddr size_pio = memmap[VIRT_PCIE_PIO].size;
> >       hwaddr base_ecam = memmap[ecam_id].base;
> >       hwaddr size_ecam = memmap[ecam_id].size;
> > +    /*
> > +     * 0x600000 would be enough for pxb device
> > +     * if it is too small, there is no enough space
> > +     * for a pcie device plugged in a pcie-root port
> > +     */
> > +    hwaddr size_addr = 0x600000;
> > +    hwaddr size_io = 0x4000;
> >       int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> > +    PCIBus *bus = VIRT_MACHINE(vms)->bus;
> > +
> > +    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);
> > +
> > +            if (!pci_bus_is_root(bus)) {
> > +                continue;
> > +            }
> > +            /*
> > +             * Coded up the MIN of the busNr defined for pxb-pcie,
> > +             * the MIN - 1 would be the MAX bus number for the main
> > +             * host bridge.
> > +             */
> > +            if (bus_num < nr_pcie_buses) {
> > +                nr_pcie_buses = bus_num;
> > +            }
> > +            count++;
> > +            dev = aml_device("PC%.02X", bus_num);
> > +            aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> > +            aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> > +            aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > +            aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> > +            aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> > +            aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
> > +            aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> > +            aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device")));
> > +            if (numa_node != NUMA_NODE_UNASSIGNED) {
> > +                method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
> > +                aml_append(method, aml_return(aml_int(numa_node)));
> > +                aml_append(dev, method);
> > +            }
> > +
> > +            acpi_dsdt_add_pci_route_table(dev, scope, irq);
> > +
> > +            method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
> > +            aml_append(method, aml_return(aml_int(base_ecam)));
> > +            aml_append(dev, method);
> > +
> > +            method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
> > +            Aml *rbuf = aml_resource_template();
> > +            aml_append(rbuf,
> > +                       aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> > +                                           AML_POS_DECODE, 0x0000,
> > +                                           bus_num, bus_num + 1, 0x0000,
> > +                                           2));
> > +            aml_append(rbuf,
> > +                       aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> > +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> > +                                        AML_READ_WRITE, 0x0000,
> > +                                        base_mmio + size_mmio -
> > +                                        size_addr * count,
> > +                                        base_mmio + size_mmio - 1 -
> > +                                        size_addr * (count - 1),
> > +                                        0x0000, size_addr));
> > +            aml_append(rbuf,
> > +                       aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> > +                       AML_POS_DECODE, AML_ENTIRE_RANGE,
> > +                       0x0000, size_pio - size_io * count,
> > +                       size_pio - 1 - size_io * (count - 1),
> > +                       base_pio, size_io));
> > +
> > +            if (use_highmem) {
> > +                hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> > +                hwaddr size_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].size;
> > +
> > +                aml_append(rbuf,
> > +                       aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> > +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> > +                                        AML_READ_WRITE, 0x0000,
> > +                                        base_mmio_high + size_mmio_high -
> > +                                        size_addr * count,
> > +                                        base_mmio_high + size_mmio_high -
> > +                                        1 - size_addr * (count - 1),
> > +                                        0x0000, size_addr));
> > +            }
> > +
> > +            aml_append(method, aml_name_decl("RBUF", rbuf));
> > +            aml_append(method, aml_return(rbuf));
> > +            aml_append(dev, method);
> > +
> > +            acpi_dsdt_add_pci_osc(dev, scope);
> > +
> > +            aml_append(scope, dev);
> > +
> > +        }
> > +    }
> > -    Aml *dev = aml_device("%s", "PCI0");
> > +    dev = aml_device("%s", "PCI0");
> >       aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> >       aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> >       aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> > @@ -302,11 +401,13 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> >       aml_append(rbuf,
> >           aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> >                            AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, base_mmio,
> > -                         base_mmio + size_mmio - 1, 0x0000, size_mmio));
> > +                         base_mmio + size_mmio - 1 - size_addr * count,
> > +                         0x0000, size_mmio - size_addr * count));
> >       aml_append(rbuf,
> >           aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
> > -                     AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
> > -                     size_pio));
> > +                     AML_ENTIRE_RANGE, 0x0000, 0x0000,
> > +                     size_pio - 1 - size_io * count, base_pio,
> > +                     size_pio - size_io * count));
> >       if (use_highmem) {
> >           hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> > @@ -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >       acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> >                       (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
> >       acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
> > -                      vms->highmem, vms->highmem_ecam);
> > +                      vms->highmem, vms->highmem_ecam, vms);
> >       if (vms->acpi_dev) {
> >           build_ged_aml(scope, "\\_SB."GED_DEVICE,
> >                         HOTPLUG_HANDLER(vms->acpi_dev),
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index f788fe27d6..6314928671 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState *vms)
> >       }
> >       pci = PCI_HOST_BRIDGE(dev);
> > +
> > +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> > +
> >       if (pci->bus) {
> >           for (i = 0; i < nb_nics; i++) {
> >               NICInfo *nd = &nd_table[i];
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > index 71508bf40c..90f10a1e46 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -140,6 +140,13 @@ typedef struct {
> >       DeviceState *gic;
> >       DeviceState *acpi_dev;
> >       Notifier powerdown_notifier;
> > +    /*
> > +     * pointer to devices and objects
> > +     * Via going through the bus, all
> > +     * pci devices and related objectes
> 
> Typo "objects", but I don't understand the comment well.

Yes, I don't understand what it says either.

> > +     * could be gained.
> > +     */
> > +    PCIBus *bus;
> >   } VirtMachineState;
> >   #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
> >
Yubo Miao Feb. 25, 2020, 12:12 p.m. UTC | #3
> -----Original Message-----
> From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
> Sent: Tuesday, February 25, 2020 5:48 PM
> To: miaoyubo <miaoyubo@huawei.com>; peter.maydell@linaro.org;
> shannon.zhaosl@gmail.com
> Cc: berrange@redhat.com; mst@redhat.com; qemu-devel@nongnu.org;
> Xiexiangyou <xiexiangyou@huawei.com>; imammedo@redhat.com
> Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support for arm
> 
> On 2/25/20 2:50 AM, Yubo Miao wrote:
> > From: miaoyubo <miaoyubo@huawei.com>
> >
> > Currently virt machine is not supported by pxb-pcie, and only one main
> > host bridge described in ACPI tables.
> > In this patch,PXB-PCIE is supproted by arm and certain
> 
> Typos: "expander" in subject and "supported" here.
> 

Thanks for your reply and sorry for the mistakes.
I will check all the subjects and comments.

> > resource is allocated for each pxb-pcie in acpi table.
> > The resource for the main host bridge is also reallocated.
> >
> > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > ---
> >   hw/arm/virt-acpi-build.c | 115
> ++++++++++++++++++++++++++++++++++++---
> >   hw/arm/virt.c            |   3 +
> >   include/hw/arm/virt.h    |   7 +++
> >   3 files changed, 118 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > 37c34748a6..be1986c60d 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -49,6 +49,8 @@
> >   #include "kvm_arm.h"
> >   #include "migration/vmstate.h"
> >
> > +#include "hw/arm/virt.h"
> > +#include "hw/pci/pci_bus.h"
> >   #define ARM_SPI_BASE 32
> >
> >       if (use_highmem) {
> >           hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> @@
> > -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> >       acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> >                       (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> NUM_VIRTIO_TRANSPORTS);
> >       acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] +
> ARM_SPI_BASE),
> > -                      vms->highmem, vms->highmem_ecam);
> > +                      vms->highmem, vms->highmem_ecam, vms);
> >       if (vms->acpi_dev) {
> >           build_ged_aml(scope, "\\_SB."GED_DEVICE,
> >                         HOTPLUG_HANDLER(vms->acpi_dev), diff --git
> > a/hw/arm/virt.c b/hw/arm/virt.c index f788fe27d6..6314928671 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState *vms)
> >       }
> >
> >       pci = PCI_HOST_BRIDGE(dev);
> > +
> > +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> > +
> >       if (pci->bus) {
> >           for (i = 0; i < nb_nics; i++) {
> >               NICInfo *nd = &nd_table[i]; diff --git
> > a/include/hw/arm/virt.h b/include/hw/arm/virt.h index
> > 71508bf40c..90f10a1e46 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -140,6 +140,13 @@ typedef struct {
> >       DeviceState *gic;
> >       DeviceState *acpi_dev;
> >       Notifier powerdown_notifier;
> > +    /*
> > +     * pointer to devices and objects
> > +     * Via going through the bus, all
> > +     * pci devices and related objectes
> 
> Typo "objects", but I don't understand the comment well.
> 

Sorry for any confusion caused ,I will rewrite the comment 
/* point to the root bus, which is pcie.0 */
Does this comment make sense?

> > +     * could be gained.
> > +     */
> > +    PCIBus *bus;
> >   } VirtMachineState;
> >
> >   #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM :
> > VIRT_PCIE_ECAM)
> >

Regards,
Miao
Michael S. Tsirkin Feb. 25, 2020, 12:27 p.m. UTC | #4
On Tue, Feb 25, 2020 at 12:12:12PM +0000, miaoyubo wrote:
> 
> > -----Original Message-----
> > From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
> > Sent: Tuesday, February 25, 2020 5:48 PM
> > To: miaoyubo <miaoyubo@huawei.com>; peter.maydell@linaro.org;
> > shannon.zhaosl@gmail.com
> > Cc: berrange@redhat.com; mst@redhat.com; qemu-devel@nongnu.org;
> > Xiexiangyou <xiexiangyou@huawei.com>; imammedo@redhat.com
> > Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support for arm
> > 
> > On 2/25/20 2:50 AM, Yubo Miao wrote:
> > > From: miaoyubo <miaoyubo@huawei.com>
> > >
> > > Currently virt machine is not supported by pxb-pcie, and only one main
> > > host bridge described in ACPI tables.
> > > In this patch,PXB-PCIE is supproted by arm and certain
> > 
> > Typos: "expander" in subject and "supported" here.
> > 
> 
> Thanks for your reply and sorry for the mistakes.
> I will check all the subjects and comments.
> 
> > > resource is allocated for each pxb-pcie in acpi table.
> > > The resource for the main host bridge is also reallocated.
> > >
> > > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > > ---
> > >   hw/arm/virt-acpi-build.c | 115
> > ++++++++++++++++++++++++++++++++++++---
> > >   hw/arm/virt.c            |   3 +
> > >   include/hw/arm/virt.h    |   7 +++
> > >   3 files changed, 118 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > > 37c34748a6..be1986c60d 100644
> > > --- a/hw/arm/virt-acpi-build.c
> > > +++ b/hw/arm/virt-acpi-build.c
> > > @@ -49,6 +49,8 @@
> > >   #include "kvm_arm.h"
> > >   #include "migration/vmstate.h"
> > >
> > > +#include "hw/arm/virt.h"
> > > +#include "hw/pci/pci_bus.h"
> > >   #define ARM_SPI_BASE 32
> > >
> > >       if (use_highmem) {
> > >           hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> > @@
> > > -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > VirtMachineState *vms)
> > >       acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> > >                       (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> > NUM_VIRTIO_TRANSPORTS);
> > >       acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] +
> > ARM_SPI_BASE),
> > > -                      vms->highmem, vms->highmem_ecam);
> > > +                      vms->highmem, vms->highmem_ecam, vms);
> > >       if (vms->acpi_dev) {
> > >           build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > >                         HOTPLUG_HANDLER(vms->acpi_dev), diff --git
> > > a/hw/arm/virt.c b/hw/arm/virt.c index f788fe27d6..6314928671 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState *vms)
> > >       }
> > >
> > >       pci = PCI_HOST_BRIDGE(dev);
> > > +
> > > +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> > > +
> > >       if (pci->bus) {
> > >           for (i = 0; i < nb_nics; i++) {
> > >               NICInfo *nd = &nd_table[i]; diff --git
> > > a/include/hw/arm/virt.h b/include/hw/arm/virt.h index
> > > 71508bf40c..90f10a1e46 100644
> > > --- a/include/hw/arm/virt.h
> > > +++ b/include/hw/arm/virt.h
> > > @@ -140,6 +140,13 @@ typedef struct {
> > >       DeviceState *gic;
> > >       DeviceState *acpi_dev;
> > >       Notifier powerdown_notifier;
> > > +    /*
> > > +     * pointer to devices and objects
> > > +     * Via going through the bus, all
> > > +     * pci devices and related objectes
> > 
> > Typo "objects", but I don't understand the comment well.
> > 
> 
> Sorry for any confusion caused ,I will rewrite the comment 
> /* point to the root bus, which is pcie.0 */
> Does this comment make sense?

Not really. E.g. it doesn't say what happens if there's more than one root.

> > > +     * could be gained.
> > > +     */
> > > +    PCIBus *bus;
> > >   } VirtMachineState;
> > >
> > >   #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM :
> > > VIRT_PCIE_ECAM)
> > >
> 
> Regards,
> Miao
Yubo Miao Feb. 25, 2020, 12:44 p.m. UTC | #5
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Tuesday, February 25, 2020 8:27 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>;
> peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> berrange@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> <xiexiangyou@huawei.com>; imammedo@redhat.com
> Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support for arm
> 
> On Tue, Feb 25, 2020 at 12:12:12PM +0000, miaoyubo wrote:
> >
> > > -----Original Message-----
> > > From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
> > > Sent: Tuesday, February 25, 2020 5:48 PM
> > > To: miaoyubo <miaoyubo@huawei.com>; peter.maydell@linaro.org;
> > > shannon.zhaosl@gmail.com
> > > Cc: berrange@redhat.com; mst@redhat.com; qemu-devel@nongnu.org;
> > > Xiexiangyou <xiexiangyou@huawei.com>; imammedo@redhat.com
> > > Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support
> > > for arm
> > >
> > > On 2/25/20 2:50 AM, Yubo Miao wrote:
> > > > From: miaoyubo <miaoyubo@huawei.com>
> > > >
> > > > Currently virt machine is not supported by pxb-pcie, and only one
> > > > main host bridge described in ACPI tables.
> > > > In this patch,PXB-PCIE is supproted by arm and certain
> > >
> > > Typos: "expander" in subject and "supported" here.
> > >
> >
> > Thanks for your reply and sorry for the mistakes.
> > I will check all the subjects and comments.
> >
> > > > resource is allocated for each pxb-pcie in acpi table.
> > > > The resource for the main host bridge is also reallocated.
> > > >
> > > > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > > > ---
> > > >   hw/arm/virt-acpi-build.c | 115
> > > ++++++++++++++++++++++++++++++++++++---
> > > >   hw/arm/virt.c            |   3 +
> > > >   include/hw/arm/virt.h    |   7 +++
> > > >   3 files changed, 118 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > > > index 37c34748a6..be1986c60d 100644
> > > > --- a/hw/arm/virt-acpi-build.c
> > > > +++ b/hw/arm/virt-acpi-build.c
> > > > @@ -49,6 +49,8 @@
> > > >   #include "kvm_arm.h"
> > > >   #include "migration/vmstate.h"
> > > >
> > > > +#include "hw/arm/virt.h"
> > > > +#include "hw/pci/pci_bus.h"
> > > >   #define ARM_SPI_BASE 32
> > > >
> > > >       if (use_highmem) {
> > > >           hwaddr base_mmio_high =
> > > > memmap[VIRT_HIGH_PCIE_MMIO].base;
> > > @@
> > > > -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker
> > > > *linker,
> > > VirtMachineState *vms)
> > > >       acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> > > >                       (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> > > NUM_VIRTIO_TRANSPORTS);
> > > >       acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] +
> > > ARM_SPI_BASE),
> > > > -                      vms->highmem, vms->highmem_ecam);
> > > > +                      vms->highmem, vms->highmem_ecam, vms);
> > > >       if (vms->acpi_dev) {
> > > >           build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > > >                         HOTPLUG_HANDLER(vms->acpi_dev), diff --git
> > > > a/hw/arm/virt.c b/hw/arm/virt.c index f788fe27d6..6314928671
> > > > 100644
> > > > --- a/hw/arm/virt.c
> > > > +++ b/hw/arm/virt.c
> > > > @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState
> *vms)
> > > >       }
> > > >
> > > >       pci = PCI_HOST_BRIDGE(dev);
> > > > +
> > > > +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> > > > +
> > > >       if (pci->bus) {
> > > >           for (i = 0; i < nb_nics; i++) {
> > > >               NICInfo *nd = &nd_table[i]; diff --git
> > > > a/include/hw/arm/virt.h b/include/hw/arm/virt.h index
> > > > 71508bf40c..90f10a1e46 100644
> > > > --- a/include/hw/arm/virt.h
> > > > +++ b/include/hw/arm/virt.h
> > > > @@ -140,6 +140,13 @@ typedef struct {
> > > >       DeviceState *gic;
> > > >       DeviceState *acpi_dev;
> > > >       Notifier powerdown_notifier;
> > > > +    /*
> > > > +     * pointer to devices and objects
> > > > +     * Via going through the bus, all
> > > > +     * pci devices and related objectes
> > >
> > > Typo "objects", but I don't understand the comment well.
> > >
> >
> > Sorry for any confusion caused ,I will rewrite the comment
> > /* point to the root bus, which is pcie.0 */ Does this comment make
> > sense?
> 
> Not really. E.g. it doesn't say what happens if there's more than one root.
> 

If there's more than one root, like pcie.0 and pcie.1, it still point to pcie.0.
In docs/pci_expander_bridge.txt, it points out pxb could be placed only
on bus 0 (pci.0). Therfore, the structure still could help us to find all pxb devices.
/* point to the bus 0, which is pcie.0
  * pxb devices could only be placed on bus 0.
  */
Is this ok?

> > > > +     * could be gained.
> > > > +     */
> > > > +    PCIBus *bus;
> > > >   } VirtMachineState;
> > > >
> > > >   #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM :
> > > > VIRT_PCIE_ECAM)
> > > >
> >
> > Regards,
> > Miao

Regards,
Miao
Michael S. Tsirkin Feb. 25, 2020, 1:11 p.m. UTC | #6
On Tue, Feb 25, 2020 at 12:44:15PM +0000, miaoyubo wrote:
> 
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Tuesday, February 25, 2020 8:27 PM
> > To: miaoyubo <miaoyubo@huawei.com>
> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com>;
> > peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > berrange@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > <xiexiangyou@huawei.com>; imammedo@redhat.com
> > Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support for arm
> > 
> > On Tue, Feb 25, 2020 at 12:12:12PM +0000, miaoyubo wrote:
> > >
> > > > -----Original Message-----
> > > > From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
> > > > Sent: Tuesday, February 25, 2020 5:48 PM
> > > > To: miaoyubo <miaoyubo@huawei.com>; peter.maydell@linaro.org;
> > > > shannon.zhaosl@gmail.com
> > > > Cc: berrange@redhat.com; mst@redhat.com; qemu-devel@nongnu.org;
> > > > Xiexiangyou <xiexiangyou@huawei.com>; imammedo@redhat.com
> > > > Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support
> > > > for arm
> > > >
> > > > On 2/25/20 2:50 AM, Yubo Miao wrote:
> > > > > From: miaoyubo <miaoyubo@huawei.com>
> > > > >
> > > > > Currently virt machine is not supported by pxb-pcie, and only one
> > > > > main host bridge described in ACPI tables.
> > > > > In this patch,PXB-PCIE is supproted by arm and certain
> > > >
> > > > Typos: "expander" in subject and "supported" here.
> > > >
> > >
> > > Thanks for your reply and sorry for the mistakes.
> > > I will check all the subjects and comments.
> > >
> > > > > resource is allocated for each pxb-pcie in acpi table.
> > > > > The resource for the main host bridge is also reallocated.
> > > > >
> > > > > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > > > > ---
> > > > >   hw/arm/virt-acpi-build.c | 115
> > > > ++++++++++++++++++++++++++++++++++++---
> > > > >   hw/arm/virt.c            |   3 +
> > > > >   include/hw/arm/virt.h    |   7 +++
> > > > >   3 files changed, 118 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > > > > index 37c34748a6..be1986c60d 100644
> > > > > --- a/hw/arm/virt-acpi-build.c
> > > > > +++ b/hw/arm/virt-acpi-build.c
> > > > > @@ -49,6 +49,8 @@
> > > > >   #include "kvm_arm.h"
> > > > >   #include "migration/vmstate.h"
> > > > >
> > > > > +#include "hw/arm/virt.h"
> > > > > +#include "hw/pci/pci_bus.h"
> > > > >   #define ARM_SPI_BASE 32
> > > > >
> > > > >       if (use_highmem) {
> > > > >           hwaddr base_mmio_high =
> > > > > memmap[VIRT_HIGH_PCIE_MMIO].base;
> > > > @@
> > > > > -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker
> > > > > *linker,
> > > > VirtMachineState *vms)
> > > > >       acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> > > > >                       (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> > > > NUM_VIRTIO_TRANSPORTS);
> > > > >       acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] +
> > > > ARM_SPI_BASE),
> > > > > -                      vms->highmem, vms->highmem_ecam);
> > > > > +                      vms->highmem, vms->highmem_ecam, vms);
> > > > >       if (vms->acpi_dev) {
> > > > >           build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > > > >                         HOTPLUG_HANDLER(vms->acpi_dev), diff --git
> > > > > a/hw/arm/virt.c b/hw/arm/virt.c index f788fe27d6..6314928671
> > > > > 100644
> > > > > --- a/hw/arm/virt.c
> > > > > +++ b/hw/arm/virt.c
> > > > > @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState
> > *vms)
> > > > >       }
> > > > >
> > > > >       pci = PCI_HOST_BRIDGE(dev);
> > > > > +
> > > > > +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> > > > > +
> > > > >       if (pci->bus) {
> > > > >           for (i = 0; i < nb_nics; i++) {
> > > > >               NICInfo *nd = &nd_table[i]; diff --git
> > > > > a/include/hw/arm/virt.h b/include/hw/arm/virt.h index
> > > > > 71508bf40c..90f10a1e46 100644
> > > > > --- a/include/hw/arm/virt.h
> > > > > +++ b/include/hw/arm/virt.h
> > > > > @@ -140,6 +140,13 @@ typedef struct {
> > > > >       DeviceState *gic;
> > > > >       DeviceState *acpi_dev;
> > > > >       Notifier powerdown_notifier;
> > > > > +    /*
> > > > > +     * pointer to devices and objects
> > > > > +     * Via going through the bus, all
> > > > > +     * pci devices and related objectes
> > > >
> > > > Typo "objects", but I don't understand the comment well.
> > > >
> > >
> > > Sorry for any confusion caused ,I will rewrite the comment
> > > /* point to the root bus, which is pcie.0 */ Does this comment make
> > > sense?
> > 
> > Not really. E.g. it doesn't say what happens if there's more than one root.
> > 
> 
> If there's more than one root, like pcie.0 and pcie.1, it still point to pcie.0.
> In docs/pci_expander_bridge.txt, it points out pxb could be placed only
> on bus 0 (pci.0). Therfore, the structure still could help us to find all pxb devices.
> /* point to the bus 0, which is pcie.0
>   * pxb devices could only be placed on bus 0.
>   */
> Is this ok?

All this needs more comments in the code constructing the tables.

Also, instead of trying to store bus and spreading logic
around like this, how about just using object_resolve_path_type?


> > > > > +     * could be gained.
> > > > > +     */
> > > > > +    PCIBus *bus;
> > > > >   } VirtMachineState;
> > > > >
> > > > >   #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM :
> > > > > VIRT_PCIE_ECAM)
> > > > >
> > >
> > > Regards,
> > > Miao
> 
> Regards,
> Miao
Michael S. Tsirkin Feb. 25, 2020, 1:14 p.m. UTC | #7
On Tue, Feb 25, 2020 at 09:50:25AM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
> 
> Currently virt machine is not supported by pxb-pcie,
> and only one main host bridge described in ACPI tables.
> In this patch,PXB-PCIE is supproted by arm and certain
> resource is allocated for each pxb-pcie in acpi table.
> The resource for the main host bridge is also reallocated.
> 
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> ---
>  hw/arm/virt-acpi-build.c | 115 ++++++++++++++++++++++++++++++++++++---
>  hw/arm/virt.c            |   3 +
>  include/hw/arm/virt.h    |   7 +++
>  3 files changed, 118 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 37c34748a6..be1986c60d 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -49,6 +49,8 @@
>  #include "kvm_arm.h"
>  #include "migration/vmstate.h"
>  
> +#include "hw/arm/virt.h"
> +#include "hw/pci/pci_bus.h"
>  #define ARM_SPI_BASE 32
>  
>  static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> @@ -266,19 +268,116 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, Aml *scope)
>  }
>  
>  static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> -                              uint32_t irq, bool use_highmem, bool highmem_ecam)
> +                              uint32_t irq, bool use_highmem, bool highmem_ecam,
> +                              VirtMachineState *vms)
>  {
>      int ecam_id = VIRT_ECAM_ID(highmem_ecam);
> -    Aml *method, *crs;
> +    Aml *method, *crs, *dev;
> +    int count = 0;
>      hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base;
>      hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size;
>      hwaddr base_pio = memmap[VIRT_PCIE_PIO].base;
>      hwaddr size_pio = memmap[VIRT_PCIE_PIO].size;
>      hwaddr base_ecam = memmap[ecam_id].base;
>      hwaddr size_ecam = memmap[ecam_id].size;
> +    /*
> +     * 0x600000 would be enough for pxb device

It's not clear where does pxb come in here.

> +     * if it is too small, there is no enough space
> +     * for a pcie device plugged in a pcie-root port
> +     */
> +    hwaddr size_addr = 0x600000;
> +    hwaddr size_io = 0x4000;

Please explain how are memory and io partitioned.
Looks like you are forcing a very specific layout here,
but it's not documented anywhere, which
can cause issues like running out of memory where
we previously were ok.


>      int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> +    PCIBus *bus = VIRT_MACHINE(vms)->bus;
> +
> +    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);
> +
> +            if (!pci_bus_is_root(bus)) {
> +                continue;
> +            }
> +            /*
> +             * Coded up the MIN of the busNr defined for pxb-pcie,
> +             * the MIN - 1 would be the MAX bus number for the main
> +             * host bridge.
> +             */
> +            if (bus_num < nr_pcie_buses) {
> +                nr_pcie_buses = bus_num;
> +            }
> +            count++;
> +            dev = aml_device("PC%.02X", bus_num);
> +            aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> +            aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> +            aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> +            aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +            aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> +            aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
> +            aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> +            aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device")));

how do you know it's a pxb device?

> +            if (numa_node != NUMA_NODE_UNASSIGNED) {
> +                method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
> +                aml_append(method, aml_return(aml_int(numa_node)));
> +                aml_append(dev, method);
> +            }
> +
> +            acpi_dsdt_add_pci_route_table(dev, scope, irq);
> +
> +            method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
> +            aml_append(method, aml_return(aml_int(base_ecam)));
> +            aml_append(dev, method);
> +
> +            method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
> +            Aml *rbuf = aml_resource_template();
> +            aml_append(rbuf,
> +                       aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> +                                           AML_POS_DECODE, 0x0000,
> +                                           bus_num, bus_num + 1, 0x0000,
> +                                           2));
> +            aml_append(rbuf,
> +                       aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> +                                        AML_READ_WRITE, 0x0000,
> +                                        base_mmio + size_mmio -
> +                                        size_addr * count,
> +                                        base_mmio + size_mmio - 1 -
> +                                        size_addr * (count - 1),
> +                                        0x0000, size_addr));
> +            aml_append(rbuf,
> +                       aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> +                       AML_POS_DECODE, AML_ENTIRE_RANGE,
> +                       0x0000, size_pio - size_io * count,
> +                       size_pio - 1 - size_io * (count - 1),
> +                       base_pio, size_io));
> +
> +            if (use_highmem) {
> +                hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> +                hwaddr size_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].size;
> +
> +                aml_append(rbuf,
> +                       aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> +                                        AML_READ_WRITE, 0x0000,
> +                                        base_mmio_high + size_mmio_high -
> +                                        size_addr * count,
> +                                        base_mmio_high + size_mmio_high -
> +                                        1 - size_addr * (count - 1),
> +                                        0x0000, size_addr));
> +            }
> +
> +            aml_append(method, aml_name_decl("RBUF", rbuf));
> +            aml_append(method, aml_return(rbuf));
> +            aml_append(dev, method);
> +
> +            acpi_dsdt_add_pci_osc(dev, scope);
> +
> +            aml_append(scope, dev);
> +
> +        }
> +    }
>  
> -    Aml *dev = aml_device("%s", "PCI0");
> +    dev = aml_device("%s", "PCI0");
>      aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
>      aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
>      aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> @@ -302,11 +401,13 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>      aml_append(rbuf,
>          aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
>                           AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, base_mmio,
> -                         base_mmio + size_mmio - 1, 0x0000, size_mmio));
> +                         base_mmio + size_mmio - 1 - size_addr * count,
> +                         0x0000, size_mmio - size_addr * count));
>      aml_append(rbuf,
>          aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
> -                     AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
> -                     size_pio));
> +                     AML_ENTIRE_RANGE, 0x0000, 0x0000,
> +                     size_pio - 1 - size_io * count, base_pio,
> +                     size_pio - size_io * count));
>  
>      if (use_highmem) {
>          hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> @@ -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>                      (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
>      acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
> -                      vms->highmem, vms->highmem_ecam);
> +                      vms->highmem, vms->highmem_ecam, vms);
>      if (vms->acpi_dev) {
>          build_ged_aml(scope, "\\_SB."GED_DEVICE,
>                        HOTPLUG_HANDLER(vms->acpi_dev),
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f788fe27d6..6314928671 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState *vms)
>      }
>  
>      pci = PCI_HOST_BRIDGE(dev);
> +
> +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> +
>      if (pci->bus) {
>          for (i = 0; i < nb_nics; i++) {
>              NICInfo *nd = &nd_table[i];
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 71508bf40c..90f10a1e46 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -140,6 +140,13 @@ typedef struct {
>      DeviceState *gic;
>      DeviceState *acpi_dev;
>      Notifier powerdown_notifier;
> +    /*
> +     * pointer to devices and objects
> +     * Via going through the bus, all
> +     * pci devices and related objectes
> +     * could be gained.
> +     */
> +    PCIBus *bus;
>  } VirtMachineState;
>  
>  #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
> -- 
> 2.19.1
>
Yubo Miao Feb. 26, 2020, 10:42 a.m. UTC | #8
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Tuesday, February 25, 2020 9:12 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>;
> peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> berrange@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> <xiexiangyou@huawei.com>; imammedo@redhat.com
> Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support for arm
> 
> On Tue, Feb 25, 2020 at 12:44:15PM +0000, miaoyubo wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > Sent: Tuesday, February 25, 2020 8:27 PM
> > > To: miaoyubo <miaoyubo@huawei.com>
> > > Cc: Philippe Mathieu-Daudé <philmd@redhat.com>;
> > > peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > > berrange@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > > <xiexiangyou@huawei.com>; imammedo@redhat.com
> > > Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support
> > > for arm
> > >
> > > On Tue, Feb 25, 2020 at 12:12:12PM +0000, miaoyubo wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
> > > > > Sent: Tuesday, February 25, 2020 5:48 PM
> > > > > To: miaoyubo <miaoyubo@huawei.com>; peter.maydell@linaro.org;
> > > > > shannon.zhaosl@gmail.com
> > > > > Cc: berrange@redhat.com; mst@redhat.com; qemu-
> devel@nongnu.org;
> > > > > Xiexiangyou <xiexiangyou@huawei.com>; imammedo@redhat.com
> > > > > Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb
> > > > > support for arm
> > > > >
> > > > > On 2/25/20 2:50 AM, Yubo Miao wrote:
> > > > > > From: miaoyubo <miaoyubo@huawei.com>
> > > > > >
> > > > > > Currently virt machine is not supported by pxb-pcie, and only
> > > > > > one main host bridge described in ACPI tables.
> > > > > > In this patch,PXB-PCIE is supproted by arm and certain
> > > > >
> > > > > Typos: "expander" in subject and "supported" here.
> > > > >
> > > >
> > > > Thanks for your reply and sorry for the mistakes.
> > > > I will check all the subjects and comments.
> > > >
> > > > > > resource is allocated for each pxb-pcie in acpi table.
> > > > > > The resource for the main host bridge is also reallocated.
> > > > > >
> > > > > > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > > > > > ---
> > > > > >   hw/arm/virt-acpi-build.c | 115
> > > > > ++++++++++++++++++++++++++++++++++++---
> > > > > >   hw/arm/virt.c            |   3 +
> > > > > >   include/hw/arm/virt.h    |   7 +++
> > > > > >   3 files changed, 118 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/arm/virt-acpi-build.c
> > > > > > b/hw/arm/virt-acpi-build.c index 37c34748a6..be1986c60d 100644
> > > > > > --- a/hw/arm/virt-acpi-build.c
> > > > > > +++ b/hw/arm/virt-acpi-build.c
> > > > > > @@ -49,6 +49,8 @@
> > > > > >   #include "kvm_arm.h"
> > > > > >   #include "migration/vmstate.h"
> > > > > >
> > > > > > +#include "hw/arm/virt.h"
> > > > > > +#include "hw/pci/pci_bus.h"
> > > > > >   #define ARM_SPI_BASE 32
> > > > > >
> > > > > >       if (use_highmem) {
> > > > > >           hwaddr base_mmio_high =
> > > > > > memmap[VIRT_HIGH_PCIE_MMIO].base;
> > > > > @@
> > > > > > -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker
> > > > > > *linker,
> > > > > VirtMachineState *vms)
> > > > > >       acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> > > > > >                       (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> > > > > NUM_VIRTIO_TRANSPORTS);
> > > > > >       acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] +
> > > > > ARM_SPI_BASE),
> > > > > > -                      vms->highmem, vms->highmem_ecam);
> > > > > > +                      vms->highmem, vms->highmem_ecam, vms);
> > > > > >       if (vms->acpi_dev) {
> > > > > >           build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > > > > >                         HOTPLUG_HANDLER(vms->acpi_dev), diff
> > > > > > --git a/hw/arm/virt.c b/hw/arm/virt.c index
> > > > > > f788fe27d6..6314928671
> > > > > > 100644
> > > > > > --- a/hw/arm/virt.c
> > > > > > +++ b/hw/arm/virt.c
> > > > > > @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState
> > > *vms)
> > > > > >       }
> > > > > >
> > > > > >       pci = PCI_HOST_BRIDGE(dev);
> > > > > > +
> > > > > > +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> > > > > > +
> > > > > >       if (pci->bus) {
> > > > > >           for (i = 0; i < nb_nics; i++) {
> > > > > >               NICInfo *nd = &nd_table[i]; diff --git
> > > > > > a/include/hw/arm/virt.h b/include/hw/arm/virt.h index
> > > > > > 71508bf40c..90f10a1e46 100644
> > > > > > --- a/include/hw/arm/virt.h
> > > > > > +++ b/include/hw/arm/virt.h
> > > > > > @@ -140,6 +140,13 @@ typedef struct {
> > > > > >       DeviceState *gic;
> > > > > >       DeviceState *acpi_dev;
> > > > > >       Notifier powerdown_notifier;
> > > > > > +    /*
> > > > > > +     * pointer to devices and objects
> > > > > > +     * Via going through the bus, all
> > > > > > +     * pci devices and related objectes
> > > > >
> > > > > Typo "objects", but I don't understand the comment well.
> > > > >
> > > >
> > > > Sorry for any confusion caused ,I will rewrite the comment
> > > > /* point to the root bus, which is pcie.0 */ Does this comment
> > > > make sense?
> > >
> > > Not really. E.g. it doesn't say what happens if there's more than one root.
> > >
> >
> > If there's more than one root, like pcie.0 and pcie.1, it still point to pcie.0.
> > In docs/pci_expander_bridge.txt, it points out pxb could be placed
> > only on bus 0 (pci.0). Therfore, the structure still could help us to find all
> pxb devices.
> > /* point to the bus 0, which is pcie.0
> >   * pxb devices could only be placed on bus 0.
> >   */
> > Is this ok?
> 
> All this needs more comments in the code constructing the tables.
> 

Thanks for replying, I will add more comments in the table construction.

> Also, instead of trying to store bus and spreading logic around like this, how
> about just using object_resolve_path_type?
> 
> 

Thanks for the suggestion, using object_resolve_path_type  seems to be better. 

> > > > > > +     * could be gained.
> > > > > > +     */
> > > > > > +    PCIBus *bus;
> > > > > >   } VirtMachineState;
> > > > > >
> > > > > >   #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM :
> > > > > > VIRT_PCIE_ECAM)
> > > > > >
> > > >
> > > > Regards,
> > > > Miao
> >
> > Regards,
> > Miao

Regards,
Miao
Yubo Miao Feb. 26, 2020, 10:56 a.m. UTC | #9
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Tuesday, February 25, 2020 9:15 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com; Xiexiangyou
> <xiexiangyou@huawei.com>; imammedo@redhat.com; qemu-
> devel@nongnu.org; berrange@redhat.com
> Subject: Re: [PATCH v4 2/3] acpi:pci-expender-bus: Add pxb support for arm
> 
> On Tue, Feb 25, 2020 at 09:50:25AM +0800, Yubo Miao wrote:
> > From: miaoyubo <miaoyubo@huawei.com>
> >
> > Currently virt machine is not supported by pxb-pcie, and only one main
> > host bridge described in ACPI tables.
> > In this patch,PXB-PCIE is supproted by arm and certain resource is
> > allocated for each pxb-pcie in acpi table.
> > The resource for the main host bridge is also reallocated.
> >
> > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > ---
> >  hw/arm/virt-acpi-build.c | 115
> ++++++++++++++++++++++++++++++++++++---
> >  hw/arm/virt.c            |   3 +
> >  include/hw/arm/virt.h    |   7 +++
> >  3 files changed, 118 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > 37c34748a6..be1986c60d 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -49,6 +49,8 @@
> >  #include "kvm_arm.h"
> >  #include "migration/vmstate.h"
> >
> > +#include "hw/arm/virt.h"
> > +#include "hw/pci/pci_bus.h"
> >  #define ARM_SPI_BASE 32
> >
> >  static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus) @@ -266,19
> > +268,116 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, Aml *scope)  }
> >
> >  static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry
> *memmap,
> > -                              uint32_t irq, bool use_highmem, bool highmem_ecam)
> > +                              uint32_t irq, bool use_highmem, bool highmem_ecam,
> > +                              VirtMachineState *vms)
> >  {
> >      int ecam_id = VIRT_ECAM_ID(highmem_ecam);
> > -    Aml *method, *crs;
> > +    Aml *method, *crs, *dev;
> > +    int count = 0;
> >      hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base;
> >      hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size;
> >      hwaddr base_pio = memmap[VIRT_PCIE_PIO].base;
> >      hwaddr size_pio = memmap[VIRT_PCIE_PIO].size;
> >      hwaddr base_ecam = memmap[ecam_id].base;
> >      hwaddr size_ecam = memmap[ecam_id].size;
> > +    /*
> > +     * 0x600000 would be enough for pxb device
> 
> It's not clear where does pxb come in here.
> 

Thanks for replying, I would add comments when pxb comes.

> > +     * if it is too small, there is no enough space
> > +     * for a pcie device plugged in a pcie-root port
> > +     */
> > +    hwaddr size_addr = 0x600000;
> > +    hwaddr size_io = 0x4000;
> 
> Please explain how are memory and io partitioned.
> Looks like you are forcing a very specific layout here, but it's not documented
> anywhere, which can cause issues like running out of memory where we
> previously were ok.
> 
> 

The resources allocated for pxb is  in the end of the resources used to
be allocated for pci host bridge. The memory length for each pxb-pcie is
0x600000 and the io is 16k. The resources allocated for pxbs are quiet small,
Therefore, the resources for the main host bridge should be large enough.
 
For piix4, the memory resource length for each PXB is  0x200000, but
In arm, 200000 is not enough, if one nvme is plugged on pcie-root-port and plug
on the pxb, 600000 is enough for the memory, therefore, the memory length are defined
as 0x600000. 
 
Just using 0x600000 and 0x4000 is not so decent, 
I will try to find if there's a better way to allocate the resources.

> >      int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> > +    PCIBus *bus = VIRT_MACHINE(vms)->bus;
> > +
> > +    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);
> > +
> > +            if (!pci_bus_is_root(bus)) {
> > +                continue;
> > +            }
> > +            /*
> > +             * Coded up the MIN of the busNr defined for pxb-pcie,
> > +             * the MIN - 1 would be the MAX bus number for the main
> > +             * host bridge.
> > +             */
> > +            if (bus_num < nr_pcie_buses) {
> > +                nr_pcie_buses = bus_num;
> > +            }
> > +            count++;
> > +            dev = aml_device("PC%.02X", bus_num);
> > +            aml_append(dev, aml_name_decl("_HID",
> aml_string("PNP0A08")));
> > +            aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> > +            aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > +            aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> > +            aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> > +            aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
> > +            aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> > +            aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb
> > + Device")));
> 
> how do you know it's a pxb device?
> 

if (!pci_bus_is_root(bus)) 
continue;
Just go through the children of bus.0, if it is root bus, then it is a pxb device.

> > +            if (numa_node != NUMA_NODE_UNASSIGNED) {
> > +                method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
> > +                aml_append(method, aml_return(aml_int(numa_node)));
> > +                aml_append(dev, method);
> > +            }
> > +
> > +            acpi_dsdt_add_pci_route_table(dev, scope, irq);
> > +
> > +            method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
> > +            aml_append(method, aml_return(aml_int(base_ecam)));
> > +            aml_append(dev, method);
> > +
> > +            method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
> > +            Aml *rbuf = aml_resource_template();
> > +            aml_append(rbuf,
> > +                       aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> > +                                           AML_POS_DECODE, 0x0000,
> > +                                           bus_num, bus_num + 1, 0x0000,
> > +                                           2));
> > +            aml_append(rbuf,
> > +                       aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> > +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> > +                                        AML_READ_WRITE, 0x0000,
> > +                                        base_mmio + size_mmio -
> > +                                        size_addr * count,
> > +                                        base_mmio + size_mmio - 1 -
> > +                                        size_addr * (count - 1),
> > +                                        0x0000, size_addr));
> > +            aml_append(rbuf,
> > +                       aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> > +                       AML_POS_DECODE, AML_ENTIRE_RANGE,
> > +                       0x0000, size_pio - size_io * count,
> > +                       size_pio - 1 - size_io * (count - 1),
> > +                       base_pio, size_io));
> > +
> > +            if (use_highmem) {
> > +                hwaddr base_mmio_high =
> memmap[VIRT_HIGH_PCIE_MMIO].base;
> > +                hwaddr size_mmio_high =
> > + memmap[VIRT_HIGH_PCIE_MMIO].size;
> > +
> > +                aml_append(rbuf,
> > +                       aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> > +                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
> > +                                        AML_READ_WRITE, 0x0000,
> > +                                        base_mmio_high + size_mmio_high -
> > +                                        size_addr * count,
> > +                                        base_mmio_high + size_mmio_high -
> > +                                        1 - size_addr * (count - 1),
> > +                                        0x0000, size_addr));
> > +            }
> > +
> > +            aml_append(method, aml_name_decl("RBUF", rbuf));
> > +            aml_append(method, aml_return(rbuf));
> > +            aml_append(dev, method);
> > +
> > +            acpi_dsdt_add_pci_osc(dev, scope);
> > +
> > +            aml_append(scope, dev);
> > +
> > +        }
> > +    }
> >
> > -    Aml *dev = aml_device("%s", "PCI0");
> > +    dev = aml_device("%s", "PCI0");
> >      aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> >      aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> >      aml_append(dev, aml_name_decl("_SEG", aml_int(0))); @@ -302,11
> > +401,13 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap,
> >      aml_append(rbuf,
> >          aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> AML_MAX_FIXED,
> >                           AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
> base_mmio,
> > -                         base_mmio + size_mmio - 1, 0x0000, size_mmio));
> > +                         base_mmio + size_mmio - 1 - size_addr * count,
> > +                         0x0000, size_mmio - size_addr * count));
> >      aml_append(rbuf,
> >          aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE,
> > -                     AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
> > -                     size_pio));
> > +                     AML_ENTIRE_RANGE, 0x0000, 0x0000,
> > +                     size_pio - 1 - size_io * count, base_pio,
> > +                     size_pio - size_io * count));
> >
> >      if (use_highmem) {
> >          hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> @@
> > -746,7 +847,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> >      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> >                      (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> NUM_VIRTIO_TRANSPORTS);
> >      acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] +
> ARM_SPI_BASE),
> > -                      vms->highmem, vms->highmem_ecam);
> > +                      vms->highmem, vms->highmem_ecam, vms);
> >      if (vms->acpi_dev) {
> >          build_ged_aml(scope, "\\_SB."GED_DEVICE,
> >                        HOTPLUG_HANDLER(vms->acpi_dev), diff --git
> > a/hw/arm/virt.c b/hw/arm/virt.c index f788fe27d6..6314928671 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1246,6 +1246,9 @@ static void create_pcie(VirtMachineState *vms)
> >      }
> >
> >      pci = PCI_HOST_BRIDGE(dev);
> > +
> > +    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> > +
> >      if (pci->bus) {
> >          for (i = 0; i < nb_nics; i++) {
> >              NICInfo *nd = &nd_table[i]; diff --git
> > a/include/hw/arm/virt.h b/include/hw/arm/virt.h index
> > 71508bf40c..90f10a1e46 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -140,6 +140,13 @@ typedef struct {
> >      DeviceState *gic;
> >      DeviceState *acpi_dev;
> >      Notifier powerdown_notifier;
> > +    /*
> > +     * pointer to devices and objects
> > +     * Via going through the bus, all
> > +     * pci devices and related objectes
> > +     * could be gained.
> > +     */
> > +    PCIBus *bus;
> >  } VirtMachineState;
> >
> >  #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM :
> > VIRT_PCIE_ECAM)
> > --
> > 2.19.1
> >
diff mbox series

Patch

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 37c34748a6..be1986c60d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -49,6 +49,8 @@ 
 #include "kvm_arm.h"
 #include "migration/vmstate.h"
 
+#include "hw/arm/virt.h"
+#include "hw/pci/pci_bus.h"
 #define ARM_SPI_BASE 32
 
 static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
@@ -266,19 +268,116 @@  static void acpi_dsdt_add_pci_osc(Aml *dev, Aml *scope)
 }
 
 static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
-                              uint32_t irq, bool use_highmem, bool highmem_ecam)
+                              uint32_t irq, bool use_highmem, bool highmem_ecam,
+                              VirtMachineState *vms)
 {
     int ecam_id = VIRT_ECAM_ID(highmem_ecam);
-    Aml *method, *crs;
+    Aml *method, *crs, *dev;
+    int count = 0;
     hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base;
     hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size;
     hwaddr base_pio = memmap[VIRT_PCIE_PIO].base;
     hwaddr size_pio = memmap[VIRT_PCIE_PIO].size;
     hwaddr base_ecam = memmap[ecam_id].base;
     hwaddr size_ecam = memmap[ecam_id].size;
+    /*
+     * 0x600000 would be enough for pxb device
+     * if it is too small, there is no enough space
+     * for a pcie device plugged in a pcie-root port
+     */
+    hwaddr size_addr = 0x600000;
+    hwaddr size_io = 0x4000;
     int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
+    PCIBus *bus = VIRT_MACHINE(vms)->bus;
+
+    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);
+
+            if (!pci_bus_is_root(bus)) {
+                continue;
+            }
+            /*
+             * Coded up the MIN of the busNr defined for pxb-pcie,
+             * the MIN - 1 would be the MAX bus number for the main
+             * host bridge.
+             */
+            if (bus_num < nr_pcie_buses) {
+                nr_pcie_buses = bus_num;
+            }
+            count++;
+            dev = aml_device("PC%.02X", bus_num);
+            aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
+            aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
+            aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+            aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+            aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
+            aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
+            aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
+            aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device")));
+            if (numa_node != NUMA_NODE_UNASSIGNED) {
+                method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
+                aml_append(method, aml_return(aml_int(numa_node)));
+                aml_append(dev, method);
+            }
+
+            acpi_dsdt_add_pci_route_table(dev, scope, irq);
+
+            method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
+            aml_append(method, aml_return(aml_int(base_ecam)));
+            aml_append(dev, method);
+
+            method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
+            Aml *rbuf = aml_resource_template();
+            aml_append(rbuf,
+                       aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
+                                           AML_POS_DECODE, 0x0000,
+                                           bus_num, bus_num + 1, 0x0000,
+                                           2));
+            aml_append(rbuf,
+                       aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
+                                        AML_READ_WRITE, 0x0000,
+                                        base_mmio + size_mmio -
+                                        size_addr * count,
+                                        base_mmio + size_mmio - 1 -
+                                        size_addr * (count - 1),
+                                        0x0000, size_addr));
+            aml_append(rbuf,
+                       aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
+                       AML_POS_DECODE, AML_ENTIRE_RANGE,
+                       0x0000, size_pio - size_io * count,
+                       size_pio - 1 - size_io * (count - 1),
+                       base_pio, size_io));
+
+            if (use_highmem) {
+                hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
+                hwaddr size_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].size;
+
+                aml_append(rbuf,
+                       aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+                                        AML_MAX_FIXED, AML_NON_CACHEABLE,
+                                        AML_READ_WRITE, 0x0000,
+                                        base_mmio_high + size_mmio_high -
+                                        size_addr * count,
+                                        base_mmio_high + size_mmio_high -
+                                        1 - size_addr * (count - 1),
+                                        0x0000, size_addr));
+            }
+
+            aml_append(method, aml_name_decl("RBUF", rbuf));
+            aml_append(method, aml_return(rbuf));
+            aml_append(dev, method);
+
+            acpi_dsdt_add_pci_osc(dev, scope);
+
+            aml_append(scope, dev);
+
+        }
+    }
 
-    Aml *dev = aml_device("%s", "PCI0");
+    dev = aml_device("%s", "PCI0");
     aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
     aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
     aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
@@ -302,11 +401,13 @@  static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
     aml_append(rbuf,
         aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
                          AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, base_mmio,
-                         base_mmio + size_mmio - 1, 0x0000, size_mmio));
+                         base_mmio + size_mmio - 1 - size_addr * count,
+                         0x0000, size_mmio - size_addr * count));
     aml_append(rbuf,
         aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
-                     AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
-                     size_pio));
+                     AML_ENTIRE_RANGE, 0x0000, 0x0000,
+                     size_pio - 1 - size_io * count, base_pio,
+                     size_pio - size_io * count));
 
     if (use_highmem) {
         hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
@@ -746,7 +847,7 @@  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
                     (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
     acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
-                      vms->highmem, vms->highmem_ecam);
+                      vms->highmem, vms->highmem_ecam, vms);
     if (vms->acpi_dev) {
         build_ged_aml(scope, "\\_SB."GED_DEVICE,
                       HOTPLUG_HANDLER(vms->acpi_dev),
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f788fe27d6..6314928671 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1246,6 +1246,9 @@  static void create_pcie(VirtMachineState *vms)
     }
 
     pci = PCI_HOST_BRIDGE(dev);
+
+    VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
+
     if (pci->bus) {
         for (i = 0; i < nb_nics; i++) {
             NICInfo *nd = &nd_table[i];
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 71508bf40c..90f10a1e46 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -140,6 +140,13 @@  typedef struct {
     DeviceState *gic;
     DeviceState *acpi_dev;
     Notifier powerdown_notifier;
+    /*
+     * pointer to devices and objects
+     * Via going through the bus, all
+     * pci devices and related objectes
+     * could be gained.
+     */
+    PCIBus *bus;
 } VirtMachineState;
 
 #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)