On 08/27/14 10:27, Ard Biesheuvel wrote:
> On 27 August 2014 01:36, Laszlo Ersek <[email protected]> wrote:
>> On 08/26/14 15:03, Ard Biesheuvel wrote:
>>> +#pragma pack (1)
>>> +typedef struct {
>>> + VENDOR_DEVICE_PATH Vendor;
>>> + UINT32 Instance;
>>> + EFI_DEVICE_PATH_PROTOCOL End;
>>> +} VIRTIO_BLK_DEVICE_PATH;
>>> +#pragma pack ()
>>
>> Ugh. :)
>>
>> Have you seen my earlier patches where the vendor device path node
>> corresponding to a virtio-mmio transport actually included the base
>> address of the MMIO register block? I think such a hwvendor devpath node
>> is much more useful; it can actually help you debug.
>>
>
> I will just stick a EFI_PHYSICAL_ADDRESS in there instead, or would
> you prefer UINT64?
UINT64 sounds good.
>>> + FdtImageData = AllocatePages (EFI_SIZE_TO_PAGES (fdt_totalsize
>>> (MemoryBase)));
>>
>> What's wrong with AllocatePool()? Does the EFI stub require full pages,
>> and page alignment?
>>
>
> No, AllocatePool() should be fine.
OK.
Tying back to the other patch's discussion, I've already mentioned
AllocateCopyPool(). It's a nice convenience function that can save you a
separate call to CopyMem().
(Independently, we should be very clear that the
AllocatePages()/AllocatePool() calls discussed here, vs. the one that
you'll add to the platform PEI, are *very* different. They are from the
same MemoryAllocationLib library class, but the library instances in
question are specific to the UEFI phase:
- MdePkg/Library/PeiMemoryAllocationLib/
- MdePkg/Library/UefiMemoryAllocationLib/
The former is ultimately backed by HOB creation, while the latter is
backed by a UEFI boot service.
So, you'll remove this DTB relocation from DXE completely, and do it in
the platform PEIM instead (either with in-place coverage, or real
relocation to permanent PEI RAM, with AllocateCopyPool()).)
> I will check the logic carefully. What should happen is that the
> traversal of the device tree should proceed, so other peripherals are
> detected and installed even if the virtio transports are not.
Sounds great. Actually, I think we should keep whatever virtio-mmio
transports we managed to install as well; it's not like there's some
inter-dependency between virtio devices. Whatever works is likely useful
in isolation too.
>>> + break;
>>> +
>>> + case GIC:
>>> + ASSERT (Len == 32);
>>> +
>>> + DistBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
>>> + CpuBase = fdt64_to_cpu (((UINT64 *)RegProp)[2]);
>>> + PcdSet32 (PcdGicDistributorBase, DistBase);
>>> + PcdSet32 (PcdGicInterruptInterfaceBase, CpuBase);
>>
>> DistBase and CpuBase are UINT32s, matching the PCDs, but they don't
>> match the FDT (UINT64). Is that okay?
>>
>
> Well, it is highly unlikely that peripherals like GIC would be moved
> beyond 4 GB in the physical address space, and the device tree just
> uses 64-bit addresses in this instantiation, so for now, I think this
> is fine. Otherwise, we should update the GIC driver to use UINT64
> instead.
Alright. Please either add an assert, or just a small comment, for the
uninitiated like me :)
[snip]
Thanks
Laszlo
------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds. Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
