Re: [PATCH v12 6/6] vt-d: fix vt-d Device-TLB flush timeout issue

["Xu, Quan" <quan.xu@intel.com>]

On June 24, 2016 7:55 PM, Tian, Kevin <kevin.tian@intel.com> wrote:
> > From: Xu, Quan
> > Sent: Friday, June 24, 2016 1:52 PM
> > diff --git a/xen/drivers/passthrough/vtd/extern.h
> > b/xen/drivers/passthrough/vtd/extern.h
> > index 45357f2..efaff28 100644
> > --- a/xen/drivers/passthrough/vtd/extern.h
> > +++ b/xen/drivers/passthrough/vtd/extern.h
> > @@ -25,6 +25,7 @@
> >
> >  #define VTDPREFIX "[VT-D]"
> >
> > +struct pci_ats_dev;
> >  extern bool_t rwbf_quirk;
> >
> >  void print_iommu_regs(struct acpi_drhd_unit *drhd); @@ -60,8 +61,8 @@
> > int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
> >                           u64 addr, unsigned int size_order, u64
> > type);
> >
> >  int __must_check qinval_device_iotlb_sync(struct iommu *iommu,
> > -                                          u32 max_invs_pend,
> > -                                          u16 sid, u16 size, u64 addr);
> > +                                          struct pci_ats_dev *ats_dev,
> > +                                          u16 did, u16 size, u64
> > + addr);
> >
> >  unsigned int get_cache_line_size(void);  void cacheline_flush(char
> > *); diff --git a/xen/drivers/passthrough/vtd/qinval.c
> > b/xen/drivers/passthrough/vtd/qinval.c
> > index 4492b29..e4e2771 100644
> > --- a/xen/drivers/passthrough/vtd/qinval.c
> > +++ b/xen/drivers/passthrough/vtd/qinval.c
> > @@ -27,11 +27,11 @@
> >  #include "dmar.h"
> >  #include "vtd.h"
> >  #include "extern.h"
> > +#include "../ats.h"
> 
> Earlier you said:
> >    1. a forward declaration struct pci_ats_dev*, instead of
> >       including ats.h.
>

This context is 'in extern.h', but..

> But above you still have ats.h included.
> 

.. I really need to include 'ats.h' here, as the 'struct pci_ats_dev*' is used in this file.

> >
> >  #define VTD_QI_TIMEOUT	1
> >
> > -static int __must_check invalidate_sync(struct iommu *iommu,
> > -                                        bool_t flush_dev_iotlb);
> > +static int __must_check invalidate_sync(struct iommu *iommu);
> 
> I don't understand the rationale behind. In earlier patch you introduce a new
> parameter which is however just removed later here....
> 
In earlier patch, refactor invalidate_sync() to indicate whether we need to flush device IOTLB or not.
change it back here, as I add a specific function - dev_invalidate_sync() for device IOTLB invalidation..

Quan





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