Re: [Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Jan Beulich]

>>> On 05.07.16 at 15:56,  wrote:
> Just check it, do you agree to pick up this series?

Depends on Kevin's view; I wanted to send out review feedback
publicly regardless of who'd be picking it up.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

Jan, 
Just check it, do you agree to pick up this series?

Quan

On July 05, 2016 9:46 PM, Jan Beulich  wrote:
> >>> On 04.07.16 at 11:11,  wrote:
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -361,6 +361,30 @@ int iommu_iotlb_flush_all(struct domain *d)
> >  return rc;
> >  }
> >
> > +void iommu_dev_iotlb_flush_timeout(struct domain *d,
> 
> const
> 
> > +   struct pci_dev *pdev) {
> > +pcidevs_lock();
> > +
> > +ASSERT(pdev->domain);
> > +if ( d != pdev->domain )
> > +{
> > +pcidevs_unlock();
> > +return;
> > +}
> > +
> > +list_del(>domain_list);
> > +pdev->domain = NULL;
> > +pci_hide_existing_device(pdev);
> > +if ( !d->is_shutting_down && printk_ratelimit() )
> > +printk(XENLOG_ERR
> > +   "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
> > +   d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> > +   PCI_FUNC(pdev->devfn));
> > +
> > +pcidevs_unlock();
> > +}
> 
> I'm missing the domain_crash() part here (which would be the only reason
> why the parameter above can't be const).
> 
> > +static int __must_check dev_invalidate_sync(struct iommu *iommu,
> > +struct pci_dev *pdev, u16
> > +did)
> 
> const
> 
> Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Jan Beulich]

>>> On 04.07.16 at 11:11,  wrote:
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -361,6 +361,30 @@ int iommu_iotlb_flush_all(struct domain *d)
>  return rc;
>  }
>  
> +void iommu_dev_iotlb_flush_timeout(struct domain *d,

const

> +   struct pci_dev *pdev)
> +{
> +pcidevs_lock();
> +
> +ASSERT(pdev->domain);
> +if ( d != pdev->domain )
> +{
> +pcidevs_unlock();
> +return;
> +}
> +
> +list_del(>domain_list);
> +pdev->domain = NULL;
> +pci_hide_existing_device(pdev);
> +if ( !d->is_shutting_down && printk_ratelimit() )
> +printk(XENLOG_ERR
> +   "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
> +   d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> +   PCI_FUNC(pdev->devfn));
> +
> +pcidevs_unlock();
> +}

I'm missing the domain_crash() part here (which would be the only
reason why the parameter above can't be const).

> +static int __must_check dev_invalidate_sync(struct iommu *iommu,
> +struct pci_dev *pdev, u16 did)

const

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

On July 05, 2016 3:00 PM, Jan Beulich  wrote:
> >>> On 05.07.16 at 04:04,  wrote:
> > On July 05, 2016 8:47 AM, Tian, Kevin  wrote:
> >> > From: Xu, Quan
> >> > Sent: Monday, July 04, 2016 5:12 PM
> >> >
> >> > From: Quan Xu 
> >> >
> >> > If Device-TLB flush timed out, we hide the target ATS device
> >> > immediately. By hiding the device, we make sure it can't be
> >> > assigned to any domain any longer (see device_assigned).
> >> >
> >> > Signed-off-by: Quan Xu 
> >> >
> >> > CC: Jan Beulich 
> >> > CC: Kevin Tian 
> >> > CC: Feng Wu 
> >>
> >> Acked-by: Kevin Tian 
> >
> > Jan,  now there patches  are all acked-by Kevin. Is it ready to go in?
> > If not,  could you send out your comments, I shall be very grateful
> > and fix it today.
> 
> You sent the most recent version yesterday. Please allow more than just one
> day before you ping.
> 

Sorry for that, as today is my last working day at Intel Corporation. 
I really want to close it as a  milestone (but if there is still some comment, 
I will continue to fix it).

Quan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Jan Beulich]

>>> On 05.07.16 at 04:04,  wrote:
> On July 05, 2016 8:47 AM, Tian, Kevin  wrote:
>> > From: Xu, Quan
>> > Sent: Monday, July 04, 2016 5:12 PM
>> >
>> > From: Quan Xu 
>> >
>> > If Device-TLB flush timed out, we hide the target ATS device
>> > immediately. By hiding the device, we make sure it can't be assigned
>> > to any domain any longer (see device_assigned).
>> >
>> > Signed-off-by: Quan Xu 
>> >
>> > CC: Jan Beulich 
>> > CC: Kevin Tian 
>> > CC: Feng Wu 
>> 
>> Acked-by: Kevin Tian 
> 
> Jan,  now there patches  are all acked-by Kevin. Is it ready to go in?
> If not,  could you send out your comments, I shall be very grateful and fix 
> it today.

You sent the most recent version yesterday. Please allow more than
just one day before you ping.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

