Re: [Xen-devel] [PATCH v2 07/11] IOMMU/MMU: propagate IOMMU Device-TLB flush error up to iommu_iotlb_flush{, _all} (top level ones).

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.04.16 at 08:21, <quan.xu@xxxxxxxxx> wrote:
> On April 26, 2016 8:49 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> Hmm, the "positive" here has nothing to do with the "positive" in patch 1.
>> Please just have a look at xenmem_add_to_physmap() as a whole.
>> 
> 
> Thanks for reminding me. The 'positive'  is ' rc = start + done'..
> 
> Think twice, I found I need two other new return values for this  function  
> (correct me if I am wrong!).
> If the first iommu_iotlb_flush() is failed, I shouldn't  accumulate the 
> return value of
> the second iommu_iotlb_flush() -- but instead properly accumulate the return 
> value.

I think just one will do.

> The following is my modification:
> 
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -640,6 +640,10 @@ static int xenmem_add_to_physmap(struct domain *d,
>      unsigned int done = 0;
>      long rc = 0;
> 
> +#ifdef CONFIG_HAS_PASSTHROUGH
> +    int ret, rv;
> +#endif

This should go into the scope below, allowing to avoid the extra
#ifdef.

> @@ -678,8 +682,15 @@ static int xenmem_add_to_physmap(struct domain *d,
>      if ( need_iommu(d) )
>      {
>          this_cpu(iommu_dont_flush_iotlb) = 0;
> -        iommu_iotlb_flush(d, xatp->idx - done, done);
> -        iommu_iotlb_flush(d, xatp->gpfn - done, done);
> +        ret = iommu_iotlb_flush(d, xatp->idx - done, done);
> +
> +        if ( rc >= 0 && ret < 0 )

Why "ret < 0" instead of just "ret"?

> +            rc = ret;
> +
> +        rv = iommu_iotlb_flush(d, xatp->gpfn - done, done);
> +
> +        if ( rc >=0 && ret == 0 && rv < 0 )
> +            rc = rv;

I don't see the middle part being needed here, eliminating the
need for "rv".

Jan


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