Re: [PATCH 00/14] ArmVirtPkg: clean up set-but-unused PCDs

[Laszlo Ersek <lersek@redhat.com>]

On 02/06/19 13:10, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: armvirt_pcd_clean
> 
> (1) The procedure described below depends on:
> 
>     [edk2] [PATCH]
>     BaseTools/BuildReport: fix report for platforms/arches without struct PCDs
> 
>     http://mid.mail-archive.com/20190205112213.682-1-lersek@redhat.com
>     https://lists.01.org/pipermail/edk2-devel/2019-February/036320.html
> 
> (2) Background: while working on the fix in (1), I noticed that the PCD
>     sections in the build reports of various ArmVirt platforms contained
>     "PCDs not used by modules or in conditional directives". I thought
>     that we should attempt to clean those up. Subsequently I built the
>     following 36 ArmVirt platforms:
> 
>> extra_opts=("" "-D HTTP_BOOT_ENABLE -D NETWORK_IP6_ENABLE -D SECURE_BOOT_ENABLE -D TTY_TERMINAL")
>> for arch in ARM AARCH64; do
>>   for platform in Qemu QemuKernel Xen; do
>>     for target in NOOPT DEBUG RELEASE; do
>>       for extra in 0 1; do
>>         GCC5_ARM_PREFIX=arm-linux-gnu- \
>>         GCC5_AARCH64_PREFIX=aarch64-linux-gnu- \
>>         build \
>>           -a $arch \
>>           -p ArmVirtPkg/ArmVirt${platform}.dsc \
>>           -t GCC5 \
>>           -b $target \
>>           -n $(getconf _NPROCESSORS_ONLN) \
>>           --report-file=$HOME/tmp/report.$arch.$platform.$target.$extra.txt \
>>           --report-type=PCD \
>>           --cmd-len=65536 \
>>           ${extra_opts[$extra]}
>>       done
>>     done
>>   done
>> done
> 
>     Then I gradually eliminated the redundant PCD settings.
> 
> (3) At the bottom of this email (i.e., the series cover letter), I'm
>     including a base64-encoded tarball of report files, saved (like
>     described in (2)) before and after the series. Diffing the reports
>     proves that the series cleans up the PCD settings without any
>     changes observable to modules.
> 
> (4) The series advances in small steps. The reason is that some of the
>     facts exposed could be surprising (I know I was surprised), and we
>     could decide that we want to do something else (e.g. file a BZ, and
>     fill the gap later). For such cases I wanted to be able to drop
>     individual patches at will.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Julien Grall <julien.grall@linaro.org>
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (14):
>   ArmVirtPkg/ArmVirtQemuKernel: don't set PcdCPUCoresStackBase
>   ArmVirtPkg: don't set PcdRelocateVectorTable
>   ArmVirtPkg/{ArmVirtQemu,ArmVirtQemuKernel}: don't set
>     PcdTrustzoneSupport
>   ArmVirtPkg: don't set PcdPostCodePropertyMask
>   ArmVirtPkg: clean up PcdSetNxForStack setting (applies to ArmVirtQemu
>     only)
>   ArmVirtPkg/PrePi: drop wrong PcdCoreCount dependency
>   ArmVirtPkg: don't set PcdCoreCount
>   ArmVirtPkg: don't set PcdDebugClearMemoryValue
>   ArmVirtPkg: don't set PcdDebugPrintErrorLevel in RELEASE builds
>   ArmVirtPkg/ArmVirtXen: don't set PcdPL031RtcBase
>   ArmVirtPkg/ArmVirtXen: don't set PcdTerminalTypeGuidBuffer
>   ArmVirtPkg/ArmVirtXen: don't set PcdShellFile
>   ArmVirtPkg/ArmVirtXen: don't set PcdTurnOffUsbLegacySupport
>   ArmVirtPkg/ArmVirtXen: don't set Pcd*ImageVerificationPolicy
> 
>  ArmVirtPkg/ArmVirt.dsc.inc                          | 27 ++------------------
>  ArmVirtPkg/ArmVirtQemu.dsc                          | 21 ++++++++++++---
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                    | 17 ++++++++----
>  ArmVirtPkg/ArmVirtXen.dsc                           |  9 -------
>  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf |  2 --
>  5 files changed, 31 insertions(+), 45 deletions(-)

series pushed as 63d8431a49cb..da06a2a2fa1e

thanks
Laszlo