> -----Original Message----- > From: Michael S. Tsirkin [mailto:m...@redhat.com] > Sent: Tuesday, February 25, 2020 9:12 PM > To: miaoyubo <miaoy...@huawei.com> > Cc: Philippe Mathieu-Daudé <phi...@redhat.com>; > peter.mayd...@linaro.org; shannon.zha...@gmail.com; > berra...@redhat.com; qemu-devel@nongnu.org; Xiexiangyou > <xiexiang...@huawei.com>; imamm...@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:m...@redhat.com] > > > Sent: Tuesday, February 25, 2020 8:27 PM > > > To: miaoyubo <miaoy...@huawei.com> > > > Cc: Philippe Mathieu-Daudé <phi...@redhat.com>; > > > peter.mayd...@linaro.org; shannon.zha...@gmail.com; > > > berra...@redhat.com; qemu-devel@nongnu.org; Xiexiangyou > > > <xiexiang...@huawei.com>; imamm...@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:phi...@redhat.com] > > > > > Sent: Tuesday, February 25, 2020 5:48 PM > > > > > To: miaoyubo <miaoy...@huawei.com>; peter.mayd...@linaro.org; > > > > > shannon.zha...@gmail.com > > > > > Cc: berra...@redhat.com; m...@redhat.com; qemu- > de...@nongnu.org; > > > > > Xiexiangyou <xiexiang...@huawei.com>; imamm...@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 <miaoy...@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 <miaoy...@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
