Re: [edk2] Display Architecture and Bring Up in UEFI

[prabin ca]

Hi Laszlo,

Thanks for your support, let me look on this 

> On 23-Nov-2018, at 2:49 PM, Laszlo Ersek  wrote:
> 
>> On 11/23/18 07:27, prabin ca wrote:
>> Hi Team,
>> 
>> I’m new to UEFI and display interface in UEFI. I would like to have deep 
>> dive into how display is working in UEFI (display architecture) and how 
>> display is have been bring up (porting of display panel in a any platform in 
>> general ).
>> 
>> Please help me with sample codes and necessary documents. I would like to 
>> get knowledge about display bring up and display architecture in UEFI
> 
> The driver writers' guide and the UEFI spec have relevant chapters on
> this. I think it's best to start reading the former, at "23 Graphics
> Driver Design Guidelines"; that part will give you the pointers to the
> rest as well.
> 
> https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer%27s-Guide
> 
> 
> For a (hopefully educational) example, I refer you to
> OvmfPkg/VirtioGpuDxe. In the series that first added this driver to
> edk2, I managed to construct the driver in stages such that each stage
> would build and even function, at the level expected from that stage. In
> particular, commit a2a4fa66701d ("OvmfPkg/VirtioGpuDxe: introduce with
> Component Name 2 and Driver Binding", 2016-09-01) could prove helpful,
> as it adds the skeleton of the driver, mostly without VirtIo GPU specifics.
> 
> 
> In addition, you might want to look into the generic
> 
>  MdeModulePkg/Universal/Console/GraphicsOutputDxe
> 
> driver. A platform may be able to incorporate that driver without any
> changes, and control it by first producing the two HOBs in the PEI phase
> that the driver consumes:
> 
>  MdePkg/Include/Guid/GraphicsInfoHob.h
> 
> (... Interestingly, due to the fact that this header file is under
> MdePkg and not MdeModulePkg, I've just learned, from the related commit
> messages, that the PEI phase has standardized graphics support,
> described in the PI spec. From the following two commit messages:
> 
> - 697c6cf32693 ("MdePkg: Add PI 1.4 Graphics HOB and PPI header files",
> 2015-04-28)
> 
> - 2af538fbf667 ("MdeModulePkg: Add GraphicsOutputDxe driver.", 2016-10-12)
> 
> it appears that enabling graphics support in the PEI phase could be a
> *requirement* for using GraphicsOutputDxe in the DXE phase. That might
> or might not match your use case, so perhaps it will prevent you from
> using GraphicsOutputDxe. I'm not sure.)
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Newbie: Getting Ovmf built

[stephano]

Hello Peter,

Thanks for giving EDK2 a try!

We have a set of simple instructions for folks building on standard 
Linux distros. Please have a look at this page:


https://github.com/tianocore/tianocore.github.io/wiki/Common-instructions

Note: Be sure the TARGET_ARCH is set correctly. (E.g. TARGET_ARCH = x64)

Once you have built the BaseTools and MdeModulePkg without any errors, 
you can try building and running in OVMF:


https://github.com/tianocore/tianocore.github.io/wiki/How-to-build-OVMF

Tip: Add the -j option so that you can grep through the log easily for 
any errors (build -j /path/to/log/file.txt).


Hopefully those links help get you started. Let me know if you run into 
any other issues.


Cheers,
Stephano

Stephano Cetola
TianoCore Community Manager

On 11/23/2018 2:44 PM, Peter Wiehe wrote:

Hello, I'm a total newbie to Tianocore/EDK2/OVMF.
(My coding is at high school level I think, not university level. I
have some (small) experience writing in Assembler, C, C++. I wrote a
little bootloader, so I know something about filesystem in general and
ext2 and pre-kernel "environment".)

I use xubuntu 18.04 on an AMD 64bit PC.

I'm currently trying to
1.) build OVMF from source
2.) and then want to run it in/with Qemu.
3.) Later I would like to try to write a simple ext2 "driver". Can't
guarantee I will succeed, but let's see.

So far I have
1.) downloaded the whole edk2 zip/tar-ball
2.) have installed nasm and ASL (iasl)
3.) Run "EmulatorPkg/build.sh"
4.) Run "OvmfPkg/build.sh -a X64"
5.) Run "OvmfPkg/build.sh -a X64 qemu"

Then I get the error message "qemu-system-x86_64: -pflash
/home/peter/Schreibtisch/edk2-master/Build/OvmfX64/DEBUG_GCC5/QEMU/bios.bin:
Could not open 
'/home/peter/Schreibtisch/edk2-master/Build/OvmfX64/DEBUG_GCC5/QEMU/bios.bin':
No such file or directory"

So my first question is how to deal with this error.

Kind regards
Peter Wiehe
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Newbie: Getting Ovmf built

[Peter Wiehe]

Hello, I'm a total newbie to Tianocore/EDK2/OVMF.
(My coding is at high school level I think, not university level. I
have some (small) experience writing in Assembler, C, C++. I wrote a
little bootloader, so I know something about filesystem in general and
ext2 and pre-kernel "environment".)

I use xubuntu 18.04 on an AMD 64bit PC.

I'm currently trying to
1.) build OVMF from source
2.) and then want to run it in/with Qemu.
3.) Later I would like to try to write a simple ext2 "driver". Can't
guarantee I will succeed, but let's see.

So far I have
1.) downloaded the whole edk2 zip/tar-ball
2.) have installed nasm and ASL (iasl)
3.) Run "EmulatorPkg/build.sh"
4.) Run "OvmfPkg/build.sh -a X64"
5.) Run "OvmfPkg/build.sh -a X64 qemu"

Then I get the error message "qemu-system-x86_64: -pflash
/home/peter/Schreibtisch/edk2-master/Build/OvmfX64/DEBUG_GCC5/QEMU/bios.bin:
Could not open 
'/home/peter/Schreibtisch/edk2-master/Build/OvmfX64/DEBUG_GCC5/QEMU/bios.bin':
No such file or directory"

So my first question is how to deal with this error.

Kind regards
Peter Wiehe
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmPkg: remove now unused BsdLib.h

[Ard Biesheuvel]

The last remaining users of the BdsLib.h header reside in the
edk2-platforms tree, and so it has been copied there. This
allows us to remove the original from ArmPkg.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Include/Library/BdsLib.h | 212 
 1 file changed, 212 deletions(-)

diff --git a/ArmPkg/Include/Library/BdsLib.h b/ArmPkg/Include/Library/BdsLib.h
deleted file mode 100644
index 4528c2e8739b..
--- a/ArmPkg/Include/Library/BdsLib.h
+++ /dev/null
@@ -1,212 +0,0 @@
-/** @file
-*
-*  Copyright (c) 2013-2015, ARM Limited. All rights reserved.
-*
-*  This program and the accompanying materials
-*  are licensed and made available under the terms and conditions of the BSD 
License
-*  which accompanies this distribution.  The full text of the license may be 
found at
-*  http://opensource.org/licenses/bsd-license.php
-*
-*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
-*
-**/
-
-#ifndef __BDS_ENTRY_H__
-#define __BDS_ENTRY_H__
-
-#define IS_DEVICE_PATH_NODE(node,type,subtype)\
-(((node)->Type == (type)) && ((node)->SubType == (subtype)))
-
-/**
-  This is defined by the UEFI specs, don't change it
-**/
-typedef struct {
-  UINT16  LoadOptionIndex;
-  EFI_LOAD_OPTION *LoadOption;
-  UINTN   LoadOptionSize;
-
-  UINT32  Attributes;
-  UINT16  FilePathListLength;
-  CHAR16  *Description;
-  EFI_DEVICE_PATH_PROTOCOL*FilePathList;
-
-  VOID*   OptionalData;
-  UINTN   OptionalDataSize;
-} BDS_LOAD_OPTION;
-
-/**
-  Connect a Device Path and return the handle of the driver that support this 
DevicePath
-
-  @param  DevicePathDevice Path of the File to connect
-  @param  HandleHandle of the driver that support this 
DevicePath
-  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match 
the driver DevicePath
-
-  @retval EFI_SUCCESS   A driver that matches the Device Path has been 
found
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
-
-**/
-EFI_STATUS
-BdsConnectDevicePath (
-  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
-  OUT EFI_HANDLE*Handle,
-  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
-  );
-
-/**
-  Connect all DXE drivers
-
-  @retval EFI_SUCCESS   All drivers have been connected
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the 
matching results.
-
-**/
-EFI_STATUS
-BdsConnectAllDrivers (
-  VOID
-  );
-
-/**
-  Return the value of a global variable defined by its VariableName.
-  The variable must be defined with the VendorGuid gEfiGlobalVariableGuid.
-
-  @param  VariableName  A Null-terminated string that is the name of 
the vendor's
-variable.
-  @param  DefaultValue  Value returned by the function if the variable 
does not exist
-  @param  DataSize  On input, the size in bytes of the return Data 
buffer.
-On output the size of data returned in Data.
-  @param  Value Value read from the UEFI Variable or copy of 
the default value
-if the UEFI Variable does not exist
-
-  @retval EFI_SUCCESS   All drivers have been connected
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the 
matching results.
-
-**/
-EFI_STATUS
-GetGlobalEnvironmentVariable (
-  IN CONST CHAR16*   VariableName,
-  IN VOID*   DefaultValue,
-  IN OUT UINTN*  Size,
-  OUTVOID**  Value
-  );
-
-/**
-  Return the value of the variable defined by its VariableName and VendorGuid
-
-  @param  VariableName  A Null-terminated string that is the name of 
the vendor's
-variable.
-  @param  VendorGuidA unique identifier for the vendor.
-  @param  DefaultValue  Value returned by the function if the variable 
does not exist
-  @param  DataSize  On input, the size in bytes of the return Data 
buffer.
-On output the size of data returned in Data.
-  @param  Value Value read from the UEFI Variable or copy of 
the default value
-if the UEFI Variable does not exist
-
-  @retval EFI_SUCCESS   All drivers have been connected
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the 
matching results.
-
-**/
-EFI_STATUS

[edk2] [PATCH] Platform/Comcast: drop bogus BdsLib.h include

[Ard Biesheuvel]

RdkBootManagerLib/RdkBootManagerLib.h includes BdsLib.h from ArmPkg
but does not actually depend on it. So drop the #include.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/Comcast/Library/RdkBootManagerLib/RdkBootManagerLib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Platform/Comcast/Library/RdkBootManagerLib/RdkBootManagerLib.h 
b/Platform/Comcast/Library/RdkBootManagerLib/RdkBootManagerLib.h
index 9d6e7dd1b677..6a18e95c1ced 100644
--- a/Platform/Comcast/Library/RdkBootManagerLib/RdkBootManagerLib.h
+++ b/Platform/Comcast/Library/RdkBootManagerLib/RdkBootManagerLib.h
@@ -13,7 +13,6 @@
 #ifndef __RDK_BOOT_MANAGER_LIB_H__
 #define __RDK_BOOT_MANAGER_LIB_H__
 
-#include 
 #include 
 #include 
 #include 
-- 
2.19.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/6] Add DSC/FDF include segment files for network stack

[Laszlo Ersek]

On 11/23/18 17:02, Gao, Liming wrote:
> Laszlo:
>   I add my comments. 
> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Thursday, November 22, 2018 11:48 PM
>> To: Gao, Liming ; Fu, Siyuan ; 
>> edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu ; Ye, Ting ; Justen, 
>> Jordan L ; Wu, Hao A
>> ; Wu, Jiaxin ; Anthony Perard 
>> ; Wei, David
>> 
>> Subject: Re: [edk2] [PATCH v2 0/6] Add DSC/FDF include segment files for 
>> network stack
>>
>> On 11/22/18 07:14, Gao, Liming wrote:
>>> Siyuan:
>>>   Thanks for your contribution. I really like this idea to share the
>>>   common DSC/FDF between all platform DSC/FDFs.  Now, FixedAtBuild PCD
>>>   can also be used in the conditional statement. Its value can be
>>>   specified by build command with --pcd option. So, I suggest to use
>>>   FixedAtBuild PCD for network feature instead of MACRO. I would like
>>>   to recommend to use PCD both for the build and firmware
>>>   configuration. For this case, one UINT16 FixedAtBuildPcd can be
>>>   introduced in NetworkPkg.dec.  Its different bit will be for the
>>>   different network features. Below is the example.
>>
>> I disagree.
>>
>> A bitmask is a very good representation for *computers*, to handle a set
>> of features. On the other hand, a bitmask is a catastrophically bad
>> representation for *humans*, when they are trying to configure a
>> platform build (that is, writing a build command line), selecting the
>> firmware features they want. It is hard to compute, and it is extremely
>> hard to grep for.
>>
>> It is trivial to grep a build script for "NETWORK_TLS_ENABLE". It is
>> much harder to grep the same script for PcdNetworkFeatureMask, and then
>> check whether bit#4 is set in the value.
> If BitMask is not good for this case, BOOLEAN type FixedAtBuildPcd can be 
> defined to match current macro one by one. 
> For example, gEfiNetworkPkgTokenSpaceGuid.PcdNetworkEnable is added to map 
> macro NETWORK_ENABLE.

I agree that this is technically doable, and that the results (regarding
the build script syntax) are very similar.

But, how is this an improvement over the current -D flags?

> 
>>
>>> Besides, I think *.dsc.inc files need to include section header. If
>>> so, *.dsc.inc is the standalone dsc file. It can easily be included in
>>> platform DSC file.
>>
>> I disagree. For a platform, spelling out the section header is not a big
>> burden, but it provides a lot of flexibility. For example, it can
>> restrict the scope of the included text (e.g., component list) to a
>> specific architecture only. For another example, the platform's section
>> header can specify the flavor of the PCDs for which the included text
>> provides the default values (fixed PCD, patchable PCD, even dynamic(ex)
>> PCD).
>>
>> Standalone DSC file is the wrong thing to aim for, in this instance --
>> in my opinion anyway. Yes, it appears simpler, but it loses too much
>> flexibility. In my earlier comments I pointed out that the "allow
>> plaintext HTTP connections" PCD was declared in NetworkPkg.dec as either
>> fixed or patchable. The dsc.inc file should not squander that
>> flexibility and dictate fixed only.
> This is not TRUE. Dynamic PCD has the different value format, such as 
> DynamicHii PCD.

