Re: [PATCH] ArmVirtPkg: remove ArmPlatformSysConfigLib dependency

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

On 16 November 2017 at 14:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 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


Pushed as f311e5d8b5e1928eec602334194187d7811d905a (with the --cpu
issue fixed due to 302e8eda3b2f848e54bd21d0a8b2e5d8b891bffd 'ArmPkg:
move RVCT PLATFORM_FLAGS override into ArmHvcLib/ArmSmcLib')