Re: [edk2] [PATCH V2 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/04/16 23:46, Yao, Jiewen wrote:
> Ah, yes. Laszlo. You are right.
> 
> I forget to push the last update yesterday. Thank you to remind me.
> Now it is synced.

Thanks! The commit message updates and the v1->v2 differences look
good/reasonable to me (I diffed the code-level end results of the two
versions, plus I compared the commit messages pairwise). I hope to test
v2 sometime next week, and I intend to look into the S3 instability too
(I took note of Paolo's advice with the "info tlb" QEMU monitor command).

Going through the (now documented) SMRAM impact again, I realize the
platform can elect to set PcdCpuSmmStaticPageTable dynamically as well.
I'm sort of guessing that we might want to set the PCD in OVMF's
PlatformPei, based on the guest-phys address width (which we also
calculate in PlatformPei), in combination with availability of 1G
paging. The case we should likely avoid is

> A) If the system only supports 2M paging,
> When the whole memory/MMIO is 48bit, we need 1+256+256*256 pages
>   (~ 257M)

Anyway, I don't want to be too clever about this until we see a problem
(out-of-SMRAM) in practice.

Thanks!
Laszlo

> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Saturday, November 5, 2016 6:40 AM
> To: Yao, Jiewen ; edk2-de...@ml01.01.org
> Cc: Kinney, Michael D ; Tian, Feng 
> ; Fan, Jeff ; Zeng, Star 
> 
> Subject: Re: [edk2] [PATCH V2 0/6] Enable SMM page level protection.
> 
> On 11/04/16 10:30, Jiewen Yao wrote:
>>  below is V2 description 
>> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
>> 2) PiSmmCpu: Add debug info on StartupAp() fails.
>> 3) PiSmmCpu: Add ASSERT for AllocatePages().
>> 4) PiSmmCpu: Add protection detail in commit message.
>> 5) UefiCpuPkg.dsc: Add page table footprint info in commit message.
> 
> Jiewen, can you please push this series to a new branch in your repo?
> 
> I see a branch called "SmmProtection_V2", but it seems to end with an
> incomplete patch (26f482d8b611d0fcb07d3ffbf3f4468fd249767b, subject
> "pismmcpu"), so I figured I'd ask explicitly.
> 
> Thanks
> Laszlo
> 
>>  below is V1 description 
>> This series patch enables SMM page level protection.
>> Features are:
>> 1) PiSmmCore reports SMM PE image code/data information
>> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
>> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
>> and set XD for data page and RO for code page.
>> 3) PiSmmCpu enables Static Paging for X64 according to
>> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
>> is used as long as it is supported.
>> 4) PiSmmCpu sets importance data structure to be read only,
>> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
>>
>> tested platform:
>> 1) Intel internal platform (X64).
>> 2) EDKII Quark IA32
>> 3) EDKII Vlv2  X64
>> 4) EDKII OVMF IA32 and IA32X64. (with -smp 8)
>>
>> Cc: Jeff Fan >
>> Cc: Feng Tian >
>> Cc: Star Zeng >
>> Cc: Michael D Kinney 
>> >
>> Cc: Laszlo Ersek >
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao >
>>
>> Jiewen Yao (6):
>>   MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
>>   MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
>>   MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
>>   UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
>>   UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
>>   QuarkPlatformPkg/dsc: enable Smm paging protection.
>>
>>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c   |   66 +
>>  MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c| 1509 
>> 
>>  MdeModulePkg/Core/PiSmmCore/Page.c |  775 +-
>>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c|   40 +
>>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   91 ++
>>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |2 +
>>  MdeModulePkg/Core/PiSmmCore/Pool.c |   16 +
>>  MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h |   51 +
>>  MdeModulePkg/MdeModulePkg.dec  |3 +
>>  QuarkPlatformPkg/Quark.dsc |6 +
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   71 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |   67 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|   68 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |   70 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S  |  226 +--
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm|   36 +-
>>  

Re: [edk2] [PATCH v4] OvmfPkg/ResetVector: Depend on PCD values of the page tables.

[Laszlo Ersek]

On 11/04/16 23:38, Jordan Justen wrote:
> On 2016-11-04 15:27:48, Laszlo Ersek wrote:
>> On 11/04/16 14:32, Marvin Häuser wrote:
>>> Currently, the value of the page tables' address is hard-coded in the
>>> ResetVector. This patch replaces these values with a PCD dependency.
>>>
>>> A check for the size has been added to alert the developer to rewrite
>>> the ASM according to the new size, if it has been changed.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Marvin Haeuser 
>>> ---
>>>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 23 ++--
>>>  OvmfPkg/ResetVector/ResetVector.inf   |  5 +
>>>  OvmfPkg/ResetVector/ResetVector.nasmb |  7 ++
>>>  3 files changed, 24 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
>>> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> index b5a4cf8d7187..6201cad1f5dc 100644
>>> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> @@ -44,10 +44,11 @@ BITS32
>>>  SetCr3ForPageTables64:
>>>  
>>>  ;
>>> -; For OVMF, build some initial page tables at 0x80-0x806000.
>>> +; For OVMF, build some initial page tables at
>>> +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
>>>  ;
>>> -; This range should match with PcdOvmfSecPageTablesBase and
>>> -; PcdOvmfSecPageTablesSize which are declared in the FDF files.
>>> +; This range should match with PcdOvmfSecPageTablesSize which is
>>> +; declared in the FDF files.
>>>  ;
>>>  ; At the end of PEI, the pages tables will be rebuilt into a
>>>  ; more permanent location by DxeIpl.
>>> @@ -56,21 +57,21 @@ SetCr3ForPageTables64:
>>>  mov ecx, 6 * 0x1000 / 4
>>>  xor eax, eax
>>>  clearPageTablesMemoryLoop:
>>> -mov dword[ecx * 4 + 0x80 - 4], eax
>>> +mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
>>>  loopclearPageTablesMemoryLoop
>>>  
>>>  ;
>>>  ; Top level Page Directory Pointers (1 * 512GB entry)
>>>  ;
>>> -mov dword[0x80], 0x801000 + PAGE_PDP_ATTR
>>> +mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
>>>  
>>>  ;
>>>  ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
>>>  ;
>>> -mov dword[0x801000], 0x802000 + PAGE_PDP_ATTR
>>> -mov dword[0x801008], 0x803000 + PAGE_PDP_ATTR
>>> -mov dword[0x801010], 0x804000 + PAGE_PDP_ATTR
>>> -mov dword[0x801018], 0x805000 + PAGE_PDP_ATTR
>>> +mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
>>> +mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR
>>> +mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR
>>> +mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR
>>>  
>>>  ;
>>>  ; Page Table Entries (2048 * 2MB entries => 4GB)
>>> @@ -81,13 +82,13 @@ pageTableEntriesLoop:
>>>  dec eax
>>>  shl eax, 21
>>>  add eax, PAGE_2M_PDE_ATTR
>>> -mov [ecx * 8 + 0x802000 - 8], eax
>>> +mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
>>>  looppageTableEntriesLoop
>>>  
>>>  ;
>>>  ; Set CR3 now that the paging structures are available
>>>  ;
>>> -mov eax, 0x80
>>> +mov eax, PT_ADDR (0)
>>>  mov cr3, eax
>>>  
>>>  OneTimeCallRet SetCr3ForPageTables64
>>> diff --git a/OvmfPkg/ResetVector/ResetVector.inf 
>>> b/OvmfPkg/ResetVector/ResetVector.inf
>>> index 46610d243ecf..d1e5d4d9bdea 100644
>>> --- a/OvmfPkg/ResetVector/ResetVector.inf
>>> +++ b/OvmfPkg/ResetVector/ResetVector.inf
>>> @@ -29,9 +29,14 @@ [Sources]
>>>ResetVector.nasmb
>>>  
>>>  [Packages]
>>> +  OvmfPkg/OvmfPkg.dec
>>>MdePkg/MdePkg.dec
>>>UefiCpuPkg/UefiCpuPkg.dec
>>>  
>>>  [BuildOptions]
>>> *_*_IA32_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
>>> *_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
>>> +
>>> +[Pcd]
>>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
>>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>>> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb 
>>> b/OvmfPkg/ResetVector/ResetVector.nasmb
>>> index 31ac06ae4a8c..29cbad367711 100644
>>> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
>>> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
>>> @@ -53,6 +53,13 @@
>>>  %include "Ia32/SearchForSecEntry.asm"
>>>  
>>>  %ifdef ARCH_X64
>>> +  #include 
>>> +
>>> +  %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) != 0x6000)
>>> +%error "This implementation inherently depends on 
>>> PcdOvmfSecPageTablesSize"
>>> +  %endif
>>> +
>>> +  %define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + 
>>> (Offset))
>>>  %include "Ia32/Flat32ToFlat64.asm"
>>>  %include "Ia32/PageTables64.asm"
>>>  %endif
>>>
>>
>> I compared this patch to the one that I worked out / simplified from
>> your v2 patch and posted as our 

Re: [edk2] [PATCH V2 0/6] Enable SMM page level protection.

[Yao, Jiewen]

Ah, yes. Laszlo. You are right.

I forget to push the last update yesterday. Thank you to remind me.
Now it is synced.

Thank you
Yao Jiewen

From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Saturday, November 5, 2016 6:40 AM
To: Yao, Jiewen ; edk2-de...@ml01.01.org
Cc: Kinney, Michael D ; Tian, Feng 
; Fan, Jeff ; Zeng, Star 

Subject: Re: [edk2] [PATCH V2 0/6] Enable SMM page level protection.

On 11/04/16 10:30, Jiewen Yao wrote:
>  below is V2 description 
> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
> 2) PiSmmCpu: Add debug info on StartupAp() fails.
> 3) PiSmmCpu: Add ASSERT for AllocatePages().
> 4) PiSmmCpu: Add protection detail in commit message.
> 5) UefiCpuPkg.dsc: Add page table footprint info in commit message.

Jiewen, can you please push this series to a new branch in your repo?

I see a branch called "SmmProtection_V2", but it seems to end with an
incomplete patch (26f482d8b611d0fcb07d3ffbf3f4468fd249767b, subject
"pismmcpu"), so I figured I'd ask explicitly.

Thanks
Laszlo

>  below is V1 description 
> This series patch enables SMM page level protection.
> Features are:
> 1) PiSmmCore reports SMM PE image code/data information
> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
> and set XD for data page and RO for code page.
> 3) PiSmmCpu enables Static Paging for X64 according to
> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
> is used as long as it is supported.
> 4) PiSmmCpu sets importance data structure to be read only,
> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
>
> tested platform:
> 1) Intel internal platform (X64).
> 2) EDKII Quark IA32
> 3) EDKII Vlv2  X64
> 4) EDKII OVMF IA32 and IA32X64. (with -smp 8)
>
> Cc: Jeff Fan >
> Cc: Feng Tian >
> Cc: Star Zeng >
> Cc: Michael D Kinney 
> >
> Cc: Laszlo Ersek >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao >
>
> Jiewen Yao (6):
>   MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
>   MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
>   MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
>   UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
>   UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
>   QuarkPlatformPkg/dsc: enable Smm paging protection.
>
>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c   |   66 +
>  MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c| 1509 
> 
>  MdeModulePkg/Core/PiSmmCore/Page.c |  775 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c|   40 +
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   91 ++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |2 +
>  MdeModulePkg/Core/PiSmmCore/Pool.c |   16 +
>  MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h |   51 +
>  MdeModulePkg/MdeModulePkg.dec  |3 +
>  QuarkPlatformPkg/Quark.dsc |6 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   71 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |   67 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|   68 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |   70 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S  |  226 +--
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm|   36 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm   |   36 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c  |   37 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c|4 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  127 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |  142 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  156 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |5 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  871 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c |   39 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h |   15 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c|  274 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   |   51 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm |   54 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm|   61 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.S   |  250 +---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.asm |   35 +-
>  

Re: [edk2] [PATCH v4] OvmfPkg/ResetVector: Depend on PCD values of the page tables.

[Jordan Justen]

On 2016-11-04 15:27:48, Laszlo Ersek wrote:
> On 11/04/16 14:32, Marvin Häuser wrote:
> > Currently, the value of the page tables' address is hard-coded in the
> > ResetVector. This patch replaces these values with a PCD dependency.
> > 
> > A check for the size has been added to alert the developer to rewrite
> > the ASM according to the new size, if it has been changed.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Marvin Haeuser 
> > ---
> >  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 23 ++--
> >  OvmfPkg/ResetVector/ResetVector.inf   |  5 +
> >  OvmfPkg/ResetVector/ResetVector.nasmb |  7 ++
> >  3 files changed, 24 insertions(+), 11 deletions(-)
> > 
> > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> > b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > index b5a4cf8d7187..6201cad1f5dc 100644
> > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > @@ -44,10 +44,11 @@ BITS32
> >  SetCr3ForPageTables64:
> >  
> >  ;
> > -; For OVMF, build some initial page tables at 0x80-0x806000.
> > +; For OVMF, build some initial page tables at
> > +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> >  ;
> > -; This range should match with PcdOvmfSecPageTablesBase and
> > -; PcdOvmfSecPageTablesSize which are declared in the FDF files.
> > +; This range should match with PcdOvmfSecPageTablesSize which is
> > +; declared in the FDF files.
> >  ;
> >  ; At the end of PEI, the pages tables will be rebuilt into a
> >  ; more permanent location by DxeIpl.
> > @@ -56,21 +57,21 @@ SetCr3ForPageTables64:
> >  mov ecx, 6 * 0x1000 / 4
> >  xor eax, eax
> >  clearPageTablesMemoryLoop:
> > -mov dword[ecx * 4 + 0x80 - 4], eax
> > +mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
> >  loopclearPageTablesMemoryLoop
> >  
> >  ;
> >  ; Top level Page Directory Pointers (1 * 512GB entry)
> >  ;
> > -mov dword[0x80], 0x801000 + PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
> >  
> >  ;
> >  ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
> >  ;
> > -mov dword[0x801000], 0x802000 + PAGE_PDP_ATTR
> > -mov dword[0x801008], 0x803000 + PAGE_PDP_ATTR
> > -mov dword[0x801010], 0x804000 + PAGE_PDP_ATTR
> > -mov dword[0x801018], 0x805000 + PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR
> >  
> >  ;
> >  ; Page Table Entries (2048 * 2MB entries => 4GB)
> > @@ -81,13 +82,13 @@ pageTableEntriesLoop:
> >  dec eax
> >  shl eax, 21
> >  add eax, PAGE_2M_PDE_ATTR
> > -mov [ecx * 8 + 0x802000 - 8], eax
> > +mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> >  looppageTableEntriesLoop
> >  
> >  ;
> >  ; Set CR3 now that the paging structures are available
> >  ;
> > -mov eax, 0x80
> > +mov eax, PT_ADDR (0)
> >  mov cr3, eax
> >  
> >  OneTimeCallRet SetCr3ForPageTables64
> > diff --git a/OvmfPkg/ResetVector/ResetVector.inf 
> > b/OvmfPkg/ResetVector/ResetVector.inf
> > index 46610d243ecf..d1e5d4d9bdea 100644
> > --- a/OvmfPkg/ResetVector/ResetVector.inf
> > +++ b/OvmfPkg/ResetVector/ResetVector.inf
> > @@ -29,9 +29,14 @@ [Sources]
> >ResetVector.nasmb
> >  
> >  [Packages]
> > +  OvmfPkg/OvmfPkg.dec
> >MdePkg/MdePkg.dec
> >UefiCpuPkg/UefiCpuPkg.dec
> >  
> >  [BuildOptions]
> > *_*_IA32_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
> > *_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
> > +
> > +[Pcd]
> > +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
> > +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> > diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb 
> > b/OvmfPkg/ResetVector/ResetVector.nasmb
> > index 31ac06ae4a8c..29cbad367711 100644
> > --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> > +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> > @@ -53,6 +53,13 @@
> >  %include "Ia32/SearchForSecEntry.asm"
> >  
> >  %ifdef ARCH_X64
> > +  #include 
> > +
> > +  %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) != 0x6000)
> > +%error "This implementation inherently depends on 
> > PcdOvmfSecPageTablesSize"
> > +  %endif
> > +
> > +  %define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + 
> > (Offset))
> >  %include "Ia32/Flat32ToFlat64.asm"
> >  %include "Ia32/PageTables64.asm"
> >  %endif
> > 
> 
> I compared this patch to the one that I worked out / simplified from
> your v2 patch and posted as our shared v3:
> 
> 

Re: [edk2] [PATCH v4] OvmfPkg/ResetVector: Depend on PCD values of the page tables.

[Laszlo Ersek]

On 11/04/16 14:32, Marvin Häuser wrote:
> Currently, the value of the page tables' address is hard-coded in the
> ResetVector. This patch replaces these values with a PCD dependency.
> 
> A check for the size has been added to alert the developer to rewrite
> the ASM according to the new size, if it has been changed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marvin Haeuser 
> ---
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 23 ++--
>  OvmfPkg/ResetVector/ResetVector.inf   |  5 +
>  OvmfPkg/ResetVector/ResetVector.nasmb |  7 ++
>  3 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index b5a4cf8d7187..6201cad1f5dc 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -44,10 +44,11 @@ BITS32
>  SetCr3ForPageTables64:
>  
>  ;
> -; For OVMF, build some initial page tables at 0x80-0x806000.
> +; For OVMF, build some initial page tables at
> +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
>  ;
> -; This range should match with PcdOvmfSecPageTablesBase and
> -; PcdOvmfSecPageTablesSize which are declared in the FDF files.
> +; This range should match with PcdOvmfSecPageTablesSize which is
> +; declared in the FDF files.
>  ;
>  ; At the end of PEI, the pages tables will be rebuilt into a
>  ; more permanent location by DxeIpl.
> @@ -56,21 +57,21 @@ SetCr3ForPageTables64:
>  mov ecx, 6 * 0x1000 / 4
>  xor eax, eax
>  clearPageTablesMemoryLoop:
> -mov dword[ecx * 4 + 0x80 - 4], eax
> +mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
>  loopclearPageTablesMemoryLoop
>  
>  ;
>  ; Top level Page Directory Pointers (1 * 512GB entry)
>  ;
> -mov dword[0x80], 0x801000 + PAGE_PDP_ATTR
> +mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
>  
>  ;
>  ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
>  ;
> -mov dword[0x801000], 0x802000 + PAGE_PDP_ATTR
> -mov dword[0x801008], 0x803000 + PAGE_PDP_ATTR
> -mov dword[0x801010], 0x804000 + PAGE_PDP_ATTR
> -mov dword[0x801018], 0x805000 + PAGE_PDP_ATTR
> +mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
> +mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR
> +mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR
> +mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR
>  
>  ;
>  ; Page Table Entries (2048 * 2MB entries => 4GB)
> @@ -81,13 +82,13 @@ pageTableEntriesLoop:
>  dec eax
>  shl eax, 21
>  add eax, PAGE_2M_PDE_ATTR
> -mov [ecx * 8 + 0x802000 - 8], eax
> +mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
>  looppageTableEntriesLoop
>  
>  ;
>  ; Set CR3 now that the paging structures are available
>  ;
> -mov eax, 0x80
> +mov eax, PT_ADDR (0)
>  mov cr3, eax
>  
>  OneTimeCallRet SetCr3ForPageTables64
> diff --git a/OvmfPkg/ResetVector/ResetVector.inf 
> b/OvmfPkg/ResetVector/ResetVector.inf
> index 46610d243ecf..d1e5d4d9bdea 100644
> --- a/OvmfPkg/ResetVector/ResetVector.inf
> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> @@ -29,9 +29,14 @@ [Sources]
>ResetVector.nasmb
>  
>  [Packages]
> +  OvmfPkg/OvmfPkg.dec
>MdePkg/MdePkg.dec
>UefiCpuPkg/UefiCpuPkg.dec
>  
>  [BuildOptions]
> *_*_IA32_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
> *_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
> +
> +[Pcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb 
> b/OvmfPkg/ResetVector/ResetVector.nasmb
> index 31ac06ae4a8c..29cbad367711 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -53,6 +53,13 @@
>  %include "Ia32/SearchForSecEntry.asm"
>  
>  %ifdef ARCH_X64
> +  #include 
> +
> +  %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) != 0x6000)
> +%error "This implementation inherently depends on 
> PcdOvmfSecPageTablesSize"
> +  %endif
> +
> +  %define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + 
> (Offset))
>  %include "Ia32/Flat32ToFlat64.asm"
>  %include "Ia32/PageTables64.asm"
>  %endif
> 

