Re: [Xen-devel] [PATCH v4 04/10] vt-d: propagate IOMMU Device-TLB flush error up to IOMMU unmapping.

[Xu, Quan]

On May 10, 2016 4:51 PM, Jan Beulich  wrote:
> >>> On 06.05.16 at 10:54,  wrote:
> > Propagate the IOMMU Device-TLB flush error up to IOMMU unmapping.
> >
> > Signed-off-by: Quan Xu 
> > Acked-by: Kevin Tian 
> 
> Reviewed-by: Jan Beulich 
> 
> but note ...
> 
> > @@ -1766,9 +1769,7 @@ static int intel_iommu_unmap_page(struct
> domain *d, unsigned long gfn)
> >  if ( iommu_passthrough && is_hardware_domain(d) )
> >  return 0;
> >
> > -dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
> > -
> > -return 0;
> > +return dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
> >  }
> 
> ... how you lose the __must_check here, since
> intel_iommu_unmap_page() isn't __must_check (which we said you may skip
> as long as the common code wrapper has it, but in the context here I'm no
> longer convinced skipping this at any layer is a good idea, as that makes
> validation of the call trees more difficult).
> (This is just a remark regarding the comment on the earlier patch, i.e. not
> something needing any further change here.)
> 


I'll be bold to add __must_check..
Quan


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

Re: [Xen-devel] [PATCH v4 04/10] vt-d: propagate IOMMU Device-TLB flush error up to IOMMU unmapping.

[Jan Beulich]

>>> On 06.05.16 at 10:54,  wrote:
> Propagate the IOMMU Device-TLB flush error up to IOMMU unmapping.
> 
> Signed-off-by: Quan Xu 
> Acked-by: Kevin Tian 

Reviewed-by: Jan Beulich 

but note ...

> @@ -1766,9 +1769,7 @@ static int intel_iommu_unmap_page(struct domain *d, 
> unsigned long gfn)
>  if ( iommu_passthrough && is_hardware_domain(d) )
>  return 0;
>  
> -dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
> -
> -return 0;
> +return dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
>  }

... how you lose the __must_check here, since
intel_iommu_unmap_page() isn't __must_check (which we said you
may skip as long as the common code wrapper has it, but in the
context here I'm no longer convinced skipping this at any layer is a
good idea, as that makes validation of the call trees more difficult).
(This is just a remark regarding the comment on the earlier patch,
i.e. not something needing any further change here.)

Jan


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