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

[Marcel Apfelbaum <marcel@redhat.com>]

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