Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 6 Jan 2023 06:18:45 -0600
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     guo.ziliang@zte.com.cn
Cc:     bhelgaas@google.com, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, chen.lin5@zte.com.cn
Subject: Re: [PATCH] PCI: of: Warn if bridge base/limit region overlaps with
 system ram region
Message-ID: <20230106121845.GA1216249@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202301061647338189934@zte.com.cn>
Precedence: bulk

On Fri, Jan 06, 2023 at 04:47:33PM +0800, guo.ziliang@zte.com.cn wrote:
> From: Chen Lin <chen.lin5@zte.com.cn>
> bridge base/limit(memory behind in lspci info, outbound pcie address/size)
> region is used to route outbound mem read/write transaction to ep. This
> base/limit region also may filter out inbound transactions which will
> result in inbound(eg: dma) transaction fail.
> 
> For example, if bridge base/limit is [0x20000000, 0x203fffff], system ram
> is [0x20000000, 0x27ffffff]. The inbound mapping is usually 1:1 equal
> mapping. When allocated system ram for inbound tansaction is 0x20004000
> (any in bridge base/limit), this inbound transactions will be filter out.
> 
> AER may report 'UnsupReq' on inbound mem read/write transactions if address
> is in this base/limit region, but not all pcie AER enabled or work well. We
> warn it also in host bridge pci address detection phase.

Is this a DT-specific thing?  It sounds like it should apply to PCI
bridges in general.

> Signed-off-by: Chen Lin <chen.lin5@zte.com.cn>
> ---
>  drivers/pci/of.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 196834ed44fe..82e09af6c638 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -314,6 +314,8 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev,
> 
>  	dev_dbg(dev, "Parsing ranges property...\n");
>  	for_each_of_pci_range(&parser, &range) {
> +		int is_ram;
> +
>  		/* Read next ranges element */
>  		if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
>  			range_type = "IO";
> @@ -332,6 +334,18 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev,
>  		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>  			continue;
> 
> +		/*
> +		 * bridge base/limit(memory behind) region may filter out inbound
> +		 * transactions which will result in inbound(eg:dma) fail of ep.
> +		 * AER may report it if enabled, we warn it also.
> +		 */
> +		is_ram = region_intersects(range.pci_addr, range.size,
> +					IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE);
> +		if (is_ram == REGION_INTERSECTS) {
> +			dev_warn(dev, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
> +				 range.pci_addr, range.pci_addr + range.size - 1);
> +		}
> +
>  		err = of_pci_range_to_resource(&range, dev_node, &tmp_res);
>  		if (err)
>  			continue;
> -- 
> 2.15.2