[PATCH 0/2] ArmVirtPkg EmbeddedPkg: fix build for CLANG35/ARM

[PATCH 0/2] ArmVirtPkg EmbeddedPkg: fix build for CLANG35/ARM

[Ard Biesheuvel]

Currently, the ArmVirtQemuKernel and ArmVirtXen platforms will not build
for ARM when using CLANG35, due to the fact that the compiler emits
MOVT/MOVW pairs into objects that are used by the relocatable PrePi, and
such instruction pairs are not runtime relocatable in ELF (i.e., there are
no dynamic relocation types to describe them)

So fix this by selectively inhibiting the use of these pairs when building
these platforms for ARM using CLANG35

Ard Biesheuvel (2):
  EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library
  ArmVirtPkg ARM: make relocatable PrePi users build with CLANG35

 ArmVirtPkg/ArmVirtQemuKernel.dsc                                          | 8 ++++++++
 ArmVirtPkg/ArmVirtXen.dsc                                                 | 9 +++++++++
 EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf | 2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH 1/2] EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library

[Ard Biesheuvel]

This library is only used by the various PrePi implementations, all of
which are of type SEC. So make this library SEC as well. This may affect
the build options used by the platform.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
index 21f6eb1e14bc..ea3d0f5da9c2 100644
--- a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
+++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
@@ -15,7 +15,7 @@ [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = PrePiMemoryAllocationLib
   FILE_GUID                      = 4f14c900-51a9-11e0-afbf-0002a5d5c51b
-  MODULE_TYPE                    = PEIM
+  MODULE_TYPE                    = SEC
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = MemoryAllocationLib
 
-- 
2.7.4

[PATCH 2/2] ArmVirtPkg ARM: make relocatable PrePi users build with CLANG35

[Ard Biesheuvel]

The clang developers have made a backward incompatible change to the
command line arguments, and have replaced '-mllvm -arm-use-movt=0'
with '-mno-movt'. This does not matter for most ARM platforms, and
therefore it has been removed from the default CLANG35/ARM CC flags
in patch 1c63516075b3 ("BaseTools CLANG35: drop problematic use-movt
and save-temps options"), but as it turns out, the relocatable PrePi
implementation used by ArmVirtQemuKernel and ArmVirtXen will fail to
build if it contains MOVT/MOVW pairs, due to the fact that these are
not runtime relocatable under ELF.

Since they are runtime relocatable under PE/COFF, and GenFw does the
right thing when encountering them, selectively controlling their
use is more appropriate than disabling them altogether. Therefore,
this patch adds the -mno-movt argument only for the platforms that
use the relocatable PrePi, and only for the module types that may
be pulled into its build.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++
 ArmVirtPkg/ArmVirtXen.dsc        | 9 +++++++++
 2 files changed, 17 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 01a1d9b4613b..6c536d9bbd2d 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -45,6 +45,9 @@ [LibraryClasses.ARM]
   ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
   ArmCpuLib|ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
 
+[LibraryClasses.ARM.SEC]
+  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7LibSec.inf
+
 [LibraryClasses.common]
   ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
 
@@ -77,6 +80,11 @@ [BuildOptions]
   GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15 -I$(WORKSPACE)/ArmVirtPkg/Include
   *_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/ArmVirtPkg/Include
 
+[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE]
+  # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE
+  # executable we build for the relocatable PrePi. They are not runtime
+  # relocatable in ELF.
+  *_CLANG35_*_CC_FLAGS = -mno-movt
 
 ################################################################################
 #
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 5ad1bf630bda..4ebead5ba6e6 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -44,6 +44,9 @@ [LibraryClasses.ARM]
   ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
   ArmCpuLib|ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
 
+[LibraryClasses.ARM.SEC]
+  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7LibSec.inf
+
 [LibraryClasses.common]
   ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
 
@@ -77,6 +80,12 @@ [BuildOptions]
   GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15 -I$(WORKSPACE)/ArmVirtPkg/Include
   GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/ArmVirtPkg/Include
 
+[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE]
+  # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE
+  # executable we build for the relocatable PrePi. They are not runtime
+  # relocatable in ELF.
+  *_CLANG35_*_CC_FLAGS = -mno-movt
+
 ################################################################################
 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
-- 
2.7.4

Re: [PATCH 1/2] EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library

[Laszlo Ersek]

On 08/03/16 10:21, Ard Biesheuvel wrote:
> This library is only used by the various PrePi implementations, all of
> which are of type SEC.

You can actually enforce that client module type restriction, by setting

    LIBRARY_CLASS                  = MemoryAllocationLib|SEC

Can you try that, in addition to the MODULE_TYPE change?

Just an idea, of course.

Thanks,
Laszlo

> So make this library SEC as well. This may affect
> the build options used by the platform.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> index 21f6eb1e14bc..ea3d0f5da9c2 100644
> --- a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> +++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> @@ -15,7 +15,7 @@ [Defines]
>    INF_VERSION                    = 0x00010005
>    BASE_NAME                      = PrePiMemoryAllocationLib
>    FILE_GUID                      = 4f14c900-51a9-11e0-afbf-0002a5d5c51b
> -  MODULE_TYPE                    = PEIM
> +  MODULE_TYPE                    = SEC
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = MemoryAllocationLib
>  
> 

Re: [PATCH 1/2] EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library

[Ard Biesheuvel]

On 3 August 2016 at 11:56, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/03/16 10:21, Ard Biesheuvel wrote:
>> This library is only used by the various PrePi implementations, all of
>> which are of type SEC.
>
> You can actually enforce that client module type restriction, by setting
>
>     LIBRARY_CLASS                  = MemoryAllocationLib|SEC
>
> Can you try that, in addition to the MODULE_TYPE change?
>
> Just an idea, of course.
>

That is a valid point, but it is kind of orthogonal to the issue I am
trying to solve.

In patch #2, I override the CC flags for SEC and BASE type modules,
but this static library gets build with the PEIM rules in effect, so I
don't really mind if anyone uses this module elsewhere. I could
perhaps simply change the type to BASE as well.

Re: [PATCH 2/2] ArmVirtPkg ARM: make relocatable PrePi users build with CLANG35

[Laszlo Ersek]

On 08/03/16 10:21, Ard Biesheuvel wrote:
> The clang developers have made a backward incompatible change to the
> command line arguments, and have replaced '-mllvm -arm-use-movt=0'
> with '-mno-movt'. This does not matter for most ARM platforms, and
> therefore it has been removed from the default CLANG35/ARM CC flags
> in patch 1c63516075b3 ("BaseTools CLANG35: drop problematic use-movt
> and save-temps options"), but as it turns out, the relocatable PrePi
> implementation used by ArmVirtQemuKernel and ArmVirtXen will fail to
> build if it contains MOVT/MOVW pairs, due to the fact that these are
> not runtime relocatable under ELF.
> 
> Since they are runtime relocatable under PE/COFF, and GenFw does the
> right thing when encountering them, selectively controlling their
> use is more appropriate than disabling them altogether. Therefore,
> this patch adds the -mno-movt argument only for the platforms that
> use the relocatable PrePi, and only for the module types that may
> be pulled into its build.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++
>  ArmVirtPkg/ArmVirtXen.dsc        | 9 +++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 01a1d9b4613b..6c536d9bbd2d 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -45,6 +45,9 @@ [LibraryClasses.ARM]
>    ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
>    ArmCpuLib|ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
>  
> +[LibraryClasses.ARM.SEC]
> +  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7LibSec.inf
> +

How does this library resolution change relate to the stated purpose of
the patch?

Thanks
Laszlo

>  [LibraryClasses.common]
>    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>  
> @@ -77,6 +80,11 @@ [BuildOptions]
>    GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15 -I$(WORKSPACE)/ArmVirtPkg/Include
>    *_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/ArmVirtPkg/Include
>  
> +[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE]
> +  # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE
> +  # executable we build for the relocatable PrePi. They are not runtime
> +  # relocatable in ELF.
> +  *_CLANG35_*_CC_FLAGS = -mno-movt
>  
>  ################################################################################
>  #
> diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
> index 5ad1bf630bda..4ebead5ba6e6 100644
> --- a/ArmVirtPkg/ArmVirtXen.dsc
> +++ b/ArmVirtPkg/ArmVirtXen.dsc
> @@ -44,6 +44,9 @@ [LibraryClasses.ARM]
>    ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
>    ArmCpuLib|ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
>  
> +[LibraryClasses.ARM.SEC]
> +  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7LibSec.inf
> +
>  [LibraryClasses.common]
>    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>  
> @@ -77,6 +80,12 @@ [BuildOptions]
>    GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15 -I$(WORKSPACE)/ArmVirtPkg/Include
>    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/ArmVirtPkg/Include
>  
> +[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE]
> +  # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE
> +  # executable we build for the relocatable PrePi. They are not runtime
> +  # relocatable in ELF.
> +  *_CLANG35_*_CC_FLAGS = -mno-movt
> +
>  ################################################################################
>  #
>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> 

Re: [PATCH 2/2] ArmVirtPkg ARM: make relocatable PrePi users build with CLANG35

[Ard Biesheuvel]

On 3 August 2016 at 12:00, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/03/16 10:21, Ard Biesheuvel wrote:
>> The clang developers have made a backward incompatible change to the
>> command line arguments, and have replaced '-mllvm -arm-use-movt=0'
>> with '-mno-movt'. This does not matter for most ARM platforms, and
>> therefore it has been removed from the default CLANG35/ARM CC flags
>> in patch 1c63516075b3 ("BaseTools CLANG35: drop problematic use-movt
>> and save-temps options"), but as it turns out, the relocatable PrePi
>> implementation used by ArmVirtQemuKernel and ArmVirtXen will fail to
>> build if it contains MOVT/MOVW pairs, due to the fact that these are
>> not runtime relocatable under ELF.
>>
>> Since they are runtime relocatable under PE/COFF, and GenFw does the
>> right thing when encountering them, selectively controlling their
>> use is more appropriate than disabling them altogether. Therefore,
>> this patch adds the -mno-movt argument only for the platforms that
>> use the relocatable PrePi, and only for the module types that may
>> be pulled into its build.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++
>>  ArmVirtPkg/ArmVirtXen.dsc        | 9 +++++++++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> index 01a1d9b4613b..6c536d9bbd2d 100644
>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> @@ -45,6 +45,9 @@ [LibraryClasses.ARM]
>>    ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
>>    ArmCpuLib|ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
>>
>> +[LibraryClasses.ARM.SEC]
>> +  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7LibSec.inf
>> +
>
> How does this library resolution change relate to the stated purpose of
> the patch?
>

It prevents a version of ArmLib being pulled into the build of the
relocatable PrePi that was built using the BuildOptions for DXE_DRIVER
type modules.

-- 
Ard.

Re: [PATCH 2/2] ArmVirtPkg ARM: make relocatable PrePi users build with CLANG35

[Laszlo Ersek]

On 08/03/16 12:02, Ard Biesheuvel wrote:
> On 3 August 2016 at 12:00, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 08/03/16 10:21, Ard Biesheuvel wrote:
>>> The clang developers have made a backward incompatible change to the
>>> command line arguments, and have replaced '-mllvm -arm-use-movt=0'
>>> with '-mno-movt'. This does not matter for most ARM platforms, and
>>> therefore it has been removed from the default CLANG35/ARM CC flags
>>> in patch 1c63516075b3 ("BaseTools CLANG35: drop problematic use-movt
>>> and save-temps options"), but as it turns out, the relocatable PrePi
>>> implementation used by ArmVirtQemuKernel and ArmVirtXen will fail to
>>> build if it contains MOVT/MOVW pairs, due to the fact that these are
>>> not runtime relocatable under ELF.
>>>
>>> Since they are runtime relocatable under PE/COFF, and GenFw does the
>>> right thing when encountering them, selectively controlling their
>>> use is more appropriate than disabling them altogether. Therefore,
>>> this patch adds the -mno-movt argument only for the platforms that
>>> use the relocatable PrePi, and only for the module types that may
>>> be pulled into its build.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++
>>>  ArmVirtPkg/ArmVirtXen.dsc        | 9 +++++++++
>>>  2 files changed, 17 insertions(+)
>>>
>>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>> index 01a1d9b4613b..6c536d9bbd2d 100644
>>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>> @@ -45,6 +45,9 @@ [LibraryClasses.ARM]
>>>    ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
>>>    ArmCpuLib|ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
>>>
>>> +[LibraryClasses.ARM.SEC]
>>> +  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7LibSec.inf
>>> +
>>
>> How does this library resolution change relate to the stated purpose of
>> the patch?
>>
> 
> It prevents a version of ArmLib being pulled into the build of the
> relocatable PrePi that was built using the BuildOptions for DXE_DRIVER
> type modules.
> 

I think it would be reasonable to add this one paragraph to the commit
message as well. If you disagree, I won't insist. :)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH 1/2] EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library

[Laszlo Ersek]

On 08/03/16 12:00, Ard Biesheuvel wrote:
> On 3 August 2016 at 11:56, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 08/03/16 10:21, Ard Biesheuvel wrote:
>>> This library is only used by the various PrePi implementations, all of
>>> which are of type SEC.
>>
>> You can actually enforce that client module type restriction, by setting
>>
>>     LIBRARY_CLASS                  = MemoryAllocationLib|SEC
>>
>> Can you try that, in addition to the MODULE_TYPE change?
>>
>> Just an idea, of course.
>>
> 
> That is a valid point, but it is kind of orthogonal to the issue I am
> trying to solve.
> 
> In patch #2, I override the CC flags for SEC and BASE type modules,
> but this static library gets build with the PEIM rules in effect, so I
> don't really mind if anyone uses this module elsewhere. I could
> perhaps simply change the type to BASE as well.

Hm, after your explanation, I think your current patch is good.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH 1/2] EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library

[Ard Biesheuvel]

On 3 August 2016 at 13:21, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/03/16 12:00, Ard Biesheuvel wrote:
>> On 3 August 2016 at 11:56, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 08/03/16 10:21, Ard Biesheuvel wrote:
>>>> This library is only used by the various PrePi implementations, all of
>>>> which are of type SEC.
>>>
>>> You can actually enforce that client module type restriction, by setting
>>>
>>>     LIBRARY_CLASS                  = MemoryAllocationLib|SEC
>>>
>>> Can you try that, in addition to the MODULE_TYPE change?
>>>
>>> Just an idea, of course.
>>>
>>
>> That is a valid point, but it is kind of orthogonal to the issue I am
>> trying to solve.
>>
>> In patch #2, I override the CC flags for SEC and BASE type modules,
>> but this static library gets build with the PEIM rules in effect, so I
>> don't really mind if anyone uses this module elsewhere. I could
>> perhaps simply change the type to BASE as well.
>
> Hm, after your explanation, I think your current patch is good.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>

Thanks.

@Leif: any objections? I'd like to merge this right away, my Jenkins
job is broken atm due to this.

-- 
Ard.

Re: [PATCH 2/2] ArmVirtPkg ARM: make relocatable PrePi users build with CLANG35

[Ard Biesheuvel]

On 3 August 2016 at 13:19, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/03/16 12:02, Ard Biesheuvel wrote:
>> On 3 August 2016 at 12:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 08/03/16 10:21, Ard Biesheuvel wrote:
>>>> The clang developers have made a backward incompatible change to the
>>>> command line arguments, and have replaced '-mllvm -arm-use-movt=0'
>>>> with '-mno-movt'. This does not matter for most ARM platforms, and
>>>> therefore it has been removed from the default CLANG35/ARM CC flags
>>>> in patch 1c63516075b3 ("BaseTools CLANG35: drop problematic use-movt
>>>> and save-temps options"), but as it turns out, the relocatable PrePi
>>>> implementation used by ArmVirtQemuKernel and ArmVirtXen will fail to
>>>> build if it contains MOVT/MOVW pairs, due to the fact that these are
>>>> not runtime relocatable under ELF.
>>>>
>>>> Since they are runtime relocatable under PE/COFF, and GenFw does the
>>>> right thing when encountering them, selectively controlling their
>>>> use is more appropriate than disabling them altogether. Therefore,
>>>> this patch adds the -mno-movt argument only for the platforms that
>>>> use the relocatable PrePi, and only for the module types that may
>>>> be pulled into its build.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++
>>>>  ArmVirtPkg/ArmVirtXen.dsc        | 9 +++++++++
>>>>  2 files changed, 17 insertions(+)
>>>>
>>>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>>> index 01a1d9b4613b..6c536d9bbd2d 100644
>>>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>>> @@ -45,6 +45,9 @@ [LibraryClasses.ARM]
>>>>    ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
>>>>    ArmCpuLib|ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
>>>>
>>>> +[LibraryClasses.ARM.SEC]
>>>> +  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7LibSec.inf
>>>> +
>>>
>>> How does this library resolution change relate to the stated purpose of
>>> the patch?
>>>
>>
>> It prevents a version of ArmLib being pulled into the build of the
>> relocatable PrePi that was built using the BuildOptions for DXE_DRIVER
>> type modules.
>>
>
> I think it would be reasonable to add this one paragraph to the commit
> message as well. If you disagree, I won't insist. :)
>

No, you're quite right. I'm still a bit confused that a SEC module can
include DXE_DRIVER type libraries, but I guess it doesn't matter if
there is no constructor.

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thank you,
Ard.

Re: [PATCH 1/2] EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library

[Leif Lindholm]

On Wed, Aug 03, 2016 at 02:50:59PM +0200, Ard Biesheuvel wrote:
> On 3 August 2016 at 13:21, Laszlo Ersek <lersek@redhat.com> wrote:
> > On 08/03/16 12:00, Ard Biesheuvel wrote:
> >> On 3 August 2016 at 11:56, Laszlo Ersek <lersek@redhat.com> wrote:
> >>> On 08/03/16 10:21, Ard Biesheuvel wrote:
> >>>> This library is only used by the various PrePi implementations, all of
> >>>> which are of type SEC.
> >>>
> >>> You can actually enforce that client module type restriction, by setting
> >>>
> >>>     LIBRARY_CLASS                  = MemoryAllocationLib|SEC
> >>>
> >>> Can you try that, in addition to the MODULE_TYPE change?
> >>>
> >>> Just an idea, of course.
> >>>
> >>
> >> That is a valid point, but it is kind of orthogonal to the issue I am
> >> trying to solve.
> >>
> >> In patch #2, I override the CC flags for SEC and BASE type modules,
> >> but this static library gets build with the PEIM rules in effect, so I
> >> don't really mind if anyone uses this module elsewhere. I could
> >> perhaps simply change the type to BASE as well.
> >
> > Hm, after your explanation, I think your current patch is good.
> >
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks.
> 
> @Leif: any objections? I'd like to merge this right away, my Jenkins
> job is broken atm due to this.

No objection. We can always change it to BASE in future if that would
appear to make sense.

(for the series)
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Regards,

Leif

Re: [PATCH 0/2] ArmVirtPkg EmbeddedPkg: fix build for CLANG35/ARM

[Ard Biesheuvel]

On 3 August 2016 at 10:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Currently, the ArmVirtQemuKernel and ArmVirtXen platforms will not build
> for ARM when using CLANG35, due to the fact that the compiler emits
> MOVT/MOVW pairs into objects that are used by the relocatable PrePi, and
> such instruction pairs are not runtime relocatable in ELF (i.e., there are
> no dynamic relocation types to describe them)
>
> So fix this by selectively inhibiting the use of these pairs when building
> these platforms for ARM using CLANG35
>

Pushed as

f846969796d3 EmbeddedPkg: make PrePiMemoryAllocationLib a SEC type library
87ee6390cbeb ArmVirtPkg ARM: make relocatable PrePi users build with CLANG35

with Laszlo's suggested commit log change to #2

Thanks all