[Qemu-devel] [PATCH for-4.0 0/2] spapr: Fix extended config space accesses

Greg Kurz posted 2 patches 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/155414129703.574858.5054152366329409898.stgit@bahia.lan
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
hw/pci/pci.c             |   24 ++++++++++++++++++++++++
hw/pci/pci_host.c        |    2 +-
hw/ppc/spapr_pci.c       |   26 +++++++++++++++++++++++++-
include/hw/pci/pci.h     |    2 ++
include/hw/pci/pci_bus.h |    1 +
5 files changed, 53 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH for-4.0 0/2] spapr: Fix extended config space accesses
Posted by Greg Kurz 5 years ago
Recent commit c2077e2ca0da7 added stricter checks that now prevent
a guest to access the extended config space of a PCIe device connected
attached to a PHB on a pseries machine.

PAPR compatible PHBs act like legacy PCI busses, but they do allow access
to the full 4k config space of PCIe devices. As discussed several times on
the list ([1] and [2]), we cannot really change PAPR PHB to have a true
PCIe root bus since it would call for massive and unwanted changes in
libvirt.

This series tries to address the issue with a new PCI bus class method
that tells if the PCI bus supports extended config space accesses,
instead of relying on pci_bus_is_express() which wants a PCIe root bus.
A new legacy PCI bus type is added to implement the PAPR behaviour.

Note that this fixes a potential 4.0 regression, hence the for-4.0 tag.

[1] https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07377.html
    https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02839.html

[2] https://lists.gnu.org/archive/html/qemu-ppc/2017-01/msg00034.html

--
Greg

---

Greg Kurz (2):
      pci: Allow PCI bus subtypes to support extended config space accesses
      spapr_pci: Fix extended config space accesses


 hw/pci/pci.c             |   24 ++++++++++++++++++++++++
 hw/pci/pci_host.c        |    2 +-
 hw/ppc/spapr_pci.c       |   26 +++++++++++++++++++++++++-
 include/hw/pci/pci.h     |    2 ++
 include/hw/pci/pci_bus.h |    1 +
 5 files changed, 53 insertions(+), 2 deletions(-)


Re: [Qemu-devel] [PATCH for-4.0 0/2] spapr: Fix extended config space accesses
Posted by Alex Williamson 5 years ago
On Mon, 01 Apr 2019 19:54:57 +0200
Greg Kurz <groug@kaod.org> wrote:

> Recent commit c2077e2ca0da7 added stricter checks that now prevent
> a guest to access the extended config space of a PCIe device connected
> attached to a PHB on a pseries machine.
> 
> PAPR compatible PHBs act like legacy PCI busses, but they do allow access
> to the full 4k config space of PCIe devices. As discussed several times on
> the list ([1] and [2]), we cannot really change PAPR PHB to have a true
> PCIe root bus since it would call for massive and unwanted changes in
> libvirt.
> 
> This series tries to address the issue with a new PCI bus class method
> that tells if the PCI bus supports extended config space accesses,
> instead of relying on pci_bus_is_express() which wants a PCIe root bus.
> A new legacy PCI bus type is added to implement the PAPR behaviour.
> 
> Note that this fixes a potential 4.0 regression, hence the for-4.0 tag.

Post-4.0, please also evaluate:

https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07164.html

Seems that PAPR only has one IOMMU AddressSpace per bus to return
through the iommu_fn so there should be no harm in aliasing the guest
view of devices, but if anything this series warns never to assume that
PAPR behaves normally.

FWIW, this series might make it easier to create a PCI-X Mode 2 bus,
which would otherwise be similar to conventional PCI for
inter-connectivity, but does support extended config space.  Though a
new bus type would be the more obvious way to do it, which clearly has
been a problem here.  Thanks,

Alex

Re: [Qemu-devel] [PATCH for-4.0 0/2] spapr: Fix extended config space accesses
Posted by David Gibson 5 years ago
On Mon, Apr 01, 2019 at 07:54:57PM +0200, Greg Kurz wrote:
> Recent commit c2077e2ca0da7 added stricter checks that now prevent
> a guest to access the extended config space of a PCIe device connected
> attached to a PHB on a pseries machine.
> 
> PAPR compatible PHBs act like legacy PCI busses, but they do allow access
> to the full 4k config space of PCIe devices. As discussed several times on
> the list ([1] and [2]), we cannot really change PAPR PHB to have a true
> PCIe root bus since it would call for massive and unwanted changes in
> libvirt.
> 
> This series tries to address the issue with a new PCI bus class method
> that tells if the PCI bus supports extended config space accesses,
> instead of relying on pci_bus_is_express() which wants a PCIe root bus.
> A new legacy PCI bus type is added to implement the PAPR behaviour.
> 
> Note that this fixes a potential 4.0 regression, hence the for-4.0 tag.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07377.html
>     https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02839.html
> 
> [2] https://lists.gnu.org/archive/html/qemu-ppc/2017-01/msg00034.html

So, I actually had some patches I'd been working on that address both
the c2077e2ca0da7 issue and the PAPR fixup in what I think is a
cleaner manner.  Can't remember now if I posted and they got lost, or
I didn't get around to posting.

Nonetheless, at this point we're fixing a real regression on PAPR.
I'll look at cleaning this up post 4.0.

Reviewed-by: David Gibson <david@gibson.dropbe

I can take this through my tree if people are ok with that.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [Qemu-ppc] [PATCH for-4.0 0/2] spapr: Fix extended config space accesses
Posted by David Gibson 5 years ago
On Tue, Apr 02, 2019 at 09:52:14AM +1100, David Gibson wrote:
> On Mon, Apr 01, 2019 at 07:54:57PM +0200, Greg Kurz wrote:
> > Recent commit c2077e2ca0da7 added stricter checks that now prevent
> > a guest to access the extended config space of a PCIe device connected
> > attached to a PHB on a pseries machine.
> > 
> > PAPR compatible PHBs act like legacy PCI busses, but they do allow access
> > to the full 4k config space of PCIe devices. As discussed several times on
> > the list ([1] and [2]), we cannot really change PAPR PHB to have a true
> > PCIe root bus since it would call for massive and unwanted changes in
> > libvirt.
> > 
> > This series tries to address the issue with a new PCI bus class method
> > that tells if the PCI bus supports extended config space accesses,
> > instead of relying on pci_bus_is_express() which wants a PCIe root bus.
> > A new legacy PCI bus type is added to implement the PAPR behaviour.
> > 
> > Note that this fixes a potential 4.0 regression, hence the for-4.0 tag.
> > 
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07377.html
> >     https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02839.html
> > 
> > [2] https://lists.gnu.org/archive/html/qemu-ppc/2017-01/msg00034.html
> 
> So, I actually had some patches I'd been working on that address both
> the c2077e2ca0da7 issue and the PAPR fixup in what I think is a
> cleaner manner.  Can't remember now if I posted and they got lost, or
> I didn't get around to posting.
> 
> Nonetheless, at this point we're fixing a real regression on PAPR.
> I'll look at cleaning this up post 4.0.
> 
> Reviewed-by: David Gibson <david@gibson.dropbe
> 
> I can take this through my tree if people are ok with that.

Never got a reply on this, but I see these have not yet been merged
via another path.  These fix a real regression for the pseries machine
so I've put them into my tree with the intention to send a pull
request tomorrow, even though they do largely touch generic code.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson