[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v7 09/11] vt-d: fix the IOMMU flush issue
> From: Xu, Quan > Sent: Wednesday, June 08, 2016 4:59 PM > @@ -545,18 +549,42 @@ 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 ) > { > iommu = drhd->iommu; > - 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); > + /* > + * The current logic for rc 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. > + * > + * Moreover, IOMMU flush handlers flush_context_qi and flush_iotlb_qi > + * (or flush_context_reg and flush_iotlb_reg, deep functions in the > + * call trees of iommu_flush_context_global and > iommu_flush_iotlb_global) > + * are with the same logic to bubble up positive return value. > + */ > + rc = iommu_flush_context_global(iommu, 0); > + if ( rc <= 0 ) > + { > + int flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0; > + int ret = iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb); > + > + ASSERT(ret <= 0); > + if ( !rc ) > + rc = ret; I'm dubious about the assertion here. Why can't above call return 1 upon error on earlier flush? I digged back your earlier reply like: > Yes, the iommu_flush_iotlb_dsi() can also return 1. > Look at the call tree, at the beginning of > flush_context_qi()/flush_iotlb_qi(), or > flush_context_reg()/flush_iotlb_reg().. > > If rc was negative when we call iommu_flush_context_device(), it is > impossible to return 1 for iommu_flush_iotlb_dsi(). But I don't think it a good idea of making so much assumptions about internal implementations of those low level interfaces. Also flush_context may fail for one specific reason which doesn't block flush_iotlb which could get 1 returned when caching mode is disabled. We'd better have return-1 case correctly handled here. Thanks Kevin _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |