[PATCH RFC 01/44] passthrough/vtd: Don't DMA to the stack in queue_invalidate_wait()

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: [PATCH RFC 01/44] passthrough/vtd: Don't DMA to the stack in queue_invalidate_wait()
Date: Thu, 4 Jan 2018 20:21:26 +0000	[thread overview]
Message-ID: <1515097329-31902-2-git-send-email-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <1515097329-31902-1-git-send-email-andrew.cooper3@citrix.com>

DMA-ing to the stack is generally considered bad practice.  In this case, if a
timeout occurs because of a sluggish device which is processing the request,
the completion notification will corrupt the stack of a subsequent deeper call
tree.

Place the poll_slot in a percpu area and DMA to that instead.

Note: This change does not address other issues with the current
implementation, such as once a timeout has been suffered, subsequent
completions can't be correlated with their requests.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Julien Grall <julien.grall@arm.com>

Julien: This wants backporting to all releases, and therefore should be
considered for 4.10 at this point.

v3:
 * Add note that there are still outstanding issues.
v2:
 * Retain volatile declaration for poll_slot.
 * Initialise poll_slot to QINVAL_STAT_INIT on each call.
---
 xen/drivers/passthrough/vtd/qinval.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index e95dc54..51aef37 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -147,13 +147,15 @@ static int __must_check queue_invalidate_wait(struct iommu *iommu,
                                               u8 iflag, u8 sw, u8 fn,
                                               bool_t flush_dev_iotlb)
 {
-    volatile u32 poll_slot = QINVAL_STAT_INIT;
+    static DEFINE_PER_CPU(volatile u32, poll_slot);
     unsigned int index;
     unsigned long flags;
     u64 entry_base;
     struct qinval_entry *qinval_entry, *qinval_entries;
+    volatile u32 *this_poll_slot = &this_cpu(poll_slot);
 
     spin_lock_irqsave(&iommu->register_lock, flags);
+    *this_poll_slot = QINVAL_STAT_INIT;
     index = qinval_next_index(iommu);
     entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
                  ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
@@ -167,7 +169,7 @@ static int __must_check queue_invalidate_wait(struct iommu *iommu,
     qinval_entry->q.inv_wait_dsc.lo.res_1 = 0;
     qinval_entry->q.inv_wait_dsc.lo.sdata = QINVAL_STAT_DONE;
     qinval_entry->q.inv_wait_dsc.hi.res_1 = 0;
-    qinval_entry->q.inv_wait_dsc.hi.saddr = virt_to_maddr(&poll_slot) >> 2;
+    qinval_entry->q.inv_wait_dsc.hi.saddr = virt_to_maddr(this_poll_slot) >> 2;
 
     unmap_vtd_domain_page(qinval_entries);
     qinval_update_qtail(iommu, index);
@@ -182,7 +184,7 @@ static int __must_check queue_invalidate_wait(struct iommu *iommu,
         timeout = NOW() + MILLISECS(flush_dev_iotlb ?
                                     iommu_dev_iotlb_timeout : VTD_QI_TIMEOUT);
 
-        while ( poll_slot != QINVAL_STAT_DONE )
+        while ( *this_poll_slot != QINVAL_STAT_DONE )
         {
             if ( NOW() > timeout )
             {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
