Re: [Qemu-devel] [Qemu-stable] [PATCH 06/25] pci: Use bus master address space for delivering MSI/MSI-X messages

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

On Thu, 28 Aug 2014 09:39:30 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> Quoting Greg Kurz (2014-08-28 05:18:06)
> > On Wed, 27 Aug 2014 12:36:03 -0500
> > Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > > From: Jan Kiszka <jan.kiszka@siemens.com>
> > > 
> > > The spec says (and real HW confirms this) that, if the bus master bit
> > > is 0, the device will not generate any PCI accesses. MSI and MSI-X
> > > messages fall among these, so we should use the corresponding address
> > > space to deliver them. This will prevent delivery if bus master support
> > > is disabled.
> > > 
> > > Cc: qemu-stable@nongnu.org
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > (cherry picked from commit cc943c36faa192cd4b32af8fe5edb31894017d35)
> > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > > ---
> > 
> > Hi Michael,
> > 
> > This commit breaks virtio for spapr. A fix is available in Alex's ppc-next:
> > 
> > From: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > spapr_pci: map the MSI window in each PHB 
> > 
> > https://github.com/agraf/qemu/commit/7af2868fc76857e19c232bbe9e99a95d61138213
> 
> Thanks, I've gone ahead and applied it. Tested virtio + pseries before/after
> and it seems to fix the issue.
> 

Michael,

We have a problem again with pseries guests...

Since commit cc943c36faa192cd4b32af8fe5edb31894017d35 was not applied
to stable, virtio PCI devices notify through the global address space.
Unfortunately, commit 7af2868fc76857e19c232bbe9e99a95d61138213 removes
the MSI window from the global address space... 

The result is that 2.1.1 is broken for pseries using virtio... Reverting
0824ca6bd126b97fac18d12d514823463f415bec from stable fixes the issue.
I guess we should do that and try again when we have it all fixed in
master.

Sorry for not having anticipated that for 2.1.1... :-\

Cheers.

--
Greg

> > 
> > Cheers.
> > 
> > --
> > Greg
> > 
> > >  hw/pci/msi.c  | 2 +-
> > >  hw/pci/msix.c | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/pci/msi.c b/hw/pci/msi.c
> > > index a4a3040..52d2313 100644
> > > --- a/hw/pci/msi.c
> > > +++ b/hw/pci/msi.c
> > > @@ -291,7 +291,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
> > >                     "notify vector 0x%x"
> > >                     " address: 0x%"PRIx64" data: 0x%"PRIx32"\n",
> > >                     vector, msg.address, msg.data);
> > > -    stl_le_phys(&address_space_memory, msg.address, msg.data);
> > > +    stl_le_phys(&dev->bus_master_as, msg.address, msg.data);
> > >  }
> > > 
> > >  /* Normally called by pci_default_write_config(). */
> > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> > > index 5c49bfc..20ae476 100644
> > > --- a/hw/pci/msix.c
> > > +++ b/hw/pci/msix.c
> > > @@ -439,7 +439,7 @@ void msix_notify(PCIDevice *dev, unsigned vector)
> > > 
> > >      msg = msix_get_message(dev, vector);
> > > 
> > > -    stl_le_phys(&address_space_memory, msg.address, msg.data);
> > > +    stl_le_phys(&dev->bus_master_as, msg.address, msg.data);
> > >  }
> > > 
> > >  void msix_reset(PCIDevice *dev)
> > 
> > 
> > 
> > -- 
> > Gregory Kurz                                     kurzgreg@fr.ibm.com
> >                                                  gkurz@linux.vnet.ibm.com
> > Software Engineer @ IBM/Meiosys                  http://www.ibm.com
> > Tel +33 (0)562 165 496
> > 
> > "Anarchy is about taking complete responsibility for yourself."
> >         Alan Moore.