[edk2] [PATCH] ArmPkg/CompilerIntrinsicsLib: fix GCC8 warning for __aeabi_memcpy aliases

[Michael Zimmermann]

This was the warning(shown for __aeabi_memcpy, __aeabi_memcpy4 and 
__aeabi_memcpy8):
ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c:42:6: error: '__aeabi_memcpy8' 
alias between functions of incompatible types 'void(void
*, const void *, size_t)' {aka 'void(void *, const void *, unsigned int)'} and 
'void *(void *, const void *, size_t)' {aka 'void
*(void *, const void *, unsigned int)'} [-Werror=attribute-alias]
void __aeabi_memcpy8(void *dest, const void *src, size_t n);
^~~
ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c:19:7: note: aliased declaration 
here
void *__memcpy(void *dest, const void *src, size_t n)

The problem is the different return type(void vs void*).
This commit adds a wrapper '__aeabi___memcpy' with a void return value.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann 
---
 ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c 
b/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
index a944e00b89e1..507234186fa9 100644
--- a/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
+++ b/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
@@ -31,14 +31,19 @@ __attribute__((__alias__("__memcpy")))
 void *memcpy(void *dest, const void *src, size_t n);
 
 #ifdef __arm__
+static __attribute__((__used__))
+void __aeabi___memcpy(void *dest, const void *src, size_t n)
+{
+  __memcpy(dest, src, n);
+}
 
-__attribute__((__alias__("__memcpy")))
+__attribute__((__alias__("__aeabi___memcpy")))
 void __aeabi_memcpy(void *dest, const void *src, size_t n);
 
-__attribute__((__alias__("__memcpy")))
+__attribute__((__alias__("__aeabi___memcpy")))
 void __aeabi_memcpy4(void *dest, const void *src, size_t n);
 
-__attribute__((__alias__("__memcpy")))
+__attribute__((__alias__("__aeabi___memcpy")))
 void __aeabi_memcpy8(void *dest, const void *src, size_t n);
 
 #endif
-- 
2.17.1

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

Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Zeng, Star]

Thanks, got the point.

It seems vague that who to ensure the cache coherency. MMU? Caller of 
UpdateCapsule? UpdateCapsule? Consumer of capsule data?


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Thursday, June 7, 2018 12:50 PM
To: Zeng, Star 
Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Leif Lindholm 
; Yao, Jiewen ; Gao, Liming 
; Kinney, Michael D 
Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before 
consuming capsule data

On 7 June 2018 at 03:37, Zeng, Star  wrote:
> Hi Ard,
>
> The input parameter CapsuleHeaderArray of UpdateCapsule has the virtual 
> address.
>

It has the virtual address of the capsules yes. But how about the data 
structures passed as the ScatterGatherList?


> CapsuleHeaderArray
> Virtual pointer to an array of virtual pointers to the capsules being 
> passed into update capsule. Each capsules is assumed to stored in 
> contiguous virtual memory. The capsules in the CapsuleHeaderArray must 
> be the same capsules as the ScatterGatherList. The CapsuleHeaderArray 
> must have the
>
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ard Biesheuvel
> Sent: Wednesday, June 6, 2018 8:10 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Ard Biesheuvel 
> ; Gao, Liming ; Yao, 
> Jiewen ; Leif Lindholm 
> ; Kinney, Michael D 
> ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache 
> before consuming capsule data
>
> On 6 June 2018 at 11:52, Ard Biesheuvel  wrote:
>> When capsule updates are staged for processing after a warm reboot, 
>> they are copied into memory with the MMU and caches enabled. When the 
>> capsule PEI gets around to coalescing the capsule, the MMU and caches 
>> may still be disabled, and so on architectures where uncached 
>> accesses are incoherent with the caches (such as ARM and AARCH64), we 
>> may read stale data if we don't clean the caches to memory first.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>
> Leif asked me to include a note why this cannot be done when
> UpdateCapsule() is called.
>
>
> """
> Note that this cache maintenance cannot be done during the invocation of 
> UpdateCapsule(), since the ScatterGatherList structures are only identified 
> by physical address, and at runtime, the firmware doesn't know whether and 
> where this memory is mapped, and cache maintenance requires a virtual address.
> """
>
>> ---
>>  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf   | 1 +
>>  MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 4 
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> index c54bc21a95a8..594e110d1f8a 100644
>> --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> @@ -48,6 +48,7 @@ [Packages]
>>
>>  [LibraryClasses]
>>BaseLib
>> +  CacheMaintenanceLib
>>HobLib
>>BaseMemoryLib
>>PeiServicesLib
>> diff --git
>> a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> index 3e7054cd38a9..1730f925adc5 100644
>> --- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> +++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>> EXPRESS OR IMPLIED.
>>  #include 
>>
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource (
>>DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in 
>> MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
>>Address, Size,
>>Index, 
>> MemoryResource[Index].PhysicalStart,
>> MemoryResource[Index].ResourceLength));
>> +
>> +  WriteBackDataCacheRange ((VOID *)(UINTN)Address, Size);
>> +
>>return TRUE;
>>  }
>>}
>> --
>> 2.17.0
>>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg/ArmDisassemblerLib: fix check for MSR instruction

[Michael Zimmermann]

CC the arm maintainers
On Thu, Jun 7, 2018 at 7:07 AM Michael Zimmermann
 wrote:
