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

[Laszlo Ersek <lersek@redhat.com>]

On 02/06/19 16:46, Ard Biesheuvel wrote:
> On Wed, 6 Feb 2019 at 12:11, Laszlo Ersek <lersek@redhat.com> 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
>>
> 
> For the series
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Thanks Laszlo
> 

Awesome, thank you! :)

Julien, can you please fetch the series and run some light
regression-tests on Xen? Again, no functionality change is intended or
expected.

Thanks!
Laszlo