Re: [edk2] [platforms: PATCH 6/8] Marvell/Armada: Enable dynamic DRAM size detection

2017-10-11 Thread Marcin Wojtas
2017-10-11 19:56 GMT+02:00 Leif Lindholm :
> On Wed, Oct 11, 2017 at 05:40:47PM +0200, Marcin Wojtas wrote:
>> Instead of using hardcoded value in PcdSystemMemorySize PCD,
>> obtain DRAM size directly from SoC registers, which are filled
>> by firmware during early initialization stage.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas 
>> ---
>>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c | 111 
>> +++-
>>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h |  49 
>> +
>>  2 files changed, 159 insertions(+), 1 deletion(-)
>>
>> diff --git 
>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c 
>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>> index 2cb2e15..22cbe47 100644
>> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>> @@ -36,8 +36,11 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
>> DAMAGE.
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>
>> +#include "Armada70x0LibMem.h"
>> +
>>  // The total number of descriptors, including the final "end-of-table" 
>> descriptor.
>>  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 16
>>
>> @@ -47,6 +50,105 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
>> DAMAGE.
>>
>>  STATIC ARM_MEMORY_REGION_DESCRIPTOR 
>> VirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
>>
>> +// Obtain DRAM size basing on register values filled by early firmware.
>> +STATIC
>> +UINT64
>> +DramSizeGet (
>
> GetDramSize?
>
>> +  UINT64 *MemSize
>
> IN, OUT?
>

2x OK.

>> +  )
>> +{
>> +  UINT64 BaseAddr;
>> +  UINT32 RegVal;
>> +  UINT8 AreaLengthMap;
>> +  UINT8 Cs;
>> +
>> +  *MemSize = 0;
>> +
>> +  for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) {
>> +
>> +RegVal = MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs));
>> +
>> +/* Exit loop on first disabled DRAM CS */
>> +if (!(RegVal & DRAM_CS_VALID_ENABLED_MASK)) {
>> +  break;
>> +}
>> +
>> +/*
>> + * Sanity check for base address of next DRAM block.
>> + * Only continuous space will be used.
>> + */
>> +BaseAddr = ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG (Cs)) <<
>> +DRAM_START_ADDR_HTOL_OFFS) | (RegVal & 
>> DRAM_START_ADDRESS_L_MASK);
>
> Please use macros, temporary variables, more parentheses or whatever
> to help make the above operation readable.
>

Sure.

