From patchwork Tue Oct 9 08:47:19 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Liu X-Patchwork-Id: 190242 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0B28A2C0087 for ; Tue, 9 Oct 2012 19:48:29 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752637Ab2JIIsY (ORCPT ); Tue, 9 Oct 2012 04:48:24 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:34478 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752124Ab2JIIsX (ORCPT ); Tue, 9 Oct 2012 04:48:23 -0400 Received: from 172.24.2.119 (EHLO szxeml209-edg.china.huawei.com) ([172.24.2.119]) by szxrg02-dlp.huawei.com (MOS 4.3.4-GA FastPath queued) with ESMTP id AQG89104; Tue, 09 Oct 2012 16:48:17 +0800 (CST) Received: from SZXEML404-HUB.china.huawei.com (10.82.67.59) by szxeml209-edg.china.huawei.com (172.24.2.184) with Microsoft SMTP Server (TLS) id 14.1.323.3; Tue, 9 Oct 2012 16:47:30 +0800 Received: from [127.0.0.1] (10.108.108.229) by szxeml404-hub.china.huawei.com (10.82.67.59) with Microsoft SMTP Server id 14.1.323.3; Tue, 9 Oct 2012 16:47:23 +0800 Message-ID: <5073E497.8020909@huawei.com> Date: Tue, 9 Oct 2012 16:47:19 +0800 From: Jiang Liu User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20111222 Thunderbird/9.0.1 MIME-Version: 1.0 To: Yijing Wang CC: Bjorn Helgaas , Hanjun Guo , , Yinghai Lu Subject: Re: [PATCH 0/7] disable bridge ari forwarding after connected ari device hot removed References: <1349751804-7476-1-git-send-email-wangyijing@huawei.com> <50739D40.8040005@huawei.com> <5073D56C.40705@huawei.com> <5073D76C.9090304@huawei.com> <5073DCCB.2050503@huawei.com> In-Reply-To: <5073DCCB.2050503@huawei.com> X-Originating-IP: [10.108.108.229] X-CFilter-Loop: Reflected Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Please use this as your base On 2012-10-9 16:14, Yijing Wang wrote: > On 2012/10/9 15:51, Jiang Liu wrote: >> When scanning PCI devices for hot-add, it guarantee function 0 is scanned >> at first. And when destroying PCI devices for hot-removal, we also need to >> destroy function 0 at last, but currently there's no such guarantees. >> > > This is an issue, btw in pciehp now only try to hot remove fun from 0 --> 8, > ARI device supports up to 256 function device. > for (j = 0; j < 8; j++) { > struct pci_dev *temp = pci_get_slot(parent, PCI_DEVFN(0, j)); > if (!temp) > continue; > > >> On 2012-10-9 15:42, Yijing Wang wrote: >>> On 2012/10/9 11:42, Jiang Liu wrote: >>>> How about this patch, which disables ARI when adding new PCI devices? >>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >>>> index 5485883..c841aa6 100644 >>>> --- a/drivers/pci/pci.c >>>> +++ b/drivers/pci/pci.c >>>> @@ -2016,13 +2016,14 @@ void pci_free_cap_save_buffers(struct pci_dev *dev) >>>> void pci_enable_ari(struct pci_dev *dev) >>>> { >>> >>> I think it's ok. Delay to disable ARI until next hot add is safe. >>> Maybe rename as pci_configure_ari is better. >>> >>>> u32 cap; >>>> + bool enable = true; >>>> struct pci_dev *bridge; >>>> >>>> if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn) >>>> return; >>>> >>>> if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) >>>> - return; >>>> + enable = false; >>>> >>>> bridge = dev->bus->self; >>>> if (!bridge) >>>> @@ -2032,8 +2033,15 @@ void pci_enable_ari(struct pci_dev *dev) >>>> if (!(cap & PCI_EXP_DEVCAP2_ARI)) >>>> return; >>>> >>>> - pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_ARI); >>>> - bridge->ari_enabled = 1; >>>> + if (enable) { >>>> + pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2, >>>> + PCI_EXP_DEVCTL2_ARI); >>>> + bridge->ari_enabled = 1; >>>> + } else { >>>> + pcie_capability_clear_word(bridge, PCI_EXP_DEVCTL2, >>>> + PCI_EXP_DEVCTL2_ARI); >>>> + bridge->ari_enabled = 0; >>>> + } >>>> } >>>> >>>> /** >>>> >>>> On 2012-10-9 11:03, Yijing Wang wrote: >>>>> pci_enable_ari will be called if an ARI pci device found, then set its bridge ARI Forwarding Enable >>>>> bit in Device Control 2 Register. But the bridge ARI Forwarding Enable bit will never be cleared >>>>> when an ARI device hot removed. >>>>> >>>>> my steps: >>>>> 1. Hot add an ARI pci device; >>>>> 2. Hot remove the ARI pci device; >>>>> 3. Hot add an non ARI pci device; >>>>> >>>>> In this case, after setp 3, we could only find fun 0 of non ARI pci device because of its bridge ARI Forwarding Enable >>>>> bit set. >>>>> >>>>> As PCIe Spec 2.0(6.13/441) recommends: >>>>> "Following a hot-plug event below a Downstream Port, it is strongly recommended that software >>>>> Clear the ARI Forwarding Enable bit in the Downstream Port until software determines that a >>>>> newly added component is in fact an ARI Device" >>>>> >>>>> This series of patches fix this problem. >>>>> >>>>> Yijing Wang (7): >>>>> PCI: rework pci_enable_ari for support disable ari forwarding >>>>> PCI, acpiphp: disable ARI forwarding for acpiphp >>>>> PCI, pciehp: disable ARI forwarding for pciehp >>>>> PCI, cpqphp: disable ARI forwarding for cpqphp >>>>> PCI, shpchp: disable ARI forwarding for shpchp >>>>> PCI, sgi: disable ARI forwarding for sgiphp >>>>> PCI, ibmphp: disable ARI forwarding for ibmphp >>>>> >>>>> drivers/pci/hotplug/acpiphp_glue.c | 1 + >>>>> drivers/pci/hotplug/cpci_hotplug_pci.c | 1 + >>>>> drivers/pci/hotplug/cpqphp_pci.c | 1 + >>>>> drivers/pci/hotplug/ibmphp_core.c | 1 + >>>>> drivers/pci/hotplug/pciehp_pci.c | 1 + >>>>> drivers/pci/hotplug/sgi_hotplug.c | 1 + >>>>> drivers/pci/hotplug/shpchp_pci.c | 1 + >>>>> drivers/pci/pci.c | 16 +++++++++++----- >>>>> drivers/pci/pci.h | 2 +- >>>>> drivers/pci/probe.c | 2 +- >>>>> 10 files changed, 20 insertions(+), 7 deletions(-) >>>>> >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>>> >>>> . >>>> >>> >>> >> >> >> >> . >> > > --- 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/drivers/pci/pci.c b/drivers/pci/pci.c index 5485883..8a277d7 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2018,22 +2018,25 @@ void pci_enable_ari(struct pci_dev *dev) u32 cap; struct pci_dev *bridge; - if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn) - return; - - if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) + if (pcie_ari_disabled || !pci_is_pcie(dev)) return; bridge = dev->bus->self; - if (!bridge) + if (!bridge || !list_empty(bridge->bus_list)) return; - pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap); if (!(cap & PCI_EXP_DEVCAP2_ARI)) return; - pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_ARI); - bridge->ari_enabled = 1; + if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) { + pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2, + PCI_EXP_DEVCTL2_ARI); + bridge->ari_enabled = 1; + } else { + pcie_capability_clear_word(bridge, PCI_EXP_DEVCTL2, + PCI_EXP_DEVCTL2_ARI); + bridge->ari_enabled = 0; + } } /**