I think DynamicHII is the only exception here, the rest can be described
with

  TokenSpaceGuid.PcdName|Value

This format should work in [PcdsDynamicDefault], [PcdsFixedAtBuild], and
[PcdsPatchableInModule] sections too. So platforms should have the
freedom to choose.

In the other case, if the DSC include file contains the section header
as well, then the platform loses this choice. The concrete example was
that NetworkPkg.dec declares "PcdAllowHttpConnections" as both
[PcdsFixedAtBuild] and [PcdsPatchableInModule], but in v1, the DSC
include file restricted it to [PcdsFixedAtBuild] only. IMO, if the DEC
file permits several types, then the DSC include file should restrict
those types as little as possible.

> Without section header, DynamicHii PCD can't be specified together with 
> FixedAtBuild Pcd.

Indeed, but it's still better to lose only DynamicHii (when using a DSC
include file) than to lose every type permitted by the DEC file, except
the one type picked by the DSC include file.

> Package level *.pcd.inc provides the recommended type and setting. 
> Platform can override Pcd value and type In their DSC file. BaseTools 
> supports PCD value override, 
> doesn't support PCD type override. This can be supported. 

Then I don't think that introducing a separate -D flag (or a separate
fixed-at-build boolean PCD) for controlling the default value of
"PcdAllowHttpConnections" buys us much.

>>> Especially for library instance, the different library instance may be
>>> for the different module type. Without section header, they can be
>>> placed into one *Libs.dsc.inc file.
>>
>> Ugh... I'm now condfused. Now it seems that you and I are asking for the
>> same thing actually. Did you perhaps 

Re: [edk2] [PATCH v3 6/6] ArmPkg/ArmMmuLib: Add MMU library inf file suitable for use in S-EL0.

[Ard Biesheuvel]

On Thu, 25 Oct 2018 at 09:33, Sughosh Ganu  wrote:
>
> From: Achin Gupta 
>
> This patch adds the definitions, sources, packages and library classes
> needed to compile and link MMU Library suitable for use in S-EL0.
>
> Currently, this is used only during the Standalone MM Core
> initialization and hence defined as MM_CORE_STANDALONE Module.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sughosh Ganu 
> ---
>  ArmPkg/Library/ArmMmuLib/{ArmMmuPeiLib.inf => ArmMmuStandaloneMmCoreLib.inf} 
> | 23 +---

The code in the previous patch looks fine, but I'd prefer it if we
expose this as a separate library class, not ArmMmuLib

So please just add a new file, say,
ArmPkg/include/Library/StandaloneMmMmuLib.h, add only the functions
you need, and add it to the LibraryClasses section of ArmPkg.dec as a
new library class.

Then, you can modify the previous patch to include an updated .inf
that describes it as being an implementation of StandaloneMmMmuLib.
You can then also drop the unimplemented ArmConfigureMmu etc.

>  1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf 
> b/ArmPkg/Library/ArmMmuLib/ArmMmuStandaloneMmCoreLib.inf
> similarity index 51%
> copy from ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> copy to ArmPkg/Library/ArmMmuLib/ArmMmuStandaloneMmCoreLib.inf
> index ecf13f790734..9f5593d3f6c8 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuStandaloneMmCoreLib.inf
> @@ -1,6 +1,6 @@
>  #/** @file
>  #
> -#  Copyright (c) 2016 Linaro Ltd. All rights reserved.
> +#  Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -13,22 +13,20 @@
>  #**/
>
>  [Defines]
> -  INF_VERSION= 0x00010005
> -  BASE_NAME  = ArmMmuPeiLib
> -  FILE_GUID  = b50d8d53-1ad1-44ea-9e69-8c89d4a6d08b
> -  MODULE_TYPE= PEIM
> +  INF_VERSION= 0x0001001A
> +  BASE_NAME  = ArmMmuStandaloneMmCoreLib
> +  FILE_GUID  = da8f0232-fb14-42f0-922c-63104d2c70bd
> +  MODULE_TYPE= MM_CORE_STANDALONE
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = ArmMmuLib|PEIM
> -  CONSTRUCTOR= ArmMmuPeiLibConstructor
> +  LIBRARY_CLASS  = 
> ArmMmuStandaloneMmCoreLib|MM_CORE_STANDALONE
> +  PI_SPECIFICATION_VERSION   = 0x00010032
> +  CONSTRUCTOR= ArmMmuStandaloneMmCoreLibConstructor
>
>  [Sources.AARCH64]
> -  AArch64/ArmMmuLibCore.c
> -  AArch64/ArmMmuPeiLibConstructor.c
> -  AArch64/ArmMmuLibReplaceEntry.S
> +  AArch64/ArmMmuStandaloneMmCoreLib.c
>
>  [Packages]
>ArmPkg/ArmPkg.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
>MdePkg/MdePkg.dec
>
>  [LibraryClasses]
> @@ -36,5 +34,4 @@ [LibraryClasses]
>CacheMaintenanceLib
>MemoryAllocationLib
>
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> +
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 2/6] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

[Ard Biesheuvel]

Hello all,

Apologies for the delay. Some questions below:

