Re: [Xen-devel] [PATCH v2 1/2] VT-d: Reduce spin timeout to 1ms, which can be boot-time changed.

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

>On 11.12.2015 at 6:01pm, <JBeulich@xxxxxxxx> wrpte:
> >>> On 10.12.15 at 10:33, <quan.xu@xxxxxxxxx> wrote:
> > @@ -167,10 +172,12 @@ static int queue_invalidate_wait(struct iommu
> *iommu,
> >          start_time = NOW();
> >          while ( poll_slot != QINVAL_STAT_DONE )
> >          {
> > -            if ( NOW() > (start_time + DMAR_OPERATION_TIMEOUT) )
> > +            if ( NOW() > (start_time + IOMMU_QI_TIMEOUT) )
> >              {
> >                  print_qi_regs(iommu);
> > -                panic("queue invalidate wait descriptor was not
> executed");
> > +                dprintk(XENLOG_WARNING VTDPREFIX,
> > +                        "Queue invalidate wait descriptor was
> timeout.\n");
> > +                return -ETIMEDOUT;
> >              }
> 
> I don't see such a change be valid without making sure callers actually honor
> errors. For example, no caller of iommu_flush_iec_{global,index}() cares to
> check. And not even your second patch addresses this (i.e.
> it's also not just bad patch ordering).
> 

I check it again. 
For iommu_flush_iec_{global,index}()  are both call __iommu_flush_iec().
In my patch, I have check it in __iommu_flush_iec().
I think it does not need to check it in iommu_flush_iec_{global,index}() again. 
Right?

-Quan


> If you require errors to be dealt with (and you do here), make use of
> __must_check all the way up the call trees.
> 
> Jan


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