From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0593FC43334 for ; Wed, 20 Jul 2022 22:07:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=yVBPBDRl9QkFVk8iLnRWdzWZNNPYGZoULEdv+MaaNTw=; b=WeoOx8E+2N2dHx O8qcaQPuHpG+HpoN6ma4LIGbdWu+C0rve16gtS1XntzynD+qqAflDh5epacnVAhqJpFcCKEo6DLq5 k7E9d8bNJZacMe1dZUKatLK02Fh3fVgjDtw/IbPrRlMYI+AhLRHChAwc8wciRvSdTsbaI65eWcQOo xmsCDru3R07RO8By0Vzl0ejwIrQSZACxJNefDoipYQJtVtMVaWoSl+0gqEpsBdRACeNBqp7yjr3DF aDcYRKpZPUSEjG3hhx8stIcFqtqBlYO+mqy/Ecn051Yf0Mj7wHYl08T1cGHjX0BysiHbt7MeKNZhC GeWjnJmArBwheVKc6okw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oEHpa-00C1d0-TA; Wed, 20 Jul 2022 22:06:11 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oEHpX-00C1TE-7i; Wed, 20 Jul 2022 22:06:09 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id A6012B821B5; Wed, 20 Jul 2022 22:06:01 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F105BC341C7; Wed, 20 Jul 2022 22:05:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1658354760; bh=qYViTxSdzdo85TRjxcG0IeVUNVFx1NYEZydO+uq4vs0=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=rH/cEngXsM5vG3FeARaAn+VpgiODrGvNEsYBjPBEEZsWiDRBuoUFwIbYVS2BWk41q p7V5o0O/kvHoiPhiCTDabYy7EJhDCWaRgmnfM9pI0jdsTV9kGU786tpttATFol5Oa2 PqoPA44LabkGXX4G0PMMkHFJh8RFiWIijX+ywehOBSy7Khs1oFmDJoLzk+n6ffneEg KDEg1zCVgJjqY461V6vjaeA0ydqk89Vgf3x/2F0KnHm4G5C6AyFws8xor1z74o29KJ DhY1nxzPp2j4bBXjEnWNJWfkGi5kJ2pBu9f+JJqQUwtw+OEADs96/rZTi1TKbpfDoQ D4LQibpCeRBBQ== Date: Wed, 20 Jul 2022 17:05:58 -0500 From: Bjorn Helgaas To: Jim Quinlan Cc: linux-pci@vger.kernel.org, Nicolas Saenz Julienne , Bjorn Helgaas , Lorenzo Pieralisi , Cyril Brulebois , bcm-kernel-feedback-list@broadcom.com, james.quinlan@broadcom.com, Florian Fainelli , Lorenzo Pieralisi , Rob Herring , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , open list Subject: Re: [PATCH v2 3/6] PCI: brcmstb: Add "refusal mode" to preclude PCIe-induced CPU aborts Message-ID: <20220720220558.GA1661469@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220716222454.29914-4-jim2101024@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220720_150607_603337_C2951071 X-CRM114-Status: GOOD ( 30.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, Jul 16, 2022 at 06:24:50PM -0400, Jim Quinlan wrote: > Our PCIe RC HW has an atypical behavior: if it does not have PCIe link > established between itself and downstream, any subsequent config space > access causes a CPU abort. This commit sets a "refusal mode" if the PCIe > link-up fails, and this has our pci_ops map_bus function returning a NULL > address, which in turn precludes the access from happening. > > Right now, "refusal mode" is window dressing. It will become relevant > in a future commit when brcm_pcie_start_link() is invoked during > enumeration instead of before it. > > Signed-off-by: Jim Quinlan > --- > drivers/pci/controller/pcie-brcmstb.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index c026446d5830..72219a4f3964 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -255,6 +255,7 @@ struct brcm_pcie { > u32 hw_rev; > void (*perst_set)(struct brcm_pcie *pcie, u32 val); > void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val); > + bool refusal_mode; > }; > > static inline bool is_bmips(const struct brcm_pcie *pcie) > @@ -687,6 +688,19 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn, > if (pci_is_root_bus(bus)) > return PCI_SLOT(devfn) ? NULL : base + where; > > + if (pcie->refusal_mode) { > + /* > + * At this point we do not have PCIe link-up. If there is > + * a config read or write access besides those targeting > + * the host bridge, our PCIe HW throws a CPU abort. To > + * prevent this we return the NULL address. The calling > + * functions -- pci_generic_config_*() -- will notice this > + * and not perform the access, and if it is a read access, > + * 0xffffffff is returned. > + */ > + return NULL; > + } Is this any different from all the other .map_bus() implementations that return NULL when the link is down? cdns_pci_map_bus() dw_pcie_other_conf_map_bus() nwl_pcie_map_bus() (see nwl_pcie_valid_device()) xilinx_pcie_map_bus() (see xilinx_pcie_valid_device()) If you can implement this the same way, i.e., using brcm_pcie_link_up(), it would be nice. > /* For devices, write to the config space index register */ > idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0); > writel(idx, pcie->base + PCIE_EXT_CFG_INDEX); > @@ -704,6 +718,11 @@ static void __iomem *brcm_pcie_map_conf32(struct pci_bus *bus, unsigned int devf > if (pci_is_root_bus(bus)) > return PCI_SLOT(devfn) ? NULL : base + (where & ~0x3); > > + if (pcie->refusal_mode) { > + /* See note above in brcm_pcie_map_conf() */ > + return NULL; > + } > + > /* For devices, write to the config space index register */ > idx = PCIE_ECAM_OFFSET(bus->number, devfn, (where & ~3)); > writel(idx, base + IDX_ADDR(pcie)); > @@ -989,6 +1008,7 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie) > dev_err(dev, "link down\n"); > return -ENODEV; > } > + pcie->refusal_mode = false; > > if (!brcm_pcie_rc_mode(pcie)) { > dev_err(dev, "PCIe misconfigured; is in EP mode\n"); > @@ -1134,6 +1154,8 @@ static void brcm_pcie_turn_off(struct brcm_pcie *pcie) > void __iomem *base = pcie->base; > int tmp; > > + pcie->refusal_mode = true; > + > if (brcm_pcie_link_up(pcie)) > brcm_pcie_enter_l23(pcie); > /* Assert fundamental reset */ > @@ -1185,6 +1207,7 @@ static int brcm_pcie_resume(struct device *dev) > u32 tmp; > int ret; > > + pcie->refusal_mode = true; > base = pcie->base; > ret = clk_prepare_enable(pcie->clk); > if (ret) > @@ -1361,6 +1384,7 @@ static int brcm_pcie_probe(struct platform_device *pdev) > pcie->type = data->type; > pcie->perst_set = data->perst_set; > pcie->bridge_sw_init_set = data->bridge_sw_init_set; > + pcie->refusal_mode = true; > > pcie->base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(pcie->base)) > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel