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 3:03pm, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 10/12/15 09:33, Quan Xu wrote:
> > Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> > ---
> >  xen/drivers/passthrough/vtd/qinval.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/vtd/qinval.c
> > b/xen/drivers/passthrough/vtd/qinval.c
> > index b81b0bd..990baf2 100644
> > --- a/xen/drivers/passthrough/vtd/qinval.c
> > +++ b/xen/drivers/passthrough/vtd/qinval.c
> > @@ -28,6 +28,11 @@
> >  #include "vtd.h"
> >  #include "extern.h"
> >
> > +static int __read_mostly iommu_qi_timeout_ms = 1;
> > +integer_param("iommu_qi_timeout_ms", iommu_qi_timeout_ms);
> > +
> > +#define IOMMU_QI_TIMEOUT (iommu_qi_timeout_ms * MILLISECS(1))
> > +
> >  static void print_qi_regs(struct iommu *iommu)  {
> >      u64 val;
> > @@ -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;
> >              }
> >              cpu_relax();
> >          }
> 
> This patch misses a second use of DMAR_OPERATION_TIMEOUT, in
> IOMMU_WAIT_OP() which in turn is used in a large number of locations.
> All of these locations equally need to be chopped down to a low number of
> milliseconds.

Andrew, thanks for your comments.

I know that DMAR_OPERATION_TIMEOUT should be also chopped down to a low number 
of milliseconds.
As Kevin Tian mentioned in 'Revisit VT-d asynchronous flush issue', We also 
confirmed with hardware team
that 1ms is large enough for IOMMU internal flush.
So I can change DMAR_OPERATION_TIMEOUT from 1000 ms to 1 ms.

IOMMU_WAIT_OP() is only for VT-d registers read/write, and there is also a 
panic. We need a further discussion 
whether or how to remove this panic. I can send another patch set to fix it. in 
this patch set, I want to focus on VT-d
QI flush.

-Quan





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