qemu-s390x
[Top][All Lists]
Advanced

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

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


From: Halil Pasic
Subject: Re: [qemu-s390x] [Qemu-devel] [PATCH v2] s390x/pci: Set the iommu region size mpcifc request
Date: Wed, 16 Jan 2019 15:50:11 +0100

On Wed, 16 Jan 2019 15:16:44 +0100
Pierre Morel <address@hidden> wrote:

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

@Connie, will you add the Fixes tag? Do we need a cc stable (since
broken since 2016-06-19)?

@Pierre: So you say it's a bug. What can go wrong because of this?
For example if we interpret pal as a size, I guess we could end up with
the memory region not fitting the guest memory, or? I'm still pretty
much in the dark about the implications of this bug.

Regards,
Halil




reply via email to

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