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.aka...@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 <s...@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.tynkky...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot