On 08/28/14 14:50, Ard Biesheuvel wrote: > On 28 August 2014 14:03, Laszlo Ersek <[email protected]> wrote: >> comments below >> >> On 08/27/14 17:12, Ard Biesheuvel wrote: >>
>>> + ASSERT (DeviceTreeBase != NULL); >> >> In the hope that I'm not suggesting something insane, you might also >> want to assert at this point that PcdDeviceTreeBaseAddress equals >> PcdSystemMemoryBase. >> > > I don't think it is necessary for this module to be that rigid. > In particular, now that we have PcdDeviceTreeBaseAddress, we can > define it in the DSC, and in QEMU's case, its default value will be > 0x4000_0000. > Then, if you are using a slightly different flavor of emulation, it > would be perfectly legal to define this to point elsewhere (as long as > it points to where you are in fact putting the DTB). If this address > is not covered by system DRAM, that is perfectly fine. Agreed. > In fact, the > asserts should only fire if we detect here that > PcdDeviceTreeBaseAddress is in DRAM, and PlatformPeim() should only > perform the relocation under that condition as well. I agree. But, this is quickly getting quite hairy, so, for now, can you please just ignore my above ASSERT() suggestion, and keep the rest of the ASSERT()s as we discussed before? It's already quite hard to keep track all of this. If you made them more dynamic only later, in further patches, that would keep my present review manageable (as opposed to, well, mind-boggling :)) >>> + VirtualMemoryTable = AllocatePages (EFI_SIZE_TO_PAGES ( >>> + sizeof (ARM_MEMORY_REGION_DESCRIPTOR) >>> + * >>> MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS)); >> >> The utterly pedantic formatting would be >> >> VirtualMemoryTable = AllocatePages ( >> EFI_SIZE_TO_PAGES ( >> sizeof (ARM_MEMORY_REGION_DESCRIPTOR) * >> MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS) >> ) >> ); >> >> I leave it to you... >> > > No no, I am going for full marks! Haha, great; 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