On Thu, 25 Oct 2018 at 09:33, Sughosh Ganu  wrote:
>
> From: Achin Gupta 
>
> PI v1.5 Specification Volume 4 defines Management Mode Core Interface
> and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
> means of communicating between drivers outside of MM and MMI
> handlers inside of MM.
>
> This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE runtime
> driver for AARCH64 platforms. It uses SMCs allocated from the standard
> SMC range defined in DEN0060A_ARM_MM_Interface_Specification.pdf
> to communicate with the standalone MM environment in the secure world.
>
> This patch also adds the MM Communication driver (.inf) file to
> define entry point for this driver and other compile
> related information the driver needs.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sughosh Ganu 
> ---
>  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  56 +++
>  ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h |  28 ++
>  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 395 
> 
>  3 files changed, 479 insertions(+)
>
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf 
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> new file mode 100644
> index ..88beafa39c05
> --- /dev/null
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> @@ -0,0 +1,56 @@
> +#/** @file
> +#
> +#  DXE MM Communicate driver
> +#
> +#  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution.  The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION= 0x0001001A
> +  BASE_NAME  = ArmMmCommunication
> +  FILE_GUID  = 09EE81D3-F15E-43F4-85B4-CB9873DA5D6B
> +  MODULE_TYPE= DXE_RUNTIME_DRIVER
> +  VERSION_STRING = 1.0
> +  ENTRY_POINT= MmCommunicationInitialize
> +
> +#
> +# The following is for reference only and not required by
> +# build tools
> +#
> +# VALID_ARCHITECTURES= AARCH64
> +#
> +
> +[Sources.AARCH64]
> +  MmCommunication.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  ArmSmcLib
> +  BaseMemoryLib
> +  DebugLib
> +  DxeServicesTableLib
> +  HobLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gEfiMmCommunicationProtocolGuid  ## PRODUCES
> +
> +[Pcd.common]
> +  gArmTokenSpaceGuid.PcdMmBufferBase
> +  gArmTokenSpaceGuid.PcdMmBufferSize
> +
> +[Depex]
> +  gEfiCpuArchProtocolGuid
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h 
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
> new file mode 100644
> index ..0bf1c8d4ca0e
> --- /dev/null
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
> @@ -0,0 +1,28 @@
> +/** @file
> +
> +  Copyright (c) 2016-2018, ARM Limited. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#if !defined _MM_COMMUNICATE_H_
> +#define _MM_COMMUNICATE_H_
> +
> +#define MM_MAJOR_VER_MASK0xEFFF
> +#define MM_MINOR_VER_MASK0x
> +#define MM_MAJOR_VER_SHIFT   16
> +
> +#define MM_MAJOR_VER(x) (((x) & MM_MAJOR_VER_MASK) >> MM_MAJOR_VER_SHIFT)
> +#define MM_MINOR_VER(x) ((x) & MM_MINOR_VER_MASK)
> +
> +#define MM_CALLER_MAJOR_VER  0x1UL
> +#define MM_CALLER_MINOR_VER  0x0
> +
> +#endif /* _MM_COMMUNICATE_H_ */
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> new file mode 100644
> index ..487db00c2a87
> --- /dev/null
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -0,0 +1,395 @@
> +/** @file
> +
> +  Copyright (c) 2016-2018, ARM Limited. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED 

Re: [edk2] [PATCH] Platform/ARM/Juno: increase max variable size to 8KB

[Ard Biesheuvel]

On Wed, 21 Nov 2018 at 17:37, Thomas Abraham  wrote:
>
> On Wed, Nov 21, 2018 at 9:26 PM Vijayenthiran Subramaniam
>  wrote:
> >
> > Commit dc37ca75 ("Edk2Platforms: Replace MdeModulePkg PXE/iSCSI/TCP with
> > NetworkPkg drivers") switched to using iSCSI driver from the NetworkPkg
> > package. This driver requires the platform to support a maximum variable
> > size of atleast 4KB.
> >
> > So increase the maximum supported variable size to 8KB on the Juno
> > platform. Without this, the iSCSI driver fails to load.
> >
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Cc: Thomas Abraham 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Vijayenthiran Subramaniam 
> > ---
> >  Platform/ARM/JunoPkg/ArmJuno.dsc | 2 ++
> >  1 file changed, 2 insertions(+)
>
> Tested-by: Thomas Abraham 
>

Pushed as d9e68a756cfb..397bbafdbff3

Thanks all


> >
> > diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc 
> > b/Platform/ARM/JunoPkg/ArmJuno.dsc
> > index ac3d63b..ac85dc0 100644
> > --- a/Platform/ARM/JunoPkg/ArmJuno.dsc
> > +++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
> > @@ -103,6 +103,8 @@
> >gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0BFE
> >gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x0001
> >
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> > +
> ># System Memory (2GB - 16MB of Trusted DRAM at the top of the 32bit 
> > address space)
> >gArmTokenSpaceGuid.PcdSystemMemoryBase|0x8000
> >
> > --
> > 2.7.4
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] Vlv2TbltDevicePkg: Add build instructions for Minnowboard Max.

[Kinney, Michael D]

Hi David,

These are not the correct instructions that take
advantage of the PACKAGES_PATH feature. The
directories with the binaries should not be
copied into the edk2 repo area.

Instead, WORKSPACE is set to the directory above
edk2, and the binaries are placed in a directory
that is a peer to edk2.

Please see Readme.me in the QuarkPlatformPkg for
an example of how to set up multiple paths with
packages.

https://github.com/tianocore/edk2/tree/master/QuarkPlatformPkg

It uses edk2 repo and edk2-non-osi repo with a binary
in the edk2-non-osi repo.

Please try the equivalent configuration for Vlv2 and
update the instructions to match.

Thanks,

Mike

> -Original Message-
> From: Wei, David
> Sent: Thursday, November 22, 2018 10:00 PM
> To: edk2-devel@lists.01.org
> Cc: Sun, Zailiang ; Qian, Yi
> ; Kinney, Michael D
> ; Wei, David
> 
> Subject: [Patch] Vlv2TbltDevicePkg: Add build
> instructions for Minnowboard Max.
> 
> Add build instructions for Minnowboard Max platform on
> edk2 master. Change stitching script to follow this new
> build instructions.
> 
> Test: Boot to Windows 10.
> 
> Cc: Zailiang Sun 
> Cc: Yi Qian 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: David Wei 
> ---
>  Vlv2TbltDevicePkg/Documents/BuildInstructions.txt | 128
> ++
>  Vlv2TbltDevicePkg/Stitch/IFWIStitch.bat   |   2
> +-
>  2 files changed, 129 insertions(+), 1 deletion(-)
>  create mode 100644
> Vlv2TbltDevicePkg/Documents/BuildInstructions.txt
> 
> diff --git
> a/Vlv2TbltDevicePkg/Documents/BuildInstructions.txt
> b/Vlv2TbltDevicePkg/Documents/BuildInstructions.txt
> new file mode 100644
> index 00..a03c6ce3c4
> --- /dev/null
> +++ b/Vlv2TbltDevicePkg/Documents/BuildInstructions.txt
> @@ -0,0 +1,128 @@
> +## @file
> +# Build Instructions for Minnowboard Max platform on
> edk2 master.
> +#
> +# Copyright (c) 2018, Intel Corporation. All rights
> reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and
> conditions of the BSD License
> +#  which accompanies this distribution. The full text of
> the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +##
> +
> +
> 
> +  HOW TO CREATE A FULL SOURCE
> TREE
> +
> 
> +1) Create a new folder (directory) on the root of your
> local hard drive (development
> +   machine) for use as your work space (this example
> uses "C:\MyWorkspace").
> +   Some paths are very long and placing the working
> directory too deep in the directory
> +   structure may cause the path to be longer than
> Windows maximum path length.
> +
> +2) Checkout edk2 from GitHub with the following command.
> +i)  Run "git clone
> https://github.com/tianocore/edk2.git;
> +* This step will created a new folder
> C:\MyWorkspace\edk2, which holds edk2 source code
> downloaded.
> +
> +5) Download MinnowBoard MAX Binary Object Modules from :
> (To be updated)
> +
> https://firmware.intel.com/sites/default/files/MinnowBoar
> d_MAX-0.97-Binary.Objects.zip
> +
> +   The MinnowBoard MAX Binary Object Modules contain
> three additional
> +   folders required for the full source tree.
> +
> +IA32FamilyCpuPkg
> +Vlv2BinaryPkg
> +Vlv2MiscBinariesPkg
> +
> +   Copy above 3 packages into "C:\MyWorkspace\edk2".
> +
> +
> +6) Follow the instructions found in the file "OpenSSL-
> HOWTO.txt" located in your
> +   workspace (e.g.
> "C:\MyWorkspace\edk2\CryptoPkg\Library\OpensslLib\OpenSSL
> -HOWTO.txt")
> +   to install the Openssl source code.
> +
> +
> 
> +  HOW TO BUILD (WINDOWS
> ENVIRONMENT)
> +
> 
> +Windows System Configuration:
> +  Microsoft Windows 10 Enterprise 64-bit*
> +
> +   Note: Below steps could also apply to Microsoft
> Windows 8.1 and other version of Windows 10,
> + but Intel only verify these steps on Microsoft
> Windows 10 Enterprise 64-bit*.
> +
> +1. Setup Build Environment
> +   1) Install C compiler Microsoft Visual Studio 2015
> with Update 3 in the build machine
> +
> +  Note: Visual Studio 2008, 2010, 2012 and 2013 are
> also supported, but Intel only verify
> +with VS2015.
> +
> +   2) Obtain and install Python 2.7.10 from:
> https://www.python.org/downloads/release/python-2710/
> +  Make sure that a file with the extension of ".py"
> will be opened by Python.exe.
> +
> +2. Extract Source Code
> +   1) Follow the instructions of "HOW TO CREATE 

Re: [edk2] [PATCH v2 3/6] ArmVirtPkg: Update DSC/FDF to use NetworkPkg's include fragment file.

[Laszlo Ersek]

sorry, one more comment for this patch :)

On 11/22/18 06:21, Fu Siyuan wrote:
> This patch updates the platform DSC/FDF files to use the include fragment
> files provided by NetworkPkg.
> The feature enabling flags in [Defines] section have been updated to use
> the NetworkPkg's terms, and the value has been overridden with the original
> default value on this platform.
> 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan 
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc   | 11 +
>  ArmVirtPkg/ArmVirtQemu.dsc   | 46 ++--
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 28 +---
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 46 ++--
>  4 files changed, 28 insertions(+), 103 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 70a0ac4d786c..d7de73a88eff 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -76,16 +76,7 @@ [LibraryClasses.common]
>BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
>  
># Networking Requirements
> -  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> -  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> -  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> -  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> -!endif
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> -!endif
> +!include NetworkPkg/NetworkLibs.dsc.inc
>  
>#
># It is not possible to prevent the ARM compiler from inserting calls to 
> intrinsic functions.

(3) In case you agree that we should introduce a new DSC include file
for UefiShellNetwork1CommandsLib and UefiShellNetwork2CommandsLib, then
"ArmVirt.dsc.inc" should be adapted to that as well.

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/6] Add DSC/FDF include segment files for network stack

[Gao, Liming]

Laszlo:
  I add my comments. 

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, November 22, 2018 11:48 PM
> To: Gao, Liming ; Fu, Siyuan ; 
> edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Ye, Ting ; Justen, 
> Jordan L ; Wu, Hao A
> ; Wu, Jiaxin ; Anthony Perard 
> ; Wei, David
> 
> Subject: Re: [edk2] [PATCH v2 0/6] Add DSC/FDF include segment files for 
> network stack
> 
> On 11/22/18 07:14, Gao, Liming wrote:
> > Siyuan:
> >   Thanks for your contribution. I really like this idea to share the
> >   common DSC/FDF between all platform DSC/FDFs.  Now, FixedAtBuild PCD
> >   can also be used in the conditional statement. Its value can be
> >   specified by build command with --pcd option. So, I suggest to use
> >   FixedAtBuild PCD for network feature instead of MACRO. I would like
> >   to recommend to use PCD both for the build and firmware
> >   configuration. For this case, one UINT16 FixedAtBuildPcd can be
> >   introduced in NetworkPkg.dec.  Its different bit will be for the
> >   different network features. Below is the example.
> 
> I disagree.
> 
> A bitmask is a very good representation for *computers*, to handle a set
> of features. On the other hand, a bitmask is a catastrophically bad
> representation for *humans*, when they are trying to configure a
> platform build (that is, writing a build command line), selecting the
> firmware features they want. It is hard to compute, and it is extremely
> hard to grep for.
> 
> It is trivial to grep a build script for "NETWORK_TLS_ENABLE". It is
> much harder to grep the same script for PcdNetworkFeatureMask, and then
> check whether bit#4 is set in the value.
If BitMask is not good for this case, BOOLEAN type FixedAtBuildPcd can be 
defined to match current macro one by one. 
For example, gEfiNetworkPkgTokenSpaceGuid.PcdNetworkEnable is added to map 
macro NETWORK_ENABLE.

> 
> > Besides, I think *.dsc.inc files need to include section header. If
> > so, *.dsc.inc is the standalone dsc file. It can easily be included in
> > platform DSC file.
> 
> I disagree. For a platform, spelling out the section header is not a big
> burden, but it provides a lot of flexibility. For example, it can
> restrict the scope of the included text (e.g., component list) to a
> specific architecture only. For another example, the platform's section
> header can specify the flavor of the PCDs for which the included text
> provides the default values (fixed PCD, patchable PCD, even dynamic(ex)
> PCD).
> 
> Standalone DSC file is the wrong thing to aim for, in this instance --
> in my opinion anyway. Yes, it appears simpler, but it loses too much
> flexibility. In my earlier comments I pointed out that the "allow
> plaintext HTTP connections" PCD was declared in NetworkPkg.dec as either
> fixed or patchable. The dsc.inc file should not squander that
> flexibility and dictate fixed only.
This is not TRUE. Dynamic PCD has the different value format, such as 
DynamicHii PCD. 
Without section header, DynamicHii PCD can't be specified together with 
FixedAtBuild Pcd.
Package level *.pcd.inc provides the recommended type and setting. 
Platform can override Pcd value and type In their DSC file. BaseTools supports 
PCD value override, 
doesn't support PCD type override. This can be supported. 

> 
> > Especially for library instance, the different library instance may be
> > for the different module type. Without section header, they can be
> > placed into one *Libs.dsc.inc file.
> 
> Ugh... I'm now condfused. Now it seems that you and I are asking for the
> same thing actually. Did you perhaps mean, above, that "*.dsc.inc files
> need *NOT* include section header"?
> 
I miss not. I mean without section header, they can't be placed into one 
*Libs.dsc.inc file.
Below is one example. One Libs.dsc.inc can list the different library instance 
for the different module type.

[LibraryClasses]
  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
[LibraryClasses.common.SEC]
  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf

> > [Defines]
> >DEFINE NETWORK_ENABLE = 0x1
> >DEFINE NETWORK_SNP_ENABLE = 0x2
> >DEFINE NETWORK_IP4_ENABLE = 0x4
> >DEFINE NETWORK_IP6_ENABLE = 0x8
> >DEFINE NETWORK_TLS_ENABLE = 0x10
> >DEFINE NETWORK_HTTP_BOOT_ENABLE   = 0x20
> >DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = 0x40
> >DEFINE NETWORK_IPSEC_ENABLE   = 0x80
> >DEFINE NETWORK_ISCSI_ENABLE   = 0x100
> >DEFINE NETWORK_VLAN_ENABLE= 0x200
> >
> > [PcdsFixedAtBuild]
> >gEfiNetworkPkgTokenSpaceGuid.PcdNetworkFeatureMask|0x
> >
> > [PcdsFixedAtBuild]
> > !if gEfiNetworkPkgTokenSpaceGuid.PcdNetworkFeatureMask & 
> > NETWORK_ALLOW_HTTP_CONNECTIONS ==
> 

Re: [edk2] [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib

[Ard Biesheuvel]

On Fri, 23 Nov 2018 at 15:16, Thomas Abraham  wrote:
>
> Hi Ard,
> On Fri, Nov 23, 2018 at 2:16 PM Ard Biesheuvel
>  wrote:
> >
> > The deprecated BdsLib library class in ArmPkg is still depended upon, but
> > only a single implementation exists, which now resides in edk2-platforms.
> >
> > This implementation has some issues in how it deals with Device Paths,
> > so let's fix those, but first move over the library interface declaration
> > and get rid of the parts that are no longer used. This will permit dropping
> > it from ArmPkg in EDK2.
> >
> > Changes since v1:
> > - add Laszlo's ack to #1
> > - update #2 to remove everything we no longer need from BdsLib
> > - drop #3 which was bogus
> > - update #4 to ensure that we only duplicate the device path when we
> >   are about to return EFI_SUCCESS
> >
> > Ard Biesheuvel (3):
> >   Platform/ARM: import ARM platform specific BdsLib header
> >   Platform/ARM/BdsLib: drop unused functions
> >   Platform/ARM/BdsLib: maintain alignment for DevicePaths
> >
> >  Platform/ARM/ARM.dec  |   3 +
> >  .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
> >  Platform/ARM/Include/Library/BdsLib.h |  26 ++
> >  Platform/ARM/Library/BdsLib/BdsAppLoader.c| 253 
> >  Platform/ARM/Library/BdsLib/BdsFilePath.c |  95 +-
> >  Platform/ARM/Library/BdsLib/BdsHelper.c   | 122 
> >  Platform/ARM/Library/BdsLib/BdsInternal.h |  16 +-
> >  Platform/ARM/Library/BdsLib/BdsLib.inf|   4 +-
> >  Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 --
> >  9 files changed, 52 insertions(+), 741 deletions(-)
> >  create mode 100644 Platform/ARM/Include/Library/BdsLib.h
> >  delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
> >  delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c
>
> Tested this patch series with the following two patch series applied
> on the Juno board.
>  - [PATCH v2 0/5] ArmPlatformPkg, ArmVirtPkg: discover NOR flash banks from 
> DTB
>  - [PATCH edk2-platforms 0/3] drop GUIDs from NOR flash bank descriptors
>
> Boot on Juno board works fine.
>
> For this and the other two patch series
> Tested-by: Thomas Abraham 
>

Thanks Thomas!
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] BaseTools: Remove GenVtf

[Gao, Liming]

Please update edk2\BaseTools\Source\C\Makefile and 
edk2\BaseTools\Source\C\GNUmakefile to remove this tool. It is not required to 
be compiled after it is removed. 

> -Original Message-
> From: Zhang, Shenglei
> Sent: Friday, November 23, 2018 1:48 PM
> To: edk2-devel@lists.01.org
> Cc: Zhu, Yonghong ; Gao, Liming 
> Subject: [PATCH] BaseTools: Remove GenVtf
> 
> GenVtf C tool is IPF specific. IPF support has been removed
> from edk2 trunk. This tool can be removed.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1349
> 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang 
> ---
>  BaseTools/Source/BinaryFiles.txt  |1 -
>  BaseTools/Source/C/GenVtf/GNUmakefile |   21 -
>  BaseTools/Source/C/GenVtf/GenVtf.c| 2811 -
>  BaseTools/Source/C/GenVtf/GenVtf.h|  326 ---
>  BaseTools/Source/C/GenVtf/Makefile|   22 -
>  BaseTools/toolsetup.bat   |1 -
>  6 files changed, 3182 deletions(-)
>  delete mode 100644 BaseTools/Source/C/GenVtf/GNUmakefile
>  delete mode 100644 BaseTools/Source/C/GenVtf/GenVtf.c
>  delete mode 100644 BaseTools/Source/C/GenVtf/GenVtf.h
>  delete mode 100644 BaseTools/Source/C/GenVtf/Makefile
> 
> diff --git a/BaseTools/Source/BinaryFiles.txt 
> b/BaseTools/Source/BinaryFiles.txt
> index aad138ebd5..d707df9dda 100644
> --- a/BaseTools/Source/BinaryFiles.txt
> +++ b/BaseTools/Source/BinaryFiles.txt
> @@ -44,7 +44,6 @@ GenFw.exe
>  GenPage.exe
>  GenPatchPcdTable.exe
>  GenSec.exe
> -GenVtf.exe
>  ImportTool.bat
>  LzmaCompress.exe
>  LzmaF86Compress.bat
> diff --git a/BaseTools/Source/C/GenVtf/GNUmakefile 
> b/BaseTools/Source/C/GenVtf/GNUmakefile
> deleted file mode 100644
> index 54160c64d4..00
> --- a/BaseTools/Source/C/GenVtf/GNUmakefile
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -## @file
> -# GNU/Linux makefile for 'GenVtf' module build.
> -#
> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
> -# This program and the accompanying materials
> -# are licensed and made available under the terms and conditions of the BSD 
> License
> -# which accompanies this distribution.  The full text of the license may be 
> found at
> -# http://opensource.org/licenses/bsd-license.php
> -#
> -# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> -#
> -MAKEROOT ?= ..
> -
> -APPNAME = GenVtf
> -
> -LIBS = -lCommon
> -
> -OBJECTS = GenVtf.o
> -
> -include $(MAKEROOT)/Makefiles/app.makefile
> diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c 
> b/BaseTools/Source/C/GenVtf/GenVtf.c
> deleted file mode 100644
> index 3a7ac06156..00
> --- a/BaseTools/Source/C/GenVtf/GenVtf.c
> +++ /dev/null
> @@ -1,2811 +0,0 @@
> -/** @file
> -This file contains functions required to generate a boot strap file (BSF) 
> also
> -known as the Volume Top File (VTF)
> -
> -Copyright (c) 1999 - 2018, Intel Corporation. All rights reserved.
> -This program and the accompanying materials are licensed and made available
> -under the terms and conditions of the BSD License which accompanies this
> -distribution.  The full text of the license may be found at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -
> -**/
> -
> -//
> -//
> -//
> -#include 
> -#include 
> -#include "GenVtf.h"
> -#include 
> -#include "CommonLib.h"
> -#include "EfiUtilityMsgs.h"
> -
> -//
> -// Global variables
> -//
> -UINTN SectionOptionFlag = 0;
> -UINTN SectionCompFlag = 0;
> -
> -UINT64DebugLevel;
> -BOOLEAN   DebugMode;
> -
> -BOOLEAN QuietMode = FALSE;
> -
> -BOOLEAN VTF_OUTPUT = FALSE;
> -CHAR8   *OutFileName1;
> -CHAR8   *OutFileName2;
> -CHAR8   *SymFileName;
> -
> -CHAR8   **TokenStr;
> -CHAR8   **OrgStrTokPtr;
> -
> -PARSED_VTF_INFO *FileListPtr;
> -PARSED_VTF_INFO *FileListHeadPtr;
> -
> -VOID*Vtf1Buffer;
> -VOID*Vtf1EndBuffer;
> -VOID*Vtf2Buffer;
> -VOID*Vtf2EndBuffer;
> -
> -UINTN   ValidLineNum= 0;
> -UINTN   ValidFFDFileListNum = 0;
> -
> -//
> -// Section Description and their number of occurences in *.INF file
> -//
> -UINTN   NumFvFiles= 0;
> -UINTN   SectionOptionNum  = 0;
> -
> -//
> -// Global flag which will check for VTF Present, if yes then will be used
> -// to decide about adding FFS header to pad data
> -//
> -BOOLEAN VTFPresent = FALSE;
> -BOOLEAN SecondVTF = FALSE;
> -
> -//
> -// Address related information
> -//
> -UINT64  Fv1BaseAddress= 0;
> -UINT64  Fv2BaseAddress= 0;
> -UINT64  Fv1EndAddress = 0;
> -UINT64  Fv2EndAddress = 0;
> -UINT32  Vtf1TotalSize = 

