RE: [PATCH 6/9] VT-d: don't lose errors when flushing TLBs on multiple IOMMUs

From: "Tian, Kevin" <kevin.tian@intel.com>
To: Jan Beulich <jbeulich@suse.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Paul Durrant <paul@xen.org>,
	"Cooper, Andrew" <andrew.cooper3@citrix.com>
Subject: RE: [PATCH 6/9] VT-d: don't lose errors when flushing TLBs on multiple IOMMUs
Date: Thu, 24 Jun 2021 05:28:44 +0000	[thread overview]
Message-ID: <MWHPR11MB18868E51563CA138A8AC660A8C079@MWHPR11MB1886.namprd11.prod.outlook.com> (raw)
In-Reply-To: <8a4f355e-c381-412c-8949-061851d0f7e2@suse.com>

> From: Jan Beulich
> Sent: Wednesday, June 9, 2021 5:29 PM
> 
> While no longer an immediate problem with flushes no longer timing out,
> errors (if any) get properly reported by iommu_flush_iotlb_{dsi,psi}().
> Overwriting such an error with, perhaps, a success indicator received
> from another IOMMU will misguide callers. Record the first error, but
> don't bail from the loop (such that further necessary invalidation gets
> carried out on a best effort basis).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -643,7 +643,7 @@ static int __must_check iommu_flush_iotl
>      struct vtd_iommu *iommu;
>      bool_t flush_dev_iotlb;
>      int iommu_domid;
> -    int rc = 0;
> +    int ret = 0;
> 
>      /*
>       * No need pcideves_lock here because we have flush
> @@ -651,6 +651,8 @@ static int __must_check iommu_flush_iotl
>       */
>      for_each_drhd_unit ( drhd )
>      {
> +        int rc;
> +
>          iommu = drhd->iommu;
> 
>          if ( !test_bit(iommu->index, &hd->arch.vtd.iommu_bitmap) )
> @@ -673,13 +675,12 @@ static int __must_check iommu_flush_iotl
>                                         flush_dev_iotlb);
> 
>          if ( rc > 0 )
> -        {
>              iommu_flush_write_buffer(iommu);
> -            rc = 0;
> -        }
> +        else if ( !ret )
> +            ret = rc;
>      }
> 
> -    return rc;
> +    return ret;
>  }
> 
>  static int __must_check iommu_flush_iotlb_pages(struct domain *d,
>