I compared this patch to the one that I worked out / simplified from
your v2 patch and posted as our shared v3:

https://lists.01.org/pipermail/edk2-devel/2016-November/004140.html

The differences are cosmetic.

Reviewed-by: Laszlo Ersek 
Tested-by: Laszlo Ersek 

I could commit & push this now (I could even find the energy in me for
that), but I'd like Jordan to okay the patch as well, in this 

Re: [edk2] Sec and Reset vector

[Rafael Machado]

All the answers were really detailed and I agree with Andrew. These answers
are a great help when we are learning.

Thanks a lot guys. I learn a lot with this community.

Thanks and regards
Rafael

Em sex, 4 de nov de 2016 19:28, Kinney, Michael D <
michael.d.kin...@intel.com> escreveu:

> Rafael,
>
> The first instruction executed for IA32 SEC phase is typically 16-bytes
> from
> the end of the Firmware Device (FD) image generated by a build.
>
> If you look at QuarkPlatformPkg/Quark.dsc as an example build, it generates
> an 8MB  file called Quark.FD.  The reset vector is 16-bytes from the end of
> that file.  The reset vector and rest of SEC code are all in a special FFS
> file
> known at the Volume Top File(VFT).
>
> The source code for the reset vector is in the file:
>
> UefiCpuPkg/SecCore/Ia32/ResetVector.nasmb
>
> This source file contains the code that fills the last 0x40 bytes of the
> VTF
> which is also the last 0x40 bytes of Quark.FD.
>
> The build tools do some special fixups on this file, so 16-bit relative JMP
> instruction at line 79 is fixed up to jump to the symbol _ModuleEntryPoint.
>
> ResetHandler:
> nop
> nop
> ApStartup:
> ;
> ; Jmp Rel16 instruction
> ; Use machine code directly in case of the assembler
> optimization
> ; SEC entry point relative address will be fixed up by some
> build tool.
> ;
> ; Typically, SEC entry point is the function
> _ModuleEntryPoint() defined in
> ; SecEntry.asm
> ;
> DB  0e9h
> DW  -3
>
> For Quark.FD, _ModuleEntryPoint is from the PlatformSecLib library at:
>
> QuarkPlatformPkg/Library/PlatformSecLib
>
> Specifically Line l5 of the file:
>
> QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.asm
>
> _ModuleEntryEntryPoint starts in 16-bit real mode and transitions to
> 32-bit protected mode, initializes ESRAM to be used as a stack, and
> transfers control to the C function PlatformSecLibStartup() at line 220.
> The goal is to minimize the amount of assembly code and get into C code
> as quickly as possible.
>
> The PlatformSecLibStartup() function implementation is in the same
> PlatformSecLib library at line 68 of the file:
>
> QuarkPlatformPkg/Library/PlatformSecLib/PlatformSecLib.c
>
> Best regards,
>
> Mike
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Rafael Machado
> > Sent: Friday, November 4, 2016 12:34 PM
> > To: Andrew Fish 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] Sec and Reset vector
> >
> > Hi Andrew
> >
> > Maybe my question was not clear.
> > But thanks for the information you provided.
> >
> > I think we can simplify what I need based on your last comment. You told:
> >
> > *"The .com file contains the hardware real mode reset vector
> (0xFFF0).
> > So execution starts up high. The 1st far jmp you do gets you running in
> the
> > 0x000F range (the ROM is aliased to the old PC address)"*
> >
> > How could I find the .com file inside the dump of a bios (I have used
> > DediProg to get the dump)?
> > I'm hunting the first far jump the firmware has.
> >
> > Thanks
> > Rafael
> >
> > Em sex, 4 de nov de 2016 às 16:50, Andrew Fish 
> escreveu:
> >
> > > On Nov 4, 2016, at 10:48 AM, Rafael Machado <
> > > rafaelrodrigues.mach...@gmail.com> wrote:
> > >
> > > Hi everyone
> > >
> > > Thanks Andrew and Marvin for the clarification.
> > > Now things start to make sense.
> > >
> > > But I was still not able to understand were things start on a real
> binary
> > >
> > >
> > > Rafeal,
> > >
> > > I'm not sure what you are asking?
> > >
> > > The .com file contains the hardware real mode reset vector
> (0xFFF0).
> > > So execution starts up high. The 1st far jmp you do gets you running
> in the
> > > 0x000F range (the ROM is aliased to the old PC address). The .com
> file
> > > is patched with the entry point of the PE/COFF (or TE) .efi and just
> jumps
> > > into that code. At some point early in the processes the code
> transitions
> > > to protected mode and starts executing up high again (only a small
> part of
> > > the ROM is aliased down low).
> > >
> > > The SEC PE/COFF  (or TE) .efi image entry point can be found in the
> > > PE/COFF (or TE) header that is part of that image. This is how the PEI
> Core
> > > passes control to PEIMs, and it is also what the PEI/DXE/SMM cores use
> to
> > > pass control to images that are loaded into memory.
> > >
> > > There is a library that given a PE/COFF or TE image will return the
> entry
> > > point.
> > >
> > >
> > >
> >
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffGetEntryPointL
> > ib/PeCoffGetEntryPoint.c#L44
> > > RETURN_STATUS
> > > EFIAPI
> > > PeCoffLoaderGetEntryPoint (
> > > IN VOID *Pe32Data,
> > > OUT VOID **EntryPoint
> > > )
> > > {
> > 

Re: [edk2] Sec and Reset vector

[Kinney, Michael D]

Rafael,

The first instruction executed for IA32 SEC phase is typically 16-bytes from
the end of the Firmware Device (FD) image generated by a build.

If you look at QuarkPlatformPkg/Quark.dsc as an example build, it generates 
an 8MB  file called Quark.FD.  The reset vector is 16-bytes from the end of
that file.  The reset vector and rest of SEC code are all in a special FFS file
known at the Volume Top File(VFT).  

The source code for the reset vector is in the file:

UefiCpuPkg/SecCore/Ia32/ResetVector.nasmb

This source file contains the code that fills the last 0x40 bytes of the VTF
which is also the last 0x40 bytes of Quark.FD.

The build tools do some special fixups on this file, so 16-bit relative JMP
instruction at line 79 is fixed up to jump to the symbol _ModuleEntryPoint.

ResetHandler:
nop
nop
ApStartup:
;
; Jmp Rel16 instruction
; Use machine code directly in case of the assembler optimization
; SEC entry point relative address will be fixed up by some build 
tool.
;
; Typically, SEC entry point is the function _ModuleEntryPoint() 
defined in
; SecEntry.asm
;
DB  0e9h
DW  -3

For Quark.FD, _ModuleEntryPoint is from the PlatformSecLib library at:

QuarkPlatformPkg/Library/PlatformSecLib

Specifically Line l5 of the file:

QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.asm

_ModuleEntryEntryPoint starts in 16-bit real mode and transitions to 
32-bit protected mode, initializes ESRAM to be used as a stack, and
transfers control to the C function PlatformSecLibStartup() at line 220.
The goal is to minimize the amount of assembly code and get into C code
as quickly as possible.

The PlatformSecLibStartup() function implementation is in the same 
PlatformSecLib library at line 68 of the file:

QuarkPlatformPkg/Library/PlatformSecLib/PlatformSecLib.c

Best regards,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rafael 
> Machado
> Sent: Friday, November 4, 2016 12:34 PM
> To: Andrew Fish 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Sec and Reset vector
> 
> Hi Andrew
> 
> Maybe my question was not clear.
> But thanks for the information you provided.
> 
> I think we can simplify what I need based on your last comment. You told:
> 
> *"The .com file contains the hardware real mode reset vector (0xFFF0).
> So execution starts up high. The 1st far jmp you do gets you running in the
> 0x000F range (the ROM is aliased to the old PC address)"*
> 
> How could I find the .com file inside the dump of a bios (I have used
> DediProg to get the dump)?
> I'm hunting the first far jump the firmware has.
> 
> Thanks
> Rafael
> 
> Em sex, 4 de nov de 2016 às 16:50, Andrew Fish  escreveu:
> 
> > On Nov 4, 2016, at 10:48 AM, Rafael Machado <
> > rafaelrodrigues.mach...@gmail.com> wrote:
> >
> > Hi everyone
> >
> > Thanks Andrew and Marvin for the clarification.
> > Now things start to make sense.
> >
> > But I was still not able to understand were things start on a real binary
> >
> >
> > Rafeal,
> >
> > I'm not sure what you are asking?
> >
> > The .com file contains the hardware real mode reset vector (0xFFF0).
> > So execution starts up high. The 1st far jmp you do gets you running in the
> > 0x000F range (the ROM is aliased to the old PC address). The .com file
> > is patched with the entry point of the PE/COFF (or TE) .efi and just jumps
> > into that code. At some point early in the processes the code transitions
> > to protected mode and starts executing up high again (only a small part of
> > the ROM is aliased down low).
> >
> > The SEC PE/COFF  (or TE) .efi image entry point can be found in the
> > PE/COFF (or TE) header that is part of that image. This is how the PEI Core
> > passes control to PEIMs, and it is also what the PEI/DXE/SMM cores use to
> > pass control to images that are loaded into memory.
> >
> > There is a library that given a PE/COFF or TE image will return the entry
> > point.
> >
> >
> >
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffGetEntryPointL
> ib/PeCoffGetEntryPoint.c#L44
> > RETURN_STATUS
> > EFIAPI
> > PeCoffLoaderGetEntryPoint (
> > IN VOID *Pe32Data,
> > OUT VOID **EntryPoint
> > )
> > {
> > EFI_IMAGE_DOS_HEADER *DosHdr;
> > EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
> > ASSERT (Pe32Data != NULL);
> > ASSERT (EntryPoint != NULL);
> > DosHdr = (EFI_IMAGE_DOS_HEADER *)Pe32Data;
> > if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
> > //
> > // DOS image header is present, so read the PE header after the DOS image
> > header.
> > //
> > Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN) Pe32Data + (UINTN)
> > ((DosHdr->e_lfanew) & 0x0));
> > } else {
> > //
> > // DOS image header is not present, so PE header is at the image base.
> > 

Re: [edk2] Sec and Reset vector

[Andrew Fish]

> On Nov 4, 2016, at 12:59 PM, Laszlo Ersek  wrote:
> 
> On 11/04/16 20:33, Rafael Machado wrote:
>> Hi Andrew
>> 
>> Maybe my question was not clear.
>> But thanks for the information you provided.
>> 
>> I think we can simplify what I need based on your last comment. You told:
>> 
>> *"The .com file contains the hardware real mode reset vector (0xFFF0).
>> So execution starts up high. The 1st far jmp you do gets you running in the
>> 0x000F range (the ROM is aliased to the old PC address)"*
>> 
>> How could I find the .com file inside the dump of a bios (I have used
>> DediProg to get the dump)?

Rafael,

Just dump the FV. The edk2 has the VolInfo command and you can pass --offset 
OFFSET.  There are other fancier GUI tools around, but I've never used them.  
There is a signature in the FV Header that you should be able to find and use 
to figure out the start of the FVs in your extracted FD.

The FV with the SEC will look something like:

FV Header
FV Block Map

FFS File 1
FFS File 2
.
FFS File N-1
Pad File
FFS File N


The FFS File N will have the VolumeTop GUID for the FFS filename. 

An FFS file looks like:
FFS Header
Section 1 Header
Section 1 Data
Section 2 Header
Section 2 Data

In our case Section 1 is the SEC .efi and Section 2 is the .com file. 

The FV layout is defined in the PI Specs and all the definitions are in the 
MdePkg. 


>> I'm hunting the first far jump the firmware has.
> 

Well the 1st instruction is real mode and 16 bytes from the end of the ROM. You 
can start disassembling there and follow the code? While the code is still in 
real mode you can assume the last 64K of the ROM image is mapped to 0x000F, 
when you transition to protected mode then that last 64K is going to map to 
0x, also it could be bigger than 64K. 

> (I have zero context, but that shan't prevent me from butting in:)
> 

Likewise! I'm sure some of those playing at home have a better understanding of 
how the SEC is constructed after this thread so a good answer to the wrong 
question does not always go unused. 

Thanks,

Andrew Fish

> it's called VTF (volume top file), and (I think) it should be at the
> very end of the stuff that you dumped from the flash chip. That's how
> the relevant contents show up just below 4GB, I presume.
> 





> Laszlo
> 
>> 
>> Thanks
>> Rafael
>> 
>> Em sex, 4 de nov de 2016 às 16:50, Andrew Fish  escreveu:
>> 
>>> On Nov 4, 2016, at 10:48 AM, Rafael Machado <
>>> rafaelrodrigues.mach...@gmail.com> wrote:
>>> 
>>> Hi everyone
>>> 
>>> Thanks Andrew and Marvin for the clarification.
>>> Now things start to make sense.
>>> 
>>> But I was still not able to understand were things start on a real binary
>>> 
>>> 
>>> Rafeal,
>>> 
>>> I'm not sure what you are asking?
>>> 
>>> The .com file contains the hardware real mode reset vector (0xFFF0).
>>> So execution starts up high. The 1st far jmp you do gets you running in the
>>> 0x000F range (the ROM is aliased to the old PC address). The .com file
>>> is patched with the entry point of the PE/COFF (or TE) .efi and just jumps
>>> into that code. At some point early in the processes the code transitions
>>> to protected mode and starts executing up high again (only a small part of
>>> the ROM is aliased down low).
>>> 
>>> The SEC PE/COFF  (or TE) .efi image entry point can be found in the
>>> PE/COFF (or TE) header that is part of that image. This is how the PEI Core
>>> passes control to PEIMs, and it is also what the PEI/DXE/SMM cores use to
>>> pass control to images that are loaded into memory.
>>> 
>>> There is a library that given a PE/COFF or TE image will return the entry
>>> point.
>>> 
>>> 
>>> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffGetEntryPointLib/PeCoffGetEntryPoint.c#L44
>>> RETURN_STATUS
>>> EFIAPI
>>> PeCoffLoaderGetEntryPoint (
>>> IN VOID *Pe32Data,
>>> OUT VOID **EntryPoint
>>> )
>>> {
>>> EFI_IMAGE_DOS_HEADER *DosHdr;
>>> EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
>>> ASSERT (Pe32Data != NULL);
>>> ASSERT (EntryPoint != NULL);
>>> DosHdr = (EFI_IMAGE_DOS_HEADER *)Pe32Data;
>>> if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
>>> //
>>> // DOS image header is present, so read the PE header after the DOS image
>>> header.
>>> //
>>> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN) Pe32Data + (UINTN)
>>> ((DosHdr->e_lfanew) & 0x0));
>>> } else {
>>> //
>>> // DOS image header is not present, so PE header is at the image base.
>>> //
>>> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)Pe32Data;
>>> }
>>> //
>>> // Calculate the entry point relative to the start of the image.
>>> // AddressOfEntryPoint is common for PE32 & PE32+
>>> //
>>> if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
>>> *EntryPoint = (VOID *)((UINTN)Pe32Data + (UINTN)(Hdr.Te->AddressOfEntryPoint
>>> & 0x0) + sizeof(EFI_TE_IMAGE_HEADER) - Hdr.Te->StrippedSize);
>>> return RETURN_SUCCESS;
>>> } else if (Hdr.Pe32->Signature == 

Re: [edk2] [PATCH 2/2] ShellPkg/Ifconfig: Enable setting MAC address

[Carsey, Jaben]

I think this is a good idea. But I see 2 issues.  
1 - ifconfig standard parameters are controlled by the UEFI Shell 
Specification.  If you want to extend the parameters you need to use parameters 
that start with underscore.  I do not quite know how to so this under the "-s" 
set of functions for this command.  Maybe just add this separate from "-s", 
using something like "-_mac"?
2 - whatever change is done here should also be done to the ipv6 version of the 
command.

-Jaben


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marcin Wojtas
> Sent: Friday, November 04, 2016 11:29 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng ; leif.lindh...@linaro.org; Gao, Liming
> ; Kinney, Michael D 
> Subject: [edk2] [PATCH 2/2] ShellPkg/Ifconfig: Enable setting MAC address
> Importance: High
> 
> This adds new feature to ifconfig shell command, which allow
> for updating MAC address of the interface or resetting it to the
> initial value.
> 
> It consumes newly added NetLib helpers for parsing the Unicode
> string to EFI_MAC_ADDRESS, calling Snp->StationAddress() callback
> and reconnecting controller, so that all interface's children
> devices could get updated with new value of the MAC address.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marcin Wojtas 
> ---
>  .../UefiShellNetwork1CommandsLib/Ifconfig.c| 54
> ++
>  .../UefiShellNetwork1CommandsLib.uni   | 12 -
>  2 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> index 5e243d5..e4ea308 100644
> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> @@ -120,6 +120,12 @@ VAR_CHECK_ITEM  mSetCheckList[] = {
>  FlagTypeSingle
>},
>{
> +L"mac",
> +0x0010,
> +0x0005,
> +FlagTypeSingle
> +  },
> +  {
>  NULL,
>  0x0,
>  0x0,
> @@ -832,6 +838,7 @@ IfConfigSetInterfaceInfo (
>EFI_EVENTTimeOutEvt;
>EFI_EVENTMappedEvt;
>BOOLEAN  IsAddressOk;
> +  BOOLEAN  MacAddressReset;
> 
>EFI_IP4_CONFIG2_POLICY   Policy;
>EFI_IP4_CONFIG2_MANUAL_ADDRESS   ManualAddress;
> @@ -840,6 +847,7 @@ IfConfigSetInterfaceInfo (
>EFI_IPv4_ADDRESS *Dns;
>ARG_LIST *Tmp;
>UINTNIndex;
> +  EFI_MAC_ADDRESS  NewMac;
> 
>CONST CHAR16* TempString;
> 
> @@ -1150,6 +1158,52 @@ IfConfigSetInterfaceInfo (
>  ShellStatus = SHELL_ACCESS_DENIED;
>  goto ON_EXIT;
>}
> +} else if (StrCmp (VarArg->Arg, L"mac") == 0) {
> +  //
> +  // Set MAC address.
> +  //
> +  MacAddressReset = FALSE;
> +  VarArg = VarArg->Next;
> +  if (VarArg == NULL) {
> +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_LACK_COMMAND), gShellNetwork1HiiHandle);
> +ShellStatus = SHELL_INVALID_PARAMETER;
> +goto ON_EXIT;
> +  }
> +
> +  if (StrCmp (VarArg->Arg, L"reset") == 0) {
> +MacAddressReset = TRUE;
> +  } else {
> +Status = NetLibStrToMac (VarArg->Arg, );
> +if (EFI_ERROR(Status)) {
> +  ShellPrintHiiEx(-1, -1, NULL,STRING_TOKEN
> (STR_IFCONFIG_INVALID_MACADDRESS), gShellNetwork1HiiHandle, VarArg-
> >Arg);
> +  ShellStatus = SHELL_INVALID_PARAMETER;
> +  goto ON_EXIT;
> +}
> +  }
> +
> +  Status = NetLibSetMacAddress (
> +  IfCb->NicHandle,
> +  ,
> +  IfCb->IfInfo->HwAddressSize,
> +  MacAddressReset
> +  );
> +  if (EFI_ERROR(Status)) {
> +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_SET_ADDR_FAILED), gShellNetwork1HiiHandle, Status);
> +ShellStatus = SHELL_ACCESS_DENIED;
> +goto ON_EXIT;
> +  }
> +
> +  //
> +  // Reconnect controller, so that all children get updated with
> +  // new MAC address information.
> +  //
> +  Status = NetLibReconnectInterface (IfCb->NicHandle);
> +  if (EFI_ERROR(Status)) {
> +ShellStatus = SHELL_DEVICE_ERROR;
> +goto ON_EXIT;
> +  }
> +
> +  VarArg = VarArg->Next;
>  }
>}
> 
> diff --git
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.uni
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.uni
> index 76b6188..ffc358d 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.uni
> +++
> 

Re: [edk2] [RFC] [MdePkg] UefiLib: CreatePopUp

