RE: [PATCH 1/3] iommu/vt-d: Missing checks for pasid tables if allocation fails

From: Liu, Yi L
Date: Fri Oct 20 2017 - 02:35:18 EST




> -----Original Message-----
> From: Lu Baolu [mailto:baolu.lu@xxxxxxxxxxxxxxx]
> Sent: Friday, October 20, 2017 8:49 AM
> To: Liu, Yi L <yi.l.liu@xxxxxxxxx>; joro@xxxxxxxxxx; dwmw2@xxxxxxxxxxxxx
> Cc: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/3] iommu/vt-d: Missing checks for pasid tables if allocation
> fails
>
> Hi Yi,
>
> On 10/19/2017 02:40 PM, Liu, Yi L wrote:
> >
> >> -----Original Message-----
> >> From: iommu-bounces@xxxxxxxxxxxxxxxxxxxxxxxxxx [mailto:iommu-
> >> bounces@xxxxxxxxxxxxxxxxxxxxxxxxxx] On Behalf Of Lu Baolu
> >> Sent: Thursday, October 19, 2017 8:39 AM
> >> To: joro@xxxxxxxxxx; dwmw2@xxxxxxxxxxxxx
> >> Cc: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> >> Subject: [PATCH 1/3] iommu/vt-d: Missing checks for pasid tables if
> >> allocation fails
> >>
> >> intel_svm_alloc_pasid_tables() might return an error but never be
> >> checked by the callers. Later when intel_svm_bind_mm() is called,
> >> there are no checks for valid pasid tables before enabling them.
> >>
> >> Signed-off-by: Ashok Raj <ashok.raj@xxxxxxxxx>
> >> Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> >> ---
> >> drivers/iommu/intel-svm.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> >> index f6697e5..43280ca 100644
> >> --- a/drivers/iommu/intel-svm.c
> >> +++ b/drivers/iommu/intel-svm.c
> >> @@ -292,7 +292,7 @@ int intel_svm_bind_mm(struct device *dev, int
> >> *pasid, int flags, struct svm_dev_
> >> int pasid_max;
> >> int ret;
> >>
> >> - if (WARN_ON(!iommu))
> >> + if (WARN_ON(!iommu || !iommu->pasid_table))
> > [Liu, Yi L] Hi Baolu, I guess there also need a check to iommu->ecap ,
> > see if the pasid bit is reported. thoughts?
> >
>
> If pasid bit is not set in ecap register, iommu->pasid_table won't be set.
>
> We did this by:
>
> if (pasid_enabled(iommu))
> intel_svm_alloc_pasid_tables(iommu);

[Liu, Yi L] Sounds good. thx.

Reviewed-by: Liu, Yi L <yi.l.liu@xxxxxxxxx>

Regards,
Yi L