Re: [Xen-devel] [PATCH v4 07/10] IOMMU: propagate IOMMU Device-TLB flush error up to iommu_iotlb_flush{, _all} (leaf ones).

["Xu, Quan" <quan.xu@xxxxxxxxx>]

On May 10, 2016 5:07 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>> On 06.05.16 at 10:54, <quan.xu@xxxxxxxxx> wrote:
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -604,15 +604,15 @@ static int iommu_flush_iotlb(struct domain *d,
> unsigned long gfn,
> >      return rc;
> >  }
> >
> > -static void iommu_flush_iotlb_page(struct domain *d, unsigned long gfn,
> > -                                   unsigned int page_count)
> > +static int iommu_flush_iotlb_page(struct domain *d, unsigned long gfn,
> > +                                  unsigned int page_count)
> >  {
> > -    iommu_flush_iotlb(d, gfn, 1, page_count);
> > +    return iommu_flush_iotlb(d, gfn, 1, page_count);
> >  }
> >
> > -static void iommu_flush_iotlb_all(struct domain *d)
> > +static int iommu_flush_iotlb_all(struct domain *d)
> >  {
> > -    iommu_flush_iotlb(d, INVALID_GFN, 0, 0);
> > +    return iommu_flush_iotlb(d, INVALID_GFN, 0, 0);
> >  }
> 
> As already indicated in a reply to an earlier patch, despite what was said on 
> the
> earlier version I think we should have __must_check here

If the static one is initialized for .callback, is it really necessary to add 
__must_check here? 
I check it with compiler, and it is ok when I didn't add __must_check here.

If yes, I'll add __must_check for ARM one as well.

> and ...
> 
> > --- a/xen/include/xen/iommu.h
> > +++ b/xen/include/xen/iommu.h
> > @@ -179,8 +179,8 @@ struct iommu_ops {
> >      void (*resume)(void);
> >      void (*share_p2m)(struct domain *d);
> >      void (*crash_shutdown)(void);
> > -    void (*iotlb_flush)(struct domain *d, unsigned long gfn, unsigned int
> page_count);
> > -    void (*iotlb_flush_all)(struct domain *d);
> > +    int (*iotlb_flush)(struct domain *d, unsigned long gfn, unsigned int
> page_count);
> > +    int (*iotlb_flush_all)(struct domain *d);
> 
> ... here.

Yes,  it is necessary here.

Quan

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