Re: [edk2] [edk2-test][PATCH] SctPkg/build: Add support for GenBin tool build

[Lokesh Belathur Veerappa]

Hello Eric/Supreeth,

Could you please review this patch.

Thanks,
Lokesh

-Original Message-
From: Lokesh B V 
Sent: Tuesday, November 20, 2018 12:21 PM
To: edk2-devel@lists.01.org; Supreeth Venkatesh ; 
eric@intel.com
Cc: Lokesh Belathur Veerappa 
Subject: [edk2-test][PATCH] SctPkg/build: Add support for GenBin tool build

As the GenBin tool is necessary for SCT build, it is appropriate to support 
it's build in the SCT build procedure.

Signed-off-by: Lokesh B V 
---
 uefi-sct/SctPkg/build.sh | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/uefi-sct/SctPkg/build.sh b/uefi-sct/SctPkg/build.sh index 
73581c9..e070ad5 100755
--- a/uefi-sct/SctPkg/build.sh
+++ b/uefi-sct/SctPkg/build.sh
@@ -228,21 +228,26 @@ else
   echo using prebuilt tools
 fi

-# Copy GenBin file to Base tools directory
+if  [[ ! -e $EDK_TOOLS_PATH/Source/C/bin/GenBin ]] then
+  # build the GenBin if it doesn't yet exist
+  echo Building GenBin
+  make -C $EDK_TOOLS_PATH/../SctPkg/Tools/Source/GenBin
+  status=$?
+  if test $status -ne 0
+  then
+  echo Error while building GenBin
+exit -1
+  fi
+else
+  echo using prebuilt GenBin
+fi
+
+# Copy GenBin file to Base tools bin directory
 DEST_DIR=`GetEdkToolsPathBinDirectory`
 # Ensure the directory exist
 mkdir -p $DEST_DIR
-case `uname -m` in
-x86_64)
-cp SctPkg/Tools/Bin/GenBin_lin_64 $DEST_DIR/GenBin
-;;
-x86_32)
-cp SctPkg/Tools/Bin/GenBin_lin_32 $DEST_DIR/GenBin
-;;
-*)
-cp SctPkg/Tools/Bin/GenBin_lin_32 $DEST_DIR/GenBin
-;;
-esac
+cp $EDK_TOOLS_PATH/Source/C/bin/GenBin $DEST_DIR/GenBin

 #
 # Build the SCT package
--
2.7.4

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib

[Thomas Abraham]

Hi Ard,
On Fri, Nov 23, 2018 at 2:16 PM Ard Biesheuvel
 wrote:
>
> The deprecated BdsLib library class in ArmPkg is still depended upon, but
> only a single implementation exists, which now resides in edk2-platforms.
>
> This implementation has some issues in how it deals with Device Paths,
> so let's fix those, but first move over the library interface declaration
> and get rid of the parts that are no longer used. This will permit dropping
> it from ArmPkg in EDK2.
>
> Changes since v1:
> - add Laszlo's ack to #1
> - update #2 to remove everything we no longer need from BdsLib
> - drop #3 which was bogus
> - update #4 to ensure that we only duplicate the device path when we
>   are about to return EFI_SUCCESS
>
> Ard Biesheuvel (3):
>   Platform/ARM: import ARM platform specific BdsLib header
>   Platform/ARM/BdsLib: drop unused functions
>   Platform/ARM/BdsLib: maintain alignment for DevicePaths
>
>  Platform/ARM/ARM.dec  |   3 +
>  .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
>  Platform/ARM/Include/Library/BdsLib.h |  26 ++
>  Platform/ARM/Library/BdsLib/BdsAppLoader.c| 253 
>  Platform/ARM/Library/BdsLib/BdsFilePath.c |  95 +-
>  Platform/ARM/Library/BdsLib/BdsHelper.c   | 122 
>  Platform/ARM/Library/BdsLib/BdsInternal.h |  16 +-
>  Platform/ARM/Library/BdsLib/BdsLib.inf|   4 +-
>  Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 --
>  9 files changed, 52 insertions(+), 741 deletions(-)
>  create mode 100644 Platform/ARM/Include/Library/BdsLib.h
>  delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
>  delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c

Tested this patch series with the following two patch series applied
on the Juno board.
 - [PATCH v2 0/5] ArmPlatformPkg, ArmVirtPkg: discover NOR flash banks from DTB
 - [PATCH edk2-platforms 0/3] drop GUIDs from NOR flash bank descriptors

Boot on Juno board works fine.

For this and the other two patch series
Tested-by: Thomas Abraham 

>
> --
> 2.17.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 5/5] ArmVirtPkg: revert PcdPrePiCpuMemorySize to is default value of 48

[Ard Biesheuvel]

On Fri, 23 Nov 2018 at 14:36, Andrew Jones  wrote:
>
> On Fri, Nov 23, 2018 at 01:14:31PM +0100, Ard Biesheuvel wrote:
> > Drop the PcdPrePiCpuMemorySize definitions that limit it to 40
> > bits on AArch64 targets. This is no longer appropriate now that
> > KVM has been enhanced to permit any IPA space size permitted by
> > the architecture. This means the value will revert back to its
> > default of 48.
>
> If we're running on older KVM, then, since KVM just passes through
> the host value of id_aa64mmfr0_el1, firmware will see whatever
> the host supports and use that (I'm not sure if the 48-bit default
> ever can come into play too). In either case, it probably doesn't
> matter, because just like the guest kernel works today on older
> KVM, as long as nothing is placed above 40 bits there isn't any
> problem. Is that the case for edk2 too?
>

The value of 48 serves as a limit now, which makes sense given that
52-bit requires 64k pages, which we don't support.

But as I said, it might make sense to permit the GCD space to describe
that much, which is actually a nice side effect of the previous patch,
which takes the value directly from the CPU system register on virt
targets.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/5] ArmPkg/ArmLib: add support for reading the max physical address space size

[Ard Biesheuvel]

On Fri, 23 Nov 2018 at 14:20, Ard Biesheuvel  wrote:
>
> On Fri, 23 Nov 2018 at 14:16, Andrew Jones  wrote:
> >
> > On Fri, Nov 23, 2018 at 01:14:27PM +0100, Ard Biesheuvel wrote:
> > > Add a helper function that returns the maximum physical address space
> > > size as supported by the current CPU.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel 
> > > ---
> > >  ArmPkg/Include/Library/ArmLib.h   |  6 ++
> > >  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 
> > >  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S |  8 
> > >  3 files changed, 30 insertions(+)
> > >
> > > diff --git a/ArmPkg/Include/Library/ArmLib.h 
> > > b/ArmPkg/Include/Library/ArmLib.h
> > > index ffda50e9d767..9a804c15fdb6 100644
> > > --- a/ArmPkg/Include/Library/ArmLib.h
> > > +++ b/ArmPkg/Include/Library/ArmLib.h
> > > @@ -733,4 +733,10 @@ ArmWriteCntvOff (
> > >UINT64   Val
> > >);
> > >
> > > +UINTN
> > > +EFIAPI
> > > +ArmGetPhysicalAddressBits (
> > > +  VOID
> > > +  );
> > > +
> > >  #endif // __ARM_LIB__
> > > diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S 
> > > b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > > index 1ef2f61f5979..75ab8dade485 100644
> > > --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > > +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > > @@ -196,4 +196,20 @@ ASM_FUNC(ArmWriteSctlr)
> > >  3:msr   sctlr_el3, x0
> > >  4:ret
> > >
> > > +ASM_FUNC(ArmGetPhysicalAddressBits)
> > > +  mrs   x0, id_aa64mmfr0_el1
> > > +  adr   x1, .LPARanges
> > > +  and   x0, x0, #7
> > > +  ldrb  w0, [x1, x0]
> > > +  ret
> > > +
> > > +//
> > > +// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
> > > +// physical address space support on this CPU:
> > > +// 0 == 32 bits, 1 == 36 bits, etc etc
> > > +// 6 and 7 are reserved
> > > +//
> > > +.LPARanges:
> > > +  .byte 32, 36, 40, 42, 44, 48, -1, -1
> >
> > Hi Ard,
> >
> > One of the things I was wondering is how much it matters what the
> > firmware's opinion of highest physical address is vs. the guest
> > kernel. Do they need to match? This patch series implies they do,
> > or at least that 40-bits won't always be sufficient for firmware.
>
> Yes. The size of the GCD space limits how much memory we can report as
> present to the OS. So it only matters if there is DRAM there.
>
> > However, guests using 64k pages running on supporting hardware can
> > use 52-bits. Considering ArmVirtPkg only uses 4k pages, that's not
> > an option for it, and that justifies not defining index 6 == 52 in
> > the above array, but will that also restrict the guest?
> >
>
> At the moment, yes. UEFI support for 52-bit/64k pages is still under
> discussion, and is presently not supported.
>

... which btw doesn't mean we can't report that much memory in the GCD
memory map, we just can't map it in UEFI.

There were some discussions about how to proceed here, since there
appear to be some SoC vendors that want to use bit 51 to distinguish
between DRAM and MMIO regions, which implies that we have to support
it to be able to boot such systems (and the architecture does not
forbid or discourage the practice)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/5] ArmPkg/ArmLib: add support for reading the max physical address space size

[Ard Biesheuvel]

On Fri, 23 Nov 2018 at 14:16, Andrew Jones  wrote:
>
> On Fri, Nov 23, 2018 at 01:14:27PM +0100, Ard Biesheuvel wrote:
> > Add a helper function that returns the maximum physical address space
> > size as supported by the current CPU.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPkg/Include/Library/ArmLib.h   |  6 ++
> >  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 
> >  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S |  8 
> >  3 files changed, 30 insertions(+)
> >
> > diff --git a/ArmPkg/Include/Library/ArmLib.h 
> > b/ArmPkg/Include/Library/ArmLib.h
> > index ffda50e9d767..9a804c15fdb6 100644
> > --- a/ArmPkg/Include/Library/ArmLib.h
> > +++ b/ArmPkg/Include/Library/ArmLib.h
> > @@ -733,4 +733,10 @@ ArmWriteCntvOff (
> >UINT64   Val
> >);
> >
> > +UINTN
> > +EFIAPI
> > +ArmGetPhysicalAddressBits (
> > +  VOID
> > +  );
> > +
> >  #endif // __ARM_LIB__
> > diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S 
> > b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > index 1ef2f61f5979..75ab8dade485 100644
> > --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > @@ -196,4 +196,20 @@ ASM_FUNC(ArmWriteSctlr)
> >  3:msr   sctlr_el3, x0
> >  4:ret
> >
> > +ASM_FUNC(ArmGetPhysicalAddressBits)
> > +  mrs   x0, id_aa64mmfr0_el1
> > +  adr   x1, .LPARanges
> > +  and   x0, x0, #7
> > +  ldrb  w0, [x1, x0]
> > +  ret
> > +
> > +//
> > +// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
> > +// physical address space support on this CPU:
> > +// 0 == 32 bits, 1 == 36 bits, etc etc
> > +// 6 and 7 are reserved
> > +//
> > +.LPARanges:
> > +  .byte 32, 36, 40, 42, 44, 48, -1, -1
>
> Hi Ard,
>
> One of the things I was wondering is how much it matters what the
> firmware's opinion of highest physical address is vs. the guest
> kernel. Do they need to match? This patch series implies they do,
> or at least that 40-bits won't always be sufficient for firmware.

Yes. The size of the GCD space limits how much memory we can report as
present to the OS. So it only matters if there is DRAM there.

> However, guests using 64k pages running on supporting hardware can
> use 52-bits. Considering ArmVirtPkg only uses 4k pages, that's not
> an option for it, and that justifies not defining index 6 == 52 in
> the above array, but will that also restrict the guest?
>

At the moment, yes. UEFI support for 52-bit/64k pages is still under
discussion, and is presently not supported.


> > diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S 
> > b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> > index f2a517671f0a..f2f3c9a25991 100644
> > --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> > +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> > @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
> >isb
> >bx  lr
> >
> > +ASM_FUNC (ArmGetPhysicalAddressBits)
> > +  mrc p15, 0, r0, c0, c1, 4   // MMFR0
> > +  and r0, r0, #0xf// VMSA [3:0]
> > +  cmp r0, #5  // >5 implies LPAE support
> > +  movlt   r0, #32 // 32 bits if no LPAE
> > +  movge   r0, #40 // 40 bits if LPAE
> > +  bx  lr
> > +
> >  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> > --
> > 2.17.1
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths

[Laszlo Ersek]

On 11/23/18 09:44, Ard Biesheuvel wrote:
> DevicePath node types may have any size, and so it is up to the
> code that manipulates them to ensure that dereferencing them only
> occurs when the pointer is aligned explicitly.
> 
> Since BdsConnectAndUpdateDevicePath() has only two callers, one of
> which itself, we can simply duplicate the device path (similar to
> how DxeCore's CoreConnectController () does it), and free the pool
> allocation again on the way out. (Note that the allocation only
> occurs when the non-recursive path is taken and the function
> returns EFI_SUCCESS)
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  Platform/ARM/Library/BdsLib/BdsFilePath.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c 
> b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> index 62f796e5526d..ad66b2f82718 100644
> --- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
> +++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> @@ -423,8 +423,8 @@ BdsConnectAndUpdateDevicePath (
>  }
>}
>  
> -  if (RemainingDevicePath) {
> -*RemainingDevicePath = Remaining;
> +  if (!EFI_ERROR (Status) && RemainingDevicePath != NULL) {
> +*RemainingDevicePath = DuplicateDevicePath (Remaining);
>}
>  
>return Status;
> @@ -1314,14 +1314,18 @@ BdsLoadImageAndUpdateDevicePath (
>}
>  
>FileLoader = FileLoaders;
> +  Status = EFI_UNSUPPORTED;
>while (FileLoader->Support != NULL) {
>  if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {
> -  return FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath, 
> Type, Image, FileSize);
> +  Status = FileLoader->LoadImage (DevicePath, Handle, 
> RemainingDevicePath,
> + Type, Image, FileSize);
> +  break;
>  }
>  FileLoader++;
>}
>  
> -  return EFI_UNSUPPORTED;
> +  FreePool (RemainingDevicePath);
> +  return Status;
>  }
>  
>  EFI_STATUS
> 

Reviewed-by: Laszlo Ersek 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions

[Laszlo Ersek]

On 11/23/18 09:44, Ard Biesheuvel wrote:
> Clean up BdsLib (which is deprecated and should not be used for future
> development) by removing all the pieces that are not being used at the
> moment.
> 
> After this patch, only BdsLoadImage() remains, and the pieces it relies
> upon. This function is used by FdtPlatformDxe to load device tree
> binaries from device paths.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  Platform/ARM/Include/Library/BdsLib.h   | 186 -
>  Platform/ARM/Library/BdsLib/BdsAppLoader.c  | 253 --
>  Platform/ARM/Library/BdsLib/BdsFilePath.c   |  83 +-
>  Platform/ARM/Library/BdsLib/BdsHelper.c | 122 -
>  Platform/ARM/Library/BdsLib/BdsInternal.h   |  15 +-
>  Platform/ARM/Library/BdsLib/BdsLib.inf  |   2 -
>  Platform/ARM/Library/BdsLib/BdsLoadOption.c | 272 
>  7 files changed, 13 insertions(+), 920 deletions(-)

Heh, this diffstat is a bit beyond my willingness to review in detail :)
I do see it only removes lines, so if it compiles, it must be good.
Also, BdsConnectDevicePath() in particular is removed:

> -/**
> -  Connect a Device Path and return the handle of the driver that support 
> this DevicePath
> -
> -  @param  DevicePathDevice Path of the File to connect
> -  @param  HandleHandle of the driver that support this 
> DevicePath
> -  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match 
> the driver DevicePath
> -
> -  @retval EFI_SUCCESS   A driver that matches the Device Path has 
> been found
> -  @retval EFI_NOT_FOUND No handles match the search.
> -  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
> -
> -**/
> -EFI_STATUS
> -BdsConnectDevicePath (
> -  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
> -  OUT EFI_HANDLE*Handle,
> -  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
> -  )
> -{
> -  return BdsConnectAndUpdateDevicePath (, Handle, 
> RemainingDevicePath);
> -}

Hence:

Acked-by: Laszlo Ersek 

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/6] ArmVirtPkg: Update DSC/FDF to use NetworkPkg's include fragment file.

[Laszlo Ersek]

On 11/23/18 13:29, Laszlo Ersek wrote:

> (2) This is good, and the end result that we want; however, it does two
> things at once -- it eliminates the MdeModulePkg network drivers, and it
> adopts the NetworkPkg includes.
> 
> In OvmfPkg, we handled the first step separately, in commit d2f1f6423bd1
> ("OvmfPkg: Replace obsoleted network drivers from platform DSC/FDF.",
> 2018-11-06).
> 
> I'm not proposing that we separate out the same change for ArmVirtPkg as
> well. After all, we can consider the current patch as a *means* to
> implementing the same -- just better. So, what I propose is, please add
> the following to the commit message:
> 
> This change automatically ports OvmfPkg commit d2f1f6423bd1
> ("OvmfPkg: Replace obsoleted network drivers from platform
> DSC/FDF.", 2018-11-06) to NetworkPkg.

Argh, obviously, I meant "... to ArmVirtPkg". :)

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/6] ArmVirtPkg: Update DSC/FDF to use NetworkPkg's include fragment file.

[Laszlo Ersek]

On 11/22/18 06:21, Fu Siyuan wrote:
> This patch updates the platform DSC/FDF files to use the include fragment
> files provided by NetworkPkg.
> The feature enabling flags in [Defines] section have been updated to use
> the NetworkPkg's terms, and the value has been overridden with the original
> default value on this platform.
> 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan 
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc   | 11 +
>  ArmVirtPkg/ArmVirtQemu.dsc   | 46 ++--
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 28 +---
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 46 ++--
>  4 files changed, 28 insertions(+), 103 deletions(-)

(1) Please reference
 in the commit message.

> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 70a0ac4d786c..d7de73a88eff 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -76,16 +76,7 @@ [LibraryClasses.common]
>BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
>  
># Networking Requirements
> -  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> -  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> -  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> -  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> -!endif
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> -!endif
> +!include NetworkPkg/NetworkLibs.dsc.inc
>  
>#
># It is not possible to prevent the ARM compiler from inserting calls to 
> intrinsic functions.
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 885c6b14b844..1414b6a5ccf8 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -33,9 +33,14 @@ [Defines]
># Defines for default states.  These can be changed on the command line.
># -D FLAG=VALUE
>#
> -  DEFINE SECURE_BOOT_ENABLE  = FALSE
> -  DEFINE NETWORK_IP6_ENABLE  = FALSE
> -  DEFINE HTTP_BOOT_ENABLE= FALSE
> +  DEFINE SECURE_BOOT_ENABLE  = FALSE
> +  DEFINE NETWORK_IP6_ENABLE  = FALSE
> +  DEFINE NETWORK_HTTP_BOOT_ENABLE= FALSE
> +  DEFINE NETWORK_SNP_ENABLE  = FALSE
> +  DEFINE NETWORK_TLS_ENABLE  = FALSE
> +  DEFINE NETWORK_IPSEC_ENABLE= FALSE
> +  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
> +!include NetworkPkg/NetworkDefines.dsc.inc
>  
>  !include ArmVirtPkg/ArmVirt.dsc.inc
>  

This is correct.

> @@ -123,9 +128,10 @@ [PcdsFixedAtBuild.common]
>#
>gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|0
>  
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
> -!endif
> +  #
> +  # Network Pcds
> +  #
> +!include NetworkPkg/NetworkPcds.dsc.inc
>  
># System Memory Base -- fixed at 0x4000_
>gArmTokenSpaceGuid.PcdSystemMemoryBase|0x4000
> @@ -338,33 +344,7 @@ [Components.common]
>#
># Networking stack
>#
> -  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> -  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> -  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> -  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> -  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> -  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> -  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> -  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> -  NetworkPkg/TcpDxe/TcpDxe.inf
> -  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> -  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> -  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> -  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  NetworkPkg/IScsiDxe/IScsiDxe.inf
> -!else
> -  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> -  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> -!endif
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  NetworkPkg/DnsDxe/DnsDxe.inf
> -  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> -  NetworkPkg/HttpDxe/HttpDxe.inf
> -  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> -!endif
> +!include NetworkPkg/NetworkComponents.dsc.inc
>  
>#
># SCSI Bus and Disk Driver

(2) This is good, and the end result that we want; however, it does two
things at once -- it eliminates the MdeModulePkg network drivers, and it
adopts the NetworkPkg includes.

In OvmfPkg, we handled the first step separately, in commit d2f1f6423bd1
("OvmfPkg: Replace obsoleted network drivers from platform DSC/FDF.",
2018-11-06).

I'm not proposing that we separate out the same change for ArmVirtPkg as
well. After all, we can consider the current patch as a *means* to
implementing 

Re: [edk2] [PATCH v2 5/6] OvmfPkg: Update DSC/FDF to use NetworkPkg's include fragment file.

[Laszlo Ersek]

Yet another comment:

On 11/22/18 06:21, Fu Siyuan wrote:
> This patch updates the platform DSC/FDF files to use the include fragment
> files provided by NetworkPkg.
> The feature enabling flags in [Defines] section have been updated to use
> the NetworkPkg's terms, and the value has been overridden with the original
> default value on this platform.
> 
> This patch also rename the TLS_ENABLE flag to PLATFORM_TLS_ENABLE for the
> platform specific configuration for TLS support.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan 
> ---
> 
> Notes:
> v2:
> Rename TLS_ENABLE flag to PLATFORM_TLS_ENABLE flag for platform specific 
> configuration for TLS support.

(6) Please reference
 in the commit message.

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 5/5] ArmVirtPkg: revert PcdPrePiCpuMemorySize to is default value of 48

[Ard Biesheuvel]

Drop the PcdPrePiCpuMemorySize definitions that limit it to 40
bits on AArch64 targets. This is no longer appropriate now that
KVM has been enhanced to permit any IPA space size permitted by
the architecture. This means the value will revert back to its
default of 48.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 4 
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 
 2 files changed, 8 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index cb59c790afcc..42f2adce80e6 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -143,10 +143,6 @@
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
 
 [PcdsFixedAtBuild.AARCH64]
-  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
-  # support anything bigger, even if the host hardware does
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
-
   # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
   # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
   # presence of the 32-bit entry point anyway (because many AARCH64 systems
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 434d6861a56f..d8fbf14e8f4e 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -157,10 +157,6 @@
   #
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
 
-  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
-  # support anything bigger, even if the host hardware does
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
-
 [PcdsDynamicDefault.common]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
 
-- 
2.17.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 4/5] ArmVirtPkg: disregard PcdPrePiCpuMemorySize PCD when sizing the GCD space

[Ard Biesheuvel]

Move the call to BuildCpuHob () into ArmVirtMemoryInitPeiLib (which is
shared between all the ArmVirtPkg targets), and drop the inclusion of
CpuPei.inf [which calls it on ArmVirtQemu] and the BuildCpuHob () call
from ArmVirtPrePiUniCoreRelocatable [for the other targets].

This makes the size of the GCD address space and page table mappings
depend only on the size of the physical address space as exposed by
the CPU system registers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc | 1 -
 ArmVirtPkg/ArmVirtQemu.fdf | 1 -
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c   | 3 +++
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf | 1 +
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c | 5 
+
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf   | 1 -
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf| 1 -
 ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf| 3 ---
 ArmVirtPkg/PrePi/PrePi.c   | 3 ---
 9 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 885c6b14b844..cb59c790afcc 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -226,7 +226,6 @@
   }
   ArmPlatformPkg/PlatformPei/PlatformPeim.inf
   ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
-  ArmPkg/Drivers/CpuPei/CpuPei.inf
 
   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index c6a22dc018f3..12bc570c4cb3 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -106,7 +106,6 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Core/Pei/PeiMain.inf
   INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
   INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
-  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
   INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
   INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
   INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
diff --git 
a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c 
b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
index 3f0e9b3a0579..3d86d31ab50e 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
@@ -116,5 +116,8 @@ MemoryPeim (
 BuildMemoryTypeInformationHob ();
   }
 
+  // Publish the CPU memory and io spaces sizes
+  BuildCpuHob (ArmGetPhysicalAddressBits (), FixedPcdGet8 (PcdPrePiCpuIoSize));
+
   return EFI_SUCCESS;
 }
