date:20170222

[edk2] [Patch 0/2] Ignore duplicated DNS address check

2017-02-22 Thread Jiaxin Wu

Having duplicated DNS server IPs specified is not an ideal
configuration, but not an error condition. This patch is to
remove the duplicated DNS address check to allow the same DNS
address setting in SetData().

Cc: Hegde Nagaraj P 
Cc: Subramanian Sriram 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 

Jiaxin Wu (2):
  MdeModulePkg/Ip4Dxe: Ignore duplicated DNS address check
  NetworkPkg/Ip6Dxe: Ignore duplicated DNS address check

 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 --
 NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c  | 10 --
 2 files changed, 20 deletions(-)

-- 
1.9.5.msysgit.1

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

[edk2] [Patch 2/2] NetworkPkg/Ip6Dxe: Ignore duplicated DNS address check

2017-02-22 Thread Jiaxin Wu

Having duplicated DNS server IPs specified is not an ideal
configuration, but not an error condition. This patch is to
remove the duplicated DNS address check to allow the same DNS
address setting in SetData().

Cc: Hegde Nagaraj P 
Cc: Subramanian Sriram 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c 
b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
index e309b69..67037bb 100644
--- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
+++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
@@ -1325,11 +1325,10 @@ Ip6ConfigSetDnsServer (
   IN VOID *Data
   )
 {
   UINTN OldIndex;
   UINTN NewIndex;
-  UINTN Index1;
   EFI_IPv6_ADDRESS  *OldDns;
   EFI_IPv6_ADDRESS  *NewDns;
   UINTN OldDnsCount;
   UINTN NewDnsCount;
   IP6_CONFIG_DATA_ITEM  *Item;
@@ -1370,19 +1369,10 @@ Ip6ConfigSetDnsServer (
 FreePool (Tmp);
   }
   return EFI_INVALID_PARAMETER;
 }
 
