Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit

zhangfei.gao
2022-04-25T02:58:30Z

Hi, Dave

On 2022/4/12 下午11:35, Dave Hansen wrote:
> On 4/12/22 08:10, Jean-Philippe Brucker wrote:
>>> I wonder if the Intel and ARM IOMMU code differ in the way they keep
>>> references to the mm, or if this affects Intel as well, but we just
>>> haven't tested the code enough.
>> The Arm code was written expecting the PASID to be freed on unbind(), not
>> mm exit. I missed the change of behavior, sorry (I thought your plan was
>> to extend PASID lifetime, not shorten it?) but as is it seems very broken.
>> For example in the iommu_sva_unbind_device(), we have
>> arm_smmu_mmu_notifier_put() clearing the PASID table entry for
>> "mm->pasid", which is going to end badly if the PASID has been cleared or
>> reallocated. We can't clear the PASID entry in mm exit because at that
>> point the device may still be issuing DMA for that PASID and we need to
>> quiesce the entry rather than deactivate it.
> I think we ended up flipping some of this around on the Intel side.
> Instead of having to quiesce the device on mm exit, we don't let the mm
> exit until the device is done.
>
> When you program the pasid into the device, it's a lot like when you
> create a thread. We bump the reference count on the mm when we program
> the page table pointer into a CPU. We drop the thread's reference to
> the mm when the thread exits and will no longer be using the page tables.
>
> Same thing with pasids. We bump the refcount on the mm when the pasid
> is programmed into the device. Once the device is done with the mm, we
> drop the mm.
>
> Basically, instead of recounting the pasid itself, we just refcount the mm.
This has issue, since refcount the mm will block  fops_release to be
called, where unbind may really happen.

For example, user driver are ended unexpectedly,
usually system will end all applications via close fd -> fops_release ->
unbind may happen here.
Now mmget is called, fops_release -> unbind has NO chance to be called,
so ioasid can NOT be freed.

Thanks