Re: [PATCH v3 3/4] iommu/vt-d: Add page request draining support

[Lu Baolu]

Hi Jacob,

On 2020/4/29 11:36, Jacob Pan wrote:
> On Wed, 22 Apr 2020 16:06:10 +0800
> Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>  wrote:
>
> > When a PASID is stopped or terminated, there can be pending PRQs
> > (requests that haven't received responses) in remapping hardware.
> > This adds the interface to drain page requests and call it when a
> > PASID is terminated.
> >
> > Signed-off-by: Jacob Pan<jacob.jun.pan@xxxxxxxxxxxxxxx>
> > Signed-off-by: Liu Yi L<yi.l.liu@xxxxxxxxx>
> > Signed-off-by: Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/iommu/intel-svm.c   | 103
> > ++++++++++++++++++++++++++++++++++-- include/linux/intel-iommu.h |
> > 4 ++ 2 files changed, 102 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > index 83dc4319f661..2534641ef707 100644
> > --- a/drivers/iommu/intel-svm.c
> > +++ b/drivers/iommu/intel-svm.c
> > @@ -23,6 +23,7 @@
> >   #include "intel-pasid.h"
> >   
> >   static irqreturn_t prq_event_thread(int irq, void *d);
> > +static void intel_svm_drain_prq(struct device *dev, int pasid);
> >   
> >   #define PRQ_ORDER 0
> >   
> > @@ -66,6 +67,8 @@ int intel_svm_enable_prq(struct intel_iommu *iommu)
> >   	dmar_writeq(iommu->reg + DMAR_PQT_REG, 0ULL);
> >   	dmar_writeq(iommu->reg + DMAR_PQA_REG,
> > virt_to_phys(iommu->prq) | PRQ_ORDER);
> > +	init_completion(&iommu->prq_complete);
> > +
> >   	return 0;
> >   }
> >   
> > @@ -208,6 +211,7 @@ static void intel_mm_release(struct mmu_notifier
> > *mn, struct mm_struct *mm) rcu_read_lock();
> >   	list_for_each_entry_rcu(sdev, &svm->devs, list) {
> >   		intel_pasid_tear_down_entry(svm->iommu, sdev->dev,
> > svm->pasid);
> > +		intel_svm_drain_prq(sdev->dev, svm->pasid);
> mmu_notifier release is called in atomic context, drain_prq needs to
> wait for completion. I tested exit path when a process crashes. I got
>
> [  +0.696214] BUG: sleeping function called from invalid context at kernel/sched/completion.c:101
> [  +0.000068] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 3235, name: dsatest
> [  +0.000046] INFO: lockdep is turned off.
> [  +0.000002] CPU: 1 PID: 3235 Comm: dsatest Not tainted 5.7.0-rc1-z-svmtest+ #1637
> [  +0.000000] Hardware name: Intel Corporation Kabylake Client platform/Skylake Halo DDR4 RVP11, BIOS
> 04.1709050855 09/05/2017
> [  +0.000001] Call Trace:
> [  +0.000004]  dump_stack+0x68/0x9b
> [  +0.000003]  ___might_sleep+0x229/0x250
> [  +0.000003]  wait_for_completion_timeout+0x3c/0x110
> [  +0.000003]  intel_svm_drain_prq+0x12f/0x210
> [  +0.000003]  intel_mm_release+0x73/0x110
> [  +0.000003]  __mmu_notifier_release+0x94/0x220
> [  +0.000002]  ? do_munmap+0x10/0x10
> [  +0.000002]  ? prepare_ftrace_return+0x5c/0x80
> [  +0.000003]  exit_mmap+0x156/0x1a0
> [  +0.000002]  ? mmput+0x44/0x120
> [  +0.000003]  ? exit_mmap+0x5/0x1a0
> [  +0.000002]  ? ftrace_graph_caller+0xa0/0xa0
> [  +0.000001]  mmput+0x5e/0x120

Thanks a lot!

Actually, we can't drain page requests in this mm_notifier code path,
right? The assumptions of page request draining are that 1) the device
driver has drained DMA requests in the device end; 2) the pasid entry
has been marked as non-present. So we could only drain page requests in
the unbind path.

Thought?

Best regards,
baolu