-for (Index1 = NewIndex + 1; Index1 < NewDnsCount; Index1++) {
-  if (EFI_IP6_EQUAL (NewDns + NewIndex, NewDns + Index1)) {
-if (Tmp != NULL) {
-  FreePool (Tmp);
-}
-return EFI_INVALID_PARAMETER;
-  }
-}
-
 if (OneAdded) {
   //
   // If any address in the new setting is not in the old settings, skip the
   // comparision below.
   //
-- 
1.9.5.msysgit.1

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

[edk2] [Patch 1/2] MdeModulePkg/Ip4Dxe: Ignore duplicated DNS address check

2017-02-22 Thread Jiaxin Wu

Having duplicated DNS server IPs specified is not an ideal
configuration, but not an error condition. This patch is to
remove the duplicated DNS address check to allow the same DNS
address setting in SetData().

Cc: Hegde Nagaraj P 
Cc: Subramanian Sriram 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index a6a3da8..0b56d76 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -703,11 +703,10 @@ Ip4Config2SetDnsServerWorker (
   IN VOID*Data
   )
 {
   UINTN OldIndex;
   UINTN NewIndex;
-  UINTN Index1;
   EFI_IPv4_ADDRESS  *OldDns;
   EFI_IPv4_ADDRESS  *NewDns;
   UINTN OldDnsCount;
   UINTN NewDnsCount;
   IP4_CONFIG2_DATA_ITEM *Item;
@@ -745,19 +744,10 @@ Ip4Config2SetDnsServerWorker (
 FreePool (Tmp);
   }
   return EFI_INVALID_PARAMETER;
 }
 
-for (Index1 = NewIndex + 1; Index1 < NewDnsCount; Index1++) {
-  if (EFI_IP4_EQUAL (NewDns + NewIndex, NewDns + Index1)) {
-if (Tmp != NULL) {
-  FreePool (Tmp);
-}
-return EFI_INVALID_PARAMETER;
-  }
-}
-
 if (OneAdded) {
   //
   // If any address in the new setting is not in the old settings, skip the
   // comparision below.
   //
-- 
1.9.5.msysgit.1

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

Re: [edk2] Testing SMM with QEMU, KVM and libvirt

2017-02-22 Thread Shi, Steven

Hi Laszlo,
I want to see the serial debug output and hope to save it to a local file. How 
could I update the ovmf.fedora.q35.template to define it?


Steven Shi
Intel\SSG\STO\UEFI Firmware

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

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, February 21, 2017 6:18 PM
> To: Shi, Steven ; edk2-devel-01  de...@ml01.01.org>
> Cc: Tian, Feng ; Justen, Jordan L
> ; Yao, Jiewen ; Kinney,
> Michael D ; Fan, Jeff ; Zeng,
> Star 
> Subject: Re: [edk2] Testing SMM with QEMU, KVM and libvirt
> 
> On 02/21/17 10:22, Shi, Steven wrote:
> > Hi Laszlo,
> > I wonder if you could offer a Ubuntu version wiki for the
> > Testing-SMM-with-QEMU,-KVM-and-libvirt?
> 
> Sorry, I can't do that.
> 
> To give you a retrospective on the current article (which targets
> Fedora), writing and testing that article took ~14 hours, on an
> operating system that I'm both familiar with and can readily install in
> our internal server farm (called "Beaker").
> 
> My method was to start with a wiped-clean physical machine, install a
> fresh, clean Fedora system, and build it all up from there. I wanted to
> make sure that all missing dependencies would be thrown in my face, so I
> could explicitly document them for readers.
> 
> This approach paid off very well (it caught a whole lot of dependencies
> that I "thought" would be available by default, but weren't!), but it
> was also the *primary* time sink while writing the article.
> 
> "Porting" the article to another Linux distribution would mean an almost
> complete rewrite. The package names are different, the package contents
> are different, the package inter-dependencies are different, the
> virtualization tools may have different versions available, the generic
> tools may be different, and so on.
> 
> > I'm trying to port your
> > steps to my Ubuntu 16.04, but meet lots of troubles. The Ubuntu
> > apt-get virsh version is too old to support smm feature in your
> > ovmf.fedora.q35.template,
> 
> That's *exactly* my point.
> 
> > and I have to build the new version libvirt
> > by myself. I meet lots of failures when configure the new version
> > virsh, and wonder if you could help.
> 
> Sorry, I don't have time for that. I don't know Ubuntu at all, have no
> contact to Ubuntu developers, and cannot even auto-install Ubuntu easily
> on a headless server in our internal server pool.
> 
> Frankly, one goal of using Fedora 25 for the host operating system was
> *exactly* that the user could avoid this kind of struggle with the
> virtualization toolstack, and they could focus on rebuilding *only* what
> was unavoidable.
> 
> (It is bad enough that at the moment I must have instructions in there
> for building QEMU from source -- once QEMU 2.9 is released and Fedora 25
> picks it up, I think I will go ahead and replace that section of the
> article, with a simple package installation command. I'll also update
> references elsewhere, such as in the domain templates.)
> 
> So, unfortunately, what you are asking for is a complete rewrite of the
> article, for Ubuntu, which I don't know and have no access to, in the
> isolated server environment that is necessary for writing and testing
> such an article.
> 
> I'm not trying to "push" Fedora with this -- a fresh Ubuntu release
> should be entirely suitable for this I *guess*, but the devil is in the
> details, and you'll need an Ubuntu person, with a corporate(-like)
> Ubuntu environment, to write that article for you.
> 
> I do confirm that I intend to support the Fedora setup with high
> priority, so if you have questions about that, I'll do my best to answer.
> 
> Thanks,
> Laszlo
> 
> 
> >
> >
> >
> > Steven Shi
> > Intel\SSG\STO\UEFI Firmware
> >
> > Tel: +86 021-61166522
> > iNet: 821-6522
> >
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >> Laszlo Ersek
> >> Sent: Tuesday, February 7, 2017 9:07 PM
> >> To: edk2-devel-01 
> >> Cc: Tian, Feng ; Justen, Jordan L
> >> ; Yao, Jiewen ; Kinney,
> >> Michael D ; Fan, Jeff ;
> Zeng,
> >> Star 
> >> Subject: [edk2] Testing SMM with QEMU, KVM and libvirt
> >>
> >> Hi,
> >>
> >> I've added the following article to the TianoCore wiki:
> >>
> >> https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-
> >> QEMU,-KVM-and-libvirt
> >>
> >> It should help both Windows and Linux desktop users build a KVM test
> >> machine / environment that closely resembles mine. Such an environment
> >> is useful for testing and regression-testing new MP and SMM features and
> >> bugfixes.
> >>
> >> The initial setup is not short, but once you

Re: [edk2] [patch] MdeModulePkg: Update the Ethernet interface name.

2017-02-22 Thread Subramanian, Sriram

Reviewed-by: Sriram Subramanian 

-Original Message-
From: Zhang Lubo [mailto:lubo.zh...@intel.com] 
Sent: Thursday, February 23, 2017 8:09 AM
To: edk2-devel@lists.01.org
Cc: Subramanian, Sriram ; Ye Ting ; Fu 
Siyuan ; Wu Jiaxin 
Subject: [patch] MdeModulePkg: Update the Ethernet interface name.

Update the interface name from ethA ethB to
eth10, eth11 etc if port number more than 9.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Sriram Subramanian 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index a6a3da8..504f247 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -1493,11 +1493,11 @@ Ip4Config2InitIfInfo (
   )
 {
   UnicodeSPrint (
 IfInfo->Name,
 EFI_IP4_CONFIG2_INTERFACE_INFO_NAME_SIZE,
-L"eth%x",
+L"eth%d",
 IpSb->Ip4Config2Instance.IfIndex
   );
 
   IfInfo->IfType= IpSb->SnpMode.IfType;
   IfInfo->HwAddressSize = IpSb->SnpMode.HwAddressSize;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [patch] NetworkPkg: Update the Ethernet interface name.

2017-02-22 Thread Subramanian, Sriram

Reviewed-by: Sriram Subramanian 

-Original Message-
From: Zhang Lubo [mailto:lubo.zh...@intel.com] 
Sent: Thursday, February 23, 2017 8:09 AM
To: edk2-devel@lists.01.org
Cc: Subramanian, Sriram ; Ye Ting ; Fu 
Siyuan ; Wu Jiaxin 
Subject: [patch] NetworkPkg: Update the Ethernet interface name.

Update the interface name from ethA ethB to
eth10, eth11 etc if port number more than 9.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Sriram Subramanian 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c 
b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
index e309b69..9287475 100644
--- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
+++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
@@ -1439,11 +1439,11 @@ Ip6ConfigInitIfInfo (
   )
 {
   UnicodeSPrint (
 IfInfo->Name,
 sizeof (IfInfo->Name),
-L"eth%x",
+L"eth%d",
 IpSb->Ip6ConfigInstance.IfIndex
   );
 
   IfInfo->IfType= IpSb->SnpMode.IfType;
   IfInfo->HwAddressSize = IpSb->SnpMode.HwAddressSize;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch] BaseTools: Fix the regression issue caused by commit dc4c77

2017-02-22 Thread Gao, Liming

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Yonghong Zhu
>Sent: Thursday, February 23, 2017 12:03 AM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: [edk2] [Patch] BaseTools: Fix the regression issue caused by commit
>dc4c77
>
>In the last commit dc4c77, the _GetHeaderInfo will be called more than
>once, which cause the self._ConstructorList.append(Value) append some
>duplicate value.
>
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Yonghong Zhu 
>---
> BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>index 0686721..c1af5c7 100644
>--- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>+++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>@@ -1828,12 +1828,10 @@ class InfBuildData(ModuleBuildClassObject):
> self.__Macros = {}
> # EDK_GLOBAL defined macros can be applied to EDK module
> if self.AutoGenVersion < 0x00010005:
> self.__Macros.update(GlobalData.gEdkGlobal)
> self.__Macros.update(GlobalData.gGlobalDefines)
>-else:
>-self.__Macros.update(self.Defines)
> return self.__Macros
>
> ## Get architecture
> def _GetArch(self):
> return self._Arch
>@@ -1894,10 +1892,11 @@ class InfBuildData(ModuleBuildClassObject):
> if Name in self:
> self[Name] = Value
> if self._Defs == None:
> self._Defs = sdict()
> self._Defs[Name] = Value
>+self._Macros[Name] = Value
> # some special items in [Defines] section need special treatment
> elif Name in ('EFI_SPECIFICATION_VERSION',
>'UEFI_SPECIFICATION_VERSION', 'EDK_RELEASE_VERSION',
>'PI_SPECIFICATION_VERSION'):
> if Name in ('EFI_SPECIFICATION_VERSION',
>'UEFI_SPECIFICATION_VERSION'):
> Name = 'UEFI_SPECIFICATION_VERSION'
> if self._Specification == None:
>@@ -1954,10 +1953,11 @@ class InfBuildData(ModuleBuildClassObject):
> self._CustomMakefile[TokenList[0]] = TokenList[1]
> else:
> if self._Defs == None:
> self._Defs = sdict()
> self._Defs[Name] = Value
>+self._Macros[Name] = Value
>
> #
> # Retrieve information in sections specific to Edk.x modules
> #
> if self.AutoGenVersion >= 0x00010005:
>--
>2.6.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] [patch] MdeModulePkg: Update the Ethernet interface name.

2017-02-22 Thread Wu, Jiaxin

Reviewed-by: Wu Jiaxin 


Thanks,
Jiaxin

> -Original Message-
> From: Zhang, Lubo
> Sent: Thursday, February 23, 2017 10:39 AM
> To: edk2-devel@lists.01.org
> Cc: Sriram Subramanian ; Ye, Ting ;
> Fu, Siyuan ; Wu, Jiaxin 
> Subject: [patch] MdeModulePkg: Update the Ethernet interface name.
> 
> Update the interface name from ethA ethB to
> eth10, eth11 etc if port number more than 9.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Sriram Subramanian 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index a6a3da8..504f247 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -1493,11 +1493,11 @@ Ip4Config2InitIfInfo (
>)
>  {
>UnicodeSPrint (
>  IfInfo->Name,
>  EFI_IP4_CONFIG2_INTERFACE_INFO_NAME_SIZE,
> -L"eth%x",
> +L"eth%d",
>  IpSb->Ip4Config2Instance.IfIndex
>);
> 
>IfInfo->IfType= IpSb->SnpMode.IfType;
>IfInfo->HwAddressSize = IpSb->SnpMode.HwAddressSize;
> --
> 1.9.5.msysgit.1

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

Re: [edk2] [patch] NetworkPkg: Update the Ethernet interface name.

2017-02-22 Thread Wu, Jiaxin

Reviewed-by: Wu Jiaxin 


Thanks,
Jiaxin

> -Original Message-
> From: Zhang, Lubo
> Sent: Thursday, February 23, 2017 10:39 AM
> To: edk2-devel@lists.01.org
> Cc: Sriram Subramanian ; Ye, Ting ;
> Fu, Siyuan ; Wu, Jiaxin 
> Subject: [patch] NetworkPkg: Update the Ethernet interface name.
> 
> Update the interface name from ethA ethB to
> eth10, eth11 etc if port number more than 9.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Sriram Subramanian 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> ---
>  NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
> b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
> index e309b69..9287475 100644
> --- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
> +++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
> @@ -1439,11 +1439,11 @@ Ip6ConfigInitIfInfo (
>)
>  {
>UnicodeSPrint (
>  IfInfo->Name,
>  sizeof (IfInfo->Name),
> -L"eth%x",
> +L"eth%d",
>  IpSb->Ip6ConfigInstance.IfIndex
>);
> 
>IfInfo->IfType= IpSb->SnpMode.IfType;
>IfInfo->HwAddressSize = IpSb->SnpMode.HwAddressSize;
> --
> 1.9.5.msysgit.1

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

Re: [edk2] [patch] NetworkPkg: Update the Ethernet interface name.

2017-02-22 Thread Fu, Siyuan

Reviewed-by: Fu Siyuan 



-Original Message-
From: Zhang, Lubo 
Sent: 2017年2月23日 10:39
To: edk2-devel@lists.01.org
Cc: Sriram Subramanian ; Ye, Ting ; Fu, 
Siyuan ; Wu, Jiaxin 
Subject: [patch] NetworkPkg: Update the Ethernet interface name.

Update the interface name from ethA ethB to eth10, eth11 etc if port number 
more than 9.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Sriram Subramanian 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c 
b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
index e309b69..9287475 100644
--- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
+++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
@@ -1439,11 +1439,11 @@ Ip6ConfigInitIfInfo (
   )
 {
   UnicodeSPrint (
 IfInfo->Name,
 sizeof (IfInfo->Name),
-L"eth%x",
+L"eth%d",
 IpSb->Ip6ConfigInstance.IfIndex
   );
 
   IfInfo->IfType= IpSb->SnpMode.IfType;
   IfInfo->HwAddressSize = IpSb->SnpMode.HwAddressSize;
--
1.9.5.msysgit.1

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

Re: [edk2] Improvements to build system etc. for edk2-platforms devel-MinnowBoard3?

2017-02-22 Thread Wei, David

Yes, as Brian said, at this stage,  patches are welcomed, including the python 
script.  You can also file bugs on https://bugzilla.tianocore.org/ for issue 
track and discussion.  Please remember to CC stakeholders. 

Specifically for the multi-thread building , I think maybe  
NUMBER_OF_PROCESSORS Windows environment variable could be used as the proper 
thread number. 

Thanks,
David  Wei 

-Original Message-
From: Richardson, Brian 
Sent: Thursday, February 23, 2017 3:10 AM
To: Rebecca Cran ; Gao, Liming ; 
edk2-devel@lists.01.org
Cc: Lu, ShifeiX A ; Zimmer, Vincent 
; Andrew Fish ; Wei, David 

Subject: RE: [edk2] Improvements to build system etc. for edk2-platforms 
devel-MinnowBoard3?

We're ok with help fixing issues (yay open source), so thanks for the help. 
Patches are welcome at this time. But we do track them in Bugzilla, so opening 
an issue there is the first step to a solution. 

For the processor thread setting, note that we have historically disabled 
processor threading by default because we don't know the build system 
configuration. While we at Intel prefer everyone own an Intel(R) Core(TM) i7 or 
Intel(R) Xeon processor, we know that's not the case ... so keeping it disabled 
has been seen as a safe option. We should definitely do a better job of 
documenting that setting change, but I think we need to consider generically 
changing that setting by default in target.txt (which requires a Bugzilla 
entry).

The .bat/.sh files are required to trigger post-build tools, which are OS 
dependent. Even if the build was triggered by a Python script, we still need to 
do some verification to make sure there are no functional differences when we 
build in a Windows environment versus Linux (work in progress).

Thanks ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_Brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rebecca 
Cran
Sent: Wednesday, February 22, 2017 11:53 AM
To: Richardson, Brian ; Gao, Liming 
; edk2-devel@lists.01.org
Cc: Lu, ShifeiX A ; Zimmer, Vincent 
; Andrew Fish ; Wei, David 

Subject: Re: [edk2] Improvements to build system etc. for edk2-platforms 
devel-MinnowBoard3?

On 2/22/2017 9:34 AM, Richardson, Brian wrote:
> Thanks for the input. For future reference, you can use the TianoCore 
> Bugzilla to report issues on any EDK II feature/platform. 
> https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues
>
> I agree the readme.md should be present, and use markup instead of plain text 
> to work better in github. You can open an issue on this in Bugzilla.
>
> Normally, we ask folks to change the number of processor threads based on 
> their system configuration. We don't add a larger thread number by default, 
> but it might be good to set it '5' by default (assuming a dual core processor 
> with hyperthreading) instead of '1' (assuming a single core system w/o 
> threading). I don't know if this will cause any compatibility issues on older 
> systems, but it's worth a check.
>
> At this time, MinnowBoard 3 build is only validated in Windows. That's why 
> there is no equivalent .sh file for BuildBIOS yet, but it will be added once 
> Linux build is verified and checked in.

I'm more about _fixing_ issues I find rather than reporting them! Are you 
saying that patches wouldn't be welcome just now?  Is there a reason why you 
don't want to make full use of the CPU while building? And I understand that 
MinnowBoard 3 only builds under Windows at the moment, but if more of it built 
using python (and python is already listed as a prerequisite in the ReadMe 
file) the porting might be simpler.

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

[edk2] [patch] MdeModulePkg: Update the Ethernet interface name.

2017-02-22 Thread Zhang Lubo

Update the interface name from ethA ethB to
eth10, eth11 etc if port number more than 9.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Sriram Subramanian 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index a6a3da8..504f247 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -1493,11 +1493,11 @@ Ip4Config2InitIfInfo (
   )
 {
   UnicodeSPrint (
 IfInfo->Name,
 EFI_IP4_CONFIG2_INTERFACE_INFO_NAME_SIZE,
-L"eth%x",
+L"eth%d",
 IpSb->Ip4Config2Instance.IfIndex
   );
 
   IfInfo->IfType= IpSb->SnpMode.IfType;
   IfInfo->HwAddressSize = IpSb->SnpMode.HwAddressSize;
-- 
1.9.5.msysgit.1

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

[edk2] [patch] NetworkPkg: Update the Ethernet interface name.

2017-02-22 Thread Zhang Lubo

Update the interface name from ethA ethB to
eth10, eth11 etc if port number more than 9.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Sriram Subramanian 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c 
b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
index e309b69..9287475 100644
--- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
+++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
@@ -1439,11 +1439,11 @@ Ip6ConfigInitIfInfo (
   )
 {
   UnicodeSPrint (
 IfInfo->Name,
 sizeof (IfInfo->Name),
-L"eth%x",
+L"eth%d",
 IpSb->Ip6ConfigInstance.IfIndex
   );
 
   IfInfo->IfType= IpSb->SnpMode.IfType;
   IfInfo->HwAddressSize = IpSb->SnpMode.HwAddressSize;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH 0/7] Mark [Ascii|Unicode]ValueToString as deprecated

2017-02-22 Thread Gao, Liming

The change in PrintLib and PrintDxe (Patch 5~7) are good to me. 

Reviewed-by: Liming Gao 

Thanks
Liming
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao Wu
Sent: Tuesday, February 21, 2017 7:35 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Yao, Jiewen 
Subject: [edk2] [PATCH 0/7] Mark [Ascii|Unicode]ValueToString as deprecated

This patch series cleans up the usage of the following 2 PrintLib APIs:
UnicodeValueToString & AsciiValueToString

and replaces them with:
UnicodeValueToStringS & AsciiValueToStringS

The series also marks [Ascii|Unicode]ValueToString with the macro
'DISABLE_NEW_DEPRECATED_INTERFACES', indicating they are deprecated.

Cc: Jiewen Yao 

Hao Wu (7):
  IntelFrameworkModulePkg: Replace [Ascii|Unicode]ValueToString
  MdeModulePkg: Replace [Ascii|Unicode]ValueToString
  Nt32Pkg: Replace [Ascii|Unicode]ValueToString
  SignedCapsulePkg: Replace [Ascii|Unicode]ValueToString
  MdeModulePkg/PrintDxe: Handle the deprecation of [A|U]ValueToString
  MdeModulePkg/PrintLib: Add deprecated flag for APIs [A|U]ValueToString
  MdePkg/BasePrintLib: Add deprecated flag for APIs [A|U]ValueToString

 IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c   | 12 ++-
 IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c   | 20 
-
 IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c  | 15 +++-
 IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c |  6 +-
 MdeModulePkg/Application/UiApp/FrontPage.c| 15 +++-
 MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c | 12 ++-
 MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c   | 20 
-
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c   | 10 ++-
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c| 24 
-
 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 12 +++
 MdeModulePkg/Library/UefiHiiLib/HiiLib.c  | 29 
+-
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c   | 18 +++-
 MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c | 10 ++-
 MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c | 37 
+---
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c  | 29 
+-
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c | 47 
--
 MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigImpl.c | 14 ++-
 MdeModulePkg/Universal/PrintDxe/Print.c   | 92 
+++-
 MdeModulePkg/Universal/SetupBrowserDxe/Expression.c   | 11 ++-
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c |  4 +-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c| 38 
++--
 MdePkg/Include/Library/PrintLib.h | 12 +++
 MdePkg/Library/BasePrintLib/PrintLib.c| 12 +++
 Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c   |  6 +-
 SignedCapsulePkg/Universal/RecoveryModuleLoadPei/ParseConfigProfile.c | 10 ++-
 SignedCapsulePkg/Universal/SystemFirmwareUpdate/ParseConfigProfile.c  | 10 ++-
 26 files changed, 443 insertions(+), 82 deletions(-)

-- 
1.9.5.msysgit.0

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

[edk2] [PATCH 10/12] OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for DXE fw_cfg instance

2017-02-22 Thread Laszlo Ersek

In the DXE fw_cfg instance:

- QemuFwCfgS3Enabled() queries S3 enablement via fw_cfg. This behavior is
  shared with the PEI fw_cfg instance, and the DXE fw_cfg instance already
  pulls in the function from "QemuFwCfgS3PeiDxe.c".

- If QemuFwCfgS3Enabled() returns TRUE, the client module is permitted to
  call QemuFwCfgS3TransferOwnership().

  We provide a fully functional implementation for
  QemuFwCfgS3TransferOwnership(). A protocol notify is installed at
  TPL_CALLBACK for EFI_S3_SAVE_STATE_PROTOCOL. If / once the protocol is
  available, the client module's Append() function is called, which is
  expected to produce ACPI S3 Boot Script opcodes using the helper
  functions listed below. In QemuFwCfgS3TransferOwnership(), we also
  allocate a reserved memory buffer, sized & typed by the client module,
  for the opcodes and (internally) the fw_cfg DMA operations to work upon,
  during S3 resume.

  This behavior is unique to the DXE fw_cfg instance. Thus, add the
  function to "QemuFwCfgS3Dxe.c".

- The QemuFwCfgS3WriteBytes(), QemuFwCfgS3ReadBytes(),
  QemuFwCfgS3SkipBytes(), and QemuFwCfgS3CheckValue() functions are also
  implemented usefully, since the client module's Append() function is
  expected to invoke them.

  Each of the first three functions produces MEM_WRITE, IO_WRITE, and
  MEM_POLL opcodes, to set up the DMA command in reserved memory, to start
  the DMA transfer, and to check the DMA result, respectively.

  The QemuFwCfgS3CheckValue() function produces a MEM_POLL opcode to
  validate an unsigned integer field in data that was read via
  QemuFwCfgS3ReadBytes().

  This behavior is again unique to the DXE fw_cfg instance, so add the
  functions to "QemuFwCfgS3Dxe.c".

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf |   8 +
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Dxe.c   | 791 

 2 files changed, 799 insertions(+)

diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf 
b/OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
index 7016575f3dab..a0e4275cb8a5 100644
--- a/OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
@@ -28,6 +28,7 @@ [Defines]
 #
 
 [Sources]
+  QemuFwCfgS3Dxe.c
   QemuFwCfgS3PeiDxe.c
 
 [Packages]
@@ -35,4 +36,11 @@ [Packages]
   OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
+  BaseLib
+  DebugLib
+  MemoryAllocationLib
   QemuFwCfgLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gEfiS3SaveStateProtocolGuid ## SOMETIMES_CONSUMES
diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Dxe.c 
b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Dxe.c
new file mode 100644
index ..51d99d66a98b
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Dxe.c
@@ -0,0 +1,791 @@
+/** @file
+  Full functionality QemuFwCfgS3Lib instance, for DXE phase modules.
+
+  Copyright (C) 2017, Red Hat, Inc.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution. The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+//
+// Event to signal when the S3SaveState protocol interface is installed.
+//
+STATIC EFI_EVENT mS3SaveStateInstalledEvent;
+
+//
+// Reference to the S3SaveState protocol interface, after it is installed.
+//
+STATIC EFI_S3_SAVE_STATE_PROTOCOL *mS3SaveState;
+
+//
+// The control structure is allocated in reserved memory, aligned at 8 bytes.
+// The client-requested ScratchBuffer will be allocated adjacently, also
+// aligned at 8 bytes.
+//
+#define RESERVED_MEM_ALIGNMENT 8
+
+STATIC FW_CFG_DMA_ACCESS *mDmaAccess;
+STATIC VOID  *mScratchBuffer;
+STATIC UINTN mScratchBufferSize;
+
+//
+// Callback provided by the client, for appending ACPI S3 Boot Script opcodes.
+// To be called from S3SaveStateInstalledNotify().
+//
+STATIC FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION *mAppend;
+
+
+/**
+  Event notification function for mS3SaveStateInstalledEvent.
+**/
+STATIC
+VOID
+EFIAPI
+S3SaveStateInstalledNotify (
+  IN EFI_EVENT Event,
+  IN VOID  *Context
+  )
+{
+  EFI_STATUS Status;
+
+  ASSERT (Event == mS3SaveStateInstalledEvent);
+
+  Status = gBS->LocateProtocol (,
+  NULL /* Registration */, (VOID **));
+  if (EFI_ERROR (Status)) {
+return;
+  }
+
+  ASSERT (mAppend != NULL);
+
+  DEBUG ((DEBUG_INFO, "%a: %a: DmaAccess@0x%Lx ScratchBuffer@[0x%Lx+0x%Lx]\n",
+gEfiCallerBaseName, __FUNCTION__,

[edk2] [PATCH 11/12] OvmfPkg/SmmControl2Dxe: save fw_cfg boot script with QemuFwCfgS3Lib

2017-02-22 Thread Laszlo Ersek

We cannot entirely eliminate the manual boot script building in this
driver, as it also programs lower-level chipset registers (SMI_EN,
GEN_PMCON_1) at S3 resume, not just registers exposed via fw_cfg.

We can nonetheless replace the manually built opcodes for the latter class
of registers with QemuFwCfgS3Lib function calls. We preserve the ordering
between the two sets of registers (low-level chipset first, fw_cfg
second).

This patch demonstrates that manual handling of S3SaveState protocol
installation can be combined with QemuFwCfgS3Lib, even without upsetting
the original order between boot script fragments. An S3SaveState notify
function running at TPL_CALLBACK can safely queue another S3SaveState
notify function at TPL_CALLBACK with QemuFwCfgS3TransferOwnership().

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/SmmControl2Dxe/SmiFeatures.h|   5 +-
 OvmfPkg/SmmControl2Dxe/SmiFeatures.c| 224 +++-
 OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c |   5 +-
 3 files changed, 77 insertions(+), 157 deletions(-)

diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.h 
b/OvmfPkg/SmmControl2Dxe/SmiFeatures.h
index 9d5f1dbcb57e..3f3a5d3ea9b1 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.h
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.h
@@ -37,13 +37,10 @@ NegotiateSmiFeatures (
 /**
   Append a boot script fragment that will re-select the previously negotiated
   SMI features during S3 resume.
-
-  @param[in] S3SaveState  The EFI_S3_SAVE_STATE_PROTOCOL instance to append to
-  the S3 boot script with.
 **/
 VOID
 SaveSmiFeatures (
-  IN EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState
+  VOID
   );
 
 #endif
diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c 
b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index bd257f15d955..ecdab2fd9a86 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "SmiFeatures.h"
 
@@ -29,29 +30,26 @@
 
 //
 // Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
-// for the S3 boot script fragment to write to and read from. The buffer
-// captures a combined fw_cfg item selection + write command using the DMA
-// access method. Note that we don't trust the runtime OS to preserve the
-// contents of the buffer, the boot script will first rewrite it.
+// for the S3 boot script fragment to write to and read from.
 //
 #pragma pack (1)
-typedef struct {
-  FW_CFG_DMA_ACCESS Access;
-  UINT64Features;
+typedef union {
+  UINT64 Features;
+  UINT8  FeaturesOk;
 } SCRATCH_BUFFER;
 #pragma pack ()
 
 //
 // These carry the selector keys of the "etc/smi/requested-features" and
 // "etc/smi/features-ok" fw_cfg files from NegotiateSmiFeatures() to
-// SaveSmiFeatures().
+// AppendFwCfgBootScript().
 //
 STATIC FIRMWARE_CONFIG_ITEM mRequestedFeaturesItem;
 STATIC FIRMWARE_CONFIG_ITEM mFeaturesOkItem;
 
 //
 // Carries the negotiated SMI features from NegotiateSmiFeatures() to
-// SaveSmiFeatures().
+// AppendFwCfgBootScript().
 //
 STATIC UINT64 mSmiFeatures;
 
@@ -168,157 +166,81 @@ FatalError:
 }
 
 /**
+  FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION provided to QemuFwCfgS3Lib.
+**/
+STATIC
+VOID
+EFIAPI
+AppendFwCfgBootScript (
+  IN OUT VOID *Context,  OPTIONAL
+  IN OUT VOID *ExternalScratchBuffer
+  )
+{
+  SCRATCH_BUFFER *ScratchBuffer;
+  RETURN_STATUS  Status;
+
+  ScratchBuffer = ExternalScratchBuffer;
+
+  //
+  // Write the negotiated feature bitmap into "etc/smi/requested-features".
+  //
+  ScratchBuffer->Features = mSmiFeatures;
+  Status = QemuFwCfgS3WriteBytes (mRequestedFeaturesItem,
+ sizeof ScratchBuffer->Features);
+  if (RETURN_ERROR (Status)) {
+goto FatalError;
+  }
+
+  //
+  // Read back "etc/smi/features-ok". This invokes the feature validation &
+  // lockdown. (The validation succeeded at first boot.)
+  //
+  Status = QemuFwCfgS3ReadBytes (mFeaturesOkItem,
+ sizeof ScratchBuffer->FeaturesOk);
+  if (RETURN_ERROR (Status)) {
+goto FatalError;
+  }
+
+  //
+  // If "etc/smi/features-ok" read as 1, we're good. Otherwise, hang the S3
+  // resume process.
+  //
+  Status = QemuFwCfgS3CheckValue (>FeaturesOk,
+ sizeof ScratchBuffer->FeaturesOk, MAX_UINT8, 1);
+  if (RETURN_ERROR (Status)) {
+goto FatalError;
+  }
+
+  DEBUG ((DEBUG_VERBOSE, "%a: SMI feature negotiation boot script saved\n",
+__FUNCTION__));
+  return;
+
+FatalError:
+  ASSERT (FALSE);
+  CpuDeadLoop ();
+}
+
+
+/**
   Append a boot script fragment that will re-select the previously negotiated
   SMI features during S3 resume.
-
-  @param[in] S3SaveState  The EFI_S3_SAVE_STATE_PROTOCOL instance to append to
-  the S3 boot script with.
 **/
 VOID
 SaveSmiFeatures (
-  IN EFI_S3_SAVE_STATE_PROTOCOL

[edk2] [PATCH 12/12] OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib

2017-02-22 Thread Laszlo Ersek

Drop the explicit S3SaveState protocol and opcode management; instead,
create ACPI S3 Boot Script opcodes for the WRITE_POINTER commands with
QemuFwCfgS3Lib functions.

In this case, we have a dynamically allocated Context structure, hence the
patch demonstrates how the FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION takes
ownership of Context.

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf  |   1 -
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |   1 -
 OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h   |   2 +-
 OvmfPkg/AcpiPlatformDxe/BootScript.c | 262 +---
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c  |   7 +
 5 files changed, 64 insertions(+), 209 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf 
b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 42edc97b3da2..9a9b2e6bb2e5 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -61,7 +61,6 @@ [LibraryClasses]
 [Protocols]
   gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
   gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CONSUMED
-  gEfiS3SaveStateProtocolGuid   # PROTOCOL SOMETIMES_CONSUMED
 
 [Guids]
   gEfiXenInfoGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf 
b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
index a9350540215d..adc50cfd9f76 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
@@ -51,7 +51,6 @@ [LibraryClasses]
 [Protocols]
   gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
   gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CONSUMED
-  gEfiS3SaveStateProtocolGuid   # PROTOCOL SOMETIMES_CONSUMED
 
 [Guids]
   gRootBridgesConnectedEventGroupGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h 
b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
index 0f035a0d5751..83b981ee005d 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
@@ -115,7 +115,7 @@ SaveCondensedWritePointerToS3Context (
 
 EFI_STATUS
 TransferS3ContextToBootScript (
-  IN CONST S3_CONTEXT *S3Context
+  IN S3_CONTEXT *S3Context
   );
 
 #endif
diff --git a/OvmfPkg/AcpiPlatformDxe/BootScript.c 
b/OvmfPkg/AcpiPlatformDxe/BootScript.c
index bff42ad8b9b0..424b4b3ee2bd 100644
--- a/OvmfPkg/AcpiPlatformDxe/BootScript.c
+++ b/OvmfPkg/AcpiPlatformDxe/BootScript.c
@@ -15,7 +15,7 @@
 
 #include 
 #include 
-#include 
+#include 
 
 #include "AcpiPlatform.h"
 
@@ -53,19 +53,11 @@ struct S3_CONTEXT {
 
 //
 // Scratch buffer, allocated in EfiReservedMemoryType type memory, for the ACPI
-// S3 Boot Script opcodes to work on. We use the buffer to compose and to
-// replay several fw_cfg select+skip and write operations, using the DMA access
-// method. The fw_cfg operations will implement the actions dictated by
-// CONDENSED_WRITE_POINTER objects.
+// S3 Boot Script opcodes to work on.
 //
 #pragma pack (1)
-typedef struct {
-  FW_CFG_DMA_ACCESS Access;   // filled in from
-  //   CONDENSED_WRITE_POINTER.PointerItem,
-  //   CONDENSED_WRITE_POINTER.PointerSize,
-  //   CONDENSED_WRITE_POINTER.PointerOffset
-  UINT64PointerValue; // filled in from
-  //   CONDENSED_WRITE_POINTER.PointerValue
+typedef union {
+  UINT64 PointerValue; // filled in from CONDENSED_WRITE_POINTER.PointerValue
 } SCRATCH_BUFFER;
 #pragma pack ()
 
@@ -197,220 +189,78 @@ SaveCondensedWritePointerToS3Context (
 
 
 /**
-  Translate and append the information from an S3_CONTEXT object to the ACPI S3
-  Boot Script.
-
-  The effects of a successful call to this function cannot be undone.
-
-  @param[in] S3Context  The S3_CONTEXT object to translate to ACPI S3 Boot
-Script opcodes.
-
-  @retval EFI_OUT_OF_RESOURCES  Out of memory.
-
-  @retval EFI_SUCCESS   The translation of S3Context to ACPI S3 Boot
-Script opcodes has been successful.
-
-  @return   Error codes from underlying functions.
+  FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION provided to QemuFwCfgS3Lib.
 **/
-EFI_STATUS
-TransferS3ContextToBootScript (
-  IN CONST S3_CONTEXT *S3Context
+STATIC
+VOID
+EFIAPI
+AppendFwCfgBootScript (
+  IN OUT VOID *Context,  OPTIONAL
+  IN OUT VOID *ExternalScratchBuffer
   )
 {
-  EFI_STATUS Status;
-  EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState;
-  SCRATCH_BUFFER *ScratchBuffer;
-  FW_CFG_DMA_ACCESS  *Access;
-  UINT64 BigEndianAddressOfAccess;
-  UINT32

[edk2] [PATCH 09/12] OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for PEI fw_cfg instance

2017-02-22 Thread Laszlo Ersek

In the PEI fw_cfg instance:

- QemuFwCfgS3Enabled() queries S3 enablement via fw_cfg. This behavior is
  shared with the DXE fw_cfg instance, and the PEI fw_cfg instance already
  pulls in the function from "QemuFwCfgS3PeiDxe.c".

- If QemuFwCfgS3Enabled() returns TRUE, the client module is permitted to
  call QemuFwCfgS3TransferOwnership(). However, in the PEI phase we have
  no support for capturing ACPI S3 Boot Script opcodes, hence we return
  RETURN_UNSUPPORTED unconditionally. This behavior is unique to the PEI
  fw_cfg instance, so add the function to "QemuFwCfgS3Pei.c".

- Consequently, the QemuFwCfgS3WriteBytes(), QemuFwCfgS3ReadBytes(),
  QemuFwCfgS3SkipBytes(), and QemuFwCfgS3CheckValue() functions must never
  be called. (They could only be called from the client module's callback,
  but QemuFwCfgS3TransferOwnership() will never install such callback in
  the PEI fw_cfg instance -- see above.)

  This behavior is not unique to the PEI fw_cfg instance (it is shared
  with the Base Null instance), so pull in these functions from
  "QemuFwCfgS3BasePei.c".

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf |  3 +
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Pei.c   | 85 

 2 files changed, 88 insertions(+)

diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf 
b/OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
index 2593af8e5b4c..890862076e81 100644
--- a/OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
@@ -31,6 +31,8 @@ [Defines]
 #
 
 [Sources]
+  QemuFwCfgS3BasePei.c
+  QemuFwCfgS3Pei.c
   QemuFwCfgS3PeiDxe.c
 
 [Packages]
@@ -38,4 +40,5 @@ [Packages]
   OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
+  DebugLib
   QemuFwCfgLib
diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Pei.c 
b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Pei.c
new file mode 100644
index ..77ed058884e1
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Pei.c
@@ -0,0 +1,85 @@
+/** @file
+  Limited functionality QemuFwCfgS3Lib instance, for PEI phase modules.
+
+  QemuFwCfgS3Enabled() queries S3 enablement via fw_cfg. Other library APIs
+  will report lack of support.
+
+  Copyright (C) 2017, Red Hat, Inc.
+
+  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 
+
+/**
+  Install the client module's FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION callback for
+  when the production of ACPI S3 Boot Script opcodes becomes possible.
+
+  Take ownership of the client-provided Context, and pass it to the callback
+  function, when the latter is invoked.
+
+  Allocate scratch space for those ACPI S3 Boot Script opcodes to work upon
+  that the client will produce in the callback function.
+
+  @param[in] Append FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION to invoke
+when the production of ACPI S3 Boot Script
+opcodes becomes possible. Append() may be
+called immediately from
+QemuFwCfgS3TransferOwnership().
+
+  @param[in,out] ContextClient-provided data structure for the Append()
+callback function to consume.
+
+If Context points to dynamically allocated
+memory, then Append() must release it.
+
+If Context points to dynamically allocated
+memory, and QemuFwCfgS3TransferOwnership()
+returns successfully, then the caller of
+QemuFwCfgS3TransferOwnership() must neither
+dereference nor even evaluate Context any
+longer, as ownership of the referenced area has
+been transferred to Append().
+
+  @param[in] ScratchBufferSize  The size of the scratch buffer that will hold,
+in reserved memory, all client data read,
+written, and checked by the ACPI S3 Boot Script
+opcodes produced by Append().
+
+  @retval RETURN_UNSUPPORTED   The library instance does not support this
+   function.
+
+  @retval RETURN_NOT_FOUND The fw_cfg

[edk2] [PATCH 08/12] OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for Base Null instance

2017-02-22 Thread Laszlo Ersek

In the Base Null instance:

- QemuFwCfgS3Enabled() returns constant FALSE. This is unique to the Base
  Null instance, and the function is already present in
  "QemuFwCfgS3Base.c".

- The QemuFwCfgS3TransferOwnership() function must never be called
  (according to the documentation, given the above). This is also unique
  to the Base Null instance, so implement the function in
  "QemuFwCfgS3Base.c".

- Consequently, the QemuFwCfgS3WriteBytes(), QemuFwCfgS3ReadBytes(),
  QemuFwCfgS3SkipBytes(), and QemuFwCfgS3CheckValue() functions must never
  be called either. This behavior is not unique to the Base Null instance
  (it will be shared with the PEI fw_cfg instance), so add these functions
  to "QemuFwCfgS3BasePei.c".

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf |   4 +
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c  |  70 ++
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3BasePei.c   | 227 

 3 files changed, 301 insertions(+)

diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf 
b/OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
index ba24a0b9d434..837fd70db6e5 100644
--- a/OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
@@ -33,7 +33,11 @@ [Defines]
 
 [Sources]
   QemuFwCfgS3Base.c
+  QemuFwCfgS3BasePei.c
 
 [Packages]
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  DebugLib
diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c 
b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c
index bed9bf3dfb57..834ac8e523c7 100644
--- a/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c
@@ -16,6 +16,7 @@
   WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 **/
 
+#include 
 #include 
 
 /**
@@ -37,3 +38,72 @@ QemuFwCfgS3Enabled (
 {
   return FALSE;
 }
+
+
+/**
+  Install the client module's FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION callback for
+  when the production of ACPI S3 Boot Script opcodes becomes possible.
+
+  Take ownership of the client-provided Context, and pass it to the callback
+  function, when the latter is invoked.
+
+  Allocate scratch space for those ACPI S3 Boot Script opcodes to work upon
+  that the client will produce in the callback function.
+
+  @param[in] Append FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION to invoke
+when the production of ACPI S3 Boot Script
+opcodes becomes possible. Append() may be
+called immediately from
+QemuFwCfgS3TransferOwnership().
+
+  @param[in,out] ContextClient-provided data structure for the Append()
+callback function to consume.
+
+If Context points to dynamically allocated
+memory, then Append() must release it.
+
+If Context points to dynamically allocated
+memory, and QemuFwCfgS3TransferOwnership()
+returns successfully, then the caller of
+QemuFwCfgS3TransferOwnership() must neither
+dereference nor even evaluate Context any
+longer, as ownership of the referenced area has
+been transferred to Append().
+
+  @param[in] ScratchBufferSize  The size of the scratch buffer that will hold,
+in reserved memory, all client data read,
+written, and checked by the ACPI S3 Boot Script
+opcodes produced by Append().
+
+  @retval RETURN_UNSUPPORTED   The library instance does not support this
+   function.
+
+  @retval RETURN_NOT_FOUND The fw_cfg DMA interface to QEMU is
+   unavailable.
+
+  @retval RETURN_BAD_BUFFER_SIZE   ScratchBufferSize is too large.
+
+  @retval RETURN_OUT_OF_RESOURCES  Memory allocation failed.
+
+  @retval RETURN_SUCCESS   Append() has been installed, and the
+   ownership of Context has been transferred.
+   Reserved memory has been allocated for the
+   scratch buffer.
+
+   A successful invocation of
+   QemuFwCfgS3TransferOwnership() cannot be
+   rolled back.
+
+  @return  Error codes from underlying functions.
+**/
+EFIAPI
+RETURN_STATUS

[edk2] [PATCH 07/12] OvmfPkg/QemuFwCfgS3Lib: add boot script opcode generation APIs to libclass

2017-02-22 Thread Laszlo Ersek

Introduce the following APIs:

- QemuFwCfgS3TransferOwnership(): central function that registers a
  callback function, with a context parameter, for when ACPI S3 Boot
  Script opcodes can be produced. This function also allocates reserved
  memory for the opcodes to operate upon.

  The client module is supposed to produce the boot script fragment in the
  callback function.

- QemuFwCfgS3WriteBytes(), QemuFwCfgS3ReadBytes(), QemuFwCfgS3SkipBytes(),
  QemuFwCfgS3CheckValue(): helper functions, available only to the above
  callback function, for composing the boot script fragment.
  QemuFwCfgS3SkipBytes() can double as a plain "select" whenever
  necessary.

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Include/Library/QemuFwCfgS3Lib.h | 318 
 1 file changed, 318 insertions(+)

diff --git a/OvmfPkg/Include/Library/QemuFwCfgS3Lib.h 
b/OvmfPkg/Include/Library/QemuFwCfgS3Lib.h
index 1c473610d11c..3499de3c74cc 100644
--- a/OvmfPkg/Include/Library/QemuFwCfgS3Lib.h
+++ b/OvmfPkg/Include/Library/QemuFwCfgS3Lib.h
@@ -19,6 +19,8 @@
 #ifndef __FW_CFG_S3_LIB__
 #define __FW_CFG_S3_LIB__
 
+#include 
+
 /**
   Determine if S3 support is explicitly enabled.
 
@@ -36,4 +38,320 @@ QemuFwCfgS3Enabled (
   VOID
   );
 
+
+/**
+  Prototype for the callback function that the client module provides.
+
+  In the callback function, the client module calls the
+  QemuFwCfgS3WriteBytes(), QemuFwCfgS3ReadBytes(), QemuFwCfgS3SkipBytes(), and
+  QemuFwCfgS3CheckValue() functions. Those functions produce ACPI S3 Boot
+  Script opcodes that will perform fw_cfg DMA operations, and will check any
+  desired values that were read, during S3 resume.
+
+  The callback function is invoked when the production of ACPI S3 Boot Script
+  opcodes becomes possible. This may occur directly on the call stack of
+  QemuFwCfgS3TransferOwnership() (see below), or after
+  QemuFwCfgS3TransferOwnership() has successfully returned.
+
+  The callback function must not return if it fails -- in the general case,
+  there is noone to propagate any errors to. Therefore, on error, an error
+  message should be logged, and CpuDeadLoop() must be called.
+
+  @param[in,out] ContextCarries information from the client module
+itself (i.e., from the invocation of
+QemuFwCfgS3TransferOwnership()) to the callback
+function.
+
+If Context points to dynamically allocated
+storage, then the callback function must
+release it.
+
+  @param[in,out] ScratchBuffer  Points to reserved memory, allocated by
+QemuFwCfgS3TransferOwnership() internally.
+
+ScratchBuffer is typed and sized by the client
+module when it calls
+QemuFwCfgS3TransferOwnership(). The client
+module defines a union type of structures for
+ScratchBuffer such that the union can hold
+client data for any desired fw_cfg DMA read and
+write operations, and value checking.
+
+The callback function casts ScratchBuffer to
+the union type described above. It passes union
+member sizes as NumberOfBytes to
+QemuFwCfgS3ReadBytes() and
+QemuFwCfgS3WriteBytes(). It passes field
+addresses and sizes in structures in the union
+as ScratchData and ValueSize to
+QemuFwCfgS3CheckValue().
+
+ScratchBuffer is aligned at 8 bytes.
+**/
+typedef
+VOID (EFIAPI FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION) (
+  IN OUT VOID *Context,  OPTIONAL
+  IN OUT VOID *ScratchBuffer
+  );
+
+
+/**
+  Install the client module's FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION callback for
+  when the production of ACPI S3 Boot Script opcodes becomes possible.
+
+  Take ownership of the client-provided Context, and pass it to the callback
+  function, when the latter is invoked.
+
+  Allocate scratch space for those ACPI S3 Boot Script opcodes to work upon
+  that the client will produce in the callback function.
+
+  @param[in] Append FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION to invoke
+when the production of ACPI S3 Boot Script
+opcodes becomes possible. Append() may be
+called immediately from
+

[edk2] [PATCH 02/12] OvmfPkg/QemuFwCfgS3Lib: add initial Base Null library instance

2017-02-22 Thread Laszlo Ersek

This library instance returns constant FALSE from QemuFwCfgS3Enabled(),
and all other library functions trigger assertion failures. It is suitable
for QEMU targets and machine types that never enable S3.

The QemuFwCfgS3Enabled() implementation is copied from
"ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c". Stubs for further
QemuFwCfgS3Lib APIs (with assertion failures, see above) will be added
later.

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf | 39 

 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c  | 39 

 2 files changed, 78 insertions(+)

diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf 
b/OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
new file mode 100644
index ..ba24a0b9d434
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
@@ -0,0 +1,39 @@
+## @file
+# Base Null library instance of the QemuFwCfgS3Lib class.
+#
+# This library instance returns constant FALSE from QemuFwCfgS3Enabled(), and
+# all other library functions trigger assertion failures. It is suitable for
+# QEMU targets and machine types that never enable S3.
+#
+# Copyright (C) 2017, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+##
+
+[Defines]
+  INF_VERSION= 1.25
+  BASE_NAME  = BaseQemuFwCfgS3LibNull
+  FILE_GUID  = EA7D2B69-D221-4950-9C2C-C38A65BCC96E
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = QemuFwCfgS3Lib
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES   = IA32 X64 ARM AARCH64 IPF EBC
+#
+
+[Sources]
+  QemuFwCfgS3Base.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c 
b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c
new file mode 100644
index ..bed9bf3dfb57
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c
@@ -0,0 +1,39 @@
+/** @file
+  Base Null library instance of the QemuFwCfgS3Lib class.
+
+  This library instance returns constant FALSE from QemuFwCfgS3Enabled(), and
+  all other library functions trigger assertion failures. It is suitable for
+  QEMU targets and machine types that never enable S3.
+
+  Copyright (C) 2017, Red Hat, Inc.
+
+  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 
+
+/**
+  Determine if S3 support is explicitly enabled.
+
+  @retval  TRUE   If S3 support is explicitly enabled. Other functions in this
+  library may be called (subject to their individual
+  restrictions).
+
+   FALSE  Otherwise. This includes unavailability of the firmware
+  configuration interface. No other function in this library
+  must be called.
+**/
+BOOLEAN
+EFIAPI
+QemuFwCfgS3Enabled (
+  VOID
+  )
+{
+  return FALSE;
+}
-- 
2.9.3


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

[edk2] [PATCH 05/12] OvmfPkg: resolve QemuFwCfgS3Lib

2017-02-22 Thread Laszlo Ersek

QemuFwCfgS3Enabled() in "OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c"
queries the "etc/system-states" fw_cfg file.

The same implementation is now available factored-out in
"OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c". It is available to
PEIMs through the PeiQemuFwCfgS3LibFwCfg instance, and to DXE_DRIVER and
DXE_RUNTIME_DRIVER modules through the DxeQemuFwCfgS3LibFwCfg instance.

Resolve QemuFwCfgS3Lib accordingly.

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/OvmfPkgIa32.dsc| 3 +++
 OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
 OvmfPkg/OvmfPkgX64.dsc | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 993547d4859e..68ac1ba22cae 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -232,6 +232,7 @@ [LibraryClasses.common.PEIM]
 !endif
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -265,6 +266,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -310,6 +312,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_APPLICATION]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index f36604ecb4d8..ee3ca38ba243 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -237,6 +237,7 @@ [LibraryClasses.common.PEIM]
 !endif
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -270,6 +271,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -315,6 +317,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_APPLICATION]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index c5bf1a672b1e..2ae6b5ad4488 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -237,6 +237,7 @@ [LibraryClasses.common.PEIM]
 !endif
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -270,6 +271,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -315,6 +317,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_APPLICATION]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-- 
2.9.3


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

[edk2] [PATCH 00/12] ArmVirtPkg, OvmfPkg: factor out QemuFwCfgS3Lib

2017-02-22 Thread Laszlo Ersek

The new QemuFwCfgS3Lib class has two goals:

(a) to query whether S3 support was enabled on the QEMU command line,

(b) to save fw_cfg DMA operations that are to be replayed at S3 resume
time, and more easily for the programmer than hacking Boot Script
opcodes manually.

Patches #1 through #5 introduce the new library class, with Base Null,
PEI fw_cfg, and DXE fw_cfg instances, covering goal (a) in both
ArmVirtPkg and OvmfPkg.

Patch #6 retires QemuFwCfgS3Enabled() from QemuFwCfgLib (including
library class and all instances), and switches all client modules to
QemuFwCfgS3Lib. This separates S3 concerns from QemuFwCfgLib.

Patches #7 through #10 cover goal (b) for all three library instances
(at levels of support that are appropriate for each, of course).

Patches #11 and #12 put the new library class to use in
OvmfPkg/SmmControl2Dxe and OvmfPkg/AcpiPlatformDxe, eliminating such
ACPI S3 Boot Script opcode hacking that is related to fw_cfg. (For
OvmfPkg/SmmControl2Dxe, that's "most of it", for
OvmfPkg/AcpiPlatformDxe, it's "all of it".)

I tested:
- ArmVirtQemu boot,
- OVMF boot without SMM, with S3 enabled and disabled, using a Linux
  guest (and when S3 was enabled, I exercised it),
- OVMF boot with SMM, with S3 enabled and disabled, using Linux and
  Windows guests,
  - and whenever S3 was enabled, I exercised it
  - and in the Windows guest, I tested VMGENID / WRITE_POINTER too.

The diffstat looks scary, but it's due to comments, I promise.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Repo: https://github.com/lersek/edk2.git
Branch:   fw_cfg_s3

NOTE: if you want to fetch & test the branch, you'll have to revert
recent commit dc4c770763d0 ("BaseTools: add error check for Macro usage
in the INF file", 2017-02-20) on top. It causes BaseTools to mis-build
OVMF. I reported the regression on the list already, in that patch's
thread.

Cc: Ard Biesheuvel 
Cc: Jordan Justen 

Thanks
Laszlo

Laszlo Ersek (12):
  OvmfPkg: introduce QemuFwCfgS3Lib class
  OvmfPkg/QemuFwCfgS3Lib: add initial Base Null library instance
  OvmfPkg/QemuFwCfgS3Lib: add initial PEI and DXE fw_cfg library
instances
  ArmVirtPkg: resolve QemuFwCfgS3Lib
  OvmfPkg: resolve QemuFwCfgS3Lib
  ArmVirtPkg, OvmfPkg: retire QemuFwCfgS3Enabled() from QemuFwCfgLib
  OvmfPkg/QemuFwCfgS3Lib: add boot script opcode generation APIs to
libclass
  OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for Base Null instance
  OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for PEI fw_cfg instance
  OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for DXE fw_cfg instance
  OvmfPkg/SmmControl2Dxe: save fw_cfg boot script with QemuFwCfgS3Lib
  OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib

 ArmVirtPkg/ArmVirtQemu.dsc|   1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc  |   1 +
 ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c|  17 -
 OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h|   2 +-
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf   |   2 +-
 OvmfPkg/AcpiPlatformDxe/BootScript.c  | 262 ++-
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c   |   8 +
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf  |   2 +-
 OvmfPkg/Include/Library/QemuFwCfgLib.h|  14 -
 OvmfPkg/Include/Library/QemuFwCfgS3Lib.h  | 357 
+
 OvmfPkg/Library/LockBoxLib/LockBoxDxe.c   |   1 +
 OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  |   1 +
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h  |   1 +
 OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |   1 +
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c   |  28 -
 OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf |  43 ++
 OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf |  46 ++
 OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf |  44 ++
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c  | 109 +++
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3BasePei.c   | 227 ++
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Dxe.c   | 791 

 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Pei.c   |  85 +++
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c|  48 ++
 OvmfPkg/OvmfPkg.dec   |   4 +
 OvmfPkg/OvmfPkgIa32.dsc   |   3 +
 OvmfPkg/OvmfPkgIa32X64.dsc|   3 +
 OvmfPkg/OvmfPkgX64.dsc|   3 +
 OvmfPkg/PlatformPei/Platform.c|   1 +
 OvmfPkg/PlatformPei/PlatformPei.inf

[edk2] [PATCH 06/12] ArmVirtPkg, OvmfPkg: retire QemuFwCfgS3Enabled() from QemuFwCfgLib

2017-02-22 Thread Laszlo Ersek

At this point we're ready to retire QemuFwCfgS3Enabled() from the
QemuFwCfgLib class, together with its implementations in:

- ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
- OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c

Extend all modules that call the function with a new QemuFwCfgS3Lib class
dependency. Thanks to the previously added library class, instances, and
class resolutions, we can do this switch now as tightly as possible.

Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf   |  1 +
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf  |  1 +
 OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  |  1 +
 OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 +
 OvmfPkg/PlatformPei/PlatformPei.inf   |  1 +
 OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf |  1 +
 OvmfPkg/Include/Library/QemuFwCfgLib.h| 14 
--
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h  |  1 +
 ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c| 17 

 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c   |  1 +
 OvmfPkg/Library/LockBoxLib/LockBoxDxe.c   |  1 +
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c   | 28 

 OvmfPkg/PlatformPei/Platform.c|  1 +
 OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c   |  1 +
 14 files changed, 11 insertions(+), 59 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf 
b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index bb5f14e0fc7a..42edc97b3da2 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -52,6 +52,7 @@ [LibraryClasses]
   UefiDriverEntryPoint
   HobLib
   QemuFwCfgLib
+  QemuFwCfgS3Lib
   MemoryAllocationLib
   BaseLib
   DxeServicesTableLib
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf 
b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
index e550ff5a4714..a9350540215d 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
@@ -44,6 +44,7 @@ [LibraryClasses]
   MemoryAllocationLib
   OrderedCollectionLib
   QemuFwCfgLib
+  QemuFwCfgS3Lib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
 
diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf 
b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
index bedf1811e0b2..eb03f4f546bc 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
@@ -40,6 +40,7 @@ [LibraryClasses]
   DebugLib
   UefiBootServicesTableLib
   QemuFwCfgLib
+  QemuFwCfgS3Lib
 
 [Protocols]
   gEfiLockBoxProtocolGuid## SOMETIMES_PRODUCES
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index f9e35c955d4d..27789b7377bc 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -52,6 +52,7 @@ [LibraryClasses]
   PciLib
   NvVarsFileLib
   QemuFwCfgLib
+  QemuFwCfgS3Lib
   LoadLinuxLib
   QemuBootOrderLib
   UefiLib
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index fbaed3182dcf..53c6dd445a0e 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -58,6 +58,7 @@ [LibraryClasses]
   PeiServicesTablePointerLib
   PeimEntryPoint
   QemuFwCfgLib
+  QemuFwCfgS3Lib
   MtrrLib
   PcdLib
 
diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf 
b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
index 31c80bd4448c..04b1ed0e4eb3 100644
--- a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
+++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
@@ -56,6 +56,7 @@ [LibraryClasses]
   PcdLib
   PciLib
   QemuFwCfgLib
+  QemuFwCfgS3Lib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
 
diff --git a/OvmfPkg/Include/Library/QemuFwCfgLib.h 
b/OvmfPkg/Include/Library/QemuFwCfgLib.h
index 2a1261327b01..596e3f25d5fe 100644
--- a/OvmfPkg/Include/Library/QemuFwCfgLib.h
+++ b/OvmfPkg/Include/Library/QemuFwCfgLib.h
@@ -179,19 +179,5 @@ QemuFwCfgFindFile (
   OUT  UINTN *Size
   );
 
-
-/**
-  Determine if S3 support is explicitly enabled.
-
-  @retval  TRUE   if S3 support is explicitly enabled.
-   FALSE  otherwise. This includes unavailability of the firmware
-  configuration interface.
-**/
-BOOLEAN
-EFIAPI
-QemuFwCfgS3Enabled (
-  VOID
-  );
-
 #endif
 
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h

[edk2] [PATCH 01/12] OvmfPkg: introduce QemuFwCfgS3Lib class

2017-02-22 Thread Laszlo Ersek

This library class will enable driver modules (a) to query whether S3
support was enabled on the QEMU command line, (b) to produce fw_cfg DMA
operations that are to be replayed at S3 resume time.

Declare the library class in OvmfPkg/OvmfPkg.dec, and add the library
class header under OvmfPkg/Include/Library/. At the moment, the only API
we expose is QemuFwCfgS3Enabled(), which we'll first migrate from
QemuFwCfgLib. Further interfaces will be added in later patches.

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Suggested-by: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/OvmfPkg.dec  |  4 ++
 OvmfPkg/Include/Library/QemuFwCfgS3Lib.h | 39 
 2 files changed, 43 insertions(+)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index a0c76a5bb448..3490f7ca7c07 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -35,6 +35,10 @@ [LibraryClasses]
   #
   QemuFwCfgLib|Include/Library/QemuFwCfgLib.h
 
+  ##  @libraryclass  S3 support for QEMU fw_cfg
+  #
+  QemuFwCfgS3Lib|Include/Library/QemuFwCfgS3Lib.h
+
   ##  @libraryclass  Rewrite the BootOrder NvVar based on QEMU's "bootorder"
   #  fw_cfg file.
   #
diff --git a/OvmfPkg/Include/Library/QemuFwCfgS3Lib.h 
b/OvmfPkg/Include/Library/QemuFwCfgS3Lib.h
new file mode 100644
index ..1c473610d11c
--- /dev/null
+++ b/OvmfPkg/Include/Library/QemuFwCfgS3Lib.h
@@ -0,0 +1,39 @@
+/** @file
+  S3 support for QEMU fw_cfg
+
+  This library class enables driver modules (a) to query whether S3 support was
+  enabled on the QEMU command line, (b) to produce fw_cfg DMA operations that
+  are to be replayed at S3 resume time.
+
+  Copyright (C) 2017, Red Hat, Inc.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#ifndef __FW_CFG_S3_LIB__
+#define __FW_CFG_S3_LIB__
+
+/**
+  Determine if S3 support is explicitly enabled.
+
+  @retval  TRUE   If S3 support is explicitly enabled. Other functions in this
+  library may be called (subject to their individual
+  restrictions).
+
+   FALSE  Otherwise. This includes unavailability of the firmware
+  configuration interface. No other function in this library
+  must be called.
+**/
+BOOLEAN
+EFIAPI
+QemuFwCfgS3Enabled (
+  VOID
+  );
+
+#endif
-- 
2.9.3


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

[edk2] [PATCH 03/12] OvmfPkg/QemuFwCfgS3Lib: add initial PEI and DXE fw_cfg library instances

2017-02-22 Thread Laszlo Ersek

This patch introduces PeiQemuFwCfgS3LibFwCfg, a limited functionality
QemuFwCfgS3Lib instance, for PEI phase modules.

The patch also introduces DxeQemuFwCfgS3LibFwCfg, a full functionality
QemuFwCfgS3Lib instance, for DXE_DRIVER and DXE_RUNTIME_DRIVER modules.

These library instances share the QemuFwCfgS3Enabled() function. The
function actually uses fw_cfg; the implementation is copied from
"OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c".

The library instances will diverge in the following patches.

Cc: Jordan Justen 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf | 38 
 OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf | 41 
+
 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c| 48 

 3 files changed, 127 insertions(+)

diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf 
b/OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
new file mode 100644
index ..7016575f3dab
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
@@ -0,0 +1,38 @@
+## @file
+# Full functionality QemuFwCfgS3Lib instance, for DXE phase modules.
+#
+# Copyright (C) 2017, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+##
+
+[Defines]
+  INF_VERSION= 1.25
+  BASE_NAME  = DxeQemuFwCfgS3LibFwCfg
+  FILE_GUID  = C5DE76EB-E8DE-4057-A487-C5A09AB039AB
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = QemuFwCfgS3Lib|DXE_DRIVER DXE_RUNTIME_DRIVER
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES   = IA32 X64 ARM AARCH64 IPF EBC
+#
+
+[Sources]
+  QemuFwCfgS3PeiDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  QemuFwCfgLib
diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf 
b/OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
new file mode 100644
index ..2593af8e5b4c
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
@@ -0,0 +1,41 @@
+## @file
+# Limited functionality QemuFwCfgS3Lib instance, for PEI phase modules.
+#
+# QemuFwCfgS3Enabled() queries S3 enablement via fw_cfg. Other library APIs
+# will report lack of support.
+#
+# Copyright (C) 2017, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+##
+
+[Defines]
+  INF_VERSION= 1.25
+  BASE_NAME  = PeiQemuFwCfgS3LibFwCfg
+  FILE_GUID  = DD8D28B4-C1DC-4CAF-BB93-074BE80DAE6D
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = QemuFwCfgS3Lib|PEIM
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES   = IA32 X64 ARM AARCH64 IPF EBC
+#
+
+[Sources]
+  QemuFwCfgS3PeiDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  QemuFwCfgLib
diff --git a/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c 
b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c
new file mode 100644
index ..f87d6b8227cf
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c
@@ -0,0 +1,48 @@
+/** @file
+  Shared code for the PEI fw_cfg and DXE fw_cfg instances of the QemuFwCfgS3Lib
+  class.
+
+  Copyright (C) 2017, Red Hat, Inc.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#include 
+#include 
+
+/**
+  Determine if S3 support is explicitly enabled.
+
+  @retval  TRUE   If S3

[edk2] [PATCH 04/12] ArmVirtPkg: resolve QemuFwCfgS3Lib

2017-02-22 Thread Laszlo Ersek

QemuFwCfgS3Enabled() in "ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c"
returns constant FALSE.

The same implementation is now available factored-out in
"OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c".

Resolve QemuFwCfgS3Lib to BaseQemuFwCfgS3LibNull.

Cc: Ard Biesheuvel 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
 2 files changed, 2 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 8fe3c3816961..9c8a2d977a8a 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -52,6 +52,7 @@ [LibraryClasses.common]
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
   
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
   QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
 
   ArmPlatformLib|ArmVirtPkg/Library/ArmVirtPlatformLib/ArmVirtPlatformLib.inf
   
ArmPlatformSysConfigLib|ArmPlatformPkg/Library/ArmPlatformSysConfigLibNull/ArmPlatformSysConfigLibNull.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index aa40374745af..6afc10e69ef5 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -51,6 +51,7 @@ [LibraryClasses.common]
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
   
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
   QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
+  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
 
   
ArmPlatformLib|ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/ArmQemuRelocatablePlatformLib.inf
   
ArmPlatformSysConfigLib|ArmPlatformPkg/Library/ArmPlatformSysConfigLibNull/ArmPlatformSysConfigLibNull.inf
-- 
2.9.3


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

Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file

2017-02-22 Thread Gao, Liming

Laszlo:
  Yonghong has sent the another patch to its regression issue. Could you verify 
it?

Thanks
Liming
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, February 23, 2017 9:12 AM
To: Zhu, Yonghong ; edk2-de...@ml01.01.org
Cc: Gao, Liming 
Subject: Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the 
INF file

On 02/23/17 02:02, Laszlo Ersek wrote:
> Hi,
> 
> On 02/21/17 02:18, Yonghong Zhu wrote:
>> Use of MACRO statements in the EDK II INF files is limited to local
>> usage only; global or external macros are not permitted. This patch
>> add the check for not defined macros.
>>
>> Cc: Liming Gao 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Yonghong Zhu 
>> ---
>>  BaseTools/Source/Python/Workspace/MetaFileParser.py| 9 -
>>  BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++-
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
>> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
>> index 1a5fdf5..37a7f5d 100644
>> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
>> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
>> @@ -1,9 +1,9 @@
>>  ## @file
>>  # This file is used to parse meta files
>>  #
>> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
>> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>>  # (C) Copyright 2015-2016 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
>>  # which accompanies this distribution.  The full text of the license may be 
>> found at
>>  # http://opensource.org/licenses/bsd-license.php
>> @@ -349,10 +349,17 @@ class MetaFileParser(object):
>>  EdkLogger.error('Parser', FORMAT_INVALID, "No value specified",
>>  ExtraData=self._CurrentLine, 
>> File=self.MetaFile, Line=self._LineIndex + 1)
>>  
>>  self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in 
>> self._ValueList]
>>  Name, Value = self._ValueList[1], self._ValueList[2]
>> +MacroUsed = GlobalData.gMacroRefPattern.findall(Value)
>> +if len(MacroUsed) != 0:
>> +for Macro in MacroUsed:
>> +if Macro in GlobalData.gGlobalDefines:
>> +EdkLogger.error("Parser", FORMAT_INVALID, "Global macro 
>> %s is not permitted." % (Macro), ExtraData=self._CurrentLine, 
>> File=self.MetaFile, Line=self._LineIndex + 1)
>> +else:
>> +EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" 
>> % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, 
>> Line=self._LineIndex + 1)
>>  # Sometimes, we need to make differences between EDK and EDK2 
>> modules 
>>  if Name == 'INF_VERSION':
>>  if re.match(r'0[xX][\da-f-A-F]{5,8}', Value):
>>  self._Version = int(Value, 0)   
>>  elif re.match(r'\d+\.\d+', Value):
>> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
>> b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>> index e7bc87d..0686721 100644
>> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>> @@ -1,9 +1,9 @@
>>  ## @file
>>  # This file is used to create a database used by build tool
>>  #
>> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
>> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>>  # (C) Copyright 2016 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
>>  # which accompanies this distribution.  The full text of the license may be 
>> found at
>>  # http://opensource.org/licenses/bsd-license.php
>> @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject):
>>  self.__Macros = {}
>>  # EDK_GLOBAL defined macros can be applied to EDK module
>>  if self.AutoGenVersion < 0x00010005:
>>  self.__Macros.update(GlobalData.gEdkGlobal)
>>  self.__Macros.update(GlobalData.gGlobalDefines)
>> +else:
>> +self.__Macros.update(self.Defines)
>>  return self.__Macros
>>  
>>  ## Get architecture
>>  def _GetArch(self):
>>  return self._Arch
>>
> 
> I don't understand how, but this patch (commit dc4c770763d0) breaks OVMF for 
> me.
> 
> I bisected it, I didn't want to believe it, then I built the tree right 
> before it, and that worked.
> 
> I also built the tree at current master (526f160f311c, "ArmPkg/ArmMmuLib: 
> AARCH64: enable stack alignment

Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file

2017-02-22 Thread Laszlo Ersek

On 02/23/17 02:02, Laszlo Ersek wrote:
> Hi,
> 
> On 02/21/17 02:18, Yonghong Zhu wrote:
>> Use of MACRO statements in the EDK II INF files is limited to local
>> usage only; global or external macros are not permitted. This patch
>> add the check for not defined macros.
>>
>> Cc: Liming Gao 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Yonghong Zhu 
>> ---
>>  BaseTools/Source/Python/Workspace/MetaFileParser.py| 9 -
>>  BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++-
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
>> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
>> index 1a5fdf5..37a7f5d 100644
>> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
>> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
>> @@ -1,9 +1,9 @@
>>  ## @file
>>  # This file is used to parse meta files
>>  #
>> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
>> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>>  # (C) Copyright 2015-2016 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
>>  # which accompanies this distribution.  The full text of the license may be 
>> found at
>>  # http://opensource.org/licenses/bsd-license.php
>> @@ -349,10 +349,17 @@ class MetaFileParser(object):
>>  EdkLogger.error('Parser', FORMAT_INVALID, "No value specified",
>>  ExtraData=self._CurrentLine, 
>> File=self.MetaFile, Line=self._LineIndex + 1)
>>  
>>  self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in 
>> self._ValueList]
>>  Name, Value = self._ValueList[1], self._ValueList[2]
>> +MacroUsed = GlobalData.gMacroRefPattern.findall(Value)
>> +if len(MacroUsed) != 0:
>> +for Macro in MacroUsed:
>> +if Macro in GlobalData.gGlobalDefines:
>> +EdkLogger.error("Parser", FORMAT_INVALID, "Global macro 
>> %s is not permitted." % (Macro), ExtraData=self._CurrentLine, 
>> File=self.MetaFile, Line=self._LineIndex + 1)
>> +else:
>> +EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" 
>> % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, 
>> Line=self._LineIndex + 1)
>>  # Sometimes, we need to make differences between EDK and EDK2 
>> modules 
>>  if Name == 'INF_VERSION':
>>  if re.match(r'0[xX][\da-f-A-F]{5,8}', Value):
>>  self._Version = int(Value, 0)   
>>  elif re.match(r'\d+\.\d+', Value):
>> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
>> b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>> index e7bc87d..0686721 100644
>> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>> @@ -1,9 +1,9 @@
>>  ## @file
>>  # This file is used to create a database used by build tool
>>  #
>> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
>> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>>  # (C) Copyright 2016 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
>>  # which accompanies this distribution.  The full text of the license may be 
>> found at
>>  # http://opensource.org/licenses/bsd-license.php
>> @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject):
>>  self.__Macros = {}
>>  # EDK_GLOBAL defined macros can be applied to EDK module
>>  if self.AutoGenVersion < 0x00010005:
>>  self.__Macros.update(GlobalData.gEdkGlobal)
>>  self.__Macros.update(GlobalData.gGlobalDefines)
>> +else:
>> +self.__Macros.update(self.Defines)
>>  return self.__Macros
>>  
>>  ## Get architecture
>>  def _GetArch(self):
>>  return self._Arch
>>
> 
> I don't understand how, but this patch (commit dc4c770763d0) breaks OVMF for 
> me.
> 
> I bisected it, I didn't want to believe it, then I built the tree right 
> before it, and that worked.
> 
> I also built the tree at current master (526f160f311c, "ArmPkg/ArmMmuLib: 
> AARCH64: enable stack alignment checking", 2017-02-22), with this commit 
> reverted on top, and that also works.
> 
> This is the error I see in the OVMF log:
> 
>> Loading driver E660EA85-058E-4B55-A54B-F02F83A24707
>> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E8752C0
>> Loading driver at 0x0007E4B7000 EntryPoint=0x0007E4B727B DisplayEngine.efi
>> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7E875518
>>

Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file

2017-02-22 Thread Laszlo Ersek

Hi,

On 02/21/17 02:18, Yonghong Zhu wrote:
> Use of MACRO statements in the EDK II INF files is limited to local
> usage only; global or external macros are not permitted. This patch
> add the check for not defined macros.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/Workspace/MetaFileParser.py| 9 -
>  BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> index 1a5fdf5..37a7f5d 100644
> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # This file is used to parse meta files
>  #
> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>  # (C) Copyright 2015-2016 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
>  # which accompanies this distribution.  The full text of the license may be 
> found at
>  # http://opensource.org/licenses/bsd-license.php
> @@ -349,10 +349,17 @@ class MetaFileParser(object):
>  EdkLogger.error('Parser', FORMAT_INVALID, "No value specified",
>  ExtraData=self._CurrentLine, File=self.MetaFile, 
> Line=self._LineIndex + 1)
>  
>  self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in 
> self._ValueList]
>  Name, Value = self._ValueList[1], self._ValueList[2]
> +MacroUsed = GlobalData.gMacroRefPattern.findall(Value)
> +if len(MacroUsed) != 0:
> +for Macro in MacroUsed:
> +if Macro in GlobalData.gGlobalDefines:
> +EdkLogger.error("Parser", FORMAT_INVALID, "Global macro 
> %s is not permitted." % (Macro), ExtraData=self._CurrentLine, 
> File=self.MetaFile, Line=self._LineIndex + 1)
> +else:
> +EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % 
> (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, 
> Line=self._LineIndex + 1)
>  # Sometimes, we need to make differences between EDK and EDK2 
> modules 
>  if Name == 'INF_VERSION':
>  if re.match(r'0[xX][\da-f-A-F]{5,8}', Value):
>  self._Version = int(Value, 0)   
>  elif re.match(r'\d+\.\d+', Value):
> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
> b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> index e7bc87d..0686721 100644
> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # This file is used to create a database used by build tool
>  #
> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>  # (C) Copyright 2016 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
>  # which accompanies this distribution.  The full text of the license may be 
> found at
>  # http://opensource.org/licenses/bsd-license.php
> @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject):
>  self.__Macros = {}
>  # EDK_GLOBAL defined macros can be applied to EDK module
>  if self.AutoGenVersion < 0x00010005:
>  self.__Macros.update(GlobalData.gEdkGlobal)
>  self.__Macros.update(GlobalData.gGlobalDefines)
> +else:
> +self.__Macros.update(self.Defines)
>  return self.__Macros
>  
>  ## Get architecture
>  def _GetArch(self):
>  return self._Arch
> 

I don't understand how, but this patch (commit dc4c770763d0) breaks OVMF for me.

I bisected it, I didn't want to believe it, then I built the tree right before 
it, and that worked.

I also built the tree at current master (526f160f311c, "ArmPkg/ArmMmuLib: 
AARCH64: enable stack alignment checking", 2017-02-22), with this commit 
reverted on top, and that also works.

This is the error I see in the OVMF log:

> Loading driver E660EA85-058E-4B55-A54B-F02F83A24707
> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E8752C0
> Loading driver at 0x0007E4B7000 EntryPoint=0x0007E4B727B DisplayEngine.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7E875518
> ProtectUefiImageCommon - 0x7E8752C0
>   - 0x7E4B7000 - 0x0001BAE0
> ASSERT 
>

Re: [edk2] Improvements to build system etc. for edk2-platforms devel-MinnowBoard3?

2017-02-22 Thread Richardson, Brian

We're ok with help fixing issues (yay open source), so thanks for the help. 
Patches are welcome at this time. But we do track them in Bugzilla, so opening 
an issue there is the first step to a solution. 

For the processor thread setting, note that we have historically disabled 
processor threading by default because we don't know the build system 
configuration. While we at Intel prefer everyone own an Intel(R) Core(TM) i7 or 
Intel(R) Xeon processor, we know that's not the case ... so keeping it disabled 
has been seen as a safe option. We should definitely do a better job of 
documenting that setting change, but I think we need to consider generically 
changing that setting by default in target.txt (which requires a Bugzilla 
entry).

The .bat/.sh files are required to trigger post-build tools, which are OS 
dependent. Even if the build was triggered by a Python script, we still need to 
do some verification to make sure there are no functional differences when we 
build in a Windows environment versus Linux (work in progress).

Thanks ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_Brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rebecca 
Cran
Sent: Wednesday, February 22, 2017 11:53 AM
To: Richardson, Brian ; Gao, Liming 
; edk2-devel@lists.01.org
Cc: Lu, ShifeiX A ; Zimmer, Vincent 
; Andrew Fish ; Wei, David 

Subject: Re: [edk2] Improvements to build system etc. for edk2-platforms 
devel-MinnowBoard3?

On 2/22/2017 9:34 AM, Richardson, Brian wrote:
> Thanks for the input. For future reference, you can use the TianoCore 
> Bugzilla to report issues on any EDK II feature/platform. 
> https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues
>
> I agree the readme.md should be present, and use markup instead of plain text 
> to work better in github. You can open an issue on this in Bugzilla.
>
> Normally, we ask folks to change the number of processor threads based on 
> their system configuration. We don't add a larger thread number by default, 
> but it might be good to set it '5' by default (assuming a dual core processor 
> with hyperthreading) instead of '1' (assuming a single core system w/o 
> threading). I don't know if this will cause any compatibility issues on older 
> systems, but it's worth a check.
>
> At this time, MinnowBoard 3 build is only validated in Windows. That's why 
> there is no equivalent .sh file for BuildBIOS yet, but it will be added once 
> Linux build is verified and checked in.

I'm more about _fixing_ issues I find rather than reporting them! Are you 
saying that patches wouldn't be welcome just now?  Is there a reason why you 
don't want to make full use of the CPU while building? And I understand that 
MinnowBoard 3 only builds under Windows at the moment, but if more of it built 
using python (and python is already listed as a prerequisite in the ReadMe 
file) the porting might be simpler.

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

[edk2] [RFC PATCH 4/4] ArmPkg/CpuDxe: remap all data regions non-executable

2017-02-22 Thread Ard Biesheuvel

When installing the CPU arch protocol, iterate over the UEFI memory map
and remove the executable permissions from each encountered non-code
region. Those will be re-added later selectively, to the extent required
according to the image protection policy and section alignment.

With a strict image protection policy in place, this all but eliminates
any regions that are mapped both writable and executable, which is an
significant improvement in security.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Drivers/CpuDxe/CpuDxe.c | 76 
 1 file changed, 76 insertions(+)

diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
index 7d328d096b1e..dd3bf44a00b3 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
@@ -15,6 +15,9 @@
 
 #include "CpuDxe.h"
 
+#include 
+#include 
+
 #include 
 
 
@@ -237,6 +240,74 @@ InitializeDma (
   CpuArchProtocol->DmaBufferAlignment = ArmCacheWritebackGranule ();
 }
 
+STATIC
+VOID
+RemapAllDataRegionsNonExec (
+  VOID
+  )
+{
+  UINTN MemoryMapSize;
+  UINTN MapKey;
+  UINTN DescriptorSize;
+  UINT32DescriptorVersion;
+  EFI_MEMORY_DESCRIPTOR *MemoryMap;
+  EFI_MEMORY_DESCRIPTOR *MemoryMapEntry;
+  EFI_MEMORY_DESCRIPTOR *MemoryMapEnd;
+  EFI_STATUSStatus;
+
+  //
+  // Iterate over the memory map, and remove execute permissions from all
+  // memory regions that are not BootServiceCode or RuntimeServicesCode.
+  //
+
+  //
+  // Get the EFI memory map.
+  //
+  MemoryMapSize = 0;
+  MemoryMap = NULL;
+
+  Status = gBS->GetMemoryMap (
+  ,
+  MemoryMap,
+  ,
+  ,
+  
+  );
+  ASSERT (Status == EFI_BUFFER_TOO_SMALL);
+  do {
+MemoryMap = (EFI_MEMORY_DESCRIPTOR *) AllocatePool (MemoryMapSize);
+ASSERT (MemoryMap != NULL);
+Status = gBS->GetMemoryMap (
+,
+MemoryMap,
+,
+,
+
+);
+if (EFI_ERROR (Status)) {
+  FreePool (MemoryMap);
+}
+  } while (Status == EFI_BUFFER_TOO_SMALL);
+  ASSERT_EFI_ERROR (Status);
+
+  MemoryMapEntry = MemoryMap;
+  MemoryMapEnd   = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap + 
MemoryMapSize);
+  while ((UINTN) MemoryMapEntry < (UINTN) MemoryMapEnd) {
+if ((MemoryMapEntry->Type != EfiBootServicesCode) &&
+(MemoryMapEntry->Type != EfiRuntimeServicesCode)) {
+
+  CpuSetMemoryAttributes (, MemoryMapEntry->PhysicalStart,
+EFI_PAGES_TO_SIZE(MemoryMapEntry->NumberOfPages), EFI_MEMORY_XP);
+  DEBUG((DEBUG_ERROR, "%a: removing exec permissions from 0x%lx - 0x%lx 
(Type == 0x%x)\n",
+__FUNCTION__, MemoryMapEntry->PhysicalStart,
+MemoryMapEntry->PhysicalStart + 
EFI_PAGES_TO_SIZE(MemoryMapEntry->NumberOfPages) - 1,
+MemoryMapEntry->Type));
+}
+MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
+  }
+  FreePool (MemoryMap);
+}
+
 EFI_STATUS
 CpuDxeInitialize (
   IN EFI_HANDLE ImageHandle,
@@ -264,6 +335,11 @@ CpuDxeInitialize (
   //
   SyncCacheConfig ();
 
+  //
+  // Remap all conventional memory as non-executable.
+  //
+  RemapAllDataRegionsNonExec ();
+
   // If the platform is a MPCore system then install the Configuration Table 
describing the
   // secondary core states
   if (ArmIsMpCore()) {
-- 
2.7.4

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

[edk2] [RFC PATCH 1/4] MdeModulePkg/DxeCore: allow BootServicesData->BootServicesCode conversion

2017-02-22 Thread Ard Biesheuvel

Unlike all other PE/COFF images loaded after it, the DXE core is loaded
into BootServicesData memory rather than BootServicesCode memory, due to
the fact that the PEI phase memory allocation routines only distinguish
between boot-time and runtime.

So in preparation of adding support for restricted permissions, allow the
direct conversion of BootServicesData to BootServicesCode.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Mem/Page.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index bda4f6397e91..b0939c596991 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -773,7 +773,8 @@ CoreConvertPagesEx (
   //
   // Debug code - verify conversion is allowed
   //
-  if (!(NewType == EfiConventionalMemory ? 1 : 0) ^ (Entry->Type == 
EfiConventionalMemory ? 1 : 0)) {
+  if (!(NewType == EfiConventionalMemory ? 1 : 0) ^ (Entry->Type == 
EfiConventionalMemory ? 1 : 0) &&
+  !(NewType == EfiBootServicesCode && Entry->Type == 
EfiBootServicesData)) {
 DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory 
types\n"));
 return EFI_NOT_FOUND;
   }
-- 
2.7.4

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

[edk2] [RFC PATCH 2/4] MdeModulePkg/DxeCore: convert the DxeCore memory region to BootServicesCode

2017-02-22 Thread Ard Biesheuvel

Before removing exec permissions from all non-code regions, ensure that
the DXE core itself is covered by a BootServicesCode region, by adding
a new function ConvertDxeCoreImage () and calling it at the right time
from DxeMain ().

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/DxeMain.h |  8 
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  2 ++
 MdeModulePkg/Core/Dxe/Mem/Page.c| 15 +++
 3 files changed, 25 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index b14be9a74d8e..300f19a3aa58 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -2949,4 +2949,12 @@ MemoryProtectionExitBootServicesCallback (
   VOID
   );
 
+/**
+  Convert DXE core image to BootServicesCode memory
+**/
+VOID
+ConvertDxeCoreImage (
+  VOID
+  );
+
 #endif
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c 
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 91e94a78d205..d3a873e737b1 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -396,6 +396,8 @@ DxeMain (
 
   MemoryProfileInstallProtocol ();
 
+  ConvertDxeCoreImage ();
+
   CoreInitializePropertiesTable ();
   CoreInitializeMemoryAttributesTable ();
   CoreInitializeMemoryProtection ();
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index b0939c596991..73b56fccf965 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1951,8 +1951,23 @@ Done:
 }
 
 
+VOID
+ConvertDxeCoreImage (
+  VOID
+  )
+{
+  CoreAcquireMemoryLock ();
 
+  //
+  // Convert the memory region that backs the DXE core to a 'code' region, so
+  // that the strict permissions handling doesn't take our exec permissions
+  // away.
+  //
+  CoreConvertPages ((UINTN)gDxeCoreLoadedImage->ImageBase,
+EFI_SIZE_TO_PAGES (gDxeCoreLoadedImage->ImageSize), EfiBootServicesCode);
 
+  CoreReleaseMemoryLock ();
+}
 
 
 
-- 
2.7.4

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

[edk2] [RFC PATCH 3/4] MdeModulePkg/DxeCore: lift non-exec permissions on loaded images

2017-02-22 Thread Ard Biesheuvel

To ensure that loaded PE/COFF images are executable regardless of
the protection policy and the section alignment, clear all permission
restrictions when loading PE/COFF images.

Subsequently, permissions may be restricted again if the protection
policy and section alignment allow it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index 652da8bf1075..cab06e821e39 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -644,6 +644,14 @@ CoreLoadPeImage (
   InvalidateInstructionCacheRange ((VOID 
*)(UINTN)Image->ImageContext.ImageAddress, 
(UINTN)Image->ImageContext.ImageSize);
 
   //
+  // Remove any permission restrictions.
+  //
+  if (gCpu != NULL) {
+gCpu->SetMemoryAttributes (gCpu, Image->ImageContext.ImageAddress,
+Image->ImageContext.ImageSize, 0);
+  }
+
+  //
   // Copy the machine type from the context to the image private data. This
   // is needed during image unload to know if we should call an EBC protocol
   // to unload the image.
-- 
2.7.4

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

[edk2] [RFC PATCH 0/4] RFC: increased memory protection

2017-02-22 Thread Ard Biesheuvel

Hello all,

This is a proof of concept implementation that removes all executable
permissions from writable memory regions, which greatly enhances security.
It is based on Jiewen's recent work, which is a step in the right direction,
but still leaves most of memory exploitable due to the default R+W+X
permissions.

The idea is that the implementation of the CPU arch protocol goes over the
memory map and removes exec permissions from all regions that are not already
marked as 'code. This requires some preparatory work to ensure that the DxeCore
itself is covered by a BootServicesCode region, not a BootServicesData region.
Exec permissions are re-granted selectively, when the PE/COFF loader allocates
the space for it. Combined with Jiewen's code/data split, this removes all
RWX mapped regions.

There is a caveat, though (and there are likely more of that kind): the EBC
driver will need some work to ensure the thunk buffers have the noexec
restriction lifted. This could be done in the EBC driver, but perhaps it is
better to either
a) modify the DXE core so it always removes noexec restrictions when allocating
   code pages, or
b) add AllocateExecPages/AllocateExecPool() functions to the MemoryAllocationLib
   API

Comments please!

Ard Biesheuvel (4):
  MdeModulePkg/DxeCore: allow BootServicesData->BootServicesCode
conversion
  MdeModulePkg/DxeCore: convert the DxeCore memory region to
BootServicesCode
  MdeModulePkg/DxeCore: lift non-exec permissions on loaded images
  ArmPkg/CpuDxe: remap all data regions non-executable

 ArmPkg/Drivers/CpuDxe/CpuDxe.c  | 76 
 MdeModulePkg/Core/Dxe/DxeMain.h |  8 +++
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  2 +
 MdeModulePkg/Core/Dxe/Image/Image.c |  8 +++
 MdeModulePkg/Core/Dxe/Mem/Page.c| 18 -
 5 files changed, 111 insertions(+), 1 deletion(-)

-- 
2.7.4

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

Re: [edk2] Improvements to build system etc. for edk2-platforms devel-MinnowBoard3?

2017-02-22 Thread Rebecca Cran


On 2/22/2017 9:34 AM, Richardson, Brian wrote:

Thanks for the input. For future reference, you can use the TianoCore Bugzilla 
to report issues on any EDK II feature/platform. 
https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues

I agree the readme.md should be present, and use markup instead of plain text 
to work better in github. You can open an issue on this in Bugzilla.

Normally, we ask folks to change the number of processor threads based on their 
system configuration. We don't add a larger thread number by default, but it 
might be good to set it '5' by default (assuming a dual core processor with 
hyperthreading) instead of '1' (assuming a single core system w/o threading). I 
don't know if this will cause any compatibility issues on older systems, but 
it's worth a check.

At this time, MinnowBoard 3 build is only validated in Windows. That's why 
there is no equivalent .sh file for BuildBIOS yet, but it will be added once 
Linux build is verified and checked in.


I'm more about _fixing_ issues I find rather than reporting them! Are 
you saying that patches wouldn't be welcome just now?  Is there a reason 
why you don't want to make full use of the CPU while building? And I 
understand that MinnowBoard 3 only builds under Windows at the moment, 
but if more of it built using python (and python is already listed as a 
prerequisite in the ReadMe file) the porting might be simpler.


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

Re: [edk2] Improvements to build system etc. for edk2-platforms devel-MinnowBoard3?

2017-02-22 Thread Richardson, Brian

Thanks for the input. For future reference, you can use the TianoCore Bugzilla 
to report issues on any EDK II feature/platform. 
https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues   

I agree the readme.md should be present, and use markup instead of plain text 
to work better in github. You can open an issue on this in Bugzilla.

Normally, we ask folks to change the number of processor threads based on their 
system configuration. We don't add a larger thread number by default, but it 
might be good to set it '5' by default (assuming a dual core processor with 
hyperthreading) instead of '1' (assuming a single core system w/o threading). I 
don't know if this will cause any compatibility issues on older systems, but 
it's worth a check.

At this time, MinnowBoard 3 build is only validated in Windows. That's why 
there is no equivalent .sh file for BuildBIOS yet, but it will be added once 
Linux build is verified and checked in.

Thanks ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_Brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rebecca 
Cran
Sent: Wednesday, February 22, 2017 12:48 AM
To: Gao, Liming ; edk2-devel@lists.01.org
Cc: Lu, ShifeiX A ; Zimmer, Vincent 
; Andrew Fish ; Gao, Liming 
; Wei, David 
Subject: Re: [edk2] Improvements to build system etc. for edk2-platforms 
devel-MinnowBoard3?

On 2/21/2017 9:50 PM, Gao, Liming wrote:

>Could you introduce what change will be done for build improvement? I am 
> also interested in this topic.

The first change I'd make is to set 'buildthreads' in BuildBIOS to 
%NUMBER_OF_PROCESSORS% - that by itself reduces the build time from around 7 
minutes to 2.5 on my system. I also have some changes to the ReadMe.MD file to 
convert it to be a MD file instead of plain text, which massively increases 
readability on GitHub.  I've also been wondering about allowing more parameters 
to be passed through to 'build' 
such as reducing the verbosity to make compiler warnings more apparent. 
Finally, and this is likely more controversial - I've been wondering if the 
build scripts like BuildBIOS should be in python instead of Windows .cmd/.bat 
since python is already used elsewhere and is more flexible.

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

Re: [edk2] [Patch] BaseTools: Fix the regression issue caused by commit dc4c77

2017-02-22 Thread Ard Biesheuvel

On 22 February 2017 at 16:03, Yonghong Zhu  wrote:
> In the last commit dc4c77, the _GetHeaderInfo will be called more than
> once, which cause the self._ConstructorList.append(Value) append some
> duplicate value.
>
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 

Works for me, thanks

Tested-by: Ard Biesheuvel 

> ---
>  BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
> b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> index 0686721..c1af5c7 100644
> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> @@ -1828,12 +1828,10 @@ class InfBuildData(ModuleBuildClassObject):
>  self.__Macros = {}
>  # EDK_GLOBAL defined macros can be applied to EDK module
>  if self.AutoGenVersion < 0x00010005:
>  self.__Macros.update(GlobalData.gEdkGlobal)
>  self.__Macros.update(GlobalData.gGlobalDefines)
> -else:
> -self.__Macros.update(self.Defines)
>  return self.__Macros
>
>  ## Get architecture
>  def _GetArch(self):
>  return self._Arch
> @@ -1894,10 +1892,11 @@ class InfBuildData(ModuleBuildClassObject):
>  if Name in self:
>  self[Name] = Value
>  if self._Defs == None:
>  self._Defs = sdict()
>  self._Defs[Name] = Value
> +self._Macros[Name] = Value
>  # some special items in [Defines] section need special treatment
>  elif Name in ('EFI_SPECIFICATION_VERSION', 
> 'UEFI_SPECIFICATION_VERSION', 'EDK_RELEASE_VERSION', 
> 'PI_SPECIFICATION_VERSION'):
>  if Name in ('EFI_SPECIFICATION_VERSION', 
> 'UEFI_SPECIFICATION_VERSION'):
>  Name = 'UEFI_SPECIFICATION_VERSION'
>  if self._Specification == None:
> @@ -1954,10 +1953,11 @@ class InfBuildData(ModuleBuildClassObject):
>  self._CustomMakefile[TokenList[0]] = TokenList[1]
>  else:
>  if self._Defs == None:
>  self._Defs = sdict()
>  self._Defs[Name] = Value
> +self._Macros[Name] = Value
>
>  #
>  # Retrieve information in sections specific to Edk.x modules
>  #
>  if self.AutoGenVersion >= 0x00010005:
> --
> 2.6.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] [Patch] BaseTools: add error check for Macro usage in the INF file

2017-02-22 Thread Zhu, Yonghong

Hi Ard,

I just sent out a patch to fix this issue,thanks.

Best Regards,
Zhu Yonghong


-Original Message-
From: Zhu, Yonghong 
Sent: Wednesday, February 22, 2017 9:39 PM
To: 'Ard Biesheuvel' ; Gao, Liming 
; Laszlo Ersek 
Cc: edk2-devel@lists.01.org; Zhu, Yonghong 
Subject: RE: [edk2] [Patch] BaseTools: add error check for Macro usage in the 
INF file

Hi Ard,

Thanks. I will try to find out the root cause and fix it ASAP.

Best Regards,
Zhu Yonghong


-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Wednesday, February 22, 2017 7:44 PM
To: Gao, Liming ; Laszlo Ersek 
Cc: Zhu, Yonghong ; edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the 
INF file

On 21 February 2017 at 10:03, Gao, Liming  wrote:
> Reviewed-by: Liming Gao 
>

This patch has broken ArmVirtQemu in the most mysterious way: it causes the 
constructor invocations in AutoGen.c to be emitted twice, e.g.,

ProcessLibraryConstructorList (
  IN EFI_HANDLEImageHandle,
  IN EFI_SYSTEM_TABLE  *SystemTable
  )
{
  EFI_STATUS  Status;

  Status = HobLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);
  Status = HobLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);

  Status = FdtPL011SerialPortLibInitialize ();
  ASSERT_EFI_ERROR (Status);
  Status = FdtPL011SerialPortLibInitialize ();
  ASSERT_EFI_ERROR (Status);

  Status = BaseDebugLibSerialPortConstructor ();
  ASSERT_EFI_ERROR (Status);
  Status = BaseDebugLibSerialPortConstructor ();
  ASSERT_EFI_ERROR (Status);


which obviously breaks some assumptions, for instance, some HII registrations 
now hit an ASSERT() because of the duplicate GUID

I have no idea how this happens, or what this patch does in the first place, so 
please investigate

Thanks,
Ard.


> -Original Message-
> From: Zhu, Yonghong
> Sent: Tuesday, February 21, 2017 9:18 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [Patch] BaseTools: add error check for Macro usage in the INF 
> file
>
> Use of MACRO statements in the EDK II INF files is limited to local 
> usage only; global or external macros are not permitted. This patch 
> add the check for not defined macros.
>
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/Workspace/MetaFileParser.py| 9 -
>  BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> index 1a5fdf5..37a7f5d 100644
> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # This file is used to parse meta files  # -# Copyright (c) 2008 - 
> 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights 
> +reserved.
>  # (C) Copyright 2015-2016 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 # 
> which accompanies this distribution.  The full text of the license may 
> be found at  # http://opensource.org/licenses/bsd-license.php
> @@ -349,10 +349,17 @@ class MetaFileParser(object):
>  EdkLogger.error('Parser', FORMAT_INVALID, "No value specified",
>  ExtraData=self._CurrentLine, 
> File=self.MetaFile, Line=self._LineIndex + 1)
>
>  self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in 
> self._ValueList]
>  Name, Value = self._ValueList[1], self._ValueList[2]
> +MacroUsed = GlobalData.gMacroRefPattern.findall(Value)
> +if len(MacroUsed) != 0:
> +for Macro in MacroUsed:
> +if Macro in GlobalData.gGlobalDefines:
> +EdkLogger.error("Parser", FORMAT_INVALID, "Global macro 
> %s is not permitted." % (Macro), ExtraData=self._CurrentLine, 
> File=self.MetaFile, Line=self._LineIndex + 1)
> +else:
> +EdkLogger.error("Parser", FORMAT_INVALID, "%s not 
> + defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, 
> + Line=self._LineIndex + 1)
>  # Sometimes, we need to make differences between EDK and EDK2 modules
>  if Name == 'INF_VERSION':
>  if re.match(r'0[xX][\da-f-A-F]{5,8}', Value):
>  self._Version = int(Value, 0)
>  elif re.match(r'\d+\.\d+', Value):
> diff --git

[edk2] [Patch] BaseTools: Fix the regression issue caused by commit dc4c77

2017-02-22 Thread Yonghong Zhu

In the last commit dc4c77, the _GetHeaderInfo will be called more than
once, which cause the self._ConstructorList.append(Value) append some
duplicate value.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
index 0686721..c1af5c7 100644
--- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
+++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
@@ -1828,12 +1828,10 @@ class InfBuildData(ModuleBuildClassObject):
 self.__Macros = {}
 # EDK_GLOBAL defined macros can be applied to EDK module
 if self.AutoGenVersion < 0x00010005:
 self.__Macros.update(GlobalData.gEdkGlobal)
 self.__Macros.update(GlobalData.gGlobalDefines)
-else:
-self.__Macros.update(self.Defines)
 return self.__Macros
 
 ## Get architecture
 def _GetArch(self):
 return self._Arch
@@ -1894,10 +1892,11 @@ class InfBuildData(ModuleBuildClassObject):
 if Name in self:
 self[Name] = Value
 if self._Defs == None:
 self._Defs = sdict()
 self._Defs[Name] = Value
+self._Macros[Name] = Value
 # some special items in [Defines] section need special treatment
 elif Name in ('EFI_SPECIFICATION_VERSION', 
'UEFI_SPECIFICATION_VERSION', 'EDK_RELEASE_VERSION', 
'PI_SPECIFICATION_VERSION'):
 if Name in ('EFI_SPECIFICATION_VERSION', 
'UEFI_SPECIFICATION_VERSION'):
 Name = 'UEFI_SPECIFICATION_VERSION'
 if self._Specification == None:
@@ -1954,10 +1953,11 @@ class InfBuildData(ModuleBuildClassObject):
 self._CustomMakefile[TokenList[0]] = TokenList[1]
 else:
 if self._Defs == None:
 self._Defs = sdict()
 self._Defs[Name] = Value
+self._Macros[Name] = Value
 
 #
 # Retrieve information in sections specific to Edk.x modules
 #
 if self.AutoGenVersion >= 0x00010005:
-- 
2.6.1.windows.1

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

Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask

2017-02-22 Thread Duran, Leo

Hi Star,

Please see my reply below.

Thanks,
Leo.

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Tuesday, February 21, 2017 7:20 PM
> To: Duran, Leo ; edk2-de...@ml01.01.org
> Cc: Singh, Brijesh ; Tian, Feng
> ; Laszlo Ersek ; Zeng, Star
> 
> Subject: RE: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> Add support for PCD PcdPteMemoryEncryptionAddressOrMask
> 
> Shouldn't
> 
> ((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex]
> & PageFaultContext->PhyMask) == Address)
> 
> be
> 
> (((*PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex] & ~(AddressSetMask &
> PAGING_4K_ADDRESS_MASK_64)) & PageFaultContext->PhyMask) ==
> Address)
> 
> like you did at other place?
[Duran, Leo] 
Yes, I agree... I will take care of that in v4 of the set.

> 
> Thanks,
> Star
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Duran, Leo
> Sent: Wednesday, February 22, 2017 12:43 AM
> To: Zeng, Star ; edk2-de...@ml01.01.org
> Cc: Singh, Brijesh ; Tian, Feng
> ; Laszlo Ersek 
> Subject: Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> Add support for PCD PcdPteMemoryEncryptionAddressOrMask
> 
> Hi Star,
> 
> Please double-check the complete [PATCH v3 2/4].
> 
> Yes, there is a non-functional change where I did break a 'very long' line 
> into
> 2 lines as you noted (I can put that back as it was before if so required).
> However the intended functional changes are applied in the rest of the patch
> in lines where I reference 'AddressSetMask'.
> 
> As for [PATCH v3 4/4]
> The intended functional changes are applied... please confirm, or please let
> me know what seems to be missing.
> 
> Thanks,
> Leo.
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Monday, February 20, 2017 12:05 AM
> > To: Duran, Leo ; edk2-de...@ml01.01.org
> > Cc: Laszlo Ersek ; Feng Tian ;
> > Singh, Brijesh ; star.z...@intel.com
> > Subject: Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> > Add support for PCD PcdPteMemoryEncryptionAddressOrMask
> >
> > Leo,
> >
> > Comments added inline.
> >
> > On 2017/2/17 5:02, Leo Duran wrote:
> > > This PCD holds the address mask for page table entries when memory
> > > encryption is enabled on AMD processors supporting the Secure
> > > Encrypted Virtualization (SEV) feature.
> > >
> > > The mask is applied when 4GB tables are created (UefiCapsule.c), and
> > > when the tables are expanded on-demand by page-faults above 4GB's
> > (X64Entry.c).
> > >
> > > Cc: Feng Tian 
> > > Cc: Star Zeng 
> > > Cc: Laszlo Ersek 
> > > Cc: Brijesh Singh 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Leo Duran 
> > > Reviewed-by: Star Zeng 
> > > ---
> > >  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf   |  2 ++
> > >  .../Universal/CapsulePei/Common/CommonHeader.h |  7 +++
> > >  MdeModulePkg/Universal/CapsulePei/UefiCapsule.c| 13 ---
> -
> > >  MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c   | 23
> > +++---
> > >  4 files changed, 34 insertions(+), 11 deletions(-)
> > >
> >
> > [snipped]
> >
> > > diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > index 5ad95d2..2197502 100644
> > > --- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > +++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > @@ -2,6 +2,8 @@
> > >The X64 entrypoint is used to process capsule in long mode.
> > >
> > >  Copyright (c) 2011 - 2016, Intel Corporation. All rights
> > > reserved.
> > > +Copyright (c) 2017, AMD Incorporated. All rights reserved.
> > > +
> > >  This program and the accompanying materials  are licensed and made
> > > available under the terms and conditions of the BSD License  which
> > > accompanies this distribution.  The full text of the license may be
> > > found at @@ -29,6 +31,7 @@ typedef struct _PAGE_FAULT_CONTEXT {
> > >UINT64PhyMask;
> > >UINTN PageFaultBuffer;
> > >UINTN PageFaultIndex;
> > > +  UINT64PteMemoryEncryptionAddressOrMask;
> > >//
> > >// Store the uplink information for each page being used.
> > >//
> > > @@ -114,6 +117,7 @@ AcquirePage (
> > >)
> > >  {
> > >UINTN Address;
> > > +  UINT64AddressSetMask;
> > >
> > >Address = PageFaultContext->PageFaultBuffer + EFI_PAGES_TO_SIZE
> >

Re: [edk2] [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5

2017-02-22 Thread Gao, Liming

Jordan:
  If enable -mabi=ms, EFIAPI and ms_var_list will not be required to be 
defined, right? Do you verify it on X64 arch?

Thanks
Liming
>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Wednesday, February 22, 2017 8:00 AM
>To: Justen, Jordan L ; Anthony PERARD
>; Rebecca Cran 
>Cc: Gao, Liming ; Zhu, Yonghong
>; Ard Biesheuvel ;
>edk2-de...@ml01.01.org; Konrad Rzeszutek Wilk 
>Subject: Re: [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with
>GCC5
>
>On 02/21/17 23:45, Jordan Justen wrote:
>> On 2017-02-21 11:08:24, Rebecca Cran wrote:
>>> On 2/21/2017 12:02 PM, Laszlo Ersek wrote:
>>>
 But in this case, the full edk2 codebase has to be grepped for
 VA_LIST-taking functions, and all of them must be flipped to EFIAPI, if
 they currently aren't EFIAPI. Covering just XenStoreVSPrint() seems
 incomplete. (Note: CryptoPkg/Library/OpensslLib is an exception.)
>>>
>>> I think this was discussed previously but I can't remember: is there a
>>> reason for not just compiling everything with -mabi=ms ?
>>>
>>
>> Originally GCC didn't support -mabi=ms. Once it gained support, it
>> then produced larger executables. Nowadays (and for quite some time),
>> I think it generally results in smaller executables.
>>
>> A benefit of not using -mabi=ms is that we are able to catch some
>> cases of misused EFIAPI
>
>I agree.
>
>> with a compiler warning, or unfortunately in
>> some cases with crashes or misbehaving code.
>>
>> I think the benefit of helping keep EFIAPI clean means that we should
>> continue to not use -mabi=ms for DEBUG builds.
>
>I agree (also for NOOPT builds).
>
>> But, I think it would
>> be good to get the size advantages of -mabi=ms by enabling it for
>> RELEASE builds.
>
>That sounds useful too (even though it wouldn't make the current problem go
>away).
>
>As one caveat, I believe -mabi=ms wouldn't be allowed for building OpensslLib
>even for RELEASE. See "-DNO_MSABI_VA_FUNCS" in "OpensslLib.inf":
>
>commit b2dc04a87fab89307240dc0f30b9a23bb5726c81
>Author: Ard Biesheuvel 
>Date:   Sun Jul 17 11:57:45 2016 +0200
>
>CryptoPkg: set new define to avoid MS ABI VA_LIST on GCC/X64
>
>Set the #define NO_MSABI_VA_FUNCS that will be introduced in a
>subsequent
>patch to avoid the use of the MS ABI in variadic functions. In EDK2, such
>functions normally require the EFIAPI modifier to be used, but for external
>libraries such as OpenSSL, which lack these annotations, it is easier to
>simply revert to the default SysV style VA_LIST ABI.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ard Biesheuvel 
>Reviewed-by: Jordan Justen 
>Tested-by: Laszlo Ersek 
>Tested-By: Liming Gao 
>Reviewed-by: Liming Gao 
>
>Thanks!
>Laszlo

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

Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file

2017-02-22 Thread Zhu, Yonghong

Hi Ard,

Thanks. I will try to find out the root cause and fix it ASAP.

Best Regards,
Zhu Yonghong


-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Wednesday, February 22, 2017 7:44 PM
To: Gao, Liming ; Laszlo Ersek 
Cc: Zhu, Yonghong ; edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the 
INF file

On 21 February 2017 at 10:03, Gao, Liming  wrote:
> Reviewed-by: Liming Gao 
>

This patch has broken ArmVirtQemu in the most mysterious way: it causes the 
constructor invocations in AutoGen.c to be emitted twice, e.g.,

ProcessLibraryConstructorList (
  IN EFI_HANDLEImageHandle,
  IN EFI_SYSTEM_TABLE  *SystemTable
  )
{
  EFI_STATUS  Status;

  Status = HobLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);
  Status = HobLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);

  Status = FdtPL011SerialPortLibInitialize ();
  ASSERT_EFI_ERROR (Status);
  Status = FdtPL011SerialPortLibInitialize ();
  ASSERT_EFI_ERROR (Status);

  Status = BaseDebugLibSerialPortConstructor ();
  ASSERT_EFI_ERROR (Status);
  Status = BaseDebugLibSerialPortConstructor ();
  ASSERT_EFI_ERROR (Status);


which obviously breaks some assumptions, for instance, some HII registrations 
now hit an ASSERT() because of the duplicate GUID

I have no idea how this happens, or what this patch does in the first place, so 
please investigate

Thanks,
Ard.


> -Original Message-
> From: Zhu, Yonghong
> Sent: Tuesday, February 21, 2017 9:18 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [Patch] BaseTools: add error check for Macro usage in the INF 
> file
>
> Use of MACRO statements in the EDK II INF files is limited to local 
> usage only; global or external macros are not permitted. This patch 
> add the check for not defined macros.
>
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/Workspace/MetaFileParser.py| 9 -
>  BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> index 1a5fdf5..37a7f5d 100644
> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # This file is used to parse meta files  # -# Copyright (c) 2008 - 
> 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights 
> +reserved.
>  # (C) Copyright 2015-2016 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  
> # which accompanies this distribution.  The full text of the license 
> may be found at  # http://opensource.org/licenses/bsd-license.php
> @@ -349,10 +349,17 @@ class MetaFileParser(object):
>  EdkLogger.error('Parser', FORMAT_INVALID, "No value specified",
>  ExtraData=self._CurrentLine, 
> File=self.MetaFile, Line=self._LineIndex + 1)
>
>  self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in 
> self._ValueList]
>  Name, Value = self._ValueList[1], self._ValueList[2]
> +MacroUsed = GlobalData.gMacroRefPattern.findall(Value)
> +if len(MacroUsed) != 0:
> +for Macro in MacroUsed:
> +if Macro in GlobalData.gGlobalDefines:
> +EdkLogger.error("Parser", FORMAT_INVALID, "Global macro 
> %s is not permitted." % (Macro), ExtraData=self._CurrentLine, 
> File=self.MetaFile, Line=self._LineIndex + 1)
> +else:
> +EdkLogger.error("Parser", FORMAT_INVALID, "%s not 
> + defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, 
> + Line=self._LineIndex + 1)
>  # Sometimes, we need to make differences between EDK and EDK2 modules
>  if Name == 'INF_VERSION':
>  if re.match(r'0[xX][\da-f-A-F]{5,8}', Value):
>  self._Version = int(Value, 0)
>  elif re.match(r'\d+\.\d+', Value):
> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
> b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> index e7bc87d..0686721 100644
> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # This file is used to create a database used by build tool  # -# 
> Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2017, Intel

Re: [edk2] [PATCH 0/4] AARCH64: enable stack alignment check

2017-02-22 Thread Ard Biesheuvel

On 22 February 2017 at 09:38, Ard Biesheuvel  wrote:
> This series enables the stack alignment check (SA) bit in the MMU for
> AArch64 platforms, as mandated by the UEFI spec. No fixes were required
> to make the existing asm code adhere to this requirement, but some issues
> were spotted in review nonetheless, so these are fixed as well.
>

All pushed, thanks.

> Ard Biesheuvel (4):
>   ArmPlatformPkg/ArmPlatformStackLib: use callee preserved registers
>   ArmPkg/ArmLib: AARCH64: set frame pointer in cache maintenance routine
>   ArmPkg/ArmLib: AARCH64: allow the stack aligment (SA) bit to be
> managed
>   ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking
>
>  ArmPkg/Include/Chipset/AArch64.h | 
> 12 ++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S   | 
> 38 -
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |  
> 1 +
>  ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S | 
> 43 +---
>  4 files changed, 68 insertions(+), 26 deletions(-)
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/4] ArmPkg/ArmLib: AARCH64: set frame pointer in cache maintenance routine

2017-02-22 Thread Leif Lindholm

On Wed, Feb 22, 2017 at 12:56:04PM +, Ard Biesheuvel wrote:
> On 22 February 2017 at 09:38, Ard Biesheuvel  
> wrote:
> > Stack and unstack the frame pointer according to the AAPCS in
> > AArch64AllDataCachesOperation ().
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
> > b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > index 5cee7c1519c3..c35c05fdf681 100644
> > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > @@ -273,7 +273,7 @@ ASM_FUNC(ArmDisableBranchPrediction)
> >  ASM_FUNC(AArch64AllDataCachesOperation)
> >  // We can use regs 0-7 and 9-15 without having to save/restore.
> >  // Save our link register on the stack. - The stack must always be 
> > quad-word aligned
> > -  str   x30, [sp, #-16]!
> > +  stp   x29, x30, [sp, #-16]!
> 
> As discussed over IRC, this needs
> 
> mov x29, sp
> 
> appended as well.
> 
> This ensures that this function will show up in a backtrace when an
> exception is taken (and not handled) in the callback invoked by this
> function. Without setting (and preserving) x29, it will still point to
> the caller of AArch64AllDataCachesOperation()

Sounds good.
With that addition:
Reviewed-by: Leif Lindholm 

> >mov   x1, x0  // Save Function call in x1
> >mrs   x6, clidr_el1   // Read EL1 CLIDR
> >and   x3, x6, #0x700  // Mask out all but Level of Coherency 
> > (LoC)
> > @@ -324,7 +324,7 @@ L_Skip:
> >  L_Finished:
> >dsb   sy
> >isb
> > -  ldr   x30, [sp], #0x10
> > +  ldp   x29, x30, [sp], #0x10
> >ret
> >
> >
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/4] ArmPlatformPkg/ArmPlatformStackLib: use callee preserved registers

2017-02-22 Thread Leif Lindholm

On Wed, Feb 22, 2017 at 12:52:10PM +, Ard Biesheuvel wrote:
> On 22 February 2017 at 12:06, Leif Lindholm  wrote:
> > On Wed, Feb 22, 2017 at 09:38:18AM +, Ard Biesheuvel wrote:
> >> The entry code in ArmPlatformStackSet () is a 1:1 transliteration of the
> >> ARM version, which uses the callee preserved registers r3 - r7 (*) to
> >> preserve the function arguments and the link register across a call to
> >> ArmPlatformIsPrimaryCore ().
> >>
> >> However, x4 - x7 are not callee preserved on AARCH64, and so we should use
> >> registers >= x18 instead.
> >
> > Err ... >= x19 really, no?
> > I mean, I guess it technically does not matter here, but the PCS
> > explicitly recommends against manual use of x18.
> >
> 
> It recommends against it on portable code, because it is up to the
> OS/execution environment to define the semantics of x18. Since the
> UEFI spec does not mention x18 at all (except for the definition of
> EFI_SYSTEM_CONTEXT_AARCH64), it indeed makes sense to avoid it.
> 
> >> While we're at it, drop an unnecessary preserve
> >> of the link register, and simplify/deobfuscate the calculation of the
> >> secondary stack position.
> >>
> >> (*) On ARM, r3 is not callee preserved either, but this should be addressed
> >> in a separate patch.
> >
> > Agreed, but doesn't need to go into the final commit message?
> >
> 
> Nope, but stating that r3 - r7 are callee save on ARM without
> mentioning that this is a mistake feels wrong as well.

Fair enough.
You can drop that one and use the reviewed-by.

> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>  ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S 
> >> | 43 +---
> >>  1 file changed, 19 insertions(+), 24 deletions(-)
> >>
> >> diff --git 
> >> a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S 
> >> b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> >> index 65d7d6c6d686..e219d53cb71d 100644
> >> --- 
> >> a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> >> +++ 
> >> b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> >> @@ -22,13 +22,13 @@
> >>  //  );
> >>  ASM_FUNC(ArmPlatformStackSet)
> >>// Save parameters
> >> -  mov   x6, x3
> >> -  mov   x5, x2
> >> -  mov   x4, x1
> >> -  mov   x3, x0
> >> +  mov   x26, x3
> >> +  mov   x25, x2
> >> +  mov   x24, x1
> >> +  mov   x23, x0
> >
> > Hah, I was going to bikeshed about why you weren't starting from x19,
> > then realised you're just prepending the 2 and avoiding typos.
> > I approve.
> >
> > If you fold in my comments on commit message:
> > Reviewed-by: Leif Lindholm 
> >
> >>
> >>// Save the Link register
> >> -  mov   x7, x30
> >> +  mov   x27, x30
> >>
> >>// Identify Stack
> >>mov   x0, x1
> >> @@ -36,13 +36,13 @@ ASM_FUNC(ArmPlatformStackSet)
> >>cmp   x0, #1
> >>
> >>// Restore parameters
> >> -  mov   x0, x3
> >> -  mov   x1, x4
> >> -  mov   x2, x5
> >> -  mov   x3, x6
> >> +  mov   x0, x23
> >> +  mov   x1, x24
> >> +  mov   x2, x25
> >> +  mov   x3, x26
> >>
> >>// Restore the Link register
> >> -  mov   x30, x7
> >> +  mov   x30, x27
> >>
> >>b.ne  0f
> >>
> >> @@ -57,10 +57,7 @@ ASM_FUNC(ArmPlatformStackSet)
> >>  //  IN UINTN SecondaryStackSize
> >>  //  );
> >>  ASM_FUNC(ArmPlatformStackSetPrimary)
> >> -  // Save the Link register
> >> -  mov   x4, x30
> >> -
> >> -  // Add stack of primary stack to StackBase
> >> +  // Add size of primary stack to StackBase
> >>add   x0, x0, x2
> >>
> >>// Compute SecondaryCoresCount * SecondaryCoreStackSize
> >> @@ -70,7 +67,7 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
> >>// Set Primary Stack ((StackBase + PrimaryStackSize) + 
> >> (SecondaryCoresCount * SecondaryCoreStackSize))
> >>add   sp, x0, x3
> >>
> >> -  brx4
> >> +  ret
> >>
> >>  //VOID
> >>  //ArmPlatformStackSetSecondary (
> >> @@ -81,30 +78,28 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
> >>  //  );
> >>  ASM_FUNC(ArmPlatformStackSetSecondary)
> >>// Save the Link register
> >> -  mov   x4, x30
> >> +  mov   x24, x30
> >>mov   sp, x0
> >>
> >>// Get Core Position
> >>mov   x0, x1
> >>blASM_PFX(ArmPlatformGetCorePosition)
> >> -  mov   x5, x0
> >> +  mov   x25, x0
> >>
> >>// Get Primary Core Position
> >>blASM_PFX(ArmPlatformGetPrimaryCoreMpId)
> >>blASM_PFX(ArmPlatformGetCorePosition)
> >>
> >>// Get Secondary Core Position. We should get consecutive secondary 
> >> stack number from 1...(CoreCount-1)
> >> -  cmp   x5, x0
> >> -  b.ls  1f
> >> +  cmp   x25, x0
> >> +
> >>// Decrement the position if after the primary core
> >> -  sub   x5, x5, #1
> >> -1:
> >> -  add   x5, x5, #1
> >> +  cinc  x25, x25, ls
> >>
> >>// Compute top of the secondary stack
> >> -  mul   x3, x3, x5
> >> +  mul

Re: [edk2] [PATCH 2/4] ArmPkg/ArmLib: AARCH64: set frame pointer in cache maintenance routine

2017-02-22 Thread Ard Biesheuvel

On 22 February 2017 at 09:38, Ard Biesheuvel  wrote:
> Stack and unstack the frame pointer according to the AAPCS in
> AArch64AllDataCachesOperation ().
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> index 5cee7c1519c3..c35c05fdf681 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> @@ -273,7 +273,7 @@ ASM_FUNC(ArmDisableBranchPrediction)
>  ASM_FUNC(AArch64AllDataCachesOperation)
>  // We can use regs 0-7 and 9-15 without having to save/restore.
>  // Save our link register on the stack. - The stack must always be quad-word 
> aligned
> -  str   x30, [sp, #-16]!
> +  stp   x29, x30, [sp, #-16]!

As discussed over IRC, this needs

mov x29, sp

appended as well.

This ensures that this function will show up in a backtrace when an
exception is taken (and not handled) in the callback invoked by this
function. Without setting (and preserving) x29, it will still point to
the caller of AArch64AllDataCachesOperation()

>mov   x1, x0  // Save Function call in x1
>mrs   x6, clidr_el1   // Read EL1 CLIDR
>and   x3, x6, #0x700  // Mask out all but Level of Coherency (LoC)
> @@ -324,7 +324,7 @@ L_Skip:
>  L_Finished:
>dsb   sy
>isb
> -  ldr   x30, [sp], #0x10
> +  ldp   x29, x30, [sp], #0x10
>ret
>
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/4] ArmPlatformPkg/ArmPlatformStackLib: use callee preserved registers

2017-02-22 Thread Ard Biesheuvel

On 22 February 2017 at 12:06, Leif Lindholm  wrote:
> On Wed, Feb 22, 2017 at 09:38:18AM +, Ard Biesheuvel wrote:
>> The entry code in ArmPlatformStackSet () is a 1:1 transliteration of the
>> ARM version, which uses the callee preserved registers r3 - r7 (*) to
>> preserve the function arguments and the link register across a call to
>> ArmPlatformIsPrimaryCore ().
>>
>> However, x4 - x7 are not callee preserved on AARCH64, and so we should use
>> registers >= x18 instead.
>
> Err ... >= x19 really, no?
> I mean, I guess it technically does not matter here, but the PCS
> explicitly recommends against manual use of x18.
>

It recommends against it on portable code, because it is up to the
OS/execution environment to define the semantics of x18. Since the
UEFI spec does not mention x18 at all (except for the definition of
EFI_SYSTEM_CONTEXT_AARCH64), it indeed makes sense to avoid it.

>> While we're at it, drop an unnecessary preserve
>> of the link register, and simplify/deobfuscate the calculation of the
>> secondary stack position.
>>
>> (*) On ARM, r3 is not callee preserved either, but this should be addressed
>> in a separate patch.
>
> Agreed, but doesn't need to go into the final commit message?
>

Nope, but stating that r3 - r7 are callee save on ARM without
mentioning that this is a mistake feels wrong as well.

>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S | 
>> 43 +---
>>  1 file changed, 19 insertions(+), 24 deletions(-)
>>
>> diff --git 
>> a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S 
>> b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
>> index 65d7d6c6d686..e219d53cb71d 100644
>> --- 
>> a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
>> +++ 
>> b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
>> @@ -22,13 +22,13 @@
>>  //  );
>>  ASM_FUNC(ArmPlatformStackSet)
>>// Save parameters
>> -  mov   x6, x3
>> -  mov   x5, x2
>> -  mov   x4, x1
>> -  mov   x3, x0
>> +  mov   x26, x3
>> +  mov   x25, x2
>> +  mov   x24, x1
>> +  mov   x23, x0
>
> Hah, I was going to bikeshed about why you weren't starting from x19,
> then realised you're just prepending the 2 and avoiding typos.
> I approve.
>
> If you fold in my comments on commit message:
> Reviewed-by: Leif Lindholm 
>
>>
>>// Save the Link register
>> -  mov   x7, x30
>> +  mov   x27, x30
>>
>>// Identify Stack
>>mov   x0, x1
>> @@ -36,13 +36,13 @@ ASM_FUNC(ArmPlatformStackSet)
>>cmp   x0, #1
>>
>>// Restore parameters
>> -  mov   x0, x3
>> -  mov   x1, x4
>> -  mov   x2, x5
>> -  mov   x3, x6
>> +  mov   x0, x23
>> +  mov   x1, x24
>> +  mov   x2, x25
>> +  mov   x3, x26
>>
>>// Restore the Link register
>> -  mov   x30, x7
>> +  mov   x30, x27
>>
>>b.ne  0f
>>
>> @@ -57,10 +57,7 @@ ASM_FUNC(ArmPlatformStackSet)
>>  //  IN UINTN SecondaryStackSize
>>  //  );
>>  ASM_FUNC(ArmPlatformStackSetPrimary)
>> -  // Save the Link register
>> -  mov   x4, x30
>> -
>> -  // Add stack of primary stack to StackBase
>> +  // Add size of primary stack to StackBase
>>add   x0, x0, x2
>>
>>// Compute SecondaryCoresCount * SecondaryCoreStackSize
>> @@ -70,7 +67,7 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
>>// Set Primary Stack ((StackBase + PrimaryStackSize) + 
>> (SecondaryCoresCount * SecondaryCoreStackSize))
>>add   sp, x0, x3
>>
>> -  brx4
>> +  ret
>>
>>  //VOID
>>  //ArmPlatformStackSetSecondary (
>> @@ -81,30 +78,28 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
>>  //  );
>>  ASM_FUNC(ArmPlatformStackSetSecondary)
>>// Save the Link register
>> -  mov   x4, x30
>> +  mov   x24, x30
>>mov   sp, x0
>>
>>// Get Core Position
>>mov   x0, x1
>>blASM_PFX(ArmPlatformGetCorePosition)
>> -  mov   x5, x0
>> +  mov   x25, x0
>>
>>// Get Primary Core Position
>>blASM_PFX(ArmPlatformGetPrimaryCoreMpId)
>>blASM_PFX(ArmPlatformGetCorePosition)
>>
>>// Get Secondary Core Position. We should get consecutive secondary stack 
>> number from 1...(CoreCount-1)
>> -  cmp   x5, x0
>> -  b.ls  1f
>> +  cmp   x25, x0
>> +
>>// Decrement the position if after the primary core
>> -  sub   x5, x5, #1
>> -1:
>> -  add   x5, x5, #1
>> +  cinc  x25, x25, ls
>>
>>// Compute top of the secondary stack
>> -  mul   x3, x3, x5
>> +  mul   x3, x3, x25
>>
>>// Set stack
>>add   sp, sp, x3
>>
>> -  brx4
>> +  ret   x24
>> --
>> 2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 4/4] ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking

2017-02-22 Thread Leif Lindholm

On Wed, Feb 22, 2017 at 09:38:21AM +, Ard Biesheuvel wrote:
> Enable the hardware stack alignment check, as mandated by the UEFI spec.
> This ensures that the stack pointer is 16 byte aligned at each instance
> where it is used as the base address in a load/store operation.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 9e0593ce598b..2f8f99d44a31 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -734,6 +734,7 @@ ArmConfigureMmu (
>MAIR_ATTR(TT_ATTR_INDX_MEMORY_WRITE_BACK, 
> MAIR_ATTR_NORMAL_MEMORY_WRITE_BACK));   // mapped to EFI_MEMORY_WB
>  
>ArmDisableAlignmentCheck ();
> +  ArmEnableStackAlignmentCheck ();
>ArmEnableInstructionCache ();
>ArmEnableDataCache ();
>  
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/4] ArmPkg/ArmLib: AARCH64: allow the stack aligment (SA) bit to be managed

2017-02-22 Thread Leif Lindholm

On Wed, Feb 22, 2017 at 09:38:20AM +, Ard Biesheuvel wrote:
> In preparation of enabling stack alignment checking, which is mandated
> by the UEFI spec for AARCH64, add the code to manage this bit to ArmLib.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Include/Chipset/AArch64.h   | 12 +++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 34 
>  2 files changed, 46 insertions(+)
> 
> diff --git a/ArmPkg/Include/Chipset/AArch64.h 
> b/ArmPkg/Include/Chipset/AArch64.h
> index 9aecb1df81e0..cebfc5da426a 100644
> --- a/ArmPkg/Include/Chipset/AArch64.h
> +++ b/ArmPkg/Include/Chipset/AArch64.h
> @@ -194,6 +194,18 @@ ArmEnableAlignmentCheck (
>  
>  VOID
>  EFIAPI
> +ArmDisableStackAlignmentCheck (
> +  VOID
> +  );
> +
> +VOID
> +EFIAPI
> +ArmEnableStackAlignmentCheck (
> +  VOID
> +  );
> +
> +VOID
> +EFIAPI
>  ArmDisableAllExceptions (
>VOID
>);
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> index c35c05fdf681..e53b5fdc5986 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> @@ -20,6 +20,7 @@
>  .set CTRL_M_BIT,  (1 << 0)
>  .set CTRL_A_BIT,  (1 << 1)
>  .set CTRL_C_BIT,  (1 << 2)
> +.set CTRL_SA_BIT, (1 << 3)
>  .set CTRL_I_BIT,  (1 << 12)
>  .set CTRL_V_BIT,  (1 << 12)
>  .set CPACR_VFP_BITS,  (3 << 20)
> @@ -259,6 +260,39 @@ ASM_FUNC(ArmDisableAlignmentCheck)
> isb
> ret
>  
> +ASM_FUNC(ArmEnableStackAlignmentCheck)
> +   EL1_OR_EL2(x1)
> +1: mrs x0, sctlr_el1// Get control register EL1
> +   b   3f
> +2: mrs x0, sctlr_el2// Get control register EL2
> +3: orr x0, x0, #CTRL_SA_BIT // Set SA (stack alignment check) bit
> +   EL1_OR_EL2(x1)
> +1: msr sctlr_el1, x0// Write back control register
> +   b   3f
> +2: msr sctlr_el2, x0// Write back control register
> +3: dsb sy
> +   isb
> +   ret
> +
> +
> +ASM_FUNC(ArmDisableStackAlignmentCheck)
> +   EL1_OR_EL2_OR_EL3(x1)
> +1: mrs x0, sctlr_el1// Get control register EL1
> +   b   4f
> +2: mrs x0, sctlr_el2// Get control register EL2
> +   b   4f
> +3: mrs x0, sctlr_el3// Get control register EL3
> +4: bic x0, x0, #CTRL_SA_BIT // Clear SA (stack alignment check) bit
> +   EL1_OR_EL2_OR_EL3(x1)
> +1: msr sctlr_el1, x0// Write back control register
> +   b   4f
> +2: msr sctlr_el2, x0// Write back control register
> +   b   4f
> +3: msr sctlr_el3, x0// Write back control register
> +4: dsb sy
> +   isb
> +   ret
> +
>  
>  // Always turned on in AArch64. Else implementation specific. Leave in for C 
> compatibility for now
>  ASM_FUNC(ArmEnableBranchPrediction)
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/4] ArmPlatformPkg/ArmPlatformStackLib: use callee preserved registers

2017-02-22 Thread Leif Lindholm

On Wed, Feb 22, 2017 at 09:38:18AM +, Ard Biesheuvel wrote:
> The entry code in ArmPlatformStackSet () is a 1:1 transliteration of the
> ARM version, which uses the callee preserved registers r3 - r7 (*) to
> preserve the function arguments and the link register across a call to
> ArmPlatformIsPrimaryCore ().
> 
> However, x4 - x7 are not callee preserved on AARCH64, and so we should use
> registers >= x18 instead.

Err ... >= x19 really, no?
I mean, I guess it technically does not matter here, but the PCS
explicitly recommends against manual use of x18.

> While we're at it, drop an unnecessary preserve
> of the link register, and simplify/deobfuscate the calculation of the
> secondary stack position.
> 
> (*) On ARM, r3 is not callee preserved either, but this should be addressed
> in a separate patch.

Agreed, but doesn't need to go into the final commit message?

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S | 
> 43 +---
>  1 file changed, 19 insertions(+), 24 deletions(-)
> 
> diff --git 
> a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S 
> b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> index 65d7d6c6d686..e219d53cb71d 100644
> --- a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> +++ b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> @@ -22,13 +22,13 @@
>  //  );
>  ASM_FUNC(ArmPlatformStackSet)
>// Save parameters
> -  mov   x6, x3
> -  mov   x5, x2
> -  mov   x4, x1
> -  mov   x3, x0
> +  mov   x26, x3
> +  mov   x25, x2
> +  mov   x24, x1
> +  mov   x23, x0

Hah, I was going to bikeshed about why you weren't starting from x19,
then realised you're just prepending the 2 and avoiding typos.
I approve.

If you fold in my comments on commit message:
Reviewed-by: Leif Lindholm 

>  
>// Save the Link register
> -  mov   x7, x30
> +  mov   x27, x30
>  
>// Identify Stack
>mov   x0, x1
> @@ -36,13 +36,13 @@ ASM_FUNC(ArmPlatformStackSet)
>cmp   x0, #1
>  
>// Restore parameters
> -  mov   x0, x3
> -  mov   x1, x4
> -  mov   x2, x5
> -  mov   x3, x6
> +  mov   x0, x23
> +  mov   x1, x24
> +  mov   x2, x25
> +  mov   x3, x26
>  
>// Restore the Link register
> -  mov   x30, x7
> +  mov   x30, x27
>  
>b.ne  0f
>  
> @@ -57,10 +57,7 @@ ASM_FUNC(ArmPlatformStackSet)
>  //  IN UINTN SecondaryStackSize
>  //  );
>  ASM_FUNC(ArmPlatformStackSetPrimary)
> -  // Save the Link register
> -  mov   x4, x30
> -
> -  // Add stack of primary stack to StackBase
> +  // Add size of primary stack to StackBase
>add   x0, x0, x2
>  
>// Compute SecondaryCoresCount * SecondaryCoreStackSize
> @@ -70,7 +67,7 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
>// Set Primary Stack ((StackBase + PrimaryStackSize) + 
> (SecondaryCoresCount * SecondaryCoreStackSize))
>add   sp, x0, x3
>  
> -  brx4
> +  ret
>  
>  //VOID
>  //ArmPlatformStackSetSecondary (
> @@ -81,30 +78,28 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
>  //  );
>  ASM_FUNC(ArmPlatformStackSetSecondary)
>// Save the Link register
> -  mov   x4, x30
> +  mov   x24, x30
>mov   sp, x0
>  
>// Get Core Position
>mov   x0, x1
>blASM_PFX(ArmPlatformGetCorePosition)
> -  mov   x5, x0
> +  mov   x25, x0
>  
>// Get Primary Core Position
>blASM_PFX(ArmPlatformGetPrimaryCoreMpId)
>blASM_PFX(ArmPlatformGetCorePosition)
>  
>// Get Secondary Core Position. We should get consecutive secondary stack 
> number from 1...(CoreCount-1)
> -  cmp   x5, x0
> -  b.ls  1f
> +  cmp   x25, x0
> +
>// Decrement the position if after the primary core
> -  sub   x5, x5, #1
> -1:
> -  add   x5, x5, #1
> +  cinc  x25, x25, ls
>  
>// Compute top of the secondary stack
> -  mul   x3, x3, x5
> +  mul   x3, x3, x25
>  
>// Set stack
>add   sp, sp, x3
>  
> -  brx4
> +  ret   x24
> -- 
> 2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmVirtPkg/ArmVirt.dsc.inc: AARCH64: enable DXE image protection feature

2017-02-22 Thread Ard Biesheuvel

Enable the new DXE image protection for all image, i.e., FV images but
also external images that originate from disk or the network, such as
OS loaders.

This complements work that is underway on the arm64/Linux kernel side,
to emit the OS loader with 4 KB section alignment, and a suitable split
between code and data.

http://marc.info/?l=linux-arm-kernel=14867227819

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index dbd6678accde..c0d5e7c6aa6d 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -17,6 +17,9 @@ [Defines]
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
   DEFINE TTY_TERMINAL= FALSE
 
+[BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
+  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1000
+
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
   GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1
@@ -380,6 +383,13 @@ [PcdsFixedAtBuild.common]
 [PcdsFixedAtBuild.ARM]
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
 
+[PcdsFixedAtBuild.AARCH64]
+  #
+  # Enable strict image permissions for all images. (This applies
+  # only to images that were built with >= 4 KB section alignment.)
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
+
 [Components.common]
   #
   # Networking stack
-- 
2.7.4

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

Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file

2017-02-22 Thread Ard Biesheuvel

On 21 February 2017 at 10:03, Gao, Liming  wrote:
> Reviewed-by: Liming Gao 
>

This patch has broken ArmVirtQemu in the most mysterious way: it
causes the constructor invocations in AutoGen.c to be emitted twice,
e.g.,

ProcessLibraryConstructorList (
  IN EFI_HANDLEImageHandle,
  IN EFI_SYSTEM_TABLE  *SystemTable
  )
{
  EFI_STATUS  Status;

  Status = HobLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);
  Status = HobLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);

  Status = FdtPL011SerialPortLibInitialize ();
  ASSERT_EFI_ERROR (Status);
  Status = FdtPL011SerialPortLibInitialize ();
  ASSERT_EFI_ERROR (Status);

  Status = BaseDebugLibSerialPortConstructor ();
  ASSERT_EFI_ERROR (Status);
  Status = BaseDebugLibSerialPortConstructor ();
  ASSERT_EFI_ERROR (Status);


which obviously breaks some assumptions, for instance, some HII
registrations now hit an ASSERT() because of the duplicate GUID

I have no idea how this happens, or what this patch does in the first
place, so please investigate

Thanks,
Ard.


> -Original Message-
> From: Zhu, Yonghong
> Sent: Tuesday, February 21, 2017 9:18 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [Patch] BaseTools: add error check for Macro usage in the INF file
>
> Use of MACRO statements in the EDK II INF files is limited to local
> usage only; global or external macros are not permitted. This patch
> add the check for not defined macros.
>
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/Workspace/MetaFileParser.py| 9 -
>  BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> index 1a5fdf5..37a7f5d 100644
> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # This file is used to parse meta files
>  #
> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>  # (C) Copyright 2015-2016 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
>  # which accompanies this distribution.  The full text of the license may be 
> found at
>  # http://opensource.org/licenses/bsd-license.php
> @@ -349,10 +349,17 @@ class MetaFileParser(object):
>  EdkLogger.error('Parser', FORMAT_INVALID, "No value specified",
>  ExtraData=self._CurrentLine, File=self.MetaFile, 
> Line=self._LineIndex + 1)
>
>  self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in 
> self._ValueList]
>  Name, Value = self._ValueList[1], self._ValueList[2]
> +MacroUsed = GlobalData.gMacroRefPattern.findall(Value)
> +if len(MacroUsed) != 0:
> +for Macro in MacroUsed:
> +if Macro in GlobalData.gGlobalDefines:
> +EdkLogger.error("Parser", FORMAT_INVALID, "Global macro 
> %s is not permitted." % (Macro), ExtraData=self._CurrentLine, 
> File=self.MetaFile, Line=self._LineIndex + 1)
> +else:
> +EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % 
> (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, 
> Line=self._LineIndex + 1)
>  # Sometimes, we need to make differences between EDK and EDK2 modules
>  if Name == 'INF_VERSION':
>  if re.match(r'0[xX][\da-f-A-F]{5,8}', Value):
>  self._Version = int(Value, 0)
>  elif re.match(r'\d+\.\d+', Value):
> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
> b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> index e7bc87d..0686721 100644
> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # This file is used to create a database used by build tool
>  #
> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>  # (C) Copyright 2016 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
>  # which accompanies this distribution.  The full text of the license may be 
> found at
>  # http://opensource.org/licenses/bsd-license.php
> @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject):
>  self.__Macros

[edk2] [PATCH 3/4] ArmPkg/ArmLib: AARCH64: allow the stack aligment (SA) bit to be managed

2017-02-22 Thread Ard Biesheuvel

In preparation of enabling stack alignment checking, which is mandated
by the UEFI spec for AARCH64, add the code to manage this bit to ArmLib.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Include/Chipset/AArch64.h   | 12 +++
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 34 
 2 files changed, 46 insertions(+)

diff --git a/ArmPkg/Include/Chipset/AArch64.h b/ArmPkg/Include/Chipset/AArch64.h
index 9aecb1df81e0..cebfc5da426a 100644
--- a/ArmPkg/Include/Chipset/AArch64.h
+++ b/ArmPkg/Include/Chipset/AArch64.h
@@ -194,6 +194,18 @@ ArmEnableAlignmentCheck (
 
 VOID
 EFIAPI
+ArmDisableStackAlignmentCheck (
+  VOID
+  );
+
+VOID
+EFIAPI
+ArmEnableStackAlignmentCheck (
+  VOID
+  );
+
+VOID
+EFIAPI
 ArmDisableAllExceptions (
   VOID
   );
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index c35c05fdf681..e53b5fdc5986 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -20,6 +20,7 @@
 .set CTRL_M_BIT,  (1 << 0)
 .set CTRL_A_BIT,  (1 << 1)
 .set CTRL_C_BIT,  (1 << 2)
+.set CTRL_SA_BIT, (1 << 3)
 .set CTRL_I_BIT,  (1 << 12)
 .set CTRL_V_BIT,  (1 << 12)
 .set CPACR_VFP_BITS,  (3 << 20)
@@ -259,6 +260,39 @@ ASM_FUNC(ArmDisableAlignmentCheck)
isb
ret
 
+ASM_FUNC(ArmEnableStackAlignmentCheck)
+   EL1_OR_EL2(x1)
+1: mrs x0, sctlr_el1// Get control register EL1
+   b   3f
+2: mrs x0, sctlr_el2// Get control register EL2
+3: orr x0, x0, #CTRL_SA_BIT // Set SA (stack alignment check) bit
+   EL1_OR_EL2(x1)
+1: msr sctlr_el1, x0// Write back control register
+   b   3f
+2: msr sctlr_el2, x0// Write back control register
+3: dsb sy
+   isb
+   ret
+
+
+ASM_FUNC(ArmDisableStackAlignmentCheck)
+   EL1_OR_EL2_OR_EL3(x1)
+1: mrs x0, sctlr_el1// Get control register EL1
+   b   4f
+2: mrs x0, sctlr_el2// Get control register EL2
+   b   4f
+3: mrs x0, sctlr_el3// Get control register EL3
+4: bic x0, x0, #CTRL_SA_BIT // Clear SA (stack alignment check) bit
+   EL1_OR_EL2_OR_EL3(x1)
+1: msr sctlr_el1, x0// Write back control register
+   b   4f
+2: msr sctlr_el2, x0// Write back control register
+   b   4f
+3: msr sctlr_el3, x0// Write back control register
+4: dsb sy
+   isb
+   ret
+
 
 // Always turned on in AArch64. Else implementation specific. Leave in for C 
compatibility for now
 ASM_FUNC(ArmEnableBranchPrediction)
-- 
2.7.4

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

[edk2] [PATCH 0/4] AARCH64: enable stack alignment check

2017-02-22 Thread Ard Biesheuvel

This series enables the stack alignment check (SA) bit in the MMU for
AArch64 platforms, as mandated by the UEFI spec. No fixes were required
to make the existing asm code adhere to this requirement, but some issues
were spotted in review nonetheless, so these are fixed as well.

Ard Biesheuvel (4):
  ArmPlatformPkg/ArmPlatformStackLib: use callee preserved registers
  ArmPkg/ArmLib: AARCH64: set frame pointer in cache maintenance routine
  ArmPkg/ArmLib: AARCH64: allow the stack aligment (SA) bit to be
managed
  ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking

 ArmPkg/Include/Chipset/AArch64.h | 12 
++
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S   | 38 
-
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |  1 +
 ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S | 43 
+---
 4 files changed, 68 insertions(+), 26 deletions(-)

-- 
2.7.4

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

[edk2] [PATCH 4/4] ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking

2017-02-22 Thread Ard Biesheuvel

Enable the hardware stack alignment check, as mandated by the UEFI spec.
This ensures that the stack pointer is 16 byte aligned at each instance
where it is used as the base address in a load/store operation.

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

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 9e0593ce598b..2f8f99d44a31 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -734,6 +734,7 @@ ArmConfigureMmu (
   MAIR_ATTR(TT_ATTR_INDX_MEMORY_WRITE_BACK, 
MAIR_ATTR_NORMAL_MEMORY_WRITE_BACK));   // mapped to EFI_MEMORY_WB
 
   ArmDisableAlignmentCheck ();
+  ArmEnableStackAlignmentCheck ();
   ArmEnableInstructionCache ();
   ArmEnableDataCache ();
 
-- 
2.7.4

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

[edk2] [PATCH 2/4] ArmPkg/ArmLib: AARCH64: set frame pointer in cache maintenance routine

2017-02-22 Thread Ard Biesheuvel

Stack and unstack the frame pointer according to the AAPCS in
AArch64AllDataCachesOperation ().

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index 5cee7c1519c3..c35c05fdf681 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -273,7 +273,7 @@ ASM_FUNC(ArmDisableBranchPrediction)
 ASM_FUNC(AArch64AllDataCachesOperation)
 // We can use regs 0-7 and 9-15 without having to save/restore.
 // Save our link register on the stack. - The stack must always be quad-word 
aligned
-  str   x30, [sp, #-16]!
+  stp   x29, x30, [sp, #-16]!
   mov   x1, x0  // Save Function call in x1
   mrs   x6, clidr_el1   // Read EL1 CLIDR
   and   x3, x6, #0x700  // Mask out all but Level of Coherency (LoC)
@@ -324,7 +324,7 @@ L_Skip:
 L_Finished:
   dsb   sy
   isb
-  ldr   x30, [sp], #0x10
+  ldp   x29, x30, [sp], #0x10
   ret
 
 
-- 
2.7.4

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

[edk2] [PATCH 1/4] ArmPlatformPkg/ArmPlatformStackLib: use callee preserved registers

2017-02-22 Thread Ard Biesheuvel

The entry code in ArmPlatformStackSet () is a 1:1 transliteration of the
ARM version, which uses the callee preserved registers r3 - r7 (*) to
preserve the function arguments and the link register across a call to
ArmPlatformIsPrimaryCore ().

However, x4 - x7 are not callee preserved on AARCH64, and so we should use
registers >= x18 instead. While we're at it, drop an unnecessary preserve
of the link register, and simplify/deobfuscate the calculation of the
secondary stack position.

(*) On ARM, r3 is not callee preserved either, but this should be addressed
in a separate patch.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S | 43 
+---
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git 
a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S 
b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
index 65d7d6c6d686..e219d53cb71d 100644
--- a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
+++ b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
@@ -22,13 +22,13 @@
 //  );
 ASM_FUNC(ArmPlatformStackSet)
   // Save parameters
-  mov   x6, x3
-  mov   x5, x2
-  mov   x4, x1
-  mov   x3, x0
+  mov   x26, x3
+  mov   x25, x2
+  mov   x24, x1
+  mov   x23, x0
 
   // Save the Link register
-  mov   x7, x30
+  mov   x27, x30
 
   // Identify Stack
   mov   x0, x1
@@ -36,13 +36,13 @@ ASM_FUNC(ArmPlatformStackSet)
   cmp   x0, #1
 
   // Restore parameters
-  mov   x0, x3
-  mov   x1, x4
-  mov   x2, x5
-  mov   x3, x6
+  mov   x0, x23
+  mov   x1, x24
+  mov   x2, x25
+  mov   x3, x26
 
   // Restore the Link register
-  mov   x30, x7
+  mov   x30, x27
 
   b.ne  0f
 
@@ -57,10 +57,7 @@ ASM_FUNC(ArmPlatformStackSet)
 //  IN UINTN SecondaryStackSize
 //  );
 ASM_FUNC(ArmPlatformStackSetPrimary)
-  // Save the Link register
-  mov   x4, x30
-
-  // Add stack of primary stack to StackBase
+  // Add size of primary stack to StackBase
   add   x0, x0, x2
 
   // Compute SecondaryCoresCount * SecondaryCoreStackSize
@@ -70,7 +67,7 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
   // Set Primary Stack ((StackBase + PrimaryStackSize) + (SecondaryCoresCount 
* SecondaryCoreStackSize))
   add   sp, x0, x3
 
-  brx4
+  ret
 
 //VOID
 //ArmPlatformStackSetSecondary (
@@ -81,30 +78,28 @@ ASM_FUNC(ArmPlatformStackSetPrimary)
 //  );
 ASM_FUNC(ArmPlatformStackSetSecondary)
   // Save the Link register
-  mov   x4, x30
+  mov   x24, x30
   mov   sp, x0
 
   // Get Core Position
   mov   x0, x1
   blASM_PFX(ArmPlatformGetCorePosition)
-  mov   x5, x0
+  mov   x25, x0
 
   // Get Primary Core Position
   blASM_PFX(ArmPlatformGetPrimaryCoreMpId)
   blASM_PFX(ArmPlatformGetCorePosition)
 
   // Get Secondary Core Position. We should get consecutive secondary stack 
number from 1...(CoreCount-1)
-  cmp   x5, x0
-  b.ls  1f
+  cmp   x25, x0
+
   // Decrement the position if after the primary core
-  sub   x5, x5, #1
-1:
-  add   x5, x5, #1
+  cinc  x25, x25, ls
 
   // Compute top of the secondary stack
-  mul   x3, x3, x5
+  mul   x3, x3, x25
 
   // Set stack
   add   sp, sp, x3
 
-  brx4
+  ret   x24
-- 
2.7.4

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

[edk2] [PATCH v2] SecurityPkg: Fix potential bug in Security Boot dxe.

2017-02-22 Thread Zhang Lubo

v2: update hash value in SecureBootConfig.vfr to keep
them consistent with macro definition in SecureBootConfigImpl.h

since we removed the sha-1 definition in Hash table
and related macro, but the macro definition HashAlg index
may be value 4 which is exceed the range of the Hash
table array.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Chao Zhang 
Cc: Long Qin 
Cc: Yao Jiewen 
---
 .../SecureBootConfigDxe/SecureBootConfig.vfr | 10 +-
 .../SecureBootConfigDxe/SecureBootConfigImpl.h   | 12 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
index 02ddf4a..6f46d91 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
@@ -457,17 +457,17 @@ formset
 
 oneof name = SignatureFormatInDbx,
   varid   = SECUREBOOT_CONFIGURATION.CertificateFormat,
   prompt  = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),
   help= STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP),
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA256), value 
= 0x2, flags = DEFAULT;
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA384), value 
= 0x3, flags = 0;
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA512), value 
= 0x4, flags = 0;
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW), value = 
0x5, flags = 0;
+  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA256), value 
= 0x1, flags = DEFAULT;
+  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA384), value 
= 0x2, flags = 0;
+  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA512), value 
= 0x3, flags = 0;
+  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW), value = 
0x4, flags = 0;
 endoneof;
 
