On 05/28/15 18:51, Laszlo Ersek wrote:
> From: Olivier Martin <[email protected]>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Olivier Martin <[email protected]>
> Cc: Olivier Martin <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> [[email protected]: no changes other than adding Cc's]
> Signed-off-by: Laszlo Ersek <[email protected]>
>
> Signed-off-by: Laszlo Ersek <[email protected]>
> ---
Okay, so I'm reviewing this because I reposted Olivier's patch without
any changes (modulo the different formatting requested from
git-format-patch).
... The diffstat just below is already good justification for this rename :)
Anyway, first of all, after applying this patch, I grepped the tree for
the string "ArmVirtualization", and I got one hit in
"ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.c".
Normally that reference should be updated too, but it's just in a
comment in a C source file, and the correct (new) place can be figured
out easily, so there's no need to update this patch just for that.
(In passing I'll note that such issues -- although it's not a real issue
in this case -- can not be caught without actually *applying* the patch
for review. So this is another argument for the parallel discussion: a
web app would not be a step forward over email, because one really needs
to run "git grep" locally.)
Anyway, back to this patch.
I checked
git ls-files | grep ArmVirtualization
too, no hits; good.
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc =>
> ArmVirtPkg/ArmVirt.dsc.inc
> | 18 +++++-----
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec =>
> ArmVirtPkg/ArmVirtPkg.dec
> | 16 ++++-----
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc =>
> ArmVirtPkg/ArmVirtQemu.dsc
> | 38
> ++++++++++----------
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.fdf =>
> ArmVirtPkg/ArmVirtQemu.fdf
> | 8 ++---
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.dsc =>
> ArmVirtPkg/ArmVirtXen.dsc
> | 32 ++++++++---------
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.fdf =>
> ArmVirtPkg/ArmVirtXen.fdf
> | 8 ++---
> {ArmPlatformPkg/ArmVirtualizationPkg => ArmVirtPkg}/Include/ArmPlatform.h
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Include/Guid/EarlyPL011BaseAddress.h
>
> | 0
>
> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationDxeHobLib/ArmVirtualizationDxeHobLib.inf
> => ArmVirtPkg/Library/ArmVirtDxeHobLib/ArmVirtDxeHobLib.inf
> | 2 +-
> {ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationDxeHobLib =>
> ArmVirtPkg/Library/ArmVirtDxeHobLib}/HobLib.c
> | 0
>
> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c
> => ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> | 0
>
> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf
> => ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf
> | 2 +-
> {ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib =>
> ArmVirtPkg/Library/ArmVirtPlatformLib}/AARCH64/VirtHelper.S
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib =>
> ArmVirtPkg/Library/ArmVirtPlatformLib}/ARM/VirtHelper.S
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib =>
> ArmVirtPkg/Library/ArmVirtPlatformLib}/ARM/VirtHelper.asm
> | 0
>
> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/ArmVirtualizationPlatformLib.inf
> => ArmVirtPkg/Library/ArmVirtPlatformLib/ArmVirtPlatformLib.inf
> | 6 ++--
> {ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib =>
> ArmVirtPkg/Library/ArmVirtPlatformLib}/Virt.c
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib =>
> ArmVirtPkg/Library/ArmVirtPlatformLib}/VirtMem.c
> | 0
>
> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.c
> => ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.c
> | 0
>
> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.inf
> =>
> ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf |
> 8 ++---
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/ArmXenRelocatablePlatformLib/AARCH64/MemnodeParser.S
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/ArmXenRelocatablePlatformLib/AARCH64/RelocatableVirtHelper.S
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/ArmXenRelocatablePlatformLib/ArmXenRelocatablePlatformLib.inf
>
> | 6 ++--
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/ArmXenRelocatablePlatformLib/RelocatableVirt.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/ArmXenRelocatablePlatformLib/XenVirtMem.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/BaseCachingPciExpressLib/PciExpressLib.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf
>
> | 6 ++--
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf
>
> | 4 +--
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/NorFlashQemuLib/NorFlashQemuLib.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/NorFlashQemuLib/NorFlashQemuLib.inf
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/PlatformIntelBdsLib/IntelBdsPlatform.h
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/PlatformIntelBdsLib/QemuKernel.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/PlatformPeiLib/PlatformPeiLib.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/PlatformPeiLib/PlatformPeiLib.inf
>
> | 6 ++--
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/QemuFwCfgLib/QemuFwCfgLib.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/QemuFwCfgLib/QemuFwCfgLib.inf
>
> | 6 ++--
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/XenRealTimeClockLib/XenRealTimeClockLib.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PciHostBridgeDxe/PciHostBridge.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PciHostBridgeDxe/PciHostBridge.h
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PciHostBridgeDxe/PciHostBridgeDxe.inf
>
> | 4 +--
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PciHostBridgeDxe/PciRootBridgeIo.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PrePi/AArch64/ArchPrePi.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PrePi/AArch64/ModuleEntryPoint.S
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>
> | 6 ++--
> {ArmPlatformPkg/ArmVirtualizationPkg => ArmVirtPkg}/PrePi/LzmaDecompress.h
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg => ArmVirtPkg}/PrePi/PrePi.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg => ArmVirtPkg}/PrePi/PrePi.h
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/PrePi/Scripts/PrePi-PIE.lds
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg => ArmVirtPkg}/VirtFdtDxe/VirtFdtDxe.c
>
> | 0
> {ArmPlatformPkg/ArmVirtualizationPkg =>
> ArmVirtPkg}/VirtFdtDxe/VirtFdtDxe.inf
>
> | 8 ++---
> 56 files changed, 92 insertions(+), 92 deletions(-)
The renames implied by the diffstat look good. I copied the diffstat to
a text editor, performed the following replacements:
ArmPlatformPkg/ArmVirtualizationPkg --> ArmVirtPkg
ArmVirtualization --> ArmVirt
and verified if the left side of each { => } matched the right side
then. They all did, so this is fine.
[snip]
I validated the source code hunks in the patch practically the same way
-- performed the above replacements on the patch itself, and confirmed
that the - lines matched the + lines.
This also brought out any other differences:
> -# Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> +# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
Okay.
> -# Copyright (c) 2011, 2013, ARM Limited. All rights reserved.
> +# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
Okay.
> -# Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> +# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
Okay.
> -# Copyright (c) 2011, 2013, ARM Limited. All rights reserved.
> +# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
Okay.
> -# Copyright (c) 2011-2014, ARM Limited. All rights reserved.
> +# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
Okay.
> -# Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2011-2015, ARM Ltd. All rights reserved.<BR>
Okay.
> -# Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2011-2015, ARM Ltd. All rights reserved.<BR>
Okay.
> -# Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2011-2015, ARM Ltd. All rights reserved.<BR>
Okay.
[snip]
The patch looks good. Let me see if it builds:
- ArmVirtXen: builds (I didn't run it)
- ArmVirtQemu: builds (I didn't run it)
- ArmVirtQemu, with -D INTEL_BDS: builds, and runs too
Reviewed-by: Laszlo Ersek <[email protected]>
Tested-by: Laszlo Ersek <[email protected]>
Thanks!
Laszlo
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel