On 21 November 2017 at 16:56, Laszlo Ersek <ler...@redhat.com> wrote:
> On 11/17/17 17:09, Ard Biesheuvel wrote:
>> Clone the existing ArmPlatformGetVirtualMemoryMap () for this platform,
>> clean it up slightly (by removing the support for uncached DRAM mappings),
>> and turn it into a new ArmVirtMemInfoLib implementation.
>
> I've looked at this patch with "git show --find-copies-harder". It looks
> OK, but the commit message could be improved:
>
> (1) the support for uncached DRAM mappings is removed in the copy-origin
> lib instance, in patch 09/15 ("ArmVirtPkg/ArmVirtPlatformLib: remove
> support for uncached mappings"). I think this sentence should be dropped
> from the commit message.
>
> (2) There are other cleanups however:
>
> - factor out TableSize and TopOfMemory
>
> - replace EFI_D_* with DEBUG_*
>
> - fetch PcdFdBaseAddress with PcdGet64(), not FixedPcdGet64(). (This is
> matched by the [Pcd] / [FixedPcd] sections in the new INF file.)
>
> Can you elaborate on the last item? I wonder if that change qualifies as
> cleanup. (I'm fine if the change is justified by some other flexibility,
> but it should be documented please.)
>
> With the commit message updated:
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
>Thanks. But is actually based on ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib, which explains the latter point. However, given that we will be sharing it between ArmVirtQemu and ArmVirtQemuKernel later on (which is apparently justified, since git can't even tell them apart), it makes sense to elaborate a bit on the differences and changes wrt the originals. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
