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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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


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