-suppressif ideqval SECUREBOOT_CONFIGURATION.CertificateFormat == 5;
+suppressif ideqval SECUREBOOT_CONFIGURATION.CertificateFormat == 4;
 checkbox varid  = SECUREBOOT_CONFIGURATION.AlwaysRevocation,
prompt = STRING_TOKEN(STR_ALWAYS_CERTIFICATE_REVOCATION_PROMPT),
help   = STRING_TOKEN(STR_ALWAYS_CERTIFICATE_REVOCATION_HELP),
flags  = INTERACTIVE,
 endcheckbox;
diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
index bea9470..58030c4 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
@@ -89,16 +89,16 @@ extern  EFI_IFR_GUID_LABEL *mEndLabel;
 #define WIN_CERT_UEFI_RSA2048_SIZE   256
 
 //
 // Support hash types
 //
-#define HASHALG_SHA224 0x0001
-#define HASHALG_SHA256 0x0002
-#define HASHALG_SHA384 0x0003
-#define HASHALG_SHA512 0x0004
-#define HASHALG_RAW0x0005
-#define HASHALG_MAX0x0005
+#define HASHALG_SHA224 0x
+#define HASHALG_SHA256 0x0001
+#define HASHALG_SHA384 0x0002
+#define HASHALG_SHA512 0x0003
+#define HASHALG_RAW0x0004
+#define HASHALG_MAX0x0004
 
 
 typedef struct {
   UINTN Signature;
   LIST_ENTRYHead;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5

2017-02-22 Thread Gao, Liming

Laszlo:
  In edk2, I find the several functions with VA_LIST have no EFIAPI. They may 
use VA_ARG() or call other functions, but they don't use VA_COPY(). In Base.h, 
VA_ARG() is defined as __builtin_va_arg(), which is same to native one. 
VA_COPY() is defined as __builtin_ms_va_copy(). So, I also think this is MS ABI 
request. That means only if the function implementation uses 
VA_START(),VA_END() or VA_COPY(), it must be declared with EFIAPI.

MdePkg\Library\BasePrintLib\PrintLibInternal.c BasePrintLibSPrintMarker()
ShellPkg\Library\UefiShellLib\UefiShellLib.c InternalShellPrintWorker()

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Wednesday, February 22, 2017 3:03 AM
>To: Anthony PERARD 
>Cc: Ard Biesheuvel ; Justen, Jordan L
>; edk2-de...@ml01.01.org; Gao, Liming
>
>Subject: Re: [edk2] [PATCH 0/4] Fix runtime issue in XenBusDxe when
>compiled with GCC5
>
>On 02/21/17 18:53, Anthony PERARD wrote:
>> On Tue, Feb 21, 2017 at 06:07:15PM +0100, Laszlo Ersek wrote:
>>> CC Rebecca & Konrad
>>>
>>> On 02/21/17 17:39, Anthony PERARD wrote:
>
>[snip]
>
 So, should I add EFIAPI to XenStoreVSPrint, as it is using VA_COPY?

>>>
>>> Hm, please help me jog my memory...
>>>
>>> If I remember correctly, this is still a GCC bug, one that we suppressed for
>gcc-6.2 with your patch as follows:
>>
>> Yes.
>>
 commit 432f1d83f77acf92d52ef18d2cee6dbf7c5b9b86
 Author: Anthony PERARD 
 Date:   Tue Dec 6 12:03:25 2016 +

 OvmfPkg/build.sh: Use GCC49 toolchains with GCC 6.[0-2]

 The goal of the patch is to avoid using -flto with GCC 6.0 to 6.2.

 This is to workaround a GCC bug:
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Anthony PERARD 
 Reviewed-by: Laszlo Ersek 
 Regression-tested-by: Laszlo Ersek 

 diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
 index 95fe8fb07647..b6e936056ca0 100755
 --- a/OvmfPkg/build.sh
 +++ b/OvmfPkg/build.sh
 @@ -102,7 +102,7 @@ case `uname` in
4.8.*)
  TARGET_TOOLS=GCC48
  ;;
 -  4.9.*)
 +  4.9.*|6.[0-2].*)
  TARGET_TOOLS=GCC49
  ;;