[Felix Poludov]

Jiewen,

You suggestion is also an option, but I was hoping to achieve the goal without 
changing all the consumers.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, 
Jiewen
Sent: Thursday, November 03, 2016 7:25 PM
To: Dong, Eric; Felix Poludov; edk2-devel@lists.01.org
Cc: Bi, Dandan; Gao, Liming
Subject: Re: [edk2] [RFC] [MdePkg] UefiLib: CreatePopUp

Hi
I have another idea to resolve the MdePkg dependency issue.

We can create a new UiLib in MdeModulePkg with interface such as: 
UiLibCreatePopUp().
Then we update all the *caller* to use the new interface. I did a search and 
find caller below:

We can add DEPRECATE flag for the UefiLib:CreatePopUp(), to avoid it being used 
any more.

C:\home\Edk-II\MdeModulePkg\Application\UiApp\FrontPage.c(1156):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , StringBuffer1, 
StringBuffer2, NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Disk\RamDiskDxe\RamDiskImpl.c(324): 
   CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\Disk\RamDiskDxe\RamDiskImpl.c(346): 
 CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\Disk\RamDiskDxe\RamDiskImpl.c(378): 
 CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\Disk\RamDiskDxe\RamDiskImpl.c(404): 
   CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\Disk\RamDiskDxe\RamDiskImpl.c(431): 
 CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\DriverSampleDxe\DriverSample.c(1407):   
   CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\DriverSampleDxe\DriverSample.c(1830):   
 CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\DriverSampleDxe\DriverSample.c(1872):   
 CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\DriverSampleDxe\DriverSample.c(1901):   
   CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(613): 
 CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid IP 
address!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(619): 
 CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Subnet 
Mask!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(625): 
 CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Gateway!", 
NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(634): 
 CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Dns 
Server!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(641): 
   CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Dns 
Server!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(1150):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid IP 
address!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(1158):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Subnet 
Mask!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(1166):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid 
Gateway!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(1177):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Dns 
Server!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\Ip4Dxe\Ip4Config2Nv.c(1184):
  CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Dns 
Server!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(101):
CreatePopUp (
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(721):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid iSCSI 
Name!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(731):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid IP 
address!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(743):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid Subnet 
Mask!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(755):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid 
Gateway!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(767):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid IP 
address!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(779):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid iSCSI 
Name!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(797):
CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Invalid LUN 
string!", NULL);
  C:\home\Edk-II\MdeModulePkg\Universal\Network\IScsiDxe\IScsiConfig.c(855):
  CreatePopUp (EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE, , L"Gateway 
address is set but subnet mask is zero.", NULL);
  

Re: [edk2] Sec and Reset vector

[Laszlo Ersek]

On 11/04/16 20:33, Rafael Machado wrote:
> Hi Andrew
> 
> Maybe my question was not clear.
> But thanks for the information you provided.
> 
> I think we can simplify what I need based on your last comment. You told:
> 
> *"The .com file contains the hardware real mode reset vector (0xFFF0).
> So execution starts up high. The 1st far jmp you do gets you running in the
> 0x000F range (the ROM is aliased to the old PC address)"*
> 
> How could I find the .com file inside the dump of a bios (I have used
> DediProg to get the dump)?
> I'm hunting the first far jump the firmware has.

(I have zero context, but that shan't prevent me from butting in:)

it's called VTF (volume top file), and (I think) it should be at the
very end of the stuff that you dumped from the flash chip. That's how
the relevant contents show up just below 4GB, I presume.

Laszlo

> 
> Thanks
> Rafael
> 
> Em sex, 4 de nov de 2016 às 16:50, Andrew Fish  escreveu:
> 
>> On Nov 4, 2016, at 10:48 AM, Rafael Machado <
>> rafaelrodrigues.mach...@gmail.com> wrote:
>>
>> Hi everyone
>>
>> Thanks Andrew and Marvin for the clarification.
>> Now things start to make sense.
>>
>> But I was still not able to understand were things start on a real binary
>>
>>
>> Rafeal,
>>
>> I'm not sure what you are asking?
>>
>> The .com file contains the hardware real mode reset vector (0xFFF0).
>> So execution starts up high. The 1st far jmp you do gets you running in the
>> 0x000F range (the ROM is aliased to the old PC address). The .com file
>> is patched with the entry point of the PE/COFF (or TE) .efi and just jumps
>> into that code. At some point early in the processes the code transitions
>> to protected mode and starts executing up high again (only a small part of
>> the ROM is aliased down low).
>>
>> The SEC PE/COFF  (or TE) .efi image entry point can be found in the
>> PE/COFF (or TE) header that is part of that image. This is how the PEI Core
>> passes control to PEIMs, and it is also what the PEI/DXE/SMM cores use to
>> pass control to images that are loaded into memory.
>>
>> There is a library that given a PE/COFF or TE image will return the entry
>> point.
>>
>>
>> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffGetEntryPointLib/PeCoffGetEntryPoint.c#L44
>> RETURN_STATUS
>> EFIAPI
>> PeCoffLoaderGetEntryPoint (
>> IN VOID *Pe32Data,
>> OUT VOID **EntryPoint
>> )
>> {
>> EFI_IMAGE_DOS_HEADER *DosHdr;
>> EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
>> ASSERT (Pe32Data != NULL);
>> ASSERT (EntryPoint != NULL);
>> DosHdr = (EFI_IMAGE_DOS_HEADER *)Pe32Data;
>> if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
>> //
>> // DOS image header is present, so read the PE header after the DOS image
>> header.
>> //
>> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN) Pe32Data + (UINTN)
>> ((DosHdr->e_lfanew) & 0x0));
>> } else {
>> //
>> // DOS image header is not present, so PE header is at the image base.
>> //
>> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)Pe32Data;
>> }
>> //
>> // Calculate the entry point relative to the start of the image.
>> // AddressOfEntryPoint is common for PE32 & PE32+
>> //
>> if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
>> *EntryPoint = (VOID *)((UINTN)Pe32Data + (UINTN)(Hdr.Te->AddressOfEntryPoint
>> & 0x0) + sizeof(EFI_TE_IMAGE_HEADER) - Hdr.Te->StrippedSize);
>> return RETURN_SUCCESS;
>> } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
>> *EntryPoint = (VOID *)((UINTN)Pe32Data + (UINTN)(Hdr.Pe32->OptionalHeader.
>> AddressOfEntryPoint & 0x0));
>> return RETURN_SUCCESS;
>> }
>> return RETURN_UNSUPPORTED;
>> }
>>
>>
>> Thanks,
>>
>> Andrew Fish
>>
>> On the attached image ResetVectorCoreboot.png we have the entry point on a
>> coreboot image.
>> What I would like to do is to find something similar to this on a UEFI Bios
>> image.
>>
>> Based on Marvin's idea, I got the UEFI Tool and star to check the image I
>> have.
>> On the attached image TopFileInfo.png we can see to Top File mentioned by
>> Andrew.
>> The details about the Top File are these:
>>
>>  Offset: FF8018h
>>
>>  File GUID: 1BA0062E-C779-4582-8566-336AE8F78F09
>>
>>  Type: 03h
>>
>>  Attributes: 08h
>>
>>  Full size: 7FE8h (32744)
>>
>>  Header size: 18h (24)
>>
>>  Body size: 7FD0h (32720)
>>
>>  Tail size: 0h (0)
>>
>>  State: F8h
>>
>>  Header checksum: 99h, valid
>>
>>  Data checksum: AAh, valid
>>
>>  Header memory address: 8018h
>>
>>  Data memory address: 8030h
>>
>>  Compressed: No
>>
>>  Fixed: No
>>
>>
>> I tried to find something similar to what I see at the coreboot image, but
>> didn't find anything. Neither on the PE Image section, not on the Raw image
>> sections.
>>
>>
>> Any idea about how could I find the entry point of sec in this case?
>>
>> Thanks and Regards
>> Rafael R. Machado
>>
>>
>> Em sáb, 22 de out de 2016 às 16:19, Andrew Fish 
>> escreveu:
>>
>> On Oct 22, 2016, at 10:03 AM, Marvin H?user 
>> 

Re: [edk2] Sec and Reset vector

[Rafael Machado]

Hi Andrew

Maybe my question was not clear.
But thanks for the information you provided.

I think we can simplify what I need based on your last comment. You told:

*"The .com file contains the hardware real mode reset vector (0xFFF0).
So execution starts up high. The 1st far jmp you do gets you running in the
0x000F range (the ROM is aliased to the old PC address)"*

How could I find the .com file inside the dump of a bios (I have used
DediProg to get the dump)?
I'm hunting the first far jump the firmware has.

Thanks
Rafael

Em sex, 4 de nov de 2016 às 16:50, Andrew Fish  escreveu:

> On Nov 4, 2016, at 10:48 AM, Rafael Machado <
> rafaelrodrigues.mach...@gmail.com> wrote:
>
> Hi everyone
>
> Thanks Andrew and Marvin for the clarification.
> Now things start to make sense.
>
> But I was still not able to understand were things start on a real binary
>
>
> Rafeal,
>
> I'm not sure what you are asking?
>
> The .com file contains the hardware real mode reset vector (0xFFF0).
> So execution starts up high. The 1st far jmp you do gets you running in the
> 0x000F range (the ROM is aliased to the old PC address). The .com file
> is patched with the entry point of the PE/COFF (or TE) .efi and just jumps
> into that code. At some point early in the processes the code transitions
> to protected mode and starts executing up high again (only a small part of
> the ROM is aliased down low).
>
> The SEC PE/COFF  (or TE) .efi image entry point can be found in the
> PE/COFF (or TE) header that is part of that image. This is how the PEI Core
> passes control to PEIMs, and it is also what the PEI/DXE/SMM cores use to
> pass control to images that are loaded into memory.
>
> There is a library that given a PE/COFF or TE image will return the entry
> point.
>
>
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffGetEntryPointLib/PeCoffGetEntryPoint.c#L44
> RETURN_STATUS
> EFIAPI
> PeCoffLoaderGetEntryPoint (
> IN VOID *Pe32Data,
> OUT VOID **EntryPoint
> )
> {
> EFI_IMAGE_DOS_HEADER *DosHdr;
> EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
> ASSERT (Pe32Data != NULL);
> ASSERT (EntryPoint != NULL);
> DosHdr = (EFI_IMAGE_DOS_HEADER *)Pe32Data;
> if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
> //
> // DOS image header is present, so read the PE header after the DOS image
> header.
> //
> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN) Pe32Data + (UINTN)
> ((DosHdr->e_lfanew) & 0x0));
> } else {
> //
> // DOS image header is not present, so PE header is at the image base.
> //
> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)Pe32Data;
> }
> //
> // Calculate the entry point relative to the start of the image.
> // AddressOfEntryPoint is common for PE32 & PE32+
> //
> if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
> *EntryPoint = (VOID *)((UINTN)Pe32Data + (UINTN)(Hdr.Te->AddressOfEntryPoint
> & 0x0) + sizeof(EFI_TE_IMAGE_HEADER) - Hdr.Te->StrippedSize);
> return RETURN_SUCCESS;
> } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
> *EntryPoint = (VOID *)((UINTN)Pe32Data + (UINTN)(Hdr.Pe32->OptionalHeader.
> AddressOfEntryPoint & 0x0));
> return RETURN_SUCCESS;
> }
> return RETURN_UNSUPPORTED;
> }
>
>
> Thanks,
>
> Andrew Fish
>
> On the attached image ResetVectorCoreboot.png we have the entry point on a
> coreboot image.
> What I would like to do is to find something similar to this on a UEFI Bios
> image.
>
> Based on Marvin's idea, I got the UEFI Tool and star to check the image I
> have.
> On the attached image TopFileInfo.png we can see to Top File mentioned by
> Andrew.
> The details about the Top File are these:
>
>  Offset: FF8018h
>
>  File GUID: 1BA0062E-C779-4582-8566-336AE8F78F09
>
>  Type: 03h
>
>  Attributes: 08h
>
>  Full size: 7FE8h (32744)
>
>  Header size: 18h (24)
>
>  Body size: 7FD0h (32720)
>
>  Tail size: 0h (0)
>
>  State: F8h
>
>  Header checksum: 99h, valid
>
>  Data checksum: AAh, valid
>
>  Header memory address: 8018h
>
>  Data memory address: 8030h
>
>  Compressed: No
>
>  Fixed: No
>
>
> I tried to find something similar to what I see at the coreboot image, but
> didn't find anything. Neither on the PE Image section, not on the Raw image
> sections.
>
>
> Any idea about how could I find the entry point of sec in this case?
>
> Thanks and Regards
> Rafael R. Machado
>
>
> Em sáb, 22 de out de 2016 às 16:19, Andrew Fish 
> escreveu:
>
> On Oct 22, 2016, at 10:03 AM, Marvin H?user 
> wrote:
>
> Hey Rafael,
>
> There actually is some generic SEC code in UefiCpuPkg you might want to
> take a look at. It's generic because it does not have "Intel NDA" code,
> such as CAR (Cache-As-RAM) etc.
> The Reset Vector may or may not be part of SecCore. It's either embedded
> within the SecCore module, or a separate file in the FFS. You can check the
> start/end address of the modules (e.g. with UEFITool) and find the Reset
> Vector file that way.
>
>
> Rafael,
>
> There is some 

Re: [edk2] Sec and Reset vector

[Andrew Fish]

> On Nov 4, 2016, at 10:48 AM, Rafael Machado 
>  wrote:
> 
> Hi everyone
> 
> Thanks Andrew and Marvin for the clarification.
> Now things start to make sense.
> 
> But I was still not able to understand were things start on a real binary

Rafeal,

I'm not sure what you are asking? 

The .com file contains the hardware real mode reset vector (0xFFF0). So 
execution starts up high. The 1st far jmp you do gets you running in the 
0x000F range (the ROM is aliased to the old PC address). The .com file is 
patched with the entry point of the PE/COFF (or TE) .efi and just jumps into 
that code. At some point early in the processes the code transitions to 
protected mode and starts executing up high again (only a small part of the ROM 
is aliased down low).

The SEC PE/COFF  (or TE) .efi image entry point can be found in the PE/COFF (or 
TE) header that is part of that image. This is how the PEI Core passes control 
to PEIMs, and it is also what the PEI/DXE/SMM cores use to pass control to 
images that are loaded into memory. 

There is a library that given a PE/COFF or TE image will return the entry 
point. 

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffGetEntryPointLib/PeCoffGetEntryPoint.c#L44
 

RETURN_STATUS
EFIAPI
PeCoffLoaderGetEntryPoint (
  IN  VOID  *Pe32Data,
  OUT VOID  **EntryPoint
  )
{
  EFI_IMAGE_DOS_HEADER  *DosHdr;
  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION   Hdr;

  ASSERT (Pe32Data   != NULL);
  ASSERT (EntryPoint != NULL);

  DosHdr = (EFI_IMAGE_DOS_HEADER *)Pe32Data;
  if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
//
// DOS image header is present, so read the PE header after the DOS image 
header.
//
Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN) Pe32Data + (UINTN) 
((DosHdr->e_lfanew) & 0x0));
  } else {
//
// DOS image header is not present, so PE header is at the image base.
//
Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)Pe32Data;
  }

  //
  // Calculate the entry point relative to the start of the image.
  // AddressOfEntryPoint is common for PE32 & PE32+
  //
  if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
*EntryPoint = (VOID *)((UINTN)Pe32Data + 
(UINTN)(Hdr.Te->AddressOfEntryPoint & 0x0) + 
sizeof(EFI_TE_IMAGE_HEADER) - Hdr.Te->StrippedSize);
return RETURN_SUCCESS;
  } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
*EntryPoint = (VOID *)((UINTN)Pe32Data + 
(UINTN)(Hdr.Pe32->OptionalHeader.AddressOfEntryPoint & 0x0));
return RETURN_SUCCESS;
  }

  return RETURN_UNSUPPORTED;
}



Thanks,

Andrew Fish   

> On the attached image ResetVectorCoreboot.png we have the entry point on a
> coreboot image.
> What I would like to do is to find something similar to this on a UEFI Bios
> image.
> 
> Based on Marvin's idea, I got the UEFI Tool and star to check the image I
> have.
> On the attached image TopFileInfo.png we can see to Top File mentioned by
> Andrew.
> The details about the Top File are these:
> 
>  Offset: FF8018h
> 
>  File GUID: 1BA0062E-C779-4582-8566-336AE8F78F09
> 
>  Type: 03h
> 
>  Attributes: 08h
> 
>  Full size: 7FE8h (32744)
> 
>  Header size: 18h (24)
> 
>  Body size: 7FD0h (32720)
> 
>  Tail size: 0h (0)
> 
>  State: F8h
> 
>  Header checksum: 99h, valid
> 
>  Data checksum: AAh, valid
> 
>  Header memory address: 8018h
> 
>  Data memory address: 8030h
> 
>  Compressed: No
> 
>  Fixed: No
> 
> 
> I tried to find something similar to what I see at the coreboot image, but
> didn't find anything. Neither on the PE Image section, not on the Raw image
> sections.
> 
> 
> Any idea about how could I find the entry point of sec in this case?
> 
> Thanks and Regards
> Rafael R. Machado
> 
> 
> Em sáb, 22 de out de 2016 às 16:19, Andrew Fish  escreveu:
> 
>> On Oct 22, 2016, at 10:03 AM, Marvin H?user 
>> wrote:
>> 
>> Hey Rafael,
>> 
>> There actually is some generic SEC code in UefiCpuPkg you might want to
>> take a look at. It's generic because it does not have "Intel NDA" code,
>> such as CAR (Cache-As-RAM) etc.
>> The Reset Vector may or may not be part of SecCore. It's either embedded
>> within the SecCore module, or a separate file in the FFS. You can check the
>> start/end address of the modules (e.g. with UEFITool) and find the Reset
>> Vector file that way.
>> 
>> 
>> Rafael,
>> 
>> There is some strange construction things going on with the SEC for X86.
>> 
>> If you look in the FDF file you will see that the SEC is a PE/COFF (or TE)
>> image and a raw binary for the 16-bit real mode reset vector code.
>> 
>> 
>> https://github.com/tianocore/edk2/blob/master/Vlv2TbltDevicePkg/PlatformPkg.fdf#L876
>> [Rule.Common.SEC]
>> FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
>> PE32 PE32 Align = 8 $(INF_OUTPUT)/$(MODULE_NAME).efi
>> RAW BIN Align = 

[edk2] [PATCH 2/2] ShellPkg/Ifconfig: Enable setting MAC address

[Marcin Wojtas]

This adds new feature to ifconfig shell command, which allow
for updating MAC address of the interface or resetting it to the
initial value.

