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 14.12.2015 at 4:19pm, <JBeulich@xxxxxxxx> wrote:
> >>> On 12.12.15 at 10:03, <quan.xu@xxxxxxxxx> wrote:
> >> 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?
> 
> No, not in the v2 version of it. While you call invalidate_timeout() in the
> -ETIMEDOUT case, you still pass on the error code, and hence the callers need
> to also either pass it on or deal with it.

Jan, sorry for that. I still don't get the point.
Check it again :(:(

Should I also check it where call iommu_flush_iec_{global,index}(), if it is 
-ETIMEDOUT, it should return or deal with it??

.i.e. 

static void free_remap_entry(struct iommu *iommu, int index)
{
...
    iommu_flush_iec_index(iommu, 0, index);
    
*<------------------------------------------------------------------------------
  (should I also check it where call iommu_flush_iec_{global,index}(), if it is 
-ETIMEDOUT, it should return or deal with it) ??
...                     
}



Quan

































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