*)
>>>
>>> Do I understand correctly that the gcc bug has not been fixed in
>>> gcc-6.3, and -- because we don't suppress it for gcc-6.3 as the
>>> above expression does not match -- it causes problems again?
>>
>> The bug describe in the GCC bugzilla is probably fix, but the
>> test-case does not make use of __builtin_ms_va_copy.
>
>:/
>
>>
>>> You also mention gcc-5.4 as problematic. I think we haven't
>>> received such reports about gcc-5 versions up to and including
>>> gcc-5.3 (that's why GCC5 is the default selection in
>>> "OvmfPkg/build.sh"). Do you mean that the gcc bug has now been
>>> "backported" from the gcc-6 series to the gcc-5 series (starting
>>> with gcc-5.4)?
>
>>
>> I don't know the state of gcc-5.0 to gcc-5.3, I have never tested -flto
>> with gcc-5.x (until now), I would say they are also problematic until
>> proven otherwise.
>
>When we enabled GCC5, it definitely worked for at least one gcc release,
>with -flto. (-flto is the default for DEBUG and RELEASE builds with
>GCC5; NOOPT disables -Os and -flto.)
>
>>
>>> If that's the case, then I suggest flipping "OvmfPkg/build.sh" from
>>> black-listing gcc versions for -flto to white-listing. In other
>>> words, assume that -flto is generally broken with GCC, except for a
>>> few known versions: 5.0 through 5.3 inclusive. Those versions
>>> should trigger the use of the GCC5 toolchain, and everything else
>>> (5.4+, 6.*, 4.9.*) should use GCC49.
>>>
>>> I don't feel comfortable about adding EFIAPI to XenStoreVSPrint
>>> just because it takes a VA_LIST parameter -- note: it is *not* a
>>> varargs function itself! --; the same issue might hit elsewhere in
>>> the edk2 tree at any time, outside of OvmfPkg too.
>>
>> From the different tests I've done, I feel more like VA_COPY might be
>> the issue, but I don't know how __builtin_ms_va_* are supposed to be
>> used.
>
>If I recall correctly, from the upstream GCC bug, the problem is that
>__builtin_va_list does not track internally whether it was created in an
>msabi or sysvabi function, and therefore the va_* functions cannot be
>used transparently on it. Instead, when va_list is accessed, the
>accessor builtins seem to apply the currently executing function's
>calling convetion to va_list. (Even if the creation context of va_list
>was different.)
>
>>
>>> Would the gcc white-listing work for you?
>>>
>>> Note that the white-listing would practically undo Konrad's commit
>>>

