From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47190) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a21Ou-0004NG-4O for qemu-devel@nongnu.org; Thu, 26 Nov 2015 13:36:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a21Oq-0001A4-UM for qemu-devel@nongnu.org; Thu, 26 Nov 2015 13:36:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57623) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a21Oq-00019z-NJ for qemu-devel@nongnu.org; Thu, 26 Nov 2015 13:36:08 -0500 References: <1448553628-5446-1-git-send-email-marcel@redhat.com> <56573AF0.2050207@redhat.com> From: Marcel Apfelbaum Message-ID: <5657510F.6080909@redhat.com> Date: Thu, 26 Nov 2015 20:35:59 +0200 MIME-Version: 1.0 In-Reply-To: <56573AF0.2050207@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, kraxel@redhat.com, laine@redhat.com, pbonzini@redhat.com, imammedo@redhat.com, rth@twiddle.net 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(-) >> >