diff --git 
a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf 
b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf
index 54879d590a8a..d0e39df84b20 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf
@@ -59,6 +59,7 @@
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
 
 [Pcd]
   gArmTokenSpaceGuid.PcdSystemMemoryBase
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
index 4eca9b895354..ded87d604f4f 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
@@ -46,7 +46,6 @@ ArmVirtGetMemoryMap (
   )
 {
   ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
-  UINT64TopOfMemory;
 
   ASSERT (VirtualMemoryMap != NULL);
 
@@ -80,11 +79,9 @@ ArmVirtGetMemoryMap (
   VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
   // Peripheral space after DRAM
-  TopOfMemory = MIN (1ULL << FixedPcdGet8 (PcdPrePiCpuMemorySize),
- TopOfAddressSpace);
   VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + 
VirtualMemoryTable[1].Length;
   VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
-  VirtualMemoryTable[2].Length   = TopOfMemory -
+  VirtualMemoryTable[2].Length   = TopOfAddressSpace -
VirtualMemoryTable[2].PhysicalBase;
   VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
index f2c461e3b55a..5c5b841051ad 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
@@ -45,4 +45,3 @@
 

[edk2] [PATCH 2/5] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account

[Ard Biesheuvel]

In preparation of permitting the virt code to define a larger PA space
size via gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize than what the
CPU actually supports, take the CPU's capabilities into account when
setting up the page tables. This is necessary because KVM will shortly
support variable PA space sizes, and to support running the same UEFI
binaries regardless of that limit, PcdPrePiCpuMemorySize needs to be
treated as an upper bound rather than a fixed size.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 4b62ecb6a476..a4fde9b59383 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -593,6 +593,7 @@ ArmConfigureMmu (
 {
   VOID* TranslationTable;
   UINT32TranslationTableAttribute;
+  UINTN MaxAddressBits;
   UINT64MaxAddress;
   UINTN T0SZ;
   UINTN RootTableEntryCount;
@@ -605,7 +606,9 @@ ArmConfigureMmu (
   }
 
   // Cover the entire GCD memory space
-  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
+  MaxAddressBits = MIN (ArmGetPhysicalAddressBits (),
+PcdGet8 (PcdPrePiCpuMemorySize));
+  MaxAddress = (1UL << MaxAddressBits) - 1;
 
   // Lookup the Table Level to get the information
   LookupAddresstoRootTable (MaxAddress, , );
-- 
2.17.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 3/5] ArmVirtPkg: refactor reading of the physical address space size

[Ard Biesheuvel]

In preparation of dropping the hardcoded 40-bit limit on the physical
address space when running under virtualization, let's refactor the
code a bit so we read the hardware limit in a single place.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/Include/Library/ArmVirtMemInfoLib.h   |  1 +
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c |  4 +-
 ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S  | 39 

 ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S  | 24 

 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c   |  3 +-
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf |  6 ---
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf  |  6 ---
 ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S   | 39 

 ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S   | 24 

 ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c |  8 +---
 ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf   |  6 ---
 11 files changed, 8 insertions(+), 152 deletions(-)

diff --git a/ArmVirtPkg/Include/Library/ArmVirtMemInfoLib.h 
b/ArmVirtPkg/Include/Library/ArmVirtMemInfoLib.h
index bdf1c513bc6d..15562b35c730 100644
--- a/ArmVirtPkg/Include/Library/ArmVirtMemInfoLib.h
+++ b/ArmVirtPkg/Include/Library/ArmVirtMemInfoLib.h
@@ -35,6 +35,7 @@
 VOID
 EFIAPI
 ArmVirtGetMemoryMap (
+  IN  EFI_PHYSICAL_ADDRESSTopOfAddressSpace,
   OUT ARM_MEMORY_REGION_DESCRIPTOR**VirtualMemoryMap
   );
 
diff --git 
a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c 
b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
index 05afd1282422..3f0e9b3a0579 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
@@ -15,6 +15,7 @@
 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -39,7 +40,8 @@ InitMmu (
   RETURN_STATUS Status;
 
   // Get Virtual Memory Map from the Platform Library
-  ArmVirtGetMemoryMap ();
+  ArmVirtGetMemoryMap (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()),
+);
 
   //Note: Because we called PeiServicesInstallPeiMemory() before to call 
InitMmu() the MMU Page Table resides in
   //  DRAM (even at the top of DRAM as it is the first permanent memory 
allocation)
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
deleted file mode 100644
index a1f6a194d59b..
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
+++ /dev/null
@@ -1,39 +0,0 @@
-#
-#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
-#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
-#
-#  This program and the accompanying materials
-#  are licensed and made available under the terms and conditions of the BSD 
License
-#  which accompanies this distribution.  The full text of the license may be 
found at
-#  http://opensource.org/licenses/bsd-license.php
-#
-#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
-#
-#
-
-#include 
-
-//EFI_PHYSICAL_ADDRESS
-//GetPhysAddrTop (
-//  VOID
-//  );
-ASM_FUNC(ArmGetPhysAddrTop)
-  mrs   x0, id_aa64mmfr0_el1
-  adr   x1, .LPARanges
-  and   x0, x0, #7
-  ldrb  w1, [x1, x0]
-  mov   x0, #1
-  lsl   x0, x0, x1
-  ret
-
-//
-// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
-// physical address space support on this CPU:
-// 0 == 32 bits, 1 == 36 bits, etc etc
-// 6 and 7 are reserved
-//
-.LPARanges:
-  .byte 32, 36, 40, 42, 44, 48, -1, -1
-
-ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
deleted file mode 100644
index 9cd81529fb3d..
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
+++ /dev/null
@@ -1,24 +0,0 @@
-#
-#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
-#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
-#
-#  This program and the accompanying materials
-#  are licensed and made available under the terms and conditions of the BSD 
License
-#  which accompanies this distribution.  The full text of the license may be 
found at
-#  http://opensource.org/licenses/bsd-license.php
-#
-#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
-#
-#
-
-#include 
-
-//EFI_PHYSICAL_ADDRESS
-//GetPhysAddrTop (
-//  VOID
-//  );
-ASM_FUNC(ArmGetPhysAddrTop)
-  mov   r0, #0x
-  mov   r1, #0x1
-  bxlr
diff --git 

[edk2] [PATCH 0/5] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit

[Ard Biesheuvel]

The ArmVirtQemu targets currently limit the size of the IPA space to
40 bits because that is all what KVM supports. However, this is about
to change, and so we need to update the code if we want to ensure that
our UEFI firmware builds can keep running on systems that set values
other than 40 (which could be > 40 or < 40)

So add a helper to ArmLib to read the number of supported address bits (#1)
and take this into account in the page table code (#2), which allows
PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of
the CPU.

Patch #3 is mostly a cleanup patch, to switch to the new helper added in
patch #1. No functional changes intended.

Patch #4 builds the CPU hob (and thus declares the size of the GCD memory
space) based on the CPU capabilities rather than the value of 
PcdPrePiCpuMemorySize, to prevent any potential regressions in memory
utilization when we bump PcdPrePiCpuMemorySize back to 48.

Patch #5 drops the definitions of PcdPrePiCpuMemorySize, reverting its
value back to the default 48.

Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Eric Auger 
Cc: Andrew Jones 
Cc: Philippe Mathieu-Daude 
Cc: Julien Grall 

Ard Biesheuvel (5):
  ArmPkg/ArmLib: add support for reading the max physical address space
size
  ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
  ArmVirtPkg: refactor reading of the physical address space size
  ArmVirtPkg: disregard PcdPrePiCpuMemorySize PCD when sizing the GCD
space
  ArmVirtPkg: revert PcdPrePiCpuMemorySize to is default value of 48

 ArmPkg/Include/Library/ArmLib.h   |  6 +++
 ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S |  8 
 .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |  5 ++-
 ArmVirtPkg/ArmVirtQemu.dsc|  5 ---
 ArmVirtPkg/ArmVirtQemu.fdf|  1 -
 ArmVirtPkg/ArmVirtQemuKernel.dsc  |  4 --
 .../Include/Library/ArmVirtMemInfoLib.h   |  1 +
 .../ArmVirtMemoryInitPeiLib.c |  7 +++-
 .../ArmVirtMemoryInitPeiLib.inf   |  1 +
 .../QemuVirtMemInfoLib/AArch64/PhysAddrTop.S  | 39 ---
 .../QemuVirtMemInfoLib/Arm/PhysAddrTop.S  | 24 
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.c   |  6 +--
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf |  7 
 .../QemuVirtMemInfoPeiLib.inf |  7 
 .../XenVirtMemInfoLib/AArch64/PhysAddrTop.S   | 39 ---
 .../XenVirtMemInfoLib/Arm/PhysAddrTop.S   | 24 
 .../XenVirtMemInfoLib/XenVirtMemInfoLib.c |  8 +---
 .../XenVirtMemInfoLib/XenVirtMemInfoLib.inf   |  6 ---
 .../PrePi/ArmVirtPrePiUniCoreRelocatable.inf  |  3 --
 ArmVirtPkg/PrePi/PrePi.c  |  3 --
 21 files changed, 46 insertions(+), 174 deletions(-)
 delete mode 100644 ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
 delete mode 100644 ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
 delete mode 100644 ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
 delete mode 100644 ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S

-- 
2.17.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 1/5] ArmPkg/ArmLib: add support for reading the max physical address space size

[Ard Biesheuvel]

Add a helper function that returns the maximum physical address space
size as supported by the current CPU.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Include/Library/ArmLib.h   |  6 ++
 ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S |  8 
 3 files changed, 30 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index ffda50e9d767..9a804c15fdb6 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -733,4 +733,10 @@ ArmWriteCntvOff (
   UINT64   Val
   );
 
+UINTN
+EFIAPI
+ArmGetPhysicalAddressBits (
+  VOID
+  );
+
 #endif // __ARM_LIB__
diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S 
b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
index 1ef2f61f5979..75ab8dade485 100644
--- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
@@ -196,4 +196,20 @@ ASM_FUNC(ArmWriteSctlr)
 3:msr   sctlr_el3, x0
 4:ret
 
+ASM_FUNC(ArmGetPhysicalAddressBits)
+  mrs   x0, id_aa64mmfr0_el1
+  adr   x1, .LPARanges
+  and   x0, x0, #7
+  ldrb  w0, [x1, x0]
+  ret
+
+//
+// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
+// physical address space support on this CPU:
+// 0 == 32 bits, 1 == 36 bits, etc etc
+// 6 and 7 are reserved
+//
+.LPARanges:
+  .byte 32, 36, 40, 42, 44, 48, -1, -1
+
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S 
b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
index f2a517671f0a..f2f3c9a25991 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
@@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
   isb
   bx  lr
 
+ASM_FUNC (ArmGetPhysicalAddressBits)
+  mrc p15, 0, r0, c0, c1, 4   // MMFR0
+  and r0, r0, #0xf// VMSA [3:0]
+  cmp r0, #5  // >5 implies LPAE support
+  movlt   r0, #32 // 32 bits if no LPAE
+  movge   r0, #40 // 40 bits if LPAE
+  bx  lr
+
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED
-- 
2.17.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 5/6] OvmfPkg: Update DSC/FDF to use NetworkPkg's include fragment file.

[Laszlo Ersek]

Another comment for this patch:

On 11/22/18 06:21, Fu Siyuan wrote:
> This patch updates the platform DSC/FDF files to use the include fragment
> files provided by NetworkPkg.
> The feature enabling flags in [Defines] section have been updated to use
> the NetworkPkg's terms, and the value has been overridden with the original
> default value on this platform.
> 
> This patch also rename the TLS_ENABLE flag to PLATFORM_TLS_ENABLE for the
> platform specific configuration for TLS support.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan 
> ---
> 
> Notes:
> v2:
> Rename TLS_ENABLE flag to PLATFORM_TLS_ENABLE flag for platform specific 
> configuration for TLS support.
> 
>  OvmfPkg/OvmfPkgIa32.dsc| 75 +--
>  OvmfPkg/OvmfPkgIa32.fdf| 27 +--
>  OvmfPkg/OvmfPkgIa32X64.dsc | 76 +---
>  OvmfPkg/OvmfPkgIa32X64.fdf | 27 +--
>  OvmfPkg/OvmfPkgX64.dsc | 75 +--
>  OvmfPkg/OvmfPkgX64.fdf | 27 +--
>  6 files changed, 102 insertions(+), 205 deletions(-)

It catches my eye that the IA32X64 dsc file has a different "changed
line count" from the others (76 vs. 75). And, indeed, there is a bug:

> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index f7f9ab06bb5a..995328992ccf 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf

> @@ -509,10 +519,10 @@ [PcdsFixedAtBuild]
>gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
>  !endif
>  
> -[PcdsFixedAtBuild.X64]
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
> -!endif
> +  #
> +  # Network Pcds
> +  #
> +!include NetworkPkg/NetworkPcds.dsc.inc
>  
>gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 
> 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
>  

(5) Please do not remove the [PcdsFixedAtBuild.X64] section header.

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 5/6] OvmfPkg: Update DSC/FDF to use NetworkPkg's include fragment file.

[Laszlo Ersek]

On 11/22/18 06:21, Fu Siyuan wrote:
> This patch updates the platform DSC/FDF files to use the include fragment
> files provided by NetworkPkg.
> The feature enabling flags in [Defines] section have been updated to use
> the NetworkPkg's terms, and the value has been overridden with the original
> default value on this platform.
>
> This patch also rename the TLS_ENABLE flag to PLATFORM_TLS_ENABLE for the
> platform specific configuration for TLS support.
>
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan 
> ---
>
> Notes:
> v2:
> Rename TLS_ENABLE flag to PLATFORM_TLS_ENABLE flag for platform specific 
> configuration for TLS support.
>
>  OvmfPkg/OvmfPkgIa32.dsc| 75 +--
>  OvmfPkg/OvmfPkgIa32.fdf| 27 +--
>  OvmfPkg/OvmfPkgIa32X64.dsc | 76 +---
>  OvmfPkg/OvmfPkgIa32X64.fdf | 27 +--
>  OvmfPkg/OvmfPkgX64.dsc | 75 +--
>  OvmfPkg/OvmfPkgX64.fdf | 27 +--
>  6 files changed, 102 insertions(+), 205 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index eccf34d3d1cb..adedd2240a8a 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -35,12 +35,25 @@ [Defines]
># -D FLAG=VALUE
>#
>DEFINE SECURE_BOOT_ENABLE  = FALSE
> -  DEFINE NETWORK_IP6_ENABLE  = FALSE
> -  DEFINE HTTP_BOOT_ENABLE= FALSE
>DEFINE SMM_REQUIRE = FALSE
> -  DEFINE TLS_ENABLE  = FALSE
>DEFINE TPM2_ENABLE = FALSE
>
> +  #
> +  # PLATFORM_TLS_ENABLE flag is used to control platform specific 
> configuration for TLS support,
> +  # which add a NULL class library instance to TlsAuthConfigDxe.inf for 
> downloading the necessary
> +  # data from QEMU via fw_cfg.
> +  #
> +  DEFINE PLATFORM_TLS_ENABLE= FALSE
> +  #
> +  # The NETWORK_TLS_ENABLE should always be set to FALSE since 
> PLATFORM_TLS_ENABLE is used.
> +  #
> +  DEFINE NETWORK_TLS_ENABLE = FALSE
> +  DEFINE NETWORK_IP6_ENABLE = FALSE
> +  DEFINE NETWORK_HTTP_BOOT_ENABLE   = FALSE
> +  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = TRUE
> +  DEFINE NETWORK_IPSEC_ENABLE   = FALSE
> +!include NetworkPkg/NetworkDefines.dsc.inc
> +

Perfect. Logically, this is exactly right.

One syntactic request:

(1) Can you please rewrap the -- otherwise spot-on -- explanation of
PLATFORM_TLS_ENABLE to 80 characters? Same for the NETWORK_TLS_ENABLE
explanation.

