[Xen-devel] [PATCH] Qemu MSI Cleaning Up

[Haitao Shan <maillists.shan@xxxxxxxxx>]

Hi, Ian, Keir, Jan,

This patch does cleaning up of QEMU MSI handling. The fixes are:
1. Changes made to MSI-X table mapping handling to eliminate the small
windows in which guest could have access to physical MSI-X table.
2. MSI-X table is mapped as read-only to QEMU, as masking of MSI-X is
already in Xen now.
3. For registers that coexists inside the MSI-X table (this could be
only PBA I think), value read from physical page would be returned.

Could you please have review?

Signed-off-by:  Shan Haitao <haitao.shan@xxxxxxxxx>


diff --git a/hw/pass-through.c b/hw/pass-through.c
index 9c5620d..6ebed64 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -92,6 +92,7 @@

 #include <unistd.h>
 #include <sys/ioctl.h>
+#include <assert.h>

 extern int gfx_passthru;
 int igd_passthru = 0;
@@ -1103,6 +1104,7 @@ static void pt_iomem_map(PCIDevice *d, int i,
uint32_t e_phys, uint32_t e_size,
 {
     struct pt_dev *assigned_device  = (struct pt_dev *)d;
     uint32_t old_ebase = assigned_device->bases[i].e_physbase;
+    uint32_t msix_last_pfn = 0, bar_last_pfn = 0;
     int first_map = ( assigned_device->bases[i].e_size == 0 );
     int ret = 0;

@@ -1118,39 +1120,127 @@ static void pt_iomem_map(PCIDevice *d, int i,
uint32_t e_phys, uint32_t e_size,

     if ( !first_map && old_ebase != -1 )
     {
-        add_msix_mapping(assigned_device, i);
-        /* Remove old mapping */
-        ret = xc_domain_memory_mapping(xc_handle, domid,
+        if ( has_msix_mapping(assigned_device, i) )
+        {
+            msix_last_pfn = (assigned_device->msix->mmio_base_addr - 1 +
+                  assigned_device->msix->total_entries * 16) >>  XC_PAGE_SHIFT;
+            bar_last_pfn = (old_ebase + e_size - 1) >> XC_PAGE_SHIFT;
+
+            unregister_iomem(assigned_device->msix->mmio_base_addr);
+
+            if ( assigned_device->msix->table_off )
+            {
+                       ret = xc_domain_memory_mapping(xc_handle, domid,
+                    old_ebase >> XC_PAGE_SHIFT,
+                    assigned_device->bases[i].access.maddr >> XC_PAGE_SHIFT,
+                    (assigned_device->msix->mmio_base_addr >> XC_PAGE_SHIFT)
+                    - (old_ebase >> XC_PAGE_SHIFT),
+                    DPCI_REMOVE_MAPPING);
+                if ( ret != 0 )
+                {
+                    PT_LOG("Error: remove old mapping failed!\n");
+                    return;
+                }
+            }
+            if ( msix_last_pfn != bar_last_pfn )
+            {
+                assert(msix_last_pfn < bar_last_pfn);
+                       ret = xc_domain_memory_mapping(xc_handle, domid,
+                    msix_last_pfn + 1,
+                    (assigned_device->bases[i].access.maddr +
+                     assigned_device->msix->table_off +
+                     assigned_device->msix->total_entries * 16 +
+                     XC_PAGE_SIZE -1) >>  XC_PAGE_SHIFT,
+                    bar_last_pfn - msix_last_pfn,
+                    DPCI_REMOVE_MAPPING);
+                if ( ret != 0 )
+                {
+                    PT_LOG("Error: remove old mapping failed!\n");
+                    return;
+                }
+            }
+        }
+        else
+        {
+                   /* Remove old mapping */
+                   ret = xc_domain_memory_mapping(xc_handle, domid,
                 old_ebase >> XC_PAGE_SHIFT,
                 assigned_device->bases[i].access.maddr >> XC_PAGE_SHIFT,
                 (e_size+XC_PAGE_SIZE-1) >> XC_PAGE_SHIFT,
                 DPCI_REMOVE_MAPPING);
-        if ( ret != 0 )
-        {
-            PT_LOG("Error: remove old mapping failed!\n");
-            return;
+            if ( ret != 0 )
+            {
+                PT_LOG("Error: remove old mapping failed!\n");
+                return;
+            }
         }
     }

     /* map only valid guest address */
     if (e_phys != -1)
     {
-        /* Create new mapping */
-        ret = xc_domain_memory_mapping(xc_handle, domid,
+        if ( has_msix_mapping(assigned_device, i) )
+               {
+            assigned_device->msix->mmio_base_addr =
+                assigned_device->bases[i].e_physbase
+                + assigned_device->msix->table_off;
+
+            msix_last_pfn = (assigned_device->msix->mmio_base_addr - 1 +
+                  assigned_device->msix->total_entries * 16) >>  XC_PAGE_SHIFT;
+            bar_last_pfn = (e_phys + e_size - 1) >> XC_PAGE_SHIFT;
+
+            cpu_register_physical_memory(assigned_device->msix->mmio_base_addr,
+                 (assigned_device->msix->total_entries * 16 + XC_PAGE_SIZE - 1)
+                  & XC_PAGE_MASK,
+                 assigned_device->msix->mmio_index);
+
+            if ( assigned_device->msix->table_off )
+            {
+                       ret = xc_domain_memory_mapping(xc_handle, domid,
+                    assigned_device->bases[i].e_physbase >> XC_PAGE_SHIFT,
+                    assigned_device->bases[i].access.maddr >> XC_PAGE_SHIFT,
+                    (assigned_device->msix->mmio_base_addr >> XC_PAGE_SHIFT)
+                    - (assigned_device->bases[i].e_physbase >> XC_PAGE_SHIFT),
+                    DPCI_ADD_MAPPING);
+                if ( ret != 0 )
+                {
+                    PT_LOG("Error: remove old mapping failed!\n");
+                    return;
+                }
+            }
+            if ( msix_last_pfn != bar_last_pfn )
+            {
+                assert(msix_last_pfn < bar_last_pfn);
+                       ret = xc_domain_memory_mapping(xc_handle, domid,
+                    msix_last_pfn + 1,
+                    (assigned_device->bases[i].access.maddr +
+                     assigned_device->msix->table_off +
+                     assigned_device->msix->total_entries * 16 +
+                     XC_PAGE_SIZE -1) >>  XC_PAGE_SHIFT,
+                    bar_last_pfn - msix_last_pfn,
+                    DPCI_ADD_MAPPING);
+                if ( ret != 0 )
+                {
+                    PT_LOG("Error: remove old mapping failed!\n");
+                    return;
+                }
+            }
+               }
+               else
+        {
+                       /* Create new mapping */
+                       ret = xc_domain_memory_mapping(xc_handle, domid,
                 assigned_device->bases[i].e_physbase >> XC_PAGE_SHIFT,
                 assigned_device->bases[i].access.maddr >> XC_PAGE_SHIFT,
                 (e_size+XC_PAGE_SIZE-1) >> XC_PAGE_SHIFT,
                 DPCI_ADD_MAPPING);

-        if ( ret != 0 )
-        {
-            PT_LOG("Error: create new mapping failed!\n");
+            if ( ret != 0 )
+            {
+                PT_LOG("Error: create new mapping failed!\n");
+            }
         }

-        ret = remove_msix_mapping(assigned_device, i);
-        if ( ret != 0 )
-            PT_LOG("Error: remove MSI-X mmio mapping failed!\n");
-
         if ( old_ebase != e_phys && old_ebase != -1 )
             pt_msix_update_remap(assigned_device, i);
     }
diff --git a/hw/pt-msi.c b/hw/pt-msi.c
index 71fa6f0..0134e62 100644
--- a/hw/pt-msi.c
+++ b/hw/pt-msi.c
@@ -284,15 +284,6 @@ void pt_disable_msi_translate(struct pt_dev *dev)
     dev->msi_trans_en = 0;
 }

-/* MSI-X virtulization functions */
-static void mask_physical_msix_entry(struct pt_dev *dev, int
entry_nr, int mask)
-{
-    void *phys_off;
-
-    phys_off = dev->msix->phys_iomem_base + 16 * entry_nr + 12;
-    *(uint32_t *)phys_off = mask;
-}
-
 static int pt_msix_update_one(struct pt_dev *dev, int entry_nr)
 {
     struct msix_entry_info *entry = &dev->msix->msix_entry[entry_nr];
@@ -486,7 +477,6 @@ static void pci_msix_writel(void *opaque,
target_phys_addr_t addr, uint32_t val)
     {
         if ( msix->enabled && !(val & 0x1) )
             pt_msix_update_one(dev, entry_nr);
-        mask_physical_msix_entry(dev, entry_nr, entry->io_mem[3] & 0x1);
     }
 }

@@ -519,7 +509,11 @@ static uint32_t pci_msix_readl(void *opaque,
target_phys_addr_t addr)
     entry_nr = (addr - msix->mmio_base_addr) / 16;
     offset = ((addr - msix->mmio_base_addr) % 16) / 4;

-    return msix->msix_entry[entry_nr].io_mem[offset];
+    if ( addr - msix->mmio_base_addr < msix->total_entries * 16 )
+        return msix->msix_entry[entry_nr].io_mem[offset];
+    else
+        return *(uint32_t *)(msix->phys_iomem_base +
+                             (addr - msix->mmio_base_addr));
 }

 static CPUReadMemoryFunc *pci_msix_read[] = {
@@ -528,39 +522,12 @@ static CPUReadMemoryFunc *pci_msix_read[] = {
     pci_msix_readl
 };

-int add_msix_mapping(struct pt_dev *dev, int bar_index)
-{
-    if ( !(dev->msix && dev->msix->bar_index == bar_index) )
-        return 0;
-
-    return xc_domain_memory_mapping(xc_handle, domid,
-                dev->msix->mmio_base_addr >> XC_PAGE_SHIFT,
-                (dev->bases[bar_index].access.maddr
-                + dev->msix->table_off) >> XC_PAGE_SHIFT,
-                (dev->msix->total_entries * 16
-                + XC_PAGE_SIZE -1) >> XC_PAGE_SHIFT,
-                DPCI_ADD_MAPPING);
-}
-
-int remove_msix_mapping(struct pt_dev *dev, int bar_index)
+int has_msix_mapping(struct pt_dev *dev, int bar_index)
 {
     if ( !(dev->msix && dev->msix->bar_index == bar_index) )
         return 0;

-    dev->msix->mmio_base_addr = dev->bases[bar_index].e_physbase
-                                + dev->msix->table_off;
-
-    cpu_register_physical_memory(dev->msix->mmio_base_addr,
-                                 dev->msix->total_entries * 16,
-                                 dev->msix->mmio_index);
-
-    return xc_domain_memory_mapping(xc_handle, domid,
-                dev->msix->mmio_base_addr >> XC_PAGE_SHIFT,
-                (dev->bases[bar_index].access.maddr
-                + dev->msix->table_off) >> XC_PAGE_SHIFT,
-                (dev->msix->total_entries * 16
-                + XC_PAGE_SIZE -1) >> XC_PAGE_SHIFT,
-                DPCI_REMOVE_MAPPING);
+       return 1;
 }

 int pt_msix_init(struct pt_dev *dev, int pos)
@@ -616,7 +583,7 @@ int pt_msix_init(struct pt_dev *dev, int pos)
     PT_LOG("table_off = %x, total_entries = %d\n", table_off, total_entries);
     dev->msix->table_offset_adjust = table_off & 0x0fff;
     dev->msix->phys_iomem_base = mmap(0, total_entries * 16 +
dev->msix->table_offset_adjust,
-                          PROT_WRITE | PROT_READ, MAP_SHARED | MAP_LOCKED,
+                          PROT_READ, MAP_SHARED | MAP_LOCKED,
                           fd, dev->msix->table_base + table_off -
dev->msix->table_offset_adjust);
     dev->msix->phys_iomem_base = (void *)((char *)dev->msix->phys_iomem_base +
                           dev->msix->table_offset_adjust);
diff --git a/hw/pt-msi.h b/hw/pt-msi.h
index 9664f89..2dc1720 100644
--- a/hw/pt-msi.h
+++ b/hw/pt-msi.h
@@ -107,10 +107,7 @@ void
 pt_msix_disable(struct pt_dev *dev);

 int
-remove_msix_mapping(struct pt_dev *dev, int bar_index);
-
-int
-add_msix_mapping(struct pt_dev *dev, int bar_index);
+has_msix_mapping(struct pt_dev *dev, int bar_index);

 int
 pt_msix_init(struct pt_dev *dev, int pos);

Attachment: qemu_msi_clean_up.patch
Description: Binary data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel