Hi Eric > -----Original Message----- > From: Auger Eric [mailto:eric.auger@xxxxxxxxxx] > Sent: 05 July 2019 13:10 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; > Alex Williamson <alex.williamson@xxxxxxxxxx> > Cc: 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 > > 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. Ok. Please go ahead. Thanks, Shameer
