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:01pm, <Tian, Kevin> wrote:
> > 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. :-)


Agreed. :):)

Quan






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