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

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

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.



> And then the variable naming is strange - both operations are IOMMU ones,
> so prefixing the variables with iommu_ doesn't help much here. How about
> ctxt_rc and iotlb_rc or some such?
> 

To align to that file, I'm better using context_rc  and iotlb_rc..

Quan

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