qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] s390x/pci: Set the iommu region size mpcifc


From: Pierre Morel
Subject: Re: [Qemu-devel] [PATCH v2] s390x/pci: Set the iommu region size mpcifc request
Date: Wed, 16 Jan 2019 15:16:44 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

On 16/01/2019 13:40, Halil Pasic wrote:
On Tue, 15 Jan 2019 10:35:42 -0500
Collin Walling <address@hidden> wrote:

On 1/10/19 8:00 AM, Pierre Morel wrote:
The size of the accessible iommu memory region in the guest
is given to the IOMMU by the guest through the mpcifc request
specifying the PCI Base Address and the PCI Address Limit.

Let set the size of the IOMMU region to:
      (PCI Address Limit) - (PCI Base Address) + 1.

Signed-off-by: Pierre Morel <address@hidden>
---
   hw/s390x/s390-pci-bus.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 69e0671..e97696a 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -660,7 +660,7 @@ void s390_pci_iommu_enable(S390PCIIOMMU *iommu)
       char *name = g_strdup_printf("iommu-s390-%04x", iommu->pbdev->uid);
       memory_region_init_iommu(&iommu->iommu_mr, sizeof(iommu->iommu_mr),
                                TYPE_S390_IOMMU_MEMORY_REGION, 
OBJECT(&iommu->mr),
-                             name, iommu->pal + 1);
+                             name, iommu->pal - iommu->pba + 1);

 From the the look of this, I would say we basically used the address
denoting the end of the region as the size of the region. This smells
like a bug to me, but the commit message and the title ain't clear about
this, and there is no fixes tag. Because of the latter I did some digging
and came to commit f7c40aa "s390x/pci: fix failures of dma
map/unmap" (Yi Min Zhao, 2016-06-19) which basically did the inverse of
this commit!

My initial motivation was to check if this is stable material. But now
I'm very confused. I'm admittedly zPCI incompetent. Could some of the
people that understand what is going on help me feel better about this
patch?

Regards,
Halil


The patch you speak about corrected the problem described in its comment by setting the offset address of the subregion to 0, making sure VFIO_PCI works for Z but introduced a bug we did not see at that time by making the subregion too large.

This patch correct the bug, I can add a reference to this with:
fixing: commit f7c40aa1e7feb50bc4d4bc171fa811bdd9a93e51

Regards,
Pierre




       iommu->enabled = true;
       memory_region_add_subregion(&iommu->mr, 0, 
MEMORY_REGION(&iommu->iommu_mr));
       g_free(name);


Acked-by: Collin Walling <address@hidden>





--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




reply via email to

[Prev in Thread] Current Thread [Next in Thread]