Re: [edk2] [patch] SecurityPkg: Fix potential bug in Security Boot dxe.

2017-02-22 Thread Zhang, Lubo

Thanks , I will update the HASH value definition  in VFR  to keep them 
consistent with macro definition  .

> -Original Message-
> From: Zhang, Chao B
> Sent: Wednesday, February 22, 2017 4:31 PM
> To: Zhang, Lubo ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Long, Qin 
> Subject: RE: [edk2] [patch] SecurityPkg: Fix potential bug in Security Boot 
> dxe.
> 
> Lubo：
> If you change HASH algo definition, please also update 
> SecureBootConfig.vfr
> accordingly.
> Or you may keep SHA1 algo definition & keep RAW & MAX unchanged
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang
> Lubo
> Sent: Tuesday, February 21, 2017 10:53 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zhang, Chao B
> ; Long, Qin 
> Subject: [edk2] [patch] SecurityPkg: Fix potential bug in Security Boot dxe.
> 
> since we removed the sha-1 definition in Hash table and related macro, but the
> macro definition HashAlg index may be value 4 which is exceed the range of the
> Hash table array.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Chao Zhang 
> Cc: Long Qin 
> Cc: Yao Jiewen 
> ---
>  .../SecureBootConfigDxe/SecureBootConfigImpl.h   | 12 
> ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.h
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.h
> index bea9470..58030c4 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.h
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigImpl.h
> @@ -89,16 +89,16 @@ extern  EFI_IFR_GUID_LABEL *mEndLabel;
>  #define WIN_CERT_UEFI_RSA2048_SIZE   256
> 
>  //
>  // Support hash types
>  //
> -#define HASHALG_SHA224 0x0001
> -#define HASHALG_SHA256 0x0002
> -#define HASHALG_SHA384 0x0003
> -#define HASHALG_SHA512 0x0004
> -#define HASHALG_RAW0x0005
> -#define HASHALG_MAX0x0005
> +#define HASHALG_SHA224 0x
> +#define HASHALG_SHA256 0x0001
> +#define HASHALG_SHA384 0x0002
> +#define HASHALG_SHA512 0x0003
> +#define HASHALG_RAW0x0004
> +#define HASHALG_MAX0x0004
> 
> 
>  typedef struct {
>UINTN Signature;
>LIST_ENTRYHead;
> --
> 1.9.5.msysgit.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] SecurityPkg: Fix potential bug in Security Boot dxe.

