Re: [Xen-devel] [PATCH v5 1/7] VT-d: Check VT-d Device-TLB flush error(IOMMU part).

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

> On February 16, 2016 7:06pm, <JBeulich@xxxxxxxx> wrote:
> >>> On 16.02.16 at 11:50, <quan.xu@xxxxxxxxx> wrote:
> >>  On February 11, 2016 at 1:01am, <JBeulich@xxxxxxxx> wrote:
> >> >>> On 05.02.16 at 11:18, <quan.xu@xxxxxxxxx> wrote:
 
> >> > @@ -369,12 +376,16 @@ void iommu_share_p2m_table(struct domain*
> d)
> >> >          ops->share_p2m(d);
> >> >  }
> >> >
> >> > -void iommu_crash_shutdown(void)
> >> > +int iommu_crash_shutdown(void)
> >> >  {
> >> >      const struct iommu_ops *ops = iommu_get_ops();
> >> > +
> >> >      if ( iommu_enabled )
> >> > -        ops->crash_shutdown();
> >> > +        return ops->crash_shutdown();
> >> > +
> >> >      iommu_enabled = iommu_intremap = 0;
> >> > +
> >> > +    return 0;
> >> >  }
> >>
> >> Here again the question is - what is the error value going to be used
> >> for? We're trying to shut down a crashed system when coming here.
> >>
> >  I tried to clean up in error handling path chained up. It logs an
> > error message,  When it calls iommu_crash_shutdown() and returns a
> > non-zero value [in patch 2/7].
> 
> That sounds okay than (I didn't get around to look at patches 2-7 yet), but is
> somewhat contrary to me request of adding __must_check as far as possible,
> which - if done here - would break the build without also adjusting the 
> caller(s).
> 


If they are in the same patch set, I think it is acceptable. 
BTW, with patch 1/7, I can build Xen successfully( make xen ).
To align this rule, I'd better merge patch1/7 and patch 2/7 into a large patch.

Quan



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