[U-Boot] inappropriate PCI configuration on arm64 qemu?

[Tuomas Tynkkynen]

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 at 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 at 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:

 >>> * 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 at 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.