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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 11.05.16 at 09:12, <quan.xu@intel.com> wrote:
> On May 11, 2016 3:06 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >>> On 11.05.16 at 08:47, <quan.xu@intel.com> wrote:
>> > On May 10, 2016 5:07 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >> >>> On 06.05.16 at 10:54, <quan.xu@intel.com> 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.

Jan


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