Re: [PATCH] ArmVirtPkg: remove ArmPlatformSysConfigLib dependency

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 16 November 2017 at 12:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>
>> On 16 Nov 2017, at 11:59, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>>
>>> On Thu, Nov 16, 2017 at 09:40:58AM +0000, Ard Biesheuvel wrote:
>>>> On 15 November 2017 at 14:03, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>>> Now that the PL031 RTC driver library no longer depends on the ARM
>>>> platform specific ArmPlatformSysConfigLib, we no longer need to
>>>> implement ArmPlatform.h or have a resolution for that library.
>>>> This allows us to get rid of a rather dodgy practice of including
>>>> platform headers using compiler flags, which is a bad idea at various
>>>> levels.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>> PL031 patch is on-list and pending.
>>>>
>>>> ArmVirtPkg/ArmVirtQemu.dsc                                         |  7 -----
>>>> ArmVirtPkg/ArmVirtQemuKernel.dsc                                   |  6 ----
>>>> ArmVirtPkg/ArmVirtXen.dsc                                          |  6 ----
>>>> ArmVirtPkg/Include/ArmPlatform.h                                   | 33 --------------------
>>>> ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/QemuVirtMem.c     |  1 -
>>>> ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/RelocatableVirt.c |  1 -
>>>> ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c                       |  1 -
>>>> ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c                    |  1 -
>>>> ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/RelocatableVirt.c  |  1 -
>>>> ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/XenVirtMem.c       |  1 -
>>>> ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c               |  7 ++++-
>>>> 11 files changed, 6 insertions(+), 59 deletions(-)
>>>>
>>>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>>>> index 8a60b61f2aa6..d1b3849d856a 100644
>>>> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>>>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>>>> @@ -49,7 +49,6 @@ [LibraryClasses.common]
>>>>   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
>>>>
>>>>   ArmPlatformLib|ArmVirtPkg/Library/ArmVirtPlatformLib/ArmVirtPlatformLib.inf
>>>> -  ArmPlatformSysConfigLib|ArmPlatformPkg/Library/ArmPlatformSysConfigLibNull/ArmPlatformSysConfigLibNull.inf
>>>>
>>>>   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
>>>>   NorFlashPlatformLib|ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
>>>> @@ -71,12 +70,6 @@ [LibraryClasses.common]
>>>> [LibraryClasses.common.UEFI_DRIVER]
>>>>   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>>>>
>>>> -[BuildOptions]
>>>> -  RVCT:*_*_ARM_PLATFORM_FLAGS == --cpu Cortex-A15 -I$(WORKSPACE)/ArmVirtPkg/Include
>>>> -  GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15 -I$(WORKSPACE)/ArmVirtPkg/Include
>>>
>>> Oops. I will need to retain the -mcpu setting here, or the 'hvc'
>>> instruction will be rejected by the assembler.
>>
>> Don't we have source-level overrides for that?
>> At least ".arch_extension virt" for GNU.
>>
>
> Yeah, or at least move the cflags override to the .inf of the library that uses the hvc instruction

As it turns out, ArmVirtQemu.dsc builds find without the -mcpu CFLAGS,
due to the fact that we already have .arch_extension virt' in
ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.S, which is the only file that is
affected by this AFAICT. I will double check how RVCT deals with this,
but I will move the --cpu=Cortex-A15 flag into
ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf at the very least