It consumes newly added NetLib helpers for parsing the Unicode
string to EFI_MAC_ADDRESS, calling Snp->StationAddress() callback
and reconnecting controller, so that all interface's children
devices could get updated with new value of the MAC address.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marcin Wojtas 
---
 .../UefiShellNetwork1CommandsLib/Ifconfig.c| 54 ++
 .../UefiShellNetwork1CommandsLib.uni   | 12 -
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
index 5e243d5..e4ea308 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
@@ -120,6 +120,12 @@ VAR_CHECK_ITEM  mSetCheckList[] = {
 FlagTypeSingle
   },
   {
+L"mac",
+0x0010,
+0x0005,
+FlagTypeSingle
+  },
+  {
 NULL,
 0x0,
 0x0,
@@ -832,6 +838,7 @@ IfConfigSetInterfaceInfo (
   EFI_EVENTTimeOutEvt;
   EFI_EVENTMappedEvt;
   BOOLEAN  IsAddressOk;
+  BOOLEAN  MacAddressReset;
 
   EFI_IP4_CONFIG2_POLICY   Policy;
   EFI_IP4_CONFIG2_MANUAL_ADDRESS   ManualAddress;
@@ -840,6 +847,7 @@ IfConfigSetInterfaceInfo (
   EFI_IPv4_ADDRESS *Dns;
   ARG_LIST *Tmp;
   UINTNIndex;
+  EFI_MAC_ADDRESS  NewMac;
 
   CONST CHAR16* TempString;
 
@@ -1150,6 +1158,52 @@ IfConfigSetInterfaceInfo (
 ShellStatus = SHELL_ACCESS_DENIED;
 goto ON_EXIT;
   }
+} else if (StrCmp (VarArg->Arg, L"mac") == 0) {
+  //
+  // Set MAC address.
+  //
+  MacAddressReset = FALSE;
+  VarArg = VarArg->Next;
+  if (VarArg == NULL) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG_LACK_COMMAND), gShellNetwork1HiiHandle);
+ShellStatus = SHELL_INVALID_PARAMETER;
+goto ON_EXIT;
+  }
+
+  if (StrCmp (VarArg->Arg, L"reset") == 0) {
+MacAddressReset = TRUE;
+  } else {
+Status = NetLibStrToMac (VarArg->Arg, );
+if (EFI_ERROR(Status)) {
+  ShellPrintHiiEx(-1, -1, NULL,STRING_TOKEN 
(STR_IFCONFIG_INVALID_MACADDRESS), gShellNetwork1HiiHandle, VarArg->Arg);
+  ShellStatus = SHELL_INVALID_PARAMETER;
+  goto ON_EXIT;
+}
+  }
+
+  Status = NetLibSetMacAddress (
+  IfCb->NicHandle,
+  ,
+  IfCb->IfInfo->HwAddressSize,
+  MacAddressReset
+  );
+  if (EFI_ERROR(Status)) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG_SET_ADDR_FAILED), gShellNetwork1HiiHandle, Status);
+ShellStatus = SHELL_ACCESS_DENIED;
+goto ON_EXIT;
+  }
+
+  //
+  // Reconnect controller, so that all children get updated with
+  // new MAC address information.
+  //
+  Status = NetLibReconnectInterface (IfCb->NicHandle);
+  if (EFI_ERROR(Status)) {
+ShellStatus = SHELL_DEVICE_ERROR;
+goto ON_EXIT;
+  }
+
+  VarArg = VarArg->Next;
 }
   }
 
diff --git 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
index 76b6188..ffc358d 100644
--- 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
+++ 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
@@ -66,6 +66,7 @@
 #string STR_IFCONFIG_LACK_COMMAND #language en-US"Lack 
interface config option.\n"
 #string STR_IFCONFIG_INVALID_INTERFACE#language en-US"Invalid 
interface name.\n"
 #string STR_IFCONFIG_INVALID_IPADDRESS#language en-US"Invalid ipv4 
address: '%H%s%N'\n"
+#string STR_IFCONFIG_INVALID_MACADDRESS   #language en-US"Invalid MAC 
address: '%H%s%N'\n"
 #string STR_IFCONFIG_DUPLICATE_COMMAND#language en-US"Duplicate 
commands. Bad command %H%s%N is skipped.\n"
 #string STR_IFCONFIG_CONFLICT_COMMAND #language en-US"Conflict 
commands. Bad command %H%s%N is skipped.\n"
 #string STR_IFCONFIG_UNKNOWN_COMMAND  #language en-US"Unknown 
commands. Bad command %H%s%N is skipped.\n"
@@ -133,7 +134,7 @@
 ".SH SYNOPSIS\r\n"
 " \r\n"
 "IFCONFIG [-r [Name]] [-l [Name]]\r\n"
-"IFCONFIG [-s  dhcp |> | >]\r\n"
+"IFCONFIG [-s  dhcp |> | > | 
 | >]\r\n"
 ".SH OPTIONS\r\n"
 " \r\n"
 "  -r  - Renew configuration of interface and set dhcp policy.\r\n"
@@ -146,6 +147,8 @@
 "  - Example: 255.255.255.0\r\n"
 "  

[edk2] [PATCH 1/2] MdeModulePkg: NetLib: introduce MAC address handling helper routines

[Marcin Wojtas]

This patch introduces three functions that can be used for handling
MAC address update. It's also a preparation commit for adding support
for this feature using 'ifconfig' shell command. New functions are as
following:

* NetLibSetMacAddress - locate simple network protocol associated with the
  Service Binding Handle and try to update/reset the MAC address from SNP,
  using Snp->StationAddress() callback.

* NetLibStrToMac - convert one Null-terminated Unicode string to
  EFI_MAC_ADDRESS format.

* NetLibAsciiStrToMac - convert one Null-terminated ASCII string to
  EFI_MAC_ADDRESS format.

* NetLibReconnectInterface - reconnect the service handle. When called
  after NetLibSetMacAddress() it allows for updating associated
  protocols with new MAC address information.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marcin Wojtas 
---
 MdeModulePkg/Include/Library/NetLib.h  |  83 
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 196 +
 2 files changed, 279 insertions(+)

diff --git a/MdeModulePkg/Include/Library/NetLib.h 
b/MdeModulePkg/Include/Library/NetLib.h
index 26709af..7a50f9c 100644
--- a/MdeModulePkg/Include/Library/NetLib.h
+++ b/MdeModulePkg/Include/Library/NetLib.h
@@ -523,6 +523,9 @@ extern IP4_ADDR gIp4AllMasks[IP4_MASK_NUM];
 extern EFI_IPv4_ADDRESS  mZeroIp4Addr;
 
 #define NET_IS_DIGIT(Ch)(('0' <= (Ch)) && ((Ch) <= '9'))
+#define NET_IS_HEX(Ch)  ((('0' <= (Ch)) && ((Ch) <= '9')) || \
+ (('A' <= (Ch)) && ((Ch) <= 'F')) || \
+ (('a' <= (Ch)) && ((Ch) <= 'f')))
 #define NET_ROUNDUP(size, unit) (((size) + (unit) - 1) & (~((unit) - 1)))
 #define NET_IS_LOWER_CASE_CHAR(Ch)  (('a' <= (Ch)) && ((Ch) <= 'z'))
 #define NET_IS_UPPER_CASE_CHAR(Ch)  (('A' <= (Ch)) && ((Ch) <= 'Z'))
@@ -1180,6 +1183,49 @@ NetLibGetMacAddress (
   );
 
 /**
+  Update or reset MAC address associated with the network service handle.
+
+  Locate simple network protocol associated with the Service Binding Handle and
+  try to update/reset the mac address from SNP, using Snp->StationAddress().
+
+  @param[in]   ServiceHandleThe handle where network service binding 
protocols are
+installed on.
+  @param[in]   MacAddress   The pointer with new MAC address.
+  @param[in]   AddressSize  The length of set MAC address.
+  @param[in]   ResetReset MAC address to initial settings.
+
+  @retval EFI_SUCCESS   MAC address was updated/reset successfully.
+  @retval OthersFailed to set SNP MAC address.
+
+**/
+EFI_STATUS
+EFIAPI
+NetLibSetMacAddress (
+  IN EFI_HANDLE   ServiceHandle,
+  IN EFI_MAC_ADDRESS  *MacAddress,
+  IN UINT32AddressSize,
+  IN BOOLEAN  Reset
+  );
+
+/**
+  Reconnect the network service handle.
+
+  Reconnect Service Binding Handle using Boot Services Dis/ConnectController() 
routines.
+
+  @param[in]   ServiceHandleThe handle where network service binding 
protocols are
+installed on.
+
+  @retval EFI_SUCCESS   Interface handle was reconnected successfully.
+  @retval OthersFailed to reconnect interface handle.
+
+**/
+EFI_STATUS
+EFIAPI
+NetLibReconnectInterface (
+  IN EFI_HANDLE ServiceHandle
+  );
+
+/**
   Convert MAC address of the NIC associated with specified Service Binding 
Handle
   to a unicode string. Callers are responsible for freeing the string storage.
 
@@ -1348,6 +1394,24 @@ NetLibDefaultUnload (
   );
 
 /**
+  Convert one Null-terminated ASCII string to EFI_MAC_ADDRESS. The format of
+  the string has to be compliant with colon-separated EUI-48.
+
+  @param[in]  String The pointer to the Ascii string.
+  @param[out] MacAddress The pointer to the converted MAC address.
+
+  @retval EFI_SUCCESSConverted to an MAC address successfully.
+  @retval EFI_INVALID_PARAMETER  The string is malformated, or MacAddress is 
NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+NetLibAsciiStrToMac (
+  IN CONST CHAR8 *String,
+  OUT  EFI_MAC_ADDRESS   *MacAddress
+  );
+
+/**
   Convert one Null-terminated ASCII string (decimal dotted) to 
EFI_IPv4_ADDRESS.
 
   @param[in]  String The pointer to the Ascii string.
@@ -1383,6 +1447,25 @@ NetLibAsciiStrToIp6 (
   );
 
 /**
+  Convert one Null-terminated Unicode string to EFI_MAC_ADDRESS. The format of
+  the string has to be compliant with colon-separated EUI-48.
+
+  @param[in]  String The pointer to the Ascii string.
+  @param[out] MacAddress The pointer to the converted MAC address.
+
+  @retval EFI_SUCCESSConverted to an MAC address successfully.
+  @retval EFI_INVALID_PARAMETER  The string is mal-formated or MacAddress is 
NULL.
+  @retval EFI_OUT_OF_RESOURCES   Failed to perform the operation due to lack 
of resources.
+

[edk2] [PATCH 0/2] MAC address configuration from Shell

[Marcin Wojtas]

Hi,

I present two commits that extend 'ifconfig' Shell command with ability
to configure/reset MAC address of the network interfaces. For that
purpose new routines were added to the NetLib. Details can be found in the
commit logs.

Any comments or remarks would be very welcome.

Best regards,
Marcin

Marcin Wojtas (2):
  MdeModulePkg: NetLib: introduce MAC address handling helper routines
  ShellPkg/Ifconfig: Enable setting MAC address

 MdeModulePkg/Include/Library/NetLib.h  |  83 +
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 196 +
 .../UefiShellNetwork1CommandsLib/Ifconfig.c|  54 ++
 .../UefiShellNetwork1CommandsLib.uni   |  12 +-
 4 files changed, 343 insertions(+), 2 deletions(-)

-- 
1.8.3.1

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

Re: [edk2] Sec and Reset vector

[Rafael Machado]

One additional information.
I checked the code mentioned by Andrew, and at the code things make sense.

Now I'd like to see that kind of initial code at the final binary.

Thanks
Rafael

Em sex, 4 de nov de 2016 às 15:48, Rafael Machado <
rafaelrodrigues.mach...@gmail.com> escreveu:

> Hi everyone
>
> Thanks Andrew and Marvin for the clarification.
> Now things start to make sense.
>
> But I was still not able to understand were things start on a real binary
> On the attached image ResetVectorCoreboot.png we have the entry point on a
> coreboot image.
> What I would like to do is to find something similar to this on a UEFI
> Bios image.
>
> Based on Marvin's idea, I got the UEFI Tool and star to check the image I
> have.
> On the attached image TopFileInfo.png we can see to Top File mentioned by
> Andrew.
> The details about the Top File are these:
>
>   Offset: FF8018h
>
>   File GUID: 1BA0062E-C779-4582-8566-336AE8F78F09
>
>   Type: 03h
>
>   Attributes: 08h
>
>   Full size: 7FE8h (32744)
>
>   Header size: 18h (24)
>
>   Body size: 7FD0h (32720)
>
>   Tail size: 0h (0)
>
>   State: F8h
>
>   Header checksum: 99h, valid
>
>   Data checksum: AAh, valid
>
>   Header memory address: 8018h
>
>   Data memory address: 8030h
>
>   Compressed: No
>
>   Fixed: No
>
>
> I tried to find something similar to what I see at the coreboot image, but
> didn't find anything. Neither on the PE Image section, not on the Raw image
> sections.
>
>
> Any idea about how could I find the entry point of sec in this case?
>
> Thanks and Regards
> Rafael R. Machado
>
>
> Em sáb, 22 de out de 2016 às 16:19, Andrew Fish 
> escreveu:
>
> On Oct 22, 2016, at 10:03 AM, Marvin H?user 
> wrote:
>
> Hey Rafael,
>
> There actually is some generic SEC code in UefiCpuPkg you might want to
> take a look at. It's generic because it does not have "Intel NDA" code,
> such as CAR (Cache-As-RAM) etc.
> The Reset Vector may or may not be part of SecCore. It's either embedded
> within the SecCore module, or a separate file in the FFS. You can check the
> start/end address of the modules (e.g. with UEFITool) and find the Reset
> Vector file that way.
>
>
> Rafael,
>
> There is some strange construction things going on with the SEC for X86.
>
> If you look in the FDF file you will see that the SEC is a PE/COFF (or TE)
> image and a raw binary for the 16-bit real mode reset vector code.
>
>
> https://github.com/tianocore/edk2/blob/master/Vlv2TbltDevicePkg/PlatformPkg.fdf#L876
> [Rule.Common.SEC]
> FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
> PE32 PE32 Align = 8 $(INF_OUTPUT)/$(MODULE_NAME).efi
> RAW BIN Align = 16 |.com
> }
>
> The .com files are constructed from *.nasmb, *.asm16, or *.S16 files.
> https://github.com/tianocore/edk2/tree/master/UefiCpuPkg/SecCore/Ia32
>
> Special extensions are needed to have special build rules. The build rules
> are here:
>
> https://github.com/tianocore/edk2/blob/master/BaseTools/Conf/build_rule.template#L480
> Look at the [Masm16-Code-File] and [Nasm-to-Binary-Code-File] rules.
>
> The build tools also do some magic to stitch the .com and PE/COFF (TE)
> file together.
>
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/SecCore/Ia32/ResetVec.nasmb#L46
> ;
> ; Pointer to the entry point of the PEI core
> ; It is located at 0xFFE0, and is fixed up by some build tool
> ; So if the value 8..1 appears in the final FD image, tool failure occurs.
> ;
> PeiCoreEntryPoint: DD 87654321h
>
>
> The reason you need special build rules is it is really hard to get code
> at the end of a PE/COFF file, so you need a stripped binary for the reset
> vector.
>
> The next problem is how do you get the FV File to be at the end of the FV
> (that is usually free space). The PI spec defines that if an FFS file has
> the File GUID of gEfiFirmwareVolumeTopFileGuid then it gets place at the
> end of the FV. Thus the X86 SEC must have this file guid. This also
> triggers the magic behavior to stitch the .com and PE/COFF together.
>
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/SecCore/SecCore.inf#L25
> FILE_GUID = 1BA0062E-C779-4582-8566-336AE8F78F09
>
>
>
> For ARM things are much simpler. The FV reserves 16-bytes at the start of
> the volume for the reset vector. If the build tools see an FV has an ARM
> SEC it can patch in a branch to the SEC PE/COFF (TE) entry point (going
> from memory hopefully I did not botch that).
>
>
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Pi/PiFirmwareVolume.h#L110
> ///
> /// The first 16 bytes are reserved to allow for the reset vector of
> /// processors whose reset vector is at address 0.
> ///
> UINT8 ZeroVector[16];
>
>
>
> PS.: Seems like inline images are not supported by the mailing list (or is
> it my error?). Either way, I do not see the image in my mail client
> (Outlook 2016).
>
>
> I don't see the image in my macOS Mail client.
>
> Thanks,
>
> Andrew Fish
>
>
> Regards,
> Marvin.
>
> -Original 

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/04/16 16:29, Paolo Bonzini wrote:
> 
> 
> On 04/11/2016 16:22, Laszlo Ersek wrote:
 What does this *KVM internal error. Suberror: 1* mean?
>> The key message is "emulation failure" -- it means that the processor
>> exits to the hypervisor (KVM) because it finds some code that it cannot
>> execute in guest mode natively, so the hypervisor needs to emulate it.
>> And, this emulation fails. The reasons can be:
>> - the code is valid, but KVM lacks the emulation code for it,
>> - the code is actually garbage (not code) -- there was some corruption
>> in the guest (the location used to contain code but it was corrupted, or
>> the guest jumped to non-code data).
>>
>> Usually the register dump contains a short hexadecimal snippet from the
>> instruction stream (near Code=...), pinpointing the byte that caused the
>> problem. However, in this case, all we have is question marks, and this
>> is the very first time I see those. That's why I CC'd Paolo and Radim :)
> 
> The question marks usually mean that the page tables do not map a page
> at that address, but I don't know offhand why KVM would fail emulation
> instead of triple-faulting.
> 
> Try "info tlb" to dump the page tables (huge output of course, you may
> want to use the GTK+ backend which has scrollable consoles).

Thanks, I'll try "info tlb" later. (I generally use virsh
qemu-monitor-command --hmp, whose output is easy to redirect to a file.)

Thanks
Laszlo

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

[edk2] [PATCH] ArmPkg: Update Redistributor granularity for GICv4

[evan . lloyd]

From: Sami Mujawar 

Updated Redistributor base calculation to allow for the fact that
GICv4 has 2 additional 64KB frames (for VLPI and a reserved frame).
The code now tests the VLPIS bit in the GICR_TYPER register
and calculates the Redistributor granularity accordingly.

The code changes are:
  GICR_TYPER register fields, etc, added to the header.
  Loop updated to pay attention to GICR_TYPER.Last.
  Derive frame "stride" size from GICR_TYPER.VLPIS.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Sami Mujawar 
Signed-off-by: Evan Lloyd 
---
Code is available at:
https://github.com/EvanLloyd/tianocore/tree/639_gicv4_v1

 ArmPkg/Include/Library/ArmGicLib.h | 17 -
 ArmPkg/Drivers/ArmGic/ArmGicLib.c  | 40 +---
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmGicLib.h 
b/ArmPkg/Include/Library/ArmGicLib.h
index 
4364f3ffef464596f64cf59881d703cf54cf0ddd..079489fe76ab481915ce9da3702d351fd3cb5f0e
 100644
--- a/ArmPkg/Include/Library/ArmGicLib.h
+++ b/ArmPkg/Include/Library/ArmGicLib.h
@@ -55,12 +55,25 @@
 // GIC Redistributor
 //
 
-#define ARM_GICR_CTLR_FRAME_SIZESIZE_64KB
-#define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB
+#define ARM_GICR_CTLR_FRAME_SIZE SIZE_64KB
+#define ARM_GICR_SGI_PPI_FRAME_SIZE  SIZE_64KB
+#define ARM_GICR_SGI_VLPI_FRAME_SIZE SIZE_64KB
+#define ARM_GICR_SGI_RESERVED_FRAME_SIZE SIZE_64KB
 
 // GIC Redistributor Control frame
 #define ARM_GICR_TYPER  0x0008  // Redistributor Type Register
 
+// GIC Redistributor TYPER bit assignments
+#define ARM_GICR_TYPER_PLPIS(1 << 0)// Physical LPIs
+#define ARM_GICR_TYPER_VLPIS(1 << 1)// Virtual LPIs
+#define ARM_GICR_TYPER_DIRECTLPI(1 << 3)// Direct LPIs
+#define ARM_GICR_TYPER_LAST (1 << 4)// Last Redistributor in series
+#define ARM_GICR_TYPER_DPGS (1 << 5)// Disable Processor Group
+// Selection Support
+#define ARM_GICR_TYPER_PROCNO   (0x << 8)  // Processor Number
+#define ARM_GICR_TYPER_COMMONLPIAFF (0x3 << 24)// Common LPI Affinity
+#define ARM_GICR_TYPER_AFFINITY (0x << 32) // Redistributor 
Affinity
+
 // GIC SGI & PPI Redistributor frame
 #define ARM_GICR_ISENABLER  0x0100  // Interrupt Set-Enable Registers
 #define ARM_GICR_ICENABLER  0x0180  // Interrupt Clear-Enable Registers
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c 
b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 
e658e9bff5d8107b3914bdf1e9e1e51a4e4d4cd7..b51d2b3ec55d277e36835669956b4dd866cfc5c6
 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+*  Copyright (c) 2011-2016, 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,6 +19,16 @@
 #include 
 #include 
 
+// In GICv3, there are 2 x 64KB frames:
+// Redistributor control frame + SGI Control & Generation frame
+#define GIC_V3_REDISTRIBUTOR_GRANULARITY  (ARM_GICR_CTLR_FRAME_SIZE \
+   + ARM_GICR_SGI_PPI_FRAME_SIZE)
+// In GICv4, there are an additional 2 x 64KB frames:
+// VLPI frame + Reserved page frame
+#define GIC_V4_REDISTRIBUTOR_GRANULARITY  (GIC_V3_REDISTRIBUTOR_GRANULARITY \
+   + ARM_GICR_SGI_VLPI_FRAME_SIZE \
+   + ARM_GICR_SGI_RESERVED_FRAME_SIZE)
+
 /**
  *
  * Return whether the Source interrupt index refers to a shared interrupt (SPI)
@@ -40,6 +50,7 @@ SourceIsSpi (
  *
  * @retval Base address of the associated GIC Redistributor
  */