>#
># Flash size selection. Setting FD_SIZE_IN_KB on the command line directly 
> to
># one of the supported values, in place of any of the convenience macros, 
> is
> @@ -144,10 +157,6 @@ [LibraryClasses]
>FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
>UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
>
> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> -  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> -  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
> -  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> -  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
>UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>
> SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
>QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> @@ -173,7 +182,7 @@ [LibraryClasses]
>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>
>IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> -!if $(TLS_ENABLE) == TRUE
> +!if $(PLATFORM_TLS_ENABLE) == TRUE
>OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>  !else
>OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> @@ -191,11 +200,12 @@ [LibraryClasses]
>
>TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf

(2) Please remove the TcpIoLib resolution as well. It is provided by
"NetworkPkg/NetworkLibs.dsc.inc".

>
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> -!endif
> +  #
> +  # Network libraries
> +  #
> +!include NetworkPkg/NetworkLibs.dsc.inc
>
> -!if $(TLS_ENABLE) == TRUE
> +!if $(PLATFORM_TLS_ENABLE) == TRUE
>TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
>  !endif
>
> @@ -442,7 +452,7 @@ [PcdsFixedAtBuild]
>  !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> -!if $(TLS_ENABLE) == FALSE
> +!if $(PLATFORM_TLS_ENABLE) == FALSE
># match PcdFlashNvStorageVariableSize purely for convenience
>gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>  !endif
> @@ -450,12 +460,12 @@ [PcdsFixedAtBuild]
>  !if $(FD_SIZE_IN_KB) == 4096
>

