From patchwork Wed Oct 16 06:33:04 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhenzhong Duan X-Patchwork-Id: 3050551 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id F185ABF924 for ; Wed, 16 Oct 2013 06:32:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8CFBA2045A for ; Wed, 16 Oct 2013 06:32:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 292E0201FD for ; Wed, 16 Oct 2013 06:32:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751155Ab3JPGco (ORCPT ); Wed, 16 Oct 2013 02:32:44 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:23367 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751094Ab3JPGcn (ORCPT ); Wed, 16 Oct 2013 02:32:43 -0400 Received: from ucsinet21.oracle.com (ucsinet21.oracle.com [156.151.31.93]) by aserp1040.oracle.com (Sentrion-MTA-4.3.1/Sentrion-MTA-4.3.1) with ESMTP id r9G6WcoP014808 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 16 Oct 2013 06:32:39 GMT Received: from userz7021.oracle.com (userz7021.oracle.com [156.151.31.85]) by ucsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id r9G6Wbx5011309 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 16 Oct 2013 06:32:37 GMT Received: from abhmt102.oracle.com (abhmt102.oracle.com [141.146.116.54]) by userz7021.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id r9G6Wb7S011277; Wed, 16 Oct 2013 06:32:37 GMT Received: from [10.191.7.247] (/10.191.7.247) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 15 Oct 2013 23:32:36 -0700 Message-ID: <525E3320.5080700@oracle.com> Date: Wed, 16 Oct 2013 14:33:04 +0800 From: Zhenzhong Duan Reply-To: zhenzhong.duan@oracle.com Organization: oracle User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 MIME-Version: 1.0 To: "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , xen-devel CC: Bjorn Helgaas , Konrad Rzeszutek Wilk , Feng Jin , Sucheta Chakraborty Subject: [PATCH 2/3] PCI: Refactor MSI/MSIX mask restore code to fix interrupt lost issue X-Source-IP: ucsinet21.oracle.com [156.151.31.93] Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, 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 Driver init call graph under baremetal: driver_init-> msix_capability_init-> msix_program_entries-> msix_mask_irq-> entry->masked = 1 request_irq-> __setup_irq-> irq_startup-> unmask_msi_irq-> msix_mask_irq-> entry->masked = 0; So entry->masked is always updated with newest value and its value could be used to restore to mask register in device. But in initial domain (aka priviliged guest), it's different. Driver init call graph under initial domain: driver_init-> msix_capability_init-> msix_program_entries-> msix_mask_irq-> entry->masked = 1 request_irq-> __setup_irq-> irq_startup-> __startup_pirq-> EVTCHNOP_bind_pirq hypercall (trap into Xen) [Xen:] pirq_guest_bind-> startup_msi_irq-> unmask_msi_irq-> msi_set_mask_bit-> entry->msi_attrib.masked = 0; So entry->msi_attrib.masked in xen side always has newest value. entry->masked in initial domain is untouched and is 1 after msix_capability_init. Based on above, it's Xen's duty to restore entry->msi_attrib.masked to device, but with current code, entry->masked is used and MSI-x interrupt is masked. Before patch, restore call graph under initial domain: pci_reset_function-> pci_restore_state-> __pci_restore_msix_state-> arch_restore_msi_irqs-> xen_initdom_restore_msi_irqs-> PHYSDEVOP_restore_msi hypercall (first mask restore) msix_mask_irq(entry, entry->masked) (second mask restore) So msix_mask_irq call in initial domain call graph needs to be removed. Based on this we can move the restore of the mask in default_restore_msi_irqs which would avoid restoring the invalid mask under Xen. Furthermore this simplifies the API by making everything related to restoring an MSI be in the platform specific APIs instead of just parts of it. After patch, restore call graph under initial domain: pci_reset_function-> pci_restore_state-> __pci_restore_msix_state-> arch_restore_msi_irqs-> xen_initdom_restore_msi_irqs-> PHYSDEVOP_restore_msi hypercall (first mask restore) Logic for baremetal is not changed. Before patch, restore call graph under baremetal: pci_reset_function-> pci_restore_state-> __pci_restore_msix_state-> arch_restore_msi_irqs-> default_restore_msi_irqs-> msix_mask_irq(entry, entry->masked) (first mask restore) After patch, restore call graph under baremetal: pci_reset_function-> pci_restore_state-> __pci_restore_msix_state-> arch_restore_msi_irqs-> default_restore_msi_irqs-> msix_mask_irq(entry, entry->masked) (first mask restore) The process for MSI code is similiar. Tested-by: Sucheta Chakraborty Signed-off-by: Zhenzhong Duan Acked-by: Konrad Rzeszutek Wilk Reviewed-by: Jingoo Han --- drivers/pci/msi.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index ecd4cdf..38237f4 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -236,6 +236,8 @@ void unmask_msi_irq(struct irq_data *data) void default_restore_msi_irqs(struct pci_dev *dev, int irq) { + int pos; + u16 control; struct msi_desc *entry; entry = NULL; @@ -248,8 +250,19 @@ void default_restore_msi_irqs(struct pci_dev *dev, int irq) entry = irq_get_msi_desc(irq); } - if (entry) + if (entry) { write_msi_msg(irq, &entry->msg); + if (dev->msix_enabled) { + msix_mask_irq(entry, entry->masked); + readl(entry->mask_base); + } else { + pos = entry->msi_attrib.pos; + pci_read_config_word(dev, pos + PCI_MSI_FLAGS, + &control); + msi_mask_irq(entry, msi_capable_mask(control), + entry->masked); + } + } } void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg) @@ -423,7 +436,6 @@ static void __pci_restore_msi_state(struct pci_dev *dev) arch_restore_msi_irqs(dev, dev->irq); pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control); - msi_mask_irq(entry, msi_capable_mask(control), entry->masked); control &= ~PCI_MSI_FLAGS_QSIZE; control |= (entry->msi_attrib.multiple << 4) | PCI_MSI_FLAGS_ENABLE; pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control); @@ -447,7 +459,6 @@ static void __pci_restore_msix_state(struct pci_dev *dev) list_for_each_entry(entry, &dev->msi_list, list) { arch_restore_msi_irqs(dev, entry->irq); - msix_mask_irq(entry, entry->masked); } control &= ~PCI_MSIX_FLAGS_MASKALL;