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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Xu, Quan
> Sent: Wednesday, March 23, 2016 11:30 AM
> 
> >
> > 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;
> >
>   Kevin,
>   now I doubt that I should put invalidation sync within 
> dev_invalidate_iotlb, which was also
> your suggestion.
> As the dev_invalidate_iotlb() is invalidation for all of domain's ATS 
> devices. If in this
> consistent way, we couldn't
> Find which ATS device flush timed out, then we need to hide all of domain's 
> ATS devices.
> Do you recall it?
>   Also I think it is reluctant to put invalidate_sync within 
> queue_invalidate_iotlb() for
> consistent issue.
> Quan

Yes I recall this story.

What about doing this? Let's wrap a _sync version for all flush interfaces, 
like below:

static int queue_invalidate_context_sync(...)
{
        queue_invalidate_context(...);
        return invalidate_sync(...);
}

Then invoke _sync version at all callers, e.g.:
static int flush_context_qi(...)
{
        ...
        if ( qi_ctrl->qinval_maddr != 0 )
                ret = queue_invalidate_context_sync(...);
}

similarly we'll have dev_invalidate_iotlb_sync for device IOTLB flush.

It simplifies caller logic and make code more readable. :-)

Thanks
Kevin

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