Re: [edk2] [PATCH v2 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.

[Laszlo Ersek]

+Ard, +Leif

On 11/22/18 06:21, Fu Siyuan wrote:
> This patch provides a set of include segment files for platform owner to
> easily enable/disable network stack support on their platform.
> 
> For DSC, there are:
> - a "NetworkDefines.dsc.inc" for the [Defines] section(s),
> - a "NetworkLibs.dsc.inc" for the [LibraryClasses*] section(s),
> - a "NetworkPcds.dsc.inc" for the [Pcds*] section(s),
> - a "NetworkComponents.dsc.inc" for the [Components*] section(s).
> For FDF, there is:
> - a "Network.fdf.inc" for the [Fv*] section(s).
> 
> These files can be added to the platform DSC/FDF file by using
>   !include NetworkPkg/xxx
> where "xxx" is the *.inc file name.
> 
> A set of flags can be changed before the include line or in build command
> line ("-D FLAG=VALUE") to enable or disable related feature set, please
> check "NetworkDefines.dsc.inc" for a detail description of each flag.
> 
> The default value of these flags are:
>   DEFINE NETWORK_ENABLE = TRUE
>   DEFINE NETWORK_SNP_ENABLE = TRUE
>   DEFINE NETWORK_IP4_ENABLE = TRUE
>   DEFINE NETWORK_IP6_ENABLE = TRUE
>   DEFINE NETWORK_TLS_ENABLE = TRUE
>   DEFINE NETWORK_HTTP_BOOT_ENABLE   = TRUE
>   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
>   DEFINE NETWORK_IPSEC_ENABLE   = TRUE
>   DEFINE NETWORK_ISCSI_ENABLE   = TRUE
>   DEFINE NETWORK_VLAN_ENABLE= TRUE
> 
> Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1293
> 
> Cc: Jiaxin Wu 
> Cc: Ting Ye 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan 
> ---
> 
> Notes:
> v2:
> 1. Split the "Network.dsc.inc" in to 4 files for different sections in DSC
> file. This could provide more flexibility to platform owner to use the
> include files.
> 2. Clarify the OpenSSL dependency of TLS, iSCSI and IPsec enable flag.
> 3. Use "!error" statement for incorrect flag value check.
> 4. Other decoration work according to Laszlo's comments.
> 
>  NetworkPkg/Network.fdf.inc   |  69 ++
>  NetworkPkg/NetworkComponents.dsc.inc |  71 ++
>  NetworkPkg/NetworkDefines.dsc.inc| 138 
>  NetworkPkg/NetworkLibs.dsc.inc   |  25 
>  NetworkPkg/NetworkPcds.dsc.inc   |  22 
>  NetworkPkg/NetworkPkg.dsc|  28 +---
>  6 files changed, 331 insertions(+), 22 deletions(-)
> 
> diff --git a/NetworkPkg/Network.fdf.inc b/NetworkPkg/Network.fdf.inc
> new file mode 100644
> index ..abd4c6c363d5
> --- /dev/null
> +++ b/NetworkPkg/Network.fdf.inc
> @@ -0,0 +1,69 @@
> +## @file
> +# Network FDF include file for All Architectures.
> +#
> +# This file can be included to a platform FDF by using "!include 
> NetworkPkg/Network.fdf.inc"
> +# to add EDKII network stack drivers according to the value of flags 
> described in
> +# "NetworkDefines.dsc.inc".
> +#
> +# Copyright (c) 2018, Intel Corporation. All rights reserved.
> +#
> +#This program and the accompanying materials
> +#are licensed and made available under the terms and conditions of the 
> BSD License
> +#which accompanies this distribution. The full text of the license may 
> be found at
> +#http://opensource.org/licenses/bsd-license.php
> +#
> +#THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +!if $(NETWORK_ENABLE) == TRUE
> +  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> +
> +  !if $(NETWORK_SNP_ENABLE) == TRUE
> +INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> +  !endif
> +
> +  !if $(NETWORK_VLAN_ENABLE) == TRUE
> +INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> +  !endif
> +
> +  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> +
> +  !if $(NETWORK_IP4_ENABLE) == TRUE
> +INF  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> +INF  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> +INF  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> +INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> +INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> +  !endif
> +
> +  !if $(NETWORK_IP6_ENABLE) == TRUE
> +INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> +INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> +INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> +INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> +  !endif
> +
> +  INF  NetworkPkg/TcpDxe/TcpDxe.inf
> +  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> +
> +  !if $(NETWORK_TLS_ENABLE) == TRUE
> +INF  NetworkPkg/TlsDxe/TlsDxe.inf
> +INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> +  !endif
> +
> +  !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE
> +INF  NetworkPkg/DnsDxe/DnsDxe.inf
> +INF  NetworkPkg/HttpDxe/HttpDxe.inf
> +INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> +INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> +  !endif
> +
> +  !if 

Re: [edk2] [PATCH edk2-platforms 4/4] Platform/ARM/BdsLib: maintain alignment for DevicePaths

[Laszlo Ersek]

On 11/23/18 09:35, Ard Biesheuvel wrote:
> On Thu, 22 Nov 2018 at 19:35, Laszlo Ersek  wrote:

>> [...]

> Indeed. I am updating the second patch to get rid of everything in
> BdsLib we are not currently using.

Thanks!

> Looking at 56bed2f41022afcbadecc9f2d537bd31c3d44cbc ("^W never mind
> ...

heh :)

> the intent appears to be that device path struct members do appear
> naturally aligned, even if the size of the data structure is not a
> multiple of the max alignment we expect to encounter.
>
> Presumably, this is why CoreConnectController () does the same in this
> regard.

Yeah, that's certainly for working around bugs elsewhere in the
firmware. The UEFI spec 2.7 says in "10.3.1 Generic Device Path
Structures":

A Device Path is a series of generic Device Path nodes. The first
Device Path node starts at byte offset zero of the Device Path. The
next Device Path node starts at the end of the previous Device Path
node. Therefore all nodes are byte-packed data structures that may
appear on any byte boundary. *All code references to device path
notes must assume all fields are unaligned.*

(Emphasis mine. And yes, the exact sentence that I'm quoting this
section for contains a typo, s/notes/nodes/.)

[...]

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Display Architecture and Bring Up in UEFI

[Laszlo Ersek]

On 11/23/18 07:27, prabin ca wrote:
> Hi Team,
> 
> I’m new to UEFI and display interface in UEFI. I would like to have deep dive 
> into how display is working in UEFI (display architecture) and how display is 
> have been bring up (porting of display panel in a any platform in general ).
> 
> Please help me with sample codes and necessary documents. I would like to get 
> knowledge about display bring up and display architecture in UEFI

The driver writers' guide and the UEFI spec have relevant chapters on
this. I think it's best to start reading the former, at "23 Graphics
Driver Design Guidelines"; that part will give you the pointers to the
rest as well.

https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer%27s-Guide


For a (hopefully educational) example, I refer you to
OvmfPkg/VirtioGpuDxe. In the series that first added this driver to
edk2, I managed to construct the driver in stages such that each stage
would build and even function, at the level expected from that stage. In
particular, commit a2a4fa66701d ("OvmfPkg/VirtioGpuDxe: introduce with
Component Name 2 and Driver Binding", 2016-09-01) could prove helpful,
as it adds the skeleton of the driver, mostly without VirtIo GPU specifics.


In addition, you might want to look into the generic

  MdeModulePkg/Universal/Console/GraphicsOutputDxe

driver. A platform may be able to incorporate that driver without any
changes, and control it by first producing the two HOBs in the PEI phase
that the driver consumes:

  MdePkg/Include/Guid/GraphicsInfoHob.h

(... Interestingly, due to the fact that this header file is under
MdePkg and not MdeModulePkg, I've just learned, from the related commit
messages, that the PEI phase has standardized graphics support,
described in the PI spec. From the following two commit messages:

- 697c6cf32693 ("MdePkg: Add PI 1.4 Graphics HOB and PPI header files",
2015-04-28)

- 2af538fbf667 ("MdeModulePkg: Add GraphicsOutputDxe driver.", 2016-10-12)

it appears that enabling graphics support in the PEI phase could be a
*requirement* for using GraphicsOutputDxe in the DXE phase. That might
or might not match your use case, so perhaps it will prevent you from
using GraphicsOutputDxe. I'm not sure.)

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths

[Ard Biesheuvel]

DevicePath node types may have any size, and so it is up to the
code that manipulates them to ensure that dereferencing them only
occurs when the pointer is aligned explicitly.

Since BdsConnectAndUpdateDevicePath() has only two callers, one of
which itself, we can simply duplicate the device path (similar to
how DxeCore's CoreConnectController () does it), and free the pool
allocation again on the way out. (Note that the allocation only
occurs when the non-recursive path is taken and the function
returns EFI_SUCCESS)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/ARM/Library/BdsLib/BdsFilePath.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c 
b/Platform/ARM/Library/BdsLib/BdsFilePath.c
index 62f796e5526d..ad66b2f82718 100644
--- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
+++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
@@ -423,8 +423,8 @@ BdsConnectAndUpdateDevicePath (
 }
   }
 
-  if (RemainingDevicePath) {
-*RemainingDevicePath = Remaining;
+  if (!EFI_ERROR (Status) && RemainingDevicePath != NULL) {
+*RemainingDevicePath = DuplicateDevicePath (Remaining);
   }
 
   return Status;
@@ -1314,14 +1314,18 @@ BdsLoadImageAndUpdateDevicePath (
   }
 
   FileLoader = FileLoaders;
+  Status = EFI_UNSUPPORTED;
   while (FileLoader->Support != NULL) {
 if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {
-  return FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath, 
Type, Image, FileSize);
+  Status = FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath,
+ Type, Image, FileSize);
+  break;
 }
 FileLoader++;
   }
 
-  return EFI_UNSUPPORTED;
+  FreePool (RemainingDevicePath);
+  return Status;
 }
 
 EFI_STATUS
-- 
2.17.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions

[Ard Biesheuvel]

Clean up BdsLib (which is deprecated and should not be used for future
development) by removing all the pieces that are not being used at the
moment.

After this patch, only BdsLoadImage() remains, and the pieces it relies
upon. This function is used by FdtPlatformDxe to load device tree
binaries from device paths.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/ARM/Include/Library/BdsLib.h   | 186 -
 Platform/ARM/Library/BdsLib/BdsAppLoader.c  | 253 --
 Platform/ARM/Library/BdsLib/BdsFilePath.c   |  83 +-
 Platform/ARM/Library/BdsLib/BdsHelper.c | 122 -
 Platform/ARM/Library/BdsLib/BdsInternal.h   |  15 +-
 Platform/ARM/Library/BdsLib/BdsLib.inf  |   2 -
 Platform/ARM/Library/BdsLib/BdsLoadOption.c | 272 
 7 files changed, 13 insertions(+), 920 deletions(-)

diff --git a/Platform/ARM/Include/Library/BdsLib.h 
b/Platform/ARM/Include/Library/BdsLib.h
index 4528c2e8739b..eab868439207 100644
--- a/Platform/ARM/Include/Library/BdsLib.h
+++ b/Platform/ARM/Include/Library/BdsLib.h
@@ -15,150 +15,6 @@
 #ifndef __BDS_ENTRY_H__
 #define __BDS_ENTRY_H__
 
-#define IS_DEVICE_PATH_NODE(node,type,subtype)\
-(((node)->Type == (type)) && ((node)->SubType == (subtype)))
-
-/**
-  This is defined by the UEFI specs, don't change it
-**/
-typedef struct {
-  UINT16  LoadOptionIndex;
-  EFI_LOAD_OPTION *LoadOption;
-  UINTN   LoadOptionSize;
-
-  UINT32  Attributes;
-  UINT16  FilePathListLength;
-  CHAR16  *Description;
-  EFI_DEVICE_PATH_PROTOCOL*FilePathList;
-
-  VOID*   OptionalData;
-  UINTN   OptionalDataSize;
-} BDS_LOAD_OPTION;
-
-/**
-  Connect a Device Path and return the handle of the driver that support this 
DevicePath
-
-  @param  DevicePathDevice Path of the File to connect
-  @param  HandleHandle of the driver that support this 
DevicePath
-  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match 
the driver DevicePath
-
-  @retval EFI_SUCCESS   A driver that matches the Device Path has been 
found
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
-
-**/
-EFI_STATUS
-BdsConnectDevicePath (
-  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
-  OUT EFI_HANDLE*Handle,
-  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
-  );
-
-/**
-  Connect all DXE drivers
-
-  @retval EFI_SUCCESS   All drivers have been connected
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the 
matching results.
-
-**/
-EFI_STATUS
-BdsConnectAllDrivers (
-  VOID
-  );
-
-/**
-  Return the value of a global variable defined by its VariableName.
-  The variable must be defined with the VendorGuid gEfiGlobalVariableGuid.
-
-  @param  VariableName  A Null-terminated string that is the name of 
the vendor's
-variable.
-  @param  DefaultValue  Value returned by the function if the variable 
does not exist
-  @param  DataSize  On input, the size in bytes of the return Data 
buffer.
-On output the size of data returned in Data.
-  @param  Value Value read from the UEFI Variable or copy of 
the default value
-if the UEFI Variable does not exist
-
-  @retval EFI_SUCCESS   All drivers have been connected
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the 
matching results.
-
-**/
-EFI_STATUS
-GetGlobalEnvironmentVariable (
-  IN CONST CHAR16*   VariableName,
-  IN VOID*   DefaultValue,
-  IN OUT UINTN*  Size,
-  OUTVOID**  Value
-  );
-
-/**
-  Return the value of the variable defined by its VariableName and VendorGuid
-
-  @param  VariableName  A Null-terminated string that is the name of 
the vendor's
-variable.
-  @param  VendorGuidA unique identifier for the vendor.
-  @param  DefaultValue  Value returned by the function if the variable 
does not exist
-  @param  DataSize  On input, the size in bytes of the return Data 
buffer.
-On output the size of data returned in Data.
-  @param  Value Value read from the UEFI Variable or copy of 
the default value
-if the UEFI Variable does not exist
-
-  @retval EFI_SUCCESS   All drivers have been connected
-  @retval EFI_NOT_FOUND No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the 

[edk2] [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib

[Ard Biesheuvel]

The deprecated BdsLib library class in ArmPkg is still depended upon, but
only a single implementation exists, which now resides in edk2-platforms.

This implementation has some issues in how it deals with Device Paths,
so let's fix those, but first move over the library interface declaration
and get rid of the parts that are no longer used. This will permit dropping
it from ArmPkg in EDK2.

Changes since v1:
- add Laszlo's ack to #1
- update #2 to remove everything we no longer need from BdsLib
- drop #3 which was bogus
- update #4 to ensure that we only duplicate the device path when we
  are about to return EFI_SUCCESS

Ard Biesheuvel (3):
  Platform/ARM: import ARM platform specific BdsLib header
  Platform/ARM/BdsLib: drop unused functions
  Platform/ARM/BdsLib: maintain alignment for DevicePaths

 Platform/ARM/ARM.dec  |   3 +
 .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
 Platform/ARM/Include/Library/BdsLib.h |  26 ++
 Platform/ARM/Library/BdsLib/BdsAppLoader.c| 253 
 Platform/ARM/Library/BdsLib/BdsFilePath.c |  95 +-
 Platform/ARM/Library/BdsLib/BdsHelper.c   | 122 
 Platform/ARM/Library/BdsLib/BdsInternal.h |  16 +-
 Platform/ARM/Library/BdsLib/BdsLib.inf|   4 +-
 Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 --
 9 files changed, 52 insertions(+), 741 deletions(-)
 create mode 100644 Platform/ARM/Include/Library/BdsLib.h
 delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
 delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c

-- 
2.17.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 edk2-platforms 1/3] Platform/ARM: import ARM platform specific BdsLib header

[Ard Biesheuvel]

The BdsLib library has been moved into Platform/ARM a while ago,
but the BdsLib.h header was left behind, and so all users in
Platform/ARM are still relying on it to be available in ArmPkg.

So let's add a copy to Platform/ARM and wire it up, so we can
drop it from ArmPkg going forward. Note that the BdsLib
implementation included ArmLib.h from ArmPkg without using
anything it provides, so drop that false dependency as well.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Acked-by: Laszlo Ersek 
---
 Platform/ARM/ARM.dec   |   3 +
 Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
 Platform/ARM/Include/Library/BdsLib.h  | 212 

 Platform/ARM/Library/BdsLib/BdsInternal.h  |   1 -
 Platform/ARM/Library/BdsLib/BdsLib.inf |   2 +-
 5 files changed, 217 insertions(+), 3 deletions(-)

diff --git a/Platform/ARM/ARM.dec b/Platform/ARM/ARM.dec
index f9bf3294a0ca..6a6eeb6559fd 100644
--- a/Platform/ARM/ARM.dec
+++ b/Platform/ARM/ARM.dec
@@ -21,5 +21,8 @@
 [Includes]
   Include# Root include for the package
 
+[LibraryClasses]
+  BdsLib|Include/Library/BdsLib.h
+
 [Guids]
   gArmBootMonFsFileInfoGuid   = { 0x41e26b9c, 0xada6, 0x45b3, { 0x80, 0x8e, 
0x23, 0x57, 0xa3, 0x5b, 0x60, 0xd6 } }
diff --git a/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf 
b/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf
index f9a5aee3596e..d4aef4bfce92 100644
--- a/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf
+++ b/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf
@@ -28,10 +28,10 @@
   ShellSetFdt.c
 
 [Packages]
-  ArmPkg/ArmPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
+  Platform/ARM/ARM.dec
   Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.dec
   ShellPkg/ShellPkg.dec
 
diff --git a/Platform/ARM/Include/Library/BdsLib.h 
b/Platform/ARM/Include/Library/BdsLib.h
new file mode 100644
index ..4528c2e8739b
--- /dev/null
+++ b/Platform/ARM/Include/Library/BdsLib.h
@@ -0,0 +1,212 @@
+/** @file
+*
+*  Copyright (c) 2013-2015, ARM Limited. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD 
License
+*  which accompanies this distribution.  The full text of the license may be 
found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#ifndef __BDS_ENTRY_H__
+#define __BDS_ENTRY_H__
+
+#define IS_DEVICE_PATH_NODE(node,type,subtype)\
+(((node)->Type == (type)) && ((node)->SubType == (subtype)))
+
+/**
+  This is defined by the UEFI specs, don't change it
+**/
+typedef struct {
+  UINT16  LoadOptionIndex;
+  EFI_LOAD_OPTION *LoadOption;
+  UINTN   LoadOptionSize;
+
+  UINT32  Attributes;
+  UINT16  FilePathListLength;
+  CHAR16  *Description;
+  EFI_DEVICE_PATH_PROTOCOL*FilePathList;
+
+  VOID*   OptionalData;
+  UINTN   OptionalDataSize;
+} BDS_LOAD_OPTION;
+
+/**
+  Connect a Device Path and return the handle of the driver that support this 
DevicePath
+
+  @param  DevicePathDevice Path of the File to connect
+  @param  HandleHandle of the driver that support this 
DevicePath
+  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match 
the driver DevicePath
+
+  @retval EFI_SUCCESS   A driver that matches the Device Path has been 
found
+  @retval EFI_NOT_FOUND No handles match the search.
+  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
+
+**/
+EFI_STATUS
+BdsConnectDevicePath (
+  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
+  OUT EFI_HANDLE*Handle,
+  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
+  );
+
+/**
+  Connect all DXE drivers
+
+  @retval EFI_SUCCESS   All drivers have been connected
+  @retval EFI_NOT_FOUND No handles match the search.
+  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the 
matching results.
+
+**/
+EFI_STATUS
+BdsConnectAllDrivers (
+  VOID
+  );
+
+/**
+  Return the value of a global variable defined by its VariableName.
+  The variable must be defined with the VendorGuid gEfiGlobalVariableGuid.
+
+  @param  VariableName  A Null-terminated string that is the name of 
the vendor's
+variable.
+  @param  DefaultValue  Value returned by the function if the variable 
does not exist
+  @param  DataSize  On input, the size in bytes of the return Data 
buffer.
+On output the size of data returned in Data.

Re: [edk2] [PATCH edk2-platforms 0/4] Platform/ARM: fix DevicePath mishandling in BdsLib

[Ard Biesheuvel]

On Fri, 23 Nov 2018 at 05:20, Thomas Abraham  wrote:
>
> On Thu, Nov 22, 2018 at 10:56 PM Ard Biesheuvel
>  wrote:
> >
> > The deprecated BdsLib library class in ArmPkg is still depended upon, but
> > only a single implementation exists, which now resides in edk2-platforms.
> >
> > This implementation has some issues in how it deals with Device Paths,
> > so let's fix those, but first move over the library interface declaration.
> > This will permit dropping it from ArmPkg in EDK2.
> >
> > Ard Biesheuvel (4):
> >   Platform/ARM: import ARM platform specific BdsLib header
> >   Platform/ARM/BdsLid: drop unused BdsStartEfiApplication ()
> >   Platform/ARM/BdsLib: don't clobber BdsLoadImage() DevicePath IN param
> >   Platform/ARM/BdsLib: maintain alignment for DevicePaths
>
> This patch series when applied with the below two patch series
> - [PATCH v2 0/5] ArmPlatformPkg, ArmVirtPkg: discover NOR flash banks from DTB
> - [PATCH edk2-platforms 0/3] drop GUIDs from NOR flash bank descriptors
>
> does solve the boot issue on the Juno board.

Wonderful! Thanks for confirming.

> >
> >  Platform/ARM/ARM.dec  |   3 +
> >  .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
> >  Platform/ARM/Include/Library/BdsLib.h | 193 ++
> >  Platform/ARM/Library/BdsLib/BdsFilePath.c |  71 ++-
> >  Platform/ARM/Library/BdsLib/BdsInternal.h |   1 -
> >  Platform/ARM/Library/BdsLib/BdsLib.inf|   2 +-
> >  6 files changed, 212 insertions(+), 60 deletions(-)
> >  create mode 100644 Platform/ARM/Include/Library/BdsLib.h
> >
> > --
> > 2.17.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms 4/4] Platform/ARM/BdsLib: maintain alignment for DevicePaths

[Ard Biesheuvel]

On Thu, 22 Nov 2018 at 19:35, Laszlo Ersek  wrote:
>
> On 11/22/18 18:26, Ard Biesheuvel wrote:
> > DevicePath node types may have any size, and so it is up to the
> > code that manipulates them to ensure that dereferencing them only
> > occurs when the pointer is aligned explicitly.
> >
> > Since BdsConnectAndUpdateDevicePath() has only two callers,
>
> at d9e68a756cfb ("Platform/ARM/SgiPkg: increase max variable size to
> 8KB", 2018-11-20), it seems to have three callers:
>
> - itself
> - BdsConnectDevicePath()
> - BdsLoadImageAndUpdateDevicePath()
>

Indeed. I am updating the second patch to get rid of everything in
BdsLib we are not currently using.

> > one of
> > which itself, we can simply duplicate the device path (similar to
> > how DxeCore's CoreConnectController () does it), and free the pool
> > allocation again on the way out. (Note that the allocation only
> > occurs when the non-recursive path is taken)
>
> I think this rather works around than fixes the problem -- just because
> every remaining device path "slice" is realigned as we advance, it's not
> guaranteed that any and all CHAR16 fields in the now-first node will be
> naturally aligned.
>
> ... However, it certainly applies to FILEPATH_DEVICE_PATH.PathName,
> which is likely the only such field that we care about. :)
>

Looking at 56bed2f41022afcbadecc9f2d537bd31c3d44cbc ("^W never mind ...
the intent appears to be that device path struct members do appear
naturally aligned, even if the size of the data structure is not a
multiple of the max alignment we expect to encounter.

Presumably, this is why CoreConnectController () does the same in this regard.

> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  Platform/ARM/Library/BdsLib/BdsFilePath.c | 10 +++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c 
> > b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> > index 74fdbbee773d..543ac8f83086 100644
> > --- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
> > +++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> > @@ -421,7 +421,7 @@ BdsConnectAndUpdateDevicePath (
> >}
> >
> >if (RemainingDevicePath) {
> > -*RemainingDevicePath = Remaining;
> > +*RemainingDevicePath = DuplicateDevicePath (Remaining);
> >}
> >
> >return Status;
>
> OK, so this makes BdsConnectAndUpdateDevicePath()'s RemainingDevicePath
> output param dynamically allocated. And this change works fine with the
> recursive logic too, as you say in the commit message.
>

Yep.

> > @@ -1333,14 +1333,18 @@ BdsLoadImageAndUpdateDevicePath (
> >}
>
> We already need some error handling here. The control flow in
> BdsConnectAndUpdateDevicePath() boggles my mind a bit, but I think it
> can output a dynamically allocated RemainingDevicePath *and* return an
> error.
>
> Namely, assume that TryRemovableDevice() is reached, and it fails.
>

That doesn't make sense. I'll update that routine to only do the clone
if it returns EFI_SUCCESS.

> So, I think we should add an error handling label
> ("FreeRemainingDevicePath"), and jump to it, from both first "return"
> statements in this function.
>
> Also, we should likely set RemainingDevicePath to NULL at the top of the
> function, and check it at the end, because... ugh...
> BdsConnectAndUpdateDevicePath() might also fail without assigning
> *RemainingDevicePath?
>

The above change should fix that as well afaict.

> >
> >FileLoader = FileLoaders;
> > +  Status = EFI_UNSUPPORTED;
> >while (FileLoader->Support != NULL) {
> >  if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {
> > -  return FileLoader->LoadImage (DevicePath, Handle, 
> > RemainingDevicePath, Type, Image, FileSize);
> > +  Status = FileLoader->LoadImage (DevicePath, Handle, 
> > RemainingDevicePath,
> > + Type, Image, FileSize);
> > +  break;
> >  }
> >  FileLoader++;
> >}
> >
> > -  return EFI_UNSUPPORTED;
> > +  FreePool (RemainingDevicePath);
> > +  return Status;
> >  }
> >
> >  EFI_STATUS
> >
>
> As I mention near the commit message, BdsConnectDevicePath() is not
> updated. Is that OK? ... Oh wait, BdsConnectDevicePath() is not called
> by anything. Append another patch to drop it, like
> BdsStartEfiApplication()? Then this patch will be fine, assuming you add
> the "goto"s.
>
> Thanks!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel