From patchwork Mon Feb 15 10:15:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 8312301 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 95931C02AA for ; Mon, 15 Feb 2016 10:16:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9CDF020481 for ; Mon, 15 Feb 2016 10:16:10 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B53F02041F for ; Mon, 15 Feb 2016 10:16:09 +0000 (UTC) Received: from localhost ([::1]:58624 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVGCP-0007V4-1L for patchwork-qemu-devel@patchwork.kernel.org; Mon, 15 Feb 2016 05:16:09 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48393) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVGC5-0007KE-11 for qemu-devel@nongnu.org; Mon, 15 Feb 2016 05:15:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVGC0-0000qR-57 for qemu-devel@nongnu.org; Mon, 15 Feb 2016 05:15:48 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:18454) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVGBz-0000pn-Lu for qemu-devel@nongnu.org; Mon, 15 Feb 2016 05:15:43 -0500 X-IronPort-AV: E=Sophos;i="5.22,449,1449532800"; d="scan'208";a="338195208" Date: Mon, 15 Feb 2016 10:15:12 +0000 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: "Michael S. Tsirkin" In-Reply-To: <1455389632-6840-1-git-send-email-mst@redhat.com> Message-ID: References: <1455389632-6840-1-git-send-email-mst@redhat.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-DLP: MIA1 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.165.176.63 Cc: qemu-devel@nongnu.org, Stefano Stabellini Subject: Re: [Qemu-devel] [PATCH] msix: fix msix_vector_masked 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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, 13 Feb 2016, Michael S. Tsirkin wrote: > commit 428c3ece97179557f2753071fb0ca97a03437267 ("fix MSI injection on Xen") > inadvertently enabled the xen-specific logic unconditionally. > Limit it to only when xen is enabled. > Additionally, msix data should be read with pci_get_log > since the format is pci little-endian. > > Reported-by: "Daniel P. Berrange" > Cc: Stefano Stabellini > Signed-off-by: Michael S. Tsirkin Thanks Daniel for finding the issue and thanks Michael for fixing my bug, sorry about that. > hw/pci/msix.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c > index eb4ef11..537fdba 100644 > --- a/hw/pci/msix.c > +++ b/hw/pci/msix.c > @@ -80,10 +80,10 @@ static void msix_clr_pending(PCIDevice *dev, int vector) > static bool msix_vector_masked(PCIDevice *dev, unsigned int vector, bool fmask) > { > unsigned offset = vector * PCI_MSIX_ENTRY_SIZE; > - uint32_t *data = (uint32_t *)&dev->msix_table[offset + PCI_MSIX_ENTRY_DATA]; > + uint8_t *data = &dev->msix_table[offset + PCI_MSIX_ENTRY_DATA]; > /* MSIs on Xen can be remapped into pirqs. In those cases, masking > * and unmasking go through the PV evtchn path. */ > - if (xen_is_pirq_msi(*data)) { > + if (xen_enabled() && xen_is_pirq_msi(pci_get_long(data))) { > return false; > } > return fmask || dev->msix_table[offset + PCI_MSIX_ENTRY_VECTOR_CTRL] & I think this is good, but moving the xen_enabled() check inside xen_is_pirq_msi is even be better, so that we cover all call sites at once. diff --git a/xen-hvm.c b/xen-hvm.c index 039680a..991f6b7 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -151,7 +151,8 @@ int xen_is_pirq_msi(uint32_t msi_data) /* If vector is 0, the msi is remapped into a pirq, passed as * dest_id. */ - return ((msi_data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT) == 0; + return xen_enabled() && + ((msi_data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT) == 0; } void xen_hvm_inject_msi(uint64_t addr, uint32_t data)