From patchwork Thu Apr 25 16:42:38 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 2488701 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 06118DF5B1 for ; Thu, 25 Apr 2013 16:42:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758985Ab3DYQmn (ORCPT ); Thu, 25 Apr 2013 12:42:43 -0400 Received: from mail-gg0-f172.google.com ([209.85.161.172]:39948 "EHLO mail-gg0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758982Ab3DYQmm (ORCPT ); Thu, 25 Apr 2013 12:42:42 -0400 Received: by mail-gg0-f172.google.com with SMTP id f1so419890ggn.31 for ; Thu, 25 Apr 2013 09:42:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=usWKf21JRfD//aQJe1vtYnqzsoClg7Qnlbl2ZT6yq7k=; b=ZqZewx6umzfcjZgAnrQR88dteyyuTSwx303oLhtoOOSfQK+YDqdX3vrCkQZvvFPe8p HRR7spkT50fLGqeBuKa+vDG3C3xbVy4k6YJ5ssMpE/hkZhAc/DvWrhYLqC6LTjch3h/j L9llM6gPr8YoKLuaE6N7I7SYwj/bWx8McYTjvtt+dU3mVPv4GKuvsaigsY33JlEIL+da opFVx0Y1tH3vyImRggyCXOD0yfoiO7YCdTAlSCeRXDbHaRrJPmx9Jry73qiZg5OnUqbO +2fNE/I2y/OWjBgjFw+pd3nwtkflKcRVXVQZEaV6kqPF2+wRGzG0rfpBWJL5XEfHYkUX Vr7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent:x-gm-message-state; bh=usWKf21JRfD//aQJe1vtYnqzsoClg7Qnlbl2ZT6yq7k=; b=YYGNTVeudltdEbYdt/zFtFRG9dZz3522cLaOY3NrrHZqxLy3hvad8d+E1QjFSluJNy 4he1cvs+LbGEuNksDjETEWyW+bpeM8ieQ3MJ5JJA9UPAtZMpG2wqWHzQ5VwQamC6x4Z0 gzc6GqasP4FK1TzHJEvDweoVYoEjIvZeBIaM27Cd/PfDb3S80Ib3HdKZ96iMF3FsT+Mb tHfrfQuMslEpoocdLQjT1/5aYMBQjHrNuoRau+xY1j9x0fCIBVnoOU+UHEhQt9ZVHfeC hAezb1391kb1/HfLWcgxTBQPkUAVL6qL9gEysj6PBTqlKOEMApFXQXV+gYKD/hqs5OSt Gz3Q== X-Received: by 10.236.160.233 with SMTP id u69mr25647472yhk.124.1366908161770; Thu, 25 Apr 2013 09:42:41 -0700 (PDT) Received: from google.com ([172.29.121.89]) by mx.google.com with ESMTPSA id u33sm10647217yhn.7.2013.04.25.09.42.40 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 25 Apr 2013 09:42:41 -0700 (PDT) Date: Thu, 25 Apr 2013 10:42:38 -0600 From: Bjorn Helgaas To: Jan Beulich Cc: Jeremy Fitzhardinge , Gavin Shan , xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk , "linux-pci@vger.kernel.org" Subject: Re: [Xen-devel] [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET Message-ID: <20130425164238.GA3052@google.com> References: <20130422230012.32621.15224.stgit@bhelgaas-glaptop> <20130422231214.32621.37394.stgit@bhelgaas-glaptop> <5179164902000078000D0A26@nat28.tlf.novell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5179164902000078000D0A26@nat28.tlf.novell.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQmvcRNs4CINef1m1r7Z5MYrzPnZpnRqw0vgHQh7cPrv4RDBU9mf6dNNzVSfxmWlb9nAVCRMfuq+i6D4EL7clPKm1ttGKjM4uNju+1THsrLhg9KExNWghUIQPV2UmsgTA/SAZ++hNdTRrIIr3wpsI5hM7Fpco6bwRpru7WDvdXZOSAcJUuhr8VIxpHw9gQ5IP/h/cs0PeaNHIDow6a34IfeX75H7CQ== Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Thu, Apr 25, 2013 at 10:40:57AM +0100, Jan Beulich wrote: > >>> On 24.04.13 at 18:34, Bjorn Helgaas wrote: > > On Mon, Apr 22, 2013 at 5:12 PM, Bjorn Helgaas wrote: > >> The MSI-X Table structure may be at a non-zero offset into the > >> device BAR, and we should account for that. > > NAK: The base is just being use to pass to the hypervisor, which > then takes care to add the offset. Thanks for noticing this. I updated the patch to just add a comment to avert future confusion, and refreshed the subsequent xen patches as below. > Recent hypervisors will actually > only consume this to issue a warning if not matching what gets > read from the corresponding BAR. Earlier hypervisors used this > instead of reading the BAR. Note that pci_resource_start() gives you a CPU address, and what you read from the BAR is a PCI bus address. These are not in the same address space and can't be directly compared. I assume the hypervisors take that into account and do the appropriate conversions? Bjorn commit 34f394aad6706a42fb69922ed184c918ec9f9f81 Author: Bjorn Helgaas Date: Thu Apr 18 15:02:34 2013 -0600 xen/pci: Comment on unusual PCI_MSIX_TABLE usage To find the MSI-X Table structure, you look at the BAR specified by PCI_MSIX_FLAGS_BIRMASK, then apply the PCI_MSIX_TABLE_OFFSET to the address from the BAR. So most readers of PCI_MSIX_TABLE use PCI_MSIX_TABLE_OFFSET, but in xen's case, the hypervisor takes care of applying the offset, so we don't need to do it here. Signed-off-by: Bjorn Helgaas CC: Konrad Rzeszutek Wilk CC: Jan Beulich --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index 94e7662..96e44fc 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -301,6 +301,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) &table_offset); bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK); + /* Hypervisor takes care of PCI_MSIX_TABLE_OFFSET */ map_irq.table_base = pci_resource_start(dev, bir); map_irq.entry_nr = msidesc->msi_attrib.entry_nr; } commit 311d82d74afa19cb0ea03c516e290457f9aab63d Author: Bjorn Helgaas Date: Thu Apr 18 12:40:33 2013 -0600 xen/pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK PCI_MSIX_FLAGS_BIRMASK is mis-named because the BIR mask is in the Table Offset register, not the flags ("Message Control" per spec) register. Signed-off-by: Bjorn Helgaas CC: Konrad Rzeszutek Wilk CC: Jan Beulich diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index 96e44fc..bec03d4 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -299,7 +299,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) pci_read_config_dword(dev, pos + PCI_MSIX_TABLE, &table_offset); - bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK); + bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR); /* Hypervisor takes care of PCI_MSIX_TABLE_OFFSET */ map_irq.table_base = pci_resource_start(dev, bir); commit 4a5b938fe2ed3e5f5c51f4906c0d1c1486f37e49 Author: Bjorn Helgaas Date: Thu Apr 18 15:10:58 2013 -0600 xen/pci: Used cached MSI-X capability offset We now cache the MSI-X capability offset in the struct pci_dev, so no need to find the capability again. Signed-off-by: Bjorn Helgaas CC: Konrad Rzeszutek Wilk CC: Jan Beulich diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index bec03d4..a151b02 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -295,8 +295,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) int pos; u32 table_offset, bir; - pos = pci_find_capability(dev, PCI_CAP_ID_MSIX); - + pos = dev->msix_cap; pci_read_config_dword(dev, pos + PCI_MSIX_TABLE, &table_offset); bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);