>
> From: M1cha 
>
> GCC8 reported it with the following warning:
> ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c: In function 
> 'DisassembleArmInstruction':
> ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c:397:30: error: bitwise
> comparison always evaluates to false [-Werror=tautological-compare]
> if ((OpCode  & 0x0db0) == 0x0320) {
>
> This condition trys to be true for both the immediate and the register
> version of the MSR instruction. They get identified inside the if-block
> using the variable I, which contains the value of bit 25.
>
> The problem with the comparison reported by GCC is that the
> bitmask excludes bit 25, while the value requires it to be set to one:
> 0x0db0:  11011 0 11 00 00  
> 0x0320:  00110 0 10 00 00  
>^
> So the solution is to just don't require that bit to be set, because
> it gets checked later using 'I', which results in the following value:
> 0x0120:  00010 0 10 00 00  
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael Zimmermann 
> ---
>  ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c 
> b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
> index 29d9414a78b3..b449a5d3cd83 100644
> --- a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
> +++ b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
> @@ -394,7 +394,7 @@ DisassembleArmInstruction (
>}
>
>
> -  if ((OpCode  & 0x0db0) == 0x0320) {
> +  if ((OpCode  & 0x0db0) == 0x0120) {
>  // A4.1.38 MSR{} CPSR_, # MSR{} 
> CPSR_, 
>  if (I) {
>// MSR{} CPSR_, #
> --
> 2.17.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmPkg/ArmDisassemblerLib: fix check for MSR instruction

[Michael Zimmermann]

From: M1cha 

GCC8 reported it with the following warning:
ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c: In function 
'DisassembleArmInstruction':
ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c:397:30: error: bitwise
comparison always evaluates to false [-Werror=tautological-compare]
if ((OpCode  & 0x0db0) == 0x0320) {

This condition trys to be true for both the immediate and the register
version of the MSR instruction. They get identified inside the if-block
using the variable I, which contains the value of bit 25.

The problem with the comparison reported by GCC is that the
bitmask excludes bit 25, while the value requires it to be set to one:
0x0db0:  11011 0 11 00 00  
0x0320:  00110 0 10 00 00  
   ^
So the solution is to just don't require that bit to be set, because
it gets checked later using 'I', which results in the following value:
0x0120:  00010 0 10 00 00  

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann 
---
 ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c 
b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
index 29d9414a78b3..b449a5d3cd83 100644
--- a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
+++ b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
@@ -394,7 +394,7 @@ DisassembleArmInstruction (
   }
 
 
-  if ((OpCode  & 0x0db0) == 0x0320) {
+  if ((OpCode  & 0x0db0) == 0x0120) {
 // A4.1.38 MSR{} CPSR_, # MSR{} 
CPSR_, 
 if (I) {
   // MSR{} CPSR_, #
-- 
2.17.1

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

Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Ard Biesheuvel]

On 7 June 2018 at 03:37, Zeng, Star  wrote:
> Hi Ard,
>
> The input parameter CapsuleHeaderArray of UpdateCapsule has the virtual 
> address.
>

It has the virtual address of the capsules yes. But how about the data
structures passed as the ScatterGatherList?


> CapsuleHeaderArray
> Virtual pointer to an array of virtual pointers to the capsules being passed 
> into update capsule. Each capsules is assumed to stored in contiguous virtual 
> memory. The capsules in the CapsuleHeaderArray must be the same capsules as 
> the ScatterGatherList. The CapsuleHeaderArray must have the
>
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
> Biesheuvel
> Sent: Wednesday, June 6, 2018 8:10 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Ard Biesheuvel 
> ; Gao, Liming ; Yao, Jiewen 
> ; Leif Lindholm ; Kinney, 
> Michael D ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before 
> consuming capsule data
>
> On 6 June 2018 at 11:52, Ard Biesheuvel  wrote:
>> When capsule updates are staged for processing after a warm reboot,
>> they are copied into memory with the MMU and caches enabled. When the
>> capsule PEI gets around to coalescing the capsule, the MMU and caches
>> may still be disabled, and so on architectures where uncached accesses
>> are incoherent with the caches (such as ARM and AARCH64), we may read
>> stale data if we don't clean the caches to memory first.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>
> Leif asked me to include a note why this cannot be done when
> UpdateCapsule() is called.
>
>
> """
> Note that this cache maintenance cannot be done during the invocation of 
> UpdateCapsule(), since the ScatterGatherList structures are only identified 
> by physical address, and at runtime, the firmware doesn't know whether and 
> where this memory is mapped, and cache maintenance requires a virtual address.
> """
>
>> ---
>>  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf   | 1 +
>>  MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 4 
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> index c54bc21a95a8..594e110d1f8a 100644
>> --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
>> @@ -48,6 +48,7 @@ [Packages]
>>
>>  [LibraryClasses]
>>BaseLib
>> +  CacheMaintenanceLib
>>HobLib
>>BaseMemoryLib
>>PeiServicesLib
>> diff --git
>> a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> index 3e7054cd38a9..1730f925adc5 100644
>> --- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> +++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
>> @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>> EXPRESS OR IMPLIED.
>>  #include 
>>
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource (
>>DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in 
>> MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
>>Address, Size,
>>Index, MemoryResource[Index].PhysicalStart,
>> MemoryResource[Index].ResourceLength));
>> +
>> +  WriteBackDataCacheRange ((VOID *)(UINTN)Address, Size);
>> +
>>return TRUE;
>>  }
>>}
>> --
>> 2.17.0
>>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

[Shi, Steven]

Hi Zenith,
BTW, besides the build pass, did you try to run a Uefi binary, e.g. a simple 
shell application, which contain the GOTPCREL relocations? If yes. Please send 
out your test case code as well. I appreciate if you could contribute your test 
cases as the patch together. 


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522


> -Original Message-
> From: Shi, Steven
> Sent: Thursday, June 7, 2018 10:18 AM
> To: Shi, Steven ; Gao, Liming
> ; Zenith432 ;
> edk2-devel@lists.01.org
> Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> GenFw
> 
> Please see more details in
> https://bugzilla.tianocore.org/show_bug.cgi?id=970
> 
> 
> Steven Shi
> Intel\SSG\STO\UEFI Firmware
> 
> Tel: +86 021-61166522
> iNet: 821-6522
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Shi,
> > Steven
> > Sent: Thursday, June 7, 2018 10:16 AM
> > To: Gao, Liming ; Zenith432
> > ; edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support
> > to GenFw
> >
> > Yes. If we disable the '#pragma GCC visibility push (hidden)' in
> > ProcessorBind.h, the GOTPCREL support is mandatory. 3rd party module
> > build might need it. It's more complete to add GOTPCREL support.
> >
> > The hidden #pragma can hide all ELF symbols' visibility, including 'extern'
> > functions, which is to let Linux compilers aware that we don’t need loading
> > dynamic link library in Uefi runing, please not emit the GOTPCREL based
> > relocations in the obj file, and we will ensure the linker can get all 
> > extern
> > functions statically solved. The reason of not emitting GOTPCREL based
> > relocations is because it is hard to convert the GOTPCREL based relocations
> > in ELF binary to PE/COFF related relocations, and our current GenFw has
> not
> > supported them. I'm glade if we can add the GOTPCREL support in GenFW
> to
> > totally solve this problem.
> >
> > Steven Shi
> > Intel\SSG\STO\UEFI Firmware
> >
> > Tel: +86 021-61166522
> > iNet: 821-6522
> >
> > > -Original Message-
> > > From: Gao, Liming
> > > Sent: Thursday, June 7, 2018 9:32 AM
> > > To: Zenith432 ; edk2-
> > de...@lists.01.org
> > > Cc: Shi, Steven ; Zhu, Yonghong
> > > 
> > > Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> > > GenFw
> > >
> > > What's purpose to support GOTPCREL in GenFw? Could you introduce
> your
> > > usage model?
> > >
> > > > -Original Message-
> > > > From: Zenith432 [mailto:zenith...@users.sourceforge.net]
> > > > Sent: Thursday, June 7, 2018 2:01 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Shi, Steven ; Zhu, Yonghong
> > > ; Gao, Liming 
> > > > Subject: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> > GenFw
> > > >
> > > > Adds support for the following X64 ELF relocations to GenFw
> > > >   R_X86_64_GOTPCREL
> > > >   R_X86_64_GOTPCRELX
> > > >   R_X86_64_REX_GOTPCRELX
> > > >
> > > > CC: Shi Steven 
> > > > CC: Yonghong Zhu 
> > > > CC: Liming Gao 
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Zenith432 
> > > > ---
> > > >  BaseTools/Source/C/GenFw/Elf64Convert.c | 166
> > > +++-
> > > >  BaseTools/Source/C/GenFw/elf_common.h   |  23 
> > > >  2 files changed, 188 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > > index c39bdff0..d2f9bb46 100644
> > > > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > > @@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
> > > >  STATIC Elf_Shdr *mShdrBase;
> > > >  STATIC Elf_Phdr *mPhdrBase;
> > > >
> > > > +//
> > > > +// GOT information
> > > > +//
> > > > +STATIC Elf_Shdr *mGOTShdr = NULL;
> > > > +STATIC UINT32   mGOTShindex = 0;
> > > > +STATIC UINT32   *mGOTCoffEntries = NULL;
> > > > +STATIC UINT32   mGOTMaxCoffEntries = 0;
> > > > +STATIC UINT32   mGOTNumCoffEntries = 0;
> > > > +
> > > >  //
> > > >  // Coff information
> > > >  //
> > > > @@ -322,6 +331,117 @@ GetSymName (
> > > >return StrtabContents + Sym->st_name;
> > > >  }
> > > >
> > > > +//
> > > > +// Find the ELF section hosting the GOT from an ELF Rva
> > > > +//   of a single GOT entry.  Normally, GOT is placed in
> > > > +//   ELF .text section, so assume once we find in which
> > > > +//   section the GOT is, all GOT entries are there, and
> > > > +//   just verify this.
> > > > +//
> > > > +STATIC
> > > > +VOID
> > > > +FindElfGOTSectionFromGOTEntryElfRva (
> > > > +  Elf64_Addr GOTEntryElfRva
> > > > +  )
> > > > +{
> > > > +  UINT32 i;
> > > > +  if (mGOTShdr != NULL) {
> > > > +if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
> > > > +GOTEntryElfRva <  mGOTShdr->sh_addr + mGOTShdr->sh_size)
> > > > +  return;
> > > > +Error (NULL, 0, 3000, "Unsupported",
> > > "FindElfGOTSectionFromGOTEntryElfRva: GOT entries found 

Re: [edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

[Shi, Steven]

Please see more details in https://bugzilla.tianocore.org/show_bug.cgi?id=970 


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Shi,
> Steven
> Sent: Thursday, June 7, 2018 10:16 AM
> To: Gao, Liming ; Zenith432
> ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support
> to GenFw
> 
> Yes. If we disable the '#pragma GCC visibility push (hidden)' in
> ProcessorBind.h, the GOTPCREL support is mandatory. 3rd party module
> build might need it. It's more complete to add GOTPCREL support.
> 
> The hidden #pragma can hide all ELF symbols' visibility, including 'extern'
> functions, which is to let Linux compilers aware that we don’t need loading
> dynamic link library in Uefi runing, please not emit the GOTPCREL based
> relocations in the obj file, and we will ensure the linker can get all extern
> functions statically solved. The reason of not emitting GOTPCREL based
> relocations is because it is hard to convert the GOTPCREL based relocations
> in ELF binary to PE/COFF related relocations, and our current GenFw has not
> supported them. I'm glade if we can add the GOTPCREL support in GenFW to
> totally solve this problem.
> 
> Steven Shi
> Intel\SSG\STO\UEFI Firmware
> 
> Tel: +86 021-61166522
> iNet: 821-6522
> 
> > -Original Message-
> > From: Gao, Liming
> > Sent: Thursday, June 7, 2018 9:32 AM
> > To: Zenith432 ; edk2-
> de...@lists.01.org
> > Cc: Shi, Steven ; Zhu, Yonghong
> > 
> > Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> > GenFw
> >
> > What's purpose to support GOTPCREL in GenFw? Could you introduce your
> > usage model?
> >
> > > -Original Message-
> > > From: Zenith432 [mailto:zenith...@users.sourceforge.net]
> > > Sent: Thursday, June 7, 2018 2:01 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Shi, Steven ; Zhu, Yonghong
> > ; Gao, Liming 
> > > Subject: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> GenFw
> > >
> > > Adds support for the following X64 ELF relocations to GenFw
> > >   R_X86_64_GOTPCREL
> > >   R_X86_64_GOTPCRELX
> > >   R_X86_64_REX_GOTPCRELX
> > >
> > > CC: Shi Steven 
> > > CC: Yonghong Zhu 
> > > CC: Liming Gao 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Zenith432 
> > > ---
> > >  BaseTools/Source/C/GenFw/Elf64Convert.c | 166
> > +++-
> > >  BaseTools/Source/C/GenFw/elf_common.h   |  23 
> > >  2 files changed, 188 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > index c39bdff0..d2f9bb46 100644
> > > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > @@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
> > >  STATIC Elf_Shdr *mShdrBase;
> > >  STATIC Elf_Phdr *mPhdrBase;
> > >
> > > +//
> > > +// GOT information
> > > +//
> > > +STATIC Elf_Shdr *mGOTShdr = NULL;
> > > +STATIC UINT32   mGOTShindex = 0;
> > > +STATIC UINT32   *mGOTCoffEntries = NULL;
> > > +STATIC UINT32   mGOTMaxCoffEntries = 0;
> > > +STATIC UINT32   mGOTNumCoffEntries = 0;
> > > +
> > >  //
> > >  // Coff information
> > >  //
> > > @@ -322,6 +331,117 @@ GetSymName (
> > >return StrtabContents + Sym->st_name;
> > >  }
> > >
> > > +//
> > > +// Find the ELF section hosting the GOT from an ELF Rva
> > > +//   of a single GOT entry.  Normally, GOT is placed in
> > > +//   ELF .text section, so assume once we find in which
> > > +//   section the GOT is, all GOT entries are there, and
> > > +//   just verify this.
> > > +//
> > > +STATIC
> > > +VOID
> > > +FindElfGOTSectionFromGOTEntryElfRva (
> > > +  Elf64_Addr GOTEntryElfRva
> > > +  )
> > > +{
> > > +  UINT32 i;
> > > +  if (mGOTShdr != NULL) {
> > > +if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
> > > +GOTEntryElfRva <  mGOTShdr->sh_addr + mGOTShdr->sh_size)
> > > +  return;
> > > +Error (NULL, 0, 3000, "Unsupported",
> > "FindElfGOTSectionFromGOTEntryElfRva: GOT entries found in multiple
> > sections.");
> > > +exit(EXIT_FAILURE);
> > > +  }
> > > +  for (i = 0; i < mEhdr->e_shnum; i++) {
> > > +Elf_Shdr *shdr = GetShdrByIndex(i);
> > > +if (GOTEntryElfRva >= shdr->sh_addr &&
> > > +GOTEntryElfRva <  shdr->sh_addr + shdr->sh_size) {
> > > +  mGOTShdr = shdr;
> > > +  mGOTShindex = i;
> > > +  return;
> > > +}
> > > +  }
> > > +  Error (NULL, 0, 3000, "Invalid",
> "FindElfGOTSectionFromGOTEntryElfRva:
> > ElfRva 0x%016LX for GOT entry not found in any section.",
> > > GOTEntryElfRva);
> > > +  exit(EXIT_FAILURE);
> > > +}
> > > +
> > > +//
> > > +// Stores locations of GOT entries in COFF image.
> > > +//   Returns TRUE if GOT entry is new.
> > > +//   Simple implementation as number of GOT
> > > +//   entries is expected to be low.
> > > +//
> > > +
> > > +STATIC
> > > +BOOLEAN
> 

Re: [edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

[Shi, Steven]

Yes. If we disable the '#pragma GCC visibility push (hidden)' in 
ProcessorBind.h, the GOTPCREL support is mandatory. 3rd party module build 
might need it. It's more complete to add GOTPCREL support.

The hidden #pragma can hide all ELF symbols' visibility, including 'extern' 
functions, which is to let Linux compilers aware that we don’t need loading 
dynamic link library in Uefi runing, please not emit the GOTPCREL based 
relocations in the obj file, and we will ensure the linker can get all extern 
functions statically solved. The reason of not emitting GOTPCREL based 
relocations is because it is hard to convert the GOTPCREL based relocations in 
ELF binary to PE/COFF related relocations, and our current GenFw has not 
supported them. I'm glade if we can add the GOTPCREL support in GenFW to 
totally solve this problem.

Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

> -Original Message-
> From: Gao, Liming
> Sent: Thursday, June 7, 2018 9:32 AM
> To: Zenith432 ; edk2-devel@lists.01.org
> Cc: Shi, Steven ; Zhu, Yonghong
> 
> Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> GenFw
> 
> What's purpose to support GOTPCREL in GenFw? Could you introduce your
> usage model?
> 
> > -Original Message-
> > From: Zenith432 [mailto:zenith...@users.sourceforge.net]
> > Sent: Thursday, June 7, 2018 2:01 AM
> > To: edk2-devel@lists.01.org
> > Cc: Shi, Steven ; Zhu, Yonghong
> ; Gao, Liming 
> > Subject: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw
> >
> > Adds support for the following X64 ELF relocations to GenFw
> >   R_X86_64_GOTPCREL
> >   R_X86_64_GOTPCRELX
> >   R_X86_64_REX_GOTPCRELX
> >
> > CC: Shi Steven 
> > CC: Yonghong Zhu 
> > CC: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Zenith432 
> > ---
> >  BaseTools/Source/C/GenFw/Elf64Convert.c | 166
> +++-
> >  BaseTools/Source/C/GenFw/elf_common.h   |  23 
> >  2 files changed, 188 insertions(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > index c39bdff0..d2f9bb46 100644
> > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > @@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
> >  STATIC Elf_Shdr *mShdrBase;
> >  STATIC Elf_Phdr *mPhdrBase;
> >
> > +//
> > +// GOT information
> > +//
> > +STATIC Elf_Shdr *mGOTShdr = NULL;
> > +STATIC UINT32   mGOTShindex = 0;
> > +STATIC UINT32   *mGOTCoffEntries = NULL;
> > +STATIC UINT32   mGOTMaxCoffEntries = 0;
> > +STATIC UINT32   mGOTNumCoffEntries = 0;
> > +
> >  //
> >  // Coff information
> >  //
> > @@ -322,6 +331,117 @@ GetSymName (
> >return StrtabContents + Sym->st_name;
> >  }
> >
> > +//
> > +// Find the ELF section hosting the GOT from an ELF Rva
> > +//   of a single GOT entry.  Normally, GOT is placed in
> > +//   ELF .text section, so assume once we find in which
> > +//   section the GOT is, all GOT entries are there, and
> > +//   just verify this.
> > +//
> > +STATIC
> > +VOID
> > +FindElfGOTSectionFromGOTEntryElfRva (
> > +  Elf64_Addr GOTEntryElfRva
> > +  )
> > +{
> > +  UINT32 i;
> > +  if (mGOTShdr != NULL) {
> > +if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
> > +GOTEntryElfRva <  mGOTShdr->sh_addr + mGOTShdr->sh_size)
> > +  return;
> > +Error (NULL, 0, 3000, "Unsupported",
> "FindElfGOTSectionFromGOTEntryElfRva: GOT entries found in multiple
> sections.");
> > +exit(EXIT_FAILURE);
> > +  }
> > +  for (i = 0; i < mEhdr->e_shnum; i++) {
> > +Elf_Shdr *shdr = GetShdrByIndex(i);
> > +if (GOTEntryElfRva >= shdr->sh_addr &&
> > +GOTEntryElfRva <  shdr->sh_addr + shdr->sh_size) {
> > +  mGOTShdr = shdr;
> > +  mGOTShindex = i;
> > +  return;
> > +}
> > +  }
> > +  Error (NULL, 0, 3000, "Invalid", "FindElfGOTSectionFromGOTEntryElfRva:
> ElfRva 0x%016LX for GOT entry not found in any section.",
> > GOTEntryElfRva);
> > +  exit(EXIT_FAILURE);
> > +}
> > +
> > +//
> > +// Stores locations of GOT entries in COFF image.
> > +//   Returns TRUE if GOT entry is new.
> > +//   Simple implementation as number of GOT
> > +//   entries is expected to be low.
> > +//
> > +
> > +STATIC
> > +BOOLEAN
> > +AccumulateCoffGOTEntries (
> > +  UINT32 GOTCoffEntry
> > +  )
> > +{
> > +  UINT32 i;
> > +  if (mGOTCoffEntries != NULL) {
> > +for (i = 0; i < mGOTNumCoffEntries; i++)
> > +  if (mGOTCoffEntries[i] == GOTCoffEntry)
> > +return FALSE;
> > +  }
> > +  if (mGOTCoffEntries == NULL) {
> > +mGOTCoffEntries = (UINT32*)malloc(5 * sizeof *mGOTCoffEntries);
> > +if (mGOTCoffEntries == NULL) {
> > +  Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
> > +}
> > +assert (mGOTCoffEntries != NULL);
> > +mGOTMaxCoffEntries = 5;
> > +mGOTNumCoffEntries = 0;
> > +  } else if (mGOTNumCoffEntries == mGOTMaxCoffEntries) {
> > +mGOTCoffEntries = 

[edk2] [Patch] BaseTools: Fix Section header size larger than elf file size bug

[Yonghong Zhu]

From: Yunhua Feng 

Add the logic to handle the case that Section header size larger than
elf file size.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/C/GenFw/Elf32Convert.c |  3 +++
 BaseTools/Source/C/GenFw/Elf64Convert.c |  3 +++
 BaseTools/Source/C/GenFw/ElfConvert.c   | 20 
 BaseTools/Source/C/GenFw/ElfConvert.h   |  3 ++-
 4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
b/BaseTools/Source/C/GenFw/Elf32Convert.c
index e0f6491..e26b10b 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -672,10 +672,13 @@ WriteSections32 (
 Elf_Shdr *Shdr = GetShdrByIndex(Idx);
 if ((*Filter)(Shdr)) {
   switch (Shdr->sh_type) {
   case SHT_PROGBITS:
 /* Copy.  */
+if (Shdr->sh_offset + Shdr->sh_size > mFileBufferSize) {
+  return FALSE;
+}
 memcpy(mCoffFile + mCoffSectionsOffset[Idx],
   (UINT8*)mEhdr + Shdr->sh_offset,
   Shdr->sh_size);
 break;
 
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 9e68d22..cc0c2cf 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -668,10 +668,13 @@ WriteSections64 (
 Elf_Shdr *Shdr = GetShdrByIndex(Idx);
 if ((*Filter)(Shdr)) {
   switch (Shdr->sh_type) {
   case SHT_PROGBITS:
 /* Copy.  */
+if (Shdr->sh_offset + Shdr->sh_size > mFileBufferSize) {
+  return FALSE;
+}
 memcpy(mCoffFile + mCoffSectionsOffset[Idx],
   (UINT8*)mEhdr + Shdr->sh_offset,
   (size_t) Shdr->sh_size);
 break;
 
diff --git a/BaseTools/Source/C/GenFw/ElfConvert.c 
b/BaseTools/Source/C/GenFw/ElfConvert.c
index 17913ff..6844c69 100644
--- a/BaseTools/Source/C/GenFw/ElfConvert.c
+++ b/BaseTools/Source/C/GenFw/ElfConvert.c
@@ -1,9 +1,9 @@
 /** @file
 Elf convert solution
 
-Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials are licensed and made available 
 under the terms and conditions of the BSD License which accompanies this 
 distribution.  The full text of the license may be found at 
 http://opensource.org/licenses/bsd-license.php
@@ -56,10 +56,15 @@ UINT32 mCoffOffset;
 // Offset in Coff file of headers and sections.
 //
 UINT32 mTableOffset;
 
 //
+//mFileBufferSize
+//
+UINT32 mFileBufferSize;
+
+//
 //*
 // Common ELF Functions
 //*
 //
 
@@ -171,10 +176,11 @@ ConvertElf (
   )
 {
   ELF_FUNCTION_TABLE  ElfFunctions;
   UINT8   EiClass;
 
+  mFileBufferSize = *FileLength;
   //
   // Determine ELF type and set function table pointer correctly.
   //
   VerboseMsg ("Check Elf Image Header");
   EiClass = (*FileBuffer)[EI_CLASS];
@@ -199,13 +205,19 @@ ConvertElf (
 
   //
   // Write and relocate sections.
   //
   VerboseMsg ("Write and relocate sections.");
-  ElfFunctions.WriteSections (SECTION_TEXT);
-  ElfFunctions.WriteSections (SECTION_DATA);
-  ElfFunctions.WriteSections (SECTION_HII);
+  if (!ElfFunctions.WriteSections (SECTION_TEXT)) {
+return FALSE;
+  }
+  if (!ElfFunctions.WriteSections (SECTION_DATA)) {
+return FALSE;
+  }
+  if (!ElfFunctions.WriteSections (SECTION_HII)) {
+return FALSE;
+  }
 
   //
   // Translate and write relocations.
   //
   VerboseMsg ("Translate and write relocations.");
diff --git a/BaseTools/Source/C/GenFw/ElfConvert.h 
b/BaseTools/Source/C/GenFw/ElfConvert.h
index abf434d..fc8c63f 100644
--- a/BaseTools/Source/C/GenFw/ElfConvert.h
+++ b/BaseTools/Source/C/GenFw/ElfConvert.h
@@ -1,9 +1,9 @@
 /** @file
 Header file for Elf convert solution
 
-Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials are licensed and made available 
 under the terms and conditions of the BSD License which accompanies this 
 distribution.  The full text of the license may be found at 
 http://opensource.org/licenses/bsd-license.php
@@ -27,10 +27,11 @@ extern UINT32 mCoffOffset;
 extern CHAR8  *mInImageName;
 extern UINT32 mImageTimeStamp;
 extern UINT8  *mCoffFile;
 extern UINT32 mTableOffset;
 extern UINT32 mOutImageType;
+extern UINT32 mFileBufferSize;
 
 //
 // Common EFI specific data.
 //
 #define ELF_HII_SECTION_NAME ".hii"
-- 
2.6.1.windows.1

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

[edk2] [Patch] BaseTools: Check elf sections alignment with MAX_COFF_ALIGNMENT

[Yonghong Zhu]

From: Yunhua Feng 

Add the logic to check whether mCoffAlignment is larger than
MAX_COFF_ALIGNMENT, and report error for it.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/C/GenFw/Elf32Convert.c | 10 +-
 BaseTools/Source/C/GenFw/Elf64Convert.c | 11 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
b/BaseTools/Source/C/GenFw/Elf32Convert.c
index 14fe4a2..e0f6491 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -1,9 +1,9 @@
 /** @file
 Elf32 Convert solution
 
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 Portions copyright (c) 2013, ARM 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
@@ -382,10 +382,18 @@ ScanSections32 (
   mCoffAlignment = (UINT32)shdr->sh_addralign;
 }
   }
 
   //
+  // Check if mCoffAlignment larger than MAX_COFF_ALIGNMENT
+  //
+  if (mCoffAlignment > MAX_COFF_ALIGNMENT) {
+Error (NULL, 0, 3000, "Invalid", "Section alignment larger than 
MAX_COFF_ALIGNMENT.");
+assert (FALSE);
+  }
+
+  //
   // Move the PE/COFF header right before the first section. This will help us
   // save space when converting to TE.
   //
   if (mCoffAlignment > mCoffOffset) {
 mNtHdrOffset += mCoffAlignment - mCoffOffset;
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c39bdff..9e68d22 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1,9 +1,9 @@
 /** @file
 Elf64 convert solution
 
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 Portions copyright (c) 2013-2014, ARM 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
@@ -375,10 +375,19 @@ ScanSections64 (
   mCoffAlignment = (UINT32)shdr->sh_addralign;
 }
   }
 
   //
+  // Check if mCoffAlignment larger than MAX_COFF_ALIGNMENT
+  //
+  if (mCoffAlignment > MAX_COFF_ALIGNMENT) {
+Error (NULL, 0, 3000, "Invalid", "Section alignment larger than 
MAX_COFF_ALIGNMENT.");
+assert (FALSE);
+  }
+
+
+  //
   // Move the PE/COFF header right before the first section. This will help us
   // save space when converting to TE.
   //
   if (mCoffAlignment > mCoffOffset) {
 mNtHdrOffset += mCoffAlignment - mCoffOffset;
-- 
2.6.1.windows.1

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

[edk2] [RFC] MdePkg/PerformanceLib.h: Add new Perf macros

[Dandan Bi]

We plan to add a group of new Perf macros in performance library for
performance logging. Which will simplify the usage model of performance
measurement.

New Perf Macros:
Macros 1:
#define PERF_START_IMAGE_BEGIN(ModuleHandle)
#define PERF_START_IMAGE_END(ModuleHandle)

Macros 2:
#define PERF_LOAD_IMAGE_BEGIN(ModuleHandle)
#define PERF_LOAD_IMAGE_END(ModuleHandle)

Macros 3:
#define PERF_DRIVER_BINDING_SUPPORT_BEGIN(ModuleHandle, ControllerHandle)
#define PERF_DRIVER_BINDING_SUPPORT_END(ModuleHandle, ControllerHandle)

Macros 4:
#define PERF_DRIVER_BINDING_START_BEGIN(ModuleHandle, ControllerHandle)
#define PERF_DRIVER_BINDING_START_END(ModuleHandle, ControllerHandle)

Macros 5:
#define PERF_DRIVER_BINDING_STOP_BEGIN(ModuleHandle, ControllerHandle)
#define PERF_DRIVER_BINDING_STOP_END(ModuleHandle, ControllerHandle)

Macros 6:
#define PERF_EVENT(EventString)

Macros 7:
#define PERF_EVENT_SIGNAL_BEGIN(EventGuid)
#define PERF_EVENT_SIGNAL_END(EventGuid)

Macros 8:
#define PERF_CALLBACK_BEGIN(TriggerGuid)
#define PERF_CALLBACK_END(TriggerGuid)

Macros 9:
#define PERF_FUNCTION_BEGIN()
#define PERF_FUNCTION_END()

Macros 10:
#define PERF_INMODULE_BEGIN(MeasurementString)
#define PERF_INMODULE_END(MeasurementString)

Macros 11:
#define PERF_CROSSMODULE_BEGIN(MeasurementString)
#define PERF_CROSSMODULE_END(MeasurementString)

The detailed descriptions and changes are listed here.
1. Reasons of introducing these new Perf macros:
   a. Simplify the usage model of Perf macros.
  New Perf macros will have less input parameters.
  And different use cases can map to different Perf macros.

   b. Introduce new control functionality for performance logging.
  Such as there are lots of DriverBinding Support (Start/Stop) Perf
  entries in core codes. User can stop logging the performance of
  these entries by configure PCD.

   c. User can get more info through the new Perf macros.
  Like the ControllerHandle for DriverBinding Perf entries...

2. Macros 1-11, each macro will be identified by different perf identifiers.
   Macros 1-5 are for core use cases and used in the
   PeiCore, DxeCore and SmmCore.
   Macros 6-11 are for general use cases and can be used in any modules.
   Map each pair of core macros in Macros 1-5 to a core Perf type:
   PERF_CORE_START_IMAGE
   PERF_CORE_LOAD_IMAGE
   PERF_CORE_DB_SUPPORT
   PERF_CORE_DB_START
   PERF_CORE_DB_STOP
   And map all general Perf macros in Macros 6-11 to PERF_GENERAL_TYPE
   Then we can configure PCD to disable different Perf macros.

3. Update PcdPerformanceLibraryPropertyMask to control performance logging.
   Now BIT0 of PcdPerformanceLibraryPropertyMaskis is to enable the
   performance measurement. We will add BIT1-BIT6 to control different
   Perf types. Only when performance measurement is enabled, we can check
   BIT1-BIT6 to see whether a specific Perf type is disabled or not.
   This configuration are compatible with current performance enable
   behavior, set BIT0 can enable all the performance measurement.
   PcdPerformanceLibraryPropertyMask in MdePkg
   #  BIT0 - Enable Performance Measurement.
   #  BIT1 - Disable Start Image Logging.
   #  BIT2 - Disable Load Image logging.
   #  BIT3 - Disable Binding Support logging.
   #  BIT4 - Disable Binding Start logging.
   #  BIT5 - Disable Binding Stop logging.
   #  BIT6 - Disable all other general Perfs.
   #  BIT1-BIT6 are evaluated when BIT0 is set.

4. Introduce two new Library APIs for PerformanceLib library class.
   a. LogPerformanceMeasurementEnabled (Type);
   b. LogPerformanceMeasurement (CallerId, Guid, String, Address, PerfId);
   LogPerformanceMeasurementEnabled (Type):
   check whether a specified performance measurement can be logged.
   LogPerformanceMeasurement (CallerId, Guid, String, Address, PerfId):
   Log performance measurement info.

5. Code updates
   a. Performance library instances update:
  MdePkg:
  BasePerformanceLibNull
  MdeModulePkg:
  PeiPerformanceLib
  DxePerformanceLib
  DxeCorePerformanceLib
  SmmPerformanceLib
  SmmCorePerformanceLib

   b. Consumer code update:
  Update the old perf macros to use new perf macros for
  MdeModulePkg, UefiCpuPkg and SecurityPkg.
  For other packages, if old Perf macros are used, packages owners
  can decide whether using new Perf macros to replace old ones.

Can refer to the following patch for more detailed code changes and
description of new macros, APIs, definitions and PCD updating.

Cc: Liming Gao 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdePkg/Include/Library/PerformanceLib.h | 407 +++-
 MdePkg/MdePkg.dec   |   9 +-
 2 files changed, 414 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/PerformanceLib.h 
b/MdePkg/Include/Library/PerformanceLib.h
index 3ecd62bd201..dea79a9ff53 100644
--- a/MdePkg/Include/Library/PerformanceLib.h
+++ 

Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Zeng, Star]

Hi Ard,

The input parameter CapsuleHeaderArray of UpdateCapsule has the virtual address.

CapsuleHeaderArray
Virtual pointer to an array of virtual pointers to the capsules being passed 
into update capsule. Each capsules is assumed to stored in contiguous virtual 
memory. The capsules in the CapsuleHeaderArray must be the same capsules as the 
ScatterGatherList. The CapsuleHeaderArray must have the


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Wednesday, June 6, 2018 8:10 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Ard Biesheuvel ; 
Gao, Liming ; Yao, Jiewen ; Leif 
Lindholm ; Kinney, Michael D 
; Zeng, Star 
Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before 
consuming capsule data

On 6 June 2018 at 11:52, Ard Biesheuvel  wrote:
> When capsule updates are staged for processing after a warm reboot, 
> they are copied into memory with the MMU and caches enabled. When the 
> capsule PEI gets around to coalescing the capsule, the MMU and caches 
> may still be disabled, and so on architectures where uncached accesses 
> are incoherent with the caches (such as ARM and AARCH64), we may read 
> stale data if we don't clean the caches to memory first.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Leif asked me to include a note why this cannot be done when
UpdateCapsule() is called.


"""
Note that this cache maintenance cannot be done during the invocation of 
UpdateCapsule(), since the ScatterGatherList structures are only identified by 
physical address, and at runtime, the firmware doesn't know whether and where 
this memory is mapped, and cache maintenance requires a virtual address.
"""

> ---
>  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf   | 1 +
>  MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 4 
>  2 files changed, 5 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf 
> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> index c54bc21a95a8..594e110d1f8a 100644
> --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> @@ -48,6 +48,7 @@ [Packages]
>
>  [LibraryClasses]
>BaseLib
> +  CacheMaintenanceLib
>HobLib
>BaseMemoryLib
>PeiServicesLib
> diff --git 
> a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c 
> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> index 3e7054cd38a9..1730f925adc5 100644
> --- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> +++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource (
>DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in 
> MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
>Address, Size,
>Index, MemoryResource[Index].PhysicalStart, 
> MemoryResource[Index].ResourceLength));
> +
> +  WriteBackDataCacheRange ((VOID *)(UINTN)Address, Size);
> +
>return TRUE;
>  }
>}
> --
> 2.17.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

[Gao, Liming]

What's purpose to support GOTPCREL in GenFw? Could you introduce your usage 
model?

> -Original Message-
> From: Zenith432 [mailto:zenith...@users.sourceforge.net]
> Sent: Thursday, June 7, 2018 2:01 AM
> To: edk2-devel@lists.01.org
> Cc: Shi, Steven ; Zhu, Yonghong 
> ; Gao, Liming 
> Subject: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw
> 
> Adds support for the following X64 ELF relocations to GenFw
>   R_X86_64_GOTPCREL
>   R_X86_64_GOTPCRELX
>   R_X86_64_REX_GOTPCRELX
> 
> CC: Shi Steven 
> CC: Yonghong Zhu 
> CC: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zenith432 
> ---
>  BaseTools/Source/C/GenFw/Elf64Convert.c | 166 +++-
>  BaseTools/Source/C/GenFw/elf_common.h   |  23 
>  2 files changed, 188 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index c39bdff0..d2f9bb46 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
>  STATIC Elf_Shdr *mShdrBase;
>  STATIC Elf_Phdr *mPhdrBase;
> 
> +//
> +// GOT information
> +//
> +STATIC Elf_Shdr *mGOTShdr = NULL;
> +STATIC UINT32   mGOTShindex = 0;
> +STATIC UINT32   *mGOTCoffEntries = NULL;
> +STATIC UINT32   mGOTMaxCoffEntries = 0;
> +STATIC UINT32   mGOTNumCoffEntries = 0;
> +
>  //
>  // Coff information
>  //
> @@ -322,6 +331,117 @@ GetSymName (
>return StrtabContents + Sym->st_name;
>  }
> 
> +//
> +// Find the ELF section hosting the GOT from an ELF Rva
> +//   of a single GOT entry.  Normally, GOT is placed in
> +//   ELF .text section, so assume once we find in which
> +//   section the GOT is, all GOT entries are there, and
> +//   just verify this.
> +//
> +STATIC
> +VOID
> +FindElfGOTSectionFromGOTEntryElfRva (
> +  Elf64_Addr GOTEntryElfRva
> +  )
> +{
> +  UINT32 i;
> +  if (mGOTShdr != NULL) {
> +if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
> +GOTEntryElfRva <  mGOTShdr->sh_addr + mGOTShdr->sh_size)
> +  return;
> +Error (NULL, 0, 3000, "Unsupported", 
> "FindElfGOTSectionFromGOTEntryElfRva: GOT entries found in multiple 
> sections.");
> +exit(EXIT_FAILURE);
> +  }
> +  for (i = 0; i < mEhdr->e_shnum; i++) {
> +Elf_Shdr *shdr = GetShdrByIndex(i);
> +if (GOTEntryElfRva >= shdr->sh_addr &&
> +GOTEntryElfRva <  shdr->sh_addr + shdr->sh_size) {
> +  mGOTShdr = shdr;
> +  mGOTShindex = i;
> +  return;
> +}
> +  }
> +  Error (NULL, 0, 3000, "Invalid", "FindElfGOTSectionFromGOTEntryElfRva: 
> ElfRva 0x%016LX for GOT entry not found in any section.",
> GOTEntryElfRva);
> +  exit(EXIT_FAILURE);
> +}
> +
> +//
> +// Stores locations of GOT entries in COFF image.
> +//   Returns TRUE if GOT entry is new.
> +//   Simple implementation as number of GOT
> +//   entries is expected to be low.
> +//
> +
> +STATIC
> +BOOLEAN
> +AccumulateCoffGOTEntries (
> +  UINT32 GOTCoffEntry
> +  )
> +{
> +  UINT32 i;
> +  if (mGOTCoffEntries != NULL) {
> +for (i = 0; i < mGOTNumCoffEntries; i++)
> +  if (mGOTCoffEntries[i] == GOTCoffEntry)
> +return FALSE;
> +  }
> +  if (mGOTCoffEntries == NULL) {
> +mGOTCoffEntries = (UINT32*)malloc(5 * sizeof *mGOTCoffEntries);
> +if (mGOTCoffEntries == NULL) {
> +  Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
> +}
> +assert (mGOTCoffEntries != NULL);
> +mGOTMaxCoffEntries = 5;
> +mGOTNumCoffEntries = 0;
> +  } else if (mGOTNumCoffEntries == mGOTMaxCoffEntries) {
> +mGOTCoffEntries = (UINT32*)realloc(mGOTCoffEntries, 2 * 
> mGOTMaxCoffEntries * sizeof *mGOTCoffEntries);
> +if (mGOTCoffEntries == NULL) {
> +  Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
> +}
> +assert (mGOTCoffEntries != NULL);
> +mGOTMaxCoffEntries += mGOTMaxCoffEntries;
> +  }
> +  mGOTCoffEntries[mGOTNumCoffEntries++] = GOTCoffEntry;
> +  return TRUE;
> +}
> +
> +STATIC
> +int
> +__comparator (
> +  const void* lhs,
> +  const void* rhs
> +  )
> +{
> +  if (*(const UINT32*)lhs < *(const UINT32*)rhs)
> +return -1;
> +  return *(const UINT32*)lhs > *(const UINT32*)rhs;
> +}
> +
> +STATIC
> +VOID
> +EmitGOTRelocations (
> +  VOID
> +  )
> +{
> +  UINT32 i;
> +  if (mGOTCoffEntries == NULL)
> +return;
> +  qsort(
> +mGOTCoffEntries,
> +mGOTNumCoffEntries,
> +sizeof *mGOTCoffEntries,
> +__comparator);
> +  for (i = 0; i < mGOTNumCoffEntries; i++) {
> +VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X", 
> mGOTCoffEntries[i]);
> +CoffAddFixup(
> +  mGOTCoffEntries[i],
> +  EFI_IMAGE_REL_BASED_DIR64);
> +  }
> +  free(mGOTCoffEntries);
> +  mGOTCoffEntries = NULL;
> +  mGOTMaxCoffEntries = 0;
> +  mGOTNumCoffEntries = 0;
> +}
> +
>  //
>  // Elf functions interface implementation
>  //
> @@ -698,7 +818,7 @@ WriteSections64 (
>  // section that applies to the entire binary, and which will have 

Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

[Ye, Ting]

Hi Siva,

Per design, all successful attempts will be published to iBFT, no matter it is 
currently used or aborted. Only failed attempts are not published. 
I don't think the current behavior is wrong according to iBFT specification, 
see 
http://download.microsoft.com/download/7/E/7/7E7662CF-CBEA-470B-A97E-CE7CE0D98DC2/iBFT.docx.


Thanks,
Ting

-Original Message-
From: Sivaraman Nainar [mailto:sivaram...@amiindia.co.in] 
Sent: Wednesday, June 6, 2018 5:01 PM
To: Ye, Ting ; Omkar K ; 
edk2-devel@lists.01.org
Cc: Madhan B. Santharam 
Subject: RE: reg: ISCSI Aborted attempt entry in IBFT Table

Hello Ting,

Yes. In the use case we said:

NIC 1 - Target 1 : Login success
NIC 1 - Target 2 : Login success

NIC 2 - Target 1 : Login success

But iSCSI Driver will choose the first login session and abort the other in the 
same NIC. But even it is aborted it is Published to IBFT.

As you said the ESXi and SLES not able to proceed installation assuming more 
than one IBFT for the same NIC as illegal.

So what the clarification we are asking is since it's an aborted attempt can we 
skip adding IBFT entry for the aborted attempt.

Please refer the ticket already created for this has the proposed solution.

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

-Siva
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ye, Ting
Sent: Wednesday, June 06, 2018 2:04 PM
To: Omkar K; edk2-devel@lists.01.org
Cc: Madhan B. Santharam
Subject: Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hi Omkar,

If not MPIO, current iSCSI driver will try the configured attempts and only 
publish the successful entries in iBFT. The failed attempts will be removed.
In your case, it looks the ESXi and SLES OS treat the multiple entries on one 
NIC (different targets) are illegal. 


Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Omkar K
Sent: Tuesday, June 5, 2018 5:01 PM
To: Ye, Ting ; edk2-devel@lists.01.org
Cc: Madhan B. Santharam 
Subject: Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hello Ting,

1. We did not enable MPIO.
2. in IScsiStart(), at this point

//
// Select the first login session. Abort others.
//
if (Private->Session == NULL) {
Private->Session = Session;
BootSelected = AttemptConfigData->AttemptConfigIndex;
//
// Don't validate other attempt in multipath mode if one is success.
//
if (mPrivate->EnableMpio) {
  break;
}
} else {
IScsiSessionAbort (Session);
FreePool (Session);
}

other than one attempt per Nic, other sessions are aborted. Still, all the 
attempts are published in IBFT.
We can observe the issue when different targets are configured on one NIC where 
all the attempts are published in IBFT.
But, the issue disappeared when the aborted attempts are not published in IBFT.

Thanks,
Omkar

-Original Message-
From: Ye, Ting [mailto:ting...@intel.com] 
Sent: Monday, June 04, 2018 2:26 PM
To: Sivaraman Nainar; edk2-devel@lists.01.org
Cc: Omkar K; Madhan B. Santharam
Subject: RE: reg: ISCSI Aborted attempt entry in IBFT Table

Hi Siva,

Per design, the iSCSI multipath I/O will publish all configured attempts to 
IBFT, no matter the connection is success or fail currently.
Did you enable the MPIO when you configure the attempts? 

I am not clear what do you mean "aborted attempt".

Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Sivaraman Nainar
Sent: Thursday, May 31, 2018 8:18 PM
To: edk2-devel@lists.01.org
Cc: Omkar K ; Madhan B. Santharam 
Subject: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hello all:
Here is the issue which requires clarification.
Issue Synopsis:
When there are more than one iSCSI target configured and Ibft table published 
with the connected and aborted attempt details, all the targets are not seen in 
ESXi and SLES OS. But in Windows it can see the targets connected.
Use case:
Target 1: IP : 192.xx.xx.31 Target 2 : IP : 192.xx.xx.1
NIC 1 configured with attempts Target 1 & Target 2 NIC 2 configured with 
attempts Target 1 Connection

Login

Ibft

Block Device in UEFI Shell

SLES / Esx OS

Windows

NIC1 Target1

Success

Published

Mounted

Target Seen



NIC1 Target2

Success

Published

Mounted

Target Seen



NIC1 Target1
NIC1 Target2
NIC2 Target1

Individual Login success

Published for all attempts (3)

NIC1 Target 1 NIC2 Target 1

None Seen

NIC1 Target 1 NIC2 Target 1


When the attempts which are login success but Aborted by ISCSI Driver are 
present in ibft table SLES and ESX not able to see the targets during 
Installation.
If the ISCSI Driver not adding the ibft entry for the aborted attempts then the 
targets are seen in Esx and SLES.
So it requires clarification that If the driver need to SKIP adding the aborted 
attempt entry to ibft or its OS responsibility to 

Re: [edk2] Bug 868 - Need to add several functions related to date and time in BaseLib.

[Laszlo Ersek]

On 06/06/18 10:21, Mohammad Younas Khan Pathan wrote:
> Hi All,
> 
> If there is any generic library function, then we do not need to have
> 2 or more definitions for same function like IsLeapYear().
> 
> Searching for 'isleapyear'
> By Mask:
>   *.c
> ArmPlatformPkg\Library\PL031RealTimeClockLib\PL031RealTimeClockLib.c(199)
> :IsLeapYear (
> ArmPlatformPkg\Library\PL031RealTimeClockLib\PL031RealTimeClockLib.c(227)
> :  (Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
> EmbeddedPkg\Library\HalRuntimeServicesExampleLib\Rtc.c(134) :IsLeapYear (
> EmbeddedPkg\Library\HalRuntimeServicesExampleLib\Rtc.c(163) :
> (Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
> EmulatorPkg\RealTimeClockRuntimeDxe\RealTimeClock.c(33) :IsLeapYear (
> EmulatorPkg\RealTimeClockRuntimeDxe\RealTimeClock.c(279) :
> (Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
> EmulatorPkg\RealTimeClockRuntimeDxe\RealTimeClock.c(288) :IsLeapYear (
> Nt32Pkg\RealTimeClockRuntimeDxe\RealTimeClock.c(37) :IsLeapYear (
> Nt32Pkg\RealTimeClockRuntimeDxe\RealTimeClock.c(356) :
> (Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
> Nt32Pkg\RealTimeClockRuntimeDxe\RealTimeClock.c(365) :IsLeapYear (
> PcAtChipsetPkg\PcatRealTimeClockRuntimeDxe\PcRtc.c(1021) :
> (Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
> PcAtChipsetPkg\PcatRealTimeClockRuntimeDxe\PcRtc.c(1038) :IsLeapYear (
> PcAtChipsetPkg\PcatRealTimeClockRuntimeDxe\PcRtc.c(1178) :  if
> ((From->Month == 2) && !IsLeapYear(From)) {
> 
> Similarly for other functions, can we have the common functions used
> for time in BaseLib or some common library?
> 
> Can we move them to BaseLib or any other common library?
> 
> Bug link:
> https://bugzilla.tianocore.org/show_bug.cgi?id=868
> 
> Please provide your suggestions on the same.

Not sure about BaseLib (or even TimerLib), but we should indeed move
this function to a generic utility package -- maybe a
"TimeCalculationLib" class that manipulates pure time concepts and
values, without any hardware accesses (even abstracted ones).

(

For that same library class, I would probably nominate a function that I
posted earlier, under "TimerTickDiffLib", namely the GetTickDifference()
function.

Because, values returned by TimerLib's GetPerformanceCounter() function
cannot simply be subtracted from each other, because (a) counters don't
necessarily increment until they wrap around (they can be decrementing
too), and (b) the wrap-around's *low* bound may not be zero.

Such differences are needed all over the tree, and even recently a buggy
subtraction was added, as far as I remember...

... Yes: see the GetElapsedTick() function in commit 0edb7ec5ced0,
"MdeModulePkg/PciHostBridge: Count the (mm)io overhead when polling",
2018-05-09. The GetElapsedTick() function does not consider issue (b) above.

The exact same issue can be seen in IsSyncTimerTimeout(),
"UefiCpuPkg/PiSmmCpuDxeSmm/SyncTimer.c".

For comparison, see the GetTickDifference() function here (you'll have
to scroll down a bit):

http://mid.mail-archive.com/8cba2a58-1333-7733-031d-0883dbd844c6@redhat.com

)

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

Re: [edk2] Bug 843 - Need to remove unnecessary return statements in void functions of EDKII.

[Laszlo Ersek]

Hello Younas,

On 06/06/18 10:28, Mohammad Younas Khan Pathan wrote:
> Hi all,
> 
> I have updated the changes after removing all the unecessary return
> statements in EDKII packages.
> 
> I have created EDKIIBug843 branch under master.
> Also attached the changes here.
> 
> Please help to review and share your comments.

please post your patches in-line for review -- please don't try to
attach them (to Bugzilla items or to emails); also don't paste them into
the email body. The edk2 development workflow uses the git toolset; that
set includes tools for formatting patches as emails, and for posting
those patch files as emails.

This is the official Wiki article:

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

And this is my personal take on the matter:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

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

[edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

[Zenith432]

Adds support for the following X64 ELF relocations to GenFw
  R_X86_64_GOTPCREL
  R_X86_64_GOTPCRELX
  R_X86_64_REX_GOTPCRELX

CC: Shi Steven 
CC: Yonghong Zhu 
CC: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zenith432 
---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 166 +++-
 BaseTools/Source/C/GenFw/elf_common.h   |  23 
 2 files changed, 188 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c39bdff0..d2f9bb46 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
 STATIC Elf_Shdr *mShdrBase;
 STATIC Elf_Phdr *mPhdrBase;
 
+//
+// GOT information
+//
+STATIC Elf_Shdr *mGOTShdr = NULL;
+STATIC UINT32   mGOTShindex = 0;
+STATIC UINT32   *mGOTCoffEntries = NULL;
+STATIC UINT32   mGOTMaxCoffEntries = 0;
+STATIC UINT32   mGOTNumCoffEntries = 0;
+
 //
 // Coff information
 //
@@ -322,6 +331,117 @@ GetSymName (
   return StrtabContents + Sym->st_name;
 }
 
+//
+// Find the ELF section hosting the GOT from an ELF Rva
+//   of a single GOT entry.  Normally, GOT is placed in
+//   ELF .text section, so assume once we find in which
+//   section the GOT is, all GOT entries are there, and
+//   just verify this.
+//
+STATIC
+VOID
+FindElfGOTSectionFromGOTEntryElfRva (
+  Elf64_Addr GOTEntryElfRva
+  )
+{
+  UINT32 i;
+  if (mGOTShdr != NULL) {
+if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
+GOTEntryElfRva <  mGOTShdr->sh_addr + mGOTShdr->sh_size)
+  return;
+Error (NULL, 0, 3000, "Unsupported", "FindElfGOTSectionFromGOTEntryElfRva: 
GOT entries found in multiple sections.");
+exit(EXIT_FAILURE);
+  }
+  for (i = 0; i < mEhdr->e_shnum; i++) {
+Elf_Shdr *shdr = GetShdrByIndex(i);
+if (GOTEntryElfRva >= shdr->sh_addr &&
+GOTEntryElfRva <  shdr->sh_addr + shdr->sh_size) {
+  mGOTShdr = shdr;
+  mGOTShindex = i;
+  return;
+}
+  }
+  Error (NULL, 0, 3000, "Invalid", "FindElfGOTSectionFromGOTEntryElfRva: 
ElfRva 0x%016LX for GOT entry not found in any section.", GOTEntryElfRva);
+  exit(EXIT_FAILURE);
+}
+
+//
+// Stores locations of GOT entries in COFF image.
+//   Returns TRUE if GOT entry is new.
+//   Simple implementation as number of GOT
+//   entries is expected to be low.
+//
+
+STATIC
+BOOLEAN
+AccumulateCoffGOTEntries (
+  UINT32 GOTCoffEntry
+  )
+{
+  UINT32 i;
+  if (mGOTCoffEntries != NULL) {
+for (i = 0; i < mGOTNumCoffEntries; i++)
+  if (mGOTCoffEntries[i] == GOTCoffEntry)
+return FALSE;
+  }
+  if (mGOTCoffEntries == NULL) {
+mGOTCoffEntries = (UINT32*)malloc(5 * sizeof *mGOTCoffEntries);
+if (mGOTCoffEntries == NULL) {
+  Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+}
+assert (mGOTCoffEntries != NULL);
+mGOTMaxCoffEntries = 5;
+mGOTNumCoffEntries = 0;
+  } else if (mGOTNumCoffEntries == mGOTMaxCoffEntries) {
+mGOTCoffEntries = (UINT32*)realloc(mGOTCoffEntries, 2 * mGOTMaxCoffEntries 
* sizeof *mGOTCoffEntries);
+if (mGOTCoffEntries == NULL) {
+  Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+}
+assert (mGOTCoffEntries != NULL);
+mGOTMaxCoffEntries += mGOTMaxCoffEntries;
+  }
+  mGOTCoffEntries[mGOTNumCoffEntries++] = GOTCoffEntry;
+  return TRUE;
+}
+
+STATIC
+int
+__comparator (
+  const void* lhs,
+  const void* rhs
+  )
+{
+  if (*(const UINT32*)lhs < *(const UINT32*)rhs)
+return -1;
+  return *(const UINT32*)lhs > *(const UINT32*)rhs;
+}
+
+STATIC
+VOID
+EmitGOTRelocations (
+  VOID
+  )
+{
+  UINT32 i;
+  if (mGOTCoffEntries == NULL)
+return;
+  qsort(
+mGOTCoffEntries,
+mGOTNumCoffEntries,
+sizeof *mGOTCoffEntries,
+__comparator);
+  for (i = 0; i < mGOTNumCoffEntries; i++) {
+VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X", 
mGOTCoffEntries[i]);
+CoffAddFixup(
+  mGOTCoffEntries[i],
+  EFI_IMAGE_REL_BASED_DIR64);
+  }
+  free(mGOTCoffEntries);
+  mGOTCoffEntries = NULL;
+  mGOTMaxCoffEntries = 0;
+  mGOTNumCoffEntries = 0;
+}
+
 //
 // Elf functions interface implementation
 //
@@ -698,7 +818,7 @@ WriteSections64 (
 // section that applies to the entire binary, and which will have its 
section
 // index set to #0 (which is a NULL section with the SHF_ALLOC bit 
cleared).
 //
-// In the absence of GOT based relocations (which we currently don't 
support),
+// In the absence of GOT based relocations,
 // this RELA section will contain redundant R_xxx_RELATIVE relocations, one
 // for every R_xxx_xx64 relocation appearing in the per-section RELA 
sections.
 // (i.e., .rela.text and .rela.data)
@@ -780,6 +900,7 @@ WriteSections64 (
 // Determine how to handle each relocation type based on the machine 
type.
 //
 if (mEhdr->e_machine == EM_X86_64) {
+  Elf64_Addr GOTEntryRva;
   switch 

Re: [edk2] [Patch v3 2/2] SignedCapsulePkg/SystemFirmwareUpdateDxe: Use progress API

[Ard Biesheuvel]

On 29 May 2018 at 18:17, Michael D Kinney  wrote:
> From: "Kinney, Michael D" 
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=801
>
> Use PlatformFlashWriteWithProgress() instead of PlatformFLashWrite()
> so the user can be informed of the progress as a capsule is used
> to update a firmware image in a firmware device.
>
> Cc: Jiewen Yao 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1

Tested-by: Ard Biesheuvel 
Reviewed-by: Ard Biesheuvel 


> ---
>  .../SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c | 90 
> --
>  1 file changed, 67 insertions(+), 23 deletions(-)
>
> diff --git 
> a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c 
> b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
> index ce6892d6a9..8e66aedf62 100644
> --- 
> a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
> +++ 
> b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
> @@ -65,11 +65,14 @@ ParseUpdateDataFile (
>  **/
>  EFI_STATUS
>  PerformUpdate (
> -  IN VOID *SystemFirmwareImage,
> -  IN UINTNSystemFirmwareImageSize,
> -  IN UPDATE_CONFIG_DATA   *ConfigData,
> -  OUT UINT32  *LastAttemptVersion,
> -  OUT UINT32  *LastAttemptStatus
> +  IN VOID   *SystemFirmwareImage,
> +  IN UINTN  SystemFirmwareImageSize,
> +  IN UPDATE_CONFIG_DATA *ConfigData,
> +  OUT UINT32*LastAttemptVersion,
> +  OUT UINT32*LastAttemptStatus,
> +  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress,
> +  IN UINTN  StartPercentage,
> +  IN UINTN  EndPercentage
>)
>  {
>EFI_STATUS   Status;
> @@ -78,13 +81,22 @@ PerformUpdate (
>DEBUG((DEBUG_INFO, "  BaseAddress - 0x%lx,", ConfigData->BaseAddress));
>DEBUG((DEBUG_INFO, "  ImageOffset - 0x%x,", ConfigData->ImageOffset));
>DEBUG((DEBUG_INFO, "  Legnth - 0x%x\n", ConfigData->Length));
> -  Status = PerformFlashWrite (
> +  if (Progress != NULL) {
> +Progress (StartPercentage);
> +  }
> +  Status = PerformFlashWriteWithProgress (
>   ConfigData->FirmwareType,
>   ConfigData->BaseAddress,
>   ConfigData->AddressType,
>   (VOID *)((UINTN)SystemFirmwareImage + 
> (UINTN)ConfigData->ImageOffset),
> - ConfigData->Length
> + ConfigData->Length,
> + Progress,
> + StartPercentage,
> + EndPercentage
>   );
> +  if (Progress != NULL) {
> +Progress (EndPercentage);
> +  }
>if (!EFI_ERROR(Status)) {
>  *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
>  if (ConfigData->FirmwareType == PlatformFirmwareTypeNvRam) {
> @@ -111,12 +123,13 @@ PerformUpdate (
>  **/
>  EFI_STATUS
>  UpdateImage (
> -  IN VOID *SystemFirmwareImage,
> -  IN UINTNSystemFirmwareImageSize,
> -  IN VOID *ConfigImage,
> -  IN UINTNConfigImageSize,
> -  OUT UINT32  *LastAttemptVersion,
> -  OUT UINT32  *LastAttemptStatus
> +  IN VOID   *SystemFirmwareImage,
> +  IN UINTN  SystemFirmwareImageSize,
> +  IN VOID   *ConfigImage,
> +  IN UINTN  ConfigImageSize,
> +  OUT UINT32*LastAttemptVersion,
> +  OUT UINT32*LastAttemptStatus,
> +  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress
>)
>  {
>EFI_STATUSStatus;
> @@ -124,19 +137,34 @@ UpdateImage (
>UPDATE_CONFIG_DATA*UpdateConfigData;
>CONFIG_HEADER ConfigHeader;
>UINTN Index;
> +  UINTN TotalSize;
> +  UINTN BytesWritten;
> +  UINTN StartPercentage;
> +  UINTN EndPercentage;
>
>if (ConfigImage == NULL) {
>  DEBUG((DEBUG_INFO, "PlatformUpdate (NoConfig):"));
>  DEBUG((DEBUG_INFO, "  BaseAddress - 0x%x,", 0));
>  DEBUG((DEBUG_INFO, "  Length - 0x%x\n", SystemFirmwareImageSize));
>  // ASSUME the whole System Firmware include NVRAM region.
> -Status = PerformFlashWrite (
> +StartPercentage = 0;
> +EndPercentage = 100;
> +if (Progress != NULL) {
> +  Progress (StartPercentage);
> +}
> +Status = PerformFlashWriteWithProgress (
>  

Re: [edk2] [Patch v3 1/2] MdeModulePkg/DxeCapsuleLibFmp: Add progress bar support

[Ard Biesheuvel]

On 29 May 2018 at 18:17, Michael D Kinney  wrote:
> From: "Kinney, Michael D" 
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=801
>
> Based on content from the following branch/commits:
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport
>
> * Change Update_Image_Progress() to UpdateImageProcess()
> * Call DisplayUpdateProgressLib from UpdateImageProgress().
> * Split out a boot service and runtime version of
>   UpdateImageProgress() so the DisplayUpdateProgressLib is
>   not used at runtime.
> * If gEdkiiFirmwareManagementProgressProtocolGuid is present,
>   then use its progress bar color and watchdog timer value.
> * If gEdkiiFirmwareManagementProgressProtocolGuid is not present,
>   then use default progress bar color and 5 min watchdog timer.
> * Remove Print() calls during capsule processing.  Instead,
>   the DisplayUpdateProgressLib is used to inform the user
>   of progress during a capsule update.
>
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: Sean Brogan 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1

Tested-by: Ard Biesheuvel 
Reviewed-by: Ard Biesheuvel 

Please don't push this before the edk2-platforms changes are in.

> ---
>  .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c   | 47 +---
>  .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf |  8 ++-
>  .../DxeCapsuleLibFmp/DxeCapsuleProcessLib.c| 84 
> --
>  .../DxeCapsuleLibFmp/DxeCapsuleProcessLibNull.c| 21 +-
>  .../DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf  |  7 +-
>  5 files changed, 131 insertions(+), 36 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c 
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index 05fcd92deb..f0226eafa5 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -45,6 +45,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
> @@ -53,6 +54,8 @@ BOOLEAN   mIsVirtualAddrConverted  = 
> FALSE;
>  BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
>  EFI_EVENT mDxeCapsuleLibEndOfDxeEvent  = NULL;
>
> +EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL  *mFmpProgress = NULL;
> +
>  /**
>Initialize capsule related variables.
>  **/
> @@ -101,18 +104,17 @@ RecordFmpCapsuleStatusVariable (
>Function indicate the current completion progress of the firmware
>update. Platform may override with own specific progress function.
>
> -  @param[in]  CompletionA value between 1 and 100 indicating the current 
> completion progress of the firmware update
> +  @param[in]  Completion  A value between 1 and 100 indicating the current
> +  completion progress of the firmware update
>
> -  @retval EFI_SUCESSInput capsule is a correct FMP capsule.
> +  @retval EFI_SUCESS The capsule update progress was updated.
> +  @retval EFI_INVALID_PARAMETER  Completion is greater than 100%.
>  **/
>  EFI_STATUS
>  EFIAPI
> -Update_Image_Progress (
> +UpdateImageProgress (
>IN UINTN  Completion
> -  )
> -{
> -  return EFI_SUCCESS;
> -}
> +  );
>
>  /**
>Return if this CapsuleGuid is a FMP capsule GUID or not.
> @@ -849,6 +851,19 @@ SetFmpImageData (
>  return Status;
>}
>
> +  //
> +  // Lookup Firmware Management Progress Protocol before SetImage() is called
> +  // This is an optional protocol that may not be present on Handle.
> +  //
> +  Status = gBS->HandleProtocol (
> +  Handle,
> +  ,
> +  (VOID **)
> +  );
> +  if (EFI_ERROR (Status)) {
> +mFmpProgress = NULL;
> +  }
> +
>if (ImageHeader->Version >= 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
>  Image = (UINT8 *)(ImageHeader + 1);
>} else {
> @@ -873,21 +888,37 @@ SetFmpImageData (
>  DEBUG((DEBUG_INFO, "(UpdateHardwareInstance - 0x%x)", 
> ImageHeader->UpdateHardwareInstance));
>}
>DEBUG((DEBUG_INFO, "\n"));
> +
> +  //
> +  // Before calling SetImage(), reset the progress bar to 0%
> +  //
> +  UpdateImageProgress (0);
> +
>Status = Fmp->SetImage(
>Fmp,
>ImageHeader->UpdateImageIndex,  // ImageIndex
>Image,  // Image
>ImageHeader->UpdateImageSize,   // ImageSize
>VendorCode, // VendorCode
> -  Update_Image_Progress,  // Progress
> +  UpdateImageProgress,// Progress
>// AbortReason
>);
> +  //
> +  // Set the progress bar to 100% after returning from SetImage()
> +  //
> +  UpdateImageProgress (100);
> +
>DEBUG((DEBUG_INFO, 

Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Ard Biesheuvel]

On 6 June 2018 at 18:07, Kinney, Michael D  wrote:
> Ard,
>
> Thanks for adding the note.  I was thinking that this could be done
> just before ResetSystem().  It could also be done in SEC phase.
>
> Since capsules are just one use of warm reset, would it make more sense
> to flush all the caches in either warm reset of SEC instead of just
> the ranges used by capsules.
>

The ARM architecture does not provide for that, unfortunately. The
only architected cache maintenance that guarantees that dirty
cachelines make it all the way to memory (point of coherency or PoC in
ARM parlance) is to clean the caches by virtual address, and cleaning
all of memory by VA is intractible.

The architecture does provide clean by set/way operations, but those
operate on each cache level individually, and only on architected
cache levels. (The architecture permits so-called system caches that
whose set/way maintenance is implementation defined, and only
maintenance by VA is guaranteed to clean the data to main memory)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Kinney, Michael D]

Ard,

Thanks for adding the note.  I was thinking that this could be done
just before ResetSystem().  It could also be done in SEC phase.

Since capsules are just one use of warm reset, would it make more sense
to flush all the caches in either warm reset of SEC instead of just
the ranges used by capsules.

Thanks,

Mike

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Wednesday, June 6, 2018 5:10 AM
> To: edk2-devel@lists.01.org
> Cc: Leif Lindholm ; Yao, Jiewen
> ; Ni, Ruiyu ;
> Zeng, Star ; Kinney, Michael D
> ; Gao, Liming
> ; Ard Biesheuvel
> 
> Subject: Re: [PATCH] MdeModulePkg/CapsulePei: clean Dcache
> before consuming capsule data
> 
> On 6 June 2018 at 11:52, Ard Biesheuvel
>  wrote:
> > When capsule updates are staged for processing after a
> warm reboot,
> > they are copied into memory with the MMU and caches
> enabled. When
> > the capsule PEI gets around to coalescing the capsule,
> the MMU and
> > caches may still be disabled, and so on architectures
> where uncached
> > accesses are incoherent with the caches (such as ARM and
> AARCH64),
> > we may read stale data if we don't clean the caches to
> memory first.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> 
> Leif asked me to include a note why this cannot be done
> when
> UpdateCapsule() is called.
> 
> 
> """
> Note that this cache maintenance cannot be done during the
> invocation
> of UpdateCapsule(), since the ScatterGatherList structures
> are only
> identified by physical address, and at runtime, the
> firmware doesn't
> know whether and where this memory is mapped, and cache
> maintenance
> requires a virtual address.
> """
> 
> > ---
> >  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> | 1 +
> >
> MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> | 4 
> >  2 files changed, 5 insertions(+)
> >
> > diff --git
> a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > index c54bc21a95a8..594e110d1f8a 100644
> > --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > @@ -48,6 +48,7 @@ [Packages]
> >
> >  [LibraryClasses]
> >BaseLib
> > +  CacheMaintenanceLib
> >HobLib
> >BaseMemoryLib
> >PeiServicesLib
> > diff --git
> a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.
> c
> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.
> c
> > index 3e7054cd38a9..1730f925adc5 100644
> > ---
> a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.
> c
> > +++
> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.
> c
> > @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS
> OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >  #include 
> >
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource (
> >DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in
> MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
> >Address, Size,
> >Index,
> MemoryResource[Index].PhysicalStart,
> MemoryResource[Index].ResourceLength));
> > +
> > +  WriteBackDataCacheRange ((VOID *)(UINTN)Address,
> Size);
> > +
> >return TRUE;
> >  }
> >}
> > --
> > 2.17.0
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 00/17] *** Standalone Management Mode Core Interface for AARCH64 Platforms ***

[Supreeth Venkatesh]

Thanks for Testing.
Appreciate it.

Supreeth

-Original Message-
From: Sughosh Ganu 
Sent: Wednesday, June 6, 2018 6:20 AM
To: Supreeth Venkatesh 
Cc: edk2-devel@lists.01.org; jiewen@intel.com; liming@intel.com
Subject: Re: [edk2] [PATCH v3 00/17] *** Standalone Management Mode Core 
Interface for AARCH64 Platforms ***

On Tue, Jun 5, 2018 at 3:43 AM, Supreeth Venkatesh  
wrote:
> ***
> This patchset v3 contains only the patches that got feedback/comments frome 
> the previous revision v2.
> The patches are
> [PATCH v3 06/17] StandaloneMmPkg: Delete StandaloneMmPkg file.
> [PATCH v3 13/17] StandaloneMmPkg: Add an AArch64 specific entry point library.
> [PATCH v3 17/17] BaseTools/AutoGen: Update header file for MM modules.
>
> Changes Since v2:
> (*) Address feedback provided for the commit "BaseTools/AutoGen: Update 
> header file for MM modules."
> (*) Edit parameters for the StandaloneMmCpu Driver in the commit 
> "StandaloneMmPkg: Add an AArch64 specific entry point library."
>
> Changes Since v1:
> (*) Reorder and Reword commits.
> (*) Reorganize structure of StandaloneMmPkg and rename libraries.
> (*) Address Review Comments from Achin, Jiewen and Daniil.
> ***
> Supreeth Venkatesh (17):
>   ArmPkg: Add PCDs needed for MM communication driver.
>   ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
>   ArmPkg/Include: Add MM interface SVC return codes.
>   ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0.
>   ArmPkg/ArmMmuLib: Add MMU library inf file suitable for use in S-EL0.
>   StandaloneMmPkg: Delete StandaloneMmPkg file.
>   StandaloneMmPkg/FvLib: Add a common FV Library for management mode.
>   StandaloneMmPkg/MemLib: Add Standalone MM instance of memory check
> library.
>   StandaloneMmPkg/MemoryAllocationLib: Add MM memory allocation library.
>   StandaloneMmPkg/HobLib: Add HOB Library for management mode.
>   StandaloneMmPkg: MM driver entry point library.
>   StandaloneMmPkg/Core: Implementation of Standalone MM Core Module.
>   StandaloneMmPkg: Add an AArch64 specific entry point library.
>   StandaloneMmPkg: Add CPU driver suitable for ARM Platforms.
>   StandaloneMmPkg: Describe the declaration and definition files.
>   ArmPkg: Extra action to update permissions for S-ELO MM Image.
>   BaseTools/AutoGen: Update header file for MM modules.

Tested all changes for RAS error injection and error handling on
SGI575 platform.

Tested-by: Sughosh Ganu 

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

Re: [edk2] Crc32 Calculation Miss

[Andrew Fish]

> On Jun 6, 2018, at 7:18 AM, Guy Raviv  wrote:
> 
> Hi Everyone,
> 
> I have an odd problem.
> In a specific module in my code (MRC training), code modification doesn't
> change the BIOS Crc calculation.
> 1. i checked in my project's fdf file if the FV base address and region
> size defined are the same as the parameters i'm putting in the calculation.

Guy,

What region did you check? There are areas of the FLASH that get updated 
(NVRAM, region used by an embedded controller, etc.), or never get updated 
(system serial # programmed at the factory). 

> 2. i also compared two different binaries and found they are exactly the
> same.
> 

Did you compare the binaries as part of the build or on the system? 

Thanks,

Andrew Fish

> does anyone have any idea what can be wrong here or where should i check?
> 
> Thanks,
> Guy
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] Crc32 Calculation Miss

[Gao, Liming]

Do you use CalculateCrc32() in BaseLib to calculate CRC32 value for the 
different buffer?

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Guy 
> Raviv
> Sent: Wednesday, June 6, 2018 10:18 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Crc32 Calculation Miss
> 
> Hi Everyone,
> 
> I have an odd problem.
> In a specific module in my code (MRC training), code modification doesn't
> change the BIOS Crc calculation.
> 1. i checked in my project's fdf file if the FV base address and region
> size defined are the same as the parameters i'm putting in the calculation.
> 2. i also compared two different binaries and found they are exactly the
> same.
> 
> does anyone have any idea what can be wrong here or where should i check?
> 
> Thanks,
> Guy
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Crc32 Calculation Miss

[Guy Raviv]

Hi Everyone,

I have an odd problem.
In a specific module in my code (MRC training), code modification doesn't
change the BIOS Crc calculation.
1. i checked in my project's fdf file if the FV base address and region
size defined are the same as the parameters i'm putting in the calculation.
2. i also compared two different binaries and found they are exactly the
same.

does anyone have any idea what can be wrong here or where should i check?

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

Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Yao, Jiewen]

Thanks. Please also include the note in the final commit message.

Reviewed-by: jiewen@intel.com


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Wednesday, June 6, 2018 5:10 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Ard Biesheuvel 
> ;
> Gao, Liming ; Yao, Jiewen ; Leif
> Lindholm ; Kinney, Michael D
> ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before
> consuming capsule data
> 
> On 6 June 2018 at 11:52, Ard Biesheuvel  wrote:
> > When capsule updates are staged for processing after a warm reboot,
> > they are copied into memory with the MMU and caches enabled. When
> > the capsule PEI gets around to coalescing the capsule, the MMU and
> > caches may still be disabled, and so on architectures where uncached
> > accesses are incoherent with the caches (such as ARM and AARCH64),
> > we may read stale data if we don't clean the caches to memory first.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> 
> Leif asked me to include a note why this cannot be done when
> UpdateCapsule() is called.
> 
> 
> """
> Note that this cache maintenance cannot be done during the invocation
> of UpdateCapsule(), since the ScatterGatherList structures are only
> identified by physical address, and at runtime, the firmware doesn't
> know whether and where this memory is mapped, and cache maintenance
> requires a virtual address.
> """
> 
> > ---
> >  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf   | 1 +
> >  MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 4 
> >  2 files changed, 5 insertions(+)
> >
> > diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > index c54bc21a95a8..594e110d1f8a 100644
> > --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > @@ -48,6 +48,7 @@ [Packages]
> >
> >  [LibraryClasses]
> >BaseLib
> > +  CacheMaintenanceLib
> >HobLib
> >BaseMemoryLib
> >PeiServicesLib
> > diff --git a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> > index 3e7054cd38a9..1730f925adc5 100644
> > --- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> > +++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> > @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> >  #include 
> >
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource (
> >DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in
> MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
> >Address, Size,
> >Index, MemoryResource[Index].PhysicalStart,
> MemoryResource[Index].ResourceLength));
> > +
> > +  WriteBackDataCacheRange ((VOID *)(UINTN)Address, Size);
> > +
> >return TRUE;
> >  }
> >}
> > --
> > 2.17.0
> >
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Bug 843 - Need to remove unnecessary return statements in void functions of EDKII.

[Mohammad Younas Khan Pathan]

Hi all,

I have updated the changes after removing all the unecessary return
statements in EDKII packages.

I have created EDKIIBug843 branch under master.
Also attached the changes here.

Please help to review and share your comments.

Thank you,
Younas khan.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] 4MB flash size as build default.

[Prerna]

On Wed, Jun 6, 2018 at 4:02 PM, Laszlo Ersek  wrote:

> On 06/06/18 05:05, Prerna wrote:
> > Hi Laszlo,
> > I noticed that the following commit was used to make the 4MB flash layout
> > the default for edk2 builds briefly:
> >
> > bba8dfbec3bb OvmfPkg: make the 4MB flash size the default
> >
> > However, it was later reverted :
> >
> > 6e49d01cfb43 Revert "OvmfPkg: make the 4MB flash size the default"
> >
> > presumably to fix this issue:
> >
> > 0c79471d6a98 OvmfPkg/PlatformPei: handle non-power-of-two spare
> >   size for emu variables
>
> No, that's not exactly the reason. Please see the following sub-thread:
>
> http://mid.mail-archive.com/bdf196e8-df87-84b9-1d6e-
> bbece5fa6...@redhat.com
>
> Briefly, the 4MB unified pflash size broke two things:
> - the EmuVariableFvbRuntimeDxe driver, which would only affect QEMU
>   command lines with -bios,
> - the PlatformPei PEIM, which would affect QEMU command lines with
>   -pflash too.
>
> (In the linked message, I explained why I hadn't noticed even regression
> #2.)
>
> The temporary solution was to (a) restore the default to 2MB -- this
> would allow people to continue using both -bios and -pflash, and (b)
> kludge around the PlatformPei regression, so that the (manually
> selected) 4MB build would be usable for -pflash, at the cost of wasting
> a little bit of reserved memory.
>
> In other words, the last two commits that you list have no
> inter-dependency between them, instead they mitigated two aspects of the
> regression.
>
> In the longer term, I extended EmuVariableFvbRuntimeDxe (and the
> interfacing code in PlatformPei) to handle the new 4MB unified size as
> well. Please refer to
> . This completed the
> 4MB support for "-bios" from the OVMF side; however it turned out that
> even Xen needed fixes.
>
> Once Xen too received the necessary patches, we re-enabled the 4MB
> default size (commit 1c47fcd465a4 -- basically a re-application of
> commit bba8dfbe). Please refer to
>  db629722c...@redhat.com>,
> and the other message it references.
>
> > Given that the above commit was also merged, why does edk2 still default
> to
> > the 2MB flash layout for builds?
>
> It doesn't; if you build upstream edk2, it defaults to 4MB.
>

Thank you. It appears I had missed the new commits that re-enabled the 4MB
size as default.



>
> We preserved the 2MB unified size in Gerd's and Fedora's OVMF builds
> because the varstore structures are incompatible between the 2MB and the
> 4MB builds. (Please refer to section "OVMF Flash Layout" in the
> "OvmfPkg/README" file.) If you have a preexistent varstore file,
> originally created from the OVMF_VARS.fd template of the 2MB build, and
> then you boot the same VM with the OVMF_CODE.fd binary of the 4MB build,
> your VM will not boot. (It will just hang with a black screen. If you
> capture the OVMF debug log, you will get some error messages, but
> practically no user ever captures the debug log.)
>
> In other words, we preserved the 2MB unified size in Fedora and in
> Gerd's repo for staying compatible with the existent VMs of the users of
> those distros / repos.
>

yes, I had played around with these two different layouts.
I found my VMs to hang and not boot in case I mismatched the OVMF_VARS.fd.



>
> As one counter-example, we switched from 2MB to 4MB in RHEL-7.4. This
> broke compatibility with VMs created against OVMF in RHEL-7.3, but that
> was fine: in RHEL-7.3, OVMF was Tech Preview, and breaking compat with
> Tech Preview packages is explicitly permitted.
>
> Now obviously users of Fedora and Gerd's repo aren't *officially*
> entitled to any kind of stability or support (which is why I don't run
> Fedora for my day to day work, by the way), so we could have broken
> compat there as well, right? Well, yes, if only we had wanted to field
> tens or maybe hundreds of complaints on the vfio-users mailing list and
> elsewhere. :) Those users most likely didn't *need* the 4MB build -- see
> the motivation for the 4MB build in commit b24fca05751f --, hence
> keeping compat for them was more important. Regarding direct consumers
> of the upstream edk2 *source* tree, we figured they were technical
> enough to deal with the change in the default size.
>
> > Do you see any other issues too with the 4MB flash layout ?
>
> Nope.
>
> If you really want to use the 4MB build in Fedora (although I don't see
> why you would), it's technically possible to provide the split 4MB build
> in addition to the current files (under different filenames). In that
> case, please file an RHBZ for the "edk2" component in Fedora (and CC
> Gerd on it so he can adapt his package as well, if he intends to).
>
>
I do not necessarily need Fedora. Will be happy to build from upstream.
Thank you for patiently explaining the implications of non-power-of-2 for

EmuVariableFvbRuntimeDxe.

My limited reading had just spotted the 

[edk2] Bug 868 - Need to add several functions related to date and time in BaseLib.

[Mohammad Younas Khan Pathan]

Hi All,

If there is any generic library function, then we do not need to have
2 or more definitions for same function like IsLeapYear().

Searching for 'isleapyear'
By Mask:
*.c
ArmPlatformPkg\Library\PL031RealTimeClockLib\PL031RealTimeClockLib.c(199)
:IsLeapYear (
ArmPlatformPkg\Library\PL031RealTimeClockLib\PL031RealTimeClockLib.c(227)
:  (Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
EmbeddedPkg\Library\HalRuntimeServicesExampleLib\Rtc.c(134) :IsLeapYear (
EmbeddedPkg\Library\HalRuntimeServicesExampleLib\Rtc.c(163) :
(Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
EmulatorPkg\RealTimeClockRuntimeDxe\RealTimeClock.c(33) :IsLeapYear (
EmulatorPkg\RealTimeClockRuntimeDxe\RealTimeClock.c(279) :
(Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
EmulatorPkg\RealTimeClockRuntimeDxe\RealTimeClock.c(288) :IsLeapYear (
Nt32Pkg\RealTimeClockRuntimeDxe\RealTimeClock.c(37) :IsLeapYear (
Nt32Pkg\RealTimeClockRuntimeDxe\RealTimeClock.c(356) :
(Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
Nt32Pkg\RealTimeClockRuntimeDxe\RealTimeClock.c(365) :IsLeapYear (
PcAtChipsetPkg\PcatRealTimeClockRuntimeDxe\PcRtc.c(1021) :
(Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))
PcAtChipsetPkg\PcatRealTimeClockRuntimeDxe\PcRtc.c(1038) :IsLeapYear (
PcAtChipsetPkg\PcatRealTimeClockRuntimeDxe\PcRtc.c(1178) :  if
((From->Month == 2) && !IsLeapYear(From)) {

Similarly for other functions, can we have the common functions used
for time in BaseLib or some common library?

Can we move them to BaseLib or any other common library?

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

Please provide your suggestions on the same.

Thank you,
Younas khan.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] 4MB flash size as build default.

[Prerna]

Hi Laszlo,
I noticed that the following commit was used to make the 4MB flash layout
the default for edk2 builds briefly:

bba8dfbec3bb OvmfPkg: make the 4MB flash size the default

However, it was later reverted :

6e49d01cfb43 Revert "OvmfPkg: make the 4MB flash size the default"

presumably to fix this issue:

0c79471d6a98 OvmfPkg/PlatformPei: handle non-power-of-two spare
  size for emu variables

Given that the above commit was also merged, why does edk2 still default to
the 2MB flash layout for builds? Do you see any other issues too with the
4MB flash layout ?

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

Re: [edk2] How to Interpret ReadKeyStrokeEX Data

[Andrew Fish]

> On Jun 6, 2018, at 6:03 AM, jim.dai...@dell.com wrote:
> 
> Thanks, Mike.  I figured that out yesterday.
> 
> I think the only problem now is that any shell built with the original change 
> to edit will not work as expected if it is run on a system whose BIOS was 
> created before the second change was made (i.e. pretty much every existing 
> UEFI BIOS in the world!).
> 
> Since the shell is a stand-alone tool that is not necessarily tied to any 
> particular machine, I think that it is likely that more than a few people may 
> run into this problem in the future.
> 

Jim,

You bring up a really good issue about the compatibility targets for the EFI 
Shell.I'll bring this up at the next Tianocore Stewards meeting so we can put 
some more thought into what the right thing to do is. 

Thanks,

Andrew Fish


> Regards,
> Jim
> 
> -Original Message-
> From: Rothman, Michael A [mailto:michael.a.roth...@intel.com] 
> Sent: Tuesday, June 5, 2018 10:38 PM
> To: Rothman, Michael A; Dailey, Jim; Ni, Ruiyu
> Cc: Carsey, Jaben; edk2-devel@lists.01.org; fel...@mail.ru
> Subject: RE: How to Interpret ReadKeyStrokeEX Data
> 
> Jim,
> 
> I think the problem you're seeing is that the USB keyboard driver you're 
> using is downrev and needs to be updated.
> 
> If you look at 
> https://github.com/tianocore/edk2/commit/dd190645eb43424706eb1709d0032c69a1935d9f
>  there was a fix checked in to address exactly the issue you're running into. 
> It's basically a symptom of running a new shell without a correspondingly 
> updated keyboard driver. The new shell in effect exposed a latent bug.
> 
> Hope that helps.
> 
> Thanks,
> Mike Rothman 
> (迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
> רועה עיקרי של חתולים
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Rothman, Michael A
> Sent: Monday, June 4, 2018 10:31 AM
> To: jim.dai...@dell.com; Ni, Ruiyu 
> Cc: Carsey, Jaben ; edk2-devel@lists.01.org; 
> fel...@mail.ru
> Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data
> 
> Since I'm largely the person who might be to blame for the language and 
> intent here and I’ll focus on the spec-related item.  See my comments below.
> 
> 
> 
> Thanks,
> 
> Mike Rothman
> 
> (迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
> 
> רועה עיקרי של חתולים
> 
> 
> 
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> jim.dai...@dell.com
> Sent: Friday, June 1, 2018 11:27 AM
> To: Ni, Ruiyu 
> Cc: Carsey, Jaben ; edk2-devel@lists.01.org; 
> fel...@mail.ru
> Subject: [edk2] How to Interpret ReadKeyStrokeEX Data
> 
> 
> 
> (Subject changed)
> 
> 
> 
> I guess this is a question of UEFI spec interpretation.  In the Console I/O 
> Protocol description of Version 2.5 of the spec (page 456), it says:
> 
> 
> 
>If the Scan Code is set to 0x00 then the Unicode character is
> 
>valid and should be used.
> 
> 
> 
> To me that clearly says it all.  The shift modifier is a don't care when the 
> scan code is zero.  And, this change in the shell code seems to be a 
> violation of that statement.
> 
> 
> 
> However, I also see some confusing (and grammatically incorrect) text in the 
> description of the ReadKeyStrokeEx() function of the simple text in protocol 
> that I am guessing is related to this change (*emphasis* mine):
> 
> 
> 
>When interpreting the data from this function, it should be
> 
>noted that if a class of printable characters that are normally
> 
>*adjusted* by shift modifiers (e.g. Shift Key + "f" key) would
> 
>be presented solely as a KeyData.Key.UnicodeChar without the
> 
>associated shift state.
> 
> 
> 
> What I think the spec is trying to say here is that for A-Z and a-z, the 
> consumer does NOT need to look at the shift state to tell "A" from "a", for 
> example, because the Unicode character will be either "A" or "a" as 
> appropriate.
> 
> 
> 
> n  No, it is any key that would create a displayable character that adjusts 
> based on the shift state. Realize that the users of 
> ReadKeyStroke/ReadKeyStrokeEx fall back to a common denominator of Scan Code 
> or Unicode Character. So if someone types a shift 4, the underlying keyboard 
> layout that the keyboard driver controls would dictate how that gets 
> translated. On my keyboard in the US it turns into a “$” symbol, while 
> someone in Europe may very well have a software-defined keyboard layout which 
> translates the same keystroke to a “€” symbol. That of course applies to the 
> many characters you specified (A-F, a-f) and many others.
> 
> n  The text above is intended to imply what it says, “a class of printable 
> characters … would be presented solely as a KeyData.Key.UnicodeChar without 
> the associated shift state. This makes consumers of both the Ex and Non-Ex 
> variant of ReadKeyStroke able to use the same logic to test for any shiftable 
> characters by simply looking at the Unicode value. I’d 

Re: [edk2] [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot

[Laszlo Ersek]

On 06/06/18 14:37, Ard Biesheuvel wrote:
> Implement ResetSystemLib's EnterS3WithImmediateWake() routine using
> a jump back to the PEI entry point with interrupts and MMU+caches
> disabled. This is only possible at boot time, when we are sure that
> the current CPU is the only one up and running. Also, it depends on
> the platform whether the PEI code is preserved in memory (it may be
> copied to DRAM rather than execute in place), so also add a feature
> PCD to selectively enable this feature.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/ArmPkg.dec|  4 
> 
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 21 
> ++--
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf |  9 
> +
>  3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index debe066b6f7b..3aa229fe2ec9 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -85,6 +85,10 @@ [PcdsFeatureFlag.common]
># Define if the GICv3 controller should use the GICv2 legacy
>gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x0042
>  
> +  # Whether to implement warm reboot for capsule update using a jump back to 
> the
> +  # PEI entry point with caches and interrupts disabled.
> +  
> gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|FALSE|BOOLEAN|0x001F
> +
>  [PcdsFeatureFlag.ARM]
># Whether to map normal memory as non-shareable. FALSE is the safe choice, 
> but
># TRUE may be appropriate to fix performance problems if you don't care 
> about
> diff --git 
> a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c 
> b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> index d6d26bce5009..cb75e32771c2 100644
> --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> @@ -15,10 +15,13 @@
>  
>  #include 
>  
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
> +#include 
>  
>  #include 
>  
> @@ -89,7 +92,21 @@ EnterS3WithImmediateWake (
>VOID
>)
>  {
> -  // Not implemented
> +  VOID (*Reset)(VOID);
> +
> +  if (FeaturePcdGet (PcdArmReenterPeiForCapsuleWarmReboot) &&
> +  !EfiAtRuntime ()) {
> +//
> +// At boot time, we are the only core running, so we can implement the
> +// immediate wake (which is used by capsule update) by disabling the MMU
> +// and interrupts, and jumping to the PEI entry point.
> +//
> +Reset = (VOID *)(UINTN)FixedPcdGet64 (PcdFvBaseAddress);

ISO C99 6.3.2.3 Pointers

  1 A pointer to void may be converted to or from a pointer to any
incomplete or object type. [...]

  [...]

  5 An integer may be converted to any pointer type. Except as
previously specified, the result is implementation-defined, might
not be correctly aligned, might not point to an entity of the
referenced type, and might be a trap representation. [...]

  [...]

  8 A pointer to a function of one type may be converted to a pointer to
a function of another type and back again; [...]

The point is, converting pointer-to-void to pointer-to-function is
undefined, and I expect at least clang will yell at us for it. However,
converting an integer to pointer-to-function is implementation-defined;
i.e., the C language implementation must support it to an extent, and
must document how it works. (The "previously specified" part is about
null pointer constants, in paragraph 3, which I'm not quoting now.)

Thus, I suggest either

  typedef VOID (*RESET_FUNCTION_PTR) (VOID);
  RESET_FUNCTION_PTR Reset;

  Reset = (RESET_FUNCTION_PTR)(UINTN)FixedPcdGet64 (PcdFvBaseAddress);

or else (for kicks and giggles regarding the syntax), just

  Reset = (VOID (*)(VOID))(UINTN)FixedPcdGet64 (PcdFvBaseAddress);

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

Re: [edk2] How to Interpret ReadKeyStrokeEX Data

[Jim.Dailey]

Thanks, Mike.  I figured that out yesterday.

I think the only problem now is that any shell built with the original change 
to edit will not work as expected if it is run on a system whose BIOS was 
created before the second change was made (i.e. pretty much every existing UEFI 
BIOS in the world!).

Since the shell is a stand-alone tool that is not necessarily tied to any 
particular machine, I think that it is likely that more than a few people may 
run into this problem in the future.

Regards,
Jim

-Original Message-
From: Rothman, Michael A [mailto:michael.a.roth...@intel.com] 
Sent: Tuesday, June 5, 2018 10:38 PM
To: Rothman, Michael A; Dailey, Jim; Ni, Ruiyu
Cc: Carsey, Jaben; edk2-devel@lists.01.org; fel...@mail.ru
Subject: RE: How to Interpret ReadKeyStrokeEX Data

Jim,

I think the problem you're seeing is that the USB keyboard driver you're using 
is downrev and needs to be updated.

If you look at 
https://github.com/tianocore/edk2/commit/dd190645eb43424706eb1709d0032c69a1935d9f
 there was a fix checked in to address exactly the issue you're running into. 
It's basically a symptom of running a new shell without a correspondingly 
updated keyboard driver. The new shell in effect exposed a latent bug.

Hope that helps.

Thanks,
Mike Rothman 
(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
רועה עיקרי של חתולים

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rothman, 
Michael A
Sent: Monday, June 4, 2018 10:31 AM
To: jim.dai...@dell.com; Ni, Ruiyu 
Cc: Carsey, Jaben ; edk2-devel@lists.01.org; 
fel...@mail.ru
Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data

Since I'm largely the person who might be to blame for the language and intent 
here and I’ll focus on the spec-related item.  See my comments below.



Thanks,

Mike Rothman

(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)

רועה עיקרי של חתולים





-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
jim.dai...@dell.com
Sent: Friday, June 1, 2018 11:27 AM
To: Ni, Ruiyu 
Cc: Carsey, Jaben ; edk2-devel@lists.01.org; 
fel...@mail.ru
Subject: [edk2] How to Interpret ReadKeyStrokeEX Data



(Subject changed)



I guess this is a question of UEFI spec interpretation.  In the Console I/O 
Protocol description of Version 2.5 of the spec (page 456), it says:



If the Scan Code is set to 0x00 then the Unicode character is

valid and should be used.



To me that clearly says it all.  The shift modifier is a don't care when the 
scan code is zero.  And, this change in the shell code seems to be a violation 
of that statement.



However, I also see some confusing (and grammatically incorrect) text in the 
description of the ReadKeyStrokeEx() function of the simple text in protocol 
that I am guessing is related to this change (*emphasis* mine):



When interpreting the data from this function, it should be

noted that if a class of printable characters that are normally

*adjusted* by shift modifiers (e.g. Shift Key + "f" key) would

be presented solely as a KeyData.Key.UnicodeChar without the

associated shift state.



What I think the spec is trying to say here is that for A-Z and a-z, the 
consumer does NOT need to look at the shift state to tell "A" from "a", for 
example, because the Unicode character will be either "A" or "a" as appropriate.



n  No, it is any key that would create a displayable character that adjusts 
based on the shift state. Realize that the users of 
ReadKeyStroke/ReadKeyStrokeEx fall back to a common denominator of Scan Code or 
Unicode Character. So if someone types a shift 4, the underlying keyboard 
layout that the keyboard driver controls would dictate how that gets 
translated. On my keyboard in the US it turns into a “$” symbol, while someone 
in Europe may very well have a software-defined keyboard layout which 
translates the same keystroke to a “€” symbol. That of course applies to the 
many characters you specified (A-F, a-f) and many others.

n  The text above is intended to imply what it says, “a class of printable 
characters … would be presented solely as a KeyData.Key.UnicodeChar without the 
associated shift state. This makes consumers of both the Ex and Non-Ex variant 
of ReadKeyStroke able to use the same logic to test for any shiftable 
characters by simply looking at the Unicode value. I’d note the shift and 
toggle states (which are only available on Ex) are there not so much for 
interpreting the translated key but to maximize flexibility associated with 
keyboard mapping as a UEFI application.



I think saying this is unnecessary simply because the earlier statement (If the 
Scan Code is set to 0x00 then the Unicode character is valid and should be 
used.) covers this case.



Further, I believe this text applies only to the A-Z keys because their 
corresponding characters are *adjusted* (to upper case) when the shift key is 
pressed. That is, if 

[edk2] [patch V2 1/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

[Dandan Bi]

Make the function comments follow EDK2 coding style.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 130 ++--
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   | 223 -
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  |  27 ++-
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  39 ++--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.c |  55 +++--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.h |  59 +++---
 .../Parsers/Bgrt/BgrtParser.c  |  12 +-
 .../Parsers/Dbg2/Dbg2Parser.c  |  29 +--
 .../Parsers/Dsdt/DsdtParser.c  |   7 +-
 .../Parsers/Fadt/FadtParser.c  |  68 +--
 .../Parsers/Gtdt/GtdtParser.c  |  52 +++--
 .../Parsers/Iort/IortParser.c  | 133 +++-
 .../Parsers/Madt/MadtParser.c  |  63 +++---
 .../Parsers/Mcfg/McfgParser.c  |  18 +-
 .../Parsers/Rsdp/RsdpParser.c  |  32 +--
 .../Parsers/Slit/SlitParser.c  |  17 +-
 .../Parsers/Spcr/SpcrParser.c  |  32 +--
 .../Parsers/Srat/SratParser.c  |  61 +++---
 .../Parsers/Ssdt/SsdtParser.c  |   7 +-
 .../Parsers/Xsdt/XsdtParser.c  |  11 +-
 .../UefiShellAcpiViewCommandLib.c  |  13 +-
 .../UefiShellAcpiViewCommandLib.h  |   4 +-
 22 files changed, 645 insertions(+), 447 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 318f386fda1..088575d0144 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -1,6 +1,6 @@
-/**
+/** @file
   ACPI parser
 
   Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -19,88 +19,95 @@
 
 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
 STATIC UINT32   mTableWarningCount;
 
-/** This function resets the ACPI table error counter to Zero.
-*/
+/**
+  This function resets the ACPI table error counter to Zero.
+**/
 VOID
 ResetErrorCount (
   VOID
   )
 {
   mTableErrorCount = 0;
 }
 
-/** This function returns the ACPI table error count.
+/**
+  This function returns the ACPI table error count.
 
   @retval Returns the count of errors detected in the ACPI tables.
-*/
+**/
 UINT32
 GetErrorCount (
   VOID
   )
 {
   return mTableErrorCount;
 }
 
-/** This function resets the ACPI table warning counter to Zero.
-*/
+/**
+  This function resets the ACPI table warning counter to Zero.
+**/
 VOID
 ResetWarningCount (
   VOID
   )
 {
   mTableWarningCount = 0;
 }
 
-/** This function returns the ACPI table warning count.
+/**
+  This function returns the ACPI table warning count.
 
   @retval Returns the count of warning detected in the ACPI tables.
-*/
+**/
 UINT32
 GetWarningCount (
   VOID
   )
 {
   return mTableWarningCount;
 }
 
-/** This function increments the ACPI table error counter.
-*/
+/**
+  This function increments the ACPI table error counter.
+**/
 VOID
 EFIAPI
 IncrementErrorCount (
   VOID
   )
 {
   mTableErrorCount++;
 }
 
-/** This function increments the ACPI table warning counter.
-*/
+/**
+  This function increments the ACPI table warning counter.
+**/
 VOID
 EFIAPI
 IncrementWarningCount (
   VOID
   )
 {
   mTableWarningCount++;
 }
 
-/** This function verifies the ACPI table checksum.
+/**
+  This function verifies the ACPI table checksum.
 
   This function verifies the checksum for the ACPI table and optionally
   prints the status.
 
   @param [in] Log If TRUE log the status of the checksum.
   @param [in] Ptr Pointer to the start of the table buffer.
   @param [in] Length  The length of the buffer.
 
   @retval TRUEThe checksum is OK.
   @retval FALSE   The checksum failed.
-*/
+**/
 BOOLEAN
 EFIAPI
 VerifyChecksum (
   IN BOOLEAN Log,
   IN UINT8*  Ptr,
@@ -144,15 +151,16 @@ VerifyChecksum (
   }
 
   return (Checksum == 0);
 }
 
-/** This function performs a raw data dump of the ACPI table.
+/**
+  This function performs a raw data dump of the ACPI table.
 
   @param [in] Ptr Pointer to the start of the table buffer.
   @param [in] Length  The length of the buffer.
-*/
+**/
 VOID
 EFIAPI
 DumpRaw (
   IN UINT8* Ptr,
   IN UINT32 Length
@@ -203,64 +211,64 @@ DumpRaw (
   // Print ASCII data for the final line.
   AsciiBuffer[AsciiBufferIndex] = '\0';
   Print (L"  %a", AsciiBuffer);
 }
 
-/** This function traces 1 byte of data as specified in the
-format string.
+/**
+  This function traces 1 byte of data as specified in the 

[edk2] [patch V2 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

[Dandan Bi]

1. Separate variable definition and initialization.
2. Make the variable naming following Edk2 rule.

V2: Remove the updates of guard macros in header files.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 44 ++--
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  | 50 +--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.c | 58 ++
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.h | 10 ++--
 .../Parsers/Dbg2/Dbg2Parser.c  |  5 +-
 .../Parsers/Gtdt/GtdtParser.c  |  5 +-
 .../Parsers/Iort/IortParser.c  | 26 +-
 .../Parsers/Madt/MadtParser.c  |  4 +-
 .../Parsers/Rsdp/RsdpParser.c  | 10 +++-
 .../Parsers/Slit/SlitParser.c  | 44 
 .../Parsers/Spcr/SpcrParser.c  | 10 +++-
 .../Parsers/Srat/SratParser.c  | 21 +---
 .../UefiShellAcpiViewCommandLib.c  |  5 +-
 .../UefiShellAcpiViewCommandLib.inf|  3 ++
 14 files changed, 178 insertions(+), 117 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 088575d0144..6d3bc451acd 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -19,10 +19,19 @@
 
 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
 STATIC UINT32   mTableWarningCount;
 
+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+  An ACPI_PARSER array describing the ACPI header.
+**/
+STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
+  PARSE_ACPI_HEADER ()
+};
+
 /**
   This function resets the ACPI table error counter to Zero.
 **/
 VOID
 ResetErrorCount (
@@ -112,14 +121,17 @@ VerifyChecksum (
   IN BOOLEAN Log,
   IN UINT8*  Ptr,
   IN UINT32  Length
   )
 {
-  UINTN ByteCount = 0;
-  UINT8 Checksum = 0;
+  UINTN ByteCount;
+  UINT8 Checksum;
   UINTN OriginalAttribute;
 
+  ByteCount = 0;
+  Checksum = 0;
+
   while (ByteCount < Length) {
 Checksum += *(Ptr++);
 ByteCount++;
   }
 
@@ -164,15 +176,18 @@ EFIAPI
 DumpRaw (
   IN UINT8* Ptr,
   IN UINT32 Length
   )
 {
-  UINTN ByteCount = 0;
+  UINTN ByteCount;
   UINTN PartLineChars;
-  UINTN AsciiBufferIndex = 0;
+  UINTN AsciiBufferIndex;
   CHAR8 AsciiBuffer[17];
 
+  ByteCount = 0;
+  AsciiBufferIndex = 0;
+
   Print (L"Address  : 0x%p\n", Ptr);
   Print (L"Length   : %d\n", Length);
 
   while (ByteCount < Length) {
 if ((ByteCount & 0x0F) == 0) {
@@ -275,11 +290,14 @@ DumpUint64 (
   )
 {
   // Some fields are not aligned and this causes alignment faults
   // on ARM platforms if the compiler generates LDRD instructions.
   // Perform word access so that LDRD instructions are not generated.
-  UINT64 Val = *(UINT32*)(Ptr + sizeof (UINT32));
+  UINT64 Val;
+
+  Val = *(UINT32*)(Ptr + sizeof (UINT32));
+
   Val <<= 32;
   Val |= *(UINT32*)Ptr;
 
   Print (Format, Val);
 }
@@ -454,17 +472,20 @@ ParseAcpi (
   IN CONST ACPI_PARSER* Parser,
   IN UINT32 ParserItems
 )
 {
   UINT32  Index;
-  UINT32  Offset = 0;
+  UINT32  Offset;
+  BOOLEAN HighLight;
+
+  Offset = 0;
 
   // Increment the Indent
   gIndent += Indent;
 
   if (Trace && (AsciiName != NULL)){
-BOOLEAN HighLight = GetColourHighlighting ();
+HighLight = GetColourHighlighting ();
 UINTN   OriginalAttribute;
 
 if (HighLight) {
   OriginalAttribute = gST->ConOut->Mode->Attribute;
   gST->ConOut->SetAttribute (
@@ -618,15 +639,10 @@ UINT32
 EFIAPI
 DumpAcpiHeader (
   IN UINT8* Ptr
   )
 {
-  ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
-  ACPI_PARSER AcpiHeaderParser[] = {
-PARSE_ACPI_HEADER ()
-  };
-
   return ParseAcpi (
TRUE,
0,
"ACPI Table Header",
Ptr,
@@ -656,14 +672,10 @@ ParseAcpiHeader (
   OUT CONST UINT32** Length,
   OUT CONST UINT8**  Revision
   )
 {
   UINT32BytesParsed;
-  ACPI_DESCRIPTION_HEADER_INFO  AcpiHdrInfo;
-  ACPI_PARSER AcpiHeaderParser[] = {
-PARSE_ACPI_HEADER ()
-  };
 
   BytesParsed = ParseAcpi (
   FALSE,
   0,
   NULL,
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
index 7b1a02cad3e..fff5757bf58 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
@@ -43,36 +43,36 @@ EFIAPI
 RegisterParser (
   IN  UINT32  Signature,
   IN  PARSE_ACPI_TABLE_PROC   ParserProc
   )
 {
-  UINT32 index;
+  UINT32 Index;
 
   if ((ParserProc == NULL) || (Signature == ACPI_PARSER_SIGNATURE_NULL)) {

[edk2] [patch V2 0/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

[Dandan Bi]

ECC tool report some coding style issue in UefiShellAcpiViewCommandLib.
This patch series is to clean these issues.

V2: Remove the updates of guard macros in header files in patch 2.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 

Dandan Bi (2):
  ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues
  ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 174 +---
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   | 223 -
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  |  77 +++
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  39 ++--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.c | 113 +++
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.h |  69 ---
 .../Parsers/Bgrt/BgrtParser.c  |  12 +-
 .../Parsers/Dbg2/Dbg2Parser.c  |  34 ++--
 .../Parsers/Dsdt/DsdtParser.c  |   7 +-
 .../Parsers/Fadt/FadtParser.c  |  68 +--
 .../Parsers/Gtdt/GtdtParser.c  |  57 --
 .../Parsers/Iort/IortParser.c  | 159 ---
 .../Parsers/Madt/MadtParser.c  |  67 ---
 .../Parsers/Mcfg/McfgParser.c  |  18 +-
 .../Parsers/Rsdp/RsdpParser.c  |  42 ++--
 .../Parsers/Slit/SlitParser.c  |  61 +++---
 .../Parsers/Spcr/SpcrParser.c  |  42 ++--
 .../Parsers/Srat/SratParser.c  |  82 +---
 .../Parsers/Ssdt/SsdtParser.c  |   7 +-
 .../Parsers/Xsdt/XsdtParser.c  |  11 +-
 .../UefiShellAcpiViewCommandLib.c  |  18 +-
 .../UefiShellAcpiViewCommandLib.h  |   4 +-
 .../UefiShellAcpiViewCommandLib.inf|   3 +
 23 files changed, 823 insertions(+), 564 deletions(-)

-- 
2.14.3.windows.1

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

Re: [edk2] [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot

[Leif Lindholm]

On Wed, Jun 06, 2018 at 02:37:01PM +0200, Ard Biesheuvel wrote:
> Implement ResetSystemLib's EnterS3WithImmediateWake() routine using
> a jump back to the PEI entry point with interrupts and MMU+caches
> disabled. This is only possible at boot time, when we are sure that
> the current CPU is the only one up and running. Also, it depends on
> the platform whether the PEI code is preserved in memory (it may be
> copied to DRAM rather than execute in place), so also add a feature
> PCD to selectively enable this feature.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

It's not pretty, but it lets us start modelling desirable behaviour on
systems we alwready have. So
Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/ArmPkg.dec|  4 
> 
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 21 
> ++--
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf |  9 
> +
>  3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index debe066b6f7b..3aa229fe2ec9 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -85,6 +85,10 @@ [PcdsFeatureFlag.common]
># Define if the GICv3 controller should use the GICv2 legacy
>gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x0042
>  
> +  # Whether to implement warm reboot for capsule update using a jump back to 
> the
> +  # PEI entry point with caches and interrupts disabled.
> +  
> gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|FALSE|BOOLEAN|0x001F
> +
>  [PcdsFeatureFlag.ARM]
># Whether to map normal memory as non-shareable. FALSE is the safe choice, 
> but
># TRUE may be appropriate to fix performance problems if you don't care 
> about
> diff --git 
> a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c 
> b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> index d6d26bce5009..cb75e32771c2 100644
> --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> @@ -15,10 +15,13 @@
>  
>  #include 
>  
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
> +#include 
>  
>  #include 
>  
> @@ -89,7 +92,21 @@ EnterS3WithImmediateWake (
>VOID
>)
>  {
> -  // Not implemented
> +  VOID (*Reset)(VOID);
> +
> +  if (FeaturePcdGet (PcdArmReenterPeiForCapsuleWarmReboot) &&
> +  !EfiAtRuntime ()) {
> +//
> +// At boot time, we are the only core running, so we can implement the
> +// immediate wake (which is used by capsule update) by disabling the MMU
> +// and interrupts, and jumping to the PEI entry point.
> +//
> +Reset = (VOID *)(UINTN)FixedPcdGet64 (PcdFvBaseAddress);
> +
> +gBS->RaiseTPL (TPL_HIGH_LEVEL);
> +ArmDisableMmu ();
> +Reset ();
> +  }
>  }
>  
>  /**
> diff --git 
> a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf 
> b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> index 5a1ee976e5bc..19021cd1e8b6 100644
> --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> @@ -30,6 +30,15 @@ [Packages]
>MdePkg/MdePkg.dec
>  
>  [LibraryClasses]
> +  ArmMmuLib
>ArmSmcLib
>BaseLib
>DebugLib
> +  UefiBootServicesTableLib
> +  UefiRuntimeLib
> +
> +[FeaturePcd]
> +  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> -- 
> 2.17.0
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot

[Ard Biesheuvel]

Implement ResetSystemLib's EnterS3WithImmediateWake() routine using
a jump back to the PEI entry point with interrupts and MMU+caches
disabled. This is only possible at boot time, when we are sure that
the current CPU is the only one up and running. Also, it depends on
the platform whether the PEI code is preserved in memory (it may be
copied to DRAM rather than execute in place), so also add a feature
PCD to selectively enable this feature.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/ArmPkg.dec|  4 
 ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 21 
++--
 ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf |  9 
+
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index debe066b6f7b..3aa229fe2ec9 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -85,6 +85,10 @@ [PcdsFeatureFlag.common]
   # Define if the GICv3 controller should use the GICv2 legacy
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x0042
 
+  # Whether to implement warm reboot for capsule update using a jump back to 
the
+  # PEI entry point with caches and interrupts disabled.
+  
gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|FALSE|BOOLEAN|0x001F
+
 [PcdsFeatureFlag.ARM]
   # Whether to map normal memory as non-shareable. FALSE is the safe choice, 
but
   # TRUE may be appropriate to fix performance problems if you don't care about
diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c 
b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
index d6d26bce5009..cb75e32771c2 100644
--- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
+++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
@@ -15,10 +15,13 @@
 
 #include 
 
+#include 
+#include 
 #include 
 #include 
 #include 
-#include 
+#include 
+#include 
 
 #include 
 
@@ -89,7 +92,21 @@ EnterS3WithImmediateWake (
   VOID
   )
 {
-  // Not implemented
+  VOID (*Reset)(VOID);
+
+  if (FeaturePcdGet (PcdArmReenterPeiForCapsuleWarmReboot) &&
+  !EfiAtRuntime ()) {
+//
+// At boot time, we are the only core running, so we can implement the
+// immediate wake (which is used by capsule update) by disabling the MMU
+// and interrupts, and jumping to the PEI entry point.
+//
+Reset = (VOID *)(UINTN)FixedPcdGet64 (PcdFvBaseAddress);
+
+gBS->RaiseTPL (TPL_HIGH_LEVEL);
+ArmDisableMmu ();
+Reset ();
+  }
 }
 
 /**
diff --git 
a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf 
b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
index 5a1ee976e5bc..19021cd1e8b6 100644
--- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
+++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
@@ -30,6 +30,15 @@ [Packages]
   MdePkg/MdePkg.dec
 
 [LibraryClasses]
+  ArmMmuLib
   ArmSmcLib
   BaseLib
   DebugLib
+  UefiBootServicesTableLib
+  UefiRuntimeLib
+
+[FeaturePcd]
+  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFvBaseAddress
-- 
2.17.0

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

Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Ard Biesheuvel]

On 6 June 2018 at 11:52, Ard Biesheuvel  wrote:
> When capsule updates are staged for processing after a warm reboot,
> they are copied into memory with the MMU and caches enabled. When
> the capsule PEI gets around to coalescing the capsule, the MMU and
> caches may still be disabled, and so on architectures where uncached
> accesses are incoherent with the caches (such as ARM and AARCH64),
> we may read stale data if we don't clean the caches to memory first.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Leif asked me to include a note why this cannot be done when
UpdateCapsule() is called.


"""
Note that this cache maintenance cannot be done during the invocation
of UpdateCapsule(), since the ScatterGatherList structures are only
identified by physical address, and at runtime, the firmware doesn't
know whether and where this memory is mapped, and cache maintenance
requires a virtual address.
"""

> ---
>  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf   | 1 +
>  MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 4 
>  2 files changed, 5 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf 
> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> index c54bc21a95a8..594e110d1f8a 100644
> --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> @@ -48,6 +48,7 @@ [Packages]
>
>  [LibraryClasses]
>BaseLib
> +  CacheMaintenanceLib
>HobLib
>BaseMemoryLib
>PeiServicesLib
> diff --git a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c 
> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> index 3e7054cd38a9..1730f925adc5 100644
> --- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> +++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
> @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource (
>DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in 
> MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
>Address, Size,
>Index, MemoryResource[Index].PhysicalStart, 
> MemoryResource[Index].ResourceLength));
> +
> +  WriteBackDataCacheRange ((VOID *)(UINTN)Address, Size);
> +
>return TRUE;
>  }
>}
> --
> 2.17.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 00/17] *** Standalone Management Mode Core Interface for AARCH64 Platforms ***

[Sughosh Ganu]

On Tue, Jun 5, 2018 at 3:43 AM, Supreeth Venkatesh
 wrote:
> ***
> This patchset v3 contains only the patches that got feedback/comments frome 
> the previous revision v2.
> The patches are
> [PATCH v3 06/17] StandaloneMmPkg: Delete StandaloneMmPkg file.
> [PATCH v3 13/17] StandaloneMmPkg: Add an AArch64 specific entry point library.
> [PATCH v3 17/17] BaseTools/AutoGen: Update header file for MM modules.
>
> Changes Since v2:
> (*) Address feedback provided for the commit "BaseTools/AutoGen: Update 
> header file for MM modules."
> (*) Edit parameters for the StandaloneMmCpu Driver in the commit 
> "StandaloneMmPkg: Add an AArch64 specific entry point library."
>
> Changes Since v1:
> (*) Reorder and Reword commits.
> (*) Reorganize structure of StandaloneMmPkg and rename libraries.
> (*) Address Review Comments from Achin, Jiewen and Daniil.
> ***
> Supreeth Venkatesh (17):
>   ArmPkg: Add PCDs needed for MM communication driver.
>   ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
>   ArmPkg/Include: Add MM interface SVC return codes.
>   ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0.
>   ArmPkg/ArmMmuLib: Add MMU library inf file suitable for use in S-EL0.
>   StandaloneMmPkg: Delete StandaloneMmPkg file.
>   StandaloneMmPkg/FvLib: Add a common FV Library for management mode.
>   StandaloneMmPkg/MemLib: Add Standalone MM instance of memory check
> library.
>   StandaloneMmPkg/MemoryAllocationLib: Add MM memory allocation library.
>   StandaloneMmPkg/HobLib: Add HOB Library for management mode.
>   StandaloneMmPkg: MM driver entry point library.
>   StandaloneMmPkg/Core: Implementation of Standalone MM Core Module.
>   StandaloneMmPkg: Add an AArch64 specific entry point library.
>   StandaloneMmPkg: Add CPU driver suitable for ARM Platforms.
>   StandaloneMmPkg: Describe the declaration and definition files.
>   ArmPkg: Extra action to update permissions for S-ELO MM Image.
>   BaseTools/AutoGen: Update header file for MM modules.

Tested all changes for RAS error injection and error handling on
SGI575 platform.

Tested-by: Sughosh Ganu 

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

[edk2] [RFC PATCH edk2-platforms] Silicon/SynQuacer: add special ResetSystemLib that implements warm reboot

[Ard Biesheuvel]

This is a clone of the generic PSCI/SMC based ResetSystemLib from ArmPkg,
with an implementation of EnterS3WithImmediateWake () added that simply
disables interrupts, disables the MMU and jumps back to the PEI entry
point when called at boot time. This is sufficiently close to an
architected warm reboot to support capsules that persist across reset,
but is otherwise a bit of a hack.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.c
   | 132 
 
Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.inf
 |  41 ++
 2 files changed, 173 insertions(+)

diff --git 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.c
 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.c
new file mode 100644
index ..a4c0c18abd76
--- /dev/null
+++ 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.c
@@ -0,0 +1,132 @@
+/** @file
+  ResetSystemLib implementation for SynQuacer using PSCI calls
+
+  Copyright (c) 2018, 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 
+#include 
+#include 
+#include 
+
+#include 
+
+/**
+  This function causes a system-wide reset (cold reset), in which
+  all circuitry within the system returns to its initial state. This type of 
reset
+  is asynchronous to system operation and operates without regard to
+  cycle boundaries.
+
+  If this function returns, it means that the system does not support cold 
reset.
+**/
+VOID
+EFIAPI
+ResetCold (
+  VOID
+  )
+{
+  ARM_SMC_ARGS ArmSmcArgs;
+
+  // Send a PSCI 0.2 SYSTEM_RESET command
+  ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_RESET;
+  ArmCallSmc ();
+}
+
+/**
+  This function causes a system-wide initialization (warm reset), in which all 
processors
+  are set to their initial state. Pending cycles are not corrupted.
+
+  If this function returns, it means that the system does not support warm 
reset.
+**/
+VOID
+EFIAPI
+ResetWarm (
+  VOID
+  )
+{
+  // Map a warm reset into a cold reset
+  ResetCold ();
+}
+
+/**
+  This function causes the system to enter a power state equivalent
+  to the ACPI G2/S5 or G3 states.
+
+  If this function returns, it means that the system does not support shutdown 
reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+  VOID
+  )
+{
+  ARM_SMC_ARGS ArmSmcArgs;
+
+  // Send a PSCI 0.2 SYSTEM_OFF command
+  ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_OFF;
+  ArmCallSmc ();
+}
+
+/**
+  This function causes the system to enter S3 and then wake up immediately.
+
+  If this function returns, it means that the system does not support S3 
feature.
+**/
+VOID
+EFIAPI
+EnterS3WithImmediateWake (
+  VOID
+  )
+{
+  VOID (*Reset)(VOID);
+
+  if (!EfiAtRuntime ()) {
+//
+// At boot time, we are the only core running, so we can implement the
+// immediate wake (which is used by capsule update) by disabling the MMU
+// and interrupts, and jumping to the PEI entry point.
+//
+Reset = (VOID *)(UINTN)FixedPcdGet64 (PcdFvBaseAddress);
+
+gBS->RaiseTPL (TPL_HIGH_LEVEL);
+ArmDisableMmu ();
+Reset ();
+  }
+  ResetWarm ();
+}
+
+/**
+  This function causes a systemwide reset. The exact type of the reset is
+  defined by the EFI_GUID that follows the Null-terminated Unicode string 
passed
+  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
+  the platform must pick a supported reset type to perform.The platform may
+  optionally log the parameters from any non-normal reset that occurs.
+
+  @param[in]  DataSize   The size, in bytes, of ResetData.
+  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
+ followed by the EFI_GUID.
+**/
+VOID
+EFIAPI
+ResetPlatformSpecific (
+  IN UINTN   DataSize,
+  IN VOID*ResetData
+  )
+{
+  // Map the platform specific reset as reboot
+  ResetCold ();
+}
diff --git 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.inf
 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.inf
new file mode 100644
index ..0952f6a8f108
--- /dev/null
+++ 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerResetSystemLib/SynQuacerResetSystemLib.inf
@@ -0,0 +1,41 @@
+#/** @file
+#  ResetSystemLib implementation for SynQuacer using PSCI calls
+#
+#  Copyright (c) 2018, Linaro Ltd. All 

Re: [edk2] 4MB flash size as build default.

[Laszlo Ersek]

On 06/06/18 05:05, Prerna wrote:
> Hi Laszlo,
> I noticed that the following commit was used to make the 4MB flash layout
> the default for edk2 builds briefly:
> 
> bba8dfbec3bb OvmfPkg: make the 4MB flash size the default
> 
> However, it was later reverted :
> 
> 6e49d01cfb43 Revert "OvmfPkg: make the 4MB flash size the default"
> 
> presumably to fix this issue:
> 
> 0c79471d6a98 OvmfPkg/PlatformPei: handle non-power-of-two spare
>   size for emu variables

No, that's not exactly the reason. Please see the following sub-thread:

http://mid.mail-archive.com/bdf196e8-df87-84b9-1d6e-bbece5fa65b7@redhat.com

Briefly, the 4MB unified pflash size broke two things:
- the EmuVariableFvbRuntimeDxe driver, which would only affect QEMU
  command lines with -bios,
- the PlatformPei PEIM, which would affect QEMU command lines with
  -pflash too.

(In the linked message, I explained why I hadn't noticed even regression
#2.)

The temporary solution was to (a) restore the default to 2MB -- this
would allow people to continue using both -bios and -pflash, and (b)
kludge around the PlatformPei regression, so that the (manually
selected) 4MB build would be usable for -pflash, at the cost of wasting
a little bit of reserved memory.

In other words, the last two commits that you list have no
inter-dependency between them, instead they mitigated two aspects of the
regression.

In the longer term, I extended EmuVariableFvbRuntimeDxe (and the
interfacing code in PlatformPei) to handle the new 4MB unified size as
well. Please refer to
. This completed the
4MB support for "-bios" from the OVMF side; however it turned out that
even Xen needed fixes.

Once Xen too received the necessary patches, we re-enabled the 4MB
default size (commit 1c47fcd465a4 -- basically a re-application of
commit bba8dfbe). Please refer to
,
and the other message it references.

> Given that the above commit was also merged, why does edk2 still default to
> the 2MB flash layout for builds?

It doesn't; if you build upstream edk2, it defaults to 4MB.

We preserved the 2MB unified size in Gerd's and Fedora's OVMF builds
because the varstore structures are incompatible between the 2MB and the
4MB builds. (Please refer to section "OVMF Flash Layout" in the
"OvmfPkg/README" file.) If you have a preexistent varstore file,
originally created from the OVMF_VARS.fd template of the 2MB build, and
then you boot the same VM with the OVMF_CODE.fd binary of the 4MB build,
your VM will not boot. (It will just hang with a black screen. If you
capture the OVMF debug log, you will get some error messages, but
practically no user ever captures the debug log.)

In other words, we preserved the 2MB unified size in Fedora and in
Gerd's repo for staying compatible with the existent VMs of the users of
those distros / repos.

As one counter-example, we switched from 2MB to 4MB in RHEL-7.4. This
broke compatibility with VMs created against OVMF in RHEL-7.3, but that
was fine: in RHEL-7.3, OVMF was Tech Preview, and breaking compat with
Tech Preview packages is explicitly permitted.

Now obviously users of Fedora and Gerd's repo aren't *officially*
entitled to any kind of stability or support (which is why I don't run
Fedora for my day to day work, by the way), so we could have broken
compat there as well, right? Well, yes, if only we had wanted to field
tens or maybe hundreds of complaints on the vfio-users mailing list and
elsewhere. :) Those users most likely didn't *need* the 4MB build -- see
the motivation for the 4MB build in commit b24fca05751f --, hence
keeping compat for them was more important. Regarding direct consumers
of the upstream edk2 *source* tree, we figured they were technical
enough to deal with the change in the default size.

> Do you see any other issues too with the 4MB flash layout ?

Nope.

If you really want to use the 4MB build in Fedora (although I don't see
why you would), it's technically possible to provide the split 4MB build
in addition to the current files (under different filenames). In that
case, please file an RHBZ for the "edk2" component in Fedora (and CC
Gerd on it so he can adapt his package as well, if he intends to).

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

Re: [edk2] MdeModulePkg/Bus/Sd/EmmcDxe: Too verbose debug print on read

[Zeng, Star]

I am ok with that, you can propose patch.


Thanks,
Star
-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Tuesday, June 5, 2018 8:15 PM
To: Pipat/メタワニットポン ピパット 
Cc: edk2-devel@lists.01.org; Ni, Ruiyu ; Dong, Eric 
; Zeng, Star 
Subject: Re: [edk2] MdeModulePkg/Bus/Sd/EmmcDxe: Too verbose debug print on read

On 5 June 2018 at 13:45,   wrote:
> Hi,
>
> My team is developing a board booting Linux from an on-board eMMC, and 
> we find that EmmcReadWrite() debug print is too verbose for INFO level.
>
> // 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c#L904
> DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba ...
>
> Is this an intended level?
> I wonder if the level should be `DEBUG_BLKIO` instead.
>

I agree. I noticed the same, when loading GRUB, kernel and initrd from eMMC, a 
significant amount of time is spent in these debug prints
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

[Ard Biesheuvel]

When capsule updates are staged for processing after a warm reboot,
they are copied into memory with the MMU and caches enabled. When
the capsule PEI gets around to coalescing the capsule, the MMU and
caches may still be disabled, and so on architectures where uncached
accesses are incoherent with the caches (such as ARM and AARCH64),
we may read stale data if we don't clean the caches to memory first.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Universal/CapsulePei/CapsulePei.inf   | 1 +
 MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 4 
 2 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf 
b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
index c54bc21a95a8..594e110d1f8a 100644
--- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
@@ -48,6 +48,7 @@ [Packages]
 
 [LibraryClasses]
   BaseLib
+  CacheMaintenanceLib
   HobLib
   BaseMemoryLib
   PeiServicesLib
diff --git a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c 
b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
index 3e7054cd38a9..1730f925adc5 100644
--- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
+++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource (
   DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in MemoryResource[0x%x] 
- Start(0x%lx) Length(0x%lx)\n",
   Address, Size,
   Index, MemoryResource[Index].PhysicalStart, 
MemoryResource[Index].ResourceLength));
+
+  WriteBackDataCacheRange ((VOID *)(UINTN)Address, Size);
+
   return TRUE;
 }
   }
-- 
2.17.0

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

Re: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

[Alexei Fedorov]

Hi Dandan,


The Check Tool should be aligned with CSS, so it should be updated.


Regards.

Alexei


From: Bi, Dandan 
Sent: 06 June 2018 03:42:07
To: Alexei Fedorov; edk2-devel@lists.01.org
Cc: Carsey, Jaben; Ni, Ruiyu
Subject: RE: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC 
issues

Hi Alexei,

Current in the Edk2 implementation, the guard macros in the include header 
files start with underscore and end with underscore.  And the number of 
underscore usually used here is one or two.

And the check tool (ECC) also follow above rule to do the check. So it will 
report error for the ACPIPARSER_H_
So do you agree to keep the changes in the patch or update it to_ ACPIPARSER_H_ 
for alignment consideration firstly?

Then there is another topic, we need to make the Check Tool align with CSS, we 
may enhance the check tool or update the Spec.

Thanks,
Dandan
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Alexei 
Fedorov
Sent: Tuesday, June 05, 2018 5:13 PM
To: Bi, Dandan ; edk2-devel@lists.01.org
Cc: Carsey, Jaben ; Ni, Ruiyu 
Subject: Re: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC 
issues

Hi Dandan Bi,


Your patch contains a number of modifications like the one below:

-#ifndef ACPIPARSER_H_
-#define ACPIPARSER_H_
+#ifndef __ACPIPARSER_H__
+#define __ACPIPARSER_H__


which violate CCS

"3.3.2 Include Files"

"4.3.5.4 The names of guard macros shall end with an underscore character."

and the given examples.


Alexei


From: edk2-devel  on behalf of Dandan Bi 

Sent: 05 June 2018 09:33
To: edk2-devel@lists.01.org
Cc: Jaben Carsey; Ruiyu Ni
Subject: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

1. Separate variable definition and initialization.
2. Make the variable naming following Edk2 rule.
Naming convention of local variable:
a.First character should be upper case.
b.Must contain lower case characters.
c.No white space characters.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 44 ++--
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   |  6 +--
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  | 50 +--  
.../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  6 +--  
.../Library/UefiShellAcpiViewCommandLib/AcpiView.c | 58 ++  
.../Library/UefiShellAcpiViewCommandLib/AcpiView.h | 16 +++---
 .../Parsers/Dbg2/Dbg2Parser.c  |  5 +-
 .../Parsers/Gtdt/GtdtParser.c  |  5 +-
 .../Parsers/Iort/IortParser.c  | 26 +-
 .../Parsers/Madt/MadtParser.c  |  4 +-
 .../Parsers/Rsdp/RsdpParser.c  | 10 +++-
 .../Parsers/Slit/SlitParser.c  | 44 
 .../Parsers/Spcr/SpcrParser.c  | 10 +++-
 .../Parsers/Srat/SratParser.c  | 21 +---
 .../UefiShellAcpiViewCommandLib.c  |  5 +-
 .../UefiShellAcpiViewCommandLib.h  |  6 +--
 .../UefiShellAcpiViewCommandLib.inf|  3 ++
 17 files changed, 190 insertions(+), 129 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 088575d0144..6d3bc451acd 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -19,10 +19,19 @@

 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
 STATIC UINT32   mTableWarningCount;

+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+  An ACPI_PARSER array describing the ACPI header.
+**/
+STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
+  PARSE_ACPI_HEADER ()
+};
+
 /**
   This function resets the ACPI table error counter to Zero.
 **/
 VOID
 ResetErrorCount (
@@ -112,14 +121,17 @@ VerifyChecksum (
   IN BOOLEAN Log,
   IN UINT8*  Ptr,
   IN UINT32  Length
   )
 {
-  UINTN ByteCount = 0;
-  UINT8 Checksum = 0;
+  UINTN ByteCount;
+  UINT8 Checksum;
   UINTN OriginalAttribute;

+  ByteCount = 0;
+  Checksum = 0;
+
   while (ByteCount < Length) {
 Checksum += *(Ptr++);
 ByteCount++;
   }

@@ -164,15 +176,18 @@ EFIAPI
 DumpRaw (
   IN UINT8* Ptr,
   IN UINT32 Length
   )
 {
-  UINTN ByteCount = 0;
+  UINTN ByteCount;
   UINTN PartLineChars;
-  UINTN AsciiBufferIndex = 0;
+  UINTN AsciiBufferIndex;
   CHAR8 AsciiBuffer[17];

+  ByteCount = 0;
+  AsciiBufferIndex = 0;
+
   Print (L"Address  : 0x%p\n", Ptr);
   Print (L"Length   : %d\n", Length);

   while (ByteCount < Length) {
 if ((ByteCount & 0x0F) == 0) {
@@ -275,11 +290,14 @@ DumpUint64 (
   )
 {
   // Some fields are not aligned and this causes alignment faults
   

Re: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure FIFO Polled Mode

[Zeng, Star]

Hi Leo,

I am ok with the code change.
I was just curious about the motivation for the change.
1. No real issue met, but just to follow the doc 8.4.2 ?
2. Real issue met, then what is the issue ?
3. What is the default value of IER for your case ?

If the information are valuable, then they can be added into the commit message 
for further easy maintenance.

-Original Message-
From: Duran, Leo [mailto:leo.du...@amd.com] 
Sent: Wednesday, June 6, 2018 9:05 AM
To: Zeng, Star ; Dong, Eric 
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure 
FIFO Polled Mode

Hi Star,

I came across a 16550 model (simulation) which required clearing IER, and it 
seems that's allowed in the 16650 spec, as noted here:
http://www.ti.com/lit/ds/symlink/pc16550d.pdf

8.4.2 FIFO Polled Mode Operation
With FCR0=1 resetting IER0, IER1, IER2, IER3 or all to zero puts the UART in 
the FIFO Polled Mode of operation.

Thanks,
Leo.

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Tuesday, June 05, 2018 7:43 PM
> To: Duran, Leo ; Dong, Eric 
> Cc: edk2-devel@lists.01.org; Zeng, Star 
> Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> It will be better to have the information that may could be added into 
> the commit message.
> 
> 1. Did you meet real issue without this patch?
> 2. what is the default value of IER in your case?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Duran, Leo [mailto:leo.du...@amd.com]
> Sent: Wednesday, June 6, 2018 5:21 AM
> To: Zeng, Star ; Dong, Eric 
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Any updates on this patch?
> 
> Do you require to know my "default value of IER"?
> 
> Thanks,
> Leo.
> 
> -Original Message-
> From: edk2-devel  On Behalf Of Duran, 
> Leo
> Sent: Friday, May 25, 2018 8:38 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Don''t have access to test platform at this time.
> But will report IER value if I,m able to.
> 
> Leo
> 
> Get Outlook for iOS 
> 
> From: Zeng, Star 
> Sent: Friday, May 25, 2018 6:13:16 AM
> To: Duran, Leo; edk2-devel@lists.01.org
> Cc: Dong, Eric; Zeng, Star
> Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Reviewed-by: Star Zeng 
> 
> Just a little curious about
> 1. Did you meet real issue without this patch?
> 2. what is the default value of IER in your case?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Leo Duran
> Sent: Friday, May 25, 2018 3:08 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star 
> Subject: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Put the UART in FIFO Polled Mode by clearing IER after setting FCR.
> Also, add comments to show DLAB state for registers 0 and 1.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leo Duran 
> Cc: Star Zeng 
> CC: Eric Dong 
> ---
>  .../BaseSerialPortLib16550/BaseSerialPortLib16550.c  | 16 --
> --
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> index 0ccac96..6532c4d 100644
> ---
> a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> +++
> b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
> +++ .c
> @@ -3,6 +3,8 @@
> 
>(C) Copyright 2014 Hewlett-Packard Development Company, L.P.
>Copyright (c) 2006 - 2016, Intel Corporation. All rights 
> reserved.
> +  Copyright (c) 2018, 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 @@ -30,10 +32,11 @@  //  // 16550 UART register 
> offsets and bitfields  //
> -#define R_UART_RXBUF  0
> -#define R_UART_TXBUF  0
> -#define R_UART_BAUD_LOW   0
> -#define R_UART_BAUD_HIGH  1
> +#define R_UART_RXBUF  0   // LCR_DLAB = 0
> +#define R_UART_TXBUF  0   // LCR_DLAB = 0
> +#define R_UART_BAUD_LOW   0   // LCR_DLAB = 1
> +#define R_UART_BAUD_HIGH  1   // LCR_DLAB = 1
> +#define R_UART_IER1   // LCR_DLAB = 0
>  #define R_UART_FCR2
>  #define   B_UART_FCR_FIFOEBIT0
>  #define   B_UART_FCR_FIFO64   BIT5
> @@ -554,6 +557,11 @@ SerialPortInitialize (
>SerialPortWriteRegister 

Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

[Sivaraman Nainar]

Hello Ting,

Yes. In the use case we said:

NIC 1 - Target 1 : Login success
NIC 1 - Target 2 : Login success

NIC 2 - Target 1 : Login success

But iSCSI Driver will choose the first login session and abort the other in the 
same NIC. But even it is aborted it is Published to IBFT.

As you said the ESXi and SLES not able to proceed installation assuming more 
than one IBFT for the same NIC as illegal.

So what the clarification we are asking is since it's an aborted attempt can we 
skip adding IBFT entry for the aborted attempt.

Please refer the ticket already created for this has the proposed solution.

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

-Siva
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ye, Ting
Sent: Wednesday, June 06, 2018 2:04 PM
To: Omkar K; edk2-devel@lists.01.org
Cc: Madhan B. Santharam
Subject: Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hi Omkar,

If not MPIO, current iSCSI driver will try the configured attempts and only 
publish the successful entries in iBFT. The failed attempts will be removed.
In your case, it looks the ESXi and SLES OS treat the multiple entries on one 
NIC (different targets) are illegal. 


Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Omkar K
Sent: Tuesday, June 5, 2018 5:01 PM
To: Ye, Ting ; edk2-devel@lists.01.org
Cc: Madhan B. Santharam 
Subject: Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hello Ting,

1. We did not enable MPIO.
2. in IScsiStart(), at this point

//
// Select the first login session. Abort others.
//
if (Private->Session == NULL) {
Private->Session = Session;
BootSelected = AttemptConfigData->AttemptConfigIndex;
//
// Don't validate other attempt in multipath mode if one is success.
//
if (mPrivate->EnableMpio) {
  break;
}
} else {
IScsiSessionAbort (Session);
FreePool (Session);
}

other than one attempt per Nic, other sessions are aborted. Still, all the 
attempts are published in IBFT.
We can observe the issue when different targets are configured on one NIC where 
all the attempts are published in IBFT.
But, the issue disappeared when the aborted attempts are not published in IBFT.

Thanks,
Omkar

-Original Message-
From: Ye, Ting [mailto:ting...@intel.com] 
Sent: Monday, June 04, 2018 2:26 PM
To: Sivaraman Nainar; edk2-devel@lists.01.org
Cc: Omkar K; Madhan B. Santharam
Subject: RE: reg: ISCSI Aborted attempt entry in IBFT Table

Hi Siva,

Per design, the iSCSI multipath I/O will publish all configured attempts to 
IBFT, no matter the connection is success or fail currently.
Did you enable the MPIO when you configure the attempts? 

I am not clear what do you mean "aborted attempt".

Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Sivaraman Nainar
Sent: Thursday, May 31, 2018 8:18 PM
To: edk2-devel@lists.01.org
Cc: Omkar K ; Madhan B. Santharam 
Subject: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hello all:
Here is the issue which requires clarification.
Issue Synopsis:
When there are more than one iSCSI target configured and Ibft table published 
with the connected and aborted attempt details, all the targets are not seen in 
ESXi and SLES OS. But in Windows it can see the targets connected.
Use case:
Target 1: IP : 192.xx.xx.31 Target 2 : IP : 192.xx.xx.1
NIC 1 configured with attempts Target 1 & Target 2 NIC 2 configured with 
attempts Target 1 Connection

Login

Ibft

Block Device in UEFI Shell

SLES / Esx OS

Windows

NIC1 Target1

Success

Published

Mounted

Target Seen



NIC1 Target2

Success

Published

Mounted

Target Seen



NIC1 Target1
NIC1 Target2
NIC2 Target1

Individual Login success

Published for all attempts (3)

NIC1 Target 1 NIC2 Target 1

None Seen

NIC1 Target 1 NIC2 Target 1


When the attempts which are login success but Aborted by ISCSI Driver are 
present in ibft table SLES and ESX not able to see the targets during 
Installation.
If the ISCSI Driver not adding the ibft entry for the aborted attempts then the 
targets are seen in Esx and SLES.
So it requires clarification that If the driver need to SKIP adding the aborted 
attempt entry to ibft or its OS responsibility to handle the invalid entries in 
ibft.
-Siva
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org

Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

[Ye, Ting]

Hi Omkar,

If not MPIO, current iSCSI driver will try the configured attempts and only 
publish the successful entries in iBFT. The failed attempts will be removed.
In your case, it looks the ESXi and SLES OS treat the multiple entries on one 
NIC (different targets) are illegal. 


Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Omkar K
Sent: Tuesday, June 5, 2018 5:01 PM
To: Ye, Ting ; edk2-devel@lists.01.org
Cc: Madhan B. Santharam 
Subject: Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hello Ting,

1. We did not enable MPIO.
2. in IScsiStart(), at this point

//
// Select the first login session. Abort others.
//
if (Private->Session == NULL) {
Private->Session = Session;
BootSelected = AttemptConfigData->AttemptConfigIndex;
//
// Don't validate other attempt in multipath mode if one is success.
//
if (mPrivate->EnableMpio) {
  break;
}
} else {
IScsiSessionAbort (Session);
FreePool (Session);
}

other than one attempt per Nic, other sessions are aborted. Still, all the 
attempts are published in IBFT.
We can observe the issue when different targets are configured on one NIC where 
all the attempts are published in IBFT.
But, the issue disappeared when the aborted attempts are not published in IBFT.

Thanks,
Omkar

-Original Message-
From: Ye, Ting [mailto:ting...@intel.com] 
Sent: Monday, June 04, 2018 2:26 PM
To: Sivaraman Nainar; edk2-devel@lists.01.org
Cc: Omkar K; Madhan B. Santharam
Subject: RE: reg: ISCSI Aborted attempt entry in IBFT Table

Hi Siva,

Per design, the iSCSI multipath I/O will publish all configured attempts to 
IBFT, no matter the connection is success or fail currently.
Did you enable the MPIO when you configure the attempts? 

I am not clear what do you mean "aborted attempt".

Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Sivaraman Nainar
Sent: Thursday, May 31, 2018 8:18 PM
To: edk2-devel@lists.01.org
Cc: Omkar K ; Madhan B. Santharam 
Subject: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hello all:
Here is the issue which requires clarification.
Issue Synopsis:
When there are more than one iSCSI target configured and Ibft table published 
with the connected and aborted attempt details, all the targets are not seen in 
ESXi and SLES OS. But in Windows it can see the targets connected.
Use case:
Target 1: IP : 192.xx.xx.31 Target 2 : IP : 192.xx.xx.1
NIC 1 configured with attempts Target 1 & Target 2 NIC 2 configured with 
attempts Target 1 Connection

Login

Ibft

Block Device in UEFI Shell

SLES / Esx OS

Windows

NIC1 Target1

Success

Published

Mounted

Target Seen



NIC1 Target2

Success

Published

Mounted

Target Seen



NIC1 Target1
NIC1 Target2
NIC2 Target1

Individual Login success

Published for all attempts (3)

NIC1 Target 1 NIC2 Target 1

None Seen

NIC1 Target 1 NIC2 Target 1


When the attempts which are login success but Aborted by ISCSI Driver are 
present in ibft table SLES and ESX not able to see the targets during 
Installation.
If the ISCSI Driver not adding the ibft entry for the aborted attempts then the 
targets are seen in Esx and SLES.
So it requires clarification that If the driver need to SKIP adding the aborted 
attempt entry to ibft or its OS responsibility to handle the invalid entries in 
ibft.
-Siva
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] TPM 2.0 ACPI tableTPM2 table event log

[Lin, Derek (HPS UEFI Dev)]

Created in Bugzilla.
https://bugzilla.tianocore.org/show_bug.cgi?id=978

Thanks,
Derek

From: Yao, Jiewen [mailto:jiewen@intel.com]
Sent: Tuesday, June 5, 2018 11:14 PM
To: Lin, Derek (HPS UEFI Dev) ; Zhang, Chao B 

Cc: edk2-devel@lists.01.org; Spottswood, Jason 
Subject: RE: TPM 2.0 ACPI tableTPM2 table event log

Yes, if you want this feature, you may file a Bugzilla to track it. 
https://bugzilla.tianocore.org/

Thank you
Yao Jiewen

From: Lin, Derek (HPS UEFI Dev) [mailto:derek.l...@hpe.com]
Sent: Tuesday, June 5, 2018 2:55 AM
To: Zhang, Chao B mailto:chao.b.zh...@intel.com>>; Yao, 
Jiewen mailto:jiewen@intel.com>>
Cc: edk2-devel@lists.01.org; Spottswood, Jason 
mailto:jason.spottsw...@hpe.com>>; Lin, Derek (HPS 
UEFI Dev) mailto:derek.l...@hpe.com>>
Subject: TPM 2.0 ACPI tableTPM2 table event log

Hi Chao and Jiewen,

There are new fields in TPM2 ACPI table for getting TCG event log under OS.

https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
Page 11.
Log Area Minimum Length (LAML)
Log Area Start Address (LASA)

I remember they are not existed in older TCG ACPI spec.

Do you have plan to implement it?

Thanks,
Derek

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

[edk2] [Patch] DSC Spec: Update two typo and definition

[Yonghong Zhu]

1. Update two typo
2. Correct the  to use {} {}, this issue
was caused by flexible PCD value format extend.

Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 3_edk_ii_dsc_file_format/310_pcd_sections.md  |  4 ++--
 3_edk_ii_dsc_file_format/311_[components]_sections.md | 12 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/3_edk_ii_dsc_file_format/310_pcd_sections.md 
b/3_edk_ii_dsc_file_format/310_pcd_sections.md
index 4eaa15c..607c479 100644
--- a/3_edk_ii_dsc_file_format/310_pcd_sections.md
+++ b/3_edk_ii_dsc_file_format/310_pcd_sections.md
@@ -496,11 +496,11 @@ sections of the DSC file.
{} {}
  elif(pcddatumtype == "UINT64"):
{} {}
  else:

- ::=  [ ]
+ ::= {} {} [ ]
::= 
   ::= {} {}
  ::= []
::= 
  ::=   [ ]
@@ -711,11 +711,11 @@ sections of the DSC file.
{} {}
  elif (pcddatumtype == "UINT64"):
{} {}
  else:

- ::=  [ ]
+ ::= {} {} [ ]
::= 
   ::= {} {}
  ::= []
::= 
  ::=   [ ]
diff --git a/3_edk_ii_dsc_file_format/311_[components]_sections.md 
b/3_edk_ii_dsc_file_format/311_[components]_sections.md
index 5a3594a..7e24bac 100644
--- a/3_edk_ii_dsc_file_format/311_[components]_sections.md
+++ b/3_edk_ii_dsc_file_format/311_[components]_sections.md
@@ -1,9 +1,9 @@
 

[edk2] [PATCH] Platform: Add PL011UartClock Lib

[Udit Kumar]

[v2]
  Updated name of clock lib

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/AMD/OverdriveBoard/OverdriveBoard.dsc   | 1 +
 Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 1 +
 Platform/Hisilicon/D05/D05.dsc   | 2 ++
 Platform/Hisilicon/HiKey/HiKey.dsc   | 1 +
 Platform/Hisilicon/HiKey960/HiKey960.dsc | 1 +
 Platform/LeMaker/CelloBoard/CelloBoard.dsc   | 1 +
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 +
 Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 1 +
 Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc  | 1 +
 9 files changed, 10 insertions(+)

diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
index 5e564f6..8dc58a6 100644
--- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
+++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
@@ -106,6 +106,7 @@ DEFINE DO_CAPSULE   = FALSE
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
   # ARM PL011 UART Driver
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc 
b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index 8a43d31..3a7dad4 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -87,6 +87,7 @@
   
RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
   # ARM PL011 UART Driver
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index b6e1a9d..8396550 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -96,6 +96,7 @@
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
 
   LpcLib|Silicon/Hisilicon/Hi1610/Library/LpcLib/LpcLib.inf
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
 [LibraryClasses.common.SEC]
@@ -104,6 +105,7 @@
 
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
 [BuildOptions]
diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc 
b/Platform/Hisilicon/HiKey/HiKey.dsc
index 83dd68a..f158529 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dsc
+++ b/Platform/Hisilicon/HiKey/HiKey.dsc
@@ -46,6 +46,7 @@
   DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
 
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   
RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc 
b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 79e6875..a65c5b4 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -45,6 +45,7 @@
   DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
 
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   
RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
diff --git a/Platform/LeMaker/CelloBoard/CelloBoard.dsc 
b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
index 7cd166d..0ca027e 100644
--- a/Platform/LeMaker/CelloBoard/CelloBoard.dsc
+++ b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
@@ -102,6 +102,7 @@ DEFINE DO_FLASHER   = FALSE
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
   # ARM PL011 UART Driver
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 

[edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

[Udit Kumar]

Some platform support dynamic clocking, Which is controlled
by some jumper setting or hardware registers.
Result of that PCD PL011UartClkInHz needs to be updated for
frequency change.
This patch implements support for dynamic frequency for
PL011 uart.
This patch implements default lib, which is using Pcd.
Platform which needs dynamic clocking needs implement
PL011UartClockLib

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar 
---
 ArmPlatformPkg/ArmPlatformPkg.dec  |  1 +
 ArmPlatformPkg/Include/Library/PL011UartClockLib.h | 32 +++
 .../Library/PL011UartClockLib/PL011UartClockLib.c  | 29 +
 .../PL011UartClockLib/PL011UartClockLib.inf| 37 ++
 4 files changed, 99 insertions(+)
 create mode 100644 ArmPlatformPkg/Include/Library/PL011UartClockLib.h
 create mode 100644 ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.c
 create mode 100644 
ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
b/ArmPlatformPkg/ArmPlatformPkg.dec
index dff4598..5f67e74 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -36,6 +36,7 @@
   LcdHwLib|Include/Library/LcdHwLib.h
   LcdPlatformLib|Include/Library/LcdPlatformLib.h
   NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
+  PL011UartClockLib|Include/Library/PL011UartClockLib.h
   PL011UartLib|Include/Library/PL011UartLib.h
 
 [Guids.common]
diff --git a/ArmPlatformPkg/Include/Library/PL011UartClockLib.h 
b/ArmPlatformPkg/Include/Library/PL011UartClockLib.h
new file mode 100644
index 000..93813a0
--- /dev/null
+++ b/ArmPlatformPkg/Include/Library/PL011UartClockLib.h
@@ -0,0 +1,32 @@
+/** @file
+*
+*  Copyright 2018 NXP
+*
+*  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 __PL011CLOCKLIB_H__
+#define __PL011CLOCKLIB_H__
+
+
+/**
+  Return frequency of PL011.
+
+  By default this function returns FixedPcdGet32 (PL011UartClkInHz)
+
+  @return Return frequency of PL011
+
+**/
+UINT32
+ArmPlatformGetPL011ClockFreq (
+  VOID
+  );
+
+#endif
diff --git a/ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.c 
b/ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.c
new file mode 100644
index 000..b56af14
--- /dev/null
+++ b/ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.c
@@ -0,0 +1,29 @@
+/** @file
+*
+*  Copyright 2018 NXP
+*
+*  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 
+
+/**
+  Return clock in for PL011 Uart IP.
+**/
+UINT32
+ArmPlatformGetPL011ClockFreq (
+  VOID
+  )
+{
+  // This function needs to be implemented on platforms which supports
+  // dynamic clocking to avoid re-building of UEFI firmware for PL011
+  // clock change
+  return FixedPcdGet32 (PL011UartClkInHz);
+}
diff --git a/ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf 
b/ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
new file mode 100644
index 000..5f6f699
--- /dev/null
+++ b/ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
@@ -0,0 +1,37 @@
+#/* @file
+#  Copyright 2018 NXP
+#
+#  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= 0x0001000A
+  BASE_NAME  = PL011UartClockLib
+  FILE_GUID  = af8fef24-afbb-472a-b8b7-13101a79703c
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = PL011UartClockLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+
+[LibraryClasses]
+  ArmLib
+  DebugLib
+
+[Sources.common]
+  PL011UartClockLib.c
+
+[FixedPcd]
+  gArmPlatformTokenSpaceGuid.PL011UartClkInHz
+
-- 
1.9.1

[edk2] [PATCH v2 0/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

[Udit Kumar]

[v2]
   Incorporated review comments of v1

Udit Kumar (2):
  ArmPlatformPkg: PL011 Dynamic clock freq Support
  ArmPlatformPkg: Include PL011UartClock Lib

 ArmPlatformPkg/ArmPlatformPkg.dec  |  1 +
 ArmPlatformPkg/Include/Library/PL011UartClockLib.h | 32 +++
 .../PL011SerialPortLib/PL011SerialPortLib.c|  5 +--
 .../PL011SerialPortLib/PL011SerialPortLib.inf  |  1 +
 .../Library/PL011UartClockLib/PL011UartClockLib.c  | 29 +
 .../PL011UartClockLib/PL011UartClockLib.inf| 37 ++
 6 files changed, 103 insertions(+), 2 deletions(-)
 create mode 100644 ArmPlatformPkg/Include/Library/PL011UartClockLib.h
 create mode 100644 ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.c
 create mode 100644 
ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf

-- 
1.9.1

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

[edk2] [PATCH 2/2] ArmPlatformPkg: Include PL011UartClock Lib

[Udit Kumar]

This patch includes, PL011UartClock lib.

In case of no implemenation of this Clock Lib,
Pcd value will be used for PL011 frequency.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar 
---
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c   | 5 +++--
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c 
b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
index 6aa8063..c73e8db 100644
--- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
+++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -48,7 +49,7 @@ SerialPortInitialize (
 
   return PL011UartInitializePort (
(UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
-   FixedPcdGet32 (PL011UartClkInHz),
+   ArmPlatformGetPL011ClockFreq(),
,
,
,
@@ -156,7 +157,7 @@ SerialPortSetAttributes (
 {
   return PL011UartInitializePort (
(UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
-   FixedPcdGet32 (PL011UartClkInHz),
+   ArmPlatformGetPL011ClockFreq(),
BaudRate,
ReceiveFifoDepth,
Parity,
diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf 
b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
index 3683e06..5ce5b2f 100644
--- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
+++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
@@ -26,6 +26,7 @@
   PL011SerialPortLib.c
 
 [LibraryClasses]
+  PL011UartClockLib
   PL011UartLib
   PcdLib
 
-- 
1.9.1

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