qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Marcel Apfelbaum <marcel@redhat.com>
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
Subject: Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
Date: Thu, 26 Nov 2015 18:01:36 +0100	[thread overview]
Message-ID: <56573AF0.2050207@redhat.com> (raw)
In-Reply-To: <1448553628-5446-1-git-send-email-marcel@redhat.com>

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.

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?

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

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

Thanks (and sorry I can't help more ATM)
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(-)
> 

  parent reply	other threads:[~2015-11-26 17:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-26 16:00 [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35 Marcel Apfelbaum
2015-11-26 16:00 ` [Qemu-devel] [PATCH V3 1/3] hw/acpi: merge pxb adjacent memory/IO ranges Marcel Apfelbaum
2015-11-26 16:00 ` [Qemu-devel] [PATCH V3 2/3] hw/pxb: introduce pxb-pcie expander for PCIe machines Marcel Apfelbaum
2015-11-26 16:00 ` [Qemu-devel] [PATCH V3 3/3] hw/i386: extend pxb query for all PC machines Marcel Apfelbaum
2015-11-27 17:28   ` Eduardo Habkost
2015-11-29  8:46     ` Marcel Apfelbaum
2015-11-30 15:07       ` Eduardo Habkost
2015-12-01 14:07         ` Marcel Apfelbaum
2015-12-01 14:48           ` Eduardo Habkost
2015-12-01 14:55             ` Marcel Apfelbaum
2015-12-01 15:09               ` Eduardo Habkost
2015-12-01 16:50                 ` Marcel Apfelbaum
2015-12-01 17:10                   ` Eduardo Habkost
2015-12-01 18:20   ` Eduardo Habkost
2015-12-01 20:53     ` Marcel Apfelbaum
2015-12-01 22:33       ` Eduardo Habkost
2015-11-26 17:01 ` Laszlo Ersek [this message]
2015-11-26 18:35   ` [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35 Marcel Apfelbaum
2015-11-27 17:04     ` Igor Mammedov
2015-11-29  8:53       ` Marcel Apfelbaum
2015-11-29 12:37   ` Marcel Apfelbaum
2015-11-30  5:23     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56573AF0.2050207@redhat.com \
    --to=lersek@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=laine@redhat.com \
    --cc=marcel@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).