Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] pseries: Add partial support for PCI

[David Gibson]

On Sun, Oct 30, 2011 at 06:06:51PM +0100, Alexander Graf wrote:
> 
> On 28.10.2011, at 03:56, David Gibson wrote:
> 
> > From: Alexey Kardashevskiy <address@hidden>
> > 
> > This patch adds a PCI bus to the pseries machine.  This instantiates
> > the qemu generic PCI bus code, advertises a PCI host bridge in the
> > guest's device tree and implements the RTAS methods specified by PAPR
> > to access PCI config space.  It also sets up the memory regions we
> > need to provide windows into the PCI memory and IO space, and
> > advertises those to the guest.
> > 
> > However, because qemu can't yet emulate an IOMMU, which is mandatory on
> > pseries, PCI devices which use DMA (i.e. most of them) will not work with
> > this code alone.  Still, this is enough to support the virtio_pci device
> > (which probably _should_ use emulated PCI DMA, but is specced to use
> > direct hypervisor access to guest physical memory instead).
> > 
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> > Makefile.target |    3 +
> > hw/spapr.c      |   36 ++++-
> > hw/spapr.h      |    2 +
> > hw/spapr_pci.c  |  515 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > hw/spapr_pci.h  |   59 +++++++
> > 5 files changed, 611 insertions(+), 4 deletions(-)
> > create mode 100644 hw/spapr_pci.c
> > create mode 100644 hw/spapr_pci.h
> > 
> > diff --git a/Makefile.target b/Makefile.target
> > index fe5f6f7..f3eb842 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -248,6 +248,9 @@ obj-ppc-y += ppc_newworld.o
> > # IBM pSeries (sPAPR)
> > obj-ppc-$(CONFIG_PSERIES) += spapr.o spapr_hcall.o spapr_rtas.o spapr_vio.o
> > obj-ppc-$(CONFIG_PSERIES) += xics.o spapr_vty.o spapr_llan.o spapr_vscsi.o
> > +ifeq ($(CONFIG_PCI),y)
> > +obj-ppc-$(CONFIG_PSERIES) += spapr_pci.o device-hotplug.o pci-hotplug.o
> > +endif
> 
> You make it conditional here ...
> 
> > # PowerPC 4xx boards
> > obj-ppc-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
> > obj-ppc-y += ppc440.o ppc440_bamboo.o
> > diff --git a/hw/spapr.c b/hw/spapr.c
> > index 933af32..bdaa938 100644
> > --- a/hw/spapr.c
> > +++ b/hw/spapr.c
> > @@ -39,10 +39,12 @@
> > 
> > #include "hw/spapr.h"
> > #include "hw/spapr_vio.h"
> > +#include "hw/spapr_pci.h"
> > #include "hw/xics.h"
> > 
> > #include "kvm.h"
> > #include "kvm_ppc.h"
> > +#include "pci.h"
> 
> ... but not here. Just throw away the condition above. We don't need
> to support -M pseries without PCI.

Good point, revision in next version.

[snip]
> > +    spapr_rtas_register("read-pci-config", rtas_read_pci_config);
> > +    spapr_rtas_register("write-pci-config", rtas_write_pci_config);
> > +    spapr_rtas_register("ibm,read-pci-config", rtas_ibm_read_pci_config);
> > +    spapr_rtas_register("ibm,write-pci-config", rtas_ibm_write_pci_config);
> > +
> > +    /*
> > +     * This is a workaround to disable PCI devices resetting as we do
> > +     * BAR allocation on the QEMU side and reset destroys this
> > +     * configuration.
> > +     */
> > +    bus->qbus.info->reset = NULL;
> 
> Eh. What? So you're breaking reset for all PCI devices? How do you
> reboot?

Oops.  That leaked in fromt he patch which adds PCI BAR allocation
(it's a workaround for the reset clobbering our allocations which we
haven't had a chance to fix properly yet).  Put back where it belongs
now.

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