Re: [PATCH v8 09/11] vt-d: fix the IOMMU flush issue

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

On June 14, 2016 4:27 PM, Jan Beulich <JBeulich@suse.com> wrote: 
> >>> On 14.06.16 at 10:10, <quan.xu@intel.com> wrote:
> > On June 13, 2016 11:52 PM, Jan Beulich <jbeulich@suse.com> wrote:
> >> >>> "Xu, Quan" <quan.xu@intel.com> 06/13/16 5:22 PM >>>
> >> >From: Quan Xu <quan.xu@intel.com>
> >> >@@ -546,17 +550,37 @@ static int __must_check iommu_flush_all(void)
> >> >struct acpi_drhd_unit *drhd; struct iommu *iommu; int
> >> >flush_dev_iotlb;
> >> >+    int rc = 0;
> >>  >
> >> >flush_all_cache();
> >> >for_each_drhd_unit ( drhd )
> >> >{
> >> >+        int iommu_rc, iommu_ret;
> >> >+
> >> >iommu = drhd->iommu;
> >> >-        iommu_flush_context_global(iommu, 0);
> >> >+        iommu_rc = iommu_flush_context_global(iommu, 0);
> >> >flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> >> >-        iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> >> >+        iommu_ret = iommu_flush_iotlb_global(iommu, 0,
> >> >+ flush_dev_iotlb);
> >> >+
> >> >+        /*
> >> >+         * The current logic for returns:
> >> >+         *   - positive  invoke iommu_flush_write_buffer to flush cache.
> >> >+         *   - zero      on success.
> >> >+         *   - negative  on failure. Continue to flush IOMMU IOTLB on a
> >> >+         *               best effort basis.
> >> >+         */
> >> >+        if ( iommu_rc > 0 || iommu_ret > 0 )
> >> >+            iommu_flush_write_buffer(iommu);
> >> >+        if ( rc >= 0 )
> >> >+            rc = iommu_rc;
> >> >+        if ( rc >= 0 )
> >> >+            rc = iommu_ret;
> >>
> >> First of all - is it correct to fold the two
> >> iommu_flush_write_buffer() invocations?
> >>
> >
> > Sure, it is correct..
> >
> > as:
> > - For updates to remapping hardware structures that require
> > context-cache, PASID-cache, IOTLB or IEC invalidation Operations to
> > flush stale entries from the hardware caches, no additional action is
> > required to make the modification Visible to hardware. This is
> > because, hardware performs an implicit write-buffer-flushing as a
> > pre-condition to context-cache, PASID-cache, IOTLB and IEC
> > invalidation operations.
> >
> > - For updates to remapping hardware structures (such as modifying a
> > currently not-present entry) that do not require Context-cache, IOTLB,
> > or IEC invalidations, software must explicitly perform
> > write-buffer-flushing to ensure the updated structures Are visible to
> > hardware.
> 
> But that's not the point. Instead my question was related to Kevin's concern
> towards you making assumptions on the behavior of
> iommu_flush_context_global() vs iommu_flush_iotlb_global(): What if the
> first returned 1 but the second didn't?
> It would seem to me that in such a
> (theoretical) case iommu_flush_write_buffer() might need to be invoked prior
> to the second flush function.

In this case, it is correct too.
As , hardware performs an __implicit__ write-buffer-flushing as a __pre-condition__ to IOTLB invalidation operation.
So software is no need to call iommu_flush_write_buffer() to explicitly perform write-buffer-flushing for that the first returned 1.

Quan

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