Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

[Sakar Arora]

The DXE core dispatcher relies on the available memory to allocate space for 
loading the rest of the modules from the UEFI image. If we declare the UEFI 
image memory space (from which DXE dispatcher reads the efi modules) open to 
allocation, it might lead to data corruption, depending on the location of UEFI 
image and cumulative size of uncompressed EFI modules.

Also, since UEFI allows unloading and loading of drivers at runtime, IMO, there 
is a need to preserve the UEFI image even after all the modules have been 
decompressed and loaded in the boot sequence.

-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Tuesday, September 19, 2017 6:18 PM
To: Sakar Arora 
Cc: Meenakshi Aggarwal ; edk2-devel@lists.01.org; 
leif.lindh...@linaro.org
Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

On 19 September 2017 at 01:07, Sakar Arora  wrote:
> This change will create the possibility for memory space holding the UEFI 
> image to be over-written by the DXE core code, since this space will then be 
> available for allocation.

Yes. But why does this matter after the entire payload has been decompressed 
into memory already?


> Any such change, if required, should be done just before booting the OS.
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Meenakshi Aggarwal
> Sent: Tuesday, September 19, 2017 6:02 PM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org;
> ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
>
> While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
> whereas this memory can be used by OS.
>
> This patch, allows OS to use UEFI code area.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69
> -
>  1 file changed, 69 deletions(-)
>
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index 2feb11f..d03214b 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -70,11 +70,7 @@ MemoryPeim (
>  {
>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> -  UINT64   ResourceLength;
>EFI_PEI_HOB_POINTERS NextHob;
> -  EFI_PHYSICAL_ADDRESS FdTop;
> -  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> -  EFI_PHYSICAL_ADDRESS ResourceTop;
>BOOLEAN  Found;
>
>// Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6 @@ 
> MemoryPeim (
>  );
>}
>
> -  //
> -  // Reserved the memory space occupied by the firmware volume
> -  //
> -
> -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemorySize);
> -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> -
> -  // EDK2 does not have the concept of boot firmware copied into
> DRAM. To avoid the DXE
> -  // core to overwrite this area we must mark the region with the
> attribute non-present
> -  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && 
> (FdTop <= SystemMemoryTop)) {
> -Found = FALSE;
> -
> -// Search for System Memory Hob that contains the firmware
> -NextHob.Raw = GetHobList ();
> -while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, 
> NextHob.Raw)) != NULL) {
> -  if ((NextHob.ResourceDescriptor->ResourceType == 
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> -  (PcdGet64 (PcdFdBaseAddress) >= 
> NextHob.ResourceDescriptor->PhysicalStart) &&
> -  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + 
> NextHob.ResourceDescriptor->ResourceLength))
> -  {
> -ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
> -ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> -ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + 
> ResourceLength;
> -
> -if (PcdGet64 (PcdFdBaseAddress) == 
> NextHob.ResourceDescriptor->PhysicalStart) {
> -  if (SystemMemoryTop == FdTop) {
> -NextHob.ResourceDescriptor->ResourceAttribute = 
> ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
> -  } else {
> -// Create the System Memory HOB for the firmware with the 
> non-present attribute
> -BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -ResourceAttributes & 
> ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
> -PcdGet64 (PcdFdBaseAddress),
> -

Re: [edk2] Storing Non volatile variables on SD/NAND

[Andrew Fish]

> On Sep 19, 2017, at 10:09 PM, Udit Kumar  wrote:
> 
>>> On Sep 19, 2017, at 9:27 PM, Udit Kumar  wrote:
>>> 
>>> 
 On 18 September 2017 at 22:28, Udit Kumar  wrote:
> Thanks Vladimir,
> With your design, you did delayed write to eMMC due to sharing with
> OS.  But it works for you:) Say if eMMC controllers offers you a
> status bit, if eMMC storage is being used for not. Then this could
> be possible to
 update at run time, both OS/UEFI needs to check and wait if
 controller is being used.
 
 That is the problem right there. The nice thing about a firmware spec
 is that you don't have to care about how it was implemented if you adhere 
 to
>> the API rules.
>>> 
>>> Yup, we are fine as long as long UEFI firmware is stored on dedicated media.
>>> 
 Imposing additional restrictions (such as requiring the OS to be
 careful about not using the eMMC when it may be in use by the
 firmware) defeats the purpose of using UEFI, since you won't be able to 
 use a
>> generic OS anyway.
 
>>> 
>>> Hmm,  so far, I haven't come across where UEFI specs says, we need a
>>> separate Storage for firmware. (May be I missed some part of specs)
>>> Irrespective of storage media, we have this problem if OS and UEFI
>>> shares same storage.
>>> 
>> 
>> Udit,
>> 
>> Can you point out the spec that states you can't boot Linux and Windows at 
>> the
>> same time on a PC? :)
>> 
>> When you write a spec it is not practical do document what is not possible, 
>> you
>> can only document the API the rest is implied by the implementation. So for
>> example the UEFI spec does not document why the firmware and OS can't share
>> a hardware device, just like you can't have 2 operating systems running on 
>> bare
>> metal at the same time. It is a little like Occam's Razor the reason that the
>> firmware and the OS can not share a hardware device is the mechanics of how
>> to share a hardware device is not defined in the spec, thus it is not part 
>> of the
>> API and not possible.
> 
> Right,  This is left on implementation how to put firmware and OS.
> Ideally, keeping both storage separate  is best case, no need to sync between 
> two.
> 
> My reply to Ard, was to highlight that in any case (NOR or eMMC /NAND)
> if we are keeping OS and firmware on same storage, we will have same 
> issue not limited to  eMMC.
> 
> For some requirement, if we need to keep firmware and OS on same media, 
> Then implementation should make sure there is exclusive access (be it
> NOR controller, SD controller etc). 
> 

Udit,

Sorry I'm a little swamped on my email right now and might be a little behind 
on the thread

Yea the only way to realistically Implement an EFI runtime service in UEFI is 
to have UEFI own the hardware device. There is no architecture for sharing the 
device, and the type of device is not really relevant. 

Thanks,

Andrew Fish

> Thanks
> Udit
> 
>> Thanks,
>> 
>> Andrew Fish
>> 
> For sure,  some synchronization issues need to be ironed out (or
> maybe I am
 just dreaming here).
> 
> On part 2) where you forked VariableRuntime driver , could we think
> of updating VariableRuntime driver, to support non-XIP or memory
> mapped
 devices.
> 
 
 I think being able to support non-memorymapped FV volumes for the
 variable store would be a big improvement. This does require changes
 to both the FaultTolerantWrite drivers and the VariableRuntime
 drivers, which both appear in PEI, DXE and SMM flavors, and require
 thorough review due to the security impact bugs have in this layer, so 
 this is a
>> rather large chunk of work to take on.
>>> 
>>> Thanks,  your list is longer than what I was thinking :-) I think, for
>>> embedded world with UEFI, later or sooner, this will be required.
>>> 
>>> Thanks
>>> Udit
>>> ___
>>> 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 0/2] Fixe out-of-sync issue between GCD and CPU driver

[Wang, Jian J]

Jiewen,

Any comments on this patch?


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jian J 
Wang
Sent: Tuesday, September 19, 2017 2:10 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Laszlo Ersek 
; Yao, Jiewen ; Dong, Eric 
; Zeng, Star 
Subject: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

There're two issues here actually.

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't
accept page related attributes. That means users cannot use it to
change page attributes, and have to turn to CPU arch protocol to do it,
which is not be allowed by PI spec.

>From CpuDxe driver perspective, it doesn't update GCD memory attributes
from current page table setup during its initialization. So the memory
attributes in GCD might not reflect all memory attributes in real world.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Michael Kinney 
Suggested-by: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 

Jian J Wang (2):
  UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
  MdeModulePkg/Core: Fix out-of-sync issue in GCD

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++
 UefiCpuPkg/CpuDxe/CpuDxe.c   |  5 ++
 UefiCpuPkg/CpuDxe/CpuDxe.h   |  9 
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 99 
 4 files changed, 140 insertions(+), 18 deletions(-)

-- 
2.14.1.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

Re: [edk2] Storing Non volatile variables on SD/NAND

[Udit Kumar]

> > On Sep 19, 2017, at 9:27 PM, Udit Kumar  wrote:
> >
> >
> >> On 18 September 2017 at 22:28, Udit Kumar  wrote:
> >>> Thanks Vladimir,
> >>> With your design, you did delayed write to eMMC due to sharing with
> >>> OS.  But it works for you:) Say if eMMC controllers offers you a
> >>> status bit, if eMMC storage is being used for not. Then this could
> >>> be possible to
> >> update at run time, both OS/UEFI needs to check and wait if
> >> controller is being used.
> >>
> >> That is the problem right there. The nice thing about a firmware spec
> >> is that you don't have to care about how it was implemented if you adhere 
> >> to
> the API rules.
> >
> > Yup, we are fine as long as long UEFI firmware is stored on dedicated media.
> >
> >> Imposing additional restrictions (such as requiring the OS to be
> >> careful about not using the eMMC when it may be in use by the
> >> firmware) defeats the purpose of using UEFI, since you won't be able to 
> >> use a
> generic OS anyway.
> >>
> >
> > Hmm,  so far, I haven't come across where UEFI specs says, we need a
> > separate Storage for firmware. (May be I missed some part of specs)
> > Irrespective of storage media, we have this problem if OS and UEFI
> > shares same storage.
> >
> 
> Udit,
> 
> Can you point out the spec that states you can't boot Linux and Windows at the
> same time on a PC? :)
> 
> When you write a spec it is not practical do document what is not possible, 
> you
> can only document the API the rest is implied by the implementation. So for
> example the UEFI spec does not document why the firmware and OS can't share
> a hardware device, just like you can't have 2 operating systems running on 
> bare
> metal at the same time. It is a little like Occam's Razor the reason that the
> firmware and the OS can not share a hardware device is the mechanics of how
> to share a hardware device is not defined in the spec, thus it is not part of 
> the
> API and not possible.

Right,  This is left on implementation how to put firmware and OS.
Ideally, keeping both storage separate  is best case, no need to sync between 
two.

My reply to Ard, was to highlight that in any case (NOR or eMMC /NAND)
if we are keeping OS and firmware on same storage, we will have same 
issue not limited to  eMMC.

For some requirement, if we need to keep firmware and OS on same media, 
Then implementation should make sure there is exclusive access (be it
NOR controller, SD controller etc). 

Thanks
Udit

> Thanks,
> 
> Andrew Fish
> 
> >>> For sure,  some synchronization issues need to be ironed out (or
> >>> maybe I am
> >> just dreaming here).
> >>>
> >>> On part 2) where you forked VariableRuntime driver , could we think
> >>> of updating VariableRuntime driver, to support non-XIP or memory
> >>> mapped
> >> devices.
> >>>
> >>
> >> I think being able to support non-memorymapped FV volumes for the
> >> variable store would be a big improvement. This does require changes
> >> to both the FaultTolerantWrite drivers and the VariableRuntime
> >> drivers, which both appear in PEI, DXE and SMM flavors, and require
> >> thorough review due to the security impact bugs have in this layer, so 
> >> this is a
> rather large chunk of work to take on.
> >
> > Thanks,  your list is longer than what I was thinking :-) I think, for
> > embedded world with UEFI, later or sooner, this will be required.
> >
> > Thanks
> > Udit
> > ___
> > 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 0/2] Correct PI EfiGcdMemoryTypePersistent definition

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Wednesday, September 20, 2017 12:03 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/2] Correct PI EfiGcdMemoryTypePersistent definition

Liming Gao (2):
  MdePkg: Correct EfiGcdMemoryTypePersistent name to follow PI spec
  MdeModulePkg: Update DxeCore to consume PI EfiGcdMemoryTypePersistent

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 4 ++--  MdeModulePkg/Core/Dxe/Mem/Page.c | 
4 ++--
 MdePkg/Include/Pi/PiDxeCis.h | 6 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

--
2.11.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

Re: [edk2] Storing Non volatile variables on SD/NAND

[Andrew Fish]

> On Sep 19, 2017, at 9:27 PM, Udit Kumar  wrote:
> 
> 
>> On 18 September 2017 at 22:28, Udit Kumar  wrote:
>>> Thanks Vladimir,
>>> With your design, you did delayed write to eMMC due to sharing with
>>> OS.  But it works for you:) Say if eMMC controllers offers you a
>>> status bit, if eMMC storage is being used for not. Then this could be 
>>> possible to
>> update at run time, both OS/UEFI needs to check and wait if controller is 
>> being
>> used.
>> 
>> That is the problem right there. The nice thing about a firmware spec is 
>> that you
>> don't have to care about how it was implemented if you adhere to the API 
>> rules.
> 
> Yup, we are fine as long as long UEFI firmware is stored on dedicated media. 
> 
>> Imposing additional restrictions (such as requiring the OS to be careful 
>> about not
>> using the eMMC when it may be in use by the firmware) defeats the purpose of
>> using UEFI, since you won't be able to use a generic OS anyway.
>> 
> 
> Hmm,  so far, I haven't come across where UEFI specs says, we need a separate
> Storage for firmware. (May be I missed some part of specs)
> Irrespective of storage media, we have this problem if OS and UEFI shares 
> same  
> storage. 
> 

Udit,

Can you point out the spec that states you can't boot Linux and Windows at the 
same time on a PC? :)

When you write a spec it is not practical do document what is not possible, you 
can only document the API the rest is implied by the implementation. So for 
example the UEFI spec does not document why the firmware and OS can't share a 
hardware device, just like you can't have 2 operating systems running on bare 
metal at the same time. It is a little like Occam's Razor the reason that the 
firmware and the OS can not share a hardware device is the mechanics of how to 
share a hardware device is not defined in the spec, thus it is not part of the 
API and not possible. 

Thanks,

Andrew Fish

>>> For sure,  some synchronization issues need to be ironed out (or maybe I am
>> just dreaming here).
>>> 
>>> On part 2) where you forked VariableRuntime driver , could we think of
>>> updating VariableRuntime driver, to support non-XIP or memory mapped
>> devices.
>>> 
>> 
>> I think being able to support non-memorymapped FV volumes for the variable
>> store would be a big improvement. This does require changes to both the
>> FaultTolerantWrite drivers and the VariableRuntime drivers, which both appear
>> in PEI, DXE and SMM flavors, and require thorough review due to the security
>> impact bugs have in this layer, so this is a rather large chunk of work to 
>> take on.
> 
> Thanks,  your list is longer than what I was thinking :-)
> I think, for embedded world with UEFI, later or sooner, this will be required.
> 
> Thanks
> Udit
> ___
> 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] Storing Non volatile variables on SD/NAND

[Udit Kumar]

> On 18 September 2017 at 22:28, Udit Kumar  wrote:
> > Thanks Vladimir,
> > With your design, you did delayed write to eMMC due to sharing with
> > OS.  But it works for you:) Say if eMMC controllers offers you a
> > status bit, if eMMC storage is being used for not. Then this could be 
> > possible to
> update at run time, both OS/UEFI needs to check and wait if controller is 
> being
> used.
> 
> That is the problem right there. The nice thing about a firmware spec is that 
> you
> don't have to care about how it was implemented if you adhere to the API 
> rules.

Yup, we are fine as long as long UEFI firmware is stored on dedicated media. 

> Imposing additional restrictions (such as requiring the OS to be careful 
> about not
> using the eMMC when it may be in use by the firmware) defeats the purpose of
> using UEFI, since you won't be able to use a generic OS anyway.
> 

Hmm,  so far, I haven't come across where UEFI specs says, we need a separate
Storage for firmware. (May be I missed some part of specs)
Irrespective of storage media, we have this problem if OS and UEFI shares same  
storage. 

> > For sure,  some synchronization issues need to be ironed out (or maybe I am
> just dreaming here).
> >
> > On part 2) where you forked VariableRuntime driver , could we think of
> > updating VariableRuntime driver, to support non-XIP or memory mapped
> devices.
> >
> 
> I think being able to support non-memorymapped FV volumes for the variable
> store would be a big improvement. This does require changes to both the
> FaultTolerantWrite drivers and the VariableRuntime drivers, which both appear
> in PEI, DXE and SMM flavors, and require thorough review due to the security
> impact bugs have in this layer, so this is a rather large chunk of work to 
> take on.

Thanks,  your list is longer than what I was thinking :-)
I think, for embedded world with UEFI, later or sooner, this will be required.

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

[edk2] [Patch 0/2] Correct PI EfiGcdMemoryTypePersistent definition

