WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH] Qemu MSI Cleaning Up
From: Haitao Shan <maillists.shan@xxxxxxxxx>
Date: Thu, 22 Sep 2011 08:35:39 +0800
Cc:
Delivery-date: Wed, 21 Sep 2011 17:36:48 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:date:message-id:subject:from:to:content-type; bh=MetpFlUgmEFF1SwTkVWggEuaoYtgkRvbLWcb4FUTHzg=; b=MoL8WYcqm2Uf4nvCaxtLIm73025S7zmGTkT6v8iVJMrVD94NyUqrJwWOFiUIAIwGFU /uAQRvZG+5TZxhKKAzImLZLR52tC6F2F1iDpXsgpo2L3bcgU4RKvpQ/0+X//ELaJ++3o BcBQNHG7cLh1GY1pmKT2yhLu2DGi/B+naUNuM=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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
<Prev in Thread] Current Thread [Next in Thread>