From patchwork Tue Dec 22 16:22:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 560140 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 62AFE140313 for ; Wed, 23 Dec 2015 03:23:00 +1100 (AEDT) Received: from localhost ([::1]:51454 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBPiE-0000S7-7S for incoming@patchwork.ozlabs.org; Tue, 22 Dec 2015 11:22:58 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBPhn-0008Hk-1P for qemu-devel@nongnu.org; Tue, 22 Dec 2015 11:22:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBPhj-00080E-A9 for qemu-devel@nongnu.org; Tue, 22 Dec 2015 11:22:30 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:5717) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBPhj-000802-3Z for qemu-devel@nongnu.org; Tue, 22 Dec 2015 11:22:27 -0500 X-IronPort-AV: E=Sophos;i="5.20,465,1444694400"; d="scan'208";a="326970604" From: Stefano Stabellini To: Date: Tue, 22 Dec 2015 16:22:14 +0000 Message-ID: <1450801337-26849-1-git-send-email-stefano.stabellini@eu.citrix.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: References: MIME-Version: 1.0 X-DLP: MIA2 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.165.176.63 Cc: xen-devel@lists.xensource.com, qemu-devel@nongnu.org, Jan Beulich , stefano.stabellini@eu.citrix.com Subject: [Qemu-devel] [PULL 1/4] xen/MSI-X: latch MSI-X table writes 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: Jan Beulich The remaining log message in pci_msix_write() is wrong, as there guest behavior may only appear to be wrong: For one, the old logic didn't take the mask-all bit into account. And then this shouldn't depend on host device state (i.e. the host may have masked the entry without the guest having done so). Plus these writes shouldn't be dropped even when an entry gets unmasked. Instead, if they can't be made take effect right away, they should take effect on the next unmasking or enabling operation - the specification explicitly describes such caching behavior. Signed-off-by: Jan Beulich Signed-off-by: Stefano Stabellini Reviewed-by: Stefano Stabellini --- hw/xen/xen_pt.h | 4 +-- hw/xen/xen_pt_config_init.c | 2 ++ hw/xen/xen_pt_msi.c | 74 ++++++++++++++++--------------------------- 3 files changed, 32 insertions(+), 48 deletions(-) diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index c545280..4f922f4 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -187,13 +187,13 @@ typedef struct XenPTMSIXEntry { int pirq; uint64_t addr; uint32_t data; - uint32_t vector_ctrl; + uint32_t latch[4]; bool updated; /* indicate whether MSI ADDR or DATA is updated */ - bool warned; /* avoid issuing (bogus) warning more than once */ } XenPTMSIXEntry; typedef struct XenPTMSIX { uint32_t ctrl_offset; bool enabled; + bool maskall; int total_entries; int bar_index; uint64_t table_base; diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c index 3d8686d..4fd7206 100644 --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -1499,6 +1499,8 @@ static int xen_pt_msixctrl_reg_write(XenPCIPassthroughState *s, xen_pt_msix_disable(s); } + s->msix->maskall = *val & PCI_MSIX_FLAGS_MASKALL; + debug_msix_enabled_old = s->msix->enabled; s->msix->enabled = !!(*val & PCI_MSIX_FLAGS_ENABLE); if (s->msix->enabled != debug_msix_enabled_old) { diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c index 82de2bc..e10cd2a 100644 --- a/hw/xen/xen_pt_msi.c +++ b/hw/xen/xen_pt_msi.c @@ -25,6 +25,7 @@ #define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12 #define XEN_PT_GFLAGSSHIFT_TRG_MODE 15 +#define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)] /* * Helpers @@ -314,7 +315,8 @@ static int msix_set_enable(XenPCIPassthroughState *s, bool enabled) enabled); } -static int xen_pt_msix_update_one(XenPCIPassthroughState *s, int entry_nr) +static int xen_pt_msix_update_one(XenPCIPassthroughState *s, int entry_nr, + uint32_t vec_ctrl) { XenPTMSIXEntry *entry = NULL; int pirq; @@ -332,6 +334,19 @@ static int xen_pt_msix_update_one(XenPCIPassthroughState *s, int entry_nr) pirq = entry->pirq; + /* + * Update the entry addr and data to the latest values only when the + * entry is masked or they are all masked, as required by the spec. + * Addr and data changes while the MSI-X entry is unmasked get deferred + * until the next masked -> unmasked transition. + */ + if (pirq == XEN_PT_UNASSIGNED_PIRQ || s->msix->maskall || + (vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT)) { + entry->addr = entry->latch(LOWER_ADDR) | + ((uint64_t)entry->latch(UPPER_ADDR) << 32); + entry->data = entry->latch(DATA); + } + rc = msi_msix_setup(s, entry->addr, entry->data, &pirq, true, entry_nr, entry->pirq == XEN_PT_UNASSIGNED_PIRQ); if (rc) { @@ -357,7 +372,7 @@ int xen_pt_msix_update(XenPCIPassthroughState *s) int i; for (i = 0; i < msix->total_entries; i++) { - xen_pt_msix_update_one(s, i); + xen_pt_msix_update_one(s, i, msix->msix_entry[i].latch(VECTOR_CTRL)); } return 0; @@ -406,35 +421,15 @@ int xen_pt_msix_update_remap(XenPCIPassthroughState *s, int bar_index) static uint32_t get_entry_value(XenPTMSIXEntry *e, int offset) { - switch (offset) { - case PCI_MSIX_ENTRY_LOWER_ADDR: - return e->addr & UINT32_MAX; - case PCI_MSIX_ENTRY_UPPER_ADDR: - return e->addr >> 32; - case PCI_MSIX_ENTRY_DATA: - return e->data; - case PCI_MSIX_ENTRY_VECTOR_CTRL: - return e->vector_ctrl; - default: - return 0; - } + return !(offset % sizeof(*e->latch)) + ? e->latch[offset / sizeof(*e->latch)] : 0; } static void set_entry_value(XenPTMSIXEntry *e, int offset, uint32_t val) { - switch (offset) { - case PCI_MSIX_ENTRY_LOWER_ADDR: - e->addr = (e->addr & ((uint64_t)UINT32_MAX << 32)) | val; - break; - case PCI_MSIX_ENTRY_UPPER_ADDR: - e->addr = (uint64_t)val << 32 | (e->addr & UINT32_MAX); - break; - case PCI_MSIX_ENTRY_DATA: - e->data = val; - break; - case PCI_MSIX_ENTRY_VECTOR_CTRL: - e->vector_ctrl = val; - break; + if (!(offset % sizeof(*e->latch))) + { + e->latch[offset / sizeof(*e->latch)] = val; } } @@ -454,39 +449,26 @@ static void pci_msix_write(void *opaque, hwaddr addr, offset = addr % PCI_MSIX_ENTRY_SIZE; if (offset != PCI_MSIX_ENTRY_VECTOR_CTRL) { - const volatile uint32_t *vec_ctrl; - if (get_entry_value(entry, offset) == val && entry->pirq != XEN_PT_UNASSIGNED_PIRQ) { return; } + entry->updated = true; + } else if (msix->enabled && entry->updated && + !(val & PCI_MSIX_ENTRY_CTRL_MASKBIT)) { + const volatile uint32_t *vec_ctrl; + /* * If Xen intercepts the mask bit access, entry->vec_ctrl may not be * up-to-date. Read from hardware directly. */ vec_ctrl = s->msix->phys_iomem_base + entry_nr * PCI_MSIX_ENTRY_SIZE + PCI_MSIX_ENTRY_VECTOR_CTRL; - - if (msix->enabled && !(*vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT)) { - if (!entry->warned) { - entry->warned = true; - XEN_PT_ERR(&s->dev, "Can't update msix entry %d since MSI-X is" - " already enabled.\n", entry_nr); - } - return; - } - - entry->updated = true; + xen_pt_msix_update_one(s, entry_nr, *vec_ctrl); } set_entry_value(entry, offset, val); - - if (offset == PCI_MSIX_ENTRY_VECTOR_CTRL) { - if (msix->enabled && !(val & PCI_MSIX_ENTRY_CTRL_MASKBIT)) { - xen_pt_msix_update_one(s, entry_nr); - } - } } static uint64_t pci_msix_read(void *opaque, hwaddr addr,