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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Xu, Quan
> Sent: Friday, December 11, 2015 10:09 AM
> 
> 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.
> 

Please describe this plan in your summary in next version. :-)

Thanks
Kevin

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