+
 STATIC
 UINTN
 GicGetCpuRedistributorBase (
@@ -47,37 +58,38 @@ GicGetCpuRedistributorBase (
   IN ARM_GIC_ARCH_REVISION Revision
   )
 {
-  UINTN Index;
   UINTN MpId;
-  UINTN CpuAffinity;
-  UINTN Affinity;
-  UINTN GicRedistributorGranularity;
+  UINT32 CpuAffinity;
+  UINT32 Affinity;
   UINTN GicCpuRedistributorBase;
+  UINT64 GicRTyper;
 
   MpId = ArmReadMpidr ();
   // Define CPU affinity as Affinity0[0:8], Affinity1[9:15], Affinity2[16:23], 
Affinity3[24:32]
   // whereas Affinity3 is defined at [32:39] in MPIDR
   CpuAffinity = (MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2)) | 
((MpId & ARM_CORE_AFF3) >> 8);
 
-  if (Revision == ARM_GIC_ARCH_REVISION_3) {
-// 2 x 64KB frame: Redistributor control frame + SGI Control & Generation 
frame
-GicRedistributorGranularity = ARM_GICR_CTLR_FRAME_SIZE + 
ARM_GICR_SGI_PPI_FRAME_SIZE;
-  } else {
+  if (Revision < ARM_GIC_ARCH_REVISION_3) {
 ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
 return 0;
   }
 
   GicCpuRedistributorBase = GicRedistributorBase;
 
-  for (Index = 0; Index < PcdGet32 (PcdCoreCount); Index++) {
-Affinity = 

Re: [edk2] [PATCH 0/4] Defer 3rd party images loading to after EndOfDxe

[Laszlo Ersek]

On 11/04/16 07:09, Ni, Ruiyu wrote:
> No.
> The open source platform patch will be sent out later.

What are the deferred / 3rd party images? Do Driver and SysPrep
qualify?

Or, is this related to value 3 ("Defer execution when there is security
violation") of:
- PcdOptionRomImageVerificationPolicy,
- PcdRemovableMediaImageVerificationPolicy,
- PcdFixedMediaImageVerificationPolicy?

Is the deferral documented somewhere in the UEFI spec, or do we have a
Mantis ticket / ECR about it?

Can we improve the commit messages please? From them, I have no idea how
the deferral is supposed to work. (I.e., what the agents are, and how
they interact.)

Thanks
Laszlo

> From: Gao, Liming
> Sent: Friday, November 4, 2016 1:14 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH 0/4] Defer 3rd party images loading to after 
> EndOfDxe
> 
> Ray:
>   Seemly, PlatformBdsLib library instance should call 
> EfiBootManagerDispatchDeferredImages(), right? Are there patches to update 
> PlatformBdsLib library instance?
> 
> Thanks
> Liming
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Ruiyu Ni
>> Sent: Friday, November 04, 2016 9:00 AM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [PATCH 0/4] Defer 3rd party images loading to after
>> EndOfDxe
>>
>> The patches change the default image loading policy by deferring 3rd
>> party images loading to after EndOfDxe and add a new BDS API to
>> dispatch the deferred images.
>>
>> Platform needs to call the new BDS API
>> EfiBootManagerDispatchDeferredImages after EndOfDxe to ensure that
>> any deferred images are loaded.
>>
>> Ruiyu Ni (4):
>>   MdeModulePkg/SecurityStubDxe: Defer 3rd party image before EndOfDxe
>>   MdeModulePkg/UefiBootManager: Add
>> EfiBootManagerDispatchDeferredImages
>>   MdeModulePkg/BdsDxe: Check deferred images before booting to OS
>>   MdeModulePkg/SecurityStubDxe: Report failure if image is load earlier
>>
>>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  13 +
>>  MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 113 ++
>>  .../Library/UefiBootManagerLib/InternalBm.h|   1 +
>>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
>>  MdeModulePkg/Universal/BdsDxe/Bds.h|   4 +-
>>  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf   |   2 +
>>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c   |  89 +
>>  .../SecurityStubDxe/Defer3rdPartyImageLoad.c   | 413
>> +
>>  .../SecurityStubDxe/Defer3rdPartyImageLoad.h   |  95 +
>>  .../Universal/SecurityStubDxe/SecurityStub.c   |  14 +-
>>  .../Universal/SecurityStubDxe/SecurityStubDxe.inf  |  11 +-
>>  11 files changed, 753 insertions(+), 3 deletions(-)
>>  create mode 100644
>> MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c
>>  create mode 100644
>> MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.h
>>
>> --
>> 2.9.0.windows.1
>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported

[Laszlo Ersek]

On 11/04/16 09:18, Jeff Fan wrote:
> If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at all
> and needn't to register callback functions.
> 
> It could improve boot performance on single supported system.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=204
> 
> Cc: Feng Tian 
> Cc: Liming Gao 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c |  7 ++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c| 39 
> +++--
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c |  8 +++
>  3 files changed, 37 insertions(+), 17 deletions(-)

The MP services protocol and PPI that depend on this library will remain
available to callers, right?

Thanks
Laszlo

> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index b399f1c..eb36d6f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -290,6 +290,13 @@ InitMpGlobalData (
>  
>SaveCpuMpData (CpuMpData);
>  
> +  if (CpuMpData->CpuCount == 1) {
> +//
> +// If only BSP exists, return
> +//
> +return;
> +  }
> +
>//
>// Avoid APs access invalid buff data which allocated by BootServices,
>// so we will allocate reserved data for AP loop code.
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 56b870e..a0edc55 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1143,6 +1143,7 @@ MpInitLibInitialize (
>} else {
>  MaxLogicalProcessorNumber = OldCpuMpData->CpuCount;
>}
> +  ASSERT (MaxLogicalProcessorNumber != 0);
>  
>AsmGetAddressMap ();
>ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof 
> (MP_CPU_EXCHANGE_INFO);
> @@ -1209,10 +1210,12 @@ MpInitLibInitialize (
>MtrrGetAllMtrrs (>MtrrTable);
>  
>if (OldCpuMpData == NULL) {
> -//
> -// Wakeup all APs and calculate the processor count in system
> -//
> -CollectProcessorCount (CpuMpData);
> +if (MaxLogicalProcessorNumber > 1) {
> +  //
> +  // Wakeup all APs and calculate the processor count in system
> +  //
> +  CollectProcessorCount (CpuMpData);
> +}
>} else {
>  //
>  // APs have been wakeup before, just get the CPU Information
> @@ -1238,19 +1241,21 @@ MpInitLibInitialize (
>  sizeof (CPU_VOLATILE_REGISTERS)
>  );
>  }
> -//
> -// Wakeup APs to do some AP initialize sync
> -//
> -WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData);
> -//
> -// Wait for all APs finished initialization
> -//
> -while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
> -  CpuPause ();
> -}
> -CpuMpData->InitFlag = ApInitDone;
> -for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> -  SetApState (>CpuData[Index], CpuStateIdle);
> +if (MaxLogicalProcessorNumber > 1) {
> +  //
> +  // Wakeup APs to do some AP initialize sync
> +  //
> +  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData);
> +  //
> +  // Wait for all APs finished initialization
> +  //
> +  while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
> +CpuPause ();
> +  }
> +  CpuMpData->InitFlag = ApInitDone;
> +  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> +SetApState (>CpuData[Index], CpuStateIdle);
> +  }
>  }
>}
>  
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index e242d37..1f2fcb8 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -321,6 +321,14 @@ InitMpGlobalData (
>EFI_STATUS  Status;
>  
>SaveCpuMpData (CpuMpData);
> +
> +  if (CpuMpData->CpuCount == 1) {
> +//
> +// If only BSP exists, return
> +//
> +return;
> +  }
> +
>//
>// Register an event for EndOfPei
>//
> 

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

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/04/16 14:50, Paolo Bonzini wrote:
> 
> 
> On 04/11/2016 14:28, Yao, Jiewen wrote:
>> I tried below way. But it does not help too much. It still takes more
>> than 1 minutes to boot with SMP=8.
>>
>>   SendSmiIpiAllExcludingSelf ();
>>   IoWrite8 (ICH9_APM_STS, DataPort== NULL ? 0 : *DataPort);
>>   IoWrite8 (ICH9_APM_CNT, CommandPort == NULL ? 0 : *CommandPort);
>>
>> I also tried to reduce the timeout PCD to:
>>
>>   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
>>
>> However, I find CPU-2 is still missing.
>>
>> Maybe it is caused by QEMU emulate AP in serial mode, not parallel mode.
> 
> Yeah, that's possible without KVM.  Do you issue a PAUSE instruction in
> the spin loops?  That could help.

There's a BaseLib helper function for that BTW:

/**
  Requests CPU to pause for a short period of time.

  Requests CPU to pause for a short period of time. Typically used in MP
  systems to prevent memory starvation while waiting for a spin lock.

**/
VOID
EFIAPI
CpuPause (
  VOID
  );


The implementation in "MdePkg/Library/BaseLib/X64/GccInline.c" is

  __asm__ __volatile__ ("pause");

Thanks
Laszlo


>> I think it might be best choice to set PcdCpuSmmSyncMode|0x1
>>
>> It also helps cover a very corner case in SMM. J
>>

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

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/04/16 04:14, Yao, Jiewen wrote:
> Mike
> 
> You are right.
> 
> After I turn on this, I saw all APs.
> 
>  
> 
> However, the system becomes extremely slow. Intolerable.
> 
>  
> 
> I suggest to add more description around below on why it is set to 0x01
> by default, and what is the impact if it becomes 0x00.

Well, the commit messages document the settings in detail -- you can
easily find them by running "git blame" on the DSC file. (I assume
that's how Mike found the commits too.)

(If someone submitted a DSC patch with comments, I wouldn't NACK it, of
course.)

Thanks
Laszlo

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

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/04/16 02:15, Yao, Jiewen wrote:
> Thank you, Mike.
> Yes, I reproduced the issue and found out what is wrong. Here is detail:
> 
> It seems OVMF never puts AP in SMM mode, even *before my patch series*.
> I rollback all my update and use e9d093.
> 
> I add some debug message in PerformRemainingTasks(), I found below:
> *(mSmmMpSyncData->CpuData[0].Present) = 1
> *(mSmmMpSyncData->CpuData[1].Present) = 0
> *(mSmmMpSyncData->CpuData[2].Present) = 0
> *(mSmmMpSyncData->CpuData[3].Present) = 0
> *(mSmmMpSyncData->CpuData[4].Present) = 0
> *(mSmmMpSyncData->CpuData[5].Present) = 0
> *(mSmmMpSyncData->CpuData[6].Present) = 0
> *(mSmmMpSyncData->CpuData[7].Present) = 0
> It is never exposed before, because there is no code to call SmmStartupAp.
> 
> I assume all APs should be present, so I added ASSERT.
> Based upon current OVMF situation, I will change ASSERT to a DEBUG_ERROR 
> message.
> 
> Last but not least important, it is unclear to me, it is a *bug* or a 
> *feature* on OVMF or QEMU.
> Anyone can confirm that?

Thank you very much for narrowing this down.

It is a QEMU platform peculiarity that raising an SMI, with
EFI_SMM_CONTROL2_PROTOCOL.Trigger(), puts *only* the processor that is
executing Trigger() into SMM. The other processors are not affected.

In PiSmmCpuDxeSmm, there is BSP code that waits for the APs to show up
in SMM, for a specific amount of time. (See PcdCpuSmmSyncMode and
PcdCpuSmmApSyncTimeout.) If the APs don't show up in the allotted time,
due to the original SMI, then the BSP pulls them in with directed LAPIC
writes, if I remember correctly.

(We discussed this at great length when we were enabling SMM in OVMF --
I think I may have forgotten most of the details by now.)

On the bare metal, the LAPIC writes are just a fallback, and the
timeouts are not expected in practice. On QEMU however, those timeouts
are the norm (because only the processor calling Trigger() enters SMM).
This caused very long waits when using the variable services (with SMM)
from the runtime OS, even if all of the related code (OS and firmware
alike) were executed only on the BSP (VCPU#0).

In order to mitigate this, we set PcdCpuSmmSyncMode to 1 (relaxed), in
the following patch:

commit 9b1e378811ff730fe4e5a294ea74ef76a915
Author: Paolo Bonzini 
Date:   Mon Nov 30 18:46:46 2015 +

OvmfPkg: use relaxed AP SMM synchronization mode

Port 0xb2 on QEMU only sends an SMI to the currently executing processor.
The SMI handler, however, and in particular SmmWaitForApArrival, currently
expects that SmmControl2DxeTrigger triggers an SMI IPI on all processors
rather than just the BSP.  Thus all SMM invocations loop for a second (the
default value of PcdCpuSmmApSyncTimeout) before SmmWaitForApArrival sends
another SMI IPI to the APs.

With the default SmmCpuFeaturesLib, 32-bit machines must broadcast SMIs
because 32-bit machines must reset the MTRRs on each entry to system
management modes (they have no SMRRs).  However, our virtual platform
does not have problems with cacheability of SMRAM, so we can use "directed"
SMIs instead.  To do this, just set 
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode
to 1 (aka SmmCpuSyncModeRelaxedAp).  This fixes SMM on multiprocessor 
virtual
machines.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael Kinney 

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19058 
6f19259b-4bc3-4df7-8a09-765794883524

Another problem remained though: if the SMI was raised by an AP in the
first place -- which is possible when the runtime OS calls the variable
services --, then the code still waited for the sync timeout to elapse.
This is very-very easy to reproduce with the following command in a
Linux guest:

  taskset -c 1 efibootmgr

The "efibootmgr" command issues a big bunch of variable service calls
(with the help of the guest kernel, of course), and the "taskset -c 1"
command ensures that all of the user space, kernel, and firmware code
involved in "efibootmgr" is executed exclusively on VCPU#1 (that is, an
AP).

(Side remark: invoking efibootmgr without any options causes it to
simply print the BootCurrent, Timeout, BootOrder and Boot variables.
Probably BootNext too, if it exists.)

Initially, this command would take up to 35-40 *seconds* to complete on
my laptop, even with Paolo's 9b1e378811ff in place. Count

- 1 second busy-looping per SMI raised,

- possibly several SMIs per variable service call (I don't know how many
  times the unprivileged part of the variable driver calls into the
  privileged part, per external service call),

- and several variable service calls issued by efibootmgr.

This was clearly intolerable, especially because in a multiprocessor
linux guest, the efibootmgr program (and the initial variable service
calls issued by the kernel during boot) can be 

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Paolo Bonzini]

On 04/11/2016 16:22, Laszlo Ersek wrote:
>> > What does this *KVM internal error. Suberror: 1* mean?
> The key message is "emulation failure" -- it means that the processor
> exits to the hypervisor (KVM) because it finds some code that it cannot
> execute in guest mode natively, so the hypervisor needs to emulate it.
> And, this emulation fails. The reasons can be:
> - the code is valid, but KVM lacks the emulation code for it,
> - the code is actually garbage (not code) -- there was some corruption
> in the guest (the location used to contain code but it was corrupted, or
> the guest jumped to non-code data).
> 
> Usually the register dump contains a short hexadecimal snippet from the
> instruction stream (near Code=...), pinpointing the byte that caused the
> problem. However, in this case, all we have is question marks, and this
> is the very first time I see those. That's why I CC'd Paolo and Radim :)

The question marks usually mean that the page tables do not map a page
at that address, but I don't know offhand why KVM would fail emulation
instead of triple-faulting.

Try "info tlb" to dump the page tables (huge output of course, you may
want to use the GTK+ backend which has scrollable consoles).

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

