[U-Boot] inappropriate PCI configuration on arm64 qemu?
When I tried to add a SD card to qemu's virt machine (2.10.0) as, ------ -device sdhci-pci \ -device sd-card,drive=my_sd \ -drive if=none,id=my_sd,format=raw,file=/path/my/sd.img ------ u-boot doesn't configure a SDHCI controller properly and an attached device is never detected.
Digging into the code, I found * reading BAR5 in dm_pciauto_setup_device() shows BAR5 is a 32-bit address, * pciauto_region_allocate() allocates a 64-bit address (0x80.ABCD.0000) to BAR5 as res->bus_lower is 0x80.0000.0000 * Upper 32-bit value is not written back to BAR5 because of !found_mem64 (BAR5 is the last one and no succeeding BAR anyway.)
On the other hand, * Qemu defines two PCI memory regions for MMIO: (from qemu's hw/arm/virt.c) ------ [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, [VIRT_MEM] = { 0x40000000, RAMLIMIT_BYTES }, /* Second PCIe window, 512GB wide at the 512GB boundary */ [VIRT_PCIE_MMIO_HIGH] = { 0x8000000000ULL, 0x8000000000ULL }, ------ * A PCI card is configured in decode_regions() so that 'hose' has only one entry per each type of memory regions. This behavior was introduced by Simon's patch: ------ commit 9526d83ac5a Author: Simon Glass sjg@chromium.org Date: Thu Nov 19 20:26:58 2015 -0700
dm: pci: Support decoding ranges with duplicate entries ------ * As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
I believe that this behavior is the root cause of my issue, and by reverting the patch mentioned above, everything works fine.
While I understand a concern mentioned in the commit message, there should be a better way to manage the case.
Any ideas?
Thanks, -Takahiro AKASHI
+Tuomas
Hi Akashi,
On 28 May 2018 at 01:59, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
When I tried to add a SD card to qemu's virt machine (2.10.0) as,
-device sdhci-pci \ -device sd-card,drive=my_sd \ -drive if=none,id=my_sd,format=raw,file=/path/my/sd.img ------u-boot doesn't configure a SDHCI controller properly and an attached device is never detected.
Digging into the code, I found
- reading BAR5 in dm_pciauto_setup_device() shows BAR5 is a 32-bit address,
- pciauto_region_allocate() allocates a 64-bit address (0x80.ABCD.0000) to BAR5 as res->bus_lower is 0x80.0000.0000
- Upper 32-bit value is not written back to BAR5 because of !found_mem64 (BAR5 is the last one and no succeeding BAR anyway.)
On the other hand,
Qemu defines two PCI memory regions for MMIO: (from qemu's hw/arm/virt.c) ------ [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, [VIRT_MEM] = { 0x40000000, RAMLIMIT_BYTES }, /* Second PCIe window, 512GB wide at the 512GB boundary */ [VIRT_PCIE_MMIO_HIGH] = { 0x8000000000ULL, 0x8000000000ULL }, ------
A PCI card is configured in decode_regions() so that 'hose' has only one entry per each type of memory regions. This behavior was introduced by Simon's patch: ------ commit 9526d83ac5a Author: Simon Glass sjg@chromium.org Date: Thu Nov 19 20:26:58 2015 -0700
dm: pci: Support decoding ranges with duplicate entries ------As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
I believe that this behavior is the root cause of my issue, and by reverting the patch mentioned above, everything works fine.
While I understand a concern mentioned in the commit message, there should be a better way to manage the case.
There was a series that changed things in this area. Can you take a look?
PCI: dm: Ignore 64-bit memory regions if CONFIG_SYS_PCI_64BIT not set
Regards, Simon
Simon,
On Wed, May 30, 2018 at 01:18:30PM -0600, Simon Glass wrote:
+Tuomas
Hi Akashi,
On 28 May 2018 at 01:59, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
When I tried to add a SD card to qemu's virt machine (2.10.0) as,
-device sdhci-pci \ -device sd-card,drive=my_sd \ -drive if=none,id=my_sd,format=raw,file=/path/my/sd.img ------u-boot doesn't configure a SDHCI controller properly and an attached device is never detected.
Digging into the code, I found
- reading BAR5 in dm_pciauto_setup_device() shows BAR5 is a 32-bit address,
- pciauto_region_allocate() allocates a 64-bit address (0x80.ABCD.0000) to BAR5 as res->bus_lower is 0x80.0000.0000
- Upper 32-bit value is not written back to BAR5 because of !found_mem64 (BAR5 is the last one and no succeeding BAR anyway.)
On the other hand,
Qemu defines two PCI memory regions for MMIO: (from qemu's hw/arm/virt.c) ------ [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, [VIRT_MEM] = { 0x40000000, RAMLIMIT_BYTES }, /* Second PCIe window, 512GB wide at the 512GB boundary */ [VIRT_PCIE_MMIO_HIGH] = { 0x8000000000ULL, 0x8000000000ULL }, ------
A PCI card is configured in decode_regions() so that 'hose' has only one entry per each type of memory regions. This behavior was introduced by Simon's patch: ------ commit 9526d83ac5a Author: Simon Glass sjg@chromium.org Date: Thu Nov 19 20:26:58 2015 -0700
dm: pci: Support decoding ranges with duplicate entries ------As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
I believe that this behavior is the root cause of my issue, and by reverting the patch mentioned above, everything works fine.
While I understand a concern mentioned in the commit message, there should be a better way to manage the case.
There was a series that changed things in this area. Can you take a look?
PCI: dm: Ignore 64-bit memory regions if CONFIG_SYS_PCI_64BIT not set
Ah, I didn't know that, but it seems to me that it is still insufficient. This hack won't work on 32-bit PCI card. I found another patch from Tuomas: --- commit d71975ae6e0 Author: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Date: Mon May 14 19:38:13 2018 +0300
PCI: autoconfig: Don't allocate 64-bit addresses to 32-bit only resources ---
This approach looks too conservative if 32-bit window is also available, in addition to 64-bit space, as in the case of qemu-arm.
I'd like to propose supporting at least two type of PCI memory regions, low mem (normal case) and high mem. Attached is my experimental implementation for this although I might have made any mistake as I'm not very much familiar with PCI specification.
(When you try to apply this, please revert the commit above.)
Thanks, -Takahiro AKASHI
Regards, Simon
---8<--- diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 1cd1e409e3e..40282c616a4 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -867,13 +867,7 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, continue; }
- pos = -1; - for (i = 0; i < hose->region_count; i++) { - if (hose->regions[i].flags == type) - pos = i; - } - if (pos == -1) - pos = hose->region_count++; + pos = hose->region_count++; debug(" - type=%d, pos=%d\n", type, pos); pci_set_region(hose->regions + pos, pci_addr, addr, size, type); } diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index d1feb503a0a..01423879552 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -19,6 +19,7 @@
void dm_pciauto_setup_device(struct udevice *dev, int bars_num, struct pci_region *mem, + struct pci_region *himem, struct pci_region *prefetch, struct pci_region *io, bool enum_only) { @@ -29,7 +30,7 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num, u8 header_type; int rom_addr; pci_addr_t bar_value; - struct pci_region *bar_res = NULL; + struct pci_region *bar_res = NULL, *bar_res32 = NULL; int found_mem64 = 0; u16 class;
@@ -88,7 +89,12 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num, PCI_BASE_ADDRESS_MEM_PREFETCH)) { bar_res = prefetch; } else { - bar_res = mem; + if (found_mem64) { + bar_res = himem; + bar_res32 = mem; + } else { + bar_res = mem; + } } }
@@ -97,8 +103,14 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num, (unsigned long long)bar_size); }
- if (!enum_only && pciauto_region_allocate(bar_res, bar_size, - &bar_value) == 0) { + if (!enum_only) { + if (pciauto_region_allocate(bar_res, bar_size, + &bar_value) && + bar_res32 && (bar_size < (0x1ULL << 32)) && + pciauto_region_allocate(bar_res32, bar_size, + &bar_value)) + goto skip_write; + /* Write it out and update our limit */ dm_pci_write_config32(dev, bar, (u32)bar_value);
@@ -116,6 +128,8 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num, dm_pci_write_config32(dev, bar, 0x00000000); #endif } +skip_write: + ; }
cmdstat |= (bar_response & PCI_BASE_ADDRESS_SPACE) ? @@ -305,6 +319,7 @@ void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus) int dm_pciauto_config_device(struct udevice *dev) { struct pci_region *pci_mem; + struct pci_region *pci_himem; struct pci_region *pci_prefetch; struct pci_region *pci_io; unsigned int sub_bus = PCI_BUS(dm_pci_get_bdf(dev)); @@ -319,6 +334,7 @@ int dm_pciauto_config_device(struct udevice *dev) #endif
pci_mem = ctlr_hose->pci_mem; + pci_himem = ctlr_hose->pci_himem; pci_prefetch = ctlr_hose->pci_prefetch; pci_io = ctlr_hose->pci_io;
@@ -329,8 +345,8 @@ int dm_pciauto_config_device(struct udevice *dev) debug("PCI Autoconfig: Found P2P bridge, device %d\n", PCI_DEV(dm_pci_get_bdf(dev)));
- dm_pciauto_setup_device(dev, 2, pci_mem, pci_prefetch, pci_io, - enum_only); + dm_pciauto_setup_device(dev, 2, pci_mem, pci_himem, + pci_prefetch, pci_io, enum_only);
n = dm_pci_hose_probe_bus(dev); if (n < 0) @@ -343,8 +359,8 @@ int dm_pciauto_config_device(struct udevice *dev) * just do a minimal setup of the bridge, * let the OS take care of the rest */ - dm_pciauto_setup_device(dev, 0, pci_mem, pci_prefetch, pci_io, - enum_only); + dm_pciauto_setup_device(dev, 0, pci_mem, pci_himem, + pci_prefetch, pci_io, enum_only);
debug("PCI Autoconfig: Found P2CardBus bridge, device %d\n", PCI_DEV(dm_pci_get_bdf(dev))); @@ -366,7 +382,7 @@ int dm_pciauto_config_device(struct udevice *dev) * the PIMMR window to be allocated (BAR0 - 1MB size) */ debug("PCI Autoconfig: Broken bridge found, only minimal config\n"); - dm_pciauto_setup_device(dev, 0, hose->pci_mem, + dm_pciauto_setup_device(dev, 0, hose->pci_mem, hose->pci_himem, hose->pci_prefetch, hose->pci_io, enum_only); break; @@ -377,8 +393,8 @@ int dm_pciauto_config_device(struct udevice *dev) /* fall through */
default: - dm_pciauto_setup_device(dev, 6, pci_mem, pci_prefetch, pci_io, - enum_only); + dm_pciauto_setup_device(dev, 6, pci_mem, pci_himem, + pci_prefetch, pci_io, enum_only); break; }
diff --git a/drivers/pci/pci_auto_common.c b/drivers/pci/pci_auto_common.c index d90dbcf91a2..e96b1cb4d12 100644 --- a/drivers/pci/pci_auto_common.c +++ b/drivers/pci/pci_auto_common.c @@ -78,6 +78,7 @@ void pciauto_config_init(struct pci_controller *hose)
hose->pci_io = NULL; hose->pci_mem = NULL; + hose->pci_himem = NULL; hose->pci_prefetch = NULL;
for (i = 0; i < hose->region_count; i++) { @@ -88,6 +89,13 @@ void pciauto_config_init(struct pci_controller *hose) hose->pci_io = hose->regions + i; break; case PCI_REGION_MEM: + if (hose->regions[i].bus_start > ((0x1ULL << 32) - 1)) { + if (!hose->pci_himem || + hose->pci_himem->size + < hose->regions[i].size) + hose->pci_himem = hose->regions + i; + break; + } if (!hose->pci_mem || hose->pci_mem->size < hose->regions[i].size) hose->pci_mem = hose->regions + i; @@ -103,6 +111,8 @@ void pciauto_config_init(struct pci_controller *hose)
if (hose->pci_mem) pciauto_show_region("Memory", hose->pci_mem); + if (hose->pci_himem) + pciauto_show_region("High Memory", hose->pci_himem); if (hose->pci_prefetch) pciauto_show_region("Prefetchable Mem", hose->pci_prefetch); if (hose->pci_io) diff --git a/include/pci.h b/include/pci.h index b9324405ee2..5fa7bad8798 100644 --- a/include/pci.h +++ b/include/pci.h @@ -582,7 +582,7 @@ struct pci_controller { #endif
/* Used by auto config */ - struct pci_region *pci_mem, *pci_io, *pci_prefetch; + struct pci_region *pci_mem, *pci_himem, *pci_io, *pci_prefetch;
#ifndef CONFIG_DM_PCI int current_busno;
Hi Akashi,
On 05/31/2018 08:05 AM, AKASHI Takahiro wrote:
Simon,
On Wed, May 30, 2018 at 01:18:30PM -0600, Simon Glass wrote:
+Tuomas
Hi Akashi,
On 28 May 2018 at 01:59, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
When I tried to add a SD card to qemu's virt machine (2.10.0) as,
-device sdhci-pci \ -device sd-card,drive=my_sd \ -drive if=none,id=my_sd,format=raw,file=/path/my/sd.img ------u-boot doesn't configure a SDHCI controller properly and an attached device is never detected.
Digging into the code, I found
- reading BAR5 in dm_pciauto_setup_device() shows BAR5 is a 32-bit address,
- pciauto_region_allocate() allocates a 64-bit address (0x80.ABCD.0000) to BAR5 as res->bus_lower is 0x80.0000.0000
- Upper 32-bit value is not written back to BAR5 because of !found_mem64 (BAR5 is the last one and no succeeding BAR anyway.)
On the other hand,
Qemu defines two PCI memory regions for MMIO: (from qemu's hw/arm/virt.c) ------ [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, [VIRT_MEM] = { 0x40000000, RAMLIMIT_BYTES }, /* Second PCIe window, 512GB wide at the 512GB boundary */ [VIRT_PCIE_MMIO_HIGH] = { 0x8000000000ULL, 0x8000000000ULL }, ------
A PCI card is configured in decode_regions() so that 'hose' has only one entry per each type of memory regions. This behavior was introduced by Simon's patch: ------ commit 9526d83ac5a Author: Simon Glass sjg@chromium.org Date: Thu Nov 19 20:26:58 2015 -0700
dm: pci: Support decoding ranges with duplicate entries ------As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
I believe that this behavior is the root cause of my issue, and by reverting the patch mentioned above, everything works fine.
While I understand a concern mentioned in the commit message, there should be a better way to manage the case.
There was a series that changed things in this area. Can you take a look?
PCI: dm: Ignore 64-bit memory regions if CONFIG_SYS_PCI_64BIT not setAh, I didn't know that, but it seems to me that it is still insufficient. This hack won't work on 32-bit PCI card. I found another patch from Tuomas:
Did you try it? As of today's master all of the patches are applied and at least the e1000 NIC and the Intel AHCI card that I tested works. The effect of the commit is to indeed avoid the problem you mentioned:
- As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
Note that even on aarch64, CONFIG_SYS_PCI_64BIT is *not* set by default. And on ARM we would need to skip that region in U-Boot anyway because we don't have the means to access physical addresses above the 4GB boundary with the CPU using U-Boot's identity-mapped page tables.
commit d71975ae6e0 Author: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> Date: Mon May 14 19:38:13 2018 +0300 PCI: autoconfig: Don't allocate 64-bit addresses to 32-bit only resources
This approach looks too conservative if 32-bit window is also available, in addition to 64-bit space, as in the case of qemu-arm.
Yes, the patch is very minimal - I just wanted to fix the silent truncation of 64-bit addresses to 32-bit addresses and complain instead, nothing more.
(As the default config of qemu_arm* doesn't have CONFIG_SYS_PCI_64BIT, the condition won't actually trigger in practice).
I'd like to propose supporting at least two type of PCI memory regions, low mem (normal case) and high mem. Attached is my experimental implementation for this although I might have made any mistake as I'm not very much familiar with PCI specification.
Yes, in theory it could be useful for some future hardware, but for QEMU point of view the current situation of totally ignoring the 64-bit mem region is probably good enough, given that most of the useful hardware that QEMU can enable is limited to 32-bit addressing anyway.
Tuomas,
On Thu, May 31, 2018 at 01:32:20PM +0300, Tuomas Tynkkynen wrote:
Hi Akashi,
On 05/31/2018 08:05 AM, AKASHI Takahiro wrote:
Simon,
On Wed, May 30, 2018 at 01:18:30PM -0600, Simon Glass wrote:
+Tuomas
Hi Akashi,
On 28 May 2018 at 01:59, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
When I tried to add a SD card to qemu's virt machine (2.10.0) as,
-device sdhci-pci \ -device sd-card,drive=my_sd \ -drive if=none,id=my_sd,format=raw,file=/path/my/sd.img ------u-boot doesn't configure a SDHCI controller properly and an attached device is never detected.
Digging into the code, I found
- reading BAR5 in dm_pciauto_setup_device() shows BAR5 is a 32-bit address,
- pciauto_region_allocate() allocates a 64-bit address (0x80.ABCD.0000) to BAR5 as res->bus_lower is 0x80.0000.0000
- Upper 32-bit value is not written back to BAR5 because of !found_mem64 (BAR5 is the last one and no succeeding BAR anyway.)
On the other hand,
Qemu defines two PCI memory regions for MMIO: (from qemu's hw/arm/virt.c) ------ [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, [VIRT_MEM] = { 0x40000000, RAMLIMIT_BYTES }, /* Second PCIe window, 512GB wide at the 512GB boundary */ [VIRT_PCIE_MMIO_HIGH] = { 0x8000000000ULL, 0x8000000000ULL }, ------
A PCI card is configured in decode_regions() so that 'hose' has only one entry per each type of memory regions. This behavior was introduced by Simon's patch: ------ commit 9526d83ac5a Author: Simon Glass sjg@chromium.org Date: Thu Nov 19 20:26:58 2015 -0700
dm: pci: Support decoding ranges with duplicate entries ------As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
I believe that this behavior is the root cause of my issue, and by reverting the patch mentioned above, everything works fine.
While I understand a concern mentioned in the commit message, there should be a better way to manage the case.
There was a series that changed things in this area. Can you take a look?
PCI: dm: Ignore 64-bit memory regions if CONFIG_SYS_PCI_64BIT not set
Ah, I didn't know that, but it seems to me that it is still insufficient. This hack won't work on 32-bit PCI card. I found another patch from Tuomas:
Did you try it? As of today's master all of the patches are applied and at least the e1000 NIC and the Intel AHCI card that I tested works. The effect of the commit is to indeed avoid the problem you mentioned:
Yes, I ran my patch but *with* CONFIG_SYS_PCI_64BIT.
- As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
Note that even on aarch64, CONFIG_SYS_PCI_64BIT is *not* set by default. And on ARM we would need to skip that region in U-Boot anyway because we don't have the means to access physical addresses above the 4GB boundary with the CPU using U-Boot's identity-mapped page tables.
Maybe you're right regarding aarch64, but the issue is not about arm/arm64 but PCI configuration. Some arch/machines, freescale mostly?, have already enabled CONFIG_SYS_PCI_64BIT. I'm afraid that there may be a possibility that your patch breaks them.
Thanks, -Takahiro AKASHI
commit d71975ae6e0 Author: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> Date: Mon May 14 19:38:13 2018 +0300 PCI: autoconfig: Don't allocate 64-bit addresses to 32-bit only resources
This approach looks too conservative if 32-bit window is also available, in addition to 64-bit space, as in the case of qemu-arm.
Yes, the patch is very minimal - I just wanted to fix the silent truncation of 64-bit addresses to 32-bit addresses and complain instead, nothing more.
(As the default config of qemu_arm* doesn't have CONFIG_SYS_PCI_64BIT, the condition won't actually trigger in practice).
I'd like to propose supporting at least two type of PCI memory regions, low mem (normal case) and high mem. Attached is my experimental implementation for this although I might have made any mistake as I'm not very much familiar with PCI specification.
Yes, in theory it could be useful for some future hardware, but for QEMU point of view the current situation of totally ignoring the 64-bit mem region is probably good enough, given that most of the useful hardware that QEMU can enable is limited to 32-bit addressing anyway.
On 06/01/2018 10:21 AM, AKASHI Takahiro wrote:
Tuomas,
On Thu, May 31, 2018 at 01:32:20PM +0300, Tuomas Tynkkynen wrote:
Hi Akashi,
On 05/31/2018 08:05 AM, AKASHI Takahiro wrote:
Simon,
On Wed, May 30, 2018 at 01:18:30PM -0600, Simon Glass wrote:
+Tuomas
Hi Akashi,
On 28 May 2018 at 01:59, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
When I tried to add a SD card to qemu's virt machine (2.10.0) as,
-device sdhci-pci \ -device sd-card,drive=my_sd \ -drive if=none,id=my_sd,format=raw,file=/path/my/sd.img ------u-boot doesn't configure a SDHCI controller properly and an attached device is never detected.
Digging into the code, I found
- reading BAR5 in dm_pciauto_setup_device() shows BAR5 is a 32-bit address,
- pciauto_region_allocate() allocates a 64-bit address (0x80.ABCD.0000) to BAR5 as res->bus_lower is 0x80.0000.0000
- Upper 32-bit value is not written back to BAR5 because of !found_mem64 (BAR5 is the last one and no succeeding BAR anyway.)
On the other hand,
Qemu defines two PCI memory regions for MMIO: (from qemu's hw/arm/virt.c) ------ [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, [VIRT_MEM] = { 0x40000000, RAMLIMIT_BYTES }, /* Second PCIe window, 512GB wide at the 512GB boundary */ [VIRT_PCIE_MMIO_HIGH] = { 0x8000000000ULL, 0x8000000000ULL }, ------
A PCI card is configured in decode_regions() so that 'hose' has only one entry per each type of memory regions. This behavior was introduced by Simon's patch: ------ commit 9526d83ac5a Author: Simon Glass sjg@chromium.org Date: Thu Nov 19 20:26:58 2015 -0700
dm: pci: Support decoding ranges with duplicate entries ------As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
I believe that this behavior is the root cause of my issue, and by reverting the patch mentioned above, everything works fine.
While I understand a concern mentioned in the commit message, there should be a better way to manage the case.
There was a series that changed things in this area. Can you take a look?
PCI: dm: Ignore 64-bit memory regions if CONFIG_SYS_PCI_64BIT not setAh, I didn't know that, but it seems to me that it is still insufficient. This hack won't work on 32-bit PCI card. I found another patch from Tuomas:
Did you try it? As of today's master all of the patches are applied and at least the e1000 NIC and the Intel AHCI card that I tested works. The effect of the commit is to indeed avoid the problem you mentioned:
Yes, I ran my patch but *with* CONFIG_SYS_PCI_64BIT.
I meant: Did you try your QEMU command that enables the SDHCI on U-Boot master, without any patches? It should just work.
- As a result, MMIO region (0x1000.0000-0x2eff.0000) is overwritten and MMIO_HIGH is the only one available at runtime.
Note that even on aarch64, CONFIG_SYS_PCI_64BIT is *not* set by default. And on ARM we would need to skip that region in U-Boot anyway because we don't have the means to access physical addresses above the 4GB boundary with the CPU using U-Boot's identity-mapped page tables.
Maybe you're right regarding aarch64, but the issue is not about arm/arm64 but PCI configuration. Some arch/machines, freescale mostly?, have already enabled CONFIG_SYS_PCI_64BIT. I'm afraid that there may be a possibility that your patch breaks them.
Which patch?
The "PCI: dm: Ignore 64-bit memory regions if CONFIG_SYS_PCI_64BIT not set" commit adds:
+ if (!IS_ENABLED(CONFIG_SYS_PCI_64BIT) && + type == PCI_REGION_MEM && upper_32_bits(pci_addr)) { + debug(" - beyond the 32-bit boundary, ignoring\n"); + continue; + }
I.e. the patch is a no-op if CONFIG_SYS_PCI_64BIT is set. Also I don't think PowerPC uses DM PCI, so that code is not executed at all on those platforms.
If you mean in "PCI: autoconfig: Don't allocate 64-bit addresses to 32-bit only resources" that someone could have been relying on the truncation of high 32 bits of bus addresses with 32-bit only devices, none of the FSL platforms have bus addresses above 32-bits, only physical addresses (seen with git grep CONFIG_SYS_PCIE._MEM_), so that shouldn't cause a problem either.
participants (3)
-
AKASHI Takahiro -
Simon Glass -
Tuomas Tynkkynen