2017-02-22 Thread Zhang, Chao B

Lubo：
If you change HASH algo definition, please also update SecureBootConfig.vfr 
accordingly.
Or you may keep SHA1 algo definition & keep RAW & MAX unchanged

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang 
Lubo
Sent: Tuesday, February 21, 2017 10:53 AM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen ; Zhang, Chao B ; 
Long, Qin 
Subject: [edk2] [patch] SecurityPkg: Fix potential bug in Security Boot dxe.

since we removed the sha-1 definition in Hash table and related macro, but the 
macro definition HashAlg index may be value 4 which is exceed the range of the 
Hash table array.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Chao Zhang 
Cc: Long Qin 
Cc: Yao Jiewen 
---
 .../SecureBootConfigDxe/SecureBootConfigImpl.h   | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
index bea9470..58030c4 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigImpl.h
@@ -89,16 +89,16 @@ extern  EFI_IFR_GUID_LABEL *mEndLabel;
 #define WIN_CERT_UEFI_RSA2048_SIZE   256
 
 //
 // Support hash types
 //
-#define HASHALG_SHA224 0x0001
-#define HASHALG_SHA256 0x0002
-#define HASHALG_SHA384 0x0003
-#define HASHALG_SHA512 0x0004
-#define HASHALG_RAW0x0005
-#define HASHALG_MAX0x0005
+#define HASHALG_SHA224 0x
+#define HASHALG_SHA256 0x0001
+#define HASHALG_SHA384 0x0002
+#define HASHALG_SHA512 0x0003
+#define HASHALG_RAW0x0004
+#define HASHALG_MAX0x0004
 
 
 typedef struct {
   UINTN Signature;
   LIST_ENTRYHead;
--
1.9.5.msysgit.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] Nt32Pkg: Add build flag to enable or disable IPv6 network stack.