Re: [edk2] [PATCH] ShellPkg/reset: Support "-fwui" flag

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Friday, November 04, 2016 1:48 AM
> To: edk2-devel@lists.01.org
> Cc: Chen, Chen A ; Carsey, Jaben
> 
> Subject: [PATCH] ShellPkg/reset: Support "-fwui" flag
> Importance: High
> 
> From: Chen A Chen 
> 
> The patch adds "-fwui" support to reset command which is newly added
> to Shell 2.2 spec.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chen A Chen 
> Reviewed-by: Ruiyu Ni 
> Cc: Jaben Carsey 
> ---
>  .../Library/UefiShellLevel2CommandsLib/Reset.c | 48
> +++---
>  .../UefiShellLevel2CommandsLib.h   |  1 +
>  2 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> index 7d4cfb4..40ad8d9 100644
> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> @@ -2,7 +2,7 @@
>Main file for attrib shell level 2 function.
> 
>(C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> -  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 2016, 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
> @@ -16,10 +16,11 @@
>  #include "UefiShellLevel2CommandsLib.h"
> 
>  STATIC CONST SHELL_PARAM_ITEM ResetParamList[] = {
> -  {L"-w", TypeValue},
> -  {L"-s", TypeValue},
> -  {L"-c", TypeValue},
> -  {NULL, TypeMax}
> +  {L"-w",TypeValue},
> +  {L"-s",TypeValue},
> +  {L"-c",TypeValue},
> +  {L"-fwui", TypeFlag },
> +  {NULL, TypeMax  }
>};
> 
>  /**
> @@ -40,6 +41,9 @@ ShellCommandRunReset (
>CONST CHAR16  *String;
>CHAR16*ProblemParam;
>SHELL_STATUS  ShellStatus;
> +  UINT64OsIndications;
> +  UINT32Attr;
> +  UINTN DataSize;
> 
>ShellStatus = SHELL_SUCCESS;
>ProblemParam = NULL;
> @@ -72,6 +76,39 @@ ShellCommandRunReset (
>ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
> gShellLevel2HiiHandle, L"reset");
>ShellStatus = SHELL_INVALID_PARAMETER;
>  } else {
> +
> +  if (ShellCommandLineGetFlag (Package, L"-fwui")) {
> +
> +DataSize  = sizeof (OsIndications);
> +Status = gRT->GetVariable (
> +EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
> ,
> +, , 
> +);
> +if (!EFI_ERROR (Status)) {
> +  if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
> +DataSize = sizeof (OsIndications);
> +Status = gRT->GetVariable (
> +EFI_OS_INDICATIONS_VARIABLE_NAME,
> ,
> +, , 
> +);
> +if (!EFI_ERROR (Status)) {
> +  OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> +} else {
> +  OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> +}
> +Status = gRT->SetVariable (
> +EFI_OS_INDICATIONS_VARIABLE_NAME,
> ,
> +EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> +sizeof (OsIndications), 
> +);
> +  }
> +}
> +if (EFI_ERROR (Status)) {
> +  ShellStatus = SHELL_UNSUPPORTED;
> +  goto Error;
> +}
> +  }
> +
>//
>// check for warm reset flag, then shutdown reset flag, then cold
> (default) reset flag
>//
> @@ -119,6 +156,7 @@ ShellCommandRunReset (
>// as the ResetSystem function should not return...
>//
> 
> +Error:
>//
>// free the command line package
>//
> diff --git
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.h
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.h
> index c262bb5..857487f 100644
> ---
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.h
> +++
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.h
> @@ -25,6 +25,7 @@
> 
>  #include 
> 
> +#include 
>  #include 
> 
>  #include 
> --
> 2.9.0.windows.1

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

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/04/16 10:35, Yao, Jiewen wrote:
> Hi Laszlo
> I just send out V2 patch.
> 
> The new update resolved OVMF ASSERT issue, and added more information in 
> commit message.

Thanks! I'll try to look at v2 (with some more testing) next week.

> 
> However, there is no update for OVMF stability issue. I have no much idea on 
> what happened.
> 
> If you want to try V2 with more tests, that is great.
> But if you want to resolve S3 stability issue at first, I am also OK.
> 
> At same time, if you have any clue on the S3 stability issue, please let me 
> know.

Right... I hope I can ask Paolo and Radim to help us with that.

Thanks
Laszlo

> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Friday, November 4, 2016 5:43 AM
> To: Yao, Jiewen 
> Cc: Tian, Feng ; Radim Krčmář ; 
> edk2-de...@ml01.01.org; Kinney, Michael D ; Paolo 
> Bonzini ; Fan, Jeff ; Zeng, Star 
> 
> Subject: Re: [edk2] [PATCH 0/6] Enable SMM page level protection.
> 
> On 11/03/16 07:53, Jiewen Yao wrote:
>> This series patch enables SMM page level protection.
>> Features are:
>> 1) PiSmmCore reports SMM PE image code/data information
>> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
>> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
>> and set XD for data page and RO for code page.
>> 3) PiSmmCpu enables Static Paging for X64 according to
>> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
>> is used as long as it is supported.
>> 4) PiSmmCpu sets importance data structure to be read only,
>> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
>>
>> tested platform:
>> 1) Intel internal platform (X64).
>> 2) EDKII Quark IA32
>> 3) EDKII Vlv2  X64
>> 4) EDKII OVMF IA32 and IA32X64.
> 
> Did you use a Windows QEMU binary for the OVMF tests?
> 
> Please find my test results below, all done on KVM
> (3.10.0-514.el7.x86_64, if anyone is curious :)).
> 
>>
>> Cc: Jeff Fan >
>> Cc: Feng Tian >
>> Cc: Star Zeng >
>> Cc: Michael D Kinney 
>> >
>> Cc: Laszlo Ersek >
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao >
>>
>> Jiewen Yao (6):
>>   MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
>>   MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
>>   MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
>>   UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
>>   UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
>>   QuarkPlatformPkg/dsc: enable Smm paging protection.
> 
> Legend:
> 
> - "untested" means the test was not executed because the same test
>   failed or proved unreliable in a less demanding configuration already,
> 
> - "n/a" means a setting or test case was impossible,
> 
> - "fail" and "unreliable" (lower case) are outside the scope of this
>   series; they either capture the pre-series status, or are expected
>   even with the series applied due to the pre-series status,
> 
> - "FAIL" and "UNRELIABLE" mean regressions caused (or exposed) by the
>   series.
> 
> In all cases, 36 bits were used as address width in the CPU HOB (--> up
> to 64GB guest-phys address space).
> 
>series  OVMF  
> VCPU boot   S3 resume
>  # applied platform PcdCpuMaxLogicalProcessorNumber PcdCpuSmmStaticPageTable 
> topology result result
> -- ---  ---  
>  -- -
>  1 no  Ia32  64  n/a 
> 1x2x2pass   unreliable
>  2 no  Ia32 255  n/a 
> 52x2x2   pass   untested
>  3 no  Ia32 255  n/a 
> 53x2x2   unreliable untested
>  4 no  Ia32X64   64  n/a 
> 1x2x2pass   pass
>  5 no  Ia32X64  255  n/a 
> 52x2x2   pass   pass
>  6 no  Ia32X64  255  n/a 
> 54x2x2   fail   n/a
>  7 yes Ia32  64  FALSE   
> 1x2x2FAIL   n/a
>  8 yes Ia32  64  TRUE
> 1x2x2FAIL   n/a
>  9 yes Ia32 255  FALSE   
> 52x2x2   untested   untested
> 10 yes Ia32 255  FALSE   
> 53x2x2   untested  

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/04/16 00:51, Yao, Jiewen wrote:
> Hi Laszlo
> I appreciate your help to validate the patch for me.

Well, part of me is just plain altruistic :), but another part of me is
responsible for OVMF at Red Hat, and I'd rather find out about any
possible regressions before they are committed ;)

> Yes, I am using a windows QEMU binary. (qemu-system-x86_64.exe)
> 
> A quick look at the code.
> 
> 
> 1)  The ASSERT issue
>> ASSERT UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c(468): !EFI_ERROR 
>> (Status)
> is caused by this line:
>   Status = SmmBlockingStartupThisAp (FlushTlbOnCurrentProcessor, Index, 
> NULL);
>   ASSERT_EFI_ERROR(Status);
> 
> I am surprise to see it, because I think is an impossible case - 
> InternalSmmStartupThisAp().
>   if (CpuIndex >= gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus ||
>   CpuIndex == gSmmCpuPrivate->SmmCoreEntryContext.CurrentlyExecutingCpu ||
>   !(*(mSmmMpSyncData->CpuData[CpuIndex].Present)) ||
>   gSmmCpuPrivate->Operation[CpuIndex] == SmmCpuRemove) {
> return EFI_INVALID_PARAMETER;
>   }
> 
> I am using below line to start QEMU:
> "qemu-system-x86_64.exe -machine q35,smm=on,accel=tcg -smp 1 -drive 
> if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly=on -drive 
> if=pflash,format=raw,unit=1,file=OVMF_VARS.fd --serial COM8 -boot 
> menu=on,splash-time=7000 fat:rw:Test"
> 
> Would you mind let me know what is difference between your environment and 
> mine?
> 
> I will add more debug info to see what happened, which of above lines 
> triggers the error.

(I will comment on this elsewhere.)

> 2)  The unstable case is a headache.
> According to "RIP=0009f0fd", it seems outside of SMM right?

So, this address can be correlated with the final part of the OVMF debug
log. The OVMF debug log says (as I wrote),

> Transfer to 16bit OS waking vector - 9A1D0

and the RIP in the failure info is 9f0fd -- the difference is just
0x4F2D (20269) bytes. So, I must think, something blows up in the resume
vector of the Linux kernel that I used as guest for testing.

(Also, you can confirm we are outside of SMM from the string "SMM=0" in
the register dump.)

> What does this *KVM internal error. Suberror: 1* mean?

The key message is "emulation failure" -- it means that the processor
exits to the hypervisor (KVM) because it finds some code that it cannot
execute in guest mode natively, so the hypervisor needs to emulate it.
And, this emulation fails. The reasons can be:
- the code is valid, but KVM lacks the emulation code for it,
- the code is actually garbage (not code) -- there was some corruption
in the guest (the location used to contain code but it was corrupted, or
the guest jumped to non-code data).

Usually the register dump contains a short hexadecimal snippet from the
instruction stream (near Code=...), pinpointing the byte that caused the
problem. However, in this case, all we have is question marks, and this
is the very first time I see those. That's why I CC'd Paolo and Radim :)

Thanks
Laszlo


> 
> Do you have any clue on what happened?
> 
> Thank you
> Yao Jiewen
> 
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Friday, November 4, 2016 5:43 AM
> To: Yao, Jiewen 
> Cc: Tian, Feng ; Radim Krčmář ; 
> edk2-de...@ml01.01.org; Kinney, Michael D ; Paolo 
> Bonzini ; Fan, Jeff ; Zeng, Star 
> 
> Subject: Re: [edk2] [PATCH 0/6] Enable SMM page level protection.
> 
> On 11/03/16 07:53, Jiewen Yao wrote:
>> This series patch enables SMM page level protection.
>> Features are:
>> 1) PiSmmCore reports SMM PE image code/data information
>> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
>> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
>> and set XD for data page and RO for code page.
>> 3) PiSmmCpu enables Static Paging for X64 according to
>> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
>> is used as long as it is supported.
>> 4) PiSmmCpu sets importance data structure to be read only,
>> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
>>
>> tested platform:
>> 1) Intel internal platform (X64).
>> 2) EDKII Quark IA32
>> 3) EDKII Vlv2  X64
>> 4) EDKII OVMF IA32 and IA32X64.
> 
> Did you use a Windows QEMU binary for the OVMF tests?
> 
> Please find my test results below, all done on KVM
> (3.10.0-514.el7.x86_64, if anyone is curious :)).
> 
>>
>> Cc: Jeff Fan >
>> Cc: Feng Tian >
>> Cc: Star Zeng >
>> Cc: Michael D Kinney 
>> >
>> Cc: Laszlo Ersek >
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen 

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Paolo Bonzini]

On 04/11/2016 14:28, Yao, Jiewen wrote:
> I tried below way. But it does not help too much. It still takes more
> than 1 minutes to boot with SMP=8.
> 
>   SendSmiIpiAllExcludingSelf ();
>   IoWrite8 (ICH9_APM_STS, DataPort== NULL ? 0 : *DataPort);
>   IoWrite8 (ICH9_APM_CNT, CommandPort == NULL ? 0 : *CommandPort);
> 
> I also tried to reduce the timeout PCD to:
> 
>   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
> 
> However, I find CPU-2 is still missing.
> 
> Maybe it is caused by QEMU emulate AP in serial mode, not parallel mode.

Yeah, that's possible without KVM.  Do you issue a PAUSE instruction in
the spin loops?  That could help.

Paolo

> I think it might be best choice to set PcdCpuSmmSyncMode|0x1
> 
> It also helps cover a very corner case in SMM. J
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3] OvmfPkg/ResetVector: Depend on PCD values of the page tables.

[Marvin Häuser]

Hey Laszlo,

I'm terribly sorry for the mistakes in v3, I made it in a hurry because it was 
late - should have postponded for today.
Because I didn't see 'your v3' (or didn't you post it yet?), I posted a v4 as 
you said, which should have fixed the three comments you had.

Thank you very much for your input!

Regards,
Marvin.

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, November 4, 2016 12:56 AM
> To: Marvin Häuser ; edk2-
> de...@lists.01.org 
> Cc: jordan.l.jus...@intel.com
> Subject: Re: [PATCH v3] OvmfPkg/ResetVector: Depend on PCD values of the
> page tables.
> 
> Three comments:
> 
> On 11/03/16 22:41, Marvin Häuser wrote:
> > Currently, the value of the page tables' address is hard-coded in the
> > ResetVector. This patch replaces these values with a PCD dependency.
> >
> > A check for the size has been added to alert the developer to rewrite
> > the ASM according to the new size, if it has been changed.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Marvin Haeuser 
> > ---
> >  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 23 ++--
> >  OvmfPkg/ResetVector/ResetVector.inf   |  5 +
> >  OvmfPkg/ResetVector/ResetVector.nasmb |  6 +
> >  3 files changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > index b5a4cf8d7187..0b95a7fa9a86 100644
> > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > @@ -44,10 +44,11 @@ BITS32
> >  SetCr3ForPageTables64:
> >
> >  ;
> > -; For OVMF, build some initial page tables at 0x80-0x806000.
> > +; For OVMF, build some initial page tables at
> > +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> >  ;
> > -; This range should match with PcdOvmfSecPageTablesBase and
> > -; PcdOvmfSecPageTablesSize which are declared in the FDF files.
> > +; This range should match with PcdOvmfSecPageTablesSize which is
> > +; declared in the FDF files.
> >  ;
> >  ; At the end of PEI, the pages tables will be rebuilt into a
> >  ; more permanent location by DxeIpl.
> > @@ -56,21 +57,21 @@ SetCr3ForPageTables64:
> >  mov ecx, 6 * 0x1000 / 4
> >  xor eax, eax
> >  clearPageTablesMemoryLoop:
> > -mov dword[ecx * 4 + 0x80 - 4], eax
> > +mov dword[ecx * 4 + PT_ADDR (0 - 4)], eax
> 
> I think this is incorrect (or dubious at least); the NASM-level replacement 
> text
> for this will be
> 
>   (0x80 + (0 - 4))
> 
> and I strongly dislike the (0 - 4) subexpression. The type in which NASM
> evaluates (0 - 4) is not specified, and I'd rather not risk it being UINT64, 
> for
> example.
> 
> >  loopclearPageTablesMemoryLoop
> >
> >  ;
> >  ; Top level Page Directory Pointers (1 * 512GB entry)
> >  ;
> > -mov dword[0x80], 0x801000 + PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
> >
> >  ;
> >  ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
> >  ;
> > -mov dword[0x801000], 0x802000 + PAGE_PDP_ATTR
> > -mov dword[0x801008], 0x803000 + PAGE_PDP_ATTR
> > -mov dword[0x801010], 0x804000 + PAGE_PDP_ATTR
> > -mov dword[0x801018], 0x805000 + PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) +
> PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) +
> PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) +
> PAGE_PDP_ATTR
> > +mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) +
> PAGE_PDP_ATTR
> >
> >  ;
> >  ; Page Table Entries (2048 * 2MB entries => 4GB) @@ -81,13 +82,13
> > @@ pageTableEntriesLoop:
> >  dec eax
> >  shl eax, 21
> >  add eax, PAGE_2M_PDE_ATTR
> > -mov [ecx * 8 + 0x802000 - 8], eax
> > +mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> >  looppageTableEntriesLoop
> >
> >  ;
> >  ; Set CR3 now that the paging structures are available
> >  ;
> > -mov eax, 0x80
> > +mov eax, PT_ADDR (0)
> >  mov cr3, eax
> >
> >  OneTimeCallRet SetCr3ForPageTables64 diff --git
> > a/OvmfPkg/ResetVector/ResetVector.inf
> > b/OvmfPkg/ResetVector/ResetVector.inf
> > index 46610d243ecf..d1e5d4d9bdea 100644
> > --- a/OvmfPkg/ResetVector/ResetVector.inf
> > +++ b/OvmfPkg/ResetVector/ResetVector.inf
> > @@ -29,9 +29,14 @@ [Sources]
> >ResetVector.nasmb
> >
> >  [Packages]
> > +  OvmfPkg/OvmfPkg.dec
> >MdePkg/MdePkg.dec
> >UefiCpuPkg/UefiCpuPkg.dec
> >
> >  [BuildOptions]
> > *_*_IA32_NASMB_FLAGS = -
> I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
> > *_*_X64_NASMB_FLAGS = -
> I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
> > +
> > +[Pcd]
> > +  

[edk2] [PATCH v4] OvmfPkg/ResetVector: Depend on PCD values of the page tables.

[Marvin Häuser]

Currently, the value of the page tables' address is hard-coded in the
ResetVector. This patch replaces these values with a PCD dependency.

A check for the size has been added to alert the developer to rewrite
the ASM according to the new size, if it has been changed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marvin Haeuser 
---
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 23 ++--
 OvmfPkg/ResetVector/ResetVector.inf   |  5 +
 OvmfPkg/ResetVector/ResetVector.nasmb |  7 ++
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index b5a4cf8d7187..6201cad1f5dc 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -44,10 +44,11 @@ BITS32
 SetCr3ForPageTables64:
 
 ;
-; For OVMF, build some initial page tables at 0x80-0x806000.
+; For OVMF, build some initial page tables at
+; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
 ;
-; This range should match with PcdOvmfSecPageTablesBase and
-; PcdOvmfSecPageTablesSize which are declared in the FDF files.
+; This range should match with PcdOvmfSecPageTablesSize which is
+; declared in the FDF files.
 ;
 ; At the end of PEI, the pages tables will be rebuilt into a
 ; more permanent location by DxeIpl.
@@ -56,21 +57,21 @@ SetCr3ForPageTables64:
 mov ecx, 6 * 0x1000 / 4
 xor eax, eax
 clearPageTablesMemoryLoop:
-mov dword[ecx * 4 + 0x80 - 4], eax
+mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
 loopclearPageTablesMemoryLoop
 
 ;
 ; Top level Page Directory Pointers (1 * 512GB entry)
 ;
-mov dword[0x80], 0x801000 + PAGE_PDP_ATTR
+mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
 
 ;
 ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
 ;
-mov dword[0x801000], 0x802000 + PAGE_PDP_ATTR
-mov dword[0x801008], 0x803000 + PAGE_PDP_ATTR
-mov dword[0x801010], 0x804000 + PAGE_PDP_ATTR
-mov dword[0x801018], 0x805000 + PAGE_PDP_ATTR
+mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
+mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR
+mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR
+mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR
 
 ;
 ; Page Table Entries (2048 * 2MB entries => 4GB)
@@ -81,13 +82,13 @@ pageTableEntriesLoop:
 dec eax
 shl eax, 21
 add eax, PAGE_2M_PDE_ATTR
-mov [ecx * 8 + 0x802000 - 8], eax
+mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
 looppageTableEntriesLoop
 
 ;
 ; Set CR3 now that the paging structures are available
 ;
-mov eax, 0x80
+mov eax, PT_ADDR (0)
 mov cr3, eax
 
 OneTimeCallRet SetCr3ForPageTables64
diff --git a/OvmfPkg/ResetVector/ResetVector.inf 
b/OvmfPkg/ResetVector/ResetVector.inf
index 46610d243ecf..d1e5d4d9bdea 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -29,9 +29,14 @@ [Sources]
   ResetVector.nasmb
 
 [Packages]
+  OvmfPkg/OvmfPkg.dec
   MdePkg/MdePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
 
 [BuildOptions]
*_*_IA32_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
*_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb 
b/OvmfPkg/ResetVector/ResetVector.nasmb
index 31ac06ae4a8c..29cbad367711 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -53,6 +53,13 @@
 %include "Ia32/SearchForSecEntry.asm"
 
 %ifdef ARCH_X64
+  #include 
+
+  %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) != 0x6000)
+%error "This implementation inherently depends on PcdOvmfSecPageTablesSize"
+  %endif
+
+  %define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + (Offset))
 %include "Ia32/Flat32ToFlat64.asm"
 %include "Ia32/PageTables64.asm"
 %endif
-- 
2.10.1.windows.1

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

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Yao, Jiewen]

Thank you Paolo.

I tried below way. But it does not help too much. It still takes more than 1 
minutes to boot with SMP=8.
  SendSmiIpiAllExcludingSelf ();
  IoWrite8 (ICH9_APM_STS, DataPort== NULL ? 0 : *DataPort);
  IoWrite8 (ICH9_APM_CNT, CommandPort == NULL ? 0 : *CommandPort);

I also tried to reduce the timeout PCD to:
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
However, I find CPU-2 is still missing.

Maybe it is caused by QEMU emulate AP in serial mode, not parallel mode.
I think it might be best choice to set PcdCpuSmmSyncMode|0x1
It also helps cover a very corner case in SMM. :)


At same time, would you mind help to take a look at the S3 unstable issue? If 
you have any clue, please let me know.


Thank you
Yao Jiewen

From: Paolo Bonzini [mailto:pbonz...@redhat.com]
Sent: Friday, November 4, 2016 7:34 PM
To: Yao, Jiewen ; Kinney, Michael D 
; Laszlo Ersek 
Cc: Tian, Feng ; Radim Kr?má? ; 
edk2-de...@ml01.01.org; Fan, Jeff ; Zeng, Star 

Subject: Re: [edk2] [PATCH 0/6] Enable SMM page level protection.



On 04/11/2016 04:20, Yao, Jiewen wrote:
> Good info. Thanks!
>
> I do not understand below word. I still see a **huge** performance gap.
>
> I am confused on how is it resolved in previous patch. Or do I need
> configure something for my QEMU?

The delay you're seeing comes from SmmWaitForApArrival.  See this
explanation:


Port 0xb2 on QEMU only sends an SMI to the currently executing
processor.  The SMI handler, however, and in particular
SmmWaitForApArrival, currently expects that SmmControl2DxeTrigger
triggers an SMI IPI on all processors rather than just the BSP.  Thus
all SMM invocations loop for a second (the default value of
PcdCpuSmmApSyncTimeout) before SmmWaitForApArrival sends another SMI IPI
to the APs.


Can you try calling SendSmiIpiAllExcludingSelf in SmmControl2DxeTrigger
(OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c) before the I/O port writes?

Thanks,

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

Re: [edk2] [PATCH v3] MdePkg/BaseMemoryLib*: check for zero length in ZeroMem ()

[Ard Biesheuvel]

On 4 November 2016 at 09:08, Gao, Liming  wrote:
> Reviewed-by: Liming Gao 
>

Pushed, thanks.

>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Ard Biesheuvel
>> Sent: Friday, November 04, 2016 5:05 PM
>> To: edk2-devel@lists.01.org; Kinney, Michael D
>> ; Gao, Liming 
>> Cc: Carsey, Jaben ; ler...@redhat.com; Ard
>> Biesheuvel 
>> Subject: [edk2] [PATCH v3] MdePkg/BaseMemoryLib*: check for zero length
>> in ZeroMem ()
>>
>> Unlike other string functions in this library, ZeroMem () does not
>> return early when the length of the input buffer is 0. So add the
>> same to ZeroMem () as well, for all implementations of BaseMemoryLib
>> living under MdePkg/
>>
>> This fixes an issue with the ARM implementation of BaseMemoryLibOPtDxe,
>> whose InternalMemZeroMem code does not expect a length of 0, and
>> always
>> writes at least a single byte.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> Acked-by: Laszlo Ersek 
>> ---
>>  MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c   | 6 +-
>>  MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c| 6 +-
>>  MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c | 6 +-
>>  MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c | 6 +-
>>  MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c | 6 +-
>>  MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c   | 6 +-
>>  MdePkg/Library/PeiMemoryLib/ZeroMemWrapper.c| 6 +-
>>  MdePkg/Library/UefiMemoryLib/ZeroMemWrapper.c   | 6 +-
>>  8 files changed, 40 insertions(+), 8 deletions(-)
>>
>> diff --git a/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
>> b/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
>> index 2a0a038fd6c5..9dd0b45e188e 100644
>> --- a/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
>> +++ b/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
>> @@ -46,7 +46,11 @@ ZeroMem (
>>IN UINTN  Length
>>)
>>  {
>> -  ASSERT (!(Buffer == NULL && Length > 0));
>> +  if (Length == 0) {
>> +return Buffer;
>> +  }
>> +
>> +  ASSERT (Buffer != NULL);
>>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>>return InternalMemZeroMem (Buffer, Length);
>>  }
>> diff --git a/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
>> b/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
>> index 2a0a038fd6c5..9dd0b45e188e 100644
>> --- a/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
>> +++ b/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
>> @@ -46,7 +46,11 @@ ZeroMem (
>>IN UINTN  Length
>>)
>>  {
>> -  ASSERT (!(Buffer == NULL && Length > 0));
>> +  if (Length == 0) {
>> +return Buffer;
>> +  }
>> +
>> +  ASSERT (Buffer != NULL);
>>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>>return InternalMemZeroMem (Buffer, Length);
>>  }
>> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
>> b/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
>> index 2a0a038fd6c5..9dd0b45e188e 100644
>> --- a/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
>> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
>> @@ -46,7 +46,11 @@ ZeroMem (
>>IN UINTN  Length
>>)
>>  {
>> -  ASSERT (!(Buffer == NULL && Length > 0));
>> +  if (Length == 0) {
>> +return Buffer;
>> +  }
>> +
>> +  ASSERT (Buffer != NULL);
>>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>>return InternalMemZeroMem (Buffer, Length);
>>  }
>> diff --git a/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
>> b/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
>> index 2a0a038fd6c5..9dd0b45e188e 100644
>> --- a/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
>> +++ b/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
>> @@ -46,7 +46,11 @@ ZeroMem (
>>IN UINTN  Length
>>)
>>  {
>> -  ASSERT (!(Buffer == NULL && Length > 0));
>> +  if (Length == 0) {
>> +return Buffer;
>> +  }
>> +
>> +  ASSERT (Buffer != NULL);
>>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>>return InternalMemZeroMem (Buffer, Length);
>>  }
>> diff --git a/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
>> b/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
>> index 2a0a038fd6c5..9dd0b45e188e 100644
>> --- a/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
>> +++ b/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
>> @@ -46,7 +46,11 @@ ZeroMem (
>>IN UINTN  Length
>>)
>>  {
>> -  ASSERT (!(Buffer == NULL && Length > 0));
>> +  if (Length == 0) {
>> +return Buffer;
>> +  }
>> +
>> +  ASSERT (Buffer != NULL);
>>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>>return InternalMemZeroMem (Buffer, Length);
>>  }
>> diff --git a/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
>> b/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
>> index 

Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

[Yao, Jiewen]

Hi Laszlo
I just send out V2 patch.

The new update resolved OVMF ASSERT issue, and added more information in commit 
message.

However, there is no update for OVMF stability issue. I have no much idea on 
what happened.

If you want to try V2 with more tests, that is great.
But if you want to resolve S3 stability issue at first, I am also OK.

At same time, if you have any clue on the S3 stability issue, please let me 
know.

Thank you
Yao Jiewen


From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Friday, November 4, 2016 5:43 AM
To: Yao, Jiewen 
Cc: Tian, Feng ; Radim Krčmář ; 
edk2-de...@ml01.01.org; Kinney, Michael D ; Paolo 
Bonzini ; Fan, Jeff ; Zeng, Star 

Subject: Re: [edk2] [PATCH 0/6] Enable SMM page level protection.

On 11/03/16 07:53, Jiewen Yao wrote:
> This series patch enables SMM page level protection.
> Features are:
> 1) PiSmmCore reports SMM PE image code/data information
> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
> and set XD for data page and RO for code page.
> 3) PiSmmCpu enables Static Paging for X64 according to
> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
> is used as long as it is supported.
> 4) PiSmmCpu sets importance data structure to be read only,
> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
>
> tested platform:
> 1) Intel internal platform (X64).
> 2) EDKII Quark IA32
> 3) EDKII Vlv2  X64
> 4) EDKII OVMF IA32 and IA32X64.

Did you use a Windows QEMU binary for the OVMF tests?

Please find my test results below, all done on KVM
(3.10.0-514.el7.x86_64, if anyone is curious :)).

>
> Cc: Jeff Fan >
> Cc: Feng Tian >
> Cc: Star Zeng >
> Cc: Michael D Kinney 
> >
> Cc: Laszlo Ersek >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao >
>
> Jiewen Yao (6):
>   MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
>   MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
>   MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
>   UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
>   UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
>   QuarkPlatformPkg/dsc: enable Smm paging protection.

Legend:

- "untested" means the test was not executed because the same test
  failed or proved unreliable in a less demanding configuration already,

- "n/a" means a setting or test case was impossible,

- "fail" and "unreliable" (lower case) are outside the scope of this
  series; they either capture the pre-series status, or are expected
  even with the series applied due to the pre-series status,

- "FAIL" and "UNRELIABLE" mean regressions caused (or exposed) by the
  series.

In all cases, 36 bits were used as address width in the CPU HOB (--> up
to 64GB guest-phys address space).

   series  OVMF  
VCPU boot   S3 resume
 # applied platform PcdCpuMaxLogicalProcessorNumber PcdCpuSmmStaticPageTable 
topology result result
-- ---  ---  
 -- -
 1 no  Ia32  64  n/a 
1x2x2pass   unreliable
 2 no  Ia32 255  n/a 
52x2x2   pass   untested
 3 no  Ia32 255  n/a 
53x2x2   unreliable untested
 4 no  Ia32X64   64  n/a 
1x2x2pass   pass
 5 no  Ia32X64  255  n/a 
52x2x2   pass   pass
 6 no  Ia32X64  255  n/a 
54x2x2   fail   n/a
 7 yes Ia32  64  FALSE   
1x2x2FAIL   n/a
 8 yes Ia32  64  TRUE
1x2x2FAIL   n/a
 9 yes Ia32 255  FALSE   
52x2x2   untested   untested
10 yes Ia32 255  FALSE   
53x2x2   untested   untested
11 yes Ia32 255  TRUE
52x2x2   untested   untested
12 yes Ia32 255  TRUE
53x2x2   untested   untested
13 yes Ia32X64   64  FALSE   
1x2x2pass   UNRELIABLE
14 yes 

[edk2] [PATCH V2 1/6] MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h

[Jiewen Yao]

This table describes the SMM memory attributes.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h | 51 

 1 file changed, 51 insertions(+)

diff --git a/MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h 
b/MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h
new file mode 100644
index 000..317eae1
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h
@@ -0,0 +1,51 @@
+/** @file
+  Define the GUID of the EDKII PI SMM memory attribute table, which
+  is published by PI SMM Core.
+
+Copyright (c) 2016, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under
+the terms and conditions of the BSD License that accompanies this distribution.
+The full text of the license may be found at
+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 _PI_SMM_MEMORY_ATTRIBUTES_TABLE_H_
+#define _PI_SMM_MEMORY_ATTRIBUTES_TABLE_H_
+
+#define EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE_GUID {\
+  0x6b9fd3f7, 0x16df, 0x45e8, {0xbd, 0x39, 0xb9, 0x4a, 0x66, 0x54, 0x1a, 0x5d} 
\
+}
+
+//
+// The PI SMM memory attribute table contains the SMM memory map for SMM image.
+//
+// This table is installed to SMST as SMM configuration table.
+//
+// This table is published at gEfiSmmEndOfDxeProtocolGuid notification, because
+// there should be no more SMM driver loaded after that. The 
EfiRuntimeServicesCode
+// region should not be changed any more.
+//
+// This table is published, if and only if all SMM PE/COFF have aligned section
+// as specified in UEFI specification Section 2.3. For example, IA32/X64 
alignment is 4KiB.
+//
+// If this table is published, the EfiRuntimeServicesCode contains code only
+// and it is EFI_MEMORY_RO; the EfiRuntimeServicesData contains data only
+// and it is EFI_MEMORY_XP.
+//
+typedef struct {
+  UINT32Version;
+  UINT32NumberOfEntries;
+  UINT32DescriptorSize;
+  UINT32Reserved;
+//EFI_MEMORY_DESCRIPTOR Entry[1];
+} EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE;
+
+#define EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE_VERSION  0x0001
+
+extern EFI_GUID gEdkiiPiSmmMemoryAttributesTableGuid;
+
+#endif
-- 
2.7.4.windows.1

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

[edk2] [PATCH V2 6/6] QuarkPlatformPkg/dsc: enable Smm paging protection.

[Jiewen Yao]

Cc: Michael D Kinney 
Cc: Kelly Steele 
Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 QuarkPlatformPkg/Quark.dsc | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/QuarkPlatformPkg/Quark.dsc b/QuarkPlatformPkg/Quark.dsc
index d5988da..9804b70 100644
--- a/QuarkPlatformPkg/Quark.dsc
+++ b/QuarkPlatformPkg/Quark.dsc
@@ -891,3 +891,9 @@
 
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096
+
+# Force PE/COFF sections to be aligned at 4KB boundaries to support page level 
protection of DXE_SMM_DRIVER/SMM_CORE modules
+[BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
+  MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
-- 
2.7.4.windows.1

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

[edk2] [PATCH V2 2/6] MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.

[Jiewen Yao]

This table describes the SMM memory attributes.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/MdeModulePkg.dec | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 74b8700..99a028f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -355,6 +355,9 @@
   ## Include/Guid/PiSmmCommunicationRegionTable.h
   gEdkiiPiSmmCommunicationRegionTableGuid = { 0x4e28ca50, 0xd582, 0x44ac, 
{0xa1, 0x1f, 0xe3, 0xd5, 0x65, 0x26, 0xdb, 0x34}}
 
+  ## Include/Guid/PiSmmMemoryAttributesTable.h
+  gEdkiiPiSmmMemoryAttributesTableGuid = { 0x6b9fd3f7, 0x16df, 0x45e8, {0xbd, 
0x39, 0xb9, 0x4a, 0x66, 0x54, 0x1a, 0x5d}}
+
 [Ppis]
   ## Include/Ppi/AtaController.h
   gPeiAtaControllerPpiGuid   = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 
0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
-- 
2.7.4.windows.1

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

[edk2] [PATCH V2 0/6] Enable SMM page level protection.

[Jiewen Yao]

 below is V2 description 
1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
2) PiSmmCpu: Add debug info on StartupAp() fails.
3) PiSmmCpu: Add ASSERT for AllocatePages().
4) PiSmmCpu: Add protection detail in commit message.
5) UefiCpuPkg.dsc: Add page table footprint info in commit message.

 below is V1 description 
This series patch enables SMM page level protection.
Features are:
1) PiSmmCore reports SMM PE image code/data information
in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
and set XD for data page and RO for code page.
3) PiSmmCpu enables Static Paging for X64 according to
PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
is used as long as it is supported.
4) PiSmmCpu sets importance data structure to be read only,
such as Gdt, Idt, SmmEntrypoint, and PageTable itself.

tested platform:
1) Intel internal platform (X64).
2) EDKII Quark IA32
3) EDKII Vlv2  X64
4) EDKII OVMF IA32 and IA32X64. (with -smp 8)

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 

Jiewen Yao (6):
  MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
  MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
  MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
  UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
  UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
  QuarkPlatformPkg/dsc: enable Smm paging protection.

 MdeModulePkg/Core/PiSmmCore/Dispatcher.c   |   66 +
 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c| 1509 

 MdeModulePkg/Core/PiSmmCore/Page.c |  775 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c|   40 +
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   91 ++
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |2 +
 MdeModulePkg/Core/PiSmmCore/Pool.c |   16 +
 MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h |   51 +
 MdeModulePkg/MdeModulePkg.dec  |3 +
 QuarkPlatformPkg/Quark.dsc |6 +
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   71 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |   67 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|   68 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |   70 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S  |  226 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm|   36 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm   |   36 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c  |   37 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c|4 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  127 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |  142 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  156 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |5 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  871 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c |   39 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h |   15 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c|  274 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   |   51 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm |   54 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm|   61 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.S   |  250 +---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.asm |   35 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.nasm|   31 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c   |   30 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c |7 +-
 UefiCpuPkg/UefiCpuPkg.dec  |8 +
 36 files changed, 4529 insertions(+), 801 deletions(-)
 create mode 100644 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
 create mode 100644 MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h
 create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c

-- 
2.7.4.windows.1

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

[edk2] [PATCH V2 3/6] MdeModulePkg/PiSmmCore: Add MemoryAttributes support.

[Jiewen Yao]

1) This patch installs LoadedImage protocol to SMM
protocol database, so that the SMM image info can be
got easily to construct the PiSmmMemoryAttributes table.

This table is produced at SmmEndOfDxe event.
So that the consumer (PiSmmCpu) may consult this table
to set memory attribute in page table.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Core/PiSmmCore/Dispatcher.c|   66 +
 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 1509 
 MdeModulePkg/Core/PiSmmCore/Page.c  |  775 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c |   40 +
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h |   91 ++
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf   |2 +
 MdeModulePkg/Core/PiSmmCore/Pool.c  |   16 +
 7 files changed, 2473 insertions(+), 26 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c 
b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
index 87f4617..1bddaf1 100644
--- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
+++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
@@ -580,6 +580,11 @@ SmmLoadImage (
   DriverEntry->LoadedImage->SystemTable   = gST;
   DriverEntry->LoadedImage->DeviceHandle  = DeviceHandle;
 
+  DriverEntry->SmmLoadedImage.Revision = 
EFI_LOADED_IMAGE_PROTOCOL_REVISION;
+  DriverEntry->SmmLoadedImage.ParentHandle = 
gSmmCorePrivate->SmmIplImageHandle;
+  DriverEntry->SmmLoadedImage.SystemTable  = gST;
+  DriverEntry->SmmLoadedImage.DeviceHandle = DeviceHandle;
+
   //
   // Make an EfiBootServicesData buffer copy of FilePath
   //
@@ -599,6 +604,25 @@ SmmLoadImage (
   DriverEntry->LoadedImage->ImageDataType = EfiRuntimeServicesData;
 
   //
+  // Make a buffer copy of FilePath
+  //
+  Status = SmmAllocatePool (EfiRuntimeServicesData, 
GetDevicePathSize(FilePath), (VOID **)>SmmLoadedImage.FilePath);
+  if (EFI_ERROR (Status)) {
+if (Buffer != NULL) {
+  gBS->FreePool (Buffer);
+}
+gBS->FreePool (DriverEntry->LoadedImage->FilePath);
+SmmFreePages (DstBuffer, PageCount);
+return Status;
+  }
+  CopyMem (DriverEntry->SmmLoadedImage.FilePath, FilePath, 
GetDevicePathSize(FilePath));
+
+  DriverEntry->SmmLoadedImage.ImageBase = (VOID 
*)(UINTN)DriverEntry->ImageBuffer;
+  DriverEntry->SmmLoadedImage.ImageSize = ImageContext.ImageSize;
+  DriverEntry->SmmLoadedImage.ImageCodeType = EfiRuntimeServicesCode;
+  DriverEntry->SmmLoadedImage.ImageDataType = EfiRuntimeServicesData;
+
+  //
   // Create a new image handle in the UEFI handle database for the SMM Driver
   //
   DriverEntry->ImageHandle = NULL;
@@ -608,6 +632,17 @@ SmmLoadImage (
   NULL
   );
 
+  //
+  // Create a new image handle in the SMM handle database for the SMM Driver
+  //
+  DriverEntry->SmmImageHandle = NULL;
+  Status = SmmInstallProtocolInterface (
+ >SmmImageHandle,
+ ,
+ EFI_NATIVE_INTERFACE,
+ >SmmLoadedImage
+ );
+
   PERF_START (DriverEntry->ImageHandle, "LoadImage:", NULL, Tick);
   PERF_END (DriverEntry->ImageHandle, "LoadImage:", NULL, 0);
 
@@ -896,6 +931,16 @@ SmmDispatcher (
   }
   gBS->FreePool (DriverEntry->LoadedImage);
 }
+Status = SmmUninstallProtocolInterface (
+   DriverEntry->SmmImageHandle,
+   ,
+   >SmmLoadedImage
+   );
+if (!EFI_ERROR(Status)) {
+  if (DriverEntry->SmmLoadedImage.FilePath != NULL) {
+SmmFreePool (DriverEntry->SmmLoadedImage.FilePath);
+  }
+}
   }
 
   REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
@@ -1327,6 +1372,27 @@ SmmDriverDispatchHandler (
 
   mSmmCoreLoadedImage->DeviceHandle = FvHandle;
 }
+if (mSmmCoreDriverEntry->SmmLoadedImage.FilePath == NULL) {
+  //
+  // Maybe one special FV contains only one SMM_CORE module, so 
its device path must
+  // be initialized completely.
+  //
+  EfiInitializeFwVolDevicepathNode (, 
);
+  SetDevicePathEndNode ();
+
+  //
+  // Make a buffer copy FilePath
+  //
+  Status = SmmAllocatePool (
+ EfiRuntimeServicesData,
+ GetDevicePathSize ((EFI_DEVICE_PATH_PROTOCOL 
*)),
+ (VOID **)>SmmLoadedImage.FilePath
+ );
+  ASSERT_EFI_ERROR (Status);
+  CopyMem (mSmmCoreDriverEntry->SmmLoadedImage.FilePath, 
, GetDevicePathSize((EFI_DEVICE_PATH_PROTOCOL *)));
+
+  mSmmCoreDriverEntry->SmmLoadedImage.DeviceHandle = FvHandle;
+}
   } 

[edk2] [PATCH V2 4/6] UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.

[Jiewen Yao]

If enabled, SMM will not use on-demand paging.
SMM will build static page table for all memory.

The page table size depend on 2 things:
1) The 1G paging capability.
2) The whole system memory/MMIO addressing capability.

A) If the system only supports 2M paging,
When the whole memory/MMIO is 32bit, we only need 1+1+4=6 pages for 4G.
When the whole memory/MMIO is 39bit, we need 1+1+256 pages (~ 1M)
When the whole memory/MMIO is 48bit, we need 1+256+256*256 pages (~ 257M)

B) If the system supports 1G paging.
When the whole memory/MMIO is 32bit, we only need 1+1+4=6 pages for 4G.
(We still generate 2M page for maintenance consideration.)
When the whole memory/MMIO is 39bit, we still need 6 pages.
(We setup 1G paging for >1G.)
When the whole memory/MMIO is 48bit, we need 1+256 pages (~ 1M).

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 UefiCpuPkg/UefiCpuPkg.dec | 8 
 1 file changed, 8 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 8674533..a110820 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -199,6 +199,14 @@
   # @Prompt The specified AP target C-state for Mwait.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate|0|UINT8|0x0007
 
+  ## Indicates if SMM uses static page table.
+  #  If enabled, SMM will not use on-demand paging. SMM will build static page 
table for all memory.
+  #  This flag only impacts X64 build, because SMM alway builds static page 
table for IA32.
+  #   TRUE  - SMM uses static page table for all memory.
+  #   FALSE - SMM uses static page table for below 4G memory and use on-demand 
paging for above 4G memory.
+  # @Prompt Use static page table for all memory in SMM.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable|TRUE|BOOLEAN|0x3213210D
+
 [PcdsDynamic, PcdsDynamicEx]
   ## Contains the pointer to a CPU S3 data buffer of structure ACPI_CPU_DATA.
   # @Prompt The pointer to a CPU S3 data buffer.
-- 
2.7.4.windows.1

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

Re: [edk2] [PATCH v3] MdePkg/BaseMemoryLib*: check for zero length in ZeroMem ()

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Friday, November 04, 2016 5:05 PM
> To: edk2-devel@lists.01.org; Kinney, Michael D
> ; Gao, Liming 
> Cc: Carsey, Jaben ; ler...@redhat.com; Ard
> Biesheuvel 
> Subject: [edk2] [PATCH v3] MdePkg/BaseMemoryLib*: check for zero length
> in ZeroMem ()
> 
> Unlike other string functions in this library, ZeroMem () does not
> return early when the length of the input buffer is 0. So add the
> same to ZeroMem () as well, for all implementations of BaseMemoryLib
> living under MdePkg/
> 
> This fixes an issue with the ARM implementation of BaseMemoryLibOPtDxe,
> whose InternalMemZeroMem code does not expect a length of 0, and
> always
> writes at least a single byte.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> Acked-by: Laszlo Ersek 
> ---
>  MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c   | 6 +-
>  MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c| 6 +-
>  MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c | 6 +-
>  MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c | 6 +-
>  MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c | 6 +-
>  MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c   | 6 +-
>  MdePkg/Library/PeiMemoryLib/ZeroMemWrapper.c| 6 +-
>  MdePkg/Library/UefiMemoryLib/ZeroMemWrapper.c   | 6 +-
>  8 files changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
> b/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
> index 2a0a038fd6c5..9dd0b45e188e 100644
> --- a/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
> +++ b/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
> @@ -46,7 +46,11 @@ ZeroMem (
>IN UINTN  Length
>)
>  {
> -  ASSERT (!(Buffer == NULL && Length > 0));
> +  if (Length == 0) {
> +return Buffer;
> +  }
> +
> +  ASSERT (Buffer != NULL);
>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>return InternalMemZeroMem (Buffer, Length);
>  }
> diff --git a/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
> b/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
> index 2a0a038fd6c5..9dd0b45e188e 100644
> --- a/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
> +++ b/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
> @@ -46,7 +46,11 @@ ZeroMem (
>IN UINTN  Length
>)
>  {
> -  ASSERT (!(Buffer == NULL && Length > 0));
> +  if (Length == 0) {
> +return Buffer;
> +  }
> +
> +  ASSERT (Buffer != NULL);
>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>return InternalMemZeroMem (Buffer, Length);
>  }
> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
> b/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
> index 2a0a038fd6c5..9dd0b45e188e 100644
> --- a/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
> @@ -46,7 +46,11 @@ ZeroMem (
>IN UINTN  Length
>)
>  {
> -  ASSERT (!(Buffer == NULL && Length > 0));
> +  if (Length == 0) {
> +return Buffer;
> +  }
> +
> +  ASSERT (Buffer != NULL);
>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>return InternalMemZeroMem (Buffer, Length);
>  }
> diff --git a/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
> b/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
> index 2a0a038fd6c5..9dd0b45e188e 100644
> --- a/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
> +++ b/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
> @@ -46,7 +46,11 @@ ZeroMem (
>IN UINTN  Length
>)
>  {
> -  ASSERT (!(Buffer == NULL && Length > 0));
> +  if (Length == 0) {
> +return Buffer;
> +  }
> +
> +  ASSERT (Buffer != NULL);
>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>return InternalMemZeroMem (Buffer, Length);
>  }
> diff --git a/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
> b/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
> index 2a0a038fd6c5..9dd0b45e188e 100644
> --- a/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
> +++ b/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
> @@ -46,7 +46,11 @@ ZeroMem (
>IN UINTN  Length
>)
>  {
> -  ASSERT (!(Buffer == NULL && Length > 0));
> +  if (Length == 0) {
> +return Buffer;
> +  }
> +
> +  ASSERT (Buffer != NULL);
>ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
>return InternalMemZeroMem (Buffer, Length);
>  }
> diff --git a/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
> b/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
> index 2a0a038fd6c5..9dd0b45e188e 100644
> --- a/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
> +++ b/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
> @@ -46,7 +46,11 @@ ZeroMem (
>IN UINTN  Length
>)
>  {
> -  

Re: [edk2] SMBIOS/DMI data creation

[Ryan Harkin]

On 4 November 2016 at 07:22, Shaveta Leekha  wrote:
> Hi,
>
> I was adding SMBIOS DMI data in UEFI for our ARM64 platform.
> One patch that I referred is of ARM JUNO platform:
> ArmPlatformPkg/ArmJunoPkg: Create SMBIOS/DMI data   for Juno
>
> In it, almost all SMBIOS table entries/structures are populated statically 
> with information.
> Is it preliminary support?

I think so.


> can we fill most of the information like:
> Memory module information
> Cache related info etc by reading from registers or from system at 
> run time instead of filling it statically?
>

I think that would be the nicest way to do it.  Static configuration
isn't ideal, IMO.  But on a board like Juno, where the config isn't
likely to change, I think it's acceptable to hard code it; or at
least, it's better than doing nothing.

> Any views?
>
> Thanks and Regards,
> Shaveta
>
> ___
> 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 v3] MdePkg/BaseMemoryLib*: check for zero length in ZeroMem ()

[Ard Biesheuvel]

Unlike other string functions in this library, ZeroMem () does not
return early when the length of the input buffer is 0. So add the
same to ZeroMem () as well, for all implementations of BaseMemoryLib
living under MdePkg/

This fixes an issue with the ARM implementation of BaseMemoryLibOPtDxe,
whose InternalMemZeroMem code does not expect a length of 0, and always
writes at least a single byte.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Acked-by: Laszlo Ersek 
---
 MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c   | 6 +-
 MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c| 6 +-
 MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c | 6 +-
 MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c | 6 +-
 MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c | 6 +-
 MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c   | 6 +-
 MdePkg/Library/PeiMemoryLib/ZeroMemWrapper.c| 6 +-
 MdePkg/Library/UefiMemoryLib/ZeroMemWrapper.c   | 6 +-
 8 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c 
b/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
index 2a0a038fd6c5..9dd0b45e188e 100644
--- a/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
+++ b/MdePkg/Library/BaseMemoryLib/ZeroMemWrapper.c
@@ -46,7 +46,11 @@ ZeroMem (
   IN UINTN  Length
   )
 {
-  ASSERT (!(Buffer == NULL && Length > 0));
+  if (Length == 0) {
+return Buffer;
+  }
+
+  ASSERT (Buffer != NULL);
   ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
   return InternalMemZeroMem (Buffer, Length);
 }
diff --git a/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c 
b/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
index 2a0a038fd6c5..9dd0b45e188e 100644
--- a/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
+++ b/MdePkg/Library/BaseMemoryLibMmx/ZeroMemWrapper.c
@@ -46,7 +46,11 @@ ZeroMem (
   IN UINTN  Length
   )
 {
-  ASSERT (!(Buffer == NULL && Length > 0));
+  if (Length == 0) {
+return Buffer;
+  }
+
+  ASSERT (Buffer != NULL);
   ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
   return InternalMemZeroMem (Buffer, Length);
 }
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c 
b/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
index 2a0a038fd6c5..9dd0b45e188e 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/ZeroMemWrapper.c
@@ -46,7 +46,11 @@ ZeroMem (
   IN UINTN  Length
   )
 {
-  ASSERT (!(Buffer == NULL && Length > 0));
+  if (Length == 0) {
+return Buffer;
+  }
+
+  ASSERT (Buffer != NULL);
   ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
   return InternalMemZeroMem (Buffer, Length);
 }
diff --git a/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c 
b/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
index 2a0a038fd6c5..9dd0b45e188e 100644
--- a/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
+++ b/MdePkg/Library/BaseMemoryLibOptPei/ZeroMemWrapper.c
@@ -46,7 +46,11 @@ ZeroMem (
   IN UINTN  Length
   )
 {
-  ASSERT (!(Buffer == NULL && Length > 0));
+  if (Length == 0) {
+return Buffer;
+  }
+
+  ASSERT (Buffer != NULL);
   ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
   return InternalMemZeroMem (Buffer, Length);
 }
diff --git a/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c 
b/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
index 2a0a038fd6c5..9dd0b45e188e 100644
--- a/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
+++ b/MdePkg/Library/BaseMemoryLibRepStr/ZeroMemWrapper.c
@@ -46,7 +46,11 @@ ZeroMem (
   IN UINTN  Length
   )
 {
-  ASSERT (!(Buffer == NULL && Length > 0));
+  if (Length == 0) {
+return Buffer;
+  }
+
+  ASSERT (Buffer != NULL);
   ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
   return InternalMemZeroMem (Buffer, Length);
 }
diff --git a/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c 
b/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
index 2a0a038fd6c5..9dd0b45e188e 100644
--- a/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
+++ b/MdePkg/Library/BaseMemoryLibSse2/ZeroMemWrapper.c
@@ -46,7 +46,11 @@ ZeroMem (
   IN UINTN  Length
   )
 {
-  ASSERT (!(Buffer == NULL && Length > 0));
+  if (Length == 0) {
+return Buffer;
+  }
+
+  ASSERT (Buffer != NULL);
   ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
   return InternalMemZeroMem (Buffer, Length);
 }
diff --git a/MdePkg/Library/PeiMemoryLib/ZeroMemWrapper.c 
b/MdePkg/Library/PeiMemoryLib/ZeroMemWrapper.c
index 5adddbbfad66..a3aa7d10a689 100644
--- a/MdePkg/Library/PeiMemoryLib/ZeroMemWrapper.c
+++ b/MdePkg/Library/PeiMemoryLib/ZeroMemWrapper.c
@@ -46,7 +46,11 @@ ZeroMem (
   IN UINTN  Length
   )
 {
-  ASSERT (!(Buffer == NULL && Length > 0));
+  if (Length == 0) {
+return Buffer;
+  }
+
+  ASSERT (Buffer != NULL);
   ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
   return InternalMemZeroMem (Buffer, Length);
 }
diff --git 

[edk2] [PATCH] ShellPkg/reset: Support "-fwui" flag

[Ruiyu Ni]

From: Chen A Chen 

The patch adds "-fwui" support to reset command which is newly added
to Shell 2.2 spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen A Chen 
Reviewed-by: Ruiyu Ni 
Cc: Jaben Carsey 
---
 .../Library/UefiShellLevel2CommandsLib/Reset.c | 48 +++---
 .../UefiShellLevel2CommandsLib.h   |  1 +
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c 
b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
index 7d4cfb4..40ad8d9 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
@@ -2,7 +2,7 @@
   Main file for attrib shell level 2 function.
 
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2016, 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
@@ -16,10 +16,11 @@
 #include "UefiShellLevel2CommandsLib.h"
 
 STATIC CONST SHELL_PARAM_ITEM ResetParamList[] = {
-  {L"-w", TypeValue},
-  {L"-s", TypeValue},
-  {L"-c", TypeValue},
-  {NULL, TypeMax}
+  {L"-w",TypeValue},
+  {L"-s",TypeValue},
+  {L"-c",TypeValue},
+  {L"-fwui", TypeFlag },
+  {NULL, TypeMax  }
   };
 
 /**
@@ -40,6 +41,9 @@ ShellCommandRunReset (
   CONST CHAR16  *String;
   CHAR16*ProblemParam;
   SHELL_STATUS  ShellStatus;
+  UINT64OsIndications;
+  UINT32Attr;
+  UINTN DataSize;
 
   ShellStatus = SHELL_SUCCESS;
   ProblemParam = NULL;
@@ -72,6 +76,39 @@ ShellCommandRunReset (
   ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), 
gShellLevel2HiiHandle, L"reset");  
   ShellStatus = SHELL_INVALID_PARAMETER;
 } else {
+
+  if (ShellCommandLineGetFlag (Package, L"-fwui")) {
+
+DataSize  = sizeof (OsIndications);
+Status = gRT->GetVariable (
+EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME, 
,
+, , 
+);
+if (!EFI_ERROR (Status)) {
+  if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
+DataSize = sizeof (OsIndications);
+Status = gRT->GetVariable (
+EFI_OS_INDICATIONS_VARIABLE_NAME, 
,
+, , 
+);
+if (!EFI_ERROR (Status)) {
+  OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
+} else {
+  OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
+}
+Status = gRT->SetVariable (
+EFI_OS_INDICATIONS_VARIABLE_NAME, 
,
+EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+sizeof (OsIndications), 
+);
+  }
+}
+if (EFI_ERROR (Status)) {
+  ShellStatus = SHELL_UNSUPPORTED;
+  goto Error;
+}
+  }
+
   //
   // check for warm reset flag, then shutdown reset flag, then cold 
(default) reset flag
   //
@@ -119,6 +156,7 @@ ShellCommandRunReset (
   // as the ResetSystem function should not return...
   //
 
+Error:
   //
   // free the command line package
   //
diff --git 
a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h 
b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h
index c262bb5..857487f 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h
@@ -25,6 +25,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include 
-- 
2.9.0.windows.1

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

[edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported

[Jeff Fan]

If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at all
and needn't to register callback functions.

It could improve boot performance on single supported system.

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

Cc: Feng Tian 
Cc: Liming Gao 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c |  7 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.c| 39 +++--
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c |  8 +++
 3 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index b399f1c..eb36d6f 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -290,6 +290,13 @@ InitMpGlobalData (
 
   SaveCpuMpData (CpuMpData);
 
+  if (CpuMpData->CpuCount == 1) {
+//
+// If only BSP exists, return
+//
+return;
+  }
+
   //
   // Avoid APs access invalid buff data which allocated by BootServices,
   // so we will allocate reserved data for AP loop code.
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 56b870e..a0edc55 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1143,6 +1143,7 @@ MpInitLibInitialize (
   } else {
 MaxLogicalProcessorNumber = OldCpuMpData->CpuCount;
   }
+  ASSERT (MaxLogicalProcessorNumber != 0);
 
   AsmGetAddressMap ();
   ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof 
(MP_CPU_EXCHANGE_INFO);
@@ -1209,10 +1210,12 @@ MpInitLibInitialize (
   MtrrGetAllMtrrs (>MtrrTable);
 
   if (OldCpuMpData == NULL) {
-//
-// Wakeup all APs and calculate the processor count in system
-//
-CollectProcessorCount (CpuMpData);
+if (MaxLogicalProcessorNumber > 1) {
+  //
+  // Wakeup all APs and calculate the processor count in system
+  //
+  CollectProcessorCount (CpuMpData);
+}
   } else {
 //
 // APs have been wakeup before, just get the CPU Information
@@ -1238,19 +1241,21 @@ MpInitLibInitialize (
 sizeof (CPU_VOLATILE_REGISTERS)
 );
 }
-//
-// Wakeup APs to do some AP initialize sync
-//
-WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData);
-//
-// Wait for all APs finished initialization
-//
-while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
-  CpuPause ();
-}
-CpuMpData->InitFlag = ApInitDone;
-for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
-  SetApState (>CpuData[Index], CpuStateIdle);
+if (MaxLogicalProcessorNumber > 1) {
+  //
+  // Wakeup APs to do some AP initialize sync
+  //
+  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData);
+  //
+  // Wait for all APs finished initialization
+  //
+  while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
+CpuPause ();
+  }
+  CpuMpData->InitFlag = ApInitDone;
+  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+SetApState (>CpuData[Index], CpuStateIdle);
+  }
 }
   }
 
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index e242d37..1f2fcb8 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -321,6 +321,14 @@ InitMpGlobalData (
   EFI_STATUS  Status;
 
   SaveCpuMpData (CpuMpData);
+
+  if (CpuMpData->CpuCount == 1) {
+//
+// If only BSP exists, return
+//
+return;
+  }
+
   //
   // Register an event for EndOfPei
   //
-- 
2.9.3.windows.2

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

[edk2] SMBIOS/DMI data creation

[Shaveta Leekha]

Hi,

I was adding SMBIOS DMI data in UEFI for our ARM64 platform.
One patch that I referred is of ARM JUNO platform:
ArmPlatformPkg/ArmJunoPkg: Create SMBIOS/DMI data   for Juno

In it, almost all SMBIOS table entries/structures are populated statically with 
information.
Is it preliminary support? can we fill most of the information like:
Memory module information
Cache related info etc by reading from registers or from system at run 
time instead of filling it statically?

Any views?

Thanks and Regards,
Shaveta

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

Re: [edk2] [PATCH] BaseTools/Edk2Setup.bat: Fix build errors from VS tools PREFIX ENV missing

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong

-Original Message-
From: Cinnamon Shia [mailto:cinnamon.s...@hpe.com] 
Sent: Friday, November 4, 2016 10:20 AM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong ; Gao, Liming ; 
Cinnamon Shia 
Subject: [PATCH] BaseTools/Edk2Setup.bat: Fix build errors from VS tools PREFIX 
ENV missing

BaseTools/set_vsprefix_envs.bat is introduced for setting the PREFIX ENV of VS 
tools in tools_def.template.
For example:

DEFINE VS2015_BIN  = ENV(VS2015_PREFIX)Vc\bin
DEFINE VS2015_DLL  = ENV(VS2015_PREFIX)Common7\IDE;DEF(VS2015_BIN)
DEFINE VS2015_BINX64   = DEF(VS2015_BIN)\x86_amd64

The issue is EdkSetup.bat calls BaseTools\set_vsprefix_envs.bat but 
Edk2Setup.bat does not.

Edk2Setup.bat should call BaseTools/set_vsprefix_envs.bat  to set up the PREFIX 
ENV of VS tools.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
 Edk2Setup.bat | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Edk2Setup.bat b/Edk2Setup.bat index 68f46dc..017e88d 100755
--- a/Edk2Setup.bat
+++ b/Edk2Setup.bat
@@ -355,6 +355,15 @@
 @if defined REBUILD_TOOLS goto SetConf
 @if defined SVN_PULL goto SetConf
 
+@REM call set_vsprefix_envs.bat to set up the PREFIX env for VS tool path.
+@IF NOT exist "%EDK_TOOLS_PATH%\set_vsprefix_envs.bat" (
+  @echo.
+  @echo !!! ERROR !!! The set_vsprefix_envs.bat was not found !!!
+  @echo.
+  @goto ExitFailure
+)
+@call %EDK_TOOLS_PATH%\set_vsprefix_envs.bat
+
 @echo.
 @echo Rebuilding of the tools is not required. Binaries of the latest,  @echo 
tested versions of the tools have been tested and included in the
--
2.10.0.windows.1

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

Re: [edk2] [PATCH 0/4] Defer 3rd party images loading to after EndOfDxe

[Ni, Ruiyu]

No.
The open source platform patch will be sent out later.

Regards,
Ray

From: Gao, Liming
Sent: Friday, November 4, 2016 1:14 PM
To: Ni, Ruiyu ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH 0/4] Defer 3rd party images loading to after EndOfDxe

Ray:
  Seemly, PlatformBdsLib library instance should call 
EfiBootManagerDispatchDeferredImages(), right? Are there patches to update 
PlatformBdsLib library instance?

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Friday, November 04, 2016 9:00 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/4] Defer 3rd party images loading to after
> EndOfDxe
>
> The patches change the default image loading policy by deferring 3rd
> party images loading to after EndOfDxe and add a new BDS API to
> dispatch the deferred images.
>
> Platform needs to call the new BDS API
> EfiBootManagerDispatchDeferredImages after EndOfDxe to ensure that
> any deferred images are loaded.
>
> Ruiyu Ni (4):
>   MdeModulePkg/SecurityStubDxe: Defer 3rd party image before EndOfDxe
>   MdeModulePkg/UefiBootManager: Add
> EfiBootManagerDispatchDeferredImages
>   MdeModulePkg/BdsDxe: Check deferred images before booting to OS
>   MdeModulePkg/SecurityStubDxe: Report failure if image is load earlier
>
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  13 +
>  MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 113 ++
>  .../Library/UefiBootManagerLib/InternalBm.h|   1 +
>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
>  MdeModulePkg/Universal/BdsDxe/Bds.h|   4 +-
>  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf   |   2 +
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c   |  89 +
>  .../SecurityStubDxe/Defer3rdPartyImageLoad.c   | 413
> +
>  .../SecurityStubDxe/Defer3rdPartyImageLoad.h   |  95 +
>  .../Universal/SecurityStubDxe/SecurityStub.c   |  14 +-
>  .../Universal/SecurityStubDxe/SecurityStubDxe.inf  |  11 +-
>  11 files changed, 753 insertions(+), 3 deletions(-)
>  create mode 100644
> MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c
>  create mode 100644
> MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.h
>
> --
> 2.9.0.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel