Re: [Qemu-devel] [edk2] Could not add PCI device with big memory to aarch64 VMs

[Laszlo Ersek]

On 11/30/15 19:45, liang yan wrote:
> 
> 
> On 11/04/2015 05:53 PM, Laszlo Ersek wrote:
>> On 11/04/15 23:22, liang yan wrote:
>>> Hello, Laszlo,
>>>
>>>
>>> (2)It also has a problem that once I use a memory bigger than 256M for
>>> ivshmem, it could not get through UEFI,
>>> the error message is
>>>
>>> PciBus: Discovered PCI @ [00|01|00]
>>>     BAR[0]: Type =  Mem32; Alignment = 0xFFF;    Length = 0x100;
>>> Offset =
>>> 0x10
>>>     BAR[1]: Type =  Mem32; Alignment = 0xFFF;    Length = 0x1000; Offset
>>> = 0x14
>>>     BAR[2]: Type = PMem64; Alignment = 0x3FFFFFFF;    Length =
>>> 0x40000000;    Offset = 0x18
>>>
>>> PciBus: HostBridge->SubmitResources() - Success
>>> ASSERT
>>> /home/liang/studio/edk2/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c(449):
>>> ((BOOLEAN)(0==1))
>>>
>>>
>>> I am wandering if there are memory limitation for pcie devices under
>>> Qemu environment?
>>>
>>>
>>> Just thank you in advance and any information would be appreciated.
>> (CC'ing Ard.)
>>
>> "Apparently", the firmware-side counterpart of QEMU commit 5125f9cd2532
>> has never been contributed to edk2.
>>
>> Therefore the the ProcessPciHost() function in
>> "ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c" ignores the
>> DTB_PCI_HOST_RANGE_MMIO64 type range from the DTB. (Thus only
>> DTB_PCI_HOST_RANGE_MMIO32 is recognized as PCI MMIO aperture.)
>>
>> However, even if said driver was extended to parse the new 64-bit
>> aperture into PCDs (which wouldn't be hard), the
>> ArmVirtPkg/PciHostBridgeDxe driver would still have to be taught to look
>> at that aperture (from the PCDs) and to serve MMIO BAR allocation
>> requests from it. That could be hard.
>>
>> Please check edk2 commits e48f1f15b0e2^..e5ceb6c9d390, approximately,
>> for the background on the current code. See also chapter 13 "Protocols -
>> PCI Bus Support" in the UEFI spec.
>>
>> Patches welcome. :)
>>
>> (A separate note on ACPI vs. DT: the firmware forwards *both* from QEMU
>> to the runtime guest OS. However, the firmware parses only the DT for
>> its own purposes.)
> Hello, Laszlo,
> 
> Thanks for your advices above, it's very helpful.
> 
> When debugging, I also found some problems for 32 bit PCI devices.
> Hope could get some clues from you.
> 
> I checked on 512M, 1G, and 2G devices.(4G return invalid parameter
> error, so I think it may be taken as a 64bit devices, is this right?).

I guess so.

> 
> 
> First,
> 
> All devices start from base address 3EFEFFFF.

According to the below:

> ProcessPciHost: Config[0x3F000000+0x1000000) Bus[0x0..0xF]
> Io[0x0+0x10000)@0x3EFF0000 Mem[0x10000000+0x2EFF0000)@0x0

the address you mention (0x3EFEFFFF) is the *highest* inclusive
guest-phys address that an MMIO BAR can take. Not sure if that's what
you meant.

The size of the MMIO aperture for the entire PCI host is 0x2EFF0000
bytes: a little less than 752 MB. So devices that need 1G and 2G MMIO
BARs have no chance.

> PcdPciMmio32Base is  10000000=====================
> PcdPciMmio32Size is  2EFF0000=====================
> 
> 
> Second,
> 
> It could not get new base address when searching memory space in GCD map.
> 
> For 512M devices,
> 
> *BaseAddress = (*BaseAddress + 1 - Length) & (~AlignmentMask);

This seems to be from CoreAllocateSpace()
[MdeModulePkg/Core/Dxe/Gcd/Gcd.c]. AlignmentMask is computed from the
Alignment input parameter.

Which in turn seems to come from the BitsOfAlignment parameter computed
in NotifyPhase(), "ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c".

> 
> BaseAddress is 3EFEFFFF==========================

So this is the highest address (inclusive) where the 512MB BAR could end.

> new BaseAddress is 1EEF0000==========================

This is the highest address (inclusive) where the 512MB BAR could start.

This should be rounded down to an 512MB alignment (I believe), and then
checked if that is still in the MMIO aperture.

512MB is 0x2000_0000.

Rounding 0x1EEF_0000 down to an integral multiple of 0x2000_0000 results
in zero:

> ~AlignmentMask is E0000000==========================
> Final BaseAddress is 0000

And that address does not fall into the MMIO aperture.

In other words, although your size requirement of 512MB could be
theoretically satisfied from the aperture (which extends from exactly
256 MB to a little lower than 1008 MB), if you *also* require the base
address to be aligned at 512MB, then that cannot be satisfied.

> 
> Status = CoreSearchGcdMapEntry (*BaseAddress, Length, &StartLink,
> &EndLink, Map);
> 
> 
> 
> For bigger devices:
> 
> all stops when searching memory space because below code, Length will
> bigger than MaxAddress(3EFEFFFF)
> 
> if ((Entry->BaseAddress + Length) > MaxAddress) {
>          continue;
> }
> 
> 
> I also checked on ArmVirtQemu.dsc which all set to 0.
> 
>   gArmPlatformTokenSpaceGuid.PcdPciBusMin|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciBusMax|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciIoBase|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciIoSize|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciMmio32Base|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciMmio32Size|0x0
>   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x0

These are just default values for the PCDs, to be set later by
VirtFdtDxe, keyed off of QEMU's DTB.

(In a platform DSC, you cannot mark a PCD as dynamic without providing a
default value too. Hence the actual default values here aren't relevant.)

> 
> 
> Do you think I should change from PcdPciMmio32Base and PcdPciMmio32Size,
> or do some change for GCD entry list, so it could allocate resources for
> PCI devices(CoreSearchGcdMapEntry)?

No, I think the code is working as expected.

For devices with 1GB and larger MMIO BAR requirements, the host bridge
simply doesn't have enough room in its MMIO aperture.

For a device with an 512MB MMIO BAR requirement, there would be enough
room (considering nothing but the size); however the alignment
requirement of the BAR makes the request impossible to satisfy. The only
address aligned at 512MB in the host bridge aperture is the absolute
address 512MB itself, and from that point upwards, you don't have 512MB
space, only approx. 1008 - 512 = 496 MB.

This is all rooted in the specific, numerical boundaries of the
VIRT_PCIE_MMIO entry, in QEMU's "hw/arm/virt.c".

Now, if you changed *those* values (that is, the VIRT_PCIE_MMIO
boundaries in QEMU), then that would trickle down to the firmware as
well, through the generated DTB, and then larger devices might work.
But, of course this is not the right way to go about it; that's why QEMU
introduced VIRT_PCIE_MMIO_HIGH.

... You didn't mention 256MB devices -- I assume that is because those
work already. Namely, two such BARs should be possible in the current
aperture; I think at 256MB and at 512MB.

If you need more room (with large alignments), then there's no way
around supporting QEMU's 64 bit aperture, VIRT_PCIE_MMIO_HIGH (see my
earlier email).

Unfortunately I can't readily help with that in the
"ArmVirtPkg/PciHostBridgeDxe" driver; there's no such (open-source)
example in the edk2 tree. Of course, I could experiment with it myself
-- only not right now.

I guess copying and adapting the TypeMem32 logic to TypeMem64 (currently
short-circuited with EFI_ABORTED) could work.

Thanks
Laszlo

> Looking forward to your reply.
> 
> 
> Thanks,
> Liang
> 
>> Thanks
>> Laszlo
>>
> 
> _______________________________________________
> edk2-devel mailing list
> address@hidden
> https://lists.01.org/mailman/listinfo/edk2-devel