>> +if (BaseAddr != *MemSize) {
>> +  DEBUG ((DEBUG_ERROR,
>> +"DramSizeGet: DRAM blocks are not contiguous, limit size to 
>> 0x%llx\n",
>> +*MemSize));
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/* Decode area length for current CS from register value */
>> +AreaLengthMap = ((RegVal & DRAM_AREA_LENGTH_MASK) >> 
>> DRAM_AREA_LENGTH_OFFS);
>> +switch (AreaLengthMap) {
>
> Why Map?
>

I stuck to mv-ddr convention, but I agree, 'AreaLength' will be better.

>> +case 0x0:
>> +  *MemSize += 0x1800;
>> +  break;
>> +case 0x1:
>> +  *MemSize += 0x3000;
>> +  break;
>> +case 0x2:
>> +  *MemSize += 0x6000;
>> +  break;
>> +case 0x3:
>> +  *MemSize += 0xC000;
>> +  break;
>
> The above ones look if not devoid of pattern, at least a bit
> unexpected - and 4-6 are comlpetely missing. Is there any
> documentation available for me to read to try to understand what is
> going on?

Do you have a docs for armada 7k or 8k? Please check 0xf0020200 table,
bits 20:16. You are right, I missed '4' (6GB), but 5 and 6 are
reserved (no value possible).

>
> The below all look predictably formatted and possible to calculate
> rather than list one by one.
>
> (1 << ((AreaLengthMap + 16))) if a quick calculation serves me right.

I think we can reduce the switch-case to this:

if  (AreaLength =< 0x4) {
  *MemSize = 0x1800 << AreaLength;
}  else if (AreaLength >= 0x7 && AreaLength < 0x1B)
  *MemSize = 1 << (AreaLengthMap + 16);
} else {
  DEBUG ((DEBUG_ERROR, "Invalid memory module size (0x%x) for
CS#%d\n", AreaLength, Cs));
}

Is above ok for you?

>
>> +case 0x7:
>> +  *MemSize += 0x0080;
>> +  break;
>> +case 0x8:
>> +  *MemSize += 0x0100;
>> +  break;
>> +case 0x9:
>> +  *MemSize += 0x0200;
>> +  break;
>> +case 0xA:
>> +  *MemSize += 0x0400;
>> +  break;
>> +case 0xB:
>> +  *MemSize += 0x0800;
>> +  break;
>> +case 0xC:
>> +  *MemSize += 0x1000;
>> +  break;
>> +case 0xD:
>> +  *MemSize += 0x2000;
>> +  break;
>> +case 0xE:
>> +  *MemSize += 0x4000;
>> +  break;
>> +case 0xF:
>> +  *MemSize += 0x8000;
>> +  break;
>> +case 0x10:
>> +  *MemSize += 0x1;
>> +  break;
>> +case 0x11:
>> +  *MemSize += 0x2;
>> +  break;
>> +case 0x12:
>> +  

Re: [edk2] [Patch] MdeModulePkg: Update RuntimeDxe Crc32 to check the input parameter

2017-10-11 Thread Wu, Hao A
Reviewed-by: Hao Wu 


Best Regards,
Hao Wu


> -Original Message-
> From: Gao, Liming
> Sent: Thursday, October 12, 2017 12:26 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [Patch] MdeModulePkg: Update RuntimeDxe Crc32 to check the input
> parameter
> 
> This is the regression issue. After apply CalculateCrc32(), the parameter
> check is missing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao 
> Cc: Wu Hao A 
> ---
>  MdeModulePkg/Core/RuntimeDxe/Crc32.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/RuntimeDxe/Crc32.c
> b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
> index 3e91e08..c271856 100644
> --- a/MdeModulePkg/Core/RuntimeDxe/Crc32.c
> +++ b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
> @@ -42,6 +42,10 @@ RuntimeDriverCalculateCrc32 (
>OUT UINT32  *CrcOut
>)
>  {
> +  if (Data == NULL || DataSize == 0 || CrcOut == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>*CrcOut = CalculateCrc32 (Data, DataSize);
>return EFI_SUCCESS;
>  }
> --
> 2.8.0.windows.1

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


Re: [edk2] [platforms: PATCH 4/8] Marvell/Armada: Add support from DRAM remapping

2017-10-11 Thread Marcin Wojtas
Hi Leif,

2017-10-11 20:18 GMT+02:00 Ard Biesheuvel :
> On 11 October 2017 at 18:08, Leif Lindholm  wrote:
>> Subject: from->for ?
>>
>> On Wed, Oct 11, 2017 at 05:40:45PM +0200, Marcin Wojtas wrote:
>>> From: Ard Biesheuvel 
>>>
>>> The Armada 70x0/80x0 DRAM controller allows a single window of DRAM
>>> to be remapped to another location in the physical address space. This
>>> allows us to free up some memory in the 32-bit addressable region for
>>> peripheral MMIO and PCI MMIO32 and CONFIG spaces. This patch adjusts
>>> memory blocks to the configuration done in ATF.
>>
>> The ARM Trusted Firmware developers tend to frown at that
>> abbreviation. Please use ARM-TF in official context.

Ok.

>>
>> Which configuration is this? Presumably, if this was changed in
>> ARM-TF, we would need to change it this end too, so does not hurt to
>> be explicit. (It uses a FixedPcd, so clearly it is not dynamically
>> detected, which the commit message can be read as.)

I checked and I can modify the code, to obtain from registers an
information about source/target and whether Dram remap is enabled at
all. This way we would be immune to any further changes in ARM-TF.
However I have a question to that setting, please see below.

>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel 
>>> Signed-off-by: Marcin Wojtas 
>>> ---
>>>  Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
>>> | 15 +
>>>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf   
>>> |  3 +
>>>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c  
>>> | 60 
>>>  Platform/Marvell/Marvell.dec  
>>> | 13 +
>>>  4 files changed, 81 insertions(+), 10 deletions(-)
>>>
>>> diff --git 
>>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
>>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
>>> index 72f8cfc..c5be1a9 100644
>>> --- 
>>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
>>> +++ 
>>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
>>> @@ -17,6 +17,21 @@
>>>
>>>  ASM_FUNC(ArmPlatformPeiBootAction)
>>>mov   x29, xzr
>>> +
>>> +  .if   FixedPcdGet64 (PcdSystemMemoryBase) != 0
>>> +  .err  PcdSystemMemoryBase should be 0x0 on this platform!
>>> +  .endif
>>> +
>>> +  .if   FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 
>>> (PcdDramRemapTarget)
>>> +//
>>> +// Use the low range for UEFI itself. The remaining memory will be 
>>> mapped
>>> +// and added to the GCD map later.
>>> +//
>>> +adr   x0, mSystemMemoryEnd
>>> +MOV64 (x1, FixedPcdGet32 (PcdDramRemapTarget) - 1)
>>> +str   x1, [x0]
>>> +  .endif
>>> +

The original commit, due lack of dram size detection, increased total
memory to 4GB, which required above modification with limiting
mSystemMemoryEnd to dram target (0xc000). Now PcdSystemMemorySize
is left at somewhat lowest reasonable value (1GB), so above code is in
fact never executed. Are you ok that
I remove it and leave mSystemMemoryEnd to be set as-is in PrePi.c:

UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +
  FixedPcdGet64(PcdSystemMemorySize) - 1;

?

>>>ret
>>>
>>>  //UINTN
>>> diff --git 
>>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf 
>>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>>> index 2e198c3..838a670 100644
>>> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>>> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>>> @@ -67,5 +67,8 @@
>>>gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
>>>gArmTokenSpaceGuid.PcdArmPrimaryCore
>>>
>>> +  gMarvellTokenSpaceGuid.PcdDramRemapSize
>>> +  gMarvellTokenSpaceGuid.PcdDramRemapTarget
>>> +
>>>  [Ppis]
>>>gArmMpCoreInfoPpiGuid
>>> diff --git 
>>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c 
>>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>>> index 74c9956..2cb2e15 100644
>>> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>>> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>>> @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
>>> DAMAGE.
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>
>>>  // The total number of descriptors, including the final "end-of-table" 
>>> descriptor.
>>> @@ -44,6 +45,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
>>> DAMAGE.
>>>  #define DDR_ATTRIBUTES_CACHED   
>>> ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK
>>>  #define DDR_ATTRIBUTES_UNCACHED 
>>> ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED
>>>
>>> 

[edk2] [Patch] MdeModulePkg: Update RuntimeDxe Crc32 to check the input parameter

2017-10-11 Thread Liming Gao
This is the regression issue. After apply CalculateCrc32(), the parameter
check is missing.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Wu Hao A 
---
 MdeModulePkg/Core/RuntimeDxe/Crc32.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/MdeModulePkg/Core/RuntimeDxe/Crc32.c 
b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
index 3e91e08..c271856 100644
--- a/MdeModulePkg/Core/RuntimeDxe/Crc32.c
+++ b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
@@ -42,6 +42,10 @@ RuntimeDriverCalculateCrc32 (
   OUT UINT32  *CrcOut
   )
 {
+  if (Data == NULL || DataSize == 0 || CrcOut == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
   *CrcOut = CalculateCrc32 (Data, DataSize);
   return EFI_SUCCESS;
 }
-- 
2.8.0.windows.1

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


Re: [edk2] [Patch v3 0/3] Add SmmEndOfS3Resume event.

2017-10-11 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 4:22 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yao, Jiewen 
> Subject: [Patch v3 0/3] Add SmmEndOfS3Resume event.
> 
> This patch series add new SmmEndOfS3Resume event which required by some
> SMM drivers.
> It will implmented by SmmCore to install the
> gEdkiiSmmEndOfS3ResumeProtocolGuid
> Protocol. Smm drivers can install this protocol's notification functions to
> hoot this envet.
> It will be trigged right after the EndOfPei event in S3 resume phase.
> 
> V2 Changes:
>   Only change patch 2/3
>   1. Change structures name to avoid they start with EFI_.
>   2. Base on DXE phase bits to provide communication buffer, current implement
>  check both PEI and DXE phase.
> 
> V3 Changes:
>   for 2/3 patch:UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to
> SmmCore.
> 1. Change structure name for better understanding.
> 2. Enhance communication buffer calculate logic to more accurate.
> 
>   for 3/3 patch: MdeModulePkg/PiSmmCore: Install Protocol when S3 resume
> finished.
> 1. Uninstall the protocol right after install it to avoid run out of 
> memory.
> 
> Cc: Ruiyu Ni 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> 
> Eric Dong (3):
>   MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
>   UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
>   MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.
> 
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c| 55
> --
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h| 24 ++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |  1 +
>  MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h   | 31 
>  MdeModulePkg/MdeModulePkg.dec  |  3 +
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 85
> ++
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
>  7 files changed, 196 insertions(+), 7 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
> 
> --
> 2.7.0.windows.1

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


[edk2] [PATCH] SecurityPkg/Pkcs7Verify: Add the comments to address security problem

2017-10-11 Thread Long Qin
Add the comments to address security problems in the Pkcs7Verify Protocol
per UEFI 2.7 updates.

The Pkcs7Verifier function VerifySignature() has problematic use cases
where it might be used to unwittingly bypass security checks.  The specific
problem is that if the supplied hash is a different algorithm from the
blacklist hash, the hash will be approved even if it should have been
denied. The added comments place a strong warning about the problem.
It is possible to use the protocol reliably, either by agreeing a hash to
use for all time (like sha256) or by looping over all supported hashes when
using the protocol.

Cc: Chao Zhang 
Cc: James Bottomley 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 MdePkg/Include/Protocol/Pkcs7Verify.h   | 10 +-
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c |  8 
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Protocol/Pkcs7Verify.h 
b/MdePkg/Include/Protocol/Pkcs7Verify.h
index ca5ec75910..eaeda48300 100644
--- a/MdePkg/Include/Protocol/Pkcs7Verify.h
+++ b/MdePkg/Include/Protocol/Pkcs7Verify.h
@@ -6,7 +6,7 @@
   PKCS#7 is a general-purpose cryptographic standard (defined by RFC2315,
   available at http://tools.ietf.org/html/rfc2315).
 
-Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2017, 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 that accompanies this distribution.
 The full text of the license may be found at
@@ -140,6 +140,14 @@ EFI_STATUS
   verifies the signature of the content is valid and signing certificate was 
not revoked
   and is contained within a list of trusted signers.
 
+  Note: because this function uses hashes and the specification contains a 
variety of
+hash choices, you should be aware that the check against the RevokedDb 
list
+will improperly succeed if the signature is revoked using a different 
hash
+algorithm.  For this reason, you should either cycle through all UEFI 
supported
+hashes to see if one is forbidden, or rely on a single hash choice 
only if the
+UEFI signature authority only signs and revokes with a single hash (at 
time
+of writing, this hash choice is SHA256).
+
   @param[in] This Pointer to EFI_PKCS7_VERIFY_PROTOCOL 
instance.
   @param[in] SignaturePoints to buffer containing ASN.1 
DER-encoded PKCS
   detached signature.
diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c 
b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 0da549a6bd..ac83e6d5c2 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -1321,6 +1321,14 @@ _Exit:
   verifies the signature of the content is valid and signing certificate was 
not revoked
   and is contained within a list of trusted signers.
 
+  Note: because this function uses hashes and the specification contains a 
variety of
+hash choices, you should be aware that the check against the RevokedDb 
list
+will improperly succeed if the signature is revoked using a different 
hash
+algorithm.  For this reason, you should either cycle through all UEFI 
supported
+hashes to see if one is forbidden, or rely on a single hash choice 
only if the
+UEFI signature authority only signs and revokes with a single hash (at 
time
+of writing, this hash choice is SHA256).
+
   @param[in] This Pointer to EFI_PKCS7_VERIFY_PROTOCOL 
instance.
   @param[in] SignaturePoints to buffer containing ASN.1 
DER-encoded PKCS
   detached signature.
-- 
2.14.1.windows.1

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


Re: [edk2] [PATCH v4 5/6] IntelFrameworkModulePkg/Csm: Add code to bypass NULL pointer detection

2017-10-11 Thread Wang, Jian J
Thanks for catching this issue. Patch has been sent out. 

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, October 12, 2017 5:30 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Wolman, Ayellet
> ; Yao, Jiewen ; Dong, Eric
> ; Zeng, Star 
> Subject: Re: [edk2] [PATCH v4 5/6] IntelFrameworkModulePkg/Csm: Add code
> to bypass NULL pointer detection
> 
> This patch breaks the GCC5 build:
> 
> On 10/09/17 16:17, Jian J Wang wrote:
> 
> > diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> > index 3d9a8b9649..f42c13cd89 100644
> > --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> > +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> > @@ -57,7 +57,11 @@ LegacyBiosInt86 (
> >IN  EFI_IA32_REGISTER_SET *Regs
> >)
> >  {
> > -  UINT32  *VectorBase;
> > +  UINT16Segment;
> > +  UINT16Offset;
> > +  LEGACY_BIOS_INSTANCE  *Private;
> > +
> > +  Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
> >
> >Regs->X.Flags.Reserved1 = 1;
> >Regs->X.Flags.Reserved2 = 0;
> > @@ -72,12 +76,15 @@ LegacyBiosInt86 (
> >// The base address of legacy interrupt vector table is 0.
> >// We use this base address to get the legacy interrupt handler.
> >//
> > -  VectorBase  = 0;
> > +  DisableNullDetection ();
> > +  Segment   = (UINT16)(((UINT32 *)0)[BiosInt] >> 16);
> > +  Offset= (UINT16)((UINT32 *)0)[BiosInt];
> > +  EnableNullDetection ();
> >
> >return InternalLegacyBiosFarCall (
> > This,
> > -   (UINT16) ((VectorBase)[BiosInt] >> 16),
> > -   (UINT16) (VectorBase)[BiosInt],
> > +   Segment,
> > +   Offset,
> > Regs,
> > >X.Flags,
> > sizeof (Regs->X.Flags)
> 
> IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c: In function
> 'LegacyBiosInt86':
> IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c:62:26: error: variable
> 'Private' set but not used [-Werror=unused-but-set-variable]
>LEGACY_BIOS_INSTANCE  *Private;
>   ^~~
> cc1: all warnings being treated as errors
> 
> Can you please send a patch?
> 
> Thanks!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] IntelFrameworkModulePkg/LegacyBiosDxe: Fix GCC5 build warning

2017-10-11 Thread Jian J Wang
Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Dandan Bi 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
index f42c13cd89..d249479c56 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
@@ -59,9 +59,6 @@ LegacyBiosInt86 (
 {
   UINT16Segment;
   UINT16Offset;
-  LEGACY_BIOS_INSTANCE  *Private;
-
-  Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
 
   Regs->X.Flags.Reserved1 = 1;
   Regs->X.Flags.Reserved2 = 0;
-- 
2.14.1.windows.1

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


[edk2] [PATCH 1/1] ArmPkg/Include: Update MM interface return codes to match specification.

2017-10-11 Thread Supreeth Venkatesh
This patch corrects the Management Mode(MM) return codes as specified in
http://infocenter.arm.com/help/topic/com.arm.doc.den0060a/DEN0060A_ARM_MM_Interface_Specification.pdf.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Supreeth Venkatesh 
---
 ArmPkg/Include/IndustryStandard/ArmStdSmc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h 
b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h
index 37d0796649..2d2af9e37d 100644
--- a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h
@@ -56,7 +56,7 @@
 #define ARM_SMC_MM_RET_NOT_SUPPORTED   -1
 #define ARM_SMC_MM_RET_INVALID_PARAMS  -2
 #define ARM_SMC_MM_RET_DENIED  -3
-#define ARM_SMC_MM_RET_NO_MEMORY   -4
+#define ARM_SMC_MM_RET_NO_MEMORY   -5
 
 /*
  * Power State Coordination Interface (PSCI) calls cover a subset of the
-- 
2.14.1

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


Re: [edk2] [PATCH v4 5/6] IntelFrameworkModulePkg/Csm: Add code to bypass NULL pointer detection

2017-10-11 Thread Laszlo Ersek
This patch breaks the GCC5 build:

On 10/09/17 16:17, Jian J Wang wrote:

> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c 
> b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> index 3d9a8b9649..f42c13cd89 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> @@ -57,7 +57,11 @@ LegacyBiosInt86 (
>IN  EFI_IA32_REGISTER_SET *Regs
>)
>  {
> -  UINT32  *VectorBase;
> +  UINT16Segment;
> +  UINT16Offset;
> +  LEGACY_BIOS_INSTANCE  *Private;
> +
> +  Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
>  
>Regs->X.Flags.Reserved1 = 1;
>Regs->X.Flags.Reserved2 = 0;
> @@ -72,12 +76,15 @@ LegacyBiosInt86 (
>// The base address of legacy interrupt vector table is 0.
>// We use this base address to get the legacy interrupt handler.
>//
> -  VectorBase  = 0;
> +  DisableNullDetection ();
> +  Segment   = (UINT16)(((UINT32 *)0)[BiosInt] >> 16);
> +  Offset= (UINT16)((UINT32 *)0)[BiosInt];
> +  EnableNullDetection ();
>
>return InternalLegacyBiosFarCall (
> This,
> -   (UINT16) ((VectorBase)[BiosInt] >> 16),
> -   (UINT16) (VectorBase)[BiosInt],
> +   Segment,
> +   Offset,
> Regs,
> >X.Flags,
> sizeof (Regs->X.Flags)

IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c: In function 
'LegacyBiosInt86':
IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c:62:26: error: variable 
'Private' set but not used [-Werror=unused-but-set-variable]
   LEGACY_BIOS_INSTANCE  *Private;
  ^~~
cc1: all warnings being treated as errors

Can you please send a patch?

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


[edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.

2017-10-11 Thread Leo Duran
This patch-set replaces Intel-specific macros with global variables
to provide support for AMD-based x86 systems.

The replaced macros are:
1) SRAM_SAVE_STATE_MAP_OFFSET
2) TXT_SMM_PSD_OFFSET
3) SMM_PSD_OFFSET

Changes since v4:
Make runtime CPUID checks and use global variables instead of PCD's.

Changes since v3:
Correction on cover letter.

Changes since v2:
The intent of this revision is to maintain compatibility with existing
packages. To that end, changes to OvmgfPkg and QuarkSocPkg are reverted.
Moreover, pertinent macros are replaced in the C code, rather than on
header files that are shared globally.

Changes since v1:
Revision to Cc list for UefiCpuPkg.

Leo Duran (2):
  UefiCpuPkg/SmmCpuFeaturesLib: Use global variables to replace macros
  UefiCpuPkg/PiSmmCpuDxeSmm: Use global variables to replace macros

 .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S  | 28 ++---
 .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm| 29 ++---
 .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm   | 43 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCommon.h   | 48 +++
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  | 59 +++---
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  3 +
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  3 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c  | 39 ++--
 .../Library/SmmCpuFeaturesLib/X64/SmiEntry.S   | 28 ++---
 .../Library/SmmCpuFeaturesLib/X64/SmiEntry.asm | 30 ++---
 .../Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm| 47 ++
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c | 22 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  | 28 ++---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm| 21 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   | 43 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 72 --
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 17 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 18 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 20 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c  | 22 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   | 34 ++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm | 22 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm| 45 ++
 23 files changed, 547 insertions(+), 174 deletions(-)
 create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCommon.h

-- 
2.7.4

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


[edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global variables to replace macros

2017-10-11 Thread Leo Duran
Set global variables on Constructor function based on CPUID checks.
The variables replace Intel macros to allow support on AMD x86 systems.

Specifically, the replaced macros are:
1) SRAM_SAVE_STATE_MAP_OFFSET
2) TXT_SMM_PSD_OFFSET

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S  | 28 ++
 .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm| 29 +++
 .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm   | 43 
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCommon.h   | 48 ++
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  | 59 ++
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  3 ++
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  3 ++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c  | 39 --
 .../Library/SmmCpuFeaturesLib/X64/SmiEntry.S   | 28 ++
 .../Library/SmmCpuFeaturesLib/X64/SmiEntry.asm | 30 +++
 .../Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm| 47 -
 11 files changed, 282 insertions(+), 75 deletions(-)
 create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCommon.h

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S
index 4c0f8c8..c7b49d7 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S
@@ -1,6 +1,8 @@
 #--
 #
 # Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2017, AMD Incorporated. 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
@@ -28,6 +30,9 @@ ASM_GLOBAL  ASM_PFX(gStmSmbase)
 ASM_GLOBAL  ASM_PFX(gStmXdSupported)
 ASM_GLOBAL  ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))
 ASM_GLOBAL  ASM_PFX(gStmSmiHandlerIdtr)
+ASM_GLOBAL  ASM_PFX(gStmPsdOffset)
+ASM_GLOBAL  ASM_PFX(gStmGdtSize)
+ASM_GLOBAL  ASM_PFX(gStmGdtPtr)
 
 .equMSR_IA32_MISC_ENABLE, 0x1A0
 .equMSR_EFER, 0xc080
@@ -36,12 +41,13 @@ ASM_GLOBAL  ASM_PFX(gStmSmiHandlerIdtr)
 #
 # Constants relating to TXT_PROCESSOR_SMM_DESCRIPTOR
 #
-.equDSC_OFFSET, 0xfb00
-.equDSC_GDTPTR, 0x48
-.equDSC_GDTSIZ, 0x50
-.equDSC_CS, 0x14
-.equDSC_DS, 0x16
-.equDSC_SS, 0x18
+# .equ  DSC_OFFSET,   0xfb00
+# .equ  DSC_GDTPTR,   0x48
+# .equ  DSC_GDTSIZ,   0x50
+#
+.equDSC_CS,   0x14
+.equDSC_DS,   0x16
+.equDSC_SS,   0x18
 .equDSC_OTHERSEG, 0x1A
 
 .equPROTECT_MODE_CS, 0x08
@@ -55,11 +61,11 @@ _StmSmiEntryPoint:
 .byte 0xbb  # mov bx, imm16
 .word _StmGdtDesc - _StmSmiEntryPoint + 0x8000
 .byte 0x2e,0xa1 # mov ax, cs:[offset16]
-.word DSC_OFFSET + DSC_GDTSIZ
+ASM_PFX(gStmGdtSize): .space 2  # .word DSC_OFFSET + DSC_GDTSIZ
 decl%eax
 movl%eax, %cs:(%edi)# mov cs:[bx], ax
 .byte 0x66,0x2e,0xa1# mov eax, cs:[offset16]
-.word   DSC_OFFSET + DSC_GDTPTR
+ASM_PFX(gStmGdtPtr): .space 2   # .word DSC_OFFSET + DSC_GDTPTR
 movw%ax, %cs:2(%edi)
 movw%ax, %bp# ebp = GDT base
 .byte 0x66
@@ -167,7 +173,11 @@ XdDone:
 movl%cr0, %ebx
 orl $0x080010023, %ebx # enable paging + WP + NE + MP + PE
 movl%ebx, %cr0
-lealDSC_OFFSET(%edi),%ebx
+
+movl$ASM_PFX(gStmPsdOffset), %ebx  # lealDSC_OFFSET(%edi), %ebx
+movzxw  (%ebx), %esi
+leal(%edi, %esi), %ebx
+
 movwDSC_DS(%ebx),%ax
 movl%eax, %ds
 movwDSC_OTHERSEG(%ebx),%ax
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm
index 91dc1eb..4dbe276 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm
@@ -1,5 +1,7 @@
 
;-- 
;
 ; Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -29,13 

[edk2] [PATCH v5 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Use global variables to replace macros

2017-10-11 Thread Leo Duran
Set global variables on Entry function based on CPUID checks.
The variables replace Intel macros to allow support on AMD x86 systems.

Specifically, the replaced macros are:
1) SRAM_SAVE_STATE_MAP_OFFSET
2) SMM_PSD_OFFSET

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c | 22 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  | 28 ++---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm| 21 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   | 43 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 72 --
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 17 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 18 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 20 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c  | 22 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   | 34 ++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm | 22 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm| 45 ++
 12 files changed, 265 insertions(+), 99 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
index 02a866b..7b2e5fb 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
@@ -1,15 +1,17 @@
 /** @file
-Semaphore mechanism to indicate to the BSP that an AP has exited SMM
-after SMBASE relocation.
+  Semaphore mechanism to indicate to the BSP that an AP has exited SMM
+  after SMBASE relocation.
 
-Copyright (c) 2009 - 2015, 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
+  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+  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.
 
 **/
 
@@ -38,7 +40,7 @@ SemaphoreHook (
 
   mRebasedFlag = RebasedFlag;
 
-  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
+  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
gSmmSmramStateMapOffset);
   mSmmRelocationOriginalAddress = (UINTN)HookReturnFromSmm (
CpuIndex,
CpuState,
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
index 3243a91..d25c099 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
@@ -1,6 +1,8 @@
 #--
 #
 # Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017, AMD Incorporated. 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
@@ -27,6 +29,9 @@ ASM_GLOBAL  ASM_PFX(gSmbase)
 ASM_GLOBAL  ASM_PFX(mXdSupported)
 ASM_GLOBAL  ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))
 ASM_GLOBAL  ASM_PFX(gSmiHandlerIdtr)
+ASM_GLOBAL  ASM_PFX(gSmmPsdOffset)
+ASM_GLOBAL  ASM_PFX(gPsdGdtSize)
+ASM_GLOBAL  ASM_PFX(gPsdGdtPtr)
 
 .equMSR_IA32_MISC_ENABLE, 0x1A0
 .equMSR_EFER, 0xc080
@@ -35,12 +40,13 @@ ASM_GLOBAL  ASM_PFX(gSmiHandlerIdtr)
 #
 # Constants relating to PROCESSOR_SMM_DESCRIPTOR
 #
-.equDSC_OFFSET, 0xfb00
-.equDSC_GDTPTR, 0x30
-.equDSC_GDTSIZ, 0x38
-.equDSC_CS, 14
-.equDSC_DS, 16
-.equDSC_SS, 18
+# .equ  DSC_OFFSET,   0xfb00
+# .equ  DSC_GDTPTR,   0x30
+# .equ  DSC_GDTSIZ,   0x38
+#
+.equDSC_CS,   14
+.equDSC_DS,   16
+.equDSC_SS,   18
 .equDSC_OTHERSEG, 20
 
 .equPROTECT_MODE_CS, 0x08
@@ -55,11 +61,11 @@ 

Re: [edk2] [platforms: PATCH 8/8] Marvell/Armada: Add 32-bit ARM support

2017-10-11 Thread Ard Biesheuvel
On 11 October 2017 at 18:58, Leif Lindholm  wrote:
> On Wed, Oct 11, 2017 at 05:40:49PM +0200, Marcin Wojtas wrote:
>> From: Ard Biesheuvel 
>>
>> Update the included components and library classes to make this platform
>> build for 32-bit ARM.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> Signed-off-by: Marcin Wojtas 
>
> Very neat!
> Reviewed-by: Leif Lindholm 
>

There are two caveats though:

- it is up to ARM-TF to enter the non-secure world in the right mode
(EL2 or SVC)
- HYP mode cannot be used, since it mandates long descriptors, while
UEFI mandates short descriptors, so we lose KVM functionality when
booting 32-bit Linux via UEFI.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 4/8] Marvell/Armada: Add support from DRAM remapping

2017-10-11 Thread Ard Biesheuvel
On 11 October 2017 at 18:08, Leif Lindholm  wrote:
> Subject: from->for ?
>
> On Wed, Oct 11, 2017 at 05:40:45PM +0200, Marcin Wojtas wrote:
>> From: Ard Biesheuvel 
>>
>> The Armada 70x0/80x0 DRAM controller allows a single window of DRAM
>> to be remapped to another location in the physical address space. This
>> allows us to free up some memory in the 32-bit addressable region for
>> peripheral MMIO and PCI MMIO32 and CONFIG spaces. This patch adjusts
>> memory blocks to the configuration done in ATF.
>
> The ARM Trusted Firmware developers tend to frown at that
> abbreviation. Please use ARM-TF in official context.
>
> Which configuration is this? Presumably, if this was changed in
> ARM-TF, we would need to change it this end too, so does not hurt to
> be explicit. (It uses a FixedPcd, so clearly it is not dynamically
> detected, which the commit message can be read as.)
>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> Signed-off-by: Marcin Wojtas 
>> ---
>>  Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 
>> 15 +
>>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf   | 
>>  3 +
>>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c  | 
>> 60 
>>  Platform/Marvell/Marvell.dec  | 
>> 13 +
>>  4 files changed, 81 insertions(+), 10 deletions(-)
>>
>> diff --git 
>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
>> index 72f8cfc..c5be1a9 100644
>> --- 
>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
>> +++ 
>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
>> @@ -17,6 +17,21 @@
>>
>>  ASM_FUNC(ArmPlatformPeiBootAction)
>>mov   x29, xzr
>> +
>> +  .if   FixedPcdGet64 (PcdSystemMemoryBase) != 0
>> +  .err  PcdSystemMemoryBase should be 0x0 on this platform!
>> +  .endif
>> +
>> +  .if   FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 
>> (PcdDramRemapTarget)
>> +//
>> +// Use the low range for UEFI itself. The remaining memory will be 
>> mapped
>> +// and added to the GCD map later.
>> +//
>> +adr   x0, mSystemMemoryEnd
>> +MOV64 (x1, FixedPcdGet32 (PcdDramRemapTarget) - 1)
>> +str   x1, [x0]
>> +  .endif
>> +
>>ret
>>
>>  //UINTN
>> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf 
>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>> index 2e198c3..838a670 100644
>> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>> @@ -67,5 +67,8 @@
>>gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
>>gArmTokenSpaceGuid.PcdArmPrimaryCore
>>
>> +  gMarvellTokenSpaceGuid.PcdDramRemapSize
>> +  gMarvellTokenSpaceGuid.PcdDramRemapTarget
>> +
>>  [Ppis]
>>gArmMpCoreInfoPpiGuid
>> diff --git 
>> a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c 
>> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>> index 74c9956..2cb2e15 100644
>> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
>> @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
>> DAMAGE.
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>
>>  // The total number of descriptors, including the final "end-of-table" 
>> descriptor.
>> @@ -44,6 +45,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
>> DAMAGE.
>>  #define DDR_ATTRIBUTES_CACHED   
>> ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK
>>  #define DDR_ATTRIBUTES_UNCACHED 
>> ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED
>>
>> +STATIC ARM_MEMORY_REGION_DESCRIPTOR 
>> VirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
>
> mVirtualMemoryTable?
>
>> +
>>  /**
>>Return the Virtual Memory Map of your platform
>>
>> @@ -59,20 +62,41 @@ ArmPlatformGetVirtualMemoryMap (
>>IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap
>>)
>>  {
>> -  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
>>UINTN Index = 0;
>> +  UINT64MemSize;
>> +  UINT64MemLowSize;
>> +  UINT64MemHighStart;
>> +  UINT64MemHighSize;
>> +  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
>>
>>ASSERT (VirtualMemoryMap != NULL);
>>
>> -  VirtualMemoryTable = 
>> (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages(EFI_SIZE_TO_PAGES 
>> (sizeof(ARM_MEMORY_REGION_DESCRIPTOR) * MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
>> -  if (VirtualMemoryTable == NULL) {
>> -  return;
>> -  }
>> +  MemSize 

Re: [edk2] [platforms: PATCH 1/8] Marvell/Armada: Implement EFI_RNG_PROTOCOL driver for EIP76 TRNG

2017-10-11 Thread Ard Biesheuvel
On 11 October 2017 at 17:47, Leif Lindholm  wrote:
> On Wed, Oct 11, 2017 at 05:40:42PM +0200, Marcin Wojtas wrote:
>> From: Ard Biesheuvel 
>>
>> Add an implementation of EFI_RNG_PROTOCOL so that the OS loader has
>> access to entropy for KASLR and other purposes (i.e., seeding the OS's
>> entropy pool very early on).
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> Signed-off-by: Marcin Wojtas 
>> ---
>>  Platform/Marvell/Armada/Armada.dsc.inc|   4 
>> +
>>  Platform/Marvell/Armada/Armada70x0.fdf|   1 
>> +
>>  Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c   | 254 
>> 
>>  Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf |  47 
>> 
>>  Platform/Marvell/Marvell.dec  |   3 
>> +
>>  5 files changed, 309 insertions(+)
>>
>> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
>> b/Platform/Marvell/Armada/Armada.dsc.inc
>> index 1aa485c..ec24d76 100644
>> --- a/Platform/Marvell/Armada/Armada.dsc.inc
>> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
>> @@ -364,6 +364,9 @@
>>gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
>>gArmTokenSpaceGuid.PcdArmScr|0x531
>>
>> +  # TRNG
>> +  gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276
>> +
>>  
>> 
>>  #
>>  # Components Section - list of all EDK II Modules needed by this Platform
>> @@ -400,6 +403,7 @@
>>Platform/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf
>>Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
>>Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
>> +  Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf
>>
>># Network support
>>MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
>> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
>> b/Platform/Marvell/Armada/Armada70x0.fdf
>> index 933c3ed..a94a9ff 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.fdf
>> +++ b/Platform/Marvell/Armada/Armada70x0.fdf
>> @@ -113,6 +113,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>>INF Platform/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf
>>INF Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
>>INF Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
>> +  INF Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf
>>
>># Network support
>>INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
>> diff --git 
>> a/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c 
>> b/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
>> new file mode 100644
>> index 000..dca6dcc
>> --- /dev/null
>> +++ b/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
>> @@ -0,0 +1,254 @@
>> +/** @file
>> +
>> +  This driver produces an EFI_RNG_PROTOCOL instance for the Armada 70x0 TRNG
>> +
>> +  Copyright (C) 2017, Linaro Ltd. 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 
>> +#include 
>> +
>> +#include 
>> +
>> +#define TRNG_OUTPUT_REG mTrngBaseAddress
>> +#define TRNG_OUTPUT_SIZE0x10
>> +
>> +#define TRNG_STATUS_REG (mTrngBaseAddress + 0x10)
>> +#define TRNG_STATUS_READY   BIT0
>> +
>> +#define TRNG_INTACK_REG (mTrngBaseAddress + 0x10)
>> +#define TRNG_INTACK_READY   BIT0
>> +
>> +#define TRNG_CONTROL_REG(mTrngBaseAddress + 0x14)
>> +#define TRNG_CONTROL_REG_ENABLE BIT10
>> +
>> +#define TRNG_CONFIG_REG (mTrngBaseAddress + 0x18)
>> +#define __MIN_REFILL_SHIFT  0
>> +#define __MAX_REFILL_SHIFT  16
>> +#define TRNG_CONFIG_MIN_REFILL_CYCLES   (0x05 << __MIN_REFILL_SHIFT)
>> +#define TRNG_CONFIG_MAX_REFILL_CYCLES   (0x22 << __MAX_REFILL_SHIFT)
>> +
>> +#define TRNG_FRODETUNE_REG  (mTrngBaseAddress + 0x24)
>> +#define TRNG_FRODETUNE_MASK 0x0
>> +
>> +#define TRNG_FROENABLE_REG  (mTrngBaseAddress + 0x20)
>> +#define TRNG_FROENABLE_MASK 0xff
>> +
>> +#define TRNG_MAX_RETRIES20
>> +
>> +STATIC EFI_PHYSICAL_ADDRESS 

Re: [edk2] [platforms: PATCH 8/8] Marvell/Armada: Add 32-bit ARM support

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 05:40:49PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel 
> 
> Update the included components and library classes to make this platform
> build for 32-bit ARM.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Signed-off-by: Marcin Wojtas 

Very neat!
Reviewed-by: Leif Lindholm 

> ---
>  Platform/Marvell/Armada/Armada.dsc.inc | 3 +--
>  Platform/Marvell/Armada/Armada70x0.dsc | 4 ++--
>  Platform/Marvell/Armada/Armada70x0.fdf | 2 +-
>  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
> b/Platform/Marvell/Armada/Armada.dsc.inc
> index b0a8240..b9fc384 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -132,7 +132,6 @@
>
> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
>FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>  
> -[LibraryClasses.AARCH64]
>ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>
> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> @@ -362,7 +361,7 @@
># ARM Pcds
>gArmTokenSpaceGuid.PcdSystemMemoryBase|0
>gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
> -  gArmTokenSpaceGuid.PcdArmScr|0x531
> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
>  
># Secure region reservation
>gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc 
> b/Platform/Marvell/Armada/Armada70x0.dsc
> index 946c93e..0396e8e 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -39,8 +39,8 @@
>PLATFORM_GUID  = f837e231-cfc7-4f56-9a0f-5b218d746ae3
>PLATFORM_VERSION   = 0.1
>DSC_SPECIFICATION  = 0x00010005
> -  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
> -  SUPPORTED_ARCHITECTURES= AARCH64
> +  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)-$(ARCH)
> +  SUPPORTED_ARCHITECTURES= AARCH64|ARM
>BUILD_TARGETS  = DEBUG|RELEASE
>SKUID_IDENTIFIER   = DEFAULT
>FLASH_DEFINITION   = Platform/Marvell/Armada/Armada70x0.fdf
> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
> b/Platform/Marvell/Armada/Armada70x0.fdf
> index a94a9ff..ec2c368 100644
> --- a/Platform/Marvell/Armada/Armada70x0.fdf
> +++ b/Platform/Marvell/Armada/Armada70x0.fdf
> @@ -237,7 +237,7 @@ READ_LOCK_STATUS   = TRUE
>  #
>  
>  
> -[Rule.AARCH64.SEC]
> +[Rule.Common.SEC]
>FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
>  TE  TEAlign = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
>}
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 7/8] Marvell/Armada: Armada70x0Lib: Add support for 32-bit ARM

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 05:40:48PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel 
> 
> Add an ARM implementation of ArmPlatformHelper.S.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Signed-off-by: Marcin Wojtas 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S | 77 
> 
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf   |  3 +
>  2 files changed, 80 insertions(+)
> 
> diff --git 
> a/Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S 
> b/Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S
> new file mode 100644
> index 000..21459e5
> --- /dev/null
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S
> @@ -0,0 +1,77 @@
> +//Based on 
> ArmPlatformPkg/Library/ArmPlatformLibNull/AArch64/ArmPlatformHelper.S
> +//
> +//  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
> +//  Copyright (c) 2016, Marvell. All rights reserved.
> +//  Copyright (c) 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 
> +#include 
> +
> +#define CCU_MC_BASE 0xF0001700
> +#define CCU_MC_RCR_OFFSET   0x0
> +#define CCU_MC_RCR_REMAP_EN BIT0
> +#define CCU_MC_RCR_REMAP_SIZE(Size) (((Size) - 1) ^ (SIZE_1MB - 1))
> +
> +#define CCU_MC_RSBR_OFFSET  0x4
> +#define CCU_MC_RSBR_SOURCE_BASE(Base)   (((Base) >> 20) << 10)
> +#define CCU_MC_RTBR_OFFSET  0x8
> +#define CCU_MC_RTBR_TARGET_BASE(Base)   (((Base) >> 20) << 10)
> +
> +ASM_FUNC(ArmPlatformPeiBootAction)
> +  .if   FixedPcdGet64 (PcdSystemMemoryBase) != 0
> +  .err  PcdSystemMemoryBase should be 0x0 on this platform!
> +  .endif
> +
> +  .if   FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 
> (PcdDramRemapTarget)
> +//
> +// Use the low range for UEFI itself. The remaining memory will be mapped
> +// and added to the GCD map later.
> +//
> +ADRL  (r0, mSystemMemoryEnd)
> +MOV32 (r2, FixedPcdGet32 (PcdDramRemapTarget) - 1)
> +mov   r3, #0
> +strd  r2, r3, [r0]
> +  .endif
> +
> +  bxlr
> +
> +//UINTN
> +//ArmPlatformGetCorePosition (
> +//  IN UINTN MpId
> +//  );
> +// With this function: CorePos = (ClusterId * 2) + CoreId
> +ASM_FUNC(ArmPlatformGetCorePosition)
> +  and   r1, r0, #ARM_CORE_MASK
> +  and   r0, r0, #ARM_CLUSTER_MASK
> +  add   r0, r1, r0, LSR #7
> +  bxlr
> +
> +//UINTN
> +//ArmPlatformGetPrimaryCoreMpId (
> +//  VOID
> +//  );
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> +  MOV32 (r0, FixedPcdGet32(PcdArmPrimaryCore))
> +  bxlr
> +
> +//UINTN
> +//ArmPlatformIsPrimaryCore (
> +//  IN UINTN MpId
> +//  );
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  MOV32 (r1, FixedPcdGet32(PcdArmPrimaryCoreMask))
> +  and   r0, r0, r1
> +  MOV32 (r1, FixedPcdGet32(PcdArmPrimaryCore))
> +  cmp   r0, r1
> +  moveq r0, #1
> +  movne r0, #0
> +  bxlr
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf 
> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
> index 838a670..0dabd4b 100644
> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
> @@ -60,6 +60,9 @@
>  [Sources.AArch64]
>AArch64/ArmPlatformHelper.S
>  
> +[Sources.ARM]
> +  ARM/ArmPlatformHelper.S
> +
>  [FixedPcd]
>gArmTokenSpaceGuid.PcdSystemMemoryBase
>gArmTokenSpaceGuid.PcdSystemMemorySize
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 6/8] Marvell/Armada: Enable dynamic DRAM size detection

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 05:40:47PM +0200, Marcin Wojtas wrote:
> Instead of using hardcoded value in PcdSystemMemorySize PCD,
> obtain DRAM size directly from SoC registers, which are filled
> by firmware during early initialization stage.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c | 111 
> +++-
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h |  49 
> +
>  2 files changed, 159 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c 
> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> index 2cb2e15..22cbe47 100644
> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> @@ -36,8 +36,11 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
> DAMAGE.
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
> +#include "Armada70x0LibMem.h"
> +
>  // The total number of descriptors, including the final "end-of-table" 
> descriptor.
>  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 16
>  
> @@ -47,6 +50,105 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
> DAMAGE.
>  
>  STATIC ARM_MEMORY_REGION_DESCRIPTOR 
> VirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
>  
> +// Obtain DRAM size basing on register values filled by early firmware.
> +STATIC
> +UINT64
> +DramSizeGet (

GetDramSize?

> +  UINT64 *MemSize

IN, OUT?

> +  )
> +{
> +  UINT64 BaseAddr;
> +  UINT32 RegVal;
> +  UINT8 AreaLengthMap;
> +  UINT8 Cs;
> +
> +  *MemSize = 0;
> +
> +  for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) {
> +
> +RegVal = MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs));
> +
> +/* Exit loop on first disabled DRAM CS */
> +if (!(RegVal & DRAM_CS_VALID_ENABLED_MASK)) {
> +  break;
> +}
> +
> +/*
> + * Sanity check for base address of next DRAM block.
> + * Only continuous space will be used.
> + */
> +BaseAddr = ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG (Cs)) <<
> +DRAM_START_ADDR_HTOL_OFFS) | (RegVal & 
> DRAM_START_ADDRESS_L_MASK);

Please use macros, temporary variables, more parentheses or whatever
to help make the above operation readable.

> +if (BaseAddr != *MemSize) {
> +  DEBUG ((DEBUG_ERROR,
> +"DramSizeGet: DRAM blocks are not contiguous, limit size to 
> 0x%llx\n",
> +*MemSize));
> +  return EFI_SUCCESS;
> +}
> +
> +/* Decode area length for current CS from register value */
> +AreaLengthMap = ((RegVal & DRAM_AREA_LENGTH_MASK) >> 
> DRAM_AREA_LENGTH_OFFS);
> +switch (AreaLengthMap) {

Why Map?

> +case 0x0:
> +  *MemSize += 0x1800;
> +  break;
> +case 0x1:
> +  *MemSize += 0x3000;
> +  break;
> +case 0x2:
> +  *MemSize += 0x6000;
> +  break;
> +case 0x3:
> +  *MemSize += 0xC000;
> +  break;

The above ones look if not devoid of pattern, at least a bit
unexpected - and 4-6 are comlpetely missing. Is there any
documentation available for me to read to try to understand what is
going on?

The below all look predictably formatted and possible to calculate
rather than list one by one.

(1 << ((AreaLengthMap + 16))) if a quick calculation serves me right.

> +case 0x7:
> +  *MemSize += 0x0080;
> +  break;
> +case 0x8:
> +  *MemSize += 0x0100;
> +  break;
> +case 0x9:
> +  *MemSize += 0x0200;
> +  break;
> +case 0xA:
> +  *MemSize += 0x0400;
> +  break;
> +case 0xB:
> +  *MemSize += 0x0800;
> +  break;
> +case 0xC:
> +  *MemSize += 0x1000;
> +  break;
> +case 0xD:
> +  *MemSize += 0x2000;
> +  break;
> +case 0xE:
> +  *MemSize += 0x4000;
> +  break;
> +case 0xF:
> +  *MemSize += 0x8000;
> +  break;
> +case 0x10:
> +  *MemSize += 0x1;
> +  break;
> +case 0x11:
> +  *MemSize += 0x2;
> +  break;
> +case 0x12:
> +  *MemSize += 0x4;
> +  break;
> +case 0x13:
> +  *MemSize += 0x8;
> +  break;
> +default:
> +  DEBUG ((DEBUG_ERROR, "Invalid area length (0x%x) for CS#%d\n", 
> AreaLengthMap, Cs));

Area length isn't really a helpful debug message. 
"memory module size"?

/
Leif

> +  return EFI_INVALID_PARAMETER;
> +}
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>Return the Virtual Memory Map of your platform
>  
> @@ -68,10 +170,17 @@ ArmPlatformGetVirtualMemoryMap (
>UINT64MemHighStart;
>UINT64MemHighSize;
>EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
> +  EFI_STATUSStatus;
>  
>ASSERT (VirtualMemoryMap != NULL);
>  
> -  MemSize = FixedPcdGet64 (PcdSystemMemorySize);
> +  

Re: [edk2] [platforms: PATCH 5/8] Marvell/Armada: Add MemoryInitPeiLib that reserves secure region

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 05:40:46PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel 
> 
> The default MemoryInitPeiLib implementation insists on reserving the
> region occupied by our own FV, while this is not necessary at all (the
> compressed payload is uncompressed elsewhere, so the moment we enter
> DXE core, we don't care about the FV contents in memory)
> 
> So clone MemoryInitPeiLib and modify it to suit our needs.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Signed-off-by: Marcin Wojtas 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/Marvell/Armada/Armada.dsc.inc   
>  |   6 +-
>  
> Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
>| 158 
>  
> Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
>  |  46 ++
>  Platform/Marvell/Marvell.dec 
>  |   8 +
>  4 files changed, 217 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
> b/Platform/Marvell/Armada/Armada.dsc.inc
> index 56d8941..b0a8240 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -153,7 +153,7 @@
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>  
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
> -  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> +  
> MemoryInitPeiLib|Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
>BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>  
>  [LibraryClasses.common.DXE_CORE]
> @@ -364,6 +364,10 @@
>gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
>gArmTokenSpaceGuid.PcdArmScr|0x531
>  
> +  # Secure region reservation
> +  gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400
> +  gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x020
> +
># TRNG
>gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276
>  
> diff --git 
> a/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
>  
> b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
> new file mode 100644
> index 000..53119f4
> --- /dev/null
> +++ 
> b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
> @@ -0,0 +1,158 @@
> +/** @file
> +*
> +*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +*  Copyright (c) 2017, 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.
> +*
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +VOID
> +BuildMemoryTypeInformationHob (
> +  VOID
> +  );
> +
> +STATIC
> +VOID
> +InitMmu (
> +  IN ARM_MEMORY_REGION_DESCRIPTOR  *MemoryTable
> +  )
> +{
> +
> +  VOID  *TranslationTableBase;
> +  UINTN TranslationTableSize;
> +  RETURN_STATUS Status;
> +
> +  Status = ArmConfigureMmu (MemoryTable,
> +,
> +);
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "Error: Failed to enable MMU\n"));
> +  }
> +}
> +
> +/*++
> +
> +Routine Description:
> +
> +
> +
> +Arguments:
> +
> +  FileHandle  - Handle of the file being invoked.
> +  PeiServices - Describes the list of possible PEI Services.
> +
> +Returns:
> +
> +  Status -  EFI_SUCCESS if the boot mode could be set
> +
> +--*/
> +EFI_STATUS
> +EFIAPI
> +MemoryPeim (
> +  IN EFI_PHYSICAL_ADDRESS   UefiMemoryBase,
> +  IN UINT64 UefiMemorySize
> +  )
> +{
> +  ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> +  UINT64   ResourceLength;
> +  EFI_PEI_HOB_POINTERS NextHob;
> +  EFI_PHYSICAL_ADDRESS SecureTop;
> +  EFI_PHYSICAL_ADDRESS ResourceTop;
> +
> +  // Get Virtual Memory Map from the Platform Library
> +  ArmPlatformGetVirtualMemoryMap ();
> +
> +  SecureTop = (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdSecureRegionBase) +
> +  FixedPcdGet32 (PcdSecureRegionSize);
> +
> +  //
> +  // Search for System Memory Hob that covers the secure firmware,
> +  // and punch a hole in it
> +  //
> +  for (NextHob.Raw = GetHobList ();
> +   NextHob.Raw != NULL;

Re: [edk2] [platforms: PATCH 4/8] Marvell/Armada: Add support from DRAM remapping

2017-10-11 Thread Leif Lindholm
Subject: from->for ?

On Wed, Oct 11, 2017 at 05:40:45PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel 
> 
> The Armada 70x0/80x0 DRAM controller allows a single window of DRAM
> to be remapped to another location in the physical address space. This
> allows us to free up some memory in the 32-bit addressable region for
> peripheral MMIO and PCI MMIO32 and CONFIG spaces. This patch adjusts
> memory blocks to the configuration done in ATF.

The ARM Trusted Firmware developers tend to frown at that
abbreviation. Please use ARM-TF in official context.

Which configuration is this? Presumably, if this was changed in
ARM-TF, we would need to change it this end too, so does not hurt to
be explicit. (It uses a FixedPcd, so clearly it is not dynamically
detected, which the commit message can be read as.)

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Signed-off-by: Marcin Wojtas 
> ---
>  Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 
> 15 +
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf   |  
> 3 +
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c  | 
> 60 
>  Platform/Marvell/Marvell.dec  | 
> 13 +
>  4 files changed, 81 insertions(+), 10 deletions(-)
> 
> diff --git 
> a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
> b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
> index 72f8cfc..c5be1a9 100644
> --- 
> a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
> +++ 
> b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
> @@ -17,6 +17,21 @@
>  
>  ASM_FUNC(ArmPlatformPeiBootAction)
>mov   x29, xzr
> +
> +  .if   FixedPcdGet64 (PcdSystemMemoryBase) != 0
> +  .err  PcdSystemMemoryBase should be 0x0 on this platform!
> +  .endif
> +
> +  .if   FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 
> (PcdDramRemapTarget)
> +//
> +// Use the low range for UEFI itself. The remaining memory will be mapped
> +// and added to the GCD map later.
> +//
> +adr   x0, mSystemMemoryEnd
> +MOV64 (x1, FixedPcdGet32 (PcdDramRemapTarget) - 1)
> +str   x1, [x0]
> +  .endif
> +
>ret
>  
>  //UINTN
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf 
> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
> index 2e198c3..838a670 100644
> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
> @@ -67,5 +67,8 @@
>gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
>gArmTokenSpaceGuid.PcdArmPrimaryCore
>  
> +  gMarvellTokenSpaceGuid.PcdDramRemapSize
> +  gMarvellTokenSpaceGuid.PcdDramRemapTarget
> +
>  [Ppis]
>gArmMpCoreInfoPpiGuid
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c 
> b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> index 74c9956..2cb2e15 100644
> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  // The total number of descriptors, including the final "end-of-table" 
> descriptor.
> @@ -44,6 +45,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define DDR_ATTRIBUTES_CACHED   
> ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK
>  #define DDR_ATTRIBUTES_UNCACHED 
> ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED
>  
> +STATIC ARM_MEMORY_REGION_DESCRIPTOR 
> VirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];

mVirtualMemoryTable?

> +
>  /**
>Return the Virtual Memory Map of your platform
>  
> @@ -59,20 +62,41 @@ ArmPlatformGetVirtualMemoryMap (
>IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap
>)
>  {
> -  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
>UINTN Index = 0;
> +  UINT64MemSize;
> +  UINT64MemLowSize;
> +  UINT64MemHighStart;
> +  UINT64MemHighSize;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
>  
>ASSERT (VirtualMemoryMap != NULL);
>  
> -  VirtualMemoryTable = 
> (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages(EFI_SIZE_TO_PAGES 
> (sizeof(ARM_MEMORY_REGION_DESCRIPTOR) * MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
> -  if (VirtualMemoryTable == NULL) {
> -  return;
> -  }
> +  MemSize = FixedPcdGet64 (PcdSystemMemorySize);
> +  MemLowSize = MIN (FixedPcdGet64 (PcdDramRemapTarget), MemSize);
> +  MemHighStart = (UINT64)FixedPcdGet64 (PcdDramRemapTarget) +
> + FixedPcdGet32 

Re: [edk2] [platforms: PATCH 3/8] Marvell/Armada: Remove custom reset library residues

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 05:40:44PM +0200, Marcin Wojtas wrote:
> When switching to generic PSCI reset library, obsolete parts
> of previous custom reset library (PCDs, documentation) remained.
> Remove them.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 

Yes, please.
Reviewed-by: Leif Lindholm 

> ---
>  Platform/Marvell/Armada/Armada70x0.dsc | 4 
>  Platform/Marvell/Marvell.dec   | 4 
>  Silicon/Marvell/Documentation/PortingGuide.txt | 9 -
>  3 files changed, 17 deletions(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc 
> b/Platform/Marvell/Armada/Armada70x0.dsc
> index 430803c..946c93e 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -138,9 +138,5 @@
>gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x1, 0x0 }
>gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1, 0x1 }
>  
> -  #ResetLib
> -  gMarvellTokenSpaceGuid.PcdResetRegAddress|0xf06f0084
> -  gMarvellTokenSpaceGuid.PcdResetRegMask|0x1
> -
>#RTC
>gMarvellTokenSpaceGuid.PcdRtcEnabled|{ 0x1 }
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index 78f5e53..434d6cb 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -188,10 +188,6 @@
>gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x0 }|VOID*|0x334
>gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x0 }|VOID*|0x335
>  
> -#ResetLib
> -  gMarvellTokenSpaceGuid.PcdResetRegAddress|0|UINT64|0x4050
> -  gMarvellTokenSpaceGuid.PcdResetRegMask|0|UINT32|0x451
> -
>  #RTC
>gMarvellTokenSpaceGuid.PcdRtcEnabled|{ 0x0 }|VOID*|0x4052
>  
> diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt 
> b/Silicon/Marvell/Documentation/PortingGuide.txt
> index 66ec918..cbe3bed 100644
> --- a/Silicon/Marvell/Documentation/PortingGuide.txt
> +++ b/Silicon/Marvell/Documentation/PortingGuide.txt
> @@ -383,15 +383,6 @@ Set pin 6 and 7 to 0xa function:
>   gMarvellTokenSpaceGuid.PcdChip0MppSel0|{ 0x0, 0x0, 0x0, 
> 0x0, 0x0, 0x0, 0xa, 0xa, 0x0, 0x0 }
>  
>  
> -MarvellResetSystemLib configuration
> -===
> -This simple library allows to mask given bits in given reg at UEFI 'reset'
> -command call. These variables are configurable through PCDs:
> -
> -  - gMarvellTokenSpaceGuid.PcdResetRegAddress
> -  - gMarvellTokenSpaceGuid.PcdResetRegMask
> -
> -
>  Ramdisk configuration
>  =
>  There is one PCD available for Ramdisk configuration
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 2/8] Marvell/Armada: Increase preallocated memory region size

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 05:40:43PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel 
> 
> In order to prevent fragmentation of the UEFI memory map, increase the
> sizes of the preallocated regions. Note that this does not increase the
> memory footprint of UEFI, it just modifies it allocation policy to keep
> similar region types together.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Signed-off-by: Marcin Wojtas 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/Marvell/Armada/Armada.dsc.inc | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
> b/Platform/Marvell/Armada/Armada.dsc.inc
> index ec24d76..56d8941 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -341,10 +341,10 @@
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0
> -  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|50
> -  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|20
> -  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|400
> -  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|2
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|1000
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|1000
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|2000
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|35000
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0
>  
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 1/8] Marvell/Armada: Implement EFI_RNG_PROTOCOL driver for EIP76 TRNG

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 05:40:42PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel 
> 
> Add an implementation of EFI_RNG_PROTOCOL so that the OS loader has
> access to entropy for KASLR and other purposes (i.e., seeding the OS's
> entropy pool very early on).
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Signed-off-by: Marcin Wojtas 
> ---
>  Platform/Marvell/Armada/Armada.dsc.inc|   4 +
>  Platform/Marvell/Armada/Armada70x0.fdf|   1 +
>  Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c   | 254 
> 
>  Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf |  47 
> 
>  Platform/Marvell/Marvell.dec  |   3 +
>  5 files changed, 309 insertions(+)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
> b/Platform/Marvell/Armada/Armada.dsc.inc
> index 1aa485c..ec24d76 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -364,6 +364,9 @@
>gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
>gArmTokenSpaceGuid.PcdArmScr|0x531
>  
> +  # TRNG
> +  gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276
> +
>  
> 
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> @@ -400,6 +403,7 @@
>Platform/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf
>Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
>Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> +  Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf
>  
># Network support
>MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
> b/Platform/Marvell/Armada/Armada70x0.fdf
> index 933c3ed..a94a9ff 100644
> --- a/Platform/Marvell/Armada/Armada70x0.fdf
> +++ b/Platform/Marvell/Armada/Armada70x0.fdf
> @@ -113,6 +113,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>INF Platform/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf
>INF Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
>INF Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> +  INF Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf
>  
># Network support
>INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> diff --git 
> a/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c 
> b/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
> new file mode 100644
> index 000..dca6dcc
> --- /dev/null
> +++ b/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
> @@ -0,0 +1,254 @@
> +/** @file
> +
> +  This driver produces an EFI_RNG_PROTOCOL instance for the Armada 70x0 TRNG
> +
> +  Copyright (C) 2017, Linaro Ltd. 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 
> +#include 
> +
> +#include 
> +
> +#define TRNG_OUTPUT_REG mTrngBaseAddress
> +#define TRNG_OUTPUT_SIZE0x10
> +
> +#define TRNG_STATUS_REG (mTrngBaseAddress + 0x10)
> +#define TRNG_STATUS_READY   BIT0
> +
> +#define TRNG_INTACK_REG (mTrngBaseAddress + 0x10)
> +#define TRNG_INTACK_READY   BIT0
> +
> +#define TRNG_CONTROL_REG(mTrngBaseAddress + 0x14)
> +#define TRNG_CONTROL_REG_ENABLE BIT10
> +
> +#define TRNG_CONFIG_REG (mTrngBaseAddress + 0x18)
> +#define __MIN_REFILL_SHIFT  0
> +#define __MAX_REFILL_SHIFT  16
> +#define TRNG_CONFIG_MIN_REFILL_CYCLES   (0x05 << __MIN_REFILL_SHIFT)
> +#define TRNG_CONFIG_MAX_REFILL_CYCLES   (0x22 << __MAX_REFILL_SHIFT)
> +
> +#define TRNG_FRODETUNE_REG  (mTrngBaseAddress + 0x24)
> +#define TRNG_FRODETUNE_MASK 0x0
> +
> +#define TRNG_FROENABLE_REG  (mTrngBaseAddress + 0x20)
> +#define TRNG_FROENABLE_MASK 0xff
> +
> +#define TRNG_MAX_RETRIES20
> +
> +STATIC EFI_PHYSICAL_ADDRESS mTrngBaseAddress;
> +
> +/**
> +  Returns information about the random number generation implementation.
> +
> +  @param[in] This A pointer to the EFI_RNG_PROTOCOL

Re: [edk2] [PATCH] MdeModulePkg/Bds: Check variable name even OptionNumber is NULL

2017-10-11 Thread Ard Biesheuvel
On 10 October 2017 at 17:59, Laszlo Ersek  wrote:
> On 10/10/17 10:58, Ruiyu Ni wrote:
>> Current implementation skips to check whether the last four
>> characters are digits when the OptionNumber is NULL.
>> Even worse, it may incorrectly return FALSE when OptionNumber is
>> NULL.
>>
>> The patch fixes it to always check the variable name even
>> OptionNumber is NULL.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ruiyu Ni 
>> Cc: Ard Biesheuvel 
>> Cc: Laszlo Ersek 
>> ---
>>  .../Library/UefiBootManagerLib/BmLoadOption.c  | 45 
>> ++
>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
>> b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
>> index b0a35058d0..32918caf32 100644
>> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
>> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
>> @@ -785,6 +785,8 @@ EfiBootManagerIsValidLoadOptionVariableName (
>>UINTN VariableNameLen;
>>UINTN Index;
>>UINTN Uint;
>> +  EFI_BOOT_MANAGER_LOAD_OPTION_TYPE LocalOptionType;
>> +  UINT16LocalOptionNumber;
>>
>>if (VariableName == NULL) {
>>  return FALSE;
>> @@ -792,39 +794,52 @@ EfiBootManagerIsValidLoadOptionVariableName (
>>
>>VariableNameLen = StrLen (VariableName);
>>
>> +  //
>> +  // Return FALSE when the variable name length is too small.
>> +  //
>>if (VariableNameLen <= 4) {
>>  return FALSE;
>>}
>>
>> -  for (Index = 0; Index < ARRAY_SIZE (mBmLoadOptionName); Index++) {
>> -if ((VariableNameLen - 4 == StrLen (mBmLoadOptionName[Index])) &&
>> -(StrnCmp (VariableName, mBmLoadOptionName[Index], VariableNameLen - 
>> 4) == 0)
>> +  //
>> +  // Return FALSE when the variable name doesn't start with 
>> Driver/SysPrep/Boot/PlatformRecovery.
>> +  //
>> +  for (LocalOptionType = 0; LocalOptionType < ARRAY_SIZE 
>> (mBmLoadOptionName); LocalOptionType++) {
>> +if ((VariableNameLen - 4 == StrLen 
>> (mBmLoadOptionName[LocalOptionType])) &&
>> +(StrnCmp (VariableName, mBmLoadOptionName[LocalOptionType], 
>> VariableNameLen - 4) == 0)
>>  ) {
>>break;
>>  }
>>}
>> +  if (LocalOptionType == ARRAY_SIZE (mBmLoadOptionName)) {
>> +return FALSE;
>> +  }
>>
>> -  if (Index == ARRAY_SIZE (mBmLoadOptionName)) {
>> +  //
>> +  // Return FALSE when the last four characters are not hex digits.
>> +  //
>> +  LocalOptionNumber = 0;
>> +  for (Index = VariableNameLen - 4; Index < VariableNameLen; Index++) {
>> +Uint = BmCharToUint (VariableName[Index]);
>> +if (Uint == -1) {
>> +  break;
>> +} else {
>> +  LocalOptionNumber = (UINT16) Uint + LocalOptionNumber * 0x10;
>> +}
>> +  }
>> +  if (Index != VariableNameLen) {
>>  return FALSE;
>>}
>>
>>if (OptionType != NULL) {
>> -*OptionType = (EFI_BOOT_MANAGER_LOAD_OPTION_TYPE) Index;
>> +*OptionType = LocalOptionType;
>>}
>>
>>if (OptionNumber != NULL) {
>> -*OptionNumber = 0;
>> -for (Index = VariableNameLen - 4; Index < VariableNameLen; Index++) {
>> -  Uint = BmCharToUint (VariableName[Index]);
>> -  if (Uint == -1) {
>> -break;
>> -  } else {
>> -*OptionNumber = (UINT16) Uint + *OptionNumber * 0x10;
>> -  }
>> -}
>> +*OptionNumber = LocalOptionNumber;
>>}
>>
>> -  return (BOOLEAN) (Index == VariableNameLen);
>> +  return TRUE;
>>  }
>>
>>  /**
>>
>
> I have some superficial comments:
>
> - I think the subject should say
>
> MdeModulePkg/Bds: Check variable name even *if* OptionNumber is NULL
>
> - I'm not a huge fan of using enum types for loop counters; however, in
> this case, I verified that there is LoadOptionTypeMax, and it equals
> ARRAY_SIZE (mBmLoadOptionName). So it should be safe. If we want to be
> more explicit about this, we could replace
>
>   CHAR16 *mBmLoadOptionName[] = {
>
> with
>
>   CHAR16 *mBmLoadOptionName[LoadOptionTypeMax] = {
>
> Anyway,
>
> Reviewed-by: Laszlo Ersek 
>
> Ard, can you test this patch in your original environment?
>

I will try this as soon as I get a chance (I have to switch back to an
older branch of the platform I am currently developing for)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 7/8] Marvell/Armada: Armada70x0Lib: Add support for 32-bit ARM

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

Add an ARM implementation of ArmPlatformHelper.S.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S | 77 

 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf   |  3 +
 2 files changed, 80 insertions(+)

diff --git 
a/Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S
new file mode 100644
index 000..21459e5
--- /dev/null
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S
@@ -0,0 +1,77 @@
+//Based on 
ArmPlatformPkg/Library/ArmPlatformLibNull/AArch64/ArmPlatformHelper.S
+//
+//  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
+//  Copyright (c) 2016, Marvell. All rights reserved.
+//  Copyright (c) 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 
+#include 
+
+#define CCU_MC_BASE 0xF0001700
+#define CCU_MC_RCR_OFFSET   0x0
+#define CCU_MC_RCR_REMAP_EN BIT0
+#define CCU_MC_RCR_REMAP_SIZE(Size) (((Size) - 1) ^ (SIZE_1MB - 1))
+
+#define CCU_MC_RSBR_OFFSET  0x4
+#define CCU_MC_RSBR_SOURCE_BASE(Base)   (((Base) >> 20) << 10)
+#define CCU_MC_RTBR_OFFSET  0x8
+#define CCU_MC_RTBR_TARGET_BASE(Base)   (((Base) >> 20) << 10)
+
+ASM_FUNC(ArmPlatformPeiBootAction)
+  .if   FixedPcdGet64 (PcdSystemMemoryBase) != 0
+  .err  PcdSystemMemoryBase should be 0x0 on this platform!
+  .endif
+
+  .if   FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 
(PcdDramRemapTarget)
+//
+// Use the low range for UEFI itself. The remaining memory will be mapped
+// and added to the GCD map later.
+//
+ADRL  (r0, mSystemMemoryEnd)
+MOV32 (r2, FixedPcdGet32 (PcdDramRemapTarget) - 1)
+mov   r3, #0
+strd  r2, r3, [r0]
+  .endif
+
+  bxlr
+
+//UINTN
+//ArmPlatformGetCorePosition (
+//  IN UINTN MpId
+//  );
+// With this function: CorePos = (ClusterId * 2) + CoreId
+ASM_FUNC(ArmPlatformGetCorePosition)
+  and   r1, r0, #ARM_CORE_MASK
+  and   r0, r0, #ARM_CLUSTER_MASK
+  add   r0, r1, r0, LSR #7
+  bxlr
+
+//UINTN
+//ArmPlatformGetPrimaryCoreMpId (
+//  VOID
+//  );
+ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
+  MOV32 (r0, FixedPcdGet32(PcdArmPrimaryCore))
+  bxlr
+
+//UINTN
+//ArmPlatformIsPrimaryCore (
+//  IN UINTN MpId
+//  );
+ASM_FUNC(ArmPlatformIsPrimaryCore)
+  MOV32 (r1, FixedPcdGet32(PcdArmPrimaryCoreMask))
+  and   r0, r0, r1
+  MOV32 (r1, FixedPcdGet32(PcdArmPrimaryCore))
+  cmp   r0, r1
+  moveq r0, #1
+  movne r0, #0
+  bxlr
diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
index 838a670..0dabd4b 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
@@ -60,6 +60,9 @@
 [Sources.AArch64]
   AArch64/ArmPlatformHelper.S
 
+[Sources.ARM]
+  ARM/ArmPlatformHelper.S
+
 [FixedPcd]
   gArmTokenSpaceGuid.PcdSystemMemoryBase
   gArmTokenSpaceGuid.PcdSystemMemorySize
-- 
2.7.4

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


[edk2] [platforms: PATCH 6/8] Marvell/Armada: Enable dynamic DRAM size detection

2017-10-11 Thread Marcin Wojtas
Instead of using hardcoded value in PcdSystemMemorySize PCD,
obtain DRAM size directly from SoC registers, which are filled
by firmware during early initialization stage.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c | 111 
+++-
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h |  49 
+
 2 files changed, 159 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
index 2cb2e15..22cbe47 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
@@ -36,8 +36,11 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include 
 #include 
 #include 
+#include 
 #include 
 
+#include "Armada70x0LibMem.h"
+
 // The total number of descriptors, including the final "end-of-table" 
descriptor.
 #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 16
 
@@ -47,6 +50,105 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 STATIC ARM_MEMORY_REGION_DESCRIPTOR 
VirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
 
+// Obtain DRAM size basing on register values filled by early firmware.
+STATIC
+UINT64
+DramSizeGet (
+  UINT64 *MemSize
+  )
+{
+  UINT64 BaseAddr;
+  UINT32 RegVal;
+  UINT8 AreaLengthMap;
+  UINT8 Cs;
+
+  *MemSize = 0;
+
+  for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) {
+
+RegVal = MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs));
+
+/* Exit loop on first disabled DRAM CS */
+if (!(RegVal & DRAM_CS_VALID_ENABLED_MASK)) {
+  break;
+}
+
+/*
+ * Sanity check for base address of next DRAM block.
+ * Only continuous space will be used.
+ */
+BaseAddr = ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG (Cs)) <<
+DRAM_START_ADDR_HTOL_OFFS) | (RegVal & 
DRAM_START_ADDRESS_L_MASK);
+if (BaseAddr != *MemSize) {
+  DEBUG ((DEBUG_ERROR,
+"DramSizeGet: DRAM blocks are not contiguous, limit size to 0x%llx\n",
+*MemSize));
+  return EFI_SUCCESS;
+}
+
+/* Decode area length for current CS from register value */
+AreaLengthMap = ((RegVal & DRAM_AREA_LENGTH_MASK) >> 
DRAM_AREA_LENGTH_OFFS);
+switch (AreaLengthMap) {
+case 0x0:
+  *MemSize += 0x1800;
+  break;
+case 0x1:
+  *MemSize += 0x3000;
+  break;
+case 0x2:
+  *MemSize += 0x6000;
+  break;
+case 0x3:
+  *MemSize += 0xC000;
+  break;
+case 0x7:
+  *MemSize += 0x0080;
+  break;
+case 0x8:
+  *MemSize += 0x0100;
+  break;
+case 0x9:
+  *MemSize += 0x0200;
+  break;
+case 0xA:
+  *MemSize += 0x0400;
+  break;
+case 0xB:
+  *MemSize += 0x0800;
+  break;
+case 0xC:
+  *MemSize += 0x1000;
+  break;
+case 0xD:
+  *MemSize += 0x2000;
+  break;
+case 0xE:
+  *MemSize += 0x4000;
+  break;
+case 0xF:
+  *MemSize += 0x8000;
+  break;
+case 0x10:
+  *MemSize += 0x1;
+  break;
+case 0x11:
+  *MemSize += 0x2;
+  break;
+case 0x12:
+  *MemSize += 0x4;
+  break;
+case 0x13:
+  *MemSize += 0x8;
+  break;
+default:
+  DEBUG ((DEBUG_ERROR, "Invalid area length (0x%x) for CS#%d\n", 
AreaLengthMap, Cs));
+  return EFI_INVALID_PARAMETER;
+}
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Return the Virtual Memory Map of your platform
 
@@ -68,10 +170,17 @@ ArmPlatformGetVirtualMemoryMap (
   UINT64MemHighStart;
   UINT64MemHighSize;
   EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
+  EFI_STATUSStatus;
 
   ASSERT (VirtualMemoryMap != NULL);
 
-  MemSize = FixedPcdGet64 (PcdSystemMemorySize);
+  // Obtain total memory size from the hardware.
+  Status = DramSizeGet ();
+  if (EFI_ERROR (Status)) {
+MemSize = FixedPcdGet64 (PcdSystemMemorySize);
+DEBUG ((DEBUG_ERROR, "Limit total memory size to %d MB\n", MemSize / 1024 
/ 1024));
+  }
+
   MemLowSize = MIN (FixedPcdGet64 (PcdDramRemapTarget), MemSize);
   MemHighStart = (UINT64)FixedPcdGet64 (PcdDramRemapTarget) +
  FixedPcdGet32 (PcdDramRemapSize);
diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
new file mode 100644
index 000..b81fd1d
--- /dev/null
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
@@ -0,0 +1,49 @@
+/***
+Copyright (C) 2017 Marvell International Ltd.
+
+Marvell BSD License Option
+
+If you received this File from Marvell, you may opt to use, redistribute and/or
+modify 

[edk2] [platforms: PATCH 5/8] Marvell/Armada: Add MemoryInitPeiLib that reserves secure region

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

The default MemoryInitPeiLib implementation insists on reserving the
region occupied by our own FV, while this is not necessary at all (the
compressed payload is uncompressed elsewhere, so the moment we enter
DXE core, we don't care about the FV contents in memory)

So clone MemoryInitPeiLib and modify it to suit our needs.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada.dsc.inc 
   |   6 +-
 
Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
   | 158 
 
Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
 |  46 ++
 Platform/Marvell/Marvell.dec   
   |   8 +
 4 files changed, 217 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 56d8941..b0a8240 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -153,7 +153,7 @@
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
 
 [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
-  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
+  
MemoryInitPeiLib|Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
 
 [LibraryClasses.common.DXE_CORE]
@@ -364,6 +364,10 @@
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
   gArmTokenSpaceGuid.PcdArmScr|0x531
 
+  # Secure region reservation
+  gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400
+  gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x020
+
   # TRNG
   gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276
 
diff --git 
a/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
 
b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
new file mode 100644
index 000..53119f4
--- /dev/null
+++ 
b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
@@ -0,0 +1,158 @@
+/** @file
+*
+*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+*  Copyright (c) 2017, 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.
+*
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+VOID
+BuildMemoryTypeInformationHob (
+  VOID
+  );
+
+STATIC
+VOID
+InitMmu (
+  IN ARM_MEMORY_REGION_DESCRIPTOR  *MemoryTable
+  )
+{
+
+  VOID  *TranslationTableBase;
+  UINTN TranslationTableSize;
+  RETURN_STATUS Status;
+
+  Status = ArmConfigureMmu (MemoryTable,
+,
+);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Error: Failed to enable MMU\n"));
+  }
+}
+
+/*++
+
+Routine Description:
+
+
+
+Arguments:
+
+  FileHandle  - Handle of the file being invoked.
+  PeiServices - Describes the list of possible PEI Services.
+
+Returns:
+
+  Status -  EFI_SUCCESS if the boot mode could be set
+
+--*/
+EFI_STATUS
+EFIAPI
+MemoryPeim (
+  IN EFI_PHYSICAL_ADDRESS   UefiMemoryBase,
+  IN UINT64 UefiMemorySize
+  )
+{
+  ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
+  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
+  UINT64   ResourceLength;
+  EFI_PEI_HOB_POINTERS NextHob;
+  EFI_PHYSICAL_ADDRESS SecureTop;
+  EFI_PHYSICAL_ADDRESS ResourceTop;
+
+  // Get Virtual Memory Map from the Platform Library
+  ArmPlatformGetVirtualMemoryMap ();
+
+  SecureTop = (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdSecureRegionBase) +
+  FixedPcdGet32 (PcdSecureRegionSize);
+
+  //
+  // Search for System Memory Hob that covers the secure firmware,
+  // and punch a hole in it
+  //
+  for (NextHob.Raw = GetHobList ();
+   NextHob.Raw != NULL;
+   NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR,
+ NextHob.Raw)) {
+
+if ((NextHob.ResourceDescriptor->ResourceType == 
EFI_RESOURCE_SYSTEM_MEMORY) &&
+(FixedPcdGet64 (PcdSecureRegionBase) >= 
NextHob.ResourceDescriptor->PhysicalStart) &&
+(SecureTop <= NextHob.ResourceDescriptor->PhysicalStart +
+  

[edk2] [platforms: PATCH 4/8] Marvell/Armada: Add support from DRAM remapping

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

The Armada 70x0/80x0 DRAM controller allows a single window of DRAM
to be remapped to another location in the physical address space. This
allows us to free up some memory in the 32-bit addressable region for
peripheral MMIO and PCI MMIO32 and CONFIG spaces. This patch adjusts
memory blocks to the configuration done in ATF.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 15 
+
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf   |  3 
+
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c  | 60 

 Platform/Marvell/Marvell.dec  | 13 
+
 4 files changed, 81 insertions(+), 10 deletions(-)

diff --git 
a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
index 72f8cfc..c5be1a9 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
@@ -17,6 +17,21 @@
 
 ASM_FUNC(ArmPlatformPeiBootAction)
   mov   x29, xzr
+
+  .if   FixedPcdGet64 (PcdSystemMemoryBase) != 0
+  .err  PcdSystemMemoryBase should be 0x0 on this platform!
+  .endif
+
+  .if   FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 
(PcdDramRemapTarget)
+//
+// Use the low range for UEFI itself. The remaining memory will be mapped
+// and added to the GCD map later.
+//
+adr   x0, mSystemMemoryEnd
+MOV64 (x1, FixedPcdGet32 (PcdDramRemapTarget) - 1)
+str   x1, [x0]
+  .endif
+
   ret
 
 //UINTN
diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
index 2e198c3..838a670 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
@@ -67,5 +67,8 @@
   gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
   gArmTokenSpaceGuid.PcdArmPrimaryCore
 
+  gMarvellTokenSpaceGuid.PcdDramRemapSize
+  gMarvellTokenSpaceGuid.PcdDramRemapTarget
+
 [Ppis]
   gArmMpCoreInfoPpiGuid
diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
index 74c9956..2cb2e15 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
@@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include 
 #include 
 #include 
+#include 
 #include 
 
 // The total number of descriptors, including the final "end-of-table" 
descriptor.
@@ -44,6 +45,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define DDR_ATTRIBUTES_CACHED   
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK
 #define DDR_ATTRIBUTES_UNCACHED 
ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED
 
+STATIC ARM_MEMORY_REGION_DESCRIPTOR 
VirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
+
 /**
   Return the Virtual Memory Map of your platform
 
@@ -59,20 +62,41 @@ ArmPlatformGetVirtualMemoryMap (
   IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap
   )
 {
-  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
   UINTN Index = 0;
+  UINT64MemSize;
+  UINT64MemLowSize;
+  UINT64MemHighStart;
+  UINT64MemHighSize;
+  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
 
   ASSERT (VirtualMemoryMap != NULL);
 
-  VirtualMemoryTable = 
(ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages(EFI_SIZE_TO_PAGES 
(sizeof(ARM_MEMORY_REGION_DESCRIPTOR) * MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
-  if (VirtualMemoryTable == NULL) {
-  return;
-  }
+  MemSize = FixedPcdGet64 (PcdSystemMemorySize);
+  MemLowSize = MIN (FixedPcdGet64 (PcdDramRemapTarget), MemSize);
+  MemHighStart = (UINT64)FixedPcdGet64 (PcdDramRemapTarget) +
+ FixedPcdGet32 (PcdDramRemapSize);
+  MemHighSize = MemSize - MemLowSize;
+
+  ResourceAttributes = (
+  EFI_RESOURCE_ATTRIBUTE_PRESENT |
+  EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+  EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
+  EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
+  EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
+  EFI_RESOURCE_ATTRIBUTE_TESTED
+  );
+
+  BuildResourceDescriptorHob (
+EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+FixedPcdGet64 (PcdSystemMemoryBase),
+MemLowSize
+);
 
   // DDR
-  VirtualMemoryTable[Index].PhysicalBase= PcdGet64 (PcdSystemMemoryBase);
-  VirtualMemoryTable[Index].VirtualBase = PcdGet64 (PcdSystemMemoryBase);
-  

[edk2] [platforms: PATCH 1/8] Marvell/Armada: Implement EFI_RNG_PROTOCOL driver for EIP76 TRNG

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

Add an implementation of EFI_RNG_PROTOCOL so that the OS loader has
access to entropy for KASLR and other purposes (i.e., seeding the OS's
entropy pool very early on).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada.dsc.inc|   4 +
 Platform/Marvell/Armada/Armada70x0.fdf|   1 +
 Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c   | 254 

 Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf |  47 

 Platform/Marvell/Marvell.dec  |   3 +
 5 files changed, 309 insertions(+)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 1aa485c..ec24d76 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -364,6 +364,9 @@
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
   gArmTokenSpaceGuid.PcdArmScr|0x531
 
+  # TRNG
+  gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform
@@ -400,6 +403,7 @@
   Platform/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf
   Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
   Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
+  Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf
 
   # Network support
   MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
b/Platform/Marvell/Armada/Armada70x0.fdf
index 933c3ed..a94a9ff 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -113,6 +113,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF Platform/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf
   INF Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
   INF Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
+  INF Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf
 
   # Network support
   INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
diff --git 
a/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c 
b/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
new file mode 100644
index 000..dca6dcc
--- /dev/null
+++ b/Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
@@ -0,0 +1,254 @@
+/** @file
+
+  This driver produces an EFI_RNG_PROTOCOL instance for the Armada 70x0 TRNG
+
+  Copyright (C) 2017, Linaro Ltd. 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 
+#include 
+
+#include 
+
+#define TRNG_OUTPUT_REG mTrngBaseAddress
+#define TRNG_OUTPUT_SIZE0x10
+
+#define TRNG_STATUS_REG (mTrngBaseAddress + 0x10)
+#define TRNG_STATUS_READY   BIT0
+
+#define TRNG_INTACK_REG (mTrngBaseAddress + 0x10)
+#define TRNG_INTACK_READY   BIT0
+
+#define TRNG_CONTROL_REG(mTrngBaseAddress + 0x14)
+#define TRNG_CONTROL_REG_ENABLE BIT10
+
+#define TRNG_CONFIG_REG (mTrngBaseAddress + 0x18)
+#define __MIN_REFILL_SHIFT  0
+#define __MAX_REFILL_SHIFT  16
+#define TRNG_CONFIG_MIN_REFILL_CYCLES   (0x05 << __MIN_REFILL_SHIFT)
+#define TRNG_CONFIG_MAX_REFILL_CYCLES   (0x22 << __MAX_REFILL_SHIFT)
+
+#define TRNG_FRODETUNE_REG  (mTrngBaseAddress + 0x24)
+#define TRNG_FRODETUNE_MASK 0x0
+
+#define TRNG_FROENABLE_REG  (mTrngBaseAddress + 0x20)
+#define TRNG_FROENABLE_MASK 0xff
+
+#define TRNG_MAX_RETRIES20
+
+STATIC EFI_PHYSICAL_ADDRESS mTrngBaseAddress;
+
+/**
+  Returns information about the random number generation implementation.
+
+  @param[in] This A pointer to the EFI_RNG_PROTOCOL
+  instance.
+  @param[in,out] RNGAlgorithmListSize On input, the size in bytes of
+  RNGAlgorithmList.
+  On output with a return code of
+  EFI_SUCCESS, the 

[edk2] [platforms: PATCH 0/8] Armada 7k/8k - memory improvements

2017-10-11 Thread Marcin Wojtas
Hi,

We have a good upstream momentum, so let's keep it:)
This patchset is a second part of general platform support improvements.
Mostly, it consists of the changes around DRAM handling (remapping feature,
dynamic size detection and others). It also enables 32-bit ARM build and
implements EFI_RNG_PROTOCOL driver.

The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/mem-upstream-r20171011

I'm looking forward to your comments or remarks.

Best regards,
Marcin

Ard Biesheuvel (6):
  Marvell/Armada: Implement EFI_RNG_PROTOCOL driver for EIP76 TRNG
  Marvell/Armada: Increase preallocated memory region size
  Marvell/Armada: Add support from DRAM remapping
  Marvell/Armada: Add MemoryInitPeiLib that reserves secure region
  Marvell/Armada: Armada70x0Lib: Add support for 32-bit ARM
  Marvell/Armada: Add 32-bit ARM support

Marcin Wojtas (2):
  Marvell/Armada: Remove custom reset library residues
  Marvell/Armada: Enable dynamic DRAM size detection

 Platform/Marvell/Armada/Armada.dsc.inc 
   |  21 +-
 Platform/Marvell/Armada/Armada70x0.dsc 
   |   8 +-
 Platform/Marvell/Armada/Armada70x0.fdf 
   |   3 +-
 Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
   | 254 
 Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf  
   |  47 
 Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S  
   |  15 ++
 Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S  
   |  77 ++
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
   |   6 +
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c   
   | 167 -
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h   
   |  49 
 
Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
   | 158 
 
Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
 |  46 
 Platform/Marvell/Marvell.dec   
   |  28 ++-
 Silicon/Marvell/Documentation/PortingGuide.txt 
   |   9 -
 14 files changed, 852 insertions(+), 36 deletions(-)
 create mode 100644 
Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.c
 create mode 100644 
Platform/Marvell/Armada/Drivers/Armada70x0RngDxe/Armada70x0RngDxe.inf
 create mode 100644 
Platform/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S
 create mode 100644 
Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
 create mode 100644 
Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
 create mode 100644 
Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf

-- 
2.7.4

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


Re: [edk2] [platforms: PATCH v2 00/12] Armada 7k/8k - misc improvements

2017-10-11 Thread Marcin Wojtas
2017-10-11 13:38 GMT+02:00 Leif Lindholm :
> On Wed, Oct 11, 2017 at 12:15:27PM +0200, Marcin Wojtas wrote:
>> Hi,
>>
>> This is a second version of misc, general improvements patchset.
>> There are minor changes, comparing to v1 - please refer to the
>> changelog below.
>>
>> The patches are available in the github:
>> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/misc-upstream-r20171011
>>
>> I'm looking forward to your comments or remarks.
>>
>> Best regards,
>> Marcin
>>
>> Changelog:
>> v1 -> v2
>> * Remove patch: "Marvell/Armada: Armada70x0Lib: Clean FV in the D-cache
>>   before boot"
>>
>> * 1/12
>>   - Specify contribution in new way in the commit log
>>   - Add Linaro copyrights to new files
>>
>> * 8/13
>>   - Improve commit log
>>
>> * 11/13
>>   - Fix indentation
>>
>> * Others
>>   - Add RB's
>
> Thanks - for the series:
> Reviewed-by: Leif Lindholm 
>
> Pushed as 5d6506e4e9..24404d5300.
>

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


Re: [edk2] [platforms: PATCH v2 00/12] Armada 7k/8k - misc improvements

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 12:15:27PM +0200, Marcin Wojtas wrote:
> Hi,
> 
> This is a second version of misc, general improvements patchset.
> There are minor changes, comparing to v1 - please refer to the
> changelog below.
> 
> The patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/misc-upstream-r20171011
> 
> I'm looking forward to your comments or remarks.
> 
> Best regards,
> Marcin
> 
> Changelog:
> v1 -> v2
> * Remove patch: "Marvell/Armada: Armada70x0Lib: Clean FV in the D-cache
>   before boot"
> 
> * 1/12
>   - Specify contribution in new way in the commit log
>   - Add Linaro copyrights to new files
> 
> * 8/13
>   - Improve commit log
> 
> * 11/13
>   - Fix indentation
> 
> * Others
>   - Add RB's

Thanks - for the series:
Reviewed-by: Leif Lindholm 

Pushed as 5d6506e4e9..24404d5300.

> Ard Biesheuvel (10):
>   Marvell/Armada: Switch to dynamic PCDs
>   Marvell/Armada: Armada70x0Lib: Terminate call stack list at entry
>   Marvell/Armada: Use 4k/64k aligned sections for DXE/DXE-rt modules
>   Marvell/Armada: Switch to generic BDS
>   Marvell/Armada: Re-enable driver model diagnostics PCDs
>   Marvell/Armada: Ensure GICC frames adjacency
>   Marvell/Armada: Disable PerformanceLibrary
>   Marvell/Armada: Switch to unicore PrePi
>   Marvell/Armada: Remove outdated SEC alignment override
>   Marvell/Armada: Add the UefiPxeBcDxe driver
> 
> Marcin Wojtas (2):
>   Marvell/Armada: Introduce platform initialization driver
>   Marvell/Documentation: Follow EDK2 coding style in the PortingGuide
> 
>  Platform/Marvell/Armada/Armada.dsc.inc|  
> 55 +-
>  Platform/Marvell/Armada/Armada70x0.fdf|  
> 23 +-
>  Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c |  
> 45 ++
>  Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf   |  
> 45 ++
>  Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S |  
>  1 +
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c |  
> 11 -
>  Platform/Marvell/Marvell.dec  |  
>  5 +
>  Silicon/Marvell/Documentation/PortingGuide.txt| 
> 800 ++--
>  8 files changed, 550 insertions(+), 435 deletions(-)
>  create mode 100644 Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
>  create mode 100644 
> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
> 
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH v2 12/12] Marvell/Documentation: Follow EDK2 coding style in the PortingGuide

2017-10-11 Thread Marcin Wojtas
This patch removes tabs and wrong line endings in the file, maiking
it acceptable to the PatchCheck.py script.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Silicon/Marvell/Documentation/PortingGuide.txt | 800 ++--
 1 file changed, 400 insertions(+), 400 deletions(-)

diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt 
b/Silicon/Marvell/Documentation/PortingGuide.txt
index f0da515..66ec918 100644
--- a/Silicon/Marvell/Documentation/PortingGuide.txt
+++ b/Silicon/Marvell/Documentation/PortingGuide.txt
@@ -1,400 +1,400 @@
-UEFI Porting Guide
-==
-
-This document provides instructions for adding support for new Marvell Armada
-board. For the sake of simplicity new Marvell board will be called "new_board".
-
-1. Create configuration files for new target
-   1.1 Create FDF file for new board
-
-- Copy and rename 
edk2-platforms/Platform/Marvell/Armada/Armada70x0.fdf to
-  edk2-platforms/Platform/Marvell/Armada/new_board.fdf
-- Change the first no-comment line:
-  [FD.Armada70x0_EFI] to [FD.{new_board}_EFI]
-
-   1.2 Create DSC file for new board
-
-- Add new_board.dsc file to edk2-platforms/Platform/Marvell/Armada 
directory
-- Insert following [Defines] section to new_board.dsc:
-
-   [Defines]
- PLATFORM_NAME  = {new_board}
- PLATFORM_GUID  = 
{newly_generated_GUID}
- PLATFORM_VERSION   = 0.1
- DSC_SPECIFICATION  = 0x00010019
- OUTPUT_DIRECTORY   = {output_directory}
- SUPPORTED_ARCHITECTURES= AARCH64
- BUILD_TARGETS  = DEBUG|RELEASE
- SKUID_IDENTIFIER   = DEFAULT
- FLASH_DEFINITION   = {path_to_fdf_file}
-
-- Add "!include Armada.dsc.inc" entry to new_board.dsc
-
-2. Driver support
- - According to content of files from
-   edk2-platforms/Silicon/Marvell/Documentation/PortingGuide.txt
-   insert PCD entries into new_board.dsc for every needed interface (as listed 
below).
-
-3. Compilation
- - Refer to edk2-platforms/Platform/Marvell/Readme.md. Remember to change
-   {platform} to new_board in order to point build system to newly created DSC 
file.
-
-4. Output file
- - Output files (and among others FD file, which may be used by ATF) are
-   generated under directory pointed by "OUTPUT_DIRECTORY" entry (see point 
1.2).
-
-
-COMPHY configuration
-
-In order to configure ComPhy library, following PCDs are available:
-
-  - gMarvellTokenSpaceGuid.PcdComPhyDevices
-
-This array indicates, which ones of the ComPhy chips defined in
-MVHW_COMPHY_DESC template will be configured.
-
-Every ComPhy PCD has  part where  stands for chip ID (order is not
-important, but configuration will be set for first PcdComPhyChipCount chips).
-
-Every chip has 3 ComPhy PCDs and three of them comprise per-board lanes
-settings for this chip. Their format is array of up to 10 values reflecting
-defined numbers for SPEED/TYPE/INVERT, whose description can be found in:
-
-  OpenPlatformPkg/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
-
-  - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
-   (Array of types - currently supported are:
-
-   CP_UNCONNECTED  0x0
-   CP_PCIE00x1
-   CP_PCIE10x2
-   CP_PCIE20x3
-   CP_PCIE30x4
-   CP_SATA00x5
-   CP_SATA10x6
-   CP_SATA20x7
-   CP_SATA30x8
-   CP_SGMII0   0x9
-   CP_SGMII1   0xA
-   CP_SGMII2   0xB
-   CP_SGMII3   0xC
-   CP_QSGMII   0xD
-   CP_USB3_HOST0   0xE
-   CP_USB3_HOST1   0xF
-   CP_USB3_DEVICE  0x10
-   CP_XAUI00x11
-   CP_XAUI10x12
-   CP_XAUI20x13
-   CP_XAUI30x14
-   CP_RXAUI0   0x15
-   CP_RXAUI1   0x16
-   CP_SFI  0x17 )
-
-  - gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds
-   (Array of speeds - currently supported are:
-
-   CP_1_25G0x1
-   CP_1_5G 0x2
-   CP_2_5G 0x3
-   CP_3G

[edk2] [platforms: PATCH v2 10/12] Marvell/Armada: Remove outdated SEC alignment override

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

The FDFs no longer require explicit alignment for sections containing
aligned objects, so change it to 'Auto' and FIXED (which allows some
padding to be removed), and remove some other cruft while at it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada70x0.fdf | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
b/Platform/Marvell/Armada/Armada70x0.fdf
index 2c3efe0..3f0471d 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -235,16 +235,9 @@ READ_LOCK_STATUS   = TRUE
 #
 
 
-[Rule.ARM.SEC]
-  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
-TE  TEAlign = 32$(INF_OUTPUT)/$(MODULE_NAME).efi
-  }
-
-# The AArch64 Vector Table requires a 2K alignment that is not supported by 
the FDF specification.
-# It is the reason 4K is used instead of 2K for the module alignment.
 [Rule.AARCH64.SEC]
-  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
-TE  TEAlign = 4K$(INF_OUTPUT)/$(MODULE_NAME).efi
+  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
+TE  TEAlign = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
   }
 
 [Rule.Common.PEI_CORE]
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 07/12] Marvell/Armada: Ensure GICC frames adjacency

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

The GIC architecture mandates that the CPU interface, which consists
of 2 consecutive 4 KB frames, can be mapped using separate mappings.
Since this is problematic on 64 KB pages, the MMU-400 aliases each
frame 16 times, and the two consecutive frames can be found at offset
0xf000.

Therefore use the last alias from the first series of aliases as the
base address, so that the first frame from the second series becomes
directly adjacent, whilst remaining covered by a separate 64KB page.

This patch is intended to expose correct GICC alias via
MADT, once ACPI support is added.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 5071bd5..bd2336f 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -263,7 +263,14 @@
 
   # ARM Generic Interrupt Controller
   gArmTokenSpaceGuid.PcdGicDistributorBase|0xF021
-  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xF022
+
+  #
+  # NOTE: the GIC architecture mandates that the CPU interface, which consists
+  # of 2 consecutive 4 KB frames, can be mapped using separate mappings.
+  # Since this is problematic on 64 KB pages, the MMU-400 aliases each frame
+  # 16 times, and the two consecutive frames can be found at offset 0xf000
+  #
+  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xF022F000
 
   # ARM Architectural Timer Support
   gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|2500
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 11/12] Marvell/Armada: Add the UefiPxeBcDxe driver

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

This driver allows automatic booting via the network.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 1 +
 Platform/Marvell/Armada/Armada70x0.fdf | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 5a5fde9..1aa485c 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -412,6 +412,7 @@
   MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
   MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
   MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
+  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
   Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf
   Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf
   Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
b/Platform/Marvell/Armada/Armada70x0.fdf
index 3f0471d..933c3ed 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -125,6 +125,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
   INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
   INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
+  INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
   INF Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf
   INF Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf
   INF Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 06/12] Marvell/Armada: Re-enable driver model diagnostics PCDs

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

For some reason, one of the early ARM platforms disabled all the
diagnostics related to the UEFI driver model, resulting in the
output of UEFI shell utilities such as 'devices' or 'drivers' to
become completely useless. Armada's shared .DSC include file
inherited this for no good reason, so let's revert it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 4 
 1 file changed, 4 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index e920461..5071bd5 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -207,10 +207,6 @@
 

 
 [PcdsFeatureFlag.common]
-  gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable|TRUE
-  gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnosticsDisable|TRUE
-  gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
-  gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE
 
   #
   # Control what commands are supported from the UI
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 08/12] Marvell/Armada: Disable PerformanceLibrary

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

Remove the gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask
setting so it reverts to its default of 0, and disables performance
profiling.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index bd2336f..b718c60 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -251,7 +251,6 @@
   gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|100
   gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|1000
   gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
-  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
   gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 09/12] Marvell/Armada: Switch to unicore PrePi

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

There is no point in using the MPCore PrePi, given that only the primary
core will enter UEFI at EL2, and the secondaries will be held in EL3
until summoned by the OS. So use the unicore flavour instead.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 2 +-
 Platform/Marvell/Armada/Armada70x0.fdf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index b718c60..5a5fde9 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -372,7 +372,7 @@
 [Components.common]
 
   # PEI Phase modules
-  ArmPlatformPkg/PrePi/PeiMPCore.inf
+  ArmPlatformPkg/PrePi/PeiUniCore.inf
 
   # DXE
   MdeModulePkg/Core/Dxe/DxeMain.inf {
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
b/Platform/Marvell/Armada/Armada70x0.fdf
index 999b968..2c3efe0 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -199,7 +199,7 @@ READ_STATUS= TRUE
 READ_LOCK_CAP  = TRUE
 READ_LOCK_STATUS   = TRUE
 
-  INF ArmPlatformPkg/PrePi/PeiMPCore.inf
+  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
 
   FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
 SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = 
TRUE {
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 05/12] Marvell/Armada: Switch to generic BDS

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

Switch from the Intel BDS to the generic BDS, which is preferred for
ARM platforms given that it is completely legacy free.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 19 ---
 Platform/Marvell/Armada/Armada70x0.fdf |  3 ++-
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 1b4e713..e920461 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -126,8 +126,9 @@
 
   # BDS Libraries
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
-  GenericBdsLib|IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
-  
PlatformBdsLib|ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
+  BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
+  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
+  
PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
   
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
 
@@ -350,6 +351,12 @@
   # Shell.
   gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
 
+  # GUID of the generic BDS UiApp
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 
0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
+
+  # use the 'TTYTERM' terminal type for optimal compatibility with Linux 
terminal emulators
+  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
+
   # ARM Pcds
   gArmTokenSpaceGuid.PcdSystemMemoryBase|0
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
@@ -459,7 +466,13 @@
   MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
   MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
-  IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
+  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  MdeModulePkg/Application/UiApp/UiApp.inf {
+
+  NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
+  NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
+  
NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
+  }
 
   # UEFI application (Shell Embedded Boot Loader)
   ShellPkg/Application/Shell/Shell.inf {
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
b/Platform/Marvell/Armada/Armada70x0.fdf
index b3d1c60..999b968 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -175,7 +175,8 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
   INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
-  INF IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
+  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  INF MdeModulePkg/Application/UiApp/UiApp.inf
 
 
 # PEI phase firmware volume
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 04/12] Marvell/Armada: Use 4k/64k aligned sections for DXE/DXE-rt modules

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

Enable strict memory protection at boot time and under the OS, by using
4 KB section alignment for DXE_DRIVER, UEFI_DRIVER and UEFI_APPLICATION
modules, and 64 KB alignment for DXE_RUNTIME_DRIVER modules. Note that
the latter is mandated by the UEFI spec.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 433892e..1b4e713 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -486,6 +486,12 @@
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
   }
 
+[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
+[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1
+
 

 #
 # Defines - platform description macros
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 03/12] Marvell/Armada: Armada70x0Lib: Terminate call stack list at entry

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

To avoid dereferencing junk when walking the call stack in exception
handlers (which may prevent us from getting a full backtrace), set
the frame pointer to 0x0 when first entering UEFI.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
index 9265636..72f8cfc 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
@@ -16,6 +16,7 @@
 #include 
 
 ASM_FUNC(ArmPlatformPeiBootAction)
+  mov   x29, xzr
   ret
 
 //UINTN
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 02/12] Marvell/Armada: Switch to dynamic PCDs

2017-10-11 Thread Marcin Wojtas
From: Ard Biesheuvel 

For full functionality, including HII forms wired to non-volatile UEFI
variables, we need dynamic PCDs as well. So let's enable those.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada.dsc.inc | 10 +++---
 Platform/Marvell/Armada/Armada70x0.fdf |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 417bb0c..433892e 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -67,8 +67,7 @@
   
UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
 
-  # Assume everything is fixed at build. do not use runtime PCD feature
-  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
 
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
 
@@ -150,6 +149,7 @@
   
PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
   PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
   ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
+  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
 
 [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
   MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
@@ -368,10 +368,14 @@
   # DXE
   MdeModulePkg/Core/Dxe/DxeMain.inf {
 
-  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   
NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
   }
 
+  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
+
+  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+
   # Architectural Protocols DXE
   ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
b/Platform/Marvell/Armada/Armada70x0.fdf
index 763d76a..b3d1c60 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -95,6 +95,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
 
   # PI DXE Drivers producing Architectural Protocols (EFI Services)
+  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
   INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
   INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
-- 
2.7.4

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


[edk2] [platforms: PATCH v2 00/12] Armada 7k/8k - misc improvements

2017-10-11 Thread Marcin Wojtas
Hi,

This is a second version of misc, general improvements patchset.
There are minor changes, comparing to v1 - please refer to the
changelog below.

The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/misc-upstream-r20171011

I'm looking forward to your comments or remarks.

Best regards,
Marcin

Changelog:
v1 -> v2
* Remove patch: "Marvell/Armada: Armada70x0Lib: Clean FV in the D-cache
  before boot"

* 1/12
  - Specify contribution in new way in the commit log
  - Add Linaro copyrights to new files

* 8/13
  - Improve commit log

* 11/13
  - Fix indentation

* Others
  - Add RB's

Ard Biesheuvel (10):
  Marvell/Armada: Switch to dynamic PCDs
  Marvell/Armada: Armada70x0Lib: Terminate call stack list at entry
  Marvell/Armada: Use 4k/64k aligned sections for DXE/DXE-rt modules
  Marvell/Armada: Switch to generic BDS
  Marvell/Armada: Re-enable driver model diagnostics PCDs
  Marvell/Armada: Ensure GICC frames adjacency
  Marvell/Armada: Disable PerformanceLibrary
  Marvell/Armada: Switch to unicore PrePi
  Marvell/Armada: Remove outdated SEC alignment override
  Marvell/Armada: Add the UefiPxeBcDxe driver

Marcin Wojtas (2):
  Marvell/Armada: Introduce platform initialization driver
  Marvell/Documentation: Follow EDK2 coding style in the PortingGuide

 Platform/Marvell/Armada/Armada.dsc.inc|  
55 +-
 Platform/Marvell/Armada/Armada70x0.fdf|  
23 +-
 Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c |  
45 ++
 Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf   |  
45 ++
 Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S |   
1 +
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c |  
11 -
 Platform/Marvell/Marvell.dec  |   
5 +
 Silicon/Marvell/Documentation/PortingGuide.txt| 
800 ++--
 8 files changed, 550 insertions(+), 435 deletions(-)
 create mode 100644 Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
 create mode 100644 Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf

-- 
2.7.4

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


[edk2] [platforms: PATCH v2 01/12] Marvell/Armada: Introduce platform initialization driver

2017-10-11 Thread Marcin Wojtas
In order to enable modification of dynamic PCD's for the libraries
and DXE drivers, this patch introduces new driver. It is
executed prior to other drivers. Mpp, ComPhy and Utmi libraries
initialization were moved from PrePi stage to DXE.

To force the correct driver dispatch sequence, introduce a protocol GUID
and install the protocol as a NULL protocol when PlatInitDxe executes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
[Introduce protocol GUID to force correct driver dispatch order]
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada.dsc.inc|  3 ++
 Platform/Marvell/Armada/Armada70x0.fdf|  5 +++
 Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c | 45 

 Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf   | 45 

 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c | 11 -
 Platform/Marvell/Marvell.dec  |  5 +++
 6 files changed, 103 insertions(+), 11 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 89fb7e7..417bb0c 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -378,6 +378,9 @@
   ArmPkg/Drivers/TimerDxe/TimerDxe.inf
   ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
 
+  # Platform Initialization
+  Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
+
   # Platform drivers
   Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
   MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf 
b/Platform/Marvell/Armada/Armada70x0.fdf
index c861e78..763d76a 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -89,6 +89,11 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
 
   INF MdeModulePkg/Core/Dxe/DxeMain.inf
 
+  #
+  # Platform Initialization
+  #
+  INF Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
+
   # PI DXE Drivers producing Architectural Protocols (EFI Services)
   INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c 
b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
new file mode 100644
index 000..1efad77
--- /dev/null
+++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
@@ -0,0 +1,45 @@
+/** @file
+  Copyright (c) 2017, Linaro Limited. All rights reserved.
+  Copyright (c) 2017, Marvell International Ltd. and its affiliates
+
+  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 
+#include 
+#include 
+#include 
+#include 
+
+EFI_STATUS
+EFIAPI
+ArmadaPlatInitDxeEntryPoint (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUSStatus;
+
+  DEBUG ((DEBUG_ERROR, "\nArmada Platform Init\n\n"));
+
+  Status = gBS->InstallProtocolInterface (,
+  ,
+  EFI_NATIVE_INTERFACE,
+  NULL);
+  ASSERT_EFI_ERROR (Status);
+
+  MvComPhyInit ();
+  UtmiPhyInit ();
+  MppInitialize ();
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf 
b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
new file mode 100644
index 000..790b7e3
--- /dev/null
+++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
@@ -0,0 +1,45 @@
+#/* @file
+#  Copyright (c) 2017, Linaro Limited. All rights reserved.
+#  Copyright (c) 2017, Marvell International Ltd. and its affiliates
+#
+#  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= 0x00010019
+  BASE_NAME  = PlatInitDxe
+  FILE_GUID  = 8c66f65b-08a6-4c91-b993-ff81e0adf818
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+
+  ENTRY_POINT= ArmadaPlatInitDxeEntryPoint
+
+[Sources]
+  PlatInitDxe.c
+
+[Packages]
+  

Re: [edk2] [Patch] SourceLevelDebugPkg: Update SmmDebugAgentLib to restore APIC timer

2017-10-11 Thread Ni, Ruiyu
Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Gao, Liming
> Sent: Tuesday, October 10, 2017 6:04 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: [Patch] SourceLevelDebugPkg: Update SmmDebugAgentLib to
> restore APIC timer
> 
> In enter SMI, APIC timer may be initialized. After exit SMI, APIC timer will 
> be
> restore.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao 
> Cc: Ruiyu Ni 
> ---
>  .../DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c | 21
> -
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git
> a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebug
> AgentLib.c
> b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebu
> gAgentLib.c
> index 11afd32..6be8c54 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebug
> AgentLib.c
> +++
> b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebu
> gAgent
> +++ Lib.c
> @@ -20,6 +20,11 @@ UINTN   mSavedDebugRegisters[6];
>  IA32_IDT_GATE_DESCRIPTORmIdtEntryTable[33];
>  BOOLEAN mSkipBreakpoint = FALSE;
>  BOOLEAN mSmmDebugIdtInitFlag = FALSE;
> +BOOLEAN mApicTimerRestore = FALSE;
> +BOOLEAN mPeriodicMode;
> +UINT32  mTimerCycle;
> +UINTN   mApicTimerDivisor;
> +UINT8   mVector;
> 
>  CHAR8 mWarningMsgIgnoreSmmEntryBreak[] = "Ignore smmentrybreak
> setting for SMI issued during DXE debugging!\r\n";
> 
> @@ -191,8 +196,6 @@ InitializeDebugAgent (
>DEBUG_AGENT_MAILBOX   *Mailbox;
>UINT64*MailboxLocation;
>UINT32DebugTimerFrequency;
> -  BOOLEAN   PeriodicMode;
> -  UINTN TimerCycle;
> 
>switch (InitFlag) {
>case DEBUG_AGENT_INIT_SMM:
> @@ -289,9 +292,10 @@ InitializeDebugAgent (
>  // Check if CPU APIC Timer is working, otherwise initialize it.
>  //
>  InitializeLocalApicSoftwareEnable (TRUE);
> -GetApicTimerState (NULL, , NULL);
> -TimerCycle = GetApicTimerInitCount ();
> -if (!PeriodicMode || TimerCycle == 0) {
> +GetApicTimerState (, , );
> +mTimerCycle = GetApicTimerInitCount ();
> +if (!mPeriodicMode || mTimerCycle == 0) {
> +  mApicTimerRestore = TRUE;
>InitializeDebugTimer (NULL, FALSE);
>  }
>  Mailbox = GetMailboxPointer ();
> @@ -327,6 +331,13 @@ InitializeDebugAgent (
>  //
>  mSkipBreakpoint = FALSE;
>  RestoreDebugRegister ();
> +//
> +// Restore APIC Timer
> +//
> +if (mApicTimerRestore) {
> +  InitializeApicTimer (mApicTimerDivisor, mTimerCycle, mPeriodicMode,
> mVector);
> +  mApicTimerRestore = FALSE;
> +}
>  break;
> 
>case DEBUG_AGENT_INIT_THUNK_PEI_IA32TOX64:
> --
> 2.8.0.windows.1

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


Re: [edk2] [platforms: PATCH 01/13] Marvell/Armada: Introduce platform initialization driver

2017-10-11 Thread Marcin Wojtas
2017-10-11 11:14 GMT+02:00 Leif Lindholm :
> On Wed, Oct 11, 2017 at 10:43:14AM +0200, Marcin Wojtas wrote:
>> >> I think it's fairly easy thing, needlessly twisted... How does above
>> >> reflect the requirement to add contributor sign-off to someone else's
>> >> patch (with his authorship and original sign-off - should they be
>> >> removed?)?
>> >
>> > Well, we're not debating this because it's critical for this one
>> > patch, but because it would be useful to have a precedent.
>>
>> I'm totally fine with precedences, it's rather your call, whether it's
>> accepted or not :) My three arugments are:
>> - I have still a lot patches ahead and it's very likely such situation
>> may occur again.
>> - Needless to say, it may happen again in the development of other platforms.
>> - Artificially splitting patches seems to me as not really needed and
>> I'm not convinced to its justification.
>>
>> >> Anyway, let's make a quick decision here - should I submit patch with
>> >> linux-like signatures and description? Or should I split the patches?
>> >
>> > Let's put it this way - if you split the patches, you remove this
>> > series from abovementioned discussion :)
>>
>> If you're ok with it, I'd go with single patch, but I can do it either
>> way - I think I'm not to decide, what's best from maintainers' point
>> of view :)
>
> For now, I would take the single patch with Linux-style description,
> like the example I sent earlier.
>

Great, will send it in v2.

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


Re: [edk2] [platforms: PATCH 01/13] Marvell/Armada: Introduce platform initialization driver

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 10:43:14AM +0200, Marcin Wojtas wrote:
> >> I think it's fairly easy thing, needlessly twisted... How does above
> >> reflect the requirement to add contributor sign-off to someone else's
> >> patch (with his authorship and original sign-off - should they be
> >> removed?)?
> >
> > Well, we're not debating this because it's critical for this one
> > patch, but because it would be useful to have a precedent.
> 
> I'm totally fine with precedences, it's rather your call, whether it's
> accepted or not :) My three arugments are:
> - I have still a lot patches ahead and it's very likely such situation
> may occur again.
> - Needless to say, it may happen again in the development of other platforms.
> - Artificially splitting patches seems to me as not really needed and
> I'm not convinced to its justification.
> 
> >> Anyway, let's make a quick decision here - should I submit patch with
> >> linux-like signatures and description? Or should I split the patches?
> >
> > Let's put it this way - if you split the patches, you remove this
> > series from abovementioned discussion :)
> 
> If you're ok with it, I'd go with single patch, but I can do it either
> way - I think I'm not to decide, what's best from maintainers' point
> of view :)

For now, I would take the single patch with Linux-style description,
like the example I sent earlier.

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


Re: [edk2] [platforms: PATCH 01/13] Marvell/Armada: Introduce platform initialization driver

2017-10-11 Thread Marcin Wojtas
Leif,

2017-10-11 10:32 GMT+02:00 Leif Lindholm :
> On Wed, Oct 11, 2017 at 06:53:05AM +0200, Marcin Wojtas wrote:
>> >> I think Contibuted-under: still needs to come first.
>> >>
>> >> I don't think we have an explicit policy for how to deal with
>> >> multi-contributor patches. The ones we do see tend to just keep a
>> >> single commit message and list the contributors.
>> >>
>> >> In Linux. it would be something like
>> >> Signed-off-by: Marcin Wojtas 
>> >> [Introduce protocol GUID to force correct driver dispatch order]
>> >> Signed-off-by: Ard Biesheuvel 
>> >> Signed-off-by: Marcin Wojtas 
>> >>
>> >> I would be quite happy to use the same format here.
>> >>
>> >
>> > Well, Tianocore still conflates authorship with a statement regarding
>> > the origin of the contribution. I wonder how this is supposed to work
>> > when Linaro engineers such as myself contribute code that was authored
>> > by engineers working in member companies, e.g., Socionext. The license
>> > and the contract that company has with Linaro give me the right to
>> > contribute that code, but that does not make me the author, and I
>> > cannot add a Signed-off-by that wasn't present when we received the
>> > code (even if I knew the name of the author)
>>
>> I think it's fairly easy thing, needlessly twisted... How does above
>> reflect the requirement to add contributor sign-off to someone else's
>> patch (with his authorship and original sign-off - should they be
>> removed?)?
>
> Well, we're not debating this because it's critical for this one
> patch, but because it would be useful to have a precedent.
>

I'm totally fine with precedences, it's rather your call, whether it's
accepted or not :) My three arugments are:
- I have still a lot patches ahead and it's very likely such situation
may occur again.
- Needless to say, it may happen again in the development of other platforms.
- Artificially splitting patches seems to me as not really needed and
I'm not convinced to its justification.

>> Anyway, let's make a quick decision here - should I submit patch with
>> linux-like signatures and description? Or should I split the patches?
>
> Let's put it this way - if you split the patches, you remove this
> series from abovementioned discussion :)
>

If you're ok with it, I'd go with single patch, but I can do it either
way - I think I'm not to decide, what's best from maintainers' point
of view :)

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


Re: [edk2] [platforms: PATCH 01/13] Marvell/Armada: Introduce platform initialization driver

2017-10-11 Thread Leif Lindholm
On Wed, Oct 11, 2017 at 06:53:05AM +0200, Marcin Wojtas wrote:
> >> I think Contibuted-under: still needs to come first.
> >>
> >> I don't think we have an explicit policy for how to deal with
> >> multi-contributor patches. The ones we do see tend to just keep a
> >> single commit message and list the contributors.
> >>
> >> In Linux. it would be something like
> >> Signed-off-by: Marcin Wojtas 
> >> [Introduce protocol GUID to force correct driver dispatch order]
> >> Signed-off-by: Ard Biesheuvel 
> >> Signed-off-by: Marcin Wojtas 
> >>
> >> I would be quite happy to use the same format here.
> >>
> >
> > Well, Tianocore still conflates authorship with a statement regarding
> > the origin of the contribution. I wonder how this is supposed to work
> > when Linaro engineers such as myself contribute code that was authored
> > by engineers working in member companies, e.g., Socionext. The license
> > and the contract that company has with Linaro give me the right to
> > contribute that code, but that does not make me the author, and I
> > cannot add a Signed-off-by that wasn't present when we received the
> > code (even if I knew the name of the author)
> 
> I think it's fairly easy thing, needlessly twisted... How does above
> reflect the requirement to add contributor sign-off to someone else's
> patch (with his authorship and original sign-off - should they be
> removed?)?

Well, we're not debating this because it's critical for this one
patch, but because it would be useful to have a precedent.

> Anyway, let's make a quick decision here - should I submit patch with
> linux-like signatures and description? Or should I split the patches?

Let's put it this way - if you split the patches, you remove this
series from abovementioned discussion :)

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


Re: [edk2] [Patch v2 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.

2017-10-11 Thread Dong, Eric
Jiewen,

Yes, it will always install new protocol and may cause run out of memory. I 
have updated code to uninstall this protocol right after install it. please 
check the V3 patch which fix your two comments.

Thanks,
Eric

-Original Message-
From: Yao, Jiewen 
Sent: Wednesday, October 11, 2017 3:13 PM
To: Dong, Eric ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu 
Subject: RE: [Patch v2 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3 
resume finished.

I have a question for below:

> +  SmmHandle = NULL;
> +  Status = SmmInstallProtocolInterface (
> + ,
> + ,
> + EFI_NATIVE_INTERFACE,
> + NULL
> + );

A platform may do S3 multiple times. What happen if the 
SmmInstallProtocolInterface() called twice?
Will the system has 2 handle and 2 protocol?

Thank you
Yao Jiewen

> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 1:32 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yao, Jiewen 
> Subject: [Patch v2 3/3] MdeModulePkg/PiSmmCore: Install Protocol when 
> S3 resume finished.
> 
> Install EdkiiSmmEndOfS3ResumeProtocol when S3 resume finished.
> S3ResumePei will send S3 resume finished event to SmmCore through 
> communication buffer.
> 
> Cc: Ruiyu Ni 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c   | 55
> +++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h   | 24 ++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf |  1 +
>  3 files changed, 73 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> index 9e4390e..aa44933 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> @@ -75,13 +75,14 @@ BOOLEAN  mInLegacyBoot = FALSE;  // Table of SMI 
> Handlers that are registered by the SMM Core when it is initialized  
> //  SMM_CORE_SMI_HANDLERS  mSmmCoreSmiHandlers[] = {
> -  { SmmDriverDispatchHandler,   ,
> NULL, TRUE  },
> -  { SmmReadyToLockHandler,  ,
> NULL, TRUE },
> -  { SmmLegacyBootHandler,   ,
> NULL, FALSE },
> -  { SmmExitBootServicesHandler, , NULL,
> FALSE },
> -  { SmmReadyToBootHandler,  ,
> NULL, FALSE },
> -  { SmmEndOfDxeHandler, ,
> NULL, TRUE },
> -  { NULL,   NULL,
> NULL, FALSE }
> +  { SmmDriverDispatchHandler,   ,
> NULL, TRUE  },
> +  { SmmReadyToLockHandler,  ,
> NULL, TRUE },
> +  { SmmLegacyBootHandler,   ,
> NULL, FALSE },
> +  { SmmExitBootServicesHandler, ,  NULL,
> FALSE },
> +  { SmmReadyToBootHandler,  ,
> NULL, FALSE },
> +  { SmmEndOfDxeHandler, ,
> NULL, TRUE },
> +  { SmmEndOfS3ResumeHandler,
> , NULL, FALSE },
> +  { NULL,   NULL,
> NULL, FALSE }
>  };
> 
>  UINTN   mFullSmramRangeCount;
> @@ -383,6 +384,46 @@ SmmEndOfDxeHandler (  }
> 
>  /**
> +  Software SMI handler that is called when the EndOfS3Resume event is
> trigged.
> +  This function installs the SMM EndOfS3Resume Protocol so SMM 
> + Drivers are
> informed that
> +  S3 resume has finished.
> +
> +  @param  DispatchHandle  The unique handle assigned to this handler 
> + by
> SmiHandlerRegister().
> +  @param  Context Points to an optional handler context which was
> specified when the handler was registered.
> +  @param  CommBuffer  A pointer to a collection of data in memory
> that will
> +  be conveyed from a non-SMM environment into
> an SMM environment.
> +  @param  CommBufferSize  The size of the CommBuffer.
> +
> +  @return Status Code
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmEndOfS3ResumeHandler (
> +  IN EFI_HANDLE  DispatchHandle,
> +  IN CONST VOID  *Context,OPTIONAL
> +  IN OUT VOID*CommBuffer, OPTIONAL
> +  IN OUT UINTN   *CommBufferSize  OPTIONAL
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_HANDLE  SmmHandle;
> +
> +  DEBUG ((EFI_D_INFO, "SmmEndOfS3ResumeHandler\n"));
> +  //
> +  // Install SMM EndOfDxe protocol
> +  //
> +  SmmHandle = NULL;
> +  Status = SmmInstallProtocolInterface (
> + ,
> + ,
> + EFI_NATIVE_INTERFACE,
> + NULL
> + );
> +  return Status;
> +}
> +
> +/**
>Determine if two buffers overlap in memory.
> 
>@param[in] Buff1  Pointer to first buffer diff --git 
> a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> index b6f815c..6cc824b 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> @@ -32,6 +32,7 @@
>  #include   #include 
>   #include 
> +#include 
> 
>  #include 
>  #include 
> @@ -802,6 +803,29 @@ SmmReadyToBootHandler (
>);
> 

[edk2] [Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.

2017-10-11 Thread Eric Dong
Add gEdkiiSmmEndOfS3ResumeProtocolGuid which used by SmmCore to
notify smm drives that S3 resume has finished.

Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h | 26 
 MdeModulePkg/MdeModulePkg.dec|  3 +++
 2 files changed, 29 insertions(+)
 create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h

diff --git a/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h 
b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
new file mode 100644
index 000..9716f6a
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
@@ -0,0 +1,26 @@
+/** @file
+  This Protocol will be installed at the end of S3 resume phase in SMM 
environment. 
+  It allows for smm drivers to hook this point and do the requried tasks.
+
+  Copyright (c) 2017, 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. 
+
+**/
+
+#ifndef __SMM_END_OF_S3_RESUME_H__
+#define __SMM_END_OF_S3_RESUME_H__
+
+#define EDKII_SMM_END_OF_S3_RESUME_PROTOCOL_GUID \
+  { \
+0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 
0xb5 } \
+  }
+
+extern EFI_GUID gEdkiiSmmEndOfS3ResumeProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a3c0633..216e4f9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -556,6 +556,9 @@
   ## Include/Protocol/IoMmu.h
   gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 
0xe1, 0xce, 0x51, 0x7c, 0x1e } }
 
+  ## Include/Protocol/SmmEndofS3Resume.h
+  gEdkiiSmmEndOfS3ResumeProtocolGuid = { 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 
0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } }
+
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x8001 | Invalid value provided.
-- 
2.7.0.windows.1

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


[edk2] [Patch v3 0/3] Add SmmEndOfS3Resume event.

2017-10-11 Thread Eric Dong
This patch series add new SmmEndOfS3Resume event which required by some
SMM drivers. 
It will implmented by SmmCore to install the gEdkiiSmmEndOfS3ResumeProtocolGuid
Protocol. Smm drivers can install this protocol's notification functions to 
hoot this envet.
It will be trigged right after the EndOfPei event in S3 resume phase.

V2 Changes: 
  Only change patch 2/3
  1. Change structures name to avoid they start with EFI_.
  2. Base on DXE phase bits to provide communication buffer, current implement
 check both PEI and DXE phase.

V3 Changes:
  for 2/3 patch:UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to 
SmmCore.
1. Change structure name for better understanding.
2. Enhance communication buffer calculate logic to more accurate.

  for 3/3 patch: MdeModulePkg/PiSmmCore: Install Protocol when S3 resume 
finished.
1. Uninstall the protocol right after install it to avoid run out of memory.

Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 

Eric Dong (3):
  MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
  UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
  MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c| 55 --
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h| 24 ++
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |  1 +
 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h   | 31 
 MdeModulePkg/MdeModulePkg.dec  |  3 +
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 85 ++
 .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
 7 files changed, 196 insertions(+), 7 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h

-- 
2.7.0.windows.1

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


[edk2] [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.

2017-10-11 Thread Eric Dong
Driver will send S3 resume finished event to SmmCore through communicate
buffer after it signals EndOfPei event.

V2 Changes:
1. Change structures name to avoid they start with EFI_.
2. Base on DXE phase bits to provide communication buffer, current implement
check both PEI and DXE phase.

V3 Changes:
1. Change structure name for better understanding.
2. Enhance communication buffer calculate logic to more accurate.

Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 87 ++
 .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
 2 files changed, 91 insertions(+)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c 
b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index e53ed21..c2171cb 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -28,6 +28,9 @@
 #include 
 #include 
 #include 
+#include 
+
+#include 
 
 #include 
 #include 
@@ -151,6 +154,22 @@ typedef union {
   UINT64Uint64;
 } PAGE_TABLE_1G_ENTRY;
 
+//
+// Define two type of smm communicate headers.
+// One for 32 bits  PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits DXE 
case.
+//
+typedef struct {
+  EFI_GUID  HeaderGuid;
+  UINT32MessageLength;
+  UINT8 Data[1];
+} SMM_COMMUNICATE_HEADER_32;
+
+typedef struct {
+  EFI_GUID  HeaderGuid;
+  UINT64MessageLength;
+  UINT8 Data[1];
+} SMM_COMMUNICATE_HEADER_64;
+
 #pragma pack()
 
 //
@@ -430,6 +449,68 @@ IsLongModeWakingVector (
 }
 
 /**
+  Send EndOfS3Resume event to SmmCore through communication buffer way.
+
+  @retval  EFI_SUCCESS Return send the event success.
+**/
+EFI_STATUS
+SignalEndOfS3Resume (
+  VOID
+  )
+{
+  EFI_STATUS Status;
+  EFI_PEI_SMM_COMMUNICATION_PPI  *SmmCommunicationPpi;
+  UINTN  CommSize;
+  SMM_COMMUNICATE_HEADER_32  Header32;
+  SMM_COMMUNICATE_HEADER_64  Header64;
+  VOID   *CommBuffer;
+
+  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
+
+  //
+  // This buffer consumed in DXE phase, so base on DXE mode to prepare 
communicate buffer.
+  // Detect whether DXE is 64 bits mode.
+  // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE also 
64 bits.
+  // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64 
bits.
+  //
+  if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet 
(PcdDxeIplSwitchToLongMode))) {
+CommBuffer = 
+Header64.MessageLength = 0;
+CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_64, Data);
+  } else {
+CommBuffer = 
+Header32.MessageLength = 0;
+CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data);
+  }
+  CopyGuid (CommBuffer, );
+
+  //
+  // Get needed resource
+  //
+  Status = PeiServicesLocatePpi (
+ ,
+ 0,
+ NULL,
+ (VOID **)
+ );
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Send command
+  //
+  Status = SmmCommunicationPpi->Communicate (
+  SmmCommunicationPpi,
+  (VOID *)CommBuffer,
+  
+  );
+  ASSERT_EFI_ERROR (Status);
+
+  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
+
+  return Status;
+}
+
+/**
   Jump to OS waking vector.
   The function will install boot script done PPI, report S3 resume status 
code, and then jump to OS waking vector.
 
@@ -504,6 +585,12 @@ S3ResumeBootOs (
   ASSERT_EFI_ERROR (Status);
 
   //
+  // Signal EndOfS3Resume event.
+  //
+  Status = SignalEndOfS3Resume ();
+  ASSERT_EFI_ERROR (Status);
+
+  //
   // report status code on S3 resume
   //
   REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE | 
EFI_SW_PEI_PC_OS_WAKE);
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf 
b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
index d514523..943f114 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
@@ -85,6 +85,10 @@
   gPeiSmmAccessPpiGuid  ## SOMETIMES_CONSUMES
   gPeiPostScriptTablePpiGuid## SOMETIMES_PRODUCES
   gEfiEndOfPeiSignalPpiGuid ## SOMETIMES_PRODUCES
+  gEfiPeiSmmCommunicationPpiGuid## SOMETIMES_CONSUMES
+
+[Protocols]
+  gEdkiiSmmEndOfS3ResumeProtocolGuid## SOMETIMES_CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ## CONSUMES
-- 
2.7.0.windows.1

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


Re: [edk2] [Patch v2 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.

2017-10-11 Thread Yao, Jiewen
I have a question for below:

> +  SmmHandle = NULL;
> +  Status = SmmInstallProtocolInterface (
> + ,
> + ,
> + EFI_NATIVE_INTERFACE,
> + NULL
> + );

A platform may do S3 multiple times. What happen if the 
SmmInstallProtocolInterface() called twice?
Will the system has 2 handle and 2 protocol?

Thank you
Yao Jiewen

> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 1:32 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yao, Jiewen 
> Subject: [Patch v2 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3
> resume finished.
> 
> Install EdkiiSmmEndOfS3ResumeProtocol when S3 resume finished.
> S3ResumePei will send S3 resume finished event to SmmCore through
> communication buffer.
> 
> Cc: Ruiyu Ni 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c   | 55
> +++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h   | 24 ++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf |  1 +
>  3 files changed, 73 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> index 9e4390e..aa44933 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> @@ -75,13 +75,14 @@ BOOLEAN  mInLegacyBoot = FALSE;
>  // Table of SMI Handlers that are registered by the SMM Core when it is
> initialized
>  //
>  SMM_CORE_SMI_HANDLERS  mSmmCoreSmiHandlers[] = {
> -  { SmmDriverDispatchHandler,   ,
> NULL, TRUE  },
> -  { SmmReadyToLockHandler,  ,
> NULL, TRUE },
> -  { SmmLegacyBootHandler,   ,
> NULL, FALSE },
> -  { SmmExitBootServicesHandler, , NULL,
> FALSE },
> -  { SmmReadyToBootHandler,  ,
> NULL, FALSE },
> -  { SmmEndOfDxeHandler, ,
> NULL, TRUE },
> -  { NULL,   NULL,
> NULL, FALSE }
> +  { SmmDriverDispatchHandler,   ,
> NULL, TRUE  },
> +  { SmmReadyToLockHandler,  ,
> NULL, TRUE },
> +  { SmmLegacyBootHandler,   ,
> NULL, FALSE },
> +  { SmmExitBootServicesHandler, ,  NULL,
> FALSE },
> +  { SmmReadyToBootHandler,  ,
> NULL, FALSE },
> +  { SmmEndOfDxeHandler, ,
> NULL, TRUE },
> +  { SmmEndOfS3ResumeHandler,
> , NULL, FALSE },
> +  { NULL,   NULL,
> NULL, FALSE }
>  };
> 
>  UINTN   mFullSmramRangeCount;
> @@ -383,6 +384,46 @@ SmmEndOfDxeHandler (
>  }
> 
>  /**
> +  Software SMI handler that is called when the EndOfS3Resume event is
> trigged.
> +  This function installs the SMM EndOfS3Resume Protocol so SMM Drivers are
> informed that
> +  S3 resume has finished.
> +
> +  @param  DispatchHandle  The unique handle assigned to this handler by
> SmiHandlerRegister().
> +  @param  Context Points to an optional handler context which was
> specified when the handler was registered.
> +  @param  CommBuffer  A pointer to a collection of data in memory
> that will
> +  be conveyed from a non-SMM environment into
> an SMM environment.
> +  @param  CommBufferSize  The size of the CommBuffer.
> +
> +  @return Status Code
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmEndOfS3ResumeHandler (
> +  IN EFI_HANDLE  DispatchHandle,
> +  IN CONST VOID  *Context,OPTIONAL
> +  IN OUT VOID*CommBuffer, OPTIONAL
> +  IN OUT UINTN   *CommBufferSize  OPTIONAL
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_HANDLE  SmmHandle;
> +
> +  DEBUG ((EFI_D_INFO, "SmmEndOfS3ResumeHandler\n"));
> +  //
> +  // Install SMM EndOfDxe protocol
> +  //
> +  SmmHandle = NULL;
> +  Status = SmmInstallProtocolInterface (
> + ,
> + ,
> + EFI_NATIVE_INTERFACE,
> + NULL
> + );
> +  return Status;
> +}
> +
> +/**
>Determine if two buffers overlap in memory.
> 
>@param[in] Buff1  Pointer to first buffer
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> index b6f815c..6cc824b 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
>  #include 
> @@ -802,6 +803,29 @@ SmmReadyToBootHandler (
>);
> 
>  /**
> +  Software SMI handler that is called when the EndOfS3Resume event is
> trigged.
> +  This function installs the SMM EndOfS3Resume Protocol so SMM Drivers are
> informed that
> +  S3 resume has finished.
> +
> +  @param  DispatchHandle  The unique handle assigned to this handler by
> SmiHandlerRegister().
> +  @param  Context Points to an optional handler context which was
> specified when the handler was registered.
> +  @param  CommBuffer  A pointer to a collection of data in memory
> 

Re: [edk2] [Patch v2 0/3] Add SmmEndOfS3Resume event.

2017-10-11 Thread Ni, Ruiyu
Please don't forget to change the structure name as proposed by Jiewen.
Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 1:32 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yao, Jiewen 
> Subject: [Patch v2 0/3] Add SmmEndOfS3Resume event.
> 
> This patch series add new SmmEndOfS3Resume event which required by
> some SMM drivers.
> It will implmented by SmmCore to install the
> gEdkiiSmmEndOfS3ResumeProtocolGuid
> Protocol. Smm drivers can install this protocol's notification functions to 
> hoot
> this envet.
> It will be trigged right after the EndOfPei event in S3 resume phase.
> 
> V2 Changes: Only change patch 2/3
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current
> implement check both PEI and DXE phase.
> 
> Cc: Ruiyu Ni 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> 
> Eric Dong (3):
>   MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
>   UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
>   MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.
> 
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c| 55 --
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h| 24 ++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |  1 +
>  MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h   | 31 
>  MdeModulePkg/MdeModulePkg.dec  |  3 +
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 85
> ++
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
>  7 files changed, 196 insertions(+), 7 deletions(-)  create mode 100644
> MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
> 
> --
> 2.7.0.windows.1

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


Re: [edk2] [Patch v2 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.

2017-10-11 Thread Dong, Eric
Jiewen,

Got it. If just change structure name, I prefer to do it when I push the code.

Thanks,
Eric

-Original Message-
From: Yao, Jiewen 
Sent: Wednesday, October 11, 2017 2:25 PM
To: Dong, Eric ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu 
Subject: RE: [Patch v2 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished 
event to SmmCore.

Hi Eric
I do not think IA64 is a good term. Traditionally, IA64 means the IPF platform, 
which is deprecated.
We use X64 to indicate it is Intel 64bit platform.

If it is just a general 64bit indicator, I suggest we use 
SMM_COMMUNICATE_HEADER_64 and SMM_COMMUNICATE_HEADER_32.

Thank you
Yao Jiewen


> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 1:32 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yao, Jiewen 
> Subject: [Patch v2 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume 
> finished event to SmmCore.
> 
> Driver will send S3 resume finished event to SmmCore through 
> communicate buffer after it signals EndOfPei event.
> 
> V2 Changes:
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current 
> implement check both PEI and DXE phase.
> 
> Cc: Ruiyu Ni 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 87
> ++
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
>  2 files changed, 91 insertions(+)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index e53ed21..56f7b37 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -28,6 +28,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +#include 
> 
>  #include 
>  #include 
> @@ -151,6 +154,22 @@ typedef union {
>UINT64Uint64;
>  } PAGE_TABLE_1G_ENTRY;
> 
> +//
> +// Define two type of smm communicate headers.
> +// One for 32 bits  PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits 
> DXE case.
> +//
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT32MessageLength;
> +  UINT8 Data[1];
> +} IA32_EFI_SMM_COMMUNICATE_HEADER;
> +
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT64MessageLength;
> +  UINT8 Data[1];
> +} IA64_EFI_SMM_COMMUNICATE_HEADER;
> +
>  #pragma pack()
> 
>  //
> @@ -430,6 +449,68 @@ IsLongModeWakingVector (  }
> 
>  /**
> +  Send EndOfS3Resume event to SmmCore through communication buffer
> way.
> +
> +  @retval  EFI_SUCCESS Return send the event success.
> +**/
> +EFI_STATUS
> +SignalEndOfS3Resume (
> +  VOID
> +  )
> +{
> +  EFI_STATUS Status;
> +  EFI_PEI_SMM_COMMUNICATION_PPI  *SmmCommunicationPpi;
> +  UINTN  CommSize;
> +  IA32_EFI_SMM_COMMUNICATE_HEADERHeader32;
> +  IA64_EFI_SMM_COMMUNICATE_HEADERHeader64;
> +  VOID   *CommBuffer;
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
> +
> +  //
> +  // This buffer consumed in DXE phase, so base on DXE mode to 
> + prepare
> communicate buffer.
> +  // Detect whether DXE is 64 bits mode.
> +  // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume 
> + DXE also 64
> bits.
> +  // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch 
> + to 64
> bits.
> +  //
> +  if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet
> (PcdDxeIplSwitchToLongMode))) {
> +CommBuffer = 
> +Header64.MessageLength = 0;
> +CommSize = sizeof (IA64_EFI_SMM_COMMUNICATE_HEADER);
> +  } else {
> +CommBuffer = 
> +Header32.MessageLength = 0;
> +CommSize = sizeof (IA32_EFI_SMM_COMMUNICATE_HEADER);
> +  }
> +  CopyGuid (CommBuffer, );
> +
> +  //
> +  // Get needed resource
> +  //
> +  Status = PeiServicesLocatePpi (
> + ,
> + 0,
> + NULL,
> + (VOID **)
> + );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Send command
> +  //
> +  Status = SmmCommunicationPpi->Communicate (
> +  SmmCommunicationPpi,
> +  (VOID *)CommBuffer,
> +  
> +  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> +
> +  return Status;
> +}
> +
> +/**
>Jump to OS waking vector.
>The function will install boot script done PPI, report S3 resume 
> status code, and then jump to OS waking vector.
> 
> @@ -504,6 +585,12 @@ S3ResumeBootOs (
>ASSERT_EFI_ERROR (Status);
> 
>//
> +  // Signal EndOfS3Resume event.
> +  //
> +  Status = SignalEndOfS3Resume ();
> +  ASSERT_EFI_ERROR 

Re: [edk2] [Patch v2 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.

2017-10-11 Thread Yao, Jiewen
Hi Eric
I do not think IA64 is a good term. Traditionally, IA64 means the IPF platform, 
which is deprecated.
We use X64 to indicate it is Intel 64bit platform.

If it is just a general 64bit indicator, I suggest we use 
SMM_COMMUNICATE_HEADER_64 and SMM_COMMUNICATE_HEADER_32.

Thank you
Yao Jiewen


> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 1:32 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yao, Jiewen 
> Subject: [Patch v2 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished
> event to SmmCore.
> 
> Driver will send S3 resume finished event to SmmCore through communicate
> buffer after it signals EndOfPei event.
> 
> V2 Changes:
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current implement
> check both PEI and DXE phase.
> 
> Cc: Ruiyu Ni 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 87
> ++
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
>  2 files changed, 91 insertions(+)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index e53ed21..56f7b37 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -28,6 +28,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +#include 
> 
>  #include 
>  #include 
> @@ -151,6 +154,22 @@ typedef union {
>UINT64Uint64;
>  } PAGE_TABLE_1G_ENTRY;
> 
> +//
> +// Define two type of smm communicate headers.
> +// One for 32 bits  PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits 
> DXE case.
> +//
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT32MessageLength;
> +  UINT8 Data[1];
> +} IA32_EFI_SMM_COMMUNICATE_HEADER;
> +
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT64MessageLength;
> +  UINT8 Data[1];
> +} IA64_EFI_SMM_COMMUNICATE_HEADER;
> +
>  #pragma pack()
> 
>  //
> @@ -430,6 +449,68 @@ IsLongModeWakingVector (
>  }
> 
>  /**
> +  Send EndOfS3Resume event to SmmCore through communication buffer
> way.
> +
> +  @retval  EFI_SUCCESS Return send the event success.
> +**/
> +EFI_STATUS
> +SignalEndOfS3Resume (
> +  VOID
> +  )
> +{
> +  EFI_STATUS Status;
> +  EFI_PEI_SMM_COMMUNICATION_PPI  *SmmCommunicationPpi;
> +  UINTN  CommSize;
> +  IA32_EFI_SMM_COMMUNICATE_HEADERHeader32;
> +  IA64_EFI_SMM_COMMUNICATE_HEADERHeader64;
> +  VOID   *CommBuffer;
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
> +
> +  //
> +  // This buffer consumed in DXE phase, so base on DXE mode to prepare
> communicate buffer.
> +  // Detect whether DXE is 64 bits mode.
> +  // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE 
> also 64
> bits.
> +  // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64
> bits.
> +  //
> +  if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet
> (PcdDxeIplSwitchToLongMode))) {
> +CommBuffer = 
> +Header64.MessageLength = 0;
> +CommSize = sizeof (IA64_EFI_SMM_COMMUNICATE_HEADER);
> +  } else {
> +CommBuffer = 
> +Header32.MessageLength = 0;
> +CommSize = sizeof (IA32_EFI_SMM_COMMUNICATE_HEADER);
> +  }
> +  CopyGuid (CommBuffer, );
> +
> +  //
> +  // Get needed resource
> +  //
> +  Status = PeiServicesLocatePpi (
> + ,
> + 0,
> + NULL,
> + (VOID **)
> + );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Send command
> +  //
> +  Status = SmmCommunicationPpi->Communicate (
> +  SmmCommunicationPpi,
> +  (VOID *)CommBuffer,
> +  
> +  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> +
> +  return Status;
> +}
> +
> +/**
>Jump to OS waking vector.
>The function will install boot script done PPI, report S3 resume status 
> code,
> and then jump to OS waking vector.
> 
> @@ -504,6 +585,12 @@ S3ResumeBootOs (
>ASSERT_EFI_ERROR (Status);
> 
>//
> +  // Signal EndOfS3Resume event.
> +  //
> +  Status = SignalEndOfS3Resume ();
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
>// report status code on S3 resume
>//
>REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE
> | EFI_SW_PEI_PC_OS_WAKE);
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> index d514523..943f114 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> +++