From patchwork Fri Feb 17 17:08:45 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 141924 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 7F2181007D6 for ; Sat, 18 Feb 2012 04:18:49 +1100 (EST) Received: from localhost ([::1]:55933 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RyRSI-0003sS-I9 for incoming@patchwork.ozlabs.org; Fri, 17 Feb 2012 12:18:46 -0500 Received: from eggs.gnu.org ([140.186.70.92]:58359) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RyRS5-0003ql-3d for qemu-devel@nongnu.org; Fri, 17 Feb 2012 12:18:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RyRS0-00085b-FZ for qemu-devel@nongnu.org; Fri, 17 Feb 2012 12:18:32 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:20649) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RyRS0-00085P-9C for qemu-devel@nongnu.org; Fri, 17 Feb 2012 12:18:28 -0500 X-IronPort-AV: E=Sophos;i="4.73,439,1325480400"; d="scan'208";a="182424737" Received: from ftlpmailmx02.citrite.net ([10.13.107.66]) by FTLPIPO02.CITRIX.COM with ESMTP/TLS/RC4-MD5; 17 Feb 2012 12:18:27 -0500 Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.66) with Microsoft SMTP Server id 8.3.213.0; Fri, 17 Feb 2012 12:18:27 -0500 Received: from [10.80.3.28] (helo=perard.uk.xensource.com) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1RyRIj-0001tz-12; Fri, 17 Feb 2012 17:08:53 +0000 From: Anthony PERARD To: QEMU-devel Date: Fri, 17 Feb 2012 17:08:45 +0000 Message-ID: <1329498525-8454-12-git-send-email-anthony.perard@citrix.com> X-Mailer: git-send-email 1.7.2.5 In-Reply-To: <1329498525-8454-1-git-send-email-anthony.perard@citrix.com> References: <1329498525-8454-1-git-send-email-anthony.perard@citrix.com> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.165.176.63 Cc: Anthony PERARD , Shan Haitao , Xen Devel , Jan Beulich , Stefano Stabellini Subject: [Qemu-devel] [PATCH V7 11/11] xen passthrough: clean up MSI-X table handling X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Shan Haitao 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. Signed-off-by: Shan Haitao Consolidated duplicate code into _pt_iomem_helper(). Fixed formatting. Signed-off-by: Jan Beulich Signed-off-by: Anthony PERARD --- hw/xen_pci_passthrough.c | 80 ++++++++++++++++++++++++++++++++---------- hw/xen_pci_passthrough.h | 11 +++++- hw/xen_pci_passthrough_msi.c | 62 ++++---------------------------- 3 files changed, 78 insertions(+), 75 deletions(-) diff --git a/hw/xen_pci_passthrough.c b/hw/xen_pci_passthrough.c index 6ea2c45..fab9fbe 100644 --- a/hw/xen_pci_passthrough.c +++ b/hw/xen_pci_passthrough.c @@ -356,6 +356,53 @@ out: } } +static int pt_iomem_helper(XenPCIPassthroughState *s, int i, + pcibus_t e_base, pcibus_t e_size, int op) +{ + uint64_t msix_last_pfn = 0; + pcibus_t bar_last_pfn = 0; + pcibus_t msix_table_size = 0; + XenPTMSIX *msix = s->msix; + int rc = 0; + + if (!pt_has_msix_mapping(s, i)) { + return xc_domain_memory_mapping(xen_xc, xen_domid, + PFN(e_base), + PFN(s->bases[i].access.maddr), + PFN(e_size + XC_PAGE_SIZE - 1), + op); + + } + + if (msix->table_off) { + rc = xc_domain_memory_mapping(xen_xc, xen_domid, + PFN(e_base), + PFN(s->bases[i].access.maddr), + PFN(msix->mmio_base_addr) - PFN(e_base), + op); + } + + msix_table_size = msix->total_entries * PCI_MSIX_ENTRY_SIZE; + msix_last_pfn = PFN(msix->mmio_base_addr + msix_table_size - 1); + bar_last_pfn = PFN(e_base + e_size - 1); + + if (rc == 0 && msix_last_pfn != bar_last_pfn) { + assert(msix_last_pfn < bar_last_pfn); + + rc = xc_domain_memory_mapping(xen_xc, xen_domid, + msix_last_pfn + 1, + PFN(s->bases[i].access.maddr + + msix->table_off + + msix_table_size + + XC_PAGE_SIZE - 1), + bar_last_pfn - msix_last_pfn, + op); + } + + return rc; +} + + /* ioport/iomem space*/ static void pt_iomem_map(XenPCIPassthroughState *s, int i, pcibus_t e_phys, pcibus_t e_size) @@ -376,13 +423,10 @@ static void pt_iomem_map(XenPCIPassthroughState *s, int i, } if (!first_map && old_ebase != PT_PCI_BAR_UNMAPPED) { - pt_add_msix_mapping(s, i); - /* Remove old mapping */ - rc = xc_domain_memory_mapping(xen_xc, xen_domid, - old_ebase >> XC_PAGE_SHIFT, - s->bases[i].access.maddr >> XC_PAGE_SHIFT, - (e_size + XC_PAGE_SIZE - 1) >> XC_PAGE_SHIFT, - DPCI_REMOVE_MAPPING); + if (pt_has_msix_mapping(s, i)) { + memory_region_set_enabled(&s->msix->mmio, false); + } + rc = pt_iomem_helper(s, i, old_ebase, e_size, DPCI_REMOVE_MAPPING); if (rc) { PT_ERR(&s->dev, "remove old mapping failed! (rc: %i)\n", rc); return; @@ -391,21 +435,19 @@ static void pt_iomem_map(XenPCIPassthroughState *s, int i, /* map only valid guest address */ if (e_phys != PCI_BAR_UNMAPPED) { - /* Create new mapping */ - rc = xc_domain_memory_mapping(xen_xc, xen_domid, - s->bases[i].e_physbase >> XC_PAGE_SHIFT, - s->bases[i].access.maddr >> XC_PAGE_SHIFT, - (e_size+XC_PAGE_SIZE-1) >> XC_PAGE_SHIFT, - DPCI_ADD_MAPPING); + if (pt_has_msix_mapping(s, i)) { + XenPTMSIX *msix = s->msix; - if (rc) { - PT_ERR(&s->dev, "create new mapping failed! (rc: %i)\n", rc); + msix->mmio_base_addr = s->bases[i].e_physbase + msix->table_off; + + memory_region_set_enabled(&msix->mmio, true); } - rc = pt_remove_msix_mapping(s, i); - if (rc != 0) { - PT_ERR(&s->dev, "Remove MSI-X MMIO mapping failed! (rc: %d)\n", - rc); + rc = pt_iomem_helper(s, i, e_phys, e_size, DPCI_ADD_MAPPING); + + if (rc) { + PT_ERR(&s->dev, "create new mapping failed! (rc: %i)\n", rc); + return; } if (old_ebase != e_phys && old_ebase != -1) { diff --git a/hw/xen_pci_passthrough.h b/hw/xen_pci_passthrough.h index 3d41e04..f36e0d2 100644 --- a/hw/xen_pci_passthrough.h +++ b/hw/xen_pci_passthrough.h @@ -30,6 +30,9 @@ void pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3); #endif +/* Helper */ +#define PFN(x) ((x) >> XC_PAGE_SHIFT) + typedef struct XenPTRegInfo XenPTRegInfo; typedef struct XenPTReg XenPTReg; @@ -295,7 +298,11 @@ void pt_msix_delete(XenPCIPassthroughState *s); int pt_msix_update(XenPCIPassthroughState *s); int pt_msix_update_remap(XenPCIPassthroughState *s, int bar_index); void pt_msix_disable(XenPCIPassthroughState *s); -int pt_add_msix_mapping(XenPCIPassthroughState *s, int bar_index); -int pt_remove_msix_mapping(XenPCIPassthroughState *s, int bar_index); + +static inline bool pt_has_msix_mapping(XenPCIPassthroughState *s, int bar) +{ + return s->msix && s->msix->bar_index == bar; +} + #endif /* !QEMU_HW_XEN_PCI_PASSTHROUGH_H */ diff --git a/hw/xen_pci_passthrough_msi.c b/hw/xen_pci_passthrough_msi.c index e848c61..e775d21 100644 --- a/hw/xen_pci_passthrough_msi.c +++ b/hw/xen_pci_passthrough_msi.c @@ -300,16 +300,6 @@ static int msix_set_enable(XenPCIPassthroughState *s, bool enabled) enabled); } -static void mask_physical_msix_entry(XenPCIPassthroughState *s, - int entry_nr, int mask) -{ - void *phys_off; - - phys_off = s->msix->phys_iomem_base + PCI_MSIX_ENTRY_SIZE * entry_nr - + PCI_MSIX_ENTRY_VECTOR_CTRL; - *(uint32_t *)phys_off = mask; -} - static int pt_msix_update_one(XenPCIPassthroughState *s, int entry_nr) { XenPTMSIXEntry *entry = NULL; @@ -480,8 +470,6 @@ static void pci_msix_write(void *opaque, target_phys_addr_t addr, if (msix->enabled && !(val & PCI_MSIX_ENTRY_CTRL_MASKBIT)) { pt_msix_update_one(s, entry_nr); } - mask_physical_msix_entry(s, entry_nr, - entry->vector_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT); } } @@ -493,14 +481,19 @@ static uint64_t pci_msix_read(void *opaque, target_phys_addr_t addr, int entry_nr, offset; entry_nr = addr / PCI_MSIX_ENTRY_SIZE; - if (entry_nr < 0 || entry_nr >= msix->total_entries) { + if (entry_nr < 0) { PT_ERR(&s->dev, "asked MSI-X entry '%i' invalid!\n", entry_nr); return 0; } offset = addr % PCI_MSIX_ENTRY_SIZE; - return get_entry_value(&msix->msix_entry[entry_nr], offset); + if (addr < msix->total_entries * PCI_MSIX_ENTRY_SIZE) { + return get_entry_value(&msix->msix_entry[entry_nr], offset); + } else { + /* Pending Bit Array (PBA) */ + return *(uint32_t *)(msix->phys_iomem_base + addr); + } } static const MemoryRegionOps pci_msix_ops = { @@ -514,45 +507,6 @@ static const MemoryRegionOps pci_msix_ops = { }, }; -int pt_add_msix_mapping(XenPCIPassthroughState *s, int bar_index) -{ - XenPTMSIX *msix = s->msix; - - if (!(s->msix && s->msix->bar_index == bar_index)) { - return 0; - } - - memory_region_set_enabled(&msix->mmio, false); - return xc_domain_memory_mapping(xen_xc, xen_domid, - s->msix->mmio_base_addr >> XC_PAGE_SHIFT, - (s->bases[bar_index].access.maddr + s->msix->table_off) - >> XC_PAGE_SHIFT, - (s->msix->total_entries * PCI_MSIX_ENTRY_SIZE + XC_PAGE_SIZE - 1) - >> XC_PAGE_SHIFT, - DPCI_ADD_MAPPING); -} - -int pt_remove_msix_mapping(XenPCIPassthroughState *s, int bar_index) -{ - XenPTMSIX *msix = s->msix; - - if (!(msix && msix->bar_index == bar_index)) { - return 0; - } - - memory_region_set_enabled(&msix->mmio, true); - - msix->mmio_base_addr = s->bases[bar_index].e_physbase + msix->table_off; - - return xc_domain_memory_mapping(xen_xc, xen_domid, - s->msix->mmio_base_addr >> XC_PAGE_SHIFT, - (s->bases[bar_index].access.maddr + s->msix->table_off) - >> XC_PAGE_SHIFT, - (s->msix->total_entries * PCI_MSIX_ENTRY_SIZE + XC_PAGE_SIZE - 1) - >> XC_PAGE_SHIFT, - DPCI_REMOVE_MAPPING); -} - int pt_msix_init(XenPCIPassthroughState *s, uint32_t base) { uint8_t id = 0; @@ -609,7 +563,7 @@ int pt_msix_init(XenPCIPassthroughState *s, uint32_t base) msix->phys_iomem_base = mmap(NULL, total_entries * PCI_MSIX_ENTRY_SIZE + msix->table_offset_adjust, - PROT_WRITE | PROT_READ, + PROT_READ, MAP_SHARED | MAP_LOCKED, fd, msix->table_base + table_off - msix->table_offset_adjust);