[PATCH 0/6] ArmPkg: eliminate calls to deprecated functions

[PATCH 0/6] ArmPkg: eliminate calls to deprecated functions

[Ard Biesheuvel]

This series prepares ArmPkg for building with the preprocess symbol
DISABLE_NEW_DEPRECATED_INTERFACES defined, by adding missing components
to ArmPkg (#1), fixing broken code or code that relies on deprecated
functionality (#2 - #5), and finally adds -DDISABLE_NEW_DEPRECATED_INTERFACES
to the CC flags for all build types, toolchains and architectures.

Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164

Ard Biesheuvel (6):
  ArmPkg: add missing components
  ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include
  ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
  ArmPkg/SemihostFs: eliminate calls to deprecated string functions
  ArmPkg/BdsLib: eliminate calls to deprecated string functions
  ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES

 ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c                |  2 +-
 ArmPkg/Application/LinuxLoader/LinuxLoader.c                  |  6 ++++--
 ArmPkg/ArmPkg.dsc                                             | 13 +++++++++++++
 ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm |  4 ----
 ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c                 | 20 ++++++++++++--------
 ArmPkg/Library/BdsLib/BdsFilePath.c                           |  8 +++++---
 6 files changed, 35 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH 1/6] ArmPkg: add missing components

[Ard Biesheuvel]

ArmPkg.dsc was a bit out of date, and some modules added over the past
years had not been added to its [Components] section yet.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/ArmPkg.dsc | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 6a8ff7e621d7..9bb1263a7f96 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -143,6 +143,16 @@ [Components.common]
 
   ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
 
+  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
+  ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
+  ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
+  ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
+  ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
+  ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
+  ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
+  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+
 [Components.ARM]
   ArmPkg/Drivers/ArmCpuLib/ArmCortexA8Lib/ArmCortexA8Lib.inf
   ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Lib.inf
@@ -151,3 +161,4 @@ [Components.ARM]
 [Components.AARCH64]
   ArmPkg/Drivers/ArmCpuLib/ArmCortexAEMv8Lib/ArmCortexAEMv8Lib.inf
   ArmPkg/Drivers/ArmCpuLib/ArmCortexA5xLib/ArmCortexA5xLib.inf
+  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
-- 
2.7.4

[PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include

[Ard Biesheuvel]

Drop the include of AsmMacroIoLib.h, which contains GCC preprocessor macros
that RVCT does not use or require, given it has its own AsmMacroIoLib.inc

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
index 1417c9a78204..882f25b780cd 100644
--- a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
+++ b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
@@ -11,10 +11,6 @@
 //
 //
 
-#include <AsmMacroIoLib.h>
-#include <Library/ArmCpuLib.h>
-#include <Chipset/ArmCortexA9.h>
-
   INCLUDE AsmMacroExport.inc
   INCLUDE AsmMacroIoLib.inc
 
-- 
2.7.4

[PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions

[Ard Biesheuvel]

Remove calls to deprecated string functions like AsciiStrCpy() and
UnicodeStrToAsciiStr()

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
 ArmPkg/Application/LinuxLoader/LinuxLoader.c   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
index fd7ee9c8624d..0b3e2489c758 100644
--- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
+++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
@@ -72,7 +72,7 @@ SetupCmdlineTag (
     mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
 
     /* place CommandLine into tag */
-    AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
+    AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
 
     // move pointer to next tag
     mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
index 70b960b66f0e..76697c3a8c9d 100644
--- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
+++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
@@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
   LIST_ENTRY                          *ResourceLink;
   SYSTEM_MEMORY_RESOURCE              *Resource;
   EFI_PHYSICAL_ADDRESS                SystemMemoryBase;
+  UINTN                               Length;
 
   Status = gBS->LocateProtocol (
                   &gEfiDevicePathFromTextProtocolGuid,
@@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
   }
 
   if (LinuxCommandLine != NULL) {
-    AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
+    Length = StrLen (LinuxCommandLine) + 1;
+    AsciiLinuxCommandLine = AllocatePool (Length);
     if (AsciiLinuxCommandLine == NULL) {
       Status = EFI_OUT_OF_RESOURCES;
       goto Error;
     }
-    UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
+    UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
   }
 
   //
-- 
2.7.4

[PATCH 4/6] ArmPkg/SemihostFs: eliminate calls to deprecated string functions

[Ard Biesheuvel]

Remove calls to deprecated string functions like AsciiStrCpy() and
UnicodeStrToAsciiStr()

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
index 6efdad9ebcce..e79b5cc5cf39 100644
--- a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
+++ b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
@@ -207,11 +207,12 @@ FileOpen (
     return EFI_WRITE_PROTECTED;
   }
 
-  AsciiFileName = AllocatePool (StrLen (FileName) + 1);
+  Length = StrLen (FileName) + 1;
+  AsciiFileName = AllocatePool (Length);
   if (AsciiFileName == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
-  UnicodeStrToAsciiStr (FileName, AsciiFileName);
+  UnicodeStrToAsciiStrS (FileName, AsciiFileName, Length);
 
   // Opening '/', '\', '.', or the NULL pathname is trying to open the root directory
   if ((AsciiStrCmp (AsciiFileName, "\\") == 0) ||
@@ -463,7 +464,7 @@ FileDelete (
     NameSize = AsciiStrLen (Fcb->FileName);
     FileName = AllocatePool (NameSize + 1);
 
-    AsciiStrCpy (FileName, Fcb->FileName);
+    AsciiStrCpyS (FileName, NameSize + 1, Fcb->FileName);
 
     // Close the file if it's open.  Disregard return status,
     // since it might give an error if the file isn't open.
@@ -828,8 +829,10 @@ GetFilesystemInfo (
   EFI_FILE_SYSTEM_INFO  *Info;
   EFI_STATUS            Status;
   UINTN                 ResultSize;
+  UINTN                 StringSize;
 
-  ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (mSemihostFsLabel);
+  StringSize = StrSize (mSemihostFsLabel);
+  ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StringSize;
 
   if (*BufferSize >= ResultSize) {
     ZeroMem (Buffer, ResultSize);
@@ -843,7 +846,7 @@ GetFilesystemInfo (
     Info->FreeSpace  = 0;
     Info->BlockSize  = 0;
 
-    StrCpy (Info->VolumeLabel, mSemihostFsLabel);
+    StrCpyS (Info->VolumeLabel, StringSize, mSemihostFsLabel);
   } else {
     Status = EFI_BUFFER_TOO_SMALL;
   }
@@ -903,7 +906,7 @@ FileGetInfo (
     ResultSize = StrSize (mSemihostFsLabel);
 
     if (*BufferSize >= ResultSize) {
-      StrCpy (Buffer, mSemihostFsLabel);
+      StrCpyS (Buffer, *BufferSize, mSemihostFsLabel);
       Status = EFI_SUCCESS;
     } else {
       Status = EFI_BUFFER_TOO_SMALL;
@@ -963,11 +966,12 @@ SetFileInfo (
     return EFI_ACCESS_DENIED;
   }
 
-  AsciiFileName = AllocatePool (StrLen (Info->FileName) + 1);
+  Length = StrLen (Info->FileName) + 1;
+  AsciiFileName = AllocatePool (Length);
   if (AsciiFileName == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
-  UnicodeStrToAsciiStr (Info->FileName, AsciiFileName);
+  UnicodeStrToAsciiStrS (Info->FileName, AsciiFileName, Length);
 
   FileSizeIsDifferent = (Info->FileSize != Fcb->Info.FileSize);
   FileNameIsDifferent = (AsciiStrCmp (AsciiFileName, Fcb->FileName) != 0);
-- 
2.7.4

[PATCH 5/6] ArmPkg/BdsLib: eliminate calls to deprecated string functions

[Ard Biesheuvel]

Remove calls to deprecated string functions like AsciiStrCpy() and
UnicodeStrToAsciiStr()

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/BdsLib/BdsFilePath.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c b/ArmPkg/Library/BdsLib/BdsFilePath.c
index aefeaed4ffb5..351a8bd8edf4 100644
--- a/ArmPkg/Library/BdsLib/BdsFilePath.c
+++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
@@ -964,7 +964,7 @@ Mtftp4CheckPacket (
       Step      = (Context->DownloadedNbOfBytes   * TFTP_PROGRESS_SLIDER_STEPS) / Context->FileSize;
       if (Step > LastStep) {
         Print (mTftpProgressDelete);
-        StrCpy (Progress, mTftpProgressFrame);
+        StrCpyS (Progress, sizeof (Progress), mTftpProgressFrame);
         for (Index = 1; Index < Step; Index++) {
           Progress[Index] = L'=';
         }
@@ -1044,6 +1044,7 @@ BdsTftpLoadImage (
   UINT64                   FileSize;
   UINT64                   TftpBufferSize;
   BDS_TFTP_CONTEXT         *TftpContext;
+  UINTN                    PathNameLen;
 
   ASSERT(IS_DEVICE_PATH_NODE (RemainingDevicePath, MESSAGING_DEVICE_PATH, MSG_IPv4_DP));
   IPv4DevicePathNode = (IPv4_DEVICE_PATH*)RemainingDevicePath;
@@ -1187,8 +1188,9 @@ BdsTftpLoadImage (
 
   // The Device Path might contain multiple FilePath nodes
   PathName      = ConvertDevicePathToText ((EFI_DEVICE_PATH_PROTOCOL*)(IPv4DevicePathNode + 1), FALSE, FALSE);
-  AsciiFilePath = AllocatePool (StrLen (PathName) + 1);
-  UnicodeStrToAsciiStr (PathName, AsciiFilePath);
+  PathNameLen   = StrLen (PathName) + 1;
+  AsciiFilePath = AllocatePool (PathNameLen);
+  UnicodeStrToAsciiStrS (PathName, AsciiFilePath, PathNameLen);
 
   //
   // Try to get the size of the file in bytes from the server. If it fails,
-- 
2.7.4

[PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES

[Ard Biesheuvel]

Define DISABLE_NEW_DEPRECATED_INTERFACES on the compiler command line by
default, to prevent deprecated interfaces from being used in core EDK2
code.

Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/ArmPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 9bb1263a7f96..cd44ec166703 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -3,6 +3,7 @@
 #
 # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.<BR>
 # Copyright (c) 2011 - 2015, ARM Ltd. All rights reserved.<BR>
+# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
 #
 #    This program and the accompanying materials
 #    are licensed and made available under the terms and conditions of the BSD License
@@ -36,6 +37,7 @@ [BuildOptions]
   RVCT:*_*_ARM_PLATFORM_FLAGS  == --cpu Cortex-A15
 
   RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
+  *_*_*_CC_FLAGS  = -DDISABLE_NEW_DEPRECATED_INTERFACES
 
 [LibraryClasses.common]
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
-- 
2.7.4

Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions

[Laszlo Ersek]

On 10/24/16 18:06, Ard Biesheuvel wrote:
> Remove calls to deprecated string functions like AsciiStrCpy() and
> UnicodeStrToAsciiStr()
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
>  ArmPkg/Application/LinuxLoader/LinuxLoader.c   | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
> index fd7ee9c8624d..0b3e2489c758 100644
> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
> @@ -72,7 +72,7 @@ SetupCmdlineTag (
>      mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>  
>      /* place CommandLine into tag */
> -    AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
> +    AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>  
>      // move pointer to next tag
>      mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);

Apparently nothing in this file checks if the tags being added still
actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
(ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
the full string").

> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
> index 70b960b66f0e..76697c3a8c9d 100644
> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
>    LIST_ENTRY                          *ResourceLink;
>    SYSTEM_MEMORY_RESOURCE              *Resource;
>    EFI_PHYSICAL_ADDRESS                SystemMemoryBase;
> +  UINTN                               Length;
>  
>    Status = gBS->LocateProtocol (
>                    &gEfiDevicePathFromTextProtocolGuid,
> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
>    }
>  
>    if (LinuxCommandLine != NULL) {
> -    AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
> +    Length = StrLen (LinuxCommandLine) + 1;
> +    AsciiLinuxCommandLine = AllocatePool (Length);
>      if (AsciiLinuxCommandLine == NULL) {
>        Status = EFI_OUT_OF_RESOURCES;
>        goto Error;
>      }
> -    UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
> +    UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
>    }
>  
>    //
> 

I prefer to call character counts without the terminating NUL "Length",
and character counts with the terminating NUL "Size", but that's just a
personal preference. (And the rest of this code uses Length differently
already, for example in "LineLength" itself, near the top.)

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

Thanks
Laszlo

Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions

[Ard Biesheuvel]

On 25 October 2016 at 11:53, Laszlo Ersek <lersek@redhat.com> wrote:
> On 10/24/16 18:06, Ard Biesheuvel wrote:
>> Remove calls to deprecated string functions like AsciiStrCpy() and
>> UnicodeStrToAsciiStr()
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
>>  ArmPkg/Application/LinuxLoader/LinuxLoader.c   | 6 ++++--
>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>> index fd7ee9c8624d..0b3e2489c758 100644
>> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>> @@ -72,7 +72,7 @@ SetupCmdlineTag (
>>      mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>>
>>      /* place CommandLine into tag */
>> -    AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
>> +    AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>>
>>      // move pointer to next tag
>>      mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
>
> Apparently nothing in this file checks if the tags being added still
> actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
> (ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
> the full string").
>

The LinuxLoader is an unmaintained piece of junk, and will be removed
as soon as we can. I did notice that none of these ATAG functions
check whether the allocation as a whole is not overrun, but fixing
/that/ goes way beyond what we're willing to do in terms of
maintenance on deprecated code.

>> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>> index 70b960b66f0e..76697c3a8c9d 100644
>> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
>>    LIST_ENTRY                          *ResourceLink;
>>    SYSTEM_MEMORY_RESOURCE              *Resource;
>>    EFI_PHYSICAL_ADDRESS                SystemMemoryBase;
>> +  UINTN                               Length;
>>
>>    Status = gBS->LocateProtocol (
>>                    &gEfiDevicePathFromTextProtocolGuid,
>> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
>>    }
>>
>>    if (LinuxCommandLine != NULL) {
>> -    AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
>> +    Length = StrLen (LinuxCommandLine) + 1;
>> +    AsciiLinuxCommandLine = AllocatePool (Length);
>>      if (AsciiLinuxCommandLine == NULL) {
>>        Status = EFI_OUT_OF_RESOURCES;
>>        goto Error;
>>      }
>> -    UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
>> +    UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
>>    }
>>
>>    //
>>
>
> I prefer to call character counts without the terminating NUL "Length",
> and character counts with the terminating NUL "Size", but that's just a
> personal preference. (And the rest of this code uses Length differently
> already, for example in "LineLength" itself, near the top.)
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>

Thanks,
Ard.

Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions

[Ryan Harkin]

On 25 October 2016 at 11:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 25 October 2016 at 11:53, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 10/24/16 18:06, Ard Biesheuvel wrote:
>>> Remove calls to deprecated string functions like AsciiStrCpy() and
>>> UnicodeStrToAsciiStr()
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
>>>  ArmPkg/Application/LinuxLoader/LinuxLoader.c   | 6 ++++--
>>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>> index fd7ee9c8624d..0b3e2489c758 100644
>>> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>> @@ -72,7 +72,7 @@ SetupCmdlineTag (
>>>      mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>>>
>>>      /* place CommandLine into tag */
>>> -    AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
>>> +    AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>>>
>>>      // move pointer to next tag
>>>      mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
>>
>> Apparently nothing in this file checks if the tags being added still
>> actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
>> (ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
>> the full string").
>>
>
> The LinuxLoader is an unmaintained piece of junk, and will be removed
> as soon as we can.

Who still uses it? Hikey?


> I did notice that none of these ATAG functions
> check whether the allocation as a whole is not overrun, but fixing
> /that/ goes way beyond what we're willing to do in terms of
> maintenance on deprecated code.
>
>>> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>> index 70b960b66f0e..76697c3a8c9d 100644
>>> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
>>>    LIST_ENTRY                          *ResourceLink;
>>>    SYSTEM_MEMORY_RESOURCE              *Resource;
>>>    EFI_PHYSICAL_ADDRESS                SystemMemoryBase;
>>> +  UINTN                               Length;
>>>
>>>    Status = gBS->LocateProtocol (
>>>                    &gEfiDevicePathFromTextProtocolGuid,
>>> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
>>>    }
>>>
>>>    if (LinuxCommandLine != NULL) {
>>> -    AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
>>> +    Length = StrLen (LinuxCommandLine) + 1;
>>> +    AsciiLinuxCommandLine = AllocatePool (Length);
>>>      if (AsciiLinuxCommandLine == NULL) {
>>>        Status = EFI_OUT_OF_RESOURCES;
>>>        goto Error;
>>>      }
>>> -    UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
>>> +    UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
>>>    }
>>>
>>>    //
>>>
>>
>> I prefer to call character counts without the terminating NUL "Length",
>> and character counts with the terminating NUL "Size", but that's just a
>> personal preference. (And the rest of this code uses Length differently
>> already, for example in "LineLength" itself, near the top.)
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>
> Thanks,
> Ard.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions

[Ard Biesheuvel]

On 25 October 2016 at 12:08, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 25 October 2016 at 11:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 25 October 2016 at 11:53, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 10/24/16 18:06, Ard Biesheuvel wrote:
>>>> Remove calls to deprecated string functions like AsciiStrCpy() and
>>>> UnicodeStrToAsciiStr()
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>>  ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
>>>>  ArmPkg/Application/LinuxLoader/LinuxLoader.c   | 6 ++++--
>>>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>>> index fd7ee9c8624d..0b3e2489c758 100644
>>>> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>>> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>>> @@ -72,7 +72,7 @@ SetupCmdlineTag (
>>>>      mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>>>>
>>>>      /* place CommandLine into tag */
>>>> -    AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
>>>> +    AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>>>>
>>>>      // move pointer to next tag
>>>>      mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
>>>
>>> Apparently nothing in this file checks if the tags being added still
>>> actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
>>> (ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
>>> the full string").
>>>
>>
>> The LinuxLoader is an unmaintained piece of junk, and will be removed
>> as soon as we can.
>
> Who still uses it? Hikey?
>

I certainly hope not. HiKey is AARCH64, and LinuxLoader is quite badly
broken on that architecture.

>
>> I did notice that none of these ATAG functions
>> check whether the allocation as a whole is not overrun, but fixing
>> /that/ goes way beyond what we're willing to do in terms of
>> maintenance on deprecated code.
>>
>>>> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>>> index 70b960b66f0e..76697c3a8c9d 100644
>>>> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>>> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>>> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
>>>>    LIST_ENTRY                          *ResourceLink;
>>>>    SYSTEM_MEMORY_RESOURCE              *Resource;
>>>>    EFI_PHYSICAL_ADDRESS                SystemMemoryBase;
>>>> +  UINTN                               Length;
>>>>
>>>>    Status = gBS->LocateProtocol (
>>>>                    &gEfiDevicePathFromTextProtocolGuid,
>>>> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
>>>>    }
>>>>
>>>>    if (LinuxCommandLine != NULL) {
>>>> -    AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
>>>> +    Length = StrLen (LinuxCommandLine) + 1;
>>>> +    AsciiLinuxCommandLine = AllocatePool (Length);
>>>>      if (AsciiLinuxCommandLine == NULL) {
>>>>        Status = EFI_OUT_OF_RESOURCES;
>>>>        goto Error;
>>>>      }
>>>> -    UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
>>>> +    UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
>>>>    }
>>>>
>>>>    //
>>>>
>>>
>>> I prefer to call character counts without the terminating NUL "Length",
>>> and character counts with the terminating NUL "Size", but that's just a
>>> personal preference. (And the rest of this code uses Length differently
>>> already, for example in "LineLength" itself, near the top.)
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>>
>>
>> Thanks,
>> Ard.
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 4/6] ArmPkg/SemihostFs: eliminate calls to deprecated string functions

[Laszlo Ersek]

On 10/24/16 18:06, Ard Biesheuvel wrote:
> Remove calls to deprecated string functions like AsciiStrCpy() and
> UnicodeStrToAsciiStr()
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
> index 6efdad9ebcce..e79b5cc5cf39 100644
> --- a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
> +++ b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
> @@ -207,11 +207,12 @@ FileOpen (
>      return EFI_WRITE_PROTECTED;
>    }
>  
> -  AsciiFileName = AllocatePool (StrLen (FileName) + 1);
> +  Length = StrLen (FileName) + 1;
> +  AsciiFileName = AllocatePool (Length);
>    if (AsciiFileName == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  UnicodeStrToAsciiStr (FileName, AsciiFileName);
> +  UnicodeStrToAsciiStrS (FileName, AsciiFileName, Length);
>  
>    // Opening '/', '\', '.', or the NULL pathname is trying to open the root directory
>    if ((AsciiStrCmp (AsciiFileName, "\\") == 0) ||

Sort of muddles the purpose of the preexistent Length variable, but it's
manageable I think.

> @@ -463,7 +464,7 @@ FileDelete (
>      NameSize = AsciiStrLen (Fcb->FileName);
>      FileName = AllocatePool (NameSize + 1);
>  
> -    AsciiStrCpy (FileName, Fcb->FileName);
> +    AsciiStrCpyS (FileName, NameSize + 1, Fcb->FileName);
>  
>      // Close the file if it's open.  Disregard return status,
>      // since it might give an error if the file isn't open.

Haha, this uses exactly the opposite meanings for Size and Length of
what I do :) Okay.

> @@ -828,8 +829,10 @@ GetFilesystemInfo (
>    EFI_FILE_SYSTEM_INFO  *Info;
>    EFI_STATUS            Status;
>    UINTN                 ResultSize;
> +  UINTN                 StringSize;
>  
> -  ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (mSemihostFsLabel);
> +  StringSize = StrSize (mSemihostFsLabel);
> +  ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StringSize;
>  
>    if (*BufferSize >= ResultSize) {
>      ZeroMem (Buffer, ResultSize);
> @@ -843,7 +846,7 @@ GetFilesystemInfo (
>      Info->FreeSpace  = 0;
>      Info->BlockSize  = 0;
>  
> -    StrCpy (Info->VolumeLabel, mSemihostFsLabel);
> +    StrCpyS (Info->VolumeLabel, StringSize, mSemihostFsLabel);
>    } else {
>      Status = EFI_BUFFER_TOO_SMALL;
>    }

This is wrong:

- StrSize() "Returns the size of a Null-terminated Unicode string in
bytes, including the Null terminator",

- but for StrCpyS(), "DestMax" is "The maximum number of Destination
Unicode char, including terminating null char."

I suggest to use CopyMem() (rather than divide).

> @@ -903,7 +906,7 @@ FileGetInfo (
>      ResultSize = StrSize (mSemihostFsLabel);
>  
>      if (*BufferSize >= ResultSize) {
> -      StrCpy (Buffer, mSemihostFsLabel);
> +      StrCpyS (Buffer, *BufferSize, mSemihostFsLabel);
>        Status = EFI_SUCCESS;
>      } else {
>        Status = EFI_BUFFER_TOO_SMALL;

Also wrong; please use CopyMem().

> @@ -963,11 +966,12 @@ SetFileInfo (
>      return EFI_ACCESS_DENIED;
>    }
>  
> -  AsciiFileName = AllocatePool (StrLen (Info->FileName) + 1);
> +  Length = StrLen (Info->FileName) + 1;
> +  AsciiFileName = AllocatePool (Length);
>    if (AsciiFileName == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  UnicodeStrToAsciiStr (Info->FileName, AsciiFileName);
> +  UnicodeStrToAsciiStrS (Info->FileName, AsciiFileName, Length);
>  
>    FileSizeIsDifferent = (Info->FileSize != Fcb->Info.FileSize);
>    FileNameIsDifferent = (AsciiStrCmp (AsciiFileName, Fcb->FileName) != 0);
> 

This hunk looks okay.

Thanks
Laszlo

Re: [PATCH 5/6] ArmPkg/BdsLib: eliminate calls to deprecated string functions

[Laszlo Ersek]

On 10/24/16 18:06, Ard Biesheuvel wrote:
> Remove calls to deprecated string functions like AsciiStrCpy() and
> UnicodeStrToAsciiStr()
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Library/BdsLib/BdsFilePath.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c b/ArmPkg/Library/BdsLib/BdsFilePath.c
> index aefeaed4ffb5..351a8bd8edf4 100644
> --- a/ArmPkg/Library/BdsLib/BdsFilePath.c
> +++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
> @@ -964,7 +964,7 @@ Mtftp4CheckPacket (
>        Step      = (Context->DownloadedNbOfBytes   * TFTP_PROGRESS_SLIDER_STEPS) / Context->FileSize;
>        if (Step > LastStep) {
>          Print (mTftpProgressDelete);
> -        StrCpy (Progress, mTftpProgressFrame);
> +        StrCpyS (Progress, sizeof (Progress), mTftpProgressFrame);
>          for (Index = 1; Index < Step; Index++) {
>            Progress[Index] = L'=';
>          }

Same problem; please use CopyMem(), or introduce an ARRAY_SIZE() macro,
and use that (which divides by the size of the element at offset zero).

> @@ -1044,6 +1044,7 @@ BdsTftpLoadImage (
>    UINT64                   FileSize;
>    UINT64                   TftpBufferSize;
>    BDS_TFTP_CONTEXT         *TftpContext;
> +  UINTN                    PathNameLen;
>  
>    ASSERT(IS_DEVICE_PATH_NODE (RemainingDevicePath, MESSAGING_DEVICE_PATH, MSG_IPv4_DP));
>    IPv4DevicePathNode = (IPv4_DEVICE_PATH*)RemainingDevicePath;
> @@ -1187,8 +1188,9 @@ BdsTftpLoadImage (
>  
>    // The Device Path might contain multiple FilePath nodes
>    PathName      = ConvertDevicePathToText ((EFI_DEVICE_PATH_PROTOCOL*)(IPv4DevicePathNode + 1), FALSE, FALSE);
> -  AsciiFilePath = AllocatePool (StrLen (PathName) + 1);
> -  UnicodeStrToAsciiStr (PathName, AsciiFilePath);
> +  PathNameLen   = StrLen (PathName) + 1;
> +  AsciiFilePath = AllocatePool (PathNameLen);
> +  UnicodeStrToAsciiStrS (PathName, AsciiFilePath, PathNameLen);
>  
>    //
>    // Try to get the size of the file in bytes from the server. If it fails,
> 

This hunk looks fine.

Thanks
Laszlo

Re: [PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES

[Laszlo Ersek]

On 10/24/16 18:06, Ard Biesheuvel wrote:
> Define DISABLE_NEW_DEPRECATED_INTERFACES on the compiler command line by
> default, to prevent deprecated interfaces from being used in core EDK2
> code.
> 
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/ArmPkg.dsc | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 9bb1263a7f96..cd44ec166703 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -3,6 +3,7 @@
>  #
>  # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.<BR>
>  # Copyright (c) 2011 - 2015, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
>  #
>  #    This program and the accompanying materials
>  #    are licensed and made available under the terms and conditions of the BSD License
> @@ -36,6 +37,7 @@ [BuildOptions]
>    RVCT:*_*_ARM_PLATFORM_FLAGS  == --cpu Cortex-A15
>  
>    RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
> +  *_*_*_CC_FLAGS  = -DDISABLE_NEW_DEPRECATED_INTERFACES
>  
>  [LibraryClasses.common]
>    BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> 

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

Re: [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions

[Ryan Harkin]

On 24 October 2016 at 17:06, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> This series prepares ArmPkg for building with the preprocess symbol
> DISABLE_NEW_DEPRECATED_INTERFACES defined, by adding missing components
> to ArmPkg (#1), fixing broken code or code that relies on deprecated
> functionality (#2 - #5), and finally adds -DDISABLE_NEW_DEPRECATED_INTERFACES
> to the CC flags for all build types, toolchains and architectures.
>
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
>
> Ard Biesheuvel (6):
>   ArmPkg: add missing components
>   ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include
>   ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
>   ArmPkg/SemihostFs: eliminate calls to deprecated string functions
>   ArmPkg/BdsLib: eliminate calls to deprecated string functions
>   ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
>

I tested the series on FVP AEMv8 & Foundation, TC2 and Juno R0/1/2:

Tested-by: Ryan Harkin <ryan.harkin@linaro.org>


>  ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c                |  2 +-
>  ArmPkg/Application/LinuxLoader/LinuxLoader.c                  |  6 ++++--
>  ArmPkg/ArmPkg.dsc                                             | 13 +++++++++++++
>  ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm |  4 ----
>  ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c                 | 20 ++++++++++++--------
>  ArmPkg/Library/BdsLib/BdsFilePath.c                           |  8 +++++---
>  6 files changed, 35 insertions(+), 18 deletions(-)
>
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] ArmPkg: add missing components

[Leif Lindholm]

On Mon, Oct 24, 2016 at 05:06:41PM +0100, Ard Biesheuvel wrote:
> ArmPkg.dsc was a bit out of date, and some modules added over the past
> years had not been added to its [Components] section yet.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/ArmPkg.dsc | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 6a8ff7e621d7..9bb1263a7f96 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -143,6 +143,16 @@ [Components.common]
>  
>    ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>  
> +  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
> +  ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +  ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> +  ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +  ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> +  ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
> +  ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
> +  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +
>  [Components.ARM]
>    ArmPkg/Drivers/ArmCpuLib/ArmCortexA8Lib/ArmCortexA8Lib.inf
>    ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Lib.inf
> @@ -151,3 +161,4 @@ [Components.ARM]
>  [Components.AARCH64]
>    ArmPkg/Drivers/ArmCpuLib/ArmCortexAEMv8Lib/ArmCortexAEMv8Lib.inf
>    ArmPkg/Drivers/ArmCpuLib/ArmCortexA5xLib/ArmCortexA5xLib.inf
> +  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> -- 
> 2.7.4
> 

Re: [PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include

[Leif Lindholm]

On Mon, Oct 24, 2016 at 05:06:42PM +0100, Ard Biesheuvel wrote:
> Drop the include of AsmMacroIoLib.h, which contains GCC preprocessor macros
> that RVCT does not use or require, given it has its own AsmMacroIoLib.inc
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Not verified, but looks sensible, so:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
...ish

> ---
>  ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
> index 1417c9a78204..882f25b780cd 100644
> --- a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
> +++ b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
> @@ -11,10 +11,6 @@
>  //
>  //
>  
> -#include <AsmMacroIoLib.h>
> -#include <Library/ArmCpuLib.h>
> -#include <Chipset/ArmCortexA9.h>
> -
>    INCLUDE AsmMacroExport.inc
>    INCLUDE AsmMacroIoLib.inc
>  
> -- 
> 2.7.4
>