2017-02-22 Thread Ni, Ruiyu

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Wednesday, February 22, 2017 4:15 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: [edk2] [Patch] Nt32Pkg: Add build flag to enable or disable IPv6 
> network
> stack.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> ---
>  Nt32Pkg/Nt32Pkg.dsc | 22 --  Nt32Pkg/Nt32Pkg.fdf |
> 12 +++-
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc index
> 47e37ec..31a60e2 100644
> --- a/Nt32Pkg/Nt32Pkg.dsc
> +++ b/Nt32Pkg/Nt32Pkg.dsc
> @@ -71,6 +71,13 @@
>#
>DEFINE ALLOW_HTTP_CONNECTIONS = TRUE
> 
> +  #
> +  # This flag is to enable or disable IPv6 network stack.
> +  # These can be changed on the command line.
> +  # -D FLAG=VALUE
> +  #
> +  DEFINE NETWORK_IP6_ENABLE = TRUE
> +
> 
> #
> ###
>  #
>  # SKU Identification section - list of all SKU IDs supported by this @@ 
> -135,6
> +142,7 @@
>NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
>IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
>UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> +  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
>DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> 
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/
> OemHookStatusCodeLibNull.inf
> @@ -458,12 +466,22 @@
>MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
>MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
>MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> -  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> -  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>Nt32Pkg/SnpNt32Dxe/SnpNt32Dxe.inf
> 
> +!if $(NETWORK_IP6_ENABLE) == TRUE
> +  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> +  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> +  NetworkPkg/TcpDxe/TcpDxe.inf
> +  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> +  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> +  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> +  NetworkPkg/IScsiDxe/IScsiDxe.inf
> +!else
> +  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> +  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> +!endif
> 
>NetworkPkg/HttpBootDxe/HttpBootDxe.inf
>NetworkPkg/DnsDxe/DnsDxe.inf
> diff --git a/Nt32Pkg/Nt32Pkg.fdf b/Nt32Pkg/Nt32Pkg.fdf index
> bbb4ccc..282e59c 100644
> --- a/Nt32Pkg/Nt32Pkg.fdf
> +++ b/Nt32Pkg/Nt32Pkg.fdf
> @@ -256,11 +256,21 @@ INF
> MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
>  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
>  INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
>  INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> -INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>  INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
>  INF  Nt32Pkg/SnpNt32Dxe/SnpNt32Dxe.inf
> +!if $(NETWORK_IP6_ENABLE) == TRUE
> +INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> +INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> +INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> +INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> +INF  NetworkPkg/TcpDxe/TcpDxe.inf
> +INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> +INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
> +!else
> +INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>  INF  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>  INF  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> +!endif
>  INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
>  INF  NetworkPkg/DnsDxe/DnsDxe.inf
>  INF  NetworkPkg/HttpDxe/HttpDxe.inf
> --
> 2.7.4.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] Nt32Pkg: Add build flag to enable or disable IPv6 network stack.

