Re: [PATCH v7 2/2] VT-d: Fix vt-d Device-TLB flush timeout issue

["Xu, Quan" <quan.xu@intel.com>]

(( __ sorry, I was out of office on Mon./Tues. __))

On March 21, 2016 11:27am, Tian, Kevin <kevin.tian@intel.com> wrote:
> > From: Xu, Quan
> > Sent: Friday, March 18, 2016 8:22 PM
> > > >  static void queue_invalidate_iec(struct iommu *iommu, u8 granu,
> > > > u8 im, u16 iidx)  {
> > > >      unsigned long flags;
> > > > @@ -342,8 +393,6 @@ static int flush_iotlb_qi(
> > > >
> > > >      if ( qi_ctrl->qinval_maddr != 0 )
> > > >      {
> > > > -        int rc;
> > > > -
> > > >          /* use queued invalidation */
> > > >          if (cap_write_drain(iommu->cap))
> > > >              dw = 1;
> > > > @@ -353,11 +402,17 @@ static int flush_iotlb_qi(
> > > >          queue_invalidate_iotlb(iommu,
> > > >                                 type >>
> > > DMA_TLB_FLUSH_GRANU_OFFSET, dr,
> > > >                                 dw, did, size_order, 0, addr);
> > > > +
> > > > +        /*
> > > > +         * Before Device-TLB invalidation we need to synchronize
> > > > +         * invalidation completions with hardware.
> > > > +         */
> > > > +        ret = invalidate_sync(iommu);
> > > > +        if ( ret )
> > > > +             return ret;
> > > > +
> > > >          if ( flush_dev_iotlb )
> > > >              ret = dev_invalidate_iotlb(iommu, did, addr, size_order,
> type);
> > > > -        rc = invalidate_sync(iommu);
> > > > -        if ( !ret )
> > > > -            ret = rc;
> > >
> > > Current change looks not consistent. For IOMMU iotlb flush, we have
> > > invalidate_sync out of invalidate operation, however below...
> > >
> >
> > Now, does it still look not consistent?
> >
> 
> Yes, still inconsistent. As I said, you put invalidation sync within
> dev_invalidate_iotlb, while for all other IOMMU invalidations the sync is put
> after. Below would be consistent then:
> 
>         if ( flush_dev_iotlb )
>             ret = dev_invalidate_iotlb(iommu, did, addr, size_order, type);
>         rc = dev_invalidate_iotlb_sync(...);
>         if ( !ret )
>             ret = rc;
> 

Make sense, I will fix it in next v8.
Now this patch set looks clear, I'd better summarize and send out v8. Of course, I would continue to
explain and fix the prereq patch set.


Quan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel