From: Chen Lin <[email protected]>
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.
Signed-off-by: Chen Lin <[email protected]>
---
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
On Fri, Jan 06, 2023 at 04:47:33PM +0800, [email protected] wrote:
> From: Chen Lin <[email protected]>
> 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 <[email protected]>
> ---
> 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
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 <[email protected]>
---
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);
+ }
+}
+
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, ®ion);
} 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, ®ion);
} else {
l = 0x0000fff0;
}
--
2.15.2
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on helgaas-pci/for-linus linus/master v6.2-rc3 next-20230109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/guo-ziliang-zte-com-cn/PATCH-PCI-of-Warn-if-bridge-base-limit-region-overlaps-with-system-ram-region/20230109-163746
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link: https://lore.kernel.org/r/202301091635256312056%40zte.com.cn
patch subject: 答复: [PATCH] PCI: of: Warn if bridge base/limit region overlaps with system ram region
config: i386-randconfig-a001-20230109
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/38ad87b57c5f9bea2f43674f02787ea51b4bbb20
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review guo-ziliang-zte-com-cn/PATCH-PCI-of-Warn-if-bridge-base-limit-region-overlaps-with-system-ram-region/20230109-163746
git checkout 38ad87b57c5f9bea2f43674f02787ea51b4bbb20
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/pci/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/pci/setup-bus.c:625:4: warning: format specifies type 'unsigned long long' but the argument has type 'pci_bus_addr_t' (aka 'unsigned int') [-Wformat]
region->start, region->end);
^~~~~~~~~~~~~
include/linux/pci.h:2527:67: note: expanded from macro 'pci_warn'
#define pci_warn(pdev, fmt, arg...) dev_warn(&(pdev)->dev, fmt, ##arg)
~~~ ^~~
include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
drivers/pci/setup-bus.c:625:19: warning: format specifies type 'unsigned long long' but the argument has type 'pci_bus_addr_t' (aka 'unsigned int') [-Wformat]
region->start, region->end);
^~~~~~~~~~~
include/linux/pci.h:2527:67: note: expanded from macro 'pci_warn'
#define pci_warn(pdev, fmt, arg...) dev_warn(&(pdev)->dev, fmt, ##arg)
~~~ ^~~
include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
2 warnings generated.
vim +625 drivers/pci/setup-bus.c
610
611 static void check_bridge_region_overlaps_systemram(struct pci_dev *bridge,
612 struct pci_bus_region *region)
613 {
614 int is_ram;
615
616 /*
617 * bridge base/limit(memory behind) region may filter out inbound
618 * transactions which will result in inbound(eg: dma) fail of ep.
619 * AER may report it if enabled, we warn it also.
620 */
621 is_ram = region_intersects(region->start, region->end - region->start + 1,
622 IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE);
623 if (is_ram == REGION_INTERSECTS) {
624 pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
> 625 region->start, region->end);
626 }
627 }
628
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on helgaas-pci/for-linus linus/master v6.2-rc3 next-20230109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/guo-ziliang-zte-com-cn/PATCH-PCI-of-Warn-if-bridge-base-limit-region-overlaps-with-system-ram-region/20230109-163746
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link: https://lore.kernel.org/r/202301091635256312056%40zte.com.cn
patch subject: 答复: [PATCH] PCI: of: Warn if bridge base/limit region overlaps with system ram region
config: mips-allmodconfig
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/38ad87b57c5f9bea2f43674f02787ea51b4bbb20
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review guo-ziliang-zte-com-cn/PATCH-PCI-of-Warn-if-bridge-base-limit-region-overlaps-with-system-ram-region/20230109-163746
git checkout 38ad87b57c5f9bea2f43674f02787ea51b4bbb20
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/pci.h:37,
from drivers/pci/setup-bus.c:20:
drivers/pci/setup-bus.c: In function 'check_bridge_region_overlaps_systemram':
>> drivers/pci/setup-bus.c:624:34: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 3 has type 'pci_bus_addr_t' {aka 'unsigned int'} [-Wformat=]
624 | pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
include/linux/pci.h:2527:41: note: in expansion of macro 'dev_warn'
2527 | #define pci_warn(pdev, fmt, arg...) dev_warn(&(pdev)->dev, fmt, ##arg)
| ^~~~~~~~
drivers/pci/setup-bus.c:624:17: note: in expansion of macro 'pci_warn'
624 | pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
| ^~~~~~~~
drivers/pci/setup-bus.c:624:42: note: format string is defined here
624 | pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
| ~~~~~~~^
| |
| long long unsigned int
| %#012x
drivers/pci/setup-bus.c:624:34: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'pci_bus_addr_t' {aka 'unsigned int'} [-Wformat=]
624 | pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
include/linux/pci.h:2527:41: note: in expansion of macro 'dev_warn'
2527 | #define pci_warn(pdev, fmt, arg...) dev_warn(&(pdev)->dev, fmt, ##arg)
| ^~~~~~~~
drivers/pci/setup-bus.c:624:17: note: in expansion of macro 'pci_warn'
624 | pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
| ^~~~~~~~
drivers/pci/setup-bus.c:624:52: note: format string is defined here
624 | pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
| ~~~~~~~^
| |
| long long unsigned int
| %#012x
vim +624 drivers/pci/setup-bus.c
610
611 static void check_bridge_region_overlaps_systemram(struct pci_dev *bridge,
612 struct pci_bus_region *region)
613 {
614 int is_ram;
615
616 /*
617 * bridge base/limit(memory behind) region may filter out inbound
618 * transactions which will result in inbound(eg: dma) fail of ep.
619 * AER may report it if enabled, we warn it also.
620 */
621 is_ram = region_intersects(region->start, region->end - region->start + 1,
622 IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE);
623 if (is_ram == REGION_INTERSECTS) {
> 624 pci_warn(bridge, "%#012llx..%#012llx bridge base/limit region overlaps with system ram, may result in inbound fail\n",
625 region->start, region->end);
626 }
627 }
628
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
[+cc Joerg, Will, Robin]
On Mon, Jan 09, 2023 at 04:35:25PM +0800, [email protected] 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 <[email protected]>
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, ®ion);
> } 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, ®ion);
> } else {
> l = 0x0000fff0;
> }
> --
> 2.15.2
On 2023-02-16 23:35, Bjorn Helgaas wrote:
> [+cc Joerg, Will, Robin]
>
> On Mon, Jan 09, 2023 at 04:35:25PM +0800, [email protected] 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 <[email protected]>
>
> 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.
IOMMUs typically wouldn't be reflected here - in fact they would
typically hide this issue anyway, since inbound DMA would then be to
virtual addresses anywhere in PCI Mem space (and we try our best to
carve out the regions used for outbound resources). However there
certainly exist systems where the PCI host bridge itself makes a static
non-identity translation between PCI Mem space and system PA space, with
potentially different mappings for inbound vs. outbound as well. So
indeed, this code looks wrong - at the very least any consideration of
region->offset is missing (assuming that's initialised correctly in this
context), but that still won't account for inbound translation.
In fact this is really the same thing as in the recent discussions of
the MSI thing in the DWC driver - it's all a matter of whether a bus
address may overlap a valid DMA address or not. A mechanism for making
that check properly would go hand-in-hand with the mechanism we need for
allocating such bus addresses for inline MSI widgets.
Thanks,
Robin.
>> + }
>> +}
>> +
>> 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, ®ion);
>> } 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, ®ion);
>> } else {
>> l = 0x0000fff0;
>> }
>> --
>> 2.15.2