Hi Shameer, On 7/4/19 2:51 PM, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On >> Behalf Of Alex Williamson >> Sent: 03 July 2019 21:34 >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx> >> Cc: eric.auger@xxxxxxxxxx; pmorel@xxxxxxxxxxxxxxxxxx; >> kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; Linuxarm <linuxarm@xxxxxxxxxx>; John >> Garry <john.garry@xxxxxxxxxx>; xuwei (O) <xuwei5@xxxxxxxxxx>; >> kevin.tian@xxxxxxxxx >> Subject: Re: [PATCH v7 2/6] vfio/type1: Check reserve region conflict and >> update iova list >> >> On Wed, 26 Jun 2019 16:12:44 +0100 >> Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx> wrote: >> >>> This retrieves the reserved regions associated with dev group and >>> checks for conflicts with any existing dma mappings. Also update >>> the iova list excluding the reserved regions. >>> >>> Reserved regions with type IOMMU_RESV_DIRECT_RELAXABLE are >>> excluded from above checks as they are considered as directly >>> mapped regions which are known to be relaxable. >>> >>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx> >>> --- >>> drivers/vfio/vfio_iommu_type1.c | 96 >> +++++++++++++++++++++++++++++++++ >>> 1 file changed, 96 insertions(+) >>> >>> diff --git a/drivers/vfio/vfio_iommu_type1.c >> b/drivers/vfio/vfio_iommu_type1.c >>> index 970d1ec06aed..b6bfdfa16c33 100644 >>> --- a/drivers/vfio/vfio_iommu_type1.c >>> +++ b/drivers/vfio/vfio_iommu_type1.c >>> @@ -1559,6 +1641,7 @@ static int vfio_iommu_type1_attach_group(void >> *iommu_data, >>> phys_addr_t resv_msi_base; >>> struct iommu_domain_geometry geo; >>> LIST_HEAD(iova_copy); >>> + LIST_HEAD(group_resv_regions); >>> >>> mutex_lock(&iommu->lock); >>> >>> @@ -1644,6 +1727,13 @@ static int vfio_iommu_type1_attach_group(void >> *iommu_data, >>> goto out_detach; >>> } >>> >>> + iommu_get_group_resv_regions(iommu_group, &group_resv_regions); >> >> This can fail and should have an error case. I assume we'd fail the >> group attach on failure. Thanks, > > Right. I will add the check. Do you think we should do the same in vfio_iommu_has_sw_msi() > as well? (In fact, it looks like iommu_get_group_resv_regions() ret is not checked anywhere in > kernel). I think the can be the topic of another series. I just noticed that in iommu_insert_resv_region(), which is recursive in case ot merge, I failed to propagate returned value or recursive calls. This also needs to be fixed. I volunteer to work on those changes if you prefer. Just let me know. Thanks Eric > > Thanks, > Shameer > >
