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 11, 2016 3:17 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>> On 11.05.16 at 09:12, <quan.xu@xxxxxxxxx> wrote:
> > On May 11, 2016 3:06 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >> >>> On 11.05.16 at 08:47, <quan.xu@xxxxxxxxx> wrote:
> >> > 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.
> >>
> >> Without you telling us what exactly you checked, I can't respond to this.
> >> Extending from the reply just sent to patch 3(?) and for the
> >> avoidance of doubt, you now obviously also need to
> >> __must_check-annotate the function pointer (to match the desire of
> >> wanting to never lose such an annotation on the way back up the call tree).
> >>
> >
> > I checked -- without __must_check for iommu_flush_iotlb_page() /
> > iommu_flush_iotlb_all().
> 
> But _what_ did you check? I.e. the question isn't which functions you did your
> check with, but what behavioral checking you did.
> 
without __must_check for iommu_flush_iotlb_page() /iommu_flush_iotlb_all(), I 
can run 'make xen' successfully. Sorry.

Quan


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