[Liming Gao]

Liming Gao (2):
  MdePkg: Correct EfiGcdMemoryTypePersistent name to follow PI spec
  MdeModulePkg: Update DxeCore to consume PI EfiGcdMemoryTypePersistent

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 4 ++--
 MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++--
 MdePkg/Include/Pi/PiDxeCis.h | 6 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.11.0.windows.1

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

[edk2] [Patch 1/2] MdePkg: Correct EfiGcdMemoryTypePersistent name to follow PI spec

[Liming Gao]

PI spec defines EfiGcdMemoryTypePersistent name, MdePkg uses
EfiGcdMemoryTypePersistentMemory name. So, EfiGcdMemoryTypePersistent
is added. And, EfiGcdMemoryTypePersistentMemory is kept for compatility.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 MdePkg/Include/Pi/PiDxeCis.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Pi/PiDxeCis.h b/MdePkg/Include/Pi/PiDxeCis.h
index 3292809851..079dd3eab1 100644
--- a/MdePkg/Include/Pi/PiDxeCis.h
+++ b/MdePkg/Include/Pi/PiDxeCis.h
@@ -52,7 +52,11 @@ typedef enum {
   /// A memory region that is visible to the boot processor. 
   /// This memory supports byte-addressable non-volatility. 
   ///
-  EfiGcdMemoryTypePersistentMemory,
+  EfiGcdMemoryTypePersistent,
+  //
+  // Keep original one for the compatibility.
+  //
+  EfiGcdMemoryTypePersistentMemory = EfiGcdMemoryTypePersistent,
   ///
   /// A memory region that provides higher reliability relative to other 
memory in the
   /// system. If all memory has the same reliability, then this bit is not 
used.
-- 
2.11.0.windows.1

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

[edk2] [Patch 2/2] MdeModulePkg: Update DxeCore to consume PI EfiGcdMemoryTypePersistent

[Liming Gao]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 4 ++--
 MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index a06f8bb77c..4f03b3e408 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -108,7 +108,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 
*mGcdMemoryTypeNames[] = {
   "Reserved ",  // EfiGcdMemoryTypeReserved
   "SystemMem",  // EfiGcdMemoryTypeSystemMemory
   "MMIO ",  // EfiGcdMemoryTypeMemoryMappedIo
-  "PersisMem",  // EfiGcdMemoryTypePersistentMemory
+  "PersisMem",  // EfiGcdMemoryTypePersistent
   "MoreRelia",  // EfiGcdMemoryTypeMoreReliable
   "Unknown  "   // EfiGcdMemoryTypeMaximum
 };
@@ -2407,7 +2407,7 @@ CoreInitializeGcdServices (
   GcdMemoryType = EfiGcdMemoryTypeReserved;
 }
 if ((ResourceHob->ResourceAttribute & 
EFI_RESOURCE_ATTRIBUTE_PERSISTENT) == EFI_RESOURCE_ATTRIBUTE_PERSISTENT) {
-  GcdMemoryType = EfiGcdMemoryTypePersistentMemory;
+  GcdMemoryType = EfiGcdMemoryTypePersistent;
 }
 break;
   case EFI_RESOURCE_MEMORY_MAPPED_IO:
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index a142c79ee2..3dd6d1b4a0 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1635,7 +1635,7 @@ CoreGetMemoryMap (
   NumberOfEntries = 0;
   for (Link = mGcdMemorySpaceMap.ForwardLink; Link !=  
Link = Link->ForwardLink) {
 GcdMapEntry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE);
-if ((GcdMapEntry->GcdMemoryType == EfiGcdMemoryTypePersistentMemory) || 
+if ((GcdMapEntry->GcdMemoryType == EfiGcdMemoryTypePersistent) || 
 (GcdMapEntry->GcdMemoryType == EfiGcdMemoryTypeReserved) ||
 ((GcdMapEntry->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo) &&
 ((GcdMapEntry->Attributes & EFI_MEMORY_RUNTIME) == 
EFI_MEMORY_RUNTIME))) {
@@ -1783,7 +1783,7 @@ CoreGetMemoryMap (
   MemoryMap = MergeMemoryMapDescriptor (MemoryMapStart, MemoryMap, Size);
 }
 
-if (MergeGcdMapEntry.GcdMemoryType == EfiGcdMemoryTypePersistentMemory) {
+if (MergeGcdMapEntry.GcdMemoryType == EfiGcdMemoryTypePersistent) {
   //
   // Page Align GCD range is required. When it is converted to 
EFI_MEMORY_DESCRIPTOR, 
   // it will be recorded as page PhysicalStart and NumberOfPages. 
-- 
2.11.0.windows.1

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

[edk2] [Patch] MdeModulePkg/DxeNetLib: Check the actual packet size before trim data from Nbuf.

[Fu Siyuan]

In NetbufTrim() function, the NetBuf TotalSize should be checked with 0 before
making the trim operation, otherwise the function will fall into infinite loop.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
Cc: Michael Turner 
---
 MdeModulePkg/Library/DxeNetLib/NetBuffer.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c 
b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
index 95cb71714b..25fc78e49e 100644
--- a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
+++ b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
@@ -1175,6 +1175,10 @@ NetbufTrim (
 
   NET_CHECK_SIGNATURE (Nbuf, NET_BUF_SIGNATURE);
 
+  if (Nbuf->TotalSize == 0) {
+return 0;
+  }
+
   if (Len > Nbuf->TotalSize) {
 Len = Nbuf->TotalSize;
   }
-- 
2.13.0.windows.1

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

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Correct Names.

[lushifex]

Correct Boot Option Names.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: lushifex 
---
 .../Include/Library/GenericBdsLib.h|  5 ++-
 .../Library/GenericBdsLib/BdsBoot.c| 51 +-
 .../Library/GenericBdsLib/GenericBdsStrings.uni|  8 +++-
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Core/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h 
b/Core/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
index c338c4d..fa59235 100644
--- a/Core/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
+++ b/Core/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
@@ -4,7 +4,7 @@
 2) BDS boot device connect interface.
 3) BDS Misc interfaces for mainting boot variable, ouput string.
 
-Copyright (c) 2004 - 2013, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under 
 the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
@@ -821,6 +821,9 @@ SetupResetReminder (
 #define  BDS_EFI_MESSAGE_USB_DEVICE_BOOT  0x0305 // Type 03; Sub-Type 05
 #define  BDS_EFI_MESSAGE_SATA_BOOT0x0312 // Type 03; Sub-Type 18
 #define  BDS_EFI_MESSAGE_MAC_BOOT 0x030b // Type 03; Sub-Type 11
+#define  BDS_EFI_MESSAGE_NVME_BOOT0x0317 // Type 03; Sub-Type 17
+#define  BDS_EFI_MESSAGE_SD_BOOT  0x031a // Type 03; Sub-Type 1a
+#define  BDS_EFI_MESSAGE_EMMC_BOOT0x031d // Type 03; Sub-Type 1d
 #define  BDS_EFI_MESSAGE_MISC_BOOT0x03FF
 
 ///
diff --git a/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c 
b/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
index ce1dd4a..41611cf 100644
--- a/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
+++ b/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
@@ -3123,6 +3123,9 @@ BdsLibEnumerateAllBootOption (
   UINT16UsbNumber;
   UINT16MiscNumber;
   UINT16ScsiNumber;
+  UINT16NvmeNumber;
+  UINT16SdNumber;
+  UINT16EmmcNumber;
   UINT16NonBlockNumber;
   UINTN NumberBlockIoHandles;
   EFI_HANDLE*BlockIoHandles;
@@ -3157,6 +3160,9 @@ BdsLibEnumerateAllBootOption (
   UsbNumber   = 0;
   MiscNumber  = 0;
   ScsiNumber  = 0;
+  NvmeNumber  = 0;
+  SdNumber= 0;
+  EmmcNumber  = 0;
   PlatLang= NULL;
   LastLang= NULL;
   ZeroMem (Buffer, sizeof (Buffer));
@@ -3299,6 +3305,36 @@ BdsLibEnumerateAllBootOption (
 ScsiNumber++;
 break;
 
+  case BDS_EFI_MESSAGE_NVME_BOOT:
+if (NvmeNumber != 0) {
+  UnicodeSPrint (Buffer, sizeof (Buffer), L"%s %d", 
BdsLibGetStringById (STRING_TOKEN (STR_DESCRIPTION_NVME)), NvmeNumber);
+} else {
+  UnicodeSPrint (Buffer, sizeof (Buffer), L"%s", BdsLibGetStringById 
(STRING_TOKEN (STR_DESCRIPTION_NVME)));
+}
+BdsLibBuildOptionFromHandle (BlockIoHandles[Index], BdsBootOptionList, 
Buffer);
+NvmeNumber++;
+break;
+
+  case BDS_EFI_MESSAGE_SD_BOOT:
+if (SdNumber != 0) {
+  UnicodeSPrint (Buffer, sizeof (Buffer), L"%s %d", 
BdsLibGetStringById (STRING_TOKEN (STR_DESCRIPTION_SD)), SdNumber);
+} else {
+  UnicodeSPrint (Buffer, sizeof (Buffer), L"%s", BdsLibGetStringById 
(STRING_TOKEN (STR_DESCRIPTION_SD)));
+}
+BdsLibBuildOptionFromHandle (BlockIoHandles[Index], BdsBootOptionList, 
Buffer);
+SdNumber++;
+break;
+
+  case BDS_EFI_MESSAGE_EMMC_BOOT:
+if (EmmcNumber != 0) {
+  UnicodeSPrint (Buffer, sizeof (Buffer), L"%s %d", 
BdsLibGetStringById (STRING_TOKEN (STR_DESCRIPTION_EMMC)), EmmcNumber);
+} else {
+  UnicodeSPrint (Buffer, sizeof (Buffer), L"%s", BdsLibGetStringById 
(STRING_TOKEN (STR_DESCRIPTION_EMMC)));
+}
+BdsLibBuildOptionFromHandle (BlockIoHandles[Index], BdsBootOptionList, 
Buffer);
+EmmcNumber++;
+break;
+
   case BDS_EFI_MESSAGE_MISC_BOOT:
   default:
 if (MiscNumber != 0) {
@@ -3927,7 +3963,8 @@ BdsGetBootTypeFromDevicePath (
 // Get the last device path node
 //
 LastDeviceNode = NextDevicePathNode (TempDevicePath);
-if (DevicePathSubType(LastDeviceNode) == MSG_DEVICE_LOGICAL_UNIT_DP) {
+if ((DevicePathSubType(LastDeviceNode) == MSG_DEVICE_LOGICAL_UNIT_DP) 
||
+(DevicePathSubType(LastDeviceNode) == HW_CONTROLLER_DP)) {
   //
   // if the next node type is Device Logical Unit, which specify the 
Logical Unit Number (LUN),
   // skip it
@@ -3965,6 +4002,18 @@ 

Re: [edk2] Failed to clear configuration in Ip4Config2 Protocol

[Wu, Jiaxin]

Hi Karunakar,

DNS Device Path is used to list all the instances of the DNS server address. If 
there are multiple DNS server address reported from DHCP server, it will like 
the Spec example: Dns(192.168.22.100, 192.168.22.101). Here, according your DNS 
Node, there is only one DNS server returned from DHCP.

Thanks,
Jiaxin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Karunakar P
> Sent: Tuesday, September 19, 2017 6:53 PM
> To: Wu, Jiaxin ; 'edk2-devel@lists.01.org'  de...@lists.01.org>; Ye, Ting 
> Subject: Re: [edk2] Failed to clear configuration in Ip4Config2 Protocol
> 
> Hi Jiaxin,
> 
> Many thanks for your help.
> 
> After the successful HTTP boot DNS parsing, I can see the DNS Node, But it
> have bit difference form UEFI2.7 spec
> //../MAC(D8CB8ADEBBAA,0x0)/IPv4(0.0.0.0)/Dns(192.168.184.1)/Uri(https
> ://www.cloudboot.com:443/EFI/Shell.efi)
> //../MAC(D8CB8ADEBBAA,0x0)/IPv6(2001:0DB8::0001::::
> 0001)/Dns(2001:0DB8::0001::::0001)/Uri(https://www.clou
> dbootip6.com:443/EFI/Shell.efi)
> 
> I can see Only DNS Server IP address in DNS Node i.e. Dns(192.168.184.1)
> But UEFI2.7 spec says(24.7.3.1 Device Path, page 1329) like below
> Dns(192.168.22.100, 192.168.22.101) for IPv4
> Dns(2016::100, 2016::101   for IPv6,  DNS node have DNS Server IP and some
> other IP address too.
> 
> Why I'm getting this difference, is there anything wrong or I'm I missing
> anything?
> 
> Thanks,
> Karunakar
> 
> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Monday, September 18, 2017 10:28 AM
> To: Karunakar P; 'edk2-devel@lists.01.org'; Ye, Ting
> Subject: RE: [edk2] Failed to clear configuration in Ip4Config2 Protocol
> 
> Hi karunakar,
> 
> You can verify the DNS device path with HTTP boot feature. After the
> successful HTTP boot DNS parsing,  the device path should be like:
> //../Mac(...)[/Vlan(...)][/Wi-Fi(...)]/IPv4(...)[/Dns(...)]/Uri(...).
> 
> That is recommend way for the verification.
> 
> Besides, you can also draft the App to call  the DevPathFromTextDns and
> DevPathToTextDns libraries for the more verification.
> 
> Thanks,
> Jiaxin
> 
> 
> > -Original Message-
> > From: Karunakar P [mailto:karunak...@amiindia.co.in]
> > Sent: Monday, September 18, 2017 12:22 PM
> > To: Wu, Jiaxin ; 'edk2-devel@lists.01.org'  > de...@lists.01.org>; Ye, Ting 
> > Subject: RE: [edk2] Failed to clear configuration in Ip4Config2
> > Protocol
> >
> > Hi Jiaxin,
> >
> > Thank you very much for your info, Yes it works fine for manual
> configuration.
> >
> > And also could you please provide steps to verify "Add DNS device path
> > node" feature.
> >
> > Thanks,
> > karunakar
> >
> > -Original Message-
> > From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> > Sent: Monday, September 18, 2017 7:46 AM
> > To: Karunakar P; 'edk2-devel@lists.01.org'; Ye, Ting
> > Subject: RE: [edk2] Failed to clear configuration in Ip4Config2
> > Protocol
> >
> > Hi Karunakar,
> >
> > According the UEFI Spec, the Ip4Config2DataTypeManualAddress,
> > Ip4Config2DataTypeGateway and Ip4Config2DataTypeDnsServer
> > configuration data are not allowed to set via SetData() if the policy is 
> > DHCP.
> > So, the clear feature is only for the manual configuration. This is
> > our design purpose and also the reason why the feature is not apply to
> > the Ip4Config2DataTypeInterfaceInfo/Ip4Config2DataTypePolicy.
> >
> > Thanks,
> > Jiaxin
> >
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Karunakar P
> > > Sent: Friday, September 15, 2017 5:41 PM
> > > To: 'edk2-devel@lists.01.org' 
> > > Subject: Re: [edk2] Failed to clear configuration in Ip4Config2
> > > Protocol
> > >
> > > Hello All,
> > >
> > > Could you please anyone provide comment on this?
> > >
> > > Thank you,
> > > karunakar
> > >
> > > From: Karunakar P
> > > Sent: Wednesday, September 13, 2017 7:04 PM
> > > To: 'edk2-devel@lists.01.org'
> > > Subject: RE: RE: Failed to clear configuration in Ip4Config2
> > > Protocol
> > >
> > > Hello All,
> > >
> > > I was trying to verify the feature "Allow SetData to clear
> > > configuration in Ip4Config2/Ip6Config Protocol" , But SetData
> > > returns with Write Protected Error Status
> > >
> > > [Steps followed]
> > >
> > > 1.   I've taken the above feature changes.
> > >
> > > 2.   I've a UEFI test Application which call to SetData with DataSize 
> > > is 0
> and
> > > Data is NULL
> > >
> > > Status = Ip4Cfg2->SetData (
> > >
> > > Ip4Cfg2,
> > >
> > > Ip4Config2DataTypeManualAddress,
> > >
> > > 0,
> > >
> > > 0
> > >
> > > );
> > >
> > > 3.   But SetData returns with Write Protected Error 

Re: [edk2] [PATCH 1/1] ArmPkg/ArmSvcLib: Add ArmSvcLib implementation.

[Ard Biesheuvel]

Hi Supreeth,

On 19 September 2017 at 12:55, Supreeth Venkatesh
 wrote:
> This patch adds a library that enables invocation of SVCs from Exception
> Level EL0. It will be used by the Standalone MM environment to request
> services from a software running in a privileged EL e.g. ARM Trusted
> Firmware. The library is a derived directly from Arm SMC Library.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S | 41 
>  ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S | 52 
> +++
>  ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm   | 51 ++
>  ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf| 31 ++

Please add this library to the [Components] section of ArmPkg.dsc so
it gets built when we build that package.

Also, it appears the header file is missing, although I suppose it
could be part of a separate patch. However, this is 1/1 and it's
missing from this patch.


>  4 files changed, 175 insertions(+)
>  create mode 100644 ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
>  create mode 100644 ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
>  create mode 100644 ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm
>  create mode 100644 ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
>
> diff --git a/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S 
> b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
> new file mode 100644
> index 00..70122bbb0e
> --- /dev/null
> +++ b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
> @@ -0,0 +1,41 @@
> +//
> +//  Copyright (c) 2012 - 2017, ARM Limited. All rights reserved.
> +//
> +//  This program and the accompanying materials
> +//  are licensed and made available under the terms and conditions of the 
> BSD License
> +//  which accompanies this distribution.  The full text of the license may 
> be found at
> +//  http://opensource.org/licenses/bsd-license.php
> +//
> +//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +//
> +//
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(ArmCallSvc)
> +
> +ASM_PFX(ArmCallSvc):
> +  // Push x0 on the stack - The stack must always be quad-word aligned
> +  str   x0, [sp, #-16]!
> +

Please create a proper stackframe here, by pushing the frame pointer
and return address to the stack as well. You should always do that
when you use the stack, even if the return address and frame pointer
are not corrupted by this function.

> +  // Load the SVC arguments values into the appropriate registers
> +  ldp   x6, x7, [x0, #48]
> +  ldp   x4, x5, [x0, #32]
> +  ldp   x2, x3, [x0, #16]
> +  ldp   x0, x1, [x0, #0]
> +
> +  svc   #0
> +
> +  // Pop the ARM_SVC_ARGS structure address from the stack into x9
> +  ldr   x9, [sp], #16
> +
> +  // Store the SVC returned values into the ARM_SVC_ARGS structure.
> +  // A SVC call can return up to 4 values - we do not need to store back 
> x4-x7.
> +  stp   x2, x3, [x9, #16]
> +  stp   x0, x1, [x9, #0]
> +

Nit: no reason to do the stores in opposite order here.

> +  mov   x0, x9
> +
> +  ret
> diff --git a/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S 
> b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
> new file mode 100644
> index 00..a7ac2966c3
> --- /dev/null
> +++ b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
> @@ -0,0 +1,52 @@
> +//
> +//  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
> +//
> +//  This program and the accompanying materials
> +//  are licensed and made available under the terms and conditions of the 
> BSD License
> +//  which accompanies this distribution.  The full text of the license may 
> be found at
> +//  http://opensource.org/licenses/bsd-license.php
> +//
> +//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +//
> +//
> +
> +.text
> +.align 3
> +.arch_extension sec
> +
> +GCC_ASM_EXPORT(ArmCallSvc)
> +
> +ASM_PFX(ArmCallSvc):
> +push{r4-r8}
> +// r0 will be popped just after the SVC call
> +push{r0}
> +

I'd prefer it if this didn't leave the stack misaligned between the
two instructions. Is there anything wrong with

push {r0, r4-r8}

?

> +// Load the SVC arguments values into the appropriate registers
> +ldr r7, [r0, #28]
> +ldr r6, [r0, #24]
> +ldr r5, [r0, #20]
> +ldr r4, [r0, #16]
> +ldr r3, [r0, #12]
> +ldr r2, [r0, #8]
> +ldr r1, [r0, #4]
> +ldr r0, [r0, #0]
> +

Does

ldm r0, {r0-r7}

not work here?

> +svc #0
> +
> +// Pop the ARM_SVC_ARGS structure address from the stack into r8
> +pop {r8}
> +
> +// Load the SVC returned values into the appropriate registers
> +// A SVC call can return up to 4 values - we do 

[edk2] [PATCH 1/1] ArmPkg/ArmSvcLib: Add ArmSvcLib implementation.

[Supreeth Venkatesh]

This patch adds a library that enables invocation of SVCs from Exception
Level EL0. It will be used by the Standalone MM environment to request
services from a software running in a privileged EL e.g. ARM Trusted
Firmware. The library is a derived directly from Arm SMC Library.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Supreeth Venkatesh 
---
 ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S | 41 
 ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S | 52 +++
 ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm   | 51 ++
 ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf| 31 ++
 4 files changed, 175 insertions(+)
 create mode 100644 ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
 create mode 100644 ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
 create mode 100644 ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm
 create mode 100644 ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf

diff --git a/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S 
b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
new file mode 100644
index 00..70122bbb0e
--- /dev/null
+++ b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
@@ -0,0 +1,41 @@
+//
+//  Copyright (c) 2012 - 2017, ARM Limited. All rights reserved.
+//
+//  This program and the accompanying materials
+//  are licensed and made available under the terms and conditions of the BSD 
License
+//  which accompanies this distribution.  The full text of the license may be 
found at
+//  http://opensource.org/licenses/bsd-license.php
+//
+//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+//
+//
+
+.text
+.align 3
+
+GCC_ASM_EXPORT(ArmCallSvc)
+
+ASM_PFX(ArmCallSvc):
+  // Push x0 on the stack - The stack must always be quad-word aligned
+  str   x0, [sp, #-16]!
+
+  // Load the SVC arguments values into the appropriate registers
+  ldp   x6, x7, [x0, #48]
+  ldp   x4, x5, [x0, #32]
+  ldp   x2, x3, [x0, #16]
+  ldp   x0, x1, [x0, #0]
+
+  svc   #0
+
+  // Pop the ARM_SVC_ARGS structure address from the stack into x9
+  ldr   x9, [sp], #16
+
+  // Store the SVC returned values into the ARM_SVC_ARGS structure.
+  // A SVC call can return up to 4 values - we do not need to store back x4-x7.
+  stp   x2, x3, [x9, #16]
+  stp   x0, x1, [x9, #0]
+
+  mov   x0, x9
+
+  ret
diff --git a/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S 
b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
new file mode 100644
index 00..a7ac2966c3
--- /dev/null
+++ b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
@@ -0,0 +1,52 @@
+//
+//  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
+//
+//  This program and the accompanying materials
+//  are licensed and made available under the terms and conditions of the BSD 
License
+//  which accompanies this distribution.  The full text of the license may be 
found at
+//  http://opensource.org/licenses/bsd-license.php
+//
+//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+//
+//
+
+.text
+.align 3
+.arch_extension sec
+
+GCC_ASM_EXPORT(ArmCallSvc)
+
+ASM_PFX(ArmCallSvc):
+push{r4-r8}
+// r0 will be popped just after the SVC call
+push{r0}
+
+// Load the SVC arguments values into the appropriate registers
+ldr r7, [r0, #28]
+ldr r6, [r0, #24]
+ldr r5, [r0, #20]
+ldr r4, [r0, #16]
+ldr r3, [r0, #12]
+ldr r2, [r0, #8]
+ldr r1, [r0, #4]
+ldr r0, [r0, #0]
+
+svc #0
+
+// Pop the ARM_SVC_ARGS structure address from the stack into r8
+pop {r8}
+
+// Load the SVC returned values into the appropriate registers
+// A SVC call can return up to 4 values - we do not need to store back 
r4-r7.
+str r3, [r8, #12]
+str r2, [r8, #8]
+str r1, [r8, #4]
+str r0, [r8, #0]
+
+mov r0, r8
+
+// Restore the registers r4-r8
+pop {r4-r8}
+
+bx  lr
diff --git a/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm 
b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm
new file mode 100644
index 00..25edbf1939
--- /dev/null
+++ b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm
@@ -0,0 +1,51 @@
+//
+//  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
+//
+//  This program and the accompanying materials
+//  are licensed and made available under the terms and conditions of the BSD 
License
+//  which accompanies this distribution.  The full text of the license may be 
found at
+//  http://opensource.org/licenses/bsd-license.php
+//
+//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+//
+//
+
+
+INCLUDE AsmMacroExport.inc
+
+ RVCT_ASM_EXPORT ArmCallSvc
+push{r4-r8}
+// r0 will be popped 

Re: [edk2] [PATCH 0/3] OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly

[Laszlo Ersek]

On 09/19/17 21:18, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: vbe_shim_q35_pam
>
> Fix the long-standing OVMF/Q35 bug recently exposed by a QEMU change,
> and reported under .
>
> Aleksei, can you please fetch the branch, build it, and report back
> with your Tested-by if it works for you?
>
> I performed my own tests as well; I'll include those in a separate
> email.

Testing:

- QEMU version: v2.10.0-611-g11e06ce1ed28.

- Edk2 basis: a3a473705101.

- ISO image used for testing:
  en_windows_server_2008_r2_with_sp1_x64_dvd_617601.iso

Test  CSM included  Machine type (2.10)  Boot Mode  Patch applied  Result
    ---  -  -  --
  01  noi440fx   UEFI   no pass
  03  noi440fx   UEFI   yespass

  02  noq35  UEFI   no fail
  04  noq35  UEFI   yespass

  05  yes   i440fx   UEFI   no pass
  09  yes   i440fx   UEFI   yespass

  06  yes   i440fx   legacy no fail
  10  yes   i440fx   legacy yesn/a

  07  yes   q35  UEFI   no pass
  11  yes   q35  UEFI   yespass

  08  yes   q35  legacy no n/a
  12  yes   q35  legacy yesn/a

(Please excuse the strange ordering of the test numbers; the reason is
that testing all the cases is much faster in a different order, while
presenting the results is best in this order. The test case numbering
reflects the order in which I performed the tests.)

Test cases 01 and 03 explain that there is no regression due to the
patches, on i440fx and in UEFI boot mode.

Test cases 02 and 04 show that the patches fix the bug reported by
Aleksei.

The remaining cases cover a build when the SeaBIOS CSM is included in
OVMF. For this I used current SeaBIOS master (at commit ec6cb17f8949),
plus a minimal patch that removes the runningOnQEMU() requirement from
qemu_debug_putc() -- because this check used to fail when running
SeaBIOS as an OVMF CSM, preventing SeaBIOS debug messages from reaching
the QEMU debug port. Furthermore, I built the SeaBIOS QXL option ROM
from the same SeaBIOS tree.

Test cases 05 and 09 explain that there is no regression due to the
patches, on i440fx and in UEFI boot mode.

Test case 06 shows that legacy boot stopped working at an unspecified
point in the past. I emphasize that this boot failure was *without* the
present patch set. I'm going to attach the OVMF debug logs for all
cases, which include the SeaBIOS and VGABIOS output as well. If someone
is interested in getting the CSM build to work again (for legacy OS
boot), please feel free to go ahead. (Personally I'd suggest to stick
with pure SeaBIOS if you have a legacy guest OS.)

Due to the failure of test case 06, I didn't check the behavior with the
current series applied on top (that is, in case 10).

Test cases 07 and 11 demonstrate that there is no regression due to the
patches, on q35 and in UEFI boot mode.

As a side note, test case 07 also demonstrates that the bug reported by
Aleksei is specific to the VBE Shim; it does not hit when the VBE Shim
is unused due to CSM presence. This is consistent with Gerd's result in
.

Test cases 08 and 12 are not possible to test (i.e., the current series
makes no difference). This is because Q35 has an AHCI/SATA controller
(with six ports), not a traditional IDE controller (with
primary/secondary and master/slave), therefore edk2's UefiBootManagerLib
and/or BdsDxe do not generate BBS (= legacy BIOS) boot options for the
CD-ROM. (I used an "ide-cd" device in all cases.)

Given my earlier experience that the edk2-devel list software does not
reflect attachments, and does not include them in the mailing list
archive, I'm manually including a base-64 encoded tarball below, with
the debug logs. The SHA256 sum is
ad196c202cf300e2028ac67256c6966b1ce7a13042e47351d19d8408bb0b32ab.

Thanks,
Laszlo

begin-base64 600 vbe_shim_q35_pam_results.tar.xz
/Td6WFoAAATm1rRGAgAhARwQz1jM8I//YDJdADsYiOHoiSZP6UBkHq3m
LIs35M7ja6B1SFm+o1y3elPl/iBZGUi0m0mP+uxrv0LVvq7zmdF0dsJ6QANV
QkqQSnThPv9HYNUFV8GdAOhuLzQIyFltWhNA3YbX5phtfiIjwdUY7RK5YdcO
iAqu2WXW0HqPzardN9e+WmC+Kub2xPrNU/6MIEv6XE6HRxKFj+afu+wFsDes
kWB23DrvbjGyeLs8YaxMmL/mmOnLdvERvL5hhzDsJ4gDv+BYCoAVTacsuJCl
nTWYjwVk7gJUK98pKK9TKq3xtVDoZ9Yy0j3I26PZv+cLBHgWbbd8ILwQSuDd
Xri++rCWQ1pBEeIn+MSKC13/VU9Ls5RZLVajFxRX/TcQ42EfpFvuFv144q40
0sn03yH0W8thJkv8rrtfJIKRR3je7QJfEg/LNa0LtwlvJnlqoeyO50UACrWA
AS01GbXFE9pJEx7M8iTFPAU/0B88dDWkeLhbOAUq0zoDKJpZG8Kavhtee+VA

[edk2] [PATCH 2/3] OvmfPkg/QemuVideoDxe/VbeShim: rename Status to Segment0AllocationStatus

[Laszlo Ersek]

This clarifies the purpose of the local variable in InstallVbeShim().

Cc: Aleksei Kovura 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Ref: https://bugs.launchpad.net/qemu/+bug/1715700
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/QemuVideoDxe/VbeShim.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c b/OvmfPkg/QemuVideoDxe/VbeShim.c
index 4c4517e9da27..bc90e067266d 100644
--- a/OvmfPkg/QemuVideoDxe/VbeShim.c
+++ b/OvmfPkg/QemuVideoDxe/VbeShim.c
@@ -63,7 +63,7 @@ InstallVbeShim (
   EFI_PHYSICAL_ADDRESS Segment0, SegmentC, SegmentF;
   UINTNSegment0Pages;
   IVT_ENTRY*Int0x10;
-  EFI_STATUS   Status;
+  EFI_STATUS   Segment0AllocationStatus;
   UINTNPam1Address;
   UINT8Pam1;
   UINTNSegmentCPages;
@@ -87,10 +87,14 @@ InstallVbeShim (
   //
   Segment0Pages = 1;
   Int0x10   = (IVT_ENTRY *)(UINTN)Segment0 + 0x10;
-  Status = gBS->AllocatePages (AllocateAddress, EfiBootServicesCode,
-  Segment0Pages, );
+  Segment0AllocationStatus = gBS->AllocatePages (
+AllocateAddress,
+EfiBootServicesCode,
+Segment0Pages,
+
+);
 
-  if (EFI_ERROR (Status)) {
+  if (EFI_ERROR (Segment0AllocationStatus)) {
 EFI_PHYSICAL_ADDRESS Handler;
 
 //
@@ -109,8 +113,12 @@ InstallVbeShim (
 // Otherwise we'll overwrite the Int10h vector, even though we may not own
 // the page at zero.
 //
-DEBUG ((EFI_D_INFO, "%a: failed to allocate page at zero: %r\n",
-  __FUNCTION__, Status));
+DEBUG ((
+  DEBUG_INFO,
+  "%a: failed to allocate page at zero: %r\n",
+  __FUNCTION__,
+  Segment0AllocationStatus
+  ));
   } else {
 //
 // We managed to allocate the page at zero. SVN r14218 guarantees that it
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 1/3] OvmfPkg/CsmSupportLib: move PAM register addresses to IndustryStandard

[Laszlo Ersek]

* Introduce the PIIX4_PAM* and MCH_PAM* macros under
  "OvmfPkg/Include/IndustryStandard". These macros capture the PAM
  register offsets (in PCI config space) on the respective Memory
  Controller B/D/F, from the respective data sheets.

* Under IndustryStandard, introduce the PMC_REGISTER_PIIX4() macro for
  PIIX4. (For Q35, we already have DRAMC_REGISTER_Q35().) In both cases,
  the B/D/F is 0/0/0.

* Under CsmSupportLib, replace the "PAMRegOffset" field (UINT8) in the
  PAM_REGISTER_VALUE structure with "PAMRegPciLibAddress" (UINTN). The new
  field contains the return value of the PCI_LIB_ADDRESS() macro.

* Under CsmSupportLib, replace the "mRegisterValues440" elements as
  follows:

REG_PAMx_OFFSET_440, ReadEnableData, WriteEnableData
-->
PMC_REGISTER_PIIX4 (PIIX4_PAMx), ReadEnableData, WriteEnableData

* Under CsmSupportLib, replace the "mRegisterValuesQ35" elements as
  follows:

REG_PAMx_OFFSET_Q35, ReadEnableData, WriteEnableData
-->
DRAMC_REGISTER_Q35 (MCH_PAMx), ReadEnableData, WriteEnableData

* Under CsmSupportLib, update the register address calculations as follows
  (for all of PciOr8(), PciAnd8() and PciRead8()):

PCI_LIB_ADDRESS (
  PAM_PCI_BUS,
  PAM_PCI_DEV,
  PAM_PCI_FUNC,
  mRegisterValues[Index].PAMRegOffset
  )
-->
mRegisterValues[Index].PAMRegPciLibAddress

* Under CsmSupportLib, remove the PAM_PCI_* and REG_PAM*_OFFSET_* macros.

Technically speaking, these changes could be split into three patches
(IndustryStandard macro additions, CsmSupportLib code updates,
CsmSupportLib macro removals). However, the patch is not big, and in this
case it is actually helpful to present the code movement / refactoring in
one step, for easier verification.

Cc: Aleksei Kovura 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Ref: https://bugs.launchpad.net/qemu/+bug/1715700
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h   | 22 +--
 OvmfPkg/Include/IndustryStandard/I440FxPiix4.h | 13 
 OvmfPkg/Include/IndustryStandard/Q35MchIch9.h  |  8 +++
 OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c   | 62 ++--
 4 files changed, 53 insertions(+), 52 deletions(-)

diff --git a/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h 
b/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h
index f755a2a359e5..01d3109a7d7d 100644
--- a/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h
+++ b/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h
@@ -30,26 +30,6 @@
 #include 
 #include 
 
-#define PAM_PCI_BUS0
-#define PAM_PCI_DEV0
-#define PAM_PCI_FUNC   0
-
-#define REG_PAM0_OFFSET_4400x59// Programmable Attribute Map 0
-#define REG_PAM1_OFFSET_4400x5a// Programmable Attribute Map 1
-#define REG_PAM2_OFFSET_4400x5b// Programmable Attribute Map 2
-#define REG_PAM3_OFFSET_4400x5c// Programmable Attribute Map 3
-#define REG_PAM4_OFFSET_4400x5d// Programmable Attribute Map 4
-#define REG_PAM5_OFFSET_4400x5e// Programmable Attribute Map 5
-#define REG_PAM6_OFFSET_4400x5f// Programmable Attribute Map 6
-
-#define REG_PAM0_OFFSET_Q350x90// Programmable Attribute Map 0
-#define REG_PAM1_OFFSET_Q350x91// Programmable Attribute Map 1
-#define REG_PAM2_OFFSET_Q350x92// Programmable Attribute Map 2
-#define REG_PAM3_OFFSET_Q350x93// Programmable Attribute Map 3
-#define REG_PAM4_OFFSET_Q350x94// Programmable Attribute Map 4
-#define REG_PAM5_OFFSET_Q350x95// Programmable Attribute Map 5
-#define REG_PAM6_OFFSET_Q350x96// Programmable Attribute Map 6
-
 #define PAM_BASE_ADDRESS   0xc
 #define PAM_LIMIT_ADDRESS  BASE_1MB
 
@@ -67,7 +47,7 @@ typedef struct {
 // Provides a map of the PAM registers and bits used to set Read/Write access.
 //
 typedef struct {
-  UINT8   PAMRegOffset;
+  UINTN   PAMRegPciLibAddress;
   UINT8   ReadEnableData;
   UINT8   WriteEnableData;
 } PAM_REGISTER_VALUE;
diff --git a/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h 
b/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h
index baa4c063f16a..efe6e5c27834 100644
--- a/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h
+++ b/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h
@@ -27,6 +27,19 @@
 //
 #define INTEL_82441_DEVICE_ID 0x1237
 
+//
+// B/D/F/Type: 0/0/0/PCI
+//
+#define PMC_REGISTER_PIIX4(Offset) PCI_LIB_ADDRESS (0, 0, 0, (Offset))
+
+#define PIIX4_PAM0  0x59
+#define PIIX4_PAM1  0x5A
+#define PIIX4_PAM2  0x5B
+#define PIIX4_PAM3  0x5C
+#define PIIX4_PAM4  0x5D
+#define PIIX4_PAM5  0x5E
+#define PIIX4_PAM6  0x5F
+
 //
 // B/D/F/Type: 0/1/3/PCI
 //
diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h 
b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
index 

[edk2] [PATCH 0/3] OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: vbe_shim_q35_pam

Fix the long-standing OVMF/Q35 bug recently exposed by a QEMU change,
and reported under .

Aleksei, can you please fetch the branch, build it, and report back with
your Tested-by if it works for you?

I performed my own tests as well; I'll include those in a separate
email.

Cc: Aleksei Kovura 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Jordan Justen 
Cc: Ruiyu Ni 

Thank you,
Laszlo

Laszlo Ersek (3):
  OvmfPkg/CsmSupportLib: move PAM register addresses to IndustryStandard
  OvmfPkg/QemuVideoDxe/VbeShim: rename Status to
Segment0AllocationStatus
  OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly

 OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c   | 62 ++--
 OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h   | 22 +--
 OvmfPkg/Include/IndustryStandard/I440FxPiix4.h | 13 
 OvmfPkg/Include/IndustryStandard/Q35MchIch9.h  |  8 +++
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf  |  3 +-
 OvmfPkg/QemuVideoDxe/VbeShim.c | 47 ---
 6 files changed, 95 insertions(+), 60 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] [PATCH 3/3] OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly

[Laszlo Ersek]

In commit db27e9f3d8f0 ("OvmfPkg/LegacyRegion: Support legacy region
manipulation of Q35", 2016-03-15), Ray extended the
OvmfPkg/Csm/CsmSupportLib PAM register manipulation to Q35. However, we
missed that the same should be done to the QemuVideoDxe VBE Shim as well.

The omission has caused no problems in practice on Q35, because QEMU has
let us write to the ROM area, regardless of the PAM1 setting, all this
time. This has now changed with recent QEMU commit 208fa0e43645 ("pc: make
'pc.rom' readonly when machine has PCI enabled", 2017-07-28). The QEMU
commit exposes the OVMF bug when Windows 7 is started on Q35, using QEMU
2.10 -- the VBE Shim is no longer put in place and Windows 7 cannot find
it.

To remedy this, assign the "Pam1Address" local variable a PciLib address
that matches the board type (i440fx vs. q35).

Regarding the PcdLib dependency: QemuVideoDxe already uses PcdLib, both
directly (see "PcdDriverSupportedEfiVersion") and indirectly (e.g. via the
DxePciLibI440FxQ35 PciLib instance). Add PcdLib to [LibraryClasses] for
completeness.

Cc: Aleksei Kovura 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Ref: https://bugs.launchpad.net/qemu/+bug/1715700
Reported-by: Aleksei Kovura 
Special-thanks-to: Gerd Hoffmann 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |  3 ++-
 OvmfPkg/QemuVideoDxe/VbeShim.c| 27 +++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf 
b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index 7c7d429bca27..577e07b0a8bf 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -60,6 +60,7 @@ [LibraryClasses]
   DebugLib
   DevicePathLib
   MemoryAllocationLib
+  PcdLib
   PciLib
   PrintLib
   TimerLib
@@ -75,4 +76,4 @@ [Protocols]
 
 [Pcd]
   gOptionRomPkgTokenSpaceGuid.PcdDriverSupportedEfiVersion
-
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c b/OvmfPkg/QemuVideoDxe/VbeShim.c
index bc90e067266d..e45a08e8873f 100644
--- a/OvmfPkg/QemuVideoDxe/VbeShim.c
+++ b/OvmfPkg/QemuVideoDxe/VbeShim.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "Qemu.h"
 #include "VbeShim.h"
@@ -64,6 +65,7 @@ InstallVbeShim (
   UINTNSegment0Pages;
   IVT_ENTRY*Int0x10;
   EFI_STATUS   Segment0AllocationStatus;
+  UINT16   HostBridgeDevId;
   UINTNPam1Address;
   UINT8Pam1;
   UINTNSegmentCPages;
@@ -131,7 +133,30 @@ InstallVbeShim (
   //
   // Put the shim in place first.
   //
-  Pam1Address = PCI_LIB_ADDRESS (0, 0, 0, 0x5A);
+  // Start by determining the address of the PAM1 register.
+  //
+  HostBridgeDevId = PcdGet16 (PcdOvmfHostBridgePciDevId);
+  switch (HostBridgeDevId) {
+  case INTEL_82441_DEVICE_ID:
+Pam1Address = PMC_REGISTER_PIIX4 (PIIX4_PAM1);
+break;
+  case INTEL_Q35_MCH_DEVICE_ID:
+Pam1Address = DRAMC_REGISTER_Q35 (MCH_PAM1);
+break;
+  default:
+DEBUG ((
+  DEBUG_ERROR,
+  "%a: unknown host bridge device ID: 0x%04x\n",
+  __FUNCTION__,
+  HostBridgeDevId
+  ));
+ASSERT (FALSE);
+
+if (!EFI_ERROR (Segment0AllocationStatus)) {
+  gBS->FreePages (Segment0, Segment0Pages);
+}
+return;
+  }
   //
   // low nibble covers 0xC to 0xC3FFF
   // high nibble covers 0xC4000 to 0xC7FFF
-- 
2.14.1.3.gb7cf6e02401b

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

Re: [edk2] [PATCH 5/6] MdeModulePkg/Crc32: Fix possible out of range left shift

[Paolo Bonzini]

On 19/09/2017 13:43, Hao Wu wrote:
>NewValue = 0;
>for (Index = 0; Index < 32; Index++) {
> -if ((Value & (1 << Index)) != 0) {
> -  NewValue = NewValue | (1 << (31 - Index));
> +if ((Value & (((UINT32)1) << Index)) != 0) {
> +  NewValue = NewValue | (((UINT32)1) << (31 - Index));
>  }


Why not just "1u" instead of the cast?

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

Re: [edk2] [PATCH 4/6] MdeModulePkg/DxeNetLib: Fix negative value left shift

[Paolo Bonzini]

On 19/09/2017 13:43, Hao Wu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=698
> 
> Within function NetRandomInitSeed(), left shift a negative value is used
> in:
> "~Time.Hour << 24"
> 
> which involves undefined behavior.
> 
> Since Time.Hour is of type UINT8 (range from 0 to 23), hence ~Time.Hour
> will be a negative value (of type int, signed).
> 
> According to the C11 spec, Section 6.5.7:
>> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>>   bits are filled with zeros. If E1 has an unsigned type, the value
>>   of the result is E1 * 2^E2 , reduced modulo one more than the
>>   maximum value representable in the result type. If E1 has a signed
>>   type and nonnegative value, and E1 * 2^E2 is representable in the
>>   result type, then that is the resulting value; otherwise, the
>>   behavior is undefined.
> 
> This commit explicitly cast 'Time.Hour' with UINT32 to resolve this issue.
> 
> Cc: Steven Shi 
> Cc: Fu Siyuan 
> Cc: Ye Ting 
> Cc: Wu Jiaxin 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c 
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index 7cd7e3aca0..ca5413edcc 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -872,7 +872,7 @@ NetRandomInitSeed (
>UINT64MonotonicCount;
>  
>gRT->GetTime (, NULL);
> -  Seed = (~Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | 
> Time.Second);
> +  Seed = (~(UINT32)Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | 
> Time.Second);
>Seed ^= Time.Nanosecond;
>Seed ^= Time.Year << 7;
Is there any reason why Seed must be XORed with 0xFF00 (that's what
the ~ does)?  Would it make sense to either write it explicitly as a
XOR, or perhaps the ~ can be removed altogether?

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

Re: [edk2] [PATCH 3/6] MdeModulePkg/Tpl: Fix negative value left shift

[Paolo Bonzini]

On 19/09/2017 13:43, Hao Wu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=695
> 
> Within function CoreRestoreTpl(), left shift a negative value -2 is used
> in:
> "while (((-2 << NewTpl) & gEventPending) != 0) {"
> 
> which involves undefined behavior.
> 
> According to the C11 spec, Section 6.5.7:
>> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>>   bits are filled with zeros. If E1 has an unsigned type, the value
>>   of the result is E1 * 2^E2 , reduced modulo one more than the
>>   maximum value representable in the result type. If E1 has a signed
>>   type and nonnegative value, and E1 * 2^E2 is representable in the
>>   result type, then that is the resulting value; otherwise, the
>>   behavior is undefined.
> 
> This commit explicitly cast '-2' with UINTN to resolve this issue.
> 
> Cc: Steven Shi 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Core/Dxe/Event/Tpl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Event/Tpl.c 
> b/MdeModulePkg/Core/Dxe/Event/Tpl.c
> index 8ad0a33701..8c50f61117 100644
> --- a/MdeModulePkg/Core/Dxe/Event/Tpl.c
> +++ b/MdeModulePkg/Core/Dxe/Event/Tpl.c
> @@ -123,7 +123,7 @@ CoreRestoreTpl (
>//
>// Dispatch any pending events
>//
> -  while (((-2 << NewTpl) & gEventPending) != 0) {
> +  while (UINTN)-2) << NewTpl) & gEventPending) != 0) {
>  gEfiCurrentTpl = (UINTN) HighBitSet64 (gEventPending);
>  if (gEfiCurrentTpl < TPL_HIGH_LEVEL) {
>CoreSetInterruptState (TRUE);
> 

Maybe:

  for (;;) {
PendingTpl = (UINTN) HighBitSet64 (gEventPending);
if (NewTpl >= PendingTpl) {
  break;
}
gEfiCurrentTpl = PendingTpl;
  }

This is much more readable, and HighBitSet64 should be efficient on most
modern processors.

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

Re: [edk2] [PATCH] BaseTools/tools_def AARCH64: enable frame pointers for RELEASE builds

[Ard Biesheuvel]

On 17 September 2017 at 22:06, Gao, Liming  wrote:
> Reviewed-by: Liming Gao 
>
[...]
>>>
>>> So remove -fomit-frame-pointer altogether this time.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel 
>>
>>Looks sensible.
>>Reviewed-by: Leif Lindholm 
>>

Pushed as 424a5ec33b3d5a842bff3f4695d0bd709c91a163

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

Re: [edk2] [PATCH v2] ArmPkg/PlatformBootManagerLib: process pending capsules

[Ard Biesheuvel]

On 18 September 2017 at 03:16, Leif Lindholm  wrote:
> On Fri, Sep 15, 2017 at 04:03:32PM -0700, Ard Biesheuvel wrote:
>> Process any capsule HOBs that were left for us by CapsulePei. This
>> involves calling ProcessCapsules() twice, as explained in the comment
>> in DxeCapsuleLibFmp [sic].
>>
>> 1) The first call must be before EndOfDxe. The system capsules is processed.
>>If device capsule FMP protocols are exposted at this time and device FMP
>>capsule has zero EmbeddedDriverCount, the device capsules are processed.
>>Each individual capsule result is recorded in capsule record variable.
>>System may reset in this function, if reset is required by capsule and
>>all capsules are processed.
>>If not all capsules are processed, reset will be defered to second call.
>>
>> 2) The second call must be after EndOfDxe and after ConnectAll, so that all
>>device capsule FMP protocols are exposed.
>>The system capsules are skipped. If the device capsules are NOT processed
>>in first call, they are processed here.
>>Each individual capsule result is recorded in capsule record variable.
>>System may reset in this function, if reset is required by capsule
>>processed in first call and second call.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>
> Reviewed-by: Leif Lindholm 
>

Pushed as 4bbcc285d5f74d34ec40733dde807f5a4f0cdf8c

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

Re: [edk2] Storing Non volatile variables on SD/NAND

[Ard Biesheuvel]

On 18 September 2017 at 22:28, Udit Kumar  wrote:
> Thanks Vladimir,
> With your design, you did delayed write to eMMC due to sharing with OS.  But 
> it works for you:)
> Say if eMMC controllers offers you a status bit, if eMMC storage is being 
> used for not. Then this
> could be possible to update at run time, both OS/UEFI needs to check and wait 
> if controller is being used.

That is the problem right there. The nice thing about a firmware spec
is that you don't have to care about how it was implemented if you
adhere to the API rules. Imposing additional restrictions (such as
requiring the OS to be careful about not using the eMMC when it may be
in use by the firmware) defeats the purpose of using UEFI, since you
won't be able to use a generic OS anyway.


> For sure,  some synchronization issues need to be ironed out (or maybe I am 
> just dreaming here).
>
> On part 2) where you forked VariableRuntime driver , could we think of 
> updating VariableRuntime driver,
> to support non-XIP or memory mapped devices.
>

I think being able to support non-memorymapped FV volumes for the
variable store would be a big improvement. This does require changes
to both the FaultTolerantWrite drivers and the VariableRuntime
drivers, which both appear in PEI, DXE and SMM flavors, and require
thorough review due to the security impact bugs have in this layer, so
this is a rather large chunk of work to take on.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/2] MdePkg/BaseLib: Avoid reading content beyond string boundary

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao Wu
Sent: Tuesday, September 19, 2017 7:39 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Ni, Ruiyu ; Kinney, 
Michael D ; Gao, Liming 
Subject: [edk2] [PATCH 2/2] MdePkg/BaseLib: Avoid reading content beyond string 
boundary

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

As mentioned in the above Bugzilla link by Steven, within the function 
PathCleanUpDirectories(), when executing command:
"cd ."

under Shell, the input parameter 'Path' string will have string length less 
than 2. Hence, it is possible for the below statement:
"if (StrCmp (Path + StrLen (Path) - 2, L"\\.") == 0) {"

to read contents before the string boundary.

This commit adds additional checks to avoid this.

Cc: Ruiyu Ni 
Cc: Steven Shi 
Cc: Michael Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdePkg/Library/BaseLib/FilePaths.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BaseLib/FilePaths.c 
b/MdePkg/Library/BaseLib/FilePaths.c
index 203045ccdc..d6f3758ecb 100644
--- a/MdePkg/Library/BaseLib/FilePaths.c
+++ b/MdePkg/Library/BaseLib/FilePaths.c
@@ -1,7 +1,7 @@
 /** @file
   Defines file-path manipulation functions.
 
-  Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2011 - 2017, Intel Corporation. All rights 
+ reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at @@ -91,7 +91,7 @@ PathCleanUpDirectories(
   while ((TempString = StrStr (Path, L"\\.\\")) != NULL) {
 CopyMem (TempString, TempString + 2, StrSize (TempString + 2));
   }
-  if (StrCmp (Path + StrLen (Path) - 2, L"\\.") == 0) {
+  if ((StrLen (Path) >= 2) && (StrCmp (Path + StrLen (Path) - 2, 
+ L"\\.") == 0)) {
 Path[StrLen (Path) - 1] = CHAR_NULL;
   }
 
--
2.12.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

Re: [edk2] [PATCH 1/2] ShellPkg/Shell: Avoid reading content beyond string boundary

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao Wu
Sent: Tuesday, September 19, 2017 7:39 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Ni, Ruiyu ; Carsey, 
Jaben 
Subject: [edk2] [PATCH 1/2] ShellPkg/Shell: Avoid reading content beyond string 
boundary

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

Within function EfiShellGetDevicePathFromFilePath(), when the input parameter 
'Path' string is like:
"FS0:"

It is possible for the below statement:
"if (*(Path+StrLen(MapName)+1) == CHAR_NULL) {"

to read the content 1 byte beyond the string boundary (both 'Path' and 
'MapName' will be FS0: in this case).

This commit adds additional checks to avoid this.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Cc: Steven Shi 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 ShellPkg/Application/Shell/ShellProtocol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/ShellProtocol.c 
b/ShellPkg/Application/Shell/ShellProtocol.c
index 40e5e653ae..5e34b8dad1 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -598,7 +598,8 @@ EfiShellGetDevicePathFromFilePath(
   //
   // build the full device path
   //
-  if (*(Path+StrLen(MapName)+1) == CHAR_NULL) {
+  if ((*(Path+StrLen(MapName)) != CHAR_NULL) &&
+  (*(Path+StrLen(MapName)+1) == CHAR_NULL)) {
 DevicePathForReturn = FileDevicePath(Handle, L"\\");
   } else {
 DevicePathForReturn = FileDevicePath(Handle, Path+StrLen(MapName));
--
2.12.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

Re: [edk2] [PATCH 1/2] ShellPkg/Shell: Avoid reading content beyond string boundary

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, September 19, 2017 4:39 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ni, Ruiyu ;
> Carsey, Jaben ; Shi, Steven
> 
> Subject: [PATCH 1/2] ShellPkg/Shell: Avoid reading content beyond string
> boundary
> Importance: High
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=690
> 
> Within function EfiShellGetDevicePathFromFilePath(), when the input
> parameter 'Path' string is like:
> "FS0:"
> 
> It is possible for the below statement:
> "if (*(Path+StrLen(MapName)+1) == CHAR_NULL) {"
> 
> to read the content 1 byte beyond the string boundary (both 'Path' and
> 'MapName' will be FS0: in this case).
> 
> This commit adds additional checks to avoid this.
> 
> Cc: Ruiyu Ni 
> Cc: Jaben Carsey 
> Cc: Steven Shi 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  ShellPkg/Application/Shell/ShellProtocol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> b/ShellPkg/Application/Shell/ShellProtocol.c
> index 40e5e653ae..5e34b8dad1 100644
> --- a/ShellPkg/Application/Shell/ShellProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> @@ -598,7 +598,8 @@ EfiShellGetDevicePathFromFilePath(
>//
>// build the full device path
>//
> -  if (*(Path+StrLen(MapName)+1) == CHAR_NULL) {
> +  if ((*(Path+StrLen(MapName)) != CHAR_NULL) &&
> +  (*(Path+StrLen(MapName)+1) == CHAR_NULL)) {
>  DevicePathForReturn = FileDevicePath(Handle, L"\\");
>} else {
>  DevicePathForReturn = FileDevicePath(Handle, Path+StrLen(MapName));
> --
> 2.12.0.windows.1

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

Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

[Ard Biesheuvel]

On 19 September 2017 at 01:07, Sakar Arora  wrote:
> This change will create the possibility for memory space holding the UEFI 
> image to be over-written by the DXE core code, since this space will then be 
> available for allocation.

Yes. But why does this matter after the entire payload has been
decompressed into memory already?


> Any such change, if required, should be done just before booting the OS.
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Meenakshi Aggarwal
> Sent: Tuesday, September 19, 2017 6:02 PM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; 
> ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
>
> While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
> whereas this memory can be used by OS.
>
> This patch, allows OS to use UEFI code area.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 
> -
>  1 file changed, 69 deletions(-)
>
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c 
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index 2feb11f..d03214b 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -70,11 +70,7 @@ MemoryPeim (
>  {
>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> -  UINT64   ResourceLength;
>EFI_PEI_HOB_POINTERS NextHob;
> -  EFI_PHYSICAL_ADDRESS FdTop;
> -  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> -  EFI_PHYSICAL_ADDRESS ResourceTop;
>BOOLEAN  Found;
>
>// Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6 @@ 
> MemoryPeim (
>  );
>}
>
> -  //
> -  // Reserved the memory space occupied by the firmware volume
> -  //
> -
> -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + 
> (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
> -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + 
> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> -
> -  // EDK2 does not have the concept of boot firmware copied into DRAM. To 
> avoid the DXE
> -  // core to overwrite this area we must mark the region with the attribute 
> non-present
> -  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && 
> (FdTop <= SystemMemoryTop)) {
> -Found = FALSE;
> -
> -// Search for System Memory Hob that contains the firmware
> -NextHob.Raw = GetHobList ();
> -while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, 
> NextHob.Raw)) != NULL) {
> -  if ((NextHob.ResourceDescriptor->ResourceType == 
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> -  (PcdGet64 (PcdFdBaseAddress) >= 
> NextHob.ResourceDescriptor->PhysicalStart) &&
> -  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + 
> NextHob.ResourceDescriptor->ResourceLength))
> -  {
> -ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
> -ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> -ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + 
> ResourceLength;
> -
> -if (PcdGet64 (PcdFdBaseAddress) == 
> NextHob.ResourceDescriptor->PhysicalStart) {
> -  if (SystemMemoryTop == FdTop) {
> -NextHob.ResourceDescriptor->ResourceAttribute = 
> ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
> -  } else {
> -// Create the System Memory HOB for the firmware with the 
> non-present attribute
> -BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -ResourceAttributes & 
> ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
> -PcdGet64 (PcdFdBaseAddress),
> -PcdGet32 (PcdFdSize));
> -
> -// Top of the FD is system memory available for UEFI
> -NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
> -NextHob.ResourceDescriptor->ResourceLength -= 
> PcdGet32(PcdFdSize);
> -  }
> -} else {
> -  // Create the System Memory HOB for the firmware with the 
> non-present attribute
> -  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -  ResourceAttributes & 
> ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
> -  PcdGet64 (PcdFdBaseAddress),
> -  PcdGet32 (PcdFdSize));
> -
> -  // Update the HOB
> -  NextHob.ResourceDescriptor->ResourceLength = PcdGet64 
> (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
> -
> -  // If there is some memory 

Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

[Udit Kumar]

Thanks for this, 

If memory is marked as reserved by HobList then gDS->AddMemorySpace will return 
 error 15. No ? 

Regards
Udit


> -Original Message-
> From: Sakar Arora [mailto:sakar.ar...@arm.com]
> Sent: Tuesday, September 19, 2017 4:51 PM
> To: Udit Kumar ; Meenakshi Aggarwal
> ; edk2-devel@lists.01.org;
> leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: RE: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> 
> You could use the DXE services method gDS->AddMemorySpace to add this
> memory space to the global gMemoryMap. This is the memory map linux efi
> stub reads to get information about all the available system memory.
> 
> Thanks,
> Sakar
> 
> -Original Message-
> From: Udit Kumar [mailto:udit.ku...@nxp.com]
> Sent: Tuesday, September 19, 2017 3:41 PM
> To: Sakar Arora ; Meenakshi Aggarwal
> ; edk2-devel@lists.01.org;
> leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: RE: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> 
> Thanks Sakar,
> I am wondering, how we can add this memory before OS,  please suggest.
> I guess, we cannot add in HobList when we are in DXE or BDS.
> 
> Regards
> Udit
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Sakar Arora
> > Sent: Tuesday, September 19, 2017 1:37 PM
> > To: Meenakshi Aggarwal ; edk2-
> > de...@lists.01.org; leif.lindh...@linaro.org;
> > ard.biesheu...@linaro.org
> > Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> >
> > This change will create the possibility for memory space holding the
> > UEFI image to be over-written by the DXE core code, since this space
> > will then be available for allocation. Any such change, if required,
> > should be done just before booting the OS.
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Meenakshi Aggarwal
> > Sent: Tuesday, September 19, 2017 6:02 PM
> > To: edk2-devel@lists.01.org; leif.lindh...@linaro.org;
> > ard.biesheu...@linaro.org
> > Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> >
> > While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
> > whereas this memory can be used by OS.
> >
> > This patch, allows OS to use UEFI code area.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Udit Kumar 
> > Signed-off-by: Meenakshi Aggarwal 
> > ---
> >  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69
> > -
> >  1 file changed, 69 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> > b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> > index 2feb11f..d03214b 100644
> > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> > @@ -70,11 +70,7 @@ MemoryPeim (
> >  {
> >ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> >EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> > -  UINT64   ResourceLength;
> >EFI_PEI_HOB_POINTERS NextHob;
> > -  EFI_PHYSICAL_ADDRESS FdTop;
> > -  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> > -  EFI_PHYSICAL_ADDRESS ResourceTop;
> >BOOLEAN  Found;
> >
> >// Get Virtual Memory Map from the Platform Library @@ -121,71
> > +117,6 @@ MemoryPeim (
> >  );
> >}
> >
> > -  //
> > -  // Reserved the memory space occupied by the firmware volume
> > -  //
> > -
> > -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> > (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
> > (PcdSystemMemorySize);
> > -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
> > (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> > -
> > -  // EDK2 does not have the concept of boot firmware copied into
> > DRAM. To avoid the DXE
> > -  // core to overwrite this area we must mark the region with the
> > attribute non- present
> > -  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase))
> > && (FdTop <= SystemMemoryTop)) {
> > -Found = FALSE;
> > -
> > -// Search for System Memory Hob that contains the firmware
> > -NextHob.Raw = GetHobList ();
> > -while ((NextHob.Raw = GetNextHob
> > (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
> > -  if ((NextHob.ResourceDescriptor->ResourceType ==
> > EFI_RESOURCE_SYSTEM_MEMORY) &&
> > -  (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor-
> > >PhysicalStart) &&
> > -  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
> > NextHob.ResourceDescriptor->ResourceLength))
> > -  {
> > -ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
> > -ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> > -ResourceTop = 

[edk2] [PATCH 6/6] MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift

[Hao Wu]

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

Within function AhciModeInitialization(), left shift operations of 'BIT0'
in the following statements:
"if ((PortImplementBitMap & (BIT0 << Port)) != 0) {"

will incur possible out of range left shift when Port is 31, since
"1 << 31" is possible to exceed the range of type 'int' (signed).

According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>   bits are filled with zeros. If E1 has an unsigned type, the value
>   of the result is E1 * 2^E2 , reduced modulo one more than the
>   maximum value representable in the result type. If E1 has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the
>   result type, then that is the resulting value; otherwise, the
>   behavior is undefined.

This commit explicitly cast 'BIT0' with UINT32 to resolve this issue.

Cc: Steven Shi 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index b954de8101..e6de5d65bc 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -1,7 +1,7 @@
 /** @file
   The file for AHCI mode of ATA host controller.
 
-  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2015 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -2314,7 +2314,7 @@ AhciModeInitialization (
   }
 
   for (Port = 0; Port < EFI_AHCI_MAX_PORTS; Port ++) {
-if ((PortImplementBitMap & (BIT0 << Port)) != 0) {
+if ((PortImplementBitMap & (((UINT32)BIT0) << Port)) != 0) {
   //
   // According to AHCI spec, MaxPortNumber should be equal or greater than 
the number of implemented ports.
   //
-- 
2.12.0.windows.1

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

[edk2] [PATCH 2/6] MdeModulePkg/PrintLib: Fix possible negative value left shift

[Hao Wu]

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

Within function InternalPrintLibSPrintMarker(), possible left shift of a
negative value is found in:
"(*(ArgumentString + 1) << 8)"

which involves undefined behavior.

Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be
promoted to type int (signed) during the left shift operation. If
'*(ArgumentString + 1)' is a negative value, the behavior will be
undefined.

According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>   bits are filled with zeros. If E1 has an unsigned type, the value
>   of the result is E1 * 2^E2 , reduced modulo one more than the
>   maximum value representable in the result type. If E1 has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the
>   result type, then that is the resulting value; otherwise, the
>   behavior is undefined.

This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve
this issue.

Cc: Steven Shi 
Cc: Michael Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c 
b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
index b58db8e011..56534e56c3 100644
--- a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
+++ b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
@@ -2108,7 +2108,7 @@ InternalPrintLibSPrintMarker (
 // Copy the string into the output buffer performing the required type 
conversions
 //
 while (Index < Count) {
-  ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1) 
<< 8)) & ArgumentMask;
+  ArgumentCharacter = ((*ArgumentString & 0xff) | 
(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask;
 
   LengthToReturn += (1 * BytesPerOutputCharacter);
   if ((Flags & COUNT_ONLY_NO_PRINT) == 0 && Buffer != NULL) {
-- 
2.12.0.windows.1

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

[edk2] [PATCH 3/6] MdeModulePkg/Tpl: Fix negative value left shift

[Hao Wu]

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

Within function CoreRestoreTpl(), left shift a negative value -2 is used
in:
"while (((-2 << NewTpl) & gEventPending) != 0) {"

which involves undefined behavior.

According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>   bits are filled with zeros. If E1 has an unsigned type, the value
>   of the result is E1 * 2^E2 , reduced modulo one more than the
>   maximum value representable in the result type. If E1 has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the
>   result type, then that is the resulting value; otherwise, the
>   behavior is undefined.

This commit explicitly cast '-2' with UINTN to resolve this issue.

Cc: Steven Shi 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/Event/Tpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Event/Tpl.c 
b/MdeModulePkg/Core/Dxe/Event/Tpl.c
index 8ad0a33701..8c50f61117 100644
--- a/MdeModulePkg/Core/Dxe/Event/Tpl.c
+++ b/MdeModulePkg/Core/Dxe/Event/Tpl.c
@@ -123,7 +123,7 @@ CoreRestoreTpl (
   //
   // Dispatch any pending events
   //
-  while (((-2 << NewTpl) & gEventPending) != 0) {
+  while (UINTN)-2) << NewTpl) & gEventPending) != 0) {
 gEfiCurrentTpl = (UINTN) HighBitSet64 (gEventPending);
 if (gEfiCurrentTpl < TPL_HIGH_LEVEL) {
   CoreSetInterruptState (TRUE);
-- 
2.12.0.windows.1

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

[edk2] [PATCH 0/6] Resolve undefined behaviours in left shift OPs

[Hao Wu]

The series resolves two kinds of undefined behaviours in left shift
operations:
  a. Left-shifting negative values;
  b. Left-shifting that incurs the result being out of range.

Cc: Steven Shi 
Cc: Michael Kinney 
Cc: Liming Gao 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Wu Jiaxin 

Hao Wu (6):
  MdePkg/PrintLib: Fix possible negative value left shift
  MdeModulePkg/PrintLib: Fix possible negative value left shift
  MdeModulePkg/Tpl: Fix negative value left shift
  MdeModulePkg/DxeNetLib: Fix negative value left shift
  MdeModulePkg/Crc32: Fix possible out of range left shift
  MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c  | 4 ++--
 MdeModulePkg/Core/Dxe/Event/Tpl.c | 2 +-
 MdeModulePkg/Core/RuntimeDxe/Crc32.c  | 6 +++---
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c| 2 +-
 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 2 +-
 MdePkg/Library/BasePrintLib/PrintLibInternal.c| 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.12.0.windows.1

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

[edk2] [PATCH 5/6] MdeModulePkg/Crc32: Fix possible out of range left shift

[Hao Wu]

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

Within function ReverseBits(), left shift operations of 1 in the
following statements:
"(1 << Index)" and "(1 << (31 - Index))"

will incur possible out of range left shift when Index is either 0 or
31, since "1 << 31" is possible to exceed the range of type 'int'
(signed).

According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>   bits are filled with zeros. If E1 has an unsigned type, the value
>   of the result is E1 * 2^E2 , reduced modulo one more than the
>   maximum value representable in the result type. If E1 has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the
>   result type, then that is the resulting value; otherwise, the
>   behavior is undefined.

This commit explicitly cast '1' with UINT32 to resolve this issue.

Cc: Steven Shi 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/RuntimeDxe/Crc32.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Core/RuntimeDxe/Crc32.c 
b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
index a6fe77fa34..5cee66ebd1 100644
--- a/MdeModulePkg/Core/RuntimeDxe/Crc32.c
+++ b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
@@ -7,7 +7,7 @@
   EFI Runtime Services Table are converted from physical address to
   virtual addresses.  This requires that the 32-bit CRC be recomputed.
 
-Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -79,8 +79,8 @@ ReverseBits (
 
   NewValue = 0;
   for (Index = 0; Index < 32; Index++) {
-if ((Value & (1 << Index)) != 0) {
-  NewValue = NewValue | (1 << (31 - Index));
+if ((Value & (((UINT32)1) << Index)) != 0) {
+  NewValue = NewValue | (((UINT32)1) << (31 - Index));
 }
   }
 
-- 
2.12.0.windows.1

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

[edk2] [PATCH 4/6] MdeModulePkg/DxeNetLib: Fix negative value left shift

[Hao Wu]

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

Within function NetRandomInitSeed(), left shift a negative value is used
in:
"~Time.Hour << 24"

which involves undefined behavior.

Since Time.Hour is of type UINT8 (range from 0 to 23), hence ~Time.Hour
will be a negative value (of type int, signed).

According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>   bits are filled with zeros. If E1 has an unsigned type, the value
>   of the result is E1 * 2^E2 , reduced modulo one more than the
>   maximum value representable in the result type. If E1 has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the
>   result type, then that is the resulting value; otherwise, the
>   behavior is undefined.

This commit explicitly cast 'Time.Hour' with UINT32 to resolve this issue.

Cc: Steven Shi 
Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c 
b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
index 7cd7e3aca0..ca5413edcc 100644
--- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
+++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
@@ -872,7 +872,7 @@ NetRandomInitSeed (
   UINT64MonotonicCount;
 
   gRT->GetTime (, NULL);
-  Seed = (~Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | Time.Second);
+  Seed = (~(UINT32)Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | 
Time.Second);
   Seed ^= Time.Nanosecond;
   Seed ^= Time.Year << 7;
 
-- 
2.12.0.windows.1

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

[edk2] [PATCH 1/6] MdePkg/PrintLib: Fix possible negative value left shift

[Hao Wu]

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

Within function InternalPrintLibSPrintMarker(), possible left shift of a
negative value is found in:
"(*(ArgumentString + 1) << 8)"

which involves undefined behavior.

Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be
promoted to type int (signed) during the left shift operation. If
'*(ArgumentString + 1)' is a negative value, the behavior will be
undefined.

According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>   bits are filled with zeros. If E1 has an unsigned type, the value
>   of the result is E1 * 2^E2 , reduced modulo one more than the
>   maximum value representable in the result type. If E1 has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the
>   result type, then that is the resulting value; otherwise, the
>   behavior is undefined.

This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve
this issue.

Cc: Steven Shi 
Cc: Michael Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdePkg/Library/BasePrintLib/PrintLibInternal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c 
b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
index cec5b3bc99..28d946472f 100644
--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
@@ -1165,7 +1165,7 @@ BasePrintLibSPrintMarker (
 // Copy the string into the output buffer performing the required type 
conversions
 //
 while (Index < Count) {
-  ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1) 
<< 8)) & ArgumentMask;
+  ArgumentCharacter = ((*ArgumentString & 0xff) | 
(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask;
 
   LengthToReturn += (1 * BytesPerOutputCharacter);
   if ((Flags & COUNT_ONLY_NO_PRINT) == 0 && Buffer != NULL) {
-- 
2.12.0.windows.1

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

[edk2] [PATCH 1/2] ShellPkg/Shell: Avoid reading content beyond string boundary

[Hao Wu]

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

Within function EfiShellGetDevicePathFromFilePath(), when the input
parameter 'Path' string is like:
"FS0:"

It is possible for the below statement:
"if (*(Path+StrLen(MapName)+1) == CHAR_NULL) {"

to read the content 1 byte beyond the string boundary (both 'Path' and
'MapName' will be FS0: in this case).

This commit adds additional checks to avoid this.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Cc: Steven Shi 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 ShellPkg/Application/Shell/ShellProtocol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/ShellProtocol.c 
b/ShellPkg/Application/Shell/ShellProtocol.c
index 40e5e653ae..5e34b8dad1 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -598,7 +598,8 @@ EfiShellGetDevicePathFromFilePath(
   //
   // build the full device path
   //
-  if (*(Path+StrLen(MapName)+1) == CHAR_NULL) {
+  if ((*(Path+StrLen(MapName)) != CHAR_NULL) &&
+  (*(Path+StrLen(MapName)+1) == CHAR_NULL)) {
 DevicePathForReturn = FileDevicePath(Handle, L"\\");
   } else {
 DevicePathForReturn = FileDevicePath(Handle, Path+StrLen(MapName));
-- 
2.12.0.windows.1

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

[edk2] [PATCH 2/2] MdePkg/BaseLib: Avoid reading content beyond string boundary

[Hao Wu]

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

As mentioned in the above Bugzilla link by Steven, within the function
PathCleanUpDirectories(), when executing command:
"cd ."

under Shell, the input parameter 'Path' string will have string length
less than 2. Hence, it is possible for the below statement:
"if (StrCmp (Path + StrLen (Path) - 2, L"\\.") == 0) {"

to read contents before the string boundary.

This commit adds additional checks to avoid this.

Cc: Ruiyu Ni 
Cc: Steven Shi 
Cc: Michael Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdePkg/Library/BaseLib/FilePaths.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BaseLib/FilePaths.c 
b/MdePkg/Library/BaseLib/FilePaths.c
index 203045ccdc..d6f3758ecb 100644
--- a/MdePkg/Library/BaseLib/FilePaths.c
+++ b/MdePkg/Library/BaseLib/FilePaths.c
@@ -1,7 +1,7 @@
 /** @file
   Defines file-path manipulation functions.
 
-  Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -91,7 +91,7 @@ PathCleanUpDirectories(
   while ((TempString = StrStr (Path, L"\\.\\")) != NULL) {
 CopyMem (TempString, TempString + 2, StrSize (TempString + 2));
   }
-  if (StrCmp (Path + StrLen (Path) - 2, L"\\.") == 0) {
+  if ((StrLen (Path) >= 2) && (StrCmp (Path + StrLen (Path) - 2, L"\\.") == 
0)) {
 Path[StrLen (Path) - 1] = CHAR_NULL;
   }
 
-- 
2.12.0.windows.1

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

[edk2] [PATCH 0/2] Avoid consuming content beyond string boundaries

[Hao Wu]

The series adds checks to avoid reading content beyond string boundaries.

Cc: Steven Shi 
Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Cc: Michael Kinney 
Cc: Liming Gao 


Hao Wu (2):
  ShellPkg/Shell: Avoid reading content beyond string boundary
  MdePkg/BaseLib: Avoid reading content beyond string boundary

 MdePkg/Library/BaseLib/FilePaths.c | 4 ++--
 ShellPkg/Application/Shell/ShellProtocol.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.12.0.windows.1

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

Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

[Sakar Arora]

You could use the DXE services method gDS->AddMemorySpace to add this memory 
space to the global gMemoryMap. This is the memory map linux efi stub reads to 
get information about all the available system memory.

Thanks,
Sakar

-Original Message-
From: Udit Kumar [mailto:udit.ku...@nxp.com]
Sent: Tuesday, September 19, 2017 3:41 PM
To: Sakar Arora ; Meenakshi Aggarwal 
; edk2-devel@lists.01.org; 
leif.lindh...@linaro.org; ard.biesheu...@linaro.org
Subject: RE: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

Thanks Sakar,
I am wondering, how we can add this memory before OS,  please suggest.
I guess, we cannot add in HobList when we are in DXE or BDS.

Regards
Udit

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Sakar Arora
> Sent: Tuesday, September 19, 2017 1:37 PM
> To: Meenakshi Aggarwal ; edk2-
> de...@lists.01.org; leif.lindh...@linaro.org;
> ard.biesheu...@linaro.org
> Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
>
> This change will create the possibility for memory space holding the
> UEFI image to be over-written by the DXE core code, since this space
> will then be available for allocation. Any such change, if required,
> should be done just before booting the OS.
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Meenakshi Aggarwal
> Sent: Tuesday, September 19, 2017 6:02 PM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org;
> ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
>
> While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
> whereas this memory can be used by OS.
>
> This patch, allows OS to use UEFI code area.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69
> -
>  1 file changed, 69 deletions(-)
>
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index 2feb11f..d03214b 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -70,11 +70,7 @@ MemoryPeim (
>  {
>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> -  UINT64   ResourceLength;
>EFI_PEI_HOB_POINTERS NextHob;
> -  EFI_PHYSICAL_ADDRESS FdTop;
> -  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> -  EFI_PHYSICAL_ADDRESS ResourceTop;
>BOOLEAN  Found;
>
>// Get Virtual Memory Map from the Platform Library @@ -121,71
> +117,6 @@ MemoryPeim (
>  );
>}
>
> -  //
> -  // Reserved the memory space occupied by the firmware volume
> -  //
> -
> -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemorySize);
> -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> -
> -  // EDK2 does not have the concept of boot firmware copied into
> DRAM. To avoid the DXE
> -  // core to overwrite this area we must mark the region with the
> attribute non- present
> -  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase))
> && (FdTop <= SystemMemoryTop)) {
> -Found = FALSE;
> -
> -// Search for System Memory Hob that contains the firmware
> -NextHob.Raw = GetHobList ();
> -while ((NextHob.Raw = GetNextHob
> (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
> -  if ((NextHob.ResourceDescriptor->ResourceType ==
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> -  (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor-
> >PhysicalStart) &&
> -  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
> NextHob.ResourceDescriptor->ResourceLength))
> -  {
> -ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
> -ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> -ResourceTop = NextHob.ResourceDescriptor->PhysicalStart +
> ResourceLength;
> -
> -if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor-
> >PhysicalStart) {
> -  if (SystemMemoryTop == FdTop) {
> -NextHob.ResourceDescriptor->ResourceAttribute = 
> ResourceAttributes
> & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
> -  } else {
> -// Create the System Memory HOB for the firmware with the non-
> present attribute
> -BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -ResourceAttributes &
> ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
> -PcdGet64 (PcdFdBaseAddress),
> -   

Re: [edk2] Failed to clear configuration in Ip4Config2 Protocol

[Karunakar P]

Hi Jiaxin,

Many thanks for your help.

After the successful HTTP boot DNS parsing, I can see the DNS Node, But it have 
bit difference form UEFI2.7 spec
//../MAC(D8CB8ADEBBAA,0x0)/IPv4(0.0.0.0)/Dns(192.168.184.1)/Uri(https://www.cloudboot.com:443/EFI/Shell.efi)
//../MAC(D8CB8ADEBBAA,0x0)/IPv6(2001:0DB8::0001::::0001)/Dns(2001:0DB8::0001::::0001)/Uri(https://www.cloudbootip6.com:443/EFI/Shell.efi)

I can see Only DNS Server IP address in DNS Node i.e. Dns(192.168.184.1)
But UEFI2.7 spec says(24.7.3.1 Device Path, page 1329) like below
Dns(192.168.22.100, 192.168.22.101) for IPv4
Dns(2016::100, 2016::101   for IPv6,  DNS node have DNS Server IP and some 
other IP address too.

Why I'm getting this difference, is there anything wrong or I'm I missing 
anything?  

Thanks,
Karunakar

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com] 
Sent: Monday, September 18, 2017 10:28 AM
To: Karunakar P; 'edk2-devel@lists.01.org'; Ye, Ting
Subject: RE: [edk2] Failed to clear configuration in Ip4Config2 Protocol

Hi karunakar,

You can verify the DNS device path with HTTP boot feature. After the successful 
HTTP boot DNS parsing,  the device path should be like: 
//../Mac(...)[/Vlan(...)][/Wi-Fi(...)]/IPv4(...)[/Dns(...)]/Uri(...).

That is recommend way for the verification.

Besides, you can also draft the App to call  the DevPathFromTextDns and 
DevPathToTextDns libraries for the more verification.

Thanks,
Jiaxin
  

> -Original Message-
> From: Karunakar P [mailto:karunak...@amiindia.co.in]
> Sent: Monday, September 18, 2017 12:22 PM
> To: Wu, Jiaxin ; 'edk2-devel@lists.01.org'  de...@lists.01.org>; Ye, Ting 
> Subject: RE: [edk2] Failed to clear configuration in Ip4Config2 
> Protocol
> 
> Hi Jiaxin,
> 
> Thank you very much for your info, Yes it works fine for manual configuration.
> 
> And also could you please provide steps to verify "Add DNS device path 
> node" feature.
> 
> Thanks,
> karunakar
> 
> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Monday, September 18, 2017 7:46 AM
> To: Karunakar P; 'edk2-devel@lists.01.org'; Ye, Ting
> Subject: RE: [edk2] Failed to clear configuration in Ip4Config2 
> Protocol
> 
> Hi Karunakar,
> 
> According the UEFI Spec, the Ip4Config2DataTypeManualAddress, 
> Ip4Config2DataTypeGateway and Ip4Config2DataTypeDnsServer 
> configuration data are not allowed to set via SetData() if the policy is DHCP.
> So, the clear feature is only for the manual configuration. This is 
> our design purpose and also the reason why the feature is not apply to 
> the Ip4Config2DataTypeInterfaceInfo/Ip4Config2DataTypePolicy.
> 
> Thanks,
> Jiaxin
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Karunakar P
> > Sent: Friday, September 15, 2017 5:41 PM
> > To: 'edk2-devel@lists.01.org' 
> > Subject: Re: [edk2] Failed to clear configuration in Ip4Config2 
> > Protocol
> >
> > Hello All,
> >
> > Could you please anyone provide comment on this?
> >
> > Thank you,
> > karunakar
> >
> > From: Karunakar P
> > Sent: Wednesday, September 13, 2017 7:04 PM
> > To: 'edk2-devel@lists.01.org'
> > Subject: RE: RE: Failed to clear configuration in Ip4Config2 
> > Protocol
> >
> > Hello All,
> >
> > I was trying to verify the feature "Allow SetData to clear 
> > configuration in Ip4Config2/Ip6Config Protocol" , But SetData 
> > returns with Write Protected Error Status
> >
> > [Steps followed]
> >
> > 1.   I've taken the above feature changes.
> >
> > 2.   I've a UEFI test Application which call to SetData with DataSize 
> > is 0 and
> > Data is NULL
> >
> > Status = Ip4Cfg2->SetData (
> >
> > Ip4Cfg2,
> >
> > Ip4Config2DataTypeManualAddress,
> >
> > 0,
> >
> > 0
> >
> > );
> >
> > 3.   But SetData returns with Write Protected Error Status// 
> > Status =
> > Write Protected
> >
> > 4.Faced the same error for setting Gateway & DnsServer
> >
> > Guess the return is happening from
> > Ip4Config2SetManualAddress() ->
> > ...
> >   if (Instance->Policy != Ip4Config2PolicyStatic) {
> > return EFI_WRITE_PROTECTED;
> >   }
> > ...
> >
> > Could you please help on this whether am I missing anything or 
> > anything else need to be done to resolve this?
> >
> > Thanks,
> > karunakar
> >
> >
> > From: Karunakar P
> > Sent: Wednesday, September 13, 2017 7:00 PM
> > To: edk2-devel@lists.01.org
> > Subject: RE: Failed to clear configuration in Ip4Config2 Protocol
> >
> > Hello All,
> >
> > I was trying to verify the feature "Allow SetData to clear 
> > configuration in Ip4Config2/Ip6Config Protocol" , But SetData 
> > returns with Write Protected Error Status
> >
> > [Steps followed]
> >
> > 1.   

Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

[Udit Kumar]

Thanks Sakar, 
I am wondering, how we can add this memory before OS,  please suggest. 
I guess, we cannot add in HobList when we are in DXE or BDS.

Regards
Udit 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Sakar
> Arora
> Sent: Tuesday, September 19, 2017 1:37 PM
> To: Meenakshi Aggarwal ; edk2-
> de...@lists.01.org; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> 
> This change will create the possibility for memory space holding the UEFI 
> image
> to be over-written by the DXE core code, since this space will then be 
> available
> for allocation. Any such change, if required, should be done just before 
> booting
> the OS.
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Meenakshi Aggarwal
> Sent: Tuesday, September 19, 2017 6:02 PM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org;
> ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> 
> While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
> whereas this memory can be used by OS.
> 
> This patch, allows OS to use UEFI code area.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 
> -
>  1 file changed, 69 deletions(-)
> 
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index 2feb11f..d03214b 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -70,11 +70,7 @@ MemoryPeim (
>  {
>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> -  UINT64   ResourceLength;
>EFI_PEI_HOB_POINTERS NextHob;
> -  EFI_PHYSICAL_ADDRESS FdTop;
> -  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> -  EFI_PHYSICAL_ADDRESS ResourceTop;
>BOOLEAN  Found;
> 
>// Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6 @@
> MemoryPeim (
>  );
>}
> 
> -  //
> -  // Reserved the memory space occupied by the firmware volume
> -  //
> -
> -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemorySize);
> -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> -
> -  // EDK2 does not have the concept of boot firmware copied into DRAM. To
> avoid the DXE
> -  // core to overwrite this area we must mark the region with the attribute 
> non-
> present
> -  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) &&
> (FdTop <= SystemMemoryTop)) {
> -Found = FALSE;
> -
> -// Search for System Memory Hob that contains the firmware
> -NextHob.Raw = GetHobList ();
> -while ((NextHob.Raw = GetNextHob
> (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
> -  if ((NextHob.ResourceDescriptor->ResourceType ==
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> -  (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor-
> >PhysicalStart) &&
> -  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
> NextHob.ResourceDescriptor->ResourceLength))
> -  {
> -ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
> -ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> -ResourceTop = NextHob.ResourceDescriptor->PhysicalStart +
> ResourceLength;
> -
> -if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor-
> >PhysicalStart) {
> -  if (SystemMemoryTop == FdTop) {
> -NextHob.ResourceDescriptor->ResourceAttribute = 
> ResourceAttributes
> & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
> -  } else {
> -// Create the System Memory HOB for the firmware with the non-
> present attribute
> -BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -ResourceAttributes &
> ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
> -PcdGet64 (PcdFdBaseAddress),
> -PcdGet32 (PcdFdSize));
> -
> -// Top of the FD is system memory available for UEFI
> -NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
> -NextHob.ResourceDescriptor->ResourceLength -= 
> PcdGet32(PcdFdSize);
> -  }
> -} else {
> -  // Create the System Memory HOB for the firmware with the 
> non-present
> attribute
> -  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -  ResourceAttributes &
> 

Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

[Sakar Arora]

This change will create the possibility for memory space holding the UEFI image 
to be over-written by the DXE core code, since this space will then be 
available for allocation. Any such change, if required, should be done just 
before booting the OS.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Meenakshi Aggarwal
Sent: Tuesday, September 19, 2017 6:02 PM
To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
whereas this memory can be used by OS.

This patch, allows OS to use UEFI code area.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar 
Signed-off-by: Meenakshi Aggarwal 
---
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 -
 1 file changed, 69 deletions(-)

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c 
b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
index 2feb11f..d03214b 100644
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
@@ -70,11 +70,7 @@ MemoryPeim (
 {
   ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
   EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
-  UINT64   ResourceLength;
   EFI_PEI_HOB_POINTERS NextHob;
-  EFI_PHYSICAL_ADDRESS FdTop;
-  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
-  EFI_PHYSICAL_ADDRESS ResourceTop;
   BOOLEAN  Found;

   // Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6 @@ 
MemoryPeim (
 );
   }

-  //
-  // Reserved the memory space occupied by the firmware volume
-  //
-
-  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + 
(EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
-  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + 
(EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
-
-  // EDK2 does not have the concept of boot firmware copied into DRAM. To 
avoid the DXE
-  // core to overwrite this area we must mark the region with the attribute 
non-present
-  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && 
(FdTop <= SystemMemoryTop)) {
-Found = FALSE;
-
-// Search for System Memory Hob that contains the firmware
-NextHob.Raw = GetHobList ();
-while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, 
NextHob.Raw)) != NULL) {
-  if ((NextHob.ResourceDescriptor->ResourceType == 
EFI_RESOURCE_SYSTEM_MEMORY) &&
-  (PcdGet64 (PcdFdBaseAddress) >= 
NextHob.ResourceDescriptor->PhysicalStart) &&
-  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + 
NextHob.ResourceDescriptor->ResourceLength))
-  {
-ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
-ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
-ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + 
ResourceLength;
-
-if (PcdGet64 (PcdFdBaseAddress) == 
NextHob.ResourceDescriptor->PhysicalStart) {
-  if (SystemMemoryTop == FdTop) {
-NextHob.ResourceDescriptor->ResourceAttribute = ResourceAttributes 
& ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
-  } else {
-// Create the System Memory HOB for the firmware with the 
non-present attribute
-BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-ResourceAttributes & 
~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-PcdGet64 (PcdFdBaseAddress),
-PcdGet32 (PcdFdSize));
-
-// Top of the FD is system memory available for UEFI
-NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
-NextHob.ResourceDescriptor->ResourceLength -= PcdGet32(PcdFdSize);
-  }
-} else {
-  // Create the System Memory HOB for the firmware with the 
non-present attribute
-  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-  ResourceAttributes & 
~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-  PcdGet64 (PcdFdBaseAddress),
-  PcdGet32 (PcdFdSize));
-
-  // Update the HOB
-  NextHob.ResourceDescriptor->ResourceLength = PcdGet64 
(PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
-
-  // If there is some memory available on the top of the FD then 
create a HOB
-  if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + 
ResourceLength) {
-// Create the System Memory HOB for the remaining region (top of 
the FD)
-BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-ResourceAttributes,
-

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

[Zeng, Star]

Thanks all the Tested-by and Reviewed-by.

The new UEFI spec 2.7a has been published at 
http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_7_A%20Sept%206.pdf
which includes the spec change "1815 OpenProtocol() / EFI_ALREADY_STARTED 
should output existent Interface" that is required for this patch.

If no more comments, I will go to push this patch. :)


Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, June 28, 2017 10:14 PM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Amit Kumar ; Kinney, Michael D 
; Gao, Liming ; Gabriel Somlo 

Subject: Re: [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by 
CoreOpenProtocol

On 06/28/17 15:22, Star Zeng wrote:
> From: Amit Kumar 
> 
> Change since v4: Revise the patch based on V4 sent by Amit Kumar
> 1) Only return the corresponding protocol interface in *Interface if 
> the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
> 2) Interface is returned unmodified for all error conditions except 
> EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in 
> *Interface when EFI_UNSUPPORTED and Attributes is not 
> EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be 
> returned in *Interface when EFI_ALREADY_STARTED.
> 
> Change since v3:
> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL and 
> Inteface = NULL case. [Reported by:star.zeng at intel.com]
> 
> Change Since v2:
> 1) Modified to use EFI_ERROR to get status code
> 
> Change since v1:
> 1) Fixed typo protocal to protocol
> 2) Fixed coding style
> 
> Cc: Laszlo Ersek 
> Cc: Amit Kumar 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Gabriel Somlo 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Amit Kumar 
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 
> +++-
>  1 file changed, 23 insertions(+), 13 deletions(-)

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

Thanks!
Laszlo

> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c 
> b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 59b89148c8f0..3862a3876f4a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -1006,12 +1006,8 @@ CoreOpenProtocol (
>//
>// Check for invalid Interface
>//
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -if (Interface == NULL) {
> -  return EFI_INVALID_PARAMETER;
> -} else {
> -  *Interface = NULL;
> -}
> +  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == 
> NULL)) {
> +return EFI_INVALID_PARAMETER;
>}
>  
>//
> @@ -1078,12 +1074,6 @@ CoreOpenProtocol (
>  goto Done;
>}
>  
> -  //
> -  // This is the protocol interface entry for this protocol
> -  //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -*Interface = Prot->Interface;
> -  }
>Status = EFI_SUCCESS;
>  
>ByDriver= FALSE;
> @@ -1177,8 +1167,28 @@ CoreOpenProtocol (
>}
>  
>  Done:
> +
> +  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> +//
> +// Keep Interface unmodified in case of any Error
> +// except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
> +//
> +if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
> +  //
> +  // EFI_ALREADY_STARTED is not an error for bus driver.
> +  // Return the corresponding protocol interface. 
> +  //
> +  *Interface = Prot->Interface;
> +} else if (Status == EFI_UNSUPPORTED) {
> +  //
> +  // Return NULL Interface if Unsupported Protocol.
> +  //
> +  *Interface = NULL;
> +}
> +  }
> +
>//
> -  // Done. Release the database lock are return
> +  // Done. Release the database lock and return
>//
>CoreReleaseProtocolLock ();
>return Status;
> 

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

Re: [edk2] [Patch V2] BaseTools: Fix a bug to correct SourceFileList

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhu, Yonghong
>Sent: Monday, September 18, 2017 12:24 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: [Patch V2] BaseTools: Fix a bug to correct SourceFileList
>
>We met a case that use two microcode files in the Microcode.inf file,
>one is .mcb file, another is .txt file. then it cause build failure
>because the SourceFileList include the .txt file's output file, while
>this output file is still not be generated, so it cause
>GetFileDependency report failure.
>
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> BaseTools/Source/Python/AutoGen/GenMake.py | 7 +++
> 1 file changed, 7 insertions(+)
>
>diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
>b/BaseTools/Source/Python/AutoGen/GenMake.py
>index 0f3ddd5..942eb44 100644
>--- a/BaseTools/Source/Python/AutoGen/GenMake.py
>+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
>@@ -787,12 +787,19 @@ cleanlib:
> ForceIncludedFile = []
> for File in self._AutoGenObject.AutoGenFileList:
> if File.Ext == '.h':
> ForceIncludedFile.append(File)
> SourceFileList = []
>+OutPutFileList = []
> for Target in self._AutoGenObject.IntroTargetList:
> SourceFileList.extend(Target.Inputs)
>+OutPutFileList.extend(Target.Outputs)
>+
>+if OutPutFileList:
>+for Item in OutPutFileList:
>+if Item in SourceFileList:
>+SourceFileList.remove(Item)
>
> self.FileDependency = self.GetFileDependency(
> SourceFileList,
> ForceIncludedFile,
> self._AutoGenObject.IncludePathList +
>self._AutoGenObject.BuildOptionIncPathList
>--
>2.6.1.windows.1

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

[edk2] [Patch V2] Build spec: add description for build with binary cache

[Yonghong Zhu]

V2:
change the option name to --binary-destination and --binary-source.

fixes:https://bugzilla.tianocore.org/show_bug.cgi?id=689
Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 .../82_auto-generation_process.md| 20 
 README.md|  1 +
 appendix_d_buildexe_command/d4_usage.md  | 19 +++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/8_pre-build_autogen_stage/82_auto-generation_process.md 
b/8_pre-build_autogen_stage/82_auto-generation_process.md
index 671a7d5..f2ddf32 100644
--- a/8_pre-build_autogen_stage/82_auto-generation_process.md
+++ b/8_pre-build_autogen_stage/82_auto-generation_process.md
@@ -1031,10 +1031,30 @@ maximum command line length.  The default value is 4096.
 **Note:** The following `FLAGS` options are included in the response file:
 `PP_FLAGS`, `CC_FLAGS`, `VFRPP_FLAGS`, `APP_FLAGS`, `ASLPP_FLAGS`, 
`ASLCC_FLAGS`,
 and `ASM_FLAGS`.
 **
 
+ 8.2.4.15 Build with Binary Cache
+
+**build** tool provides three new options for binary cache feature.
+--hash enables hash-based caching during build process. when --hash is enabled,
+build tool will base on the module hash value to do the incremental build, 
without
+--hash, build tool will base on the timestamp to do the incremental build. 
--hash
+option use md5 method to get every hash value, DSC/FDF, tools_def.txt, 
build_rule.txt
+and build command are calculated as global hash value, Package DEC and its 
include
+header files are calculated as package hash value, Module source files and its 
INF
+file are calculated as module hash value. Library hash value will combine the 
global
+hash value and its dependent package hash value. Driver hash value will 
combine the
+global hash value, its dependent package hash value and its linked library 
hash value.
+When --hash and --binary-destination are specified, build tool will copy the 
generated
+binary files for each module into the directory specified by 
binary-destination at the
+build phase. Binary-destination directory caches all the generated binary 
files.
+When --hash and --binary-source are specified, build tool will try to get the 
binary
+files from the binary source directory at the build phase. If the cached 
binary has
+the same hash value, it will be directly used. Otherwise, build tool will 
compile the
+source files and generate the binary files.
+
 ### 8.2.5 Post processing
 
 Once all files are parsed, the build tools will do following work for each EDK
 II module:
 
diff --git a/README.md b/README.md
index 52abb6a..ca59a35 100644
--- a/README.md
+++ b/README.md
@@ -215,5 +215,6 @@ Copyright (c) 2008-2017, Intel Corporation. All rights 
reserved.
 || [#523](https://bugzilla.tianocore.org/show_bug.cgi?id=523) 
Build spec: add EBNF for the --pcd syntax in the Section D.4


 |   |
 || [#517](https://bugzilla.tianocore.org/show_bug.cgi?id=517) 
Build spec: chapter 5.2.2 Guided Tools add description for Pkcs7Sign tool and 
BrotliCompress tool 

   |   |
 || [#481](https://bugzilla.tianocore.org/show_bug.cgi?id=481) 
Build Spec: add clarification for not used Pcd that build tool will not do 
additional checks on its value  

  |   |
 || [#518](https://bugzilla.tianocore.org/show_bug.cgi?id=518) 
Build Spec: Update Precedence of PCD Values 


 |   |
 || [#669](https://bugzilla.tianocore.org/show_bug.cgi?id=669) 
Build Spec: Add multi-arg support to PREBUILD/POSTBUILD 


 |   |
+|| [#689](https://bugzilla.tianocore.org/show_bug.cgi?id=689) 

[edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

[Meenakshi Aggarwal]

While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
whereas this memory can be used by OS.

This patch, allows OS to use UEFI code area.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar 
Signed-off-by: Meenakshi Aggarwal 
---
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 -
 1 file changed, 69 deletions(-)

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c 
b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
index 2feb11f..d03214b 100644
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
@@ -70,11 +70,7 @@ MemoryPeim (
 {
   ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
   EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
-  UINT64   ResourceLength;
   EFI_PEI_HOB_POINTERS NextHob;
-  EFI_PHYSICAL_ADDRESS FdTop;
-  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
-  EFI_PHYSICAL_ADDRESS ResourceTop;
   BOOLEAN  Found;
 
   // Get Virtual Memory Map from the Platform Library
@@ -121,71 +117,6 @@ MemoryPeim (
 );
   }
 
-  //
-  // Reserved the memory space occupied by the firmware volume
-  //
-
-  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + 
(EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
-  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + 
(EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
-
-  // EDK2 does not have the concept of boot firmware copied into DRAM. To 
avoid the DXE
-  // core to overwrite this area we must mark the region with the attribute 
non-present
-  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && 
(FdTop <= SystemMemoryTop)) {
-Found = FALSE;
-
-// Search for System Memory Hob that contains the firmware
-NextHob.Raw = GetHobList ();
-while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, 
NextHob.Raw)) != NULL) {
-  if ((NextHob.ResourceDescriptor->ResourceType == 
EFI_RESOURCE_SYSTEM_MEMORY) &&
-  (PcdGet64 (PcdFdBaseAddress) >= 
NextHob.ResourceDescriptor->PhysicalStart) &&
-  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + 
NextHob.ResourceDescriptor->ResourceLength))
-  {
-ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
-ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
-ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + 
ResourceLength;
-
-if (PcdGet64 (PcdFdBaseAddress) == 
NextHob.ResourceDescriptor->PhysicalStart) {
-  if (SystemMemoryTop == FdTop) {
-NextHob.ResourceDescriptor->ResourceAttribute = ResourceAttributes 
& ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
-  } else {
-// Create the System Memory HOB for the firmware with the 
non-present attribute
-BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-ResourceAttributes & 
~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-PcdGet64 (PcdFdBaseAddress),
-PcdGet32 (PcdFdSize));
-
-// Top of the FD is system memory available for UEFI
-NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
-NextHob.ResourceDescriptor->ResourceLength -= PcdGet32(PcdFdSize);
-  }
-} else {
-  // Create the System Memory HOB for the firmware with the 
non-present attribute
-  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-  ResourceAttributes & 
~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-  PcdGet64 (PcdFdBaseAddress),
-  PcdGet32 (PcdFdSize));
-
-  // Update the HOB
-  NextHob.ResourceDescriptor->ResourceLength = PcdGet64 
(PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
-
-  // If there is some memory available on the top of the FD then 
create a HOB
-  if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + 
ResourceLength) {
-// Create the System Memory HOB for the remaining region (top of 
the FD)
-BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-ResourceAttributes,
-FdTop,
-ResourceTop - FdTop);
-  }
-}
-Found = TRUE;
-break;
-  }
-  NextHob.Raw = GET_NEXT_HOB (NextHob);
-}
-
-ASSERT(Found);
-  }
-
   // Build Memory Allocation Hob
   InitMmu (MemoryTable);
 
-- 
1.9.1

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

[edk2] [Patch V2] BaseTools: add support for BIOS build with binary cache

[Yonghong Zhu]

V2:
change the option name to --binary-destination and --binary-source.

Add three new options:
--hash enables hash-based caching during build process. when --hash is
enabled, build tool will base on the module hash value to do the
incremental build, without --hash, build tool will base on the
timestamp to do the incremental build. --hash option use md5 method to
get every hash value, DSC/FDF, tools_def.txt, build_rule.txt and build
command are calculated as global hash value, Package DEC and its
include header files are calculated as package hash value, Module
source files and its INF file are calculated as module hash value.
Library hash value will combine the global hash value and its dependent
package hash value. Driver hash value will combine the global hash
value, its dependent package hash value and its linked library hash
value.
When --hash and --binary-destination are specified, build tool will
copy generated binary files for each module into the directory specified
by binary-destination at the build phase. Binary-destination directory
caches all generated binary files.
When --hash and --binary-source are specified, build tool will try to
get the binary files from the binary source directory at the build
phase.If the cached binary has the same hash value, it will be directly
used. Otherwise, build tool will compile the source files and generate
the binary files.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py   | 161 +--
 BaseTools/Source/Python/Common/GlobalData.py |   7 ++
 BaseTools/Source/Python/build/build.py   |  30 +
 3 files changed, 189 insertions(+), 9 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 1a8c0d9..585c382 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -41,10 +41,11 @@ import Common.VpdInfoFile as VpdInfoFile
 from GenPcdDb import CreatePcdDatabaseCode
 from Workspace.MetaFileCommentParser import UsageList
 from Common.MultipleWorkspace import MultipleWorkspace as mws
 import InfSectionParser
 import datetime
+import hashlib
 
 ## Regular expression for splitting Dependency Expression string into tokens
 gDepexTokenPattern = re.compile("(\(|\)|\w+| \S+\.inf)")
 
 #
@@ -263,10 +264,14 @@ class WorkspaceAutoGen(AutoGen):
 self.FdfFile= FlashDefinitionFile
 self.FdTargetList   = Fds
 self.FvTargetList   = Fvs
 self.CapTargetList  = Caps
 self.AutoGenObjectList = []
+self._BuildDir  = None
+self._FvDir = None
+self._MakeFileDir   = None
+self._BuildCommand  = None
 
 # there's many relative directory operations, so ...
 os.chdir(self.WorkspaceDir)
 
 #
@@ -642,10 +647,18 @@ class WorkspaceAutoGen(AutoGen):
 #
 Pa.CollectPlatformDynamicPcds()
 Pa.CollectFixedAtBuildPcds()
 self.AutoGenObjectList.append(Pa)
 
+#
+# Generate Package level hash value
+#
+GlobalData.gPackageHash[Arch] = {}
+if GlobalData.gUseHashCache:
+for Pkg in Pkgs:
+self._GenPkgLevelHash(Pkg)
+
 #
 # Check PCDs token value conflict in each DEC file.
 #
 self._CheckAllPcdsTokenValueConflict()
 
@@ -655,15 +668,10 @@ class WorkspaceAutoGen(AutoGen):
 self._CheckPcdDefineAndType()
 
 # if self.FdfFile:
 # self._CheckDuplicateInFV(Fdf)
 
-self._BuildDir = None
-self._FvDir = None
-self._MakeFileDir = None
-self._BuildCommand = None
-
 #
 # Create BuildOptions Macro & PCD metafile, also add the Active 
Platform and FDF file.
 #
 content = 'gCommandLineDefines: '
 content += str(GlobalData.gCommandLineDefines)
@@ -675,10 +683,14 @@ class WorkspaceAutoGen(AutoGen):
 content += str(self.Platform)
 content += os.linesep
 if self.FdfFile:
 content += 'Flash Image Definition: '
 content += str(self.FdfFile)
+content += os.linesep
+if GlobalData.gBinCacheDest:
+content += 'Cache of .efi location: '
+content += str(GlobalData.gBinCacheDest)
 SaveFileOnChange(os.path.join(self.BuildDir, 'BuildOptions'), content, 
False)
 
 #
 # Create PcdToken Number file for Dynamic/DynamicEx Pcd.
 #
@@ -704,10 +716,22 @@ class WorkspaceAutoGen(AutoGen):
 for f in AllWorkSpaceMetaFiles:
 if os.stat(f)[8] > SrcTimeStamp:
 SrcTimeStamp = os.stat(f)[8]
 self._SrcTimeStamp = SrcTimeStamp
 
+if GlobalData.gUseHashCache:
+m = hashlib.md5()
+for files in 

[edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

[Jian J Wang]

There're two issues here actually.

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't
accept page related attributes. That means users cannot use it to
change page attributes, and have to turn to CPU arch protocol to do it,
which is not be allowed by PI spec.

>From CpuDxe driver perspective, it doesn't update GCD memory attributes
from current page table setup during its initialization. So the memory
attributes in GCD might not reflect all memory attributes in real world.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Michael Kinney 
Suggested-by: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 

Jian J Wang (2):
  UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
  MdeModulePkg/Core: Fix out-of-sync issue in GCD

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++
 UefiCpuPkg/CpuDxe/CpuDxe.c   |  5 ++
 UefiCpuPkg/CpuDxe/CpuDxe.h   |  9 
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 99 
 4 files changed, 140 insertions(+), 18 deletions(-)

-- 
2.14.1.windows.1

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

[edk2] [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD

[Jian J Wang]

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't
accept page related attributes. That means users cannot use it to
change page attributes, and have to turn to CPU arch protocol to do
it, which is not be allowed by PI spec.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Michael Kinney 
Suggested-by: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 45 -
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index a06f8bb77c..e9d1d5b612 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -40,6 +40,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #define PRESENT_MEMORY_ATTRIBUTES (EFI_RESOURCE_ATTRIBUTE_PRESENT)
 
+#define EXCLUSIVE_MEMORY_ATTRIBUTES   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
+   EFI_MEMORY_WT | EFI_MEMORY_WB | \
+   EFI_MEMORY_WP | EFI_MEMORY_UCE)
+
+#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
+EFI_MEMORY_RO)
+
 #define INVALID_CPU_ARCH_ATTRIBUTES   0x
 
 //
@@ -654,28 +661,30 @@ ConverToCpuArchAttributes (
   UINT64 Attributes
   )
 {
-  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
-return EFI_MEMORY_UC;
-  }
+  UINT64  CpuArchAttributes;
 
-  if ( (Attributes & EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
-return EFI_MEMORY_WC;
+  if ((Attributes & ~(EXCLUSIVE_MEMORY_ATTRIBUTES |
+  NONEXCLUSIVE_MEMORY_ATTRIBUTES)) != 0) {
+return INVALID_CPU_ARCH_ATTRIBUTES;
   }
 
-  if ( (Attributes & EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
-return EFI_MEMORY_WT;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WB) == EFI_MEMORY_WB) {
-return EFI_MEMORY_WB;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WP) == EFI_MEMORY_WP) {
-return EFI_MEMORY_WP;
-  }
-
-  return INVALID_CPU_ARCH_ATTRIBUTES;
+  CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
 
+  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
+CpuArchAttributes |= EFI_MEMORY_UC;
+  } else if ( (Attributes & EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
+CpuArchAttributes |= EFI_MEMORY_WC;
+  } else if ( (Attributes & EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
+CpuArchAttributes |= EFI_MEMORY_WT;
+  } else if ( (Attributes & EFI_MEMORY_WB) == EFI_MEMORY_WB) {
+CpuArchAttributes |= EFI_MEMORY_WB;
+  } else if ( (Attributes & EFI_MEMORY_UCE) == EFI_MEMORY_UCE) {
+CpuArchAttributes |= EFI_MEMORY_UCE;
+  } else if ( (Attributes & EFI_MEMORY_WP) == EFI_MEMORY_WP) {
+CpuArchAttributes |= EFI_MEMORY_WP;
+  }
+
+  return CpuArchAttributes;
 }
 
 
-- 
2.14.1.windows.1

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

[edk2] [PATCH 1/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe

[Jian J Wang]

>From CpuDxe driver perspective, it doesn't update GCD memory attributes
from current page table setup during its initialization. So the memory
attributes in GCD might not reflect all memory attributes in real world.

Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Michael Kinney 
Suggested-by: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 UefiCpuPkg/CpuDxe/CpuDxe.c   |  5 ++
 UefiCpuPkg/CpuDxe/CpuDxe.h   |  9 
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 99 
 3 files changed, 113 insertions(+)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index b386f3b677..4e8fa100e0 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -863,6 +863,11 @@ RefreshGcdMemoryAttributes (
 FreePool (MemorySpaceMap);
   }
 
+  //
+  // Update page attributes
+  //
+  RefreshGcdMemoryAttributesFromPaging();
+
   mIsFlushingGCD = FALSE;
 }
 
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
index 4861abee76..a25b35c6eb 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.h
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
@@ -52,6 +52,10 @@
EFI_MEMORY_UCE   \
)
 
+#define EFI_MEMORY_PAGETYPE_MASK  (EFI_MEMORY_RP  | \
+   EFI_MEMORY_XP  | \
+   EFI_MEMORY_RO\
+   )
 
 /**
   Flush CPU data cache. If the instruction cache is fully coherent
@@ -261,5 +265,10 @@ SetDataSelectors (
   UINT16 Selector
   );
 
+VOID
+RefreshGcdMemoryAttributesFromPaging (
+  VOID
+  );
+
 #endif
 
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index 2c61e7503e..ae93f3f553 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -23,6 +23,8 @@
 #include 
 #include 
 #include 
+
+#include "CpuDxe.h"
 #include "CpuPageTable.h"
 
 ///
@@ -767,6 +769,103 @@ AssignMemoryPageAttributes (
   return Status;
 }
 
+/**
+  Update GCD memory space attributes according to current page table setup.
+**/
+VOID
+RefreshGcdMemoryAttributesFromPaging (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+  UINTN   NumberOfDescriptors;
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR *MemorySpaceMap;
+  PAGE_TABLE_LIB_PAGING_CONTEXT   PagingContext;
+  PAGE_ATTRIBUTE  PageAttribute;
+  UINT64  *PageEntry;
+  UINT64  PageLength;
+  UINT64  MemorySpaceLength;
+  UINT64  Length;
+  UINT64  BaseAddress;
+  UINT64  PageStartAddress;
+  UINT64  Attributes;
+  UINT64  Capabilities;
+  BOOLEAN DoUpdate;
+  UINTN   Index;
+
+  //
+  // Assuming that memory space map returned is sorted already; otherwise sort
+  // them in the order of lowest address to highest address.
+  //
+  Status = gDS->GetMemorySpaceMap (, );
+  ASSERT_EFI_ERROR (Status);
+
+  GetCurrentPagingContext ();
+
+  BaseAddress = 0;
+  PageLength  = 0;
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {
+if (MemorySpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
+  continue;
+}
+
+if (MemorySpaceMap[Index].BaseAddress >= (BaseAddress + PageLength)) {
+  //
+  // Current memory space starts at a new page. Resetting PageLength will
+  // trigger a retrieval of page attributes at new address.
+  //
+  PageLength = 0;
+} else {
+  //
+  // In case current memory space is not adjacent to last one
+  //
+  PageLength -= (MemorySpaceMap[Index].BaseAddress - BaseAddress);
+}
+
+// Sync real page attributes to GCD
+BaseAddress   = MemorySpaceMap[Index].BaseAddress;
+MemorySpaceLength = MemorySpaceMap[Index].Length;
+while (MemorySpaceLength > 0) {
+  if (PageLength == 0) {
+PageEntry = GetPageTableEntry (, BaseAddress, 
);
+if (PageEntry == NULL) {
+  break;
+}
+
+//
+// Note current memory space might start in the middle of a page
+//
+PageStartAddress  = (*PageEntry) & 
(UINT64)PageAttributeToMask(PageAttribute);
+PageLength= PageAttributeToLength (PageAttribute) - 
(BaseAddress - PageStartAddress);
+Attributes= GetAttributesFromPageEntry (PageEntry);
+
+if (Attributes != (MemorySpaceMap[Index].Attributes & 
EFI_MEMORY_PAGETYPE_MASK)) {
+  DoUpdate = TRUE;
+  Attributes |= (MemorySpaceMap[Index].Attributes & 

Re: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

[Wang, Jian J]

I found there's a logic hole in code. A new patch will be sent out.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jian J 
Wang
Sent: Monday, September 18, 2017 11:09 AM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Laszlo Ersek 
; Yao, Jiewen ; Zeng, Star 

Subject: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

There're two issues here actually.

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't accept
page related attributes. That means users cannot use it to change page
attributes, and have to turn to CPU arch protocol to do it, which is not
be allowed by PI spec.

>From CpuDxe driver perspective, it doesn't update GCD memory attributes 
from current page table setup during its initialization. So the memory
attributes in GCD might not reflect all memory attributes in real world.

Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Michael Kinney 
Suggested-by: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 

Jian J Wang (2):
  MdeModulePkg/Core: Fix out-of-sync issue in GCD
  UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page attributes

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 
 UefiCpuPkg/CpuDxe/CpuDxe.c   |  5 +++
 UefiCpuPkg/CpuDxe/CpuDxe.h   |  9 
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 92 
 4 files changed, 133 insertions(+), 18 deletions(-)

-- 
2.14.1.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