File xsa373-0.patch of Package xen.23721

Overview Repositories Revisions Requests Users Attributes Meta

File xsa373-0.patch of Package xen.23721

# Commit 8970834eb95586d87b064e8c7fc49ee8d2875db4
# Date 2019-07-24 14:40:10 +0100
# Author Andrew Cooper <andrew.cooper3@citrix.com>
# Committer Andrew Cooper <andrew.cooper3@citrix.com>
passthrough/vtd: Don't DMA to the stack in queue_invalidate_wait()

DMA-ing to the stack is 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.

Fix the declaration of saddr in struct qinval_entry, to avoid a shift by
two.  The requirement here is that the DMA address is dword aligned,
which is covered by poll_slot's type.

This change does not address other issues.  Correlating completions
after a timeout with their request is a more complicated change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>

--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -441,8 +441,7 @@ struct qinval_entry {
                     sdata   : 32;
             }lo;
             struct {
-                u64 res_1   : 2,
-                    saddr   : 62;
+                u64 saddr;
             }hi;
         }inv_wait_dsc;
     }q;
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -134,13 +134,15 @@ static int queue_invalidate_wait(struct
     u8 iflag, u8 sw, u8 fn)
 {
     s_time_t start_time;
-    volatile u32 poll_slot = QINVAL_STAT_INIT;
+    static DEFINE_PER_CPU(uint32_t, poll_slot);
     unsigned int index;
     unsigned long flags;
     u64 entry_base;
     struct qinval_entry *qinval_entry, *qinval_entries;
+    uint32_t *this_poll_slot = &this_cpu(poll_slot);
 
     spin_lock_irqsave(&iommu->register_lock, flags);
+    ACCESS_ONCE(*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);
@@ -153,8 +155,7 @@ static int queue_invalidate_wait(struct
     qinval_entry->q.inv_wait_dsc.lo.fn = fn;
     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);
 
     unmap_vtd_domain_page(qinval_entries);
     qinval_update_qtail(iommu, index);
@@ -165,7 +166,7 @@ static int queue_invalidate_wait(struct
     {
         /* In case all wait descriptor writes to same addr with same data */
         start_time = NOW();
-        while ( poll_slot != QINVAL_STAT_DONE )
+        while ( ACCESS_ONCE(*this_poll_slot) != QINVAL_STAT_DONE )
         {
             if ( NOW() > (start_time + DMAR_OPERATION_TIMEOUT) )
             {

Places

File xsa373-0.patch of Package xen.23721

Places