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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.05.16 at 09:20, <quan.xu@xxxxxxxxx> wrote:
> 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.

Oh, sure you can. Leaving the annotation out will always result in
no more (and likely less) diagnostics. I.e. that's not a valid criteria.

Jan


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