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

[Igor Mammedov]

On Thu, 26 Nov 2015 20:35:59 +0200
Marcel Apfelbaum <address@hidden> wrote:

> 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.
To do it, you can use patch from my drop_ASL_support_v1 branch:
"tests: acpi: print ASL diff in verbose mode"
https://github.com/imammedo/qemu/commit/2df59d77151029554a90ce53c059860babadaf30


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