Re: [Qemu-devel] [PATCH] spapr-pci: remove io ports workaround

[Greg Kurz <gkurz@linux.vnet.ibm.com>]

On Tue, 10 Dec 2013 13:43:05 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 12/10/2013 03:33 AM, Greg Kurz wrote:
> > In the past, IO space could not be mapped into the memory address space
> > so we introduced a workaround for that. Nowadays it does not look
> > necessary so we can remove the workaround and make sPAPR PCI
> > configuration simplier.
> > 
> > This workaround has also an evil side effect with virtio devices:
> > because all PHBs have their .io region at the same address, the devices
> > get mapped in the .io-alias region of every PHB (AKA. mapped multiple
> > times). This breaks the ioeventfd feature and causes qemu to abort()
> > when running with KVM and asking for more than one PHB:
> > 
> > $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
> >   -hda /local/greg/images/fedora-be.qcow2 \
> >   -device
> > virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
> > -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
> > spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
> > ioeventfd: File exists Aborted
> > 
> > This will prevent to use virtio and VFIO passthrough at the same time,
> > since VFIO needs a dedicated PHB to work on ppc.
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> 
> I have not seen this version yet so please remove me from "SOB". The patch
> you replied to was eventually reworked and went to upstream as
> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
> 

Hi Alex,

I agree you have not seen this version yet... The patch I replied to was my
primary source of inspiration and contains these bits, hence the SOB. 
Anyway, the SOB is now removed until you decide to add one yourself. :)

> This one might be correct too but I want to try this first :)
> 

Well, I hope it is. Please try it.

Thanks for the feedback.

Cheers.

--
Greg

> 
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr_pci.c |   13 ++-----------
> >  1 file changed, 2 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 7763149..7d29431 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -564,23 +564,14 @@ static int spapr_phb_init(SysBusDevice *s)
> >      memory_region_add_subregion(get_system_memory(),
> > sphb->mem_win_addr, &sphb->memwindow);
> >  
> > -    /* On ppc, we only have MMIO no specific IO space from the CPU
> > -     * perspective.  In theory we ought to be able to embed the PCI IO
> > -     * memory region direction in the system memory space.  However,
> > -     * if any of the IO BAR subregions use the old_portio mechanism,
> > -     * that won't be processed properly unless accessed from the
> > -     * system io address space.  This hack to bounce things via
> > -     * system_io works around the problem until all the users of
> > -     * old_portion are updated */
> > +    /* Initialize IO regions */
> >      sprintf(namebuf, "%s.io", sphb->dtbusname);
> >      memory_region_init(&sphb->iospace, OBJECT(sphb),
> >                         namebuf, SPAPR_PCI_IO_WIN_SIZE);
> > -    /* FIXME: fix to support multiple PHBs */
> > -    memory_region_add_subregion(get_system_io(), 0, &sphb->iospace);
> >  
> >      sprintf(namebuf, "%s.io-alias", sphb->dtbusname);
> >      memory_region_init_alias(&sphb->iowindow, OBJECT(sphb), namebuf,
> > -                             get_system_io(), 0, 
SPAPR_PCI_IO_WIN_SIZE);
> > +                             &sphb->iospace, 0, SPAPR_PCI_IO_WIN_SIZE);
> >      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
> >                                  &sphb->iowindow);
> >      /*
> > 
> 
>