From patchwork Mon May 18 04:42:54 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hidetoshi Seto X-Patchwork-Id: 24384 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n4I4huUO005656 for ; Mon, 18 May 2009 04:43:57 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751621AbZEREnx (ORCPT ); Mon, 18 May 2009 00:43:53 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751968AbZEREnx (ORCPT ); Mon, 18 May 2009 00:43:53 -0400 Received: from fgwmail5.fujitsu.co.jp ([192.51.44.35]:53595 "EHLO fgwmail5.fujitsu.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751621AbZEREnx (ORCPT ); Mon, 18 May 2009 00:43:53 -0400 Received: from m3.gw.fujitsu.co.jp ([10.0.50.73]) by fgwmail5.fujitsu.co.jp (Fujitsu Gateway) with ESMTP id n4I4hrOc029718 for (envelope-from seto.hidetoshi@jp.fujitsu.com); Mon, 18 May 2009 13:43:53 +0900 Received: from smail (m3 [127.0.0.1]) by outgoing.m3.gw.fujitsu.co.jp (Postfix) with ESMTP id C3CB945DE5D for ; Mon, 18 May 2009 13:43:52 +0900 (JST) Received: from s3.gw.fujitsu.co.jp (s3.gw.fujitsu.co.jp [10.0.50.93]) by m3.gw.fujitsu.co.jp (Postfix) with ESMTP id 1155545DD7F for ; Mon, 18 May 2009 13:43:52 +0900 (JST) Received: from s3.gw.fujitsu.co.jp (localhost.localdomain [127.0.0.1]) by s3.gw.fujitsu.co.jp (Postfix) with ESMTP id E75BAE08006 for ; Mon, 18 May 2009 13:43:51 +0900 (JST) Received: from m106.s.css.fujitsu.com (m106.s.css.fujitsu.com [10.249.87.106]) by s3.gw.fujitsu.co.jp (Postfix) with ESMTP id 6EFD61DB803C for ; Mon, 18 May 2009 13:43:51 +0900 (JST) Received: from m106.css.fujitsu.com (m106 [127.0.0.1]) by m106.s.css.fujitsu.com (Postfix) with ESMTP id 3C6345B8711; Mon, 18 May 2009 13:43:51 +0900 (JST) Received: from [127.0.0.1] (unknown [10.124.100.141]) by m106.s.css.fujitsu.com (Postfix) with ESMTP id BB2E45B859C; Mon, 18 May 2009 13:43:50 +0900 (JST) Message-ID: <4A10E74E.40503@jp.fujitsu.com> Date: Mon, 18 May 2009 13:42:54 +0900 From: Hidetoshi Seto User-Agent: Thunderbird 2.0.0.21 (Windows/20090302) MIME-Version: 1.0 To: linux-pci@vger.kernel.org CC: Michael Ellerman , Matthew Wilcox Subject: [PATCH] pci, msi: Rework error path of msix_capability_init() v2 References: <4A10D677.3090206@jp.fujitsu.com> In-Reply-To: <4A10D677.3090206@jp.fujitsu.com> Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Using msi_free_irqs() in error path of msix_capability_init() has an problem - It does writel to Vector Control register. Since reading the original value of the register (that includes preserved bits) was moved to later by previous patch for NIU problem, write before the read will violate PCI spec. And there was no iounmap if no msi_disc is allocated to the device. This patch restructures the error path of msix_capability_init() to fix above problems. v2: Add dropped arch_teardown_msi_irqs() Signed-off-by: Hidetoshi Seto --- drivers/pci/msi.c | 57 ++++++++++++++++++++++++++++++++-------------------- 1 files changed, 35 insertions(+), 22 deletions(-) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index d55db55..f8e41f6 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -446,10 +446,8 @@ static int msix_capability_init(struct pci_dev *dev, for (i = 0; i < nvec; i++) { entry = alloc_msi_entry(dev); if (!entry) { - if (!i) - return -ENOMEM; - msi_free_irqs(dev); - return i; + ret = (i > 0) ? i : -ENOMEM; + goto error1; } j = entries[i].entry; @@ -465,24 +463,8 @@ static int msix_capability_init(struct pci_dev *dev, } ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX); - if (ret < 0) { - /* If we had some success report the number of irqs - * we succeeded in setting up. */ - int avail = 0; - list_for_each_entry(entry, &dev->msi_list, list) { - if (entry->irq != 0) { - avail++; - } - } - - if (avail != 0) - ret = avail; - } - - if (ret) { - msi_free_irqs(dev); - return ret; - } + if (ret) + goto error2; i = 0; list_for_each_entry(entry, &dev->msi_list, list) { @@ -502,6 +484,37 @@ static int msix_capability_init(struct pci_dev *dev, } return 0; + +error2: + if (ret < 0) { + /* + * If we had some success report the number of irqs + * we succeeded in setting up. + */ + int avail = 0; + + list_for_each_entry(entry, &dev->msi_list, list) { + if (entry->irq != 0) + avail++; + } + + if (avail != 0) + ret = avail; + } + arch_teardown_msi_irqs(dev); +error1: + { + struct msi_desc *tmp; + + list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) { + list_del(&entry->list); + kfree(entry); + } + } + + iounmap(base); + + return ret; } /**