Re: [Xen-devel] [Patch v6 04/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU mapping (top level ones)

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.06.16 at 09:51, <quan.xu@xxxxxxxxx> wrote:
> I still have one question. If I add __must_check annotation to 
> iommu_unmap_page().
> How to fix this -- unmapping the previous IOMMU maps when IOMMU mapping is 
> failed..
> e.g.,
> 
> ept_set_entry()
> {
> ...
>                 for ( i = 0; i < (1 << order); i++ )
>                 {
>                     rc = iommu_map_page(d, gfn + i, mfn_x(mfn) + i, 
> iommu_flags);
>                     if ( unlikely(rc) )
>                     {
>                         while ( i-- )
>                             iommu_unmap_page(p2m->domain, gfn + i);
> 
>                         break;
>                     }
>                 }
> ...
> }
> 
> 
> If we leave it as is, it leads to compilation errors as __must_check 
> annotation. Also we return the first error, so we are no need to cumulate the 
> error of iommu_unmap_page().
> That's also why I hesitated to add __must_check annotation to 
> iommu_unmap_page().

Well, with there already being a message logged down the call
tree in case of error, I think the return value should simply be
latched into a local variable, and nothing really be done with it
(which should satisfy the compiler afaict, but it may be that
Coverity would grumble about such). We really don't want to omit
the __must_check just because there are a few cases where the
error is of no further relevance; the annotation gets put there to
make sure we catch all (current and future) cases where errors
must not be ignored.

Jan


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