linux-kernel - Re: 答复: [PATCH] PCI: of: Warn if bridge base/limit region overlaps with system ram region

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230216233546.GA3354802@bhelgaas>
Date:   Thu, 16 Feb 2023 17:35:46 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     guo.ziliang@....com.cn
Cc:     bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, chen.lin5@....com.cn,
        Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>
Subject: Re: 答复: [PATCH] PCI: of: Warn if bridge base/limit region overlaps with system ram region

[+cc Joerg, Will, Robin]

On Mon, Jan 09, 2023 at 04:35:25PM +0800, guo.ziliang@....com.cn wrote:
> 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 bridge pci address setting phase.
> 
> Signed-off-by: Chen Lin <chen.lin5@....com.cn>

This would need the 0-day warnings cleaned up, of course.

> ---
>  drivers/pci/setup-bus.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index b4096598dbcb..1a9f527d2317 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -608,6 +608,24 @@ static void pci_setup_bridge_io(struct pci_dev *bridge)
>  	pci_write_config_dword(bridge, PCI_IO_BASE_UPPER16, io_upper16);
>  }
> 
> +static void check_bridge_region_overlaps_systemram(struct pci_dev *bridge,
> +							struct pci_bus_region *region)
> +{
> +	int is_ram;
> +
> +	/*
> +	 * 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(region->start, region->end - region->start + 1,
> +				IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE);
> +	if (is_ram == REGION_INTERSECTS) {
> +		pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
> +			region->start, region->end);

This compares PCI bus addresses (from struct pci_bus_region) with CPU
physical addresses (the struct resources used by region_intersects()).

But I don't think you can do that directly because an IOMMU might map
those PCI bus addresses to something different before a DMA gets to
system RAM.

I see that you say "The inbound mapping is usually 1:1 equal mapping"
above, so maybe I'm missing something.  Maybe the IOMMU folks will
clue me in.

> +	}
> +}
> +
>  static void pci_setup_bridge_mmio(struct pci_dev *bridge)
>  {
>  	struct resource *res;
> @@ -621,6 +639,7 @@ static void pci_setup_bridge_mmio(struct pci_dev *bridge)
>  		l = (region.start >> 16) & 0xfff0;
>  		l |= region.end & 0xfff00000;
>  		pci_info(bridge, "  bridge window %pR\n", res);
> +		check_bridge_region_overlaps_systemram(bridge, &region);
>  	} else {
>  		l = 0x0000fff0;
>  	}
> @@ -652,6 +671,7 @@ static void pci_setup_bridge_mmio_pref(struct pci_dev *bridge)
>  			lu = upper_32_bits(region.end);
>  		}
>  		pci_info(bridge, "  bridge window %pR\n", res);
> +		check_bridge_region_overlaps_systemram(bridge, &region);
>  	} else {
>  		l = 0x0000fff0;
>  	}
> -- 
> 2.15.2