[Qemu-devel] [PATCH] spapr_pci: turn IOMMU root into an I/O region

[Qemu-devel] [PATCH] spapr_pci: turn IOMMU root into an I/O region

[Greg Kurz]

On sPAPR, virtio devices are connected to the PCI bus and use MSI-X.
Commit cc943c36faa192cd4b32af8fe5edb31894017d35 has modified MSI-X
so that writes are made using the bus master address space.
Unfortunately, the MSI-X notification hits unassigned_mem_write and
never reaches the guest... The most visible effect is that all
virtio devices are non-fonctionnal on sPAPR. :(

This patch plugs the MSI memory ops to the root IOMMU region, and
virtio devices work again. I am not sure this is the right way to
fix: please comment and enlight !

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 9ed39a9..b638a3c 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -599,8 +599,9 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
      */
     sprintf(namebuf, "%s.iommu-root", sphb->dtbusname);
 
-    memory_region_init(&sphb->iommu_root, OBJECT(sphb),
-                       namebuf, UINT64_MAX);
+    memory_region_init_io(&sphb->iommu_root, OBJECT(sphb),
+                          &spapr_msi_ops, spapr,
+                          namebuf, UINT64_MAX);
     address_space_init(&sphb->iommu_as, &sphb->iommu_root,
                        sphb->dtbusname);
 

Re: [Qemu-devel] [PATCH] spapr_pci: turn IOMMU root into an I/O region

[Alexander Graf]

On 26.08.14 18:40, Greg Kurz wrote:
> On sPAPR, virtio devices are connected to the PCI bus and use MSI-X.
> Commit cc943c36faa192cd4b32af8fe5edb31894017d35 has modified MSI-X
> so that writes are made using the bus master address space.
> Unfortunately, the MSI-X notification hits unassigned_mem_write and
> never reaches the guest... The most visible effect is that all
> virtio devices are non-fonctionnal on sPAPR. :(
> 
> This patch plugs the MSI memory ops to the root IOMMU region, and
> virtio devices work again. I am not sure this is the right way to
> fix: please comment and enlight !
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr_pci.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 9ed39a9..b638a3c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -599,8 +599,9 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>       */
>      sprintf(namebuf, "%s.iommu-root", sphb->dtbusname);
>  
> -    memory_region_init(&sphb->iommu_root, OBJECT(sphb),
> -                       namebuf, UINT64_MAX);
> +    memory_region_init_io(&sphb->iommu_root, OBJECT(sphb),
> +                          &spapr_msi_ops, spapr,
> +                          namebuf, UINT64_MAX);

This will turn all unallocated accesses in iommu_root into calls to
spapr_msi_ops, no?

Can't we instead just populate the iommu_root memory region with the MSI
subregion?


Alex

Re: [Qemu-devel] [PATCH] spapr_pci: turn IOMMU root into an I/O region

[Alexey Kardashevskiy]

On 08/27/2014 08:57 PM, Alexander Graf wrote:
> 
> 
> On 26.08.14 18:40, Greg Kurz wrote:
>> On sPAPR, virtio devices are connected to the PCI bus and use MSI-X.
>> Commit cc943c36faa192cd4b32af8fe5edb31894017d35 has modified MSI-X
>> so that writes are made using the bus master address space.
>> Unfortunately, the MSI-X notification hits unassigned_mem_write and
>> never reaches the guest... The most visible effect is that all
>> virtio devices are non-fonctionnal on sPAPR. :(
>>
>> This patch plugs the MSI memory ops to the root IOMMU region, and
>> virtio devices work again. I am not sure this is the right way to
>> fix: please comment and enlight !
>>
>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>> ---
>>  hw/ppc/spapr_pci.c |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 9ed39a9..b638a3c 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -599,8 +599,9 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>>       */
>>      sprintf(namebuf, "%s.iommu-root", sphb->dtbusname);
>>  
>> -    memory_region_init(&sphb->iommu_root, OBJECT(sphb),
>> -                       namebuf, UINT64_MAX);
>> +    memory_region_init_io(&sphb->iommu_root, OBJECT(sphb),
>> +                          &spapr_msi_ops, spapr,
>> +                          namebuf, UINT64_MAX);
> 
> This will turn all unallocated accesses in iommu_root into calls to
> spapr_msi_ops, no?

It should not, the window is small.


> Can't we instead just populate the iommu_root memory region with the MSI
> subregion?

Makes sense to me. Having one global MSI window sounds like my bug, I just
did not any good reason for having it per PHB :)



-- 
Alexey