On Thu, 2016-03-24 at 14:56 +0100, Dario Faggioli wrote:
> On Thu, 2016-03-24 at 13:57 +0800, Quan Xu wrote:
> > 
> > @@ -134,8 +133,8 @@ int dev_invalidate_iotlb(struct iommu *iommu,
> > u16
> > did,
> >              /* invalidate all translations:
> > sbit=1,bit_63=0,bit[62:12]=1 */
> >              sbit = 1;
> >              addr = (~0UL << PAGE_SHIFT_4K) & 0x7FFFFFFFFFFFFFFF;
> > -            rc = qinval_device_iotlb(iommu, pdev->ats_queue_depth,
> > -                                     sid, sbit, addr);
> > +            ret = qinval_device_iotlb_sync(iommu, pdev-
> > > 
> > > ats_queue_depth,
> > +                                           sid, sbit, addr);
> >              break;
> >          case DMA_TLB_PSI_FLUSH:
> >              if ( !device_in_domain(iommu, pdev, did) )
> > @@ -154,16 +153,13 @@ int dev_invalidate_iotlb(struct iommu *iommu,
> > u16 did,
> >                  addr |= (((u64)1 << (size_order - 1)) - 1) <<
> > PAGE_SHIFT_4K;
> >              }
> >  
> > -            rc = qinval_device_iotlb(iommu, pdev->ats_queue_depth,
> > -                                     sid, sbit, addr);
> > +            ret = qinval_device_iotlb_sync(iommu, pdev-
> > > 
> > > ats_queue_depth,
> > +                                           sid, sbit, addr);
> >              break;
> >          default:
> >              dprintk(XENLOG_WARNING VTDPREFIX, "invalid vt-d flush
> > type\n");
> >              return -EOPNOTSUPP;
> >          }
> > -
> > -        if ( !ret )
> > -            ret = rc;
> >      }
> >  
> >      return ret;
> > 
> Am I misreading something or we are introducing synchronous handling,
> which was not there before?
> 
> If yes, is it ok to do this in this patch?
> 
> And if yes again, I think that it at least should be noted in the
> changelog, as it would mean that the patch is not only introducing
> some
> wrappers.
> 
Ok, I think I see what's happening here. Before this patch,
invalidate_sync() was being called inside qinval_device_iotlb(), so we
were synchronous already, and we need to continue to be like that, by
calling the _sync() variants.

Yes, if this is what happens, this also looks ok to me.

Sorry for the noise.

Regards,
Dario

> Regads,
> Dario
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)