Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35

[Marcel Apfelbaum]

On 11/26/2015 07:01 PM, Laszlo Ersek wrote:
> Hello Marcel,
>
> On 11/26/15 17:00, Marcel Apfelbaum wrote:
> > Note:
> > I took the liberty to CC all the reviewers that took their time
> > and had a look on the previous version, thanks!!
> >
> > The PXB host bridge provides a way to have multiple PCI hierarchies (PCI root 
> > buses).
> > This series introduces the pxb-pcie counterpart for PCI Express 
> > machines(Currently Q35).
> >
> > This approach works because the Root Complexes are exposed to guest as regular
> > (legacy) opaque PCI host bridges.
> >
> > Tested on Fedora and Windows guests with both Root Ports and PCIe Switches.
> >
> > v2 -> v3:
> >   Addressed Eduardo Habkost comments:
> >   - Added a bus property to PC machines and use it when querying bus 0.
> >   Addressed comments from multiple reviewers (Paolo,Markus,Gerd,Michael)
> >   - The issue was the backport compatibility when the PXB changes.
> >   - Following all the comments I chose:
> >     - Leave the PXB intact as it does the job and all its features
> >       (including the internal pci bridge) makes sense.
> >     - Add a new device that re-uses all the PXB code but is exposed as
> >       a different device to guests.
> >     - Once the functionality of the new device diverges we will have
> >       no problem to separate the code.
>
>
> I don't think I can productively contribute to the review of this
> series, but at least I'll try to follow the comments of others.

Hi Laszlo,

Thank you for your comments.

> Also, your first patch looks like it touches code that is shared by the
> i440fx PXB. I think it should cause no change in observable behavior, right?

This was the intention. Yes, it should be no change visible to the guest.

> Should I regression test it with OVMF? If so, that might take... forever. :(

I am planning to do this myself, I might ping you if I can't compile/run it.
I also think that the new device will work out of the box with Q35 + OVMF.

> On the other hand, if you have ACPI table dumps from within an i440fx
> SeaBIOS Linux guest, both from before and after your QEMU patches, and
> those dumps are identical, then that's good evidence against
> regressions. (I tend to do such acpidump-based comparisons when messing
> with ACPI builder code.)

This is a good idea, I am going to compare the dumps and get back to you.

> Thanks (and sorry I can't help more ATM)

No problem, thank you for your time!
Marcel

> Laszlo
>
>
> > v1 -> v2:
> >   Addressed Gerd Hoffmann comments:
> >   - Added x-enable-internal-bridge compat property to keep the PCI
> >     bridge for older machine to avoid breaking migration.
> >
> > Thanks,
> > Marcel
> >
> > Marcel Apfelbaum (3):
> >    hw/acpi: merge pxb adjacent memory/IO ranges
> >    hw/pxb: introduce pxb-pcie expander for PCIe machines
> >    hw/i386: extend pxb query for all PC machines
> >
> >   hw/i386/acpi-build.c                | 126 +++++++++++++++++++++---------------
> >   hw/i386/pc.c                        |   2 +-
> >   hw/i386/pc_piix.c                   |   1 +
> >   hw/i386/pc_q35.c                    |   1 +
> >   hw/pci-bridge/pci_expander_bridge.c |  98 +++++++++++++++++++++++-----
> >   include/hw/i386/pc.h                |   1 +
> >   include/hw/pci/pci.h                |   1 +
> >   7 files changed, 163 insertions(+), 67 deletions(-)