On July 05, 2016 8:47 AM, Tian, Kevin  wrote:
> > From: Xu, Quan
> > Sent: Monday, July 04, 2016 5:12 PM
> >
> > From: Quan Xu 
> >
> > If Device-TLB flush timed out, we hide the target ATS device
> > immediately. By hiding the device, we make sure it can't be assigned
> > to any domain any longer (see device_assigned).
> >
> > Signed-off-by: Quan Xu 
> >
> > CC: Jan Beulich 
> > CC: Kevin Tian 
> > CC: Feng Wu 
> 
> Acked-by: Kevin Tian 

Jan,  now there patches  are all acked-by Kevin. Is it ready to go in?
If not,  could you send out your comments, I shall be very grateful and fix it 
today.

Quan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Tian, Kevin]

> From: Xu, Quan
> Sent: Monday, July 04, 2016 5:12 PM
> 
> From: Quan Xu 
> 
> If Device-TLB flush timed out, we hide the target ATS device
> immediately. By hiding the device, we make sure it can't be
> assigned to any domain any longer (see device_assigned).
> 
> Signed-off-by: Quan Xu 
> 
> CC: Jan Beulich 
> CC: Kevin Tian 
> CC: Feng Wu 

Acked-by: Kevin Tian 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

From: Quan Xu 

If Device-TLB flush timed out, we hide the target ATS device
immediately. By hiding the device, we make sure it can't be
assigned to any domain any longer (see device_assigned).

Signed-off-by: Quan Xu 

CC: Jan Beulich 
CC: Kevin Tian 
CC: Feng Wu 

---
v14: release the lock before return.
---
 xen/drivers/passthrough/iommu.c   | 24 +++
 xen/drivers/passthrough/pci.c |  6 ++--
 xen/drivers/passthrough/vtd/extern.h  |  5 ++--
 xen/drivers/passthrough/vtd/qinval.c  | 56 +++
 xen/drivers/passthrough/vtd/x86/ats.c | 11 ++-
 xen/include/xen/iommu.h   |  3 ++
 xen/include/xen/pci.h |  1 +
 7 files changed, 79 insertions(+), 27 deletions(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index d793f5d..2353c7d 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -361,6 +361,30 @@ int iommu_iotlb_flush_all(struct domain *d)
 return rc;
 }
 
+void iommu_dev_iotlb_flush_timeout(struct domain *d,
+   struct pci_dev *pdev)
+{
+pcidevs_lock();
+
+ASSERT(pdev->domain);
+if ( d != pdev->domain )
+{
+pcidevs_unlock();
+return;
+}
+
+list_del(>domain_list);
+pdev->domain = NULL;
+pci_hide_existing_device(pdev);
+if ( !d->is_shutting_down && printk_ratelimit() )
+printk(XENLOG_ERR
+   "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
+   d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
+   PCI_FUNC(pdev->devfn));
+
+pcidevs_unlock();
+}
+
 int __init iommu_setup(void)
 {
 int rc = -ENODEV;
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index bb5f344..58bfb79 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -419,7 +419,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev 
*pdev)
 xfree(pdev);
 }
 
-static void _pci_hide_device(struct pci_dev *pdev)
+void pci_hide_existing_device(struct pci_dev *pdev)
 {
 if ( pdev->domain )
 return;
@@ -436,7 +436,7 @@ int __init pci_hide_device(int bus, int devfn)
 pdev = alloc_pdev(get_pseg(0), bus, devfn);
 if ( pdev )
 {
-_pci_hide_device(pdev);
+pci_hide_existing_device(pdev);
 rc = 0;
 }
 pcidevs_unlock();
@@ -466,7 +466,7 @@ int __init pci_ro_device(int seg, int bus, int devfn)
 }
 
 __set_bit(PCI_BDF2(bus, devfn), pseg->ro_map);
-_pci_hide_device(pdev);
+pci_hide_existing_device(pdev);
 
 return 0;
 }
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..7a5c433 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"
 
 #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);
 
 static void print_qi_regs(struct iommu *iommu)
 {
@@ -103,7 +103,7 @@ static int __must_check 
queue_invalidate_context_sync(struct iommu *iommu,
 
 unmap_vtd_domain_page(qinval_entries);
 
-return invalidate_sync(iommu, 0);
+return invalidate_sync(iommu);
 }
 
 static int __must_check queue_invalidate_iotlb_sync(struct iommu *iommu,
@@ -140,7 +140,7 @@ static int __must_check queue_invalidate_iotlb_sync(struct 
iommu *iommu,
 qinval_update_qtail(iommu, index);
 spin_unlock_irqrestore(>register_lock, flags);
 
-return invalidate_sync(iommu, 0);
+return invalidate_sync(iommu);
 }
 
 static int __must_check queue_invalidate_wait(struct iommu *iommu,
@@ -199,25 +199,55 @@ static int __must_check queue_invalidate_wait(struct 
iommu *iommu,
 return -EOPNOTSUPP;
 }