2017-02-22 Thread Fu Siyuan

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 Nt32Pkg/Nt32Pkg.dsc | 22 --
 Nt32Pkg/Nt32Pkg.fdf | 12 +++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc
index 47e37ec..31a60e2 100644
--- a/Nt32Pkg/Nt32Pkg.dsc
+++ b/Nt32Pkg/Nt32Pkg.dsc
@@ -71,6 +71,13 @@
   #
   DEFINE ALLOW_HTTP_CONNECTIONS = TRUE
 
+  #
+  # This flag is to enable or disable IPv6 network stack.
+  # These can be changed on the command line.
+  # -D FLAG=VALUE
+  #
+  DEFINE NETWORK_IP6_ENABLE = TRUE
+
 

 #
 # SKU Identification section - list of all SKU IDs supported by this
@@ -135,6 +142,7 @@
   NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
   IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
   UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
+  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
   HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
   DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
@@ -458,12 +466,22 @@
   MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
   MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
   MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
   MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
   Nt32Pkg/SnpNt32Dxe/SnpNt32Dxe.inf
 
+!if $(NETWORK_IP6_ENABLE) == TRUE
+  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
+  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
+  NetworkPkg/TcpDxe/TcpDxe.inf
+  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
+  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
+  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
+  NetworkPkg/IScsiDxe/IScsiDxe.inf
+!else
+  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
+  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
   MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
+!endif
 
   NetworkPkg/HttpBootDxe/HttpBootDxe.inf
   NetworkPkg/DnsDxe/DnsDxe.inf
diff --git a/Nt32Pkg/Nt32Pkg.fdf b/Nt32Pkg/Nt32Pkg.fdf
index bbb4ccc..282e59c 100644
--- a/Nt32Pkg/Nt32Pkg.fdf
+++ b/Nt32Pkg/Nt32Pkg.fdf
@@ -256,11 +256,21 @@ INF  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
 INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
 INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
 INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
 INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
 INF  Nt32Pkg/SnpNt32Dxe/SnpNt32Dxe.inf
+!if $(NETWORK_IP6_ENABLE) == TRUE
+INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
+INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
+INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
+INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
+INF  NetworkPkg/TcpDxe/TcpDxe.inf
+INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
+INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
+!else
+INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
 INF  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
 INF  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
+!endif
 INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
 INF  NetworkPkg/DnsDxe/DnsDxe.inf
 INF  NetworkPkg/HttpDxe/HttpDxe.inf
-- 
2.7.4.windows.1

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

Re: [edk2] [PATCH 04/11] MdePkg/BaseLib: Add StrToGuid/StrHexToBytes/StrToIpv[4/6]Address

2017-02-22 Thread Yao, Jiewen

I think we should return better return status code.
If we return all cases with invalid parameter, the caller cannot know what is 
wrong.

Below is my thought:
I would return INVALID_PARAMETER if the something is wrong with the input 
string,
and return UNSUPPORTED if the format is not expected.


StrToIpv6Address()

+  @retval RETURN_INVALID_PARAMETER If String is NULL.

+   If Data is NULL.

+   If PcdMaximumUnicodeStringLength is not

+zero, and String contains more than

+PcdMaximumUnicodeStringLength Unicode

+characters, not including the

+Null-terminator.



+  @retval RETURN_UNSUPPORTED

+   If X contains more than four hexadecimal

+digit characters.

+   If String contains "::" and number of X

+is not less than 8.

+   If P starts with character that is not a

+valid decimal digit character.

+   If the decimal number converted from P

+exceeds 128.

StrToIpv4Address ()

+  @retval RETURN_INVALID_PARAMETER If String is NULL.

+   If Data is NULL.

+   If PcdMaximumUnicodeStringLength is not

+zero, and String contains more than

+PcdMaximumUnicodeStringLength Unicode

+characters, not including the

+Null-terminator.



+  @retval RETURN_UNSUPPORTED

+   If String is not in the correct format.

+   If any decimal number converted from D

+exceeds 255.

+   If the decimal number converted from P

+exceeds 32.

StrToGuid()

+  @retval RETURN_INVALID_PARAMETER If String is NULL.

+   If Data is NULL.

+  @retval RETURN_UNSUPPORTED

+   If String is not as the above format.



From: Ni, Ruiyu
Sent: Wednesday, February 22, 2017 3:58 PM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Cc: Gao, Liming ; Fu, Siyuan 
Subject: RE: [PATCH 04/11] MdePkg/BaseLib: Add 
StrToGuid/StrHexToBytes/StrToIpv[4/6]Address

Jiewen,
If we returns Unsupported for "   1.2.3.4".
What shall we return for "1.2.3"?
If we need to return Invalid Parameter for "   1.2.3", the implementation has 
to check whether it's a valid IP address after spaces, or an invalid IP address.

Can you just return Invalid Parameter for all cases?

Thanks/Ray

> -Original Message-
> From: Yao, Jiewen
> Sent: Wednesday, February 22, 2017 2:02 PM
> To: Ni, Ruiyu >; 
> edk2-devel@lists.01.org
> Cc: Gao, Liming >; Fu, 
> Siyuan >
> Subject: RE: [PATCH 04/11] MdePkg/BaseLib: Add
> StrToGuid/StrHexToBytes/StrToIpv[4/6]Address
>
> Thanks.
>
> I suggest we clearly say: This function does not support the leading pad 
> space ,
> which includes spaces or tab characters, before the first valid char.
>
> For example, "1.2.3.4" is considered as invalid IPv4 address.
> And I suggest we return EFI_UNSUPPORTED.
>
> The comment is applied to add [Ascii]StrToGuid/IpV4/IpV6.
>
> Thank you
> Yao Jiewen
>
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Wednesday, February 22, 2017 12:51 PM
> > To: edk2-devel@lists.01.org
> > Cc: Gao, Liming >; Yao, 
> > Jiewen
> > >; Fu, Siyuan 
> > >
> > Subject: [PATCH 04/11] MdePkg/BaseLib: Add
> > StrToGuid/StrHexToBytes/StrToIpv[4/6]Address
> >
> > The patch adds 4 APIs to convert Unicode string to GUID, bytes buffer,
> > IP v4 address and IP v6 address.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ruiyu Ni >
> > Cc: Liming Gao >
> > Cc: Jiewen Yao >
> > Cc: Siyuan Fu >
> > ---
> >  MdePkg/Include/Library/BaseLib.h| 241 ++
> >  MdePkg/Library/BaseLib/SafeString.c | 623
> > 
> >  2 files changed, 864 insertions(+)

66 matches

Mail list logo