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 EA166C433EF for ; Fri, 7 Jan 2022 23:17:37 +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=VPtbyTgJWqhOddLuNuy1Z2r37Ab8vGwPsgDAuzkCkw4=; b=bbmeqkESePAKHG MhBBEgOZVflb2ecY7KMzSX4iexryVKrUhkziIk4C66iJMC045KkQCxQvKxqY49KPSSX3XtauFqQ+A lnk3LsEMKZnkpAohUbH4iw8GshZbz2h4dUZAw9k74c10QZn3TaqvMDckyuAp1IJRE9eJFURcTQ4U0 biPGJT5tBdEbdrZFn93d0RfuP6jRQqshurVNdrigm/qs/8N24Y4ZiJ2cpt4gGEywZhYOrVCCtoMUO ofU0EtwccZpEdrh17C9xdIL6WrQ9UY6Af97tfh4UQqIrURidmvmN+wxxtGJlnjSxGuZDkcSP6OLjp DRUjf/ffkw/s0IDBKRlg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n5yTC-005TFm-Lq; Fri, 07 Jan 2022 23:16:26 +0000 Received: from ams.source.kernel.org ([2604:1380:4601:e00::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n5yT7-005TEV-Ov for linux-arm-kernel@lists.infradead.org; Fri, 07 Jan 2022 23:16:23 +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 52802B8278A; Fri, 7 Jan 2022 23:16:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D6282C36AE9; Fri, 7 Jan 2022 23:16:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1641597379; bh=SNR9q17r/H0e04aw9e89ZvRUdghwriL3CG6dv9CdeqE=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=mVBC6/mn4oeY/RScJWRT6MxWvI8neNupySllFKNZo7EJg5BHW41rLR9XhCNNMZf0V 8NvYmqzFh46mFXreRWKpe+eFuI9EF8nOTppsAkdWWe9Kxqju1o90UwNxJxtPk9Os8/ 6nR04R544JGsEYLYQJxWEySS6C3B2wb1c6iALYuyIeKrzNWVM0qrs6wnlbUuVBPESC 9ocEMUlyVWuqw8rjbB0+HpEtcJROngQoxVvwmPK2+lmlP8Bd8I0WWdDUhdTzbNAUB5 B6j1e1Nr17QlE7elC3+wIK/ppUyCNVLdGRU1i0JCZsl9fP6fkNAnof6K79cqjLQ/EI KbFDR+pla30TQ== Date: Fri, 7 Jan 2022 17:16:17 -0600 From: Bjorn Helgaas To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: Thomas Petazzoni , Lorenzo Pieralisi , Rob Herring , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Bjorn Helgaas , Marek =?iso-8859-1?Q?Beh=FAn?= , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 08/15] PCI: mvebu: Propagate errors when updating PCI_IO_BASE and PCI_MEM_BASE registers Message-ID: <20220107231617.GA425878@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220107222826.cv2bzywwayjwzy3c@pali> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220107_151622_115992_6937246B X-CRM114-Status: GOOD ( 34.51 ) 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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Jan 07, 2022 at 11:28:26PM +0100, Pali Roh=E1r wrote: > On Friday 07 January 2022 15:55:04 Bjorn Helgaas wrote: > > On Thu, Nov 25, 2021 at 01:45:58PM +0100, Pali Roh=E1r wrote: > > > Properly propagate failure from mvebu_pcie_add_windows() function bac= k to > > > the caller mvebu_pci_bridge_emul_base_conf_write() and correctly upda= tes > > > PCI_IO_BASE, PCI_MEM_BASE and PCI_IO_BASE_UPPER16 registers on error. > > > On error set base value higher than limit value which indicates that > > > address range is disabled. = > > = > > Does the spec say that if software programs something invalid, > > hardware should proactively set the base and limit registers to > > disable the window? > = > No. But this patch address something totally different. Software can do > fully valid operation, e.g. try to set forwarding memory window as large > as possible. But because this driver "emulates" pci bridge by calling > software/kernel function (mvebu_pcie_add_windows), some operations which > in real HW cannot happen, are possible in software. > = > For example there are limitations in sizes of forwarding memory windows, > because it is done by mvebu-mbus driver, which is responsible for > configuring mapping and forwarding of PCIe I/O and MEM windows. And due > to Marvell HW, there are restrictions which are not in PCIe HW. > = > Currently if such error happens, obviously kernel is not able to set > PCIe windows and it just print warnings to dmesg. Trying to access these > windows would result in the worst case in crashes. > = > With this change when mvebu_pcie_add_windows() function fails then into > emulated config space is put information that particular forwarding > window is disabled. I think that it is better to indicate it in config > space what is the current "reality" of hardware configuration. If window > is disabled in real-HW (meaning in mvebu-mbus driver) then show it also > in emulated config space of pci bridge. > = > Do you have better idea what should emulated pci bridge do, if software > try to set fully valid configuration of forwarding window, but it is not > possible to achieve it (even compliant PCI bridge must be able to do > it)? On an ACPI system, the host bridge window sizes are constrained by the host bridge _CRS method. I assume there's a similar constraint in DT. Is the fact that mvebu_pcie_add_windows() can fail a symptom of a DT that describes more available space than mvebu-bus can map? > > I'm not sure I've seen hardware that does this, and it seems ... maybe > > a little aggressive. > > = > > What happens if software writes the base and limit in the wrong order, > > so the window is invalid after the first write but valid after the > > second? That actually sounds like it could be a sensible strategy to > > prevent a partially-configured window from being active. > = > Invalid window (limit < base) means that window is disabled. And > pci-mvebu.c in its callbacks from pci-bridge-emul.c should correctly > handle it and propagates information about disablement to mvebu-mbus > driver. > = > After window is valid again (limit > base) then pci-mvebu.c call > mvebu-mbus to setup new mapping. Not sure I'm understanding the code correctly. Here's the sort of thing I'm worried about, but maybe this is actually impossible: Let's say software writes (0x00, 0xff) to the I/O (base, limit), which describes the [io 0x0000-0xffff] window. If mvebu-mbus can't handle that, it looks like you set the (base, limit) to (0xf0, 0x0f), which would describe [io 0xf000-0x0fff], which is invalid. The software writes 0x40 to the limit, so now we have (0xf0, 0x40), or [io 0xf000-0x40ff]. That's still invalid, but software thinks the 0x00 it wrote to the base is still there. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel