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

[miaoyubo <miaoyubo@huawei.com>]

> -----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