[edk2] [patch 3/3] MdeModulePkg: Avoid key notification called more than once

2018-09-10 Thread dandan bi
From: Dandan Bi 

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

Issue:
In current code logic, when a key is pressed, it will search
the whole NotifyList to find whether a notification has been
registered with the keystroke. if yes, it will en-queue the
key for notification execution later. And now if different
notification functions have been registered with the same key,
then the key will be en-queued more than once. Then it will
cause the notification executed more than once.

This patch is to enhance the code logic to fix this issue.

Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c| 1 +
 MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c   | 1 +
 MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c 
b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
index 2ee293d64d..53cb6f0b48 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
@@ -1449,10 +1449,11 @@ KeyGetchar (
   // while current TPL is TPL_NOTIFY. It will be invoked in
   // KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
   //
   PushEfikeyBufTail (>EfiKeyQueueForNotify, );
   gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent);
+  break;
 }
   }
 
   PushEfikeyBufTail (>EfiKeyQueue, );
 }
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c 
b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index b3b5fb9ff4..9cb4b5db6b 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -1693,10 +1693,11 @@ UsbKeyCodeToEfiInputKey (
   // while current TPL is TPL_NOTIFY. It will be invoked in
   // KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
   //
   Enqueue (>EfiKeyQueueForNotify, KeyData, sizeof 
(*KeyData));
   gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
+  break;
 }
   }
 
   return EFI_SUCCESS;
 }
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c 
b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 33f9b6e585..d681a3039e 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -985,10 +985,11 @@ EfiKeyFiFoInsertOneKey (
   // while current TPL is TPL_NOTIFY. It will be invoked in
   // KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
   //
   EfiKeyFiFoForNotifyInsertOneKey (TerminalDevice->EfiKeyFiFoForNotify, 
Key);
   gBS->SignalEvent (TerminalDevice->KeyNotifyProcessEvent);
+  break;
 }
   }
   if (IsEfiKeyFiFoFull (TerminalDevice)) {
 //
 // Efi Key FIFO is full
-- 
2.14.3.windows.1

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


[edk2] [patch 1/3] EmbeddedPkg/VirtualKeyboard: Avoid notification called more than once

2018-09-10 Thread dandan bi
From: Dandan Bi 

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

Issue:
In current code logic, when a key is pressed, it will search
the whole NotifyList to find whether a notification has been
registered with the keystroke. if yes, it will en-queue the
key for notification execution later. And now if different
notification functions have been registered with the same key,
then the key will be en-queued more than once. Then it will
cause the notification executed more than once.

This patch is to enhance the code logic to fix this issue.

Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c 
b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c
index 6609bc8dbe..daea9c47d2 100644
--- a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c
+++ b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c
@@ -1,9 +1,9 @@
 /** @file
   VirtualKeyboard driver
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 Copyright (c) 2018, Linaro Ltd. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
 of the BSD License which accompanies this distribution.  The
@@ -1043,10 +1043,11 @@ VirtualKeyboardTimerHandler (
   // while current TPL is TPL_NOTIFY. It will be invoked in
   // KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
   //
   Enqueue (>QueueForNotify, );
   gBS->SignalEvent (VirtualKeyboardPrivate->KeyNotifyProcessEvent);
+  break;
 }
   }
 
   Enqueue (>Queue, );
 
-- 
2.14.3.windows.1

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


[edk2] [patch 2/3] IntelFrameworkModulePkg: Avoid key notification called more than once

2018-09-10 Thread dandan bi
From: Dandan Bi 

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

Issue:
In current code logic, when a key is pressed, it will search
the whole NotifyList to find whether a notification has been
registered with the keystroke. if yes, it will en-queue the
key for notification execution later. And now if different
notification functions have been registered with the same key,
then the key will be en-queued more than once. Then it will
cause the notification executed more than once.

This patch is to enhance the code logic to fix this issue.

Cc: Ruiyu Ni 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c   | 1 +
 IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
index 202588191e..fddb0b21fb 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
@@ -1485,10 +1485,11 @@ KeyGetchar (
   // while current TPL is TPL_NOTIFY. It will be invoked in
   // KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
   //
   PushEfikeyBufTail (>EfiKeyQueueForNotify, );
   gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent);
+  break;
 }
   }
 
   PushEfikeyBufTail (>EfiKeyQueue, );
 }
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c 
b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index 63f6303995..bee5f8f5e5 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -1984,10 +1984,11 @@ BiosKeyboardTimerHandler (
   // while current TPL is TPL_NOTIFY. It will be invoked in
   // KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
   //
   Enqueue (>QueueForNotify, );
   gBS->SignalEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent);
+  break;
 }
   }
 
   Enqueue (>Queue, );
 
-- 
2.14.3.windows.1

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


[edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler

2018-09-10 Thread Liming Gao
This change is wrong introduced by e21e355e2ca7fefb15b4df7078f995d3fb9c2b89
It is not required. So, revert it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Jiewen Yao 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index 315d0f8..7b1b3ca 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -173,9 +173,8 @@ SmiHandlerIdtrAbsAddr:
 mov gs, eax
 mov ax, [rbx + DSC_SS]
 mov ss, eax
-mov rax, strict qword 0 ;   mov rax, _SmiHandler
-_SmiHandlerAbsAddr:
-jmp rax
+
+;   jmp _SmiHandler ; instruction is not needed
 
 _SmiHandler:
 mov rbx, [rsp + 0x8] ; rcx <- CpuIndex
@@ -229,8 +228,4 @@ ASM_PFX(PiSmmCpuSmiEntryFixupAddress):
 learax, [ASM_PFX(gSmiHandlerIdtr)]
 learcx, [SmiHandlerIdtrAbsAddr]
 movqword [rcx - 8], rax
-
-learax, [_SmiHandler]
-learcx, [_SmiHandlerAbsAddr]
-movqword [rcx - 8], rax
 ret
-- 
2.10.0.windows.1

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


[edk2] [PATCH] MdeModulePkg/ConPlatform: Support short-form USB device path

2018-09-10 Thread Ruiyu Ni
Today's implementation does an exact device path match to check
whether the device path of a console is in ConIn/ConOut/ErrOut.
But that doesn't work for the USB keyboard.
Because when a platform have multiple USB port, ConIn needs to
carry all device paths corresponding to each port.
Even worse, today's BDS core logic removes the device path from
ConIn/ConOut/ErrOut when the connection to that device path fails.
So if user switches the USB keyboard from one port to another across
boot, the USB keyboard doesn't work in the second boot.

ConPlatform driver solved this problem by adding the
IsHotPlugDevice() function. So that for USB keyboard, ConPlatform
doesn't care whether its device path is in ConIn or not.
But the rule is too loose, and now causes platform BDS cannot control
whether to enable USB keyboard as an active console.

The patch changes ConPlatform to support USB short-form device path
when checking whether the device path of a console is in
ConIn/ConOut/ErrOut.

The logic to always accept USB/PCCARD device as active console is
removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Hao A Wu 
Cc: Michael D Kinney 
Cc: Star Zeng 
---
 .../Universal/Console/ConPlatformDxe/ConPlatform.c | 513 ++---
 .../Universal/Console/ConPlatformDxe/ConPlatform.h |  24 +-
 .../Console/ConPlatformDxe/ConPlatformDxe.inf  |   1 +
 3 files changed, 339 insertions(+), 199 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c 
b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
index 5fa7facfca..27df8a4b56 100644
--- a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
+++ b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
@@ -202,8 +202,7 @@ ConPlatformDriverBindingSupported (
   Start this driver on ControllerHandle by opening Simple Text Input Protocol,
   reading Device Path, and installing Console In Devcice GUID on 
ControllerHandle.
 
-  If this devcie is not one hot-plug devce, append its device path into the
-  console environment variables ConInDev.
+  Append its device path into the console environment variables ConInDev.
 
   @param  This Protocol instance pointer.
   @param  ControllerHandle Handle of device to bind driver to
@@ -270,58 +269,32 @@ ConPlatformTextInDriverBindingStart (
   }
 
   //
-  // Check the device path, if it is a hot plug device,
-  // do not put the device path into ConInDev, and install
-  // gEfiConsoleInDeviceGuid to the device handle directly.
-  // The policy is, make hot plug device plug in and play immediately.
+  // Append the device path to the ConInDev environment variable
   //
-  if (IsHotPlugDevice (DevicePath)) {
+  ConPlatformUpdateDeviceVariable (
+L"ConInDev",
+DevicePath,
+Append
+);
+
+  //
+  // If the device path is an instance in the ConIn environment variable,
+  // then install EfiConsoleInDeviceGuid onto ControllerHandle
+  //
+  if (IsInConInVariable) {
 gBS->InstallMultipleProtocolInterfaces (
,
,
NULL,
NULL
);
-//
-// Append the device path to ConInDev only if it is in ConIn variable.
-//
-if (IsInConInVariable) {
-  ConPlatformUpdateDeviceVariable (
-L"ConInDev",
-DevicePath,
-Append
-);
-}
   } else {
-//
-// If it is not a hot-plug device, append the device path to the
-// ConInDev environment variable
-//
-ConPlatformUpdateDeviceVariable (
-  L"ConInDev",
-  DevicePath,
-  Append
-  );
-
-//
-// If the device path is an instance in the ConIn environment variable,
-// then install EfiConsoleInDeviceGuid onto ControllerHandle
-//
-if (IsInConInVariable) {
-  gBS->InstallMultipleProtocolInterfaces (
- ,
- ,
- NULL,
- NULL
- );
-} else {
-  gBS->CloseProtocol (
- ControllerHandle,
- ,
- This->DriverBindingHandle,
- ControllerHandle
- );
-}
+gBS->CloseProtocol (
+   ControllerHandle,
+   ,
+   This->DriverBindingHandle,
+   ControllerHandle
+   );
   }
 
   return EFI_SUCCESS;
@@ -334,8 +307,7 @@ ConPlatformTextInDriverBindingStart (
   reading Device Path, and installing Console Out Devcic GUID, Standard Error
   Device GUID on ControllerHandle.
 
-  If this devcie is not one hot-plug devce, append its device path into the
-  console environment variables ConOutDev, ErrOutDev.
+  Append its device path into the console environment variables ConOutDev, 
ErrOutDev.
 
   @param  This Protocol instance pointer.
   @param  ControllerHandle Handle of device to bind driver to
@@ -416,95 +388,59 @@ ConPlatformTextOutDriverBindingStart (
   }
 
   //
-  // Check the device path, if it is a hot plug device,
-  // do not put the device 

[edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

2018-09-10 Thread Ruiyu Ni
Today's InterlockedIncrement()/InterlockedDecrement() guarantees to
perform atomic increment/decrement but doesn't guarantee the return
value equals to the new value.

The patch fixes the behavior to use "XADD" instruction to guarantee
the return value equals to the new value.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Jiewen Yao 
Cc: Liming Gao 
Cc: Michael D Kinney 
---
 MdePkg/Include/Library/SynchronizationLib.h|  6 +--
 .../BaseSynchronizationLib.inf | 18 -
 .../BaseSynchronizationLibInternals.h  |  6 +--
 .../BaseSynchronizationLib/Ia32/GccInline.c| 18 -
 .../Ia32/InterlockedDecrement.c| 42 
 .../Ia32/InterlockedDecrement.nasm | 10 ++---
 .../Ia32/InterlockedIncrement.c| 43 
 .../Ia32/InterlockedIncrement.nasm |  7 ++--
 .../BaseSynchronizationLib/Synchronization.c   |  6 +--
 .../BaseSynchronizationLib/SynchronizationGcc.c|  6 +--
 .../BaseSynchronizationLib/SynchronizationMsc.c|  6 +--
 .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 +
 .../X64/InterlockedDecrement.c | 46 --
 .../X64/InterlockedDecrement.nasm  |  8 ++--
 .../X64/InterlockedIncrement.c | 46 --
 .../X64/InterlockedIncrement.nasm  |  5 ++-
 16 files changed, 52 insertions(+), 240 deletions(-)
 delete mode 100644 
MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
 delete mode 100644 
MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
 delete mode 100644 
MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
 delete mode 100644 
MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c

diff --git a/MdePkg/Include/Library/SynchronizationLib.h 
b/MdePkg/Include/Library/SynchronizationLib.h
index da69f6ff5e..ce3bce04f5 100644
--- a/MdePkg/Include/Library/SynchronizationLib.h
+++ b/MdePkg/Include/Library/SynchronizationLib.h
@@ -144,8 +144,7 @@ ReleaseSpinLock (
 
   Performs an atomic increment of the 32-bit unsigned integer specified by
   Value and returns the incremented value. The increment operation must be
-  performed using MP safe mechanisms. The state of the return value is not
-  guaranteed to be MP safe.
+  performed using MP safe mechanisms.
 
   If Value is NULL, then ASSERT().
 
@@ -166,8 +165,7 @@ InterlockedIncrement (
 
   Performs an atomic decrement of the 32-bit unsigned integer specified by
   Value and returns the decremented value. The decrement operation must be
-  performed using MP safe mechanisms. The state of the return value is not
-  guaranteed to be MP safe.
+  performed using MP safe mechanisms.
 
   If Value is NULL, then ASSERT().
 
diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf 
b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
index 0be1d4331f..b971cd138d 100755
--- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
+++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
@@ -34,9 +34,9 @@ [Sources.IA32]
   Ia32/InterlockedCompareExchange64.c | MSFT
   Ia32/InterlockedCompareExchange32.c | MSFT
   Ia32/InterlockedCompareExchange16.c | MSFT
-  Ia32/InterlockedDecrement.c | MSFT
-  Ia32/InterlockedIncrement.c | MSFT
-  SynchronizationMsc.c  | MSFT
+  Ia32/InterlockedDecrement.nasm | MSFT
+  Ia32/InterlockedIncrement.nasm | MSFT
+  SynchronizationMsc.c | MSFT
 
   Ia32/InterlockedCompareExchange64.nasm| INTEL
   Ia32/InterlockedCompareExchange32.nasm| INTEL
@@ -54,17 +54,15 @@ [Sources.X64]
   X64/InterlockedCompareExchange64.c | MSFT
   X64/InterlockedCompareExchange32.c | MSFT
   X64/InterlockedCompareExchange16.c | MSFT
+  X64/InterlockedDecrement.nasm | MSFT
+  X64/InterlockedIncrement.nasm | MSFT
+  SynchronizationMsc.c | MSFT
 
   X64/InterlockedCompareExchange64.nasm| INTEL
   X64/InterlockedCompareExchange32.nasm| INTEL
   X64/InterlockedCompareExchange16.nasm| INTEL
-
-  X64/InterlockedDecrement.c | MSFT
-  X64/InterlockedIncrement.c | MSFT
-  SynchronizationMsc.c | MSFT
-
-  X64/InterlockedDecrement.nasm| INTEL
-  X64/InterlockedIncrement.nasm| INTEL
+  X64/InterlockedDecrement.nasm | INTEL
+  X64/InterlockedIncrement.nasm | INTEL
   Synchronization.c | INTEL
 
   Ia32/InternalGetSpinLockProperties.c | GCC
diff --git 
a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h 
b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h
index 37edd7c188..8c363a8585 100644
--- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h
+++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h
@@ -27,8 +27,7 @@
 
   Performs an atomic increment of the 32-bit unsigned integer specified by
   Value and returns the incremented value. The increment operation must 

Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

2018-09-10 Thread Yao, Jiewen
Hi
Can we use XSUB for decrement?

Thank you
Yao Jiewen

> -Original Message-
> From: Ni, Ruiyu
> Sent: Monday, September 10, 2018 6:06 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Gao, Liming
> ; Kinney, Michael D 
> Subject: [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement
> return value
> 
> Today's InterlockedIncrement()/InterlockedDecrement() guarantees to
> perform atomic increment/decrement but doesn't guarantee the return
> value equals to the new value.
> 
> The patch fixes the behavior to use "XADD" instruction to guarantee
> the return value equals to the new value.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jiewen Yao 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> ---
>  MdePkg/Include/Library/SynchronizationLib.h|  6 +--
>  .../BaseSynchronizationLib.inf | 18 -
>  .../BaseSynchronizationLibInternals.h  |  6 +--
>  .../BaseSynchronizationLib/Ia32/GccInline.c| 18 -
>  .../Ia32/InterlockedDecrement.c| 42
> 
>  .../Ia32/InterlockedDecrement.nasm | 10 ++---
>  .../Ia32/InterlockedIncrement.c| 43
> 
>  .../Ia32/InterlockedIncrement.nasm |  7 ++--
>  .../BaseSynchronizationLib/Synchronization.c   |  6 +--
>  .../BaseSynchronizationLib/SynchronizationGcc.c|  6 +--
>  .../BaseSynchronizationLib/SynchronizationMsc.c|  6 +--
>  .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 +
>  .../X64/InterlockedDecrement.c | 46
> --
>  .../X64/InterlockedDecrement.nasm  |  8 ++--
>  .../X64/InterlockedIncrement.c | 46
> --
>  .../X64/InterlockedIncrement.nasm  |  5 ++-
>  16 files changed, 52 insertions(+), 240 deletions(-)
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> 
> diff --git a/MdePkg/Include/Library/SynchronizationLib.h
> b/MdePkg/Include/Library/SynchronizationLib.h
> index da69f6ff5e..ce3bce04f5 100644
> --- a/MdePkg/Include/Library/SynchronizationLib.h
> +++ b/MdePkg/Include/Library/SynchronizationLib.h
> @@ -144,8 +144,7 @@ ReleaseSpinLock (
> 
>Performs an atomic increment of the 32-bit unsigned integer specified by
>Value and returns the incremented value. The increment operation must
> be
> -  performed using MP safe mechanisms. The state of the return value is
> not
> -  guaranteed to be MP safe.
> +  performed using MP safe mechanisms.
> 
>If Value is NULL, then ASSERT().
> 
> @@ -166,8 +165,7 @@ InterlockedIncrement (
> 
>Performs an atomic decrement of the 32-bit unsigned integer specified
> by
>Value and returns the decremented value. The decrement operation must
> be
> -  performed using MP safe mechanisms. The state of the return value is
> not
> -  guaranteed to be MP safe.
> +  performed using MP safe mechanisms.
> 
>If Value is NULL, then ASSERT().
> 
> diff --git
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> index 0be1d4331f..b971cd138d 100755
> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> @@ -34,9 +34,9 @@ [Sources.IA32]
>Ia32/InterlockedCompareExchange64.c | MSFT
>Ia32/InterlockedCompareExchange32.c | MSFT
>Ia32/InterlockedCompareExchange16.c | MSFT
> -  Ia32/InterlockedDecrement.c | MSFT
> -  Ia32/InterlockedIncrement.c | MSFT
> -  SynchronizationMsc.c  | MSFT
> +  Ia32/InterlockedDecrement.nasm | MSFT
> +  Ia32/InterlockedIncrement.nasm | MSFT
> +  SynchronizationMsc.c | MSFT
> 
>Ia32/InterlockedCompareExchange64.nasm| INTEL
>Ia32/InterlockedCompareExchange32.nasm| INTEL
> @@ -54,17 +54,15 @@ [Sources.X64]
>X64/InterlockedCompareExchange64.c | MSFT
>X64/InterlockedCompareExchange32.c | MSFT
>X64/InterlockedCompareExchange16.c | MSFT
> +  X64/InterlockedDecrement.nasm | MSFT
> +  X64/InterlockedIncrement.nasm | MSFT
> +  SynchronizationMsc.c | MSFT
> 
>X64/InterlockedCompareExchange64.nasm| INTEL
>X64/InterlockedCompareExchange32.nasm| INTEL
>X64/InterlockedCompareExchange16.nasm| INTEL
> -
> -  X64/InterlockedDecrement.c | MSFT
> -  X64/InterlockedIncrement.c | MSFT
> -  SynchronizationMsc.c | MSFT
> -
> -  X64/InterlockedDecrement.nasm| INTEL
> -  X64/InterlockedIncrement.nasm| INTEL
> +  X64/InterlockedDecrement.nasm | INTEL
> +  X64/InterlockedIncrement.nasm | INTEL
>Synchronization.c | INTEL
> 
>

[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Logo Update.

2018-09-10 Thread zwei4
(1) Update EDK2 TianoCore logo to the latest.
(2) Extend logo showing period to the late stage of BDS.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Wei 
CC: Mike Wu  
CC: Mang Guo 
---
 .../AuroraGlacier/BoardInitPostMem/BoardInit.c |   2 +-
 .../BoardInitPostMem/BoardInitPostMem.inf  |   2 +-
 .../BensonGlacier/BoardInitPostMem/BoardInit.c |   2 +-
 .../BoardInitPostMem/BoardInitPostMem.inf  |   2 +-
 .../Board/LeafHill/BoardInitPostMem/BoardInit.c|   2 +-
 .../LeafHill/BoardInitPostMem/BoardInitPostMem.inf |   2 +-
 .../MinnowBoard3/BoardInitPostMem/BoardInit.c  |   2 +-
 .../BoardInitPostMem/BoardInitPostMem.inf  |   2 +-
 .../BoardInitPostMem/BoardInit.c   |   2 +-
 .../BoardInitPostMem/BoardInitPostMem.inf  |   2 +-
 .../Board/UP2/BoardInitPostMem/BoardInit.c |   2 +-
 .../UP2/BoardInitPostMem/BoardInitPostMem.inf  |   2 +-
 .../Common/Binaries/Logo/Tianocore.bmp | Bin 0 -> 11958 bytes
 .../Common/Library/DxeLogoLib/DxeLogoLib.inf   |   1 -
 .../Common/Library/DxeLogoLib/Logo.c   | 299 -
 .../PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf|   1 -
 .../PlatformBootManagerLib/PlatformBootManager.c   |  10 +-
 .../PlatformBootManagerLib.inf |   4 +-
 Platform/BroxtonPlatformPkg/PlatformPkg.dec|   3 +-
 Platform/BroxtonPlatformPkg/PlatformPkg.fdf|  13 +-
 20 files changed, 83 insertions(+), 272 deletions(-)
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Binaries/Logo/Tianocore.bmp

diff --git 
a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c 
b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c
index 302edb8301..ce98086895 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c
+++ 
b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c
@@ -110,7 +110,7 @@ AuroraGlacierPostMemInitCallback (
   //
   BufferSize = sizeof (EFI_GUID);
   PcdSetPtr(PcdBoardVbtFileGuid, , (UINT8 
*));
-  PcdSetPtr(PcdOemLogoFileGuid, , (UINT8 *));
+  PcdSetPtr(PcdOemLogoFileGuid, , (UINT8 
*)PcdGetPtr(PcdTianoCoreLogoFileGuid));
 
   //
   // Set PcdSueCreek
diff --git 
a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf
 
b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf
index 8fe5ea7e95..c619332cb1 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf
+++ 
b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf
@@ -64,6 +64,7 @@
   gPlatformModuleTokenSpaceGuid.PcdResetType
   gPlatformModuleTokenSpaceGuid.PcdBoardVbtFileGuid
   gPlatformModuleTokenSpaceGuid.PcdOemLogoFileGuid
+  gPlatformModuleTokenSpaceGuid.PcdTianoCoreLogoFileGuid
   gPlatformModuleTokenSpaceGuid.PcdSueCreek
   gPlatformModuleTokenSpaceGuid.PcdMaxPkgCState
   gPlatformModuleTokenSpaceGuid.PcdTi3100AudioCodecEnable
@@ -85,7 +86,6 @@
   gEfiTpmDeviceInstanceTpm20DtpmGuid
   gTpmDeviceInstanceTpm20PttPtpGuid
   gPeiAuroraGlacierVbtGuid
-  gPeiLogoGuid
 
 [Ppis]
   gBoardPostMemInitStartGuid
diff --git 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c
index 60d2324d95..856d7739b0 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c
+++ 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c
@@ -110,7 +110,7 @@ BensonGlacierPostMemInitCallback (
   //
   BufferSize = sizeof (EFI_GUID);
   PcdSetPtr(PcdBoardVbtFileGuid, , (UINT8 
*));
-  PcdSetPtr(PcdOemLogoFileGuid, , (UINT8 *));
+  PcdSetPtr(PcdOemLogoFileGuid, , (UINT8 
*)PcdGetPtr(PcdTianoCoreLogoFileGuid));
 
   //
   // Set PcdSueCreek
diff --git 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
index 782bf0d2bd..01d7f27898 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
+++ 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
@@ -64,6 +64,7 @@
   gPlatformModuleTokenSpaceGuid.PcdResetType
   gPlatformModuleTokenSpaceGuid.PcdBoardVbtFileGuid
   gPlatformModuleTokenSpaceGuid.PcdOemLogoFileGuid
+  gPlatformModuleTokenSpaceGuid.PcdTianoCoreLogoFileGuid
   gPlatformModuleTokenSpaceGuid.PcdSueCreek
   gPlatformModuleTokenSpaceGuid.PcdMaxPkgCState
   gPlatformModuleTokenSpaceGuid.PcdTi3100AudioCodecEnable
@@ -85,7 +86,6 @@
   gEfiTpmDeviceInstanceTpm20DtpmGuid
   gTpmDeviceInstanceTpm20PttPtpGuid
   gPeiBensonGlacierVbtGuid
-  gPeiLogoGuid
 
 [Ppis]
   gBoardPostMemInitStartGuid
diff --git 
a/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInit.c 

Re: [edk2] Performance enabling of Event handler

2018-09-10 Thread prabin ca
Hi Team,

Thank it’s got worked on dp -t 0.

> On 07-Sep-2018, at 9:27 PM, prabin ca  wrote:
> 
> Hi 
> 
> Yes it is included in same module (both event handler and function handler), 
> and I’m not perf_start and perf_end only two times (one is by event handler 
> and one is by normal function handler).
> 
> And I’m trying to print result using DP.efi, it shows entry for normal 
> function.
> 
>> On 07-Sep-2018, at 8:46 AM, Bi, Dandan  wrote:
>> 
>> Hi Prabin,
>> 
>> The Performance logging for the normal functions handlers and event handlers 
>> should be the same.
>> Are the normal function calls and the event handler function calls you 
>> tested in the same module?
>> If not, please double check to make sure the performance library instance 
>> used correctly for each module.
>> 
>> 
>> Thanks,
>> Dandan
>> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> prabin ca
>> Sent: Friday, September 7, 2018 10:30 AM
>> To: Laszlo Ersek 
>> Cc: Bi, Dandan ; edk2-devel@lists.01.org; 
>> af...@apple.com
>> Subject: Re: [edk2] Performance enabling of Event handler
>> 
>> Hi,
>> PerformancePkg is not working with event handlers, but it’s working with 
>> normal functions handlers. 
>> 
 On 06-Sep-2018, at 3:28 PM, Laszlo Ersek  wrote:
 
 On 09/06/18 08:10, prabin ca wrote:
 Hi Team,
 
 I’m used edk2 PerformancePkg for profiling cpu execution time taken by a 
 event handler. Event is created successfully and event handler is also 
 called successfully, but I can capture the performance of this event 
 handler with PerformancePkg (by using perf_start and perf_end check 
 points). This PerformancePkg is working fine with normal function calls.
>>> 
>>> Do you mean "can not", instead of "can"? (Sorry, I don't understand.)
>>> 
 
 Please help me to enable PerformancePkg action on event handler also.
 
>>> 
>>> Hmmm, even with the suggested typo correction, I wouldn't know what to 
>>> suggest. Sorry!
>>> 
>>> Laszlo
>> ___
>> 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] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

2018-09-10 Thread Kinney, Michael D
Ray,

Why are we removing the use of intrinsics from 
MSFT builds?  We should keep those for more efficient
code generation if the intrinsic has the correct
behavior.

Thanks,

Mike

> -Original Message-
> From: Ni, Ruiyu
> Sent: Monday, September 10, 2018 3:06 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Gao, Liming
> ; Kinney, Michael D
> 
> Subject: [PATCH] MdePkg/SynchronizationLib: fix
> Interlocked[De|In]crement return value
> 
> Today's InterlockedIncrement()/InterlockedDecrement()
> guarantees to
> perform atomic increment/decrement but doesn't guarantee
> the return
> value equals to the new value.
> 
> The patch fixes the behavior to use "XADD" instruction
> to guarantee
> the return value equals to the new value.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jiewen Yao 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> ---
>  MdePkg/Include/Library/SynchronizationLib.h|  6
> +--
>  .../BaseSynchronizationLib.inf | 18
> -
>  .../BaseSynchronizationLibInternals.h  |  6
> +--
>  .../BaseSynchronizationLib/Ia32/GccInline.c| 18
> -
>  .../Ia32/InterlockedDecrement.c| 42
> 
>  .../Ia32/InterlockedDecrement.nasm | 10
> ++---
>  .../Ia32/InterlockedIncrement.c| 43
> 
>  .../Ia32/InterlockedIncrement.nasm |  7
> ++--
>  .../BaseSynchronizationLib/Synchronization.c   |  6
> +--
>  .../BaseSynchronizationLib/SynchronizationGcc.c|  6
> +--
>  .../BaseSynchronizationLib/SynchronizationMsc.c|  6
> +--
>  .../Library/BaseSynchronizationLib/X64/GccInline.c | 19
> +
>  .../X64/InterlockedDecrement.c | 46
> --
>  .../X64/InterlockedDecrement.nasm  |  8
> ++--
>  .../X64/InterlockedIncrement.c | 46
> --
>  .../X64/InterlockedIncrement.nasm  |  5
> ++-
>  16 files changed, 52 insertions(+), 240 deletions(-)
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDe
> crement.c
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIn
> crement.c
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDec
> rement.c
>  delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedInc
> rement.c
> 
> diff --git a/MdePkg/Include/Library/SynchronizationLib.h
> b/MdePkg/Include/Library/SynchronizationLib.h
> index da69f6ff5e..ce3bce04f5 100644
> --- a/MdePkg/Include/Library/SynchronizationLib.h
> +++ b/MdePkg/Include/Library/SynchronizationLib.h
> @@ -144,8 +144,7 @@ ReleaseSpinLock (
> 
>Performs an atomic increment of the 32-bit unsigned
> integer specified by
>Value and returns the incremented value. The
> increment operation must be
> -  performed using MP safe mechanisms. The state of the
> return value is not
> -  guaranteed to be MP safe.
> +  performed using MP safe mechanisms.
> 
>If Value is NULL, then ASSERT().
> 
> @@ -166,8 +165,7 @@ InterlockedIncrement (
> 
>Performs an atomic decrement of the 32-bit unsigned
> integer specified by
>Value and returns the decremented value. The
> decrement operation must be
> -  performed using MP safe mechanisms. The state of the
> return value is not
> -  guaranteed to be MP safe.
> +  performed using MP safe mechanisms.
> 
>If Value is NULL, then ASSERT().
> 
> diff --git
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> ionLib.inf
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> ionLib.inf
> index 0be1d4331f..b971cd138d 100755
> ---
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> ionLib.inf
> +++
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> ionLib.inf
> @@ -34,9 +34,9 @@ [Sources.IA32]
>Ia32/InterlockedCompareExchange64.c | MSFT
>Ia32/InterlockedCompareExchange32.c | MSFT
>Ia32/InterlockedCompareExchange16.c | MSFT
> -  Ia32/InterlockedDecrement.c | MSFT
> -  Ia32/InterlockedIncrement.c | MSFT
> -  SynchronizationMsc.c  | MSFT
> +  Ia32/InterlockedDecrement.nasm | MSFT
> +  Ia32/InterlockedIncrement.nasm | MSFT
> +  SynchronizationMsc.c | MSFT
> 
>Ia32/InterlockedCompareExchange64.nasm| INTEL
>Ia32/InterlockedCompareExchange32.nasm| INTEL
> @@ -54,17 +54,15 @@ [Sources.X64]
>X64/InterlockedCompareExchange64.c | MSFT
>X64/InterlockedCompareExchange32.c | MSFT
>X64/InterlockedCompareExchange16.c | MSFT
> +  X64/InterlockedDecrement.nasm | MSFT
> +  X64/InterlockedIncrement.nasm | MSFT
> +  SynchronizationMsc.c | MSFT
> 
>X64/InterlockedCompareExchange64.nasm| INTEL
>X64/InterlockedCompareExchange32.nasm| INTEL
>X64/InterlockedCompareExchange16.nasm| INTEL
> -
> -  X64/InterlockedDecrement.c | MSFT
> -  X64/InterlockedIncrement.c | MSFT
> -  SynchronizationMsc.c | MSFT
> -
> -  X64/InterlockedDecrement.nasm| INTEL
> - 

Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

2018-09-10 Thread Ni, Ruiyu
I didn’t find such instruction in SDM.

发自我的 iPhone

> 在 2018年9月10日,下午7:37,Yao, Jiewen  写道:
> 
> Hi
> Can we use XSUB for decrement?
> 
> Thank you
> Yao Jiewen
> 
>> -Original Message-
>> From: Ni, Ruiyu
>> Sent: Monday, September 10, 2018 6:06 PM
>> To: edk2-devel@lists.01.org
>> Cc: Yao, Jiewen ; Gao, Liming
>> ; Kinney, Michael D 
>> Subject: [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement
>> return value
>> 
>> Today's InterlockedIncrement()/InterlockedDecrement() guarantees to
>> perform atomic increment/decrement but doesn't guarantee the return
>> value equals to the new value.
>> 
>> The patch fixes the behavior to use "XADD" instruction to guarantee
>> the return value equals to the new value.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ruiyu Ni 
>> Cc: Jiewen Yao 
>> Cc: Liming Gao 
>> Cc: Michael D Kinney 
>> ---
>> MdePkg/Include/Library/SynchronizationLib.h|  6 +--
>> .../BaseSynchronizationLib.inf | 18 -
>> .../BaseSynchronizationLibInternals.h  |  6 +--
>> .../BaseSynchronizationLib/Ia32/GccInline.c| 18 -
>> .../Ia32/InterlockedDecrement.c| 42
>> 
>> .../Ia32/InterlockedDecrement.nasm | 10 ++---
>> .../Ia32/InterlockedIncrement.c| 43
>> 
>> .../Ia32/InterlockedIncrement.nasm |  7 ++--
>> .../BaseSynchronizationLib/Synchronization.c   |  6 +--
>> .../BaseSynchronizationLib/SynchronizationGcc.c|  6 +--
>> .../BaseSynchronizationLib/SynchronizationMsc.c|  6 +--
>> .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 +
>> .../X64/InterlockedDecrement.c | 46
>> --
>> .../X64/InterlockedDecrement.nasm  |  8 ++--
>> .../X64/InterlockedIncrement.c | 46
>> --
>> .../X64/InterlockedIncrement.nasm  |  5 ++-
>> 16 files changed, 52 insertions(+), 240 deletions(-)
>> delete mode 100644
>> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
>> delete mode 100644
>> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
>> delete mode 100644
>> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
>> delete mode 100644
>> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
>> 
>> diff --git a/MdePkg/Include/Library/SynchronizationLib.h
>> b/MdePkg/Include/Library/SynchronizationLib.h
>> index da69f6ff5e..ce3bce04f5 100644
>> --- a/MdePkg/Include/Library/SynchronizationLib.h
>> +++ b/MdePkg/Include/Library/SynchronizationLib.h
>> @@ -144,8 +144,7 @@ ReleaseSpinLock (
>> 
>>   Performs an atomic increment of the 32-bit unsigned integer specified by
>>   Value and returns the incremented value. The increment operation must
>> be
>> -  performed using MP safe mechanisms. The state of the return value is
>> not
>> -  guaranteed to be MP safe.
>> +  performed using MP safe mechanisms.
>> 
>>   If Value is NULL, then ASSERT().
>> 
>> @@ -166,8 +165,7 @@ InterlockedIncrement (
>> 
>>   Performs an atomic decrement of the 32-bit unsigned integer specified
>> by
>>   Value and returns the decremented value. The decrement operation must
>> be
>> -  performed using MP safe mechanisms. The state of the return value is
>> not
>> -  guaranteed to be MP safe.
>> +  performed using MP safe mechanisms.
>> 
>>   If Value is NULL, then ASSERT().
>> 
>> diff --git
>> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>> index 0be1d4331f..b971cd138d 100755
>> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>> @@ -34,9 +34,9 @@ [Sources.IA32]
>>   Ia32/InterlockedCompareExchange64.c | MSFT
>>   Ia32/InterlockedCompareExchange32.c | MSFT
>>   Ia32/InterlockedCompareExchange16.c | MSFT
>> -  Ia32/InterlockedDecrement.c | MSFT
>> -  Ia32/InterlockedIncrement.c | MSFT
>> -  SynchronizationMsc.c  | MSFT
>> +  Ia32/InterlockedDecrement.nasm | MSFT
>> +  Ia32/InterlockedIncrement.nasm | MSFT
>> +  SynchronizationMsc.c | MSFT
>> 
>>   Ia32/InterlockedCompareExchange64.nasm| INTEL
>>   Ia32/InterlockedCompareExchange32.nasm| INTEL
>> @@ -54,17 +54,15 @@ [Sources.X64]
>>   X64/InterlockedCompareExchange64.c | MSFT
>>   X64/InterlockedCompareExchange32.c | MSFT
>>   X64/InterlockedCompareExchange16.c | MSFT
>> +  X64/InterlockedDecrement.nasm | MSFT
>> +  X64/InterlockedIncrement.nasm | MSFT
>> +  SynchronizationMsc.c | MSFT
>> 
>>   X64/InterlockedCompareExchange64.nasm| INTEL
>>   X64/InterlockedCompareExchange32.nasm| INTEL
>>   X64/InterlockedCompareExchange16.nasm| INTEL
>> -
>> -  X64/InterlockedDecrement.c | MSFT
>> -  X64/InterlockedIncrement.c | MSFT
>> -  SynchronizationMsc.c | MSFT
>> -
>> -  X64/InterlockedDecrement.nasm| INTEL
>> - 

Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

2018-09-10 Thread Yao, Jiewen
You are right. My mistake.

> -Original Message-
> From: Ni, Ruiyu
> Sent: Monday, September 10, 2018 10:59 PM
> To: Yao, Jiewen 
> Cc: edk2-devel@lists.01.org; Gao, Liming ; Kinney,
> Michael D 
> Subject: Re: [PATCH] MdePkg/SynchronizationLib: fix
> Interlocked[De|In]crement return value
> 
> I didn’t find such instruction in SDM.
> 
> 发自我的 iPhone
> 
> > 在 2018年9月10日,下午7:37,Yao, Jiewen 
> 写道:
> >
> > Hi
> > Can we use XSUB for decrement?
> >
> > Thank you
> > Yao Jiewen
> >
> >> -Original Message-
> >> From: Ni, Ruiyu
> >> Sent: Monday, September 10, 2018 6:06 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Yao, Jiewen ; Gao, Liming
> >> ; Kinney, Michael D
> 
> >> Subject: [PATCH] MdePkg/SynchronizationLib: fix
> Interlocked[De|In]crement
> >> return value
> >>
> >> Today's InterlockedIncrement()/InterlockedDecrement() guarantees to
> >> perform atomic increment/decrement but doesn't guarantee the return
> >> value equals to the new value.
> >>
> >> The patch fixes the behavior to use "XADD" instruction to guarantee
> >> the return value equals to the new value.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ruiyu Ni 
> >> Cc: Jiewen Yao 
> >> Cc: Liming Gao 
> >> Cc: Michael D Kinney 
> >> ---
> >> MdePkg/Include/Library/SynchronizationLib.h|  6 +--
> >> .../BaseSynchronizationLib.inf | 18 -
> >> .../BaseSynchronizationLibInternals.h  |  6 +--
> >> .../BaseSynchronizationLib/Ia32/GccInline.c| 18 -
> >> .../Ia32/InterlockedDecrement.c| 42
> >> 
> >> .../Ia32/InterlockedDecrement.nasm | 10 ++---
> >> .../Ia32/InterlockedIncrement.c| 43
> >> 
> >> .../Ia32/InterlockedIncrement.nasm |  7 ++--
> >> .../BaseSynchronizationLib/Synchronization.c   |  6 +--
> >> .../BaseSynchronizationLib/SynchronizationGcc.c|  6 +--
> >> .../BaseSynchronizationLib/SynchronizationMsc.c|  6 +--
> >> .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 +
> >> .../X64/InterlockedDecrement.c | 46
> >> --
> >> .../X64/InterlockedDecrement.nasm  |  8 ++--
> >> .../X64/InterlockedIncrement.c | 46
> >> --
> >> .../X64/InterlockedIncrement.nasm  |  5 ++-
> >> 16 files changed, 52 insertions(+), 240 deletions(-)
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> >>
> >> diff --git a/MdePkg/Include/Library/SynchronizationLib.h
> >> b/MdePkg/Include/Library/SynchronizationLib.h
> >> index da69f6ff5e..ce3bce04f5 100644
> >> --- a/MdePkg/Include/Library/SynchronizationLib.h
> >> +++ b/MdePkg/Include/Library/SynchronizationLib.h
> >> @@ -144,8 +144,7 @@ ReleaseSpinLock (
> >>
> >>   Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >>   Value and returns the incremented value. The increment operation
> must
> >> be
> >> -  performed using MP safe mechanisms. The state of the return value is
> >> not
> >> -  guaranteed to be MP safe.
> >> +  performed using MP safe mechanisms.
> >>
> >>   If Value is NULL, then ASSERT().
> >>
> >> @@ -166,8 +165,7 @@ InterlockedIncrement (
> >>
> >>   Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >>   Value and returns the decremented value. The decrement operation
> must
> >> be
> >> -  performed using MP safe mechanisms. The state of the return value is
> >> not
> >> -  guaranteed to be MP safe.
> >> +  performed using MP safe mechanisms.
> >>
> >>   If Value is NULL, then ASSERT().
> >>
> >> diff --git
> >> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> index 0be1d4331f..b971cd138d 100755
> >> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> +++
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> @@ -34,9 +34,9 @@ [Sources.IA32]
> >>   Ia32/InterlockedCompareExchange64.c | MSFT
> >>   Ia32/InterlockedCompareExchange32.c | MSFT
> >>   Ia32/InterlockedCompareExchange16.c | MSFT
> >> -  Ia32/InterlockedDecrement.c | MSFT
> >> -  Ia32/InterlockedIncrement.c | MSFT
> >> -  SynchronizationMsc.c  | MSFT
> >> +  Ia32/InterlockedDecrement.nasm | MSFT
> >> +  Ia32/InterlockedIncrement.nasm | MSFT
> >> +  SynchronizationMsc.c | MSFT
> >>
> >>   Ia32/InterlockedCompareExchange64.nasm| INTEL
> >>   Ia32/InterlockedCompareExchange32.nasm| INTEL
> >> @@ -54,17 +54,15 @@ [Sources.X64]
> >>   X64/InterlockedCompareExchange64.c | MSFT
> >>   

[edk2] [PATCH v2 2/9] BaseTools: AutoGen refactor WorkspaceAutoGen class

2018-09-10 Thread Jaben Carsey
Update the WorkspaceAutoGen class to use caching decorators and remove
the no longer needed private variables.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Cc: Bob Feng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 69 
 1 file changed, 28 insertions(+), 41 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 726b1c98f5bc..35dab75785eb 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -278,10 +278,6 @@ class WorkspaceAutoGen(AutoGen):
 self.FvTargetList   = Fvs if Fvs else []
 self.CapTargetList  = Caps if Caps else []
 self.AutoGenObjectList = []
-self._BuildDir  = None
-self._FvDir = None
-self._MakeFileDir   = None
-self._BuildCommand  = None
 self._GuidDict = {}
 
 # there's many relative directory operations, so ...
@@ -810,54 +806,56 @@ class WorkspaceAutoGen(AutoGen):
 return "%s [%s]" % (self.MetaFile, ", ".join(self.ArchList))
 
 ## Return the directory to store FV files
-def _GetFvDir(self):
-if self._FvDir is None:
-self._FvDir = path.join(self.BuildDir, TAB_FV_DIRECTORY)
-return self._FvDir
+@cached_property
+def FvDir(self):
+return path.join(self.BuildDir, TAB_FV_DIRECTORY)
 
 ## Return the directory to store all intermediate and final files built
-def _GetBuildDir(self):
-if self._BuildDir is None:
-return self.AutoGenObjectList[0].BuildDir
+@cached_property
+def BuildDir(self):
+return self.AutoGenObjectList[0].BuildDir
 
 ## Return the build output directory platform specifies
-def _GetOutputDir(self):
+@cached_property
+def OutputDir(self):
 return self.Platform.OutputDirectory
 
 ## Return platform name
-def _GetName(self):
+@cached_property
+def Name(self):
 return self.Platform.PlatformName
 
 ## Return meta-file GUID
-def _GetGuid(self):
+@cached_property
+def Guid(self):
 return self.Platform.Guid
 
 ## Return platform version
-def _GetVersion(self):
+@cached_property
+def Version(self):
 return self.Platform.Version
 
 ## Return paths of tools
-def _GetToolDefinition(self):
+@cached_property
+def ToolDefinition(self):
 return self.AutoGenObjectList[0].ToolDefinition
 
 ## Return directory of platform makefile
 #
 #   @retval string  Makefile directory
 #
-def _GetMakeFileDir(self):
-if self._MakeFileDir is None:
-self._MakeFileDir = self.BuildDir
-return self._MakeFileDir
+@cached_property
+def MakeFileDir(self):
+return self.BuildDir
 
 ## Return build command string
 #
 #   @retval string  Build command string
 #
-def _GetBuildCommand(self):
-if self._BuildCommand is None:
-# BuildCommand should be all the same. So just get one from 
platform AutoGen
-self._BuildCommand = self.AutoGenObjectList[0].BuildCommand
-return self._BuildCommand
+@cached_property
+def BuildCommand(self):
+# BuildCommand should be all the same. So just get one from platform 
AutoGen
+return self.AutoGenObjectList[0].BuildCommand
 
 ## Check the PCDs token value conflict in each DEC file.
 #
@@ -933,7 +931,8 @@ class WorkspaceAutoGen(AutoGen):
 )
 Count += 1
 ## Generate fds command
-def _GenFdsCommand(self):
+@property
+def GenFdsCommand(self):
 return 
(GenMake.TopLevelMakefile(self)._TEMPLATE_.Replace(GenMake.TopLevelMakefile(self)._TemplateDict)).strip()
 
 ## Create makefile for the platform and modules in it
@@ -966,18 +965,6 @@ class WorkspaceAutoGen(AutoGen):
 def CreateAsBuiltInf(self):
 return
 
-Name= property(_GetName)
-Guid= property(_GetGuid)
-Version = property(_GetVersion)
-OutputDir   = property(_GetOutputDir)
-
-ToolDefinition  = property(_GetToolDefinition)   # toolcode : tool 
path
-
-BuildDir= property(_GetBuildDir)
-FvDir   = property(_GetFvDir)
-MakeFileDir = property(_GetMakeFileDir)
-BuildCommand= property(_GetBuildCommand)
-GenFdsCommand   = property(_GenFdsCommand)
 
 ## AutoGen class for platform
 #
@@ -2587,7 +2574,7 @@ class ModuleAutoGen(AutoGen):
 ## Return the module build data object
 @cached_property
 def Module(self):
-return self.Workspace.BuildDatabase[self.MetaFile, self.Arch, 
self.BuildTarget, self.ToolChain]
+return self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget, 
self.ToolChain]
 
 ## Return the 

[edk2] [PATCH v2 0/9] BaseTools: refactor Workspace classes

2018-09-10 Thread Jaben Carsey
update the classes for the following:
1) use decorators for property
2) use decorators for caching property and caching function
  - this allows for objects to reduce in size as they get used
3) remove unused variables and properties
4) use tuple instead of custom class when apropriate
5) remove callers from accessing "private" data and use the existing 
properties
6) removed a circular dependency between APIs

v2:
fix error where class attribute M was accidentally removed.

Jaben Carsey (9):
  BaseTools: Refactor PlatformAutoGen
  BaseTools: AutoGen refactor WorkspaceAutoGen class
  BaseTools: AutoGen - refactor class properties
  BaseTools: refactor class properties
  BaseTools: Workspace classes refactor properties
  BaseTools: refactor Build Database objects
  BaseTools: Don't save unused workspace data
  BaseTools: refactor to not overcreate ModuleAutoGen objects
  BaseTools: refactor to cache InfBuildData data

 BaseTools/Source/Python/AutoGen/AutoGen.py | 692 +++---
 BaseTools/Source/Python/AutoGen/GenMake.py |  20 +-
 BaseTools/Source/Python/Common/Misc.py |  90 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |   4 +-
 BaseTools/Source/Python/Workspace/BuildClassObject.py  |  39 +-
 BaseTools/Source/Python/Workspace/DecBuildData.py  |  65 +-
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 151 ++--
 BaseTools/Source/Python/Workspace/InfBuildData.py  | 954 
+---
 BaseTools/Source/Python/Workspace/MetaFileParser.py|  18 +-
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py |  16 +-
 BaseTools/Source/Python/build/build.py |   4 +-
 11 files changed, 933 insertions(+), 1120 deletions(-)

-- 
2.16.2.windows.1

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


[edk2] [PATCH v2 5/9] BaseTools: Workspace classes refactor properties

2018-09-10 Thread Jaben Carsey
1) use decorators
2) also change some private functions to public when all callers are
external
3) change external callers to use functions instead of directly
accessing private data.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |   4 +-
 BaseTools/Source/Python/Workspace/DecBuildData.py  |  58 +++---
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 151 +++---
 BaseTools/Source/Python/Workspace/InfBuildData.py  | 212 
++--
 BaseTools/Source/Python/Workspace/MetaFileParser.py|  18 +-
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py |  16 +-
 BaseTools/Source/Python/build/build.py |   4 +-
 7 files changed, 228 insertions(+), 235 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py 
b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
index 56bb966698ad..6149bc81b4ef 100644
--- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
+++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
@@ -341,9 +341,7 @@ class FfsInfStatement(FfsInfStatementClassObject):
 self.InfModule = Inf
 self.PcdIsDriver = Inf.PcdIsDriver
 self.IsBinaryModule = Inf.IsBinaryModule
-Inf._GetDepex()
-Inf._GetDepexExpression()
-if len(Inf._Depex.data) > 0 and len(Inf._DepexExpression.data) > 0:
+if len(Inf.Depex.data) > 0 and len(Inf.DepexExpression.data) > 0:
 self.Depex = True
 
 GenFdsGlobalVariable.VerboseLogger("BaseName : %s" % self.BaseName)
diff --git a/BaseTools/Source/Python/Workspace/DecBuildData.py 
b/BaseTools/Source/Python/Workspace/DecBuildData.py
index 45beaebc63ef..1f74e898f2ef 100644
--- a/BaseTools/Source/Python/Workspace/DecBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DecBuildData.py
@@ -99,21 +99,22 @@ class DecBuildData(PackageBuildClassObject):
 self._CommonIncludes= None
 self._LibraryClasses= None
 self._Pcds  = None
-self.__Macros   = None
+self._MacroDict = None
 self._PrivateProtocols  = None
 self._PrivatePpis   = None
 self._PrivateGuids  = None
 self._PrivateIncludes   = None
 
 ## Get current effective macros
-def _GetMacros(self):
-if self.__Macros is None:
-self.__Macros = {}
-self.__Macros.update(GlobalData.gGlobalDefines)
-return self.__Macros
+@property
+def _Macros(self):
+if self._MacroDict is None:
+self._MacroDict = dict(GlobalData.gGlobalDefines)
+return self._MacroDict
 
 ## Get architecture
-def _GetArch(self):
+@property
+def Arch(self):
 return self._Arch
 
 ## Retrieve all information in [Defines] section
@@ -129,7 +130,8 @@ class DecBuildData(PackageBuildClassObject):
 self._Header = 'DUMMY'
 
 ## Retrieve package name
-def _GetPackageName(self):
+@property
+def PackageName(self):
 if self._PackageName is None:
 if self._Header is None:
 self._GetHeaderInfo()
@@ -138,7 +140,8 @@ class DecBuildData(PackageBuildClassObject):
 return self._PackageName
 
 ## Retrieve file guid
-def _GetFileGuid(self):
+@property
+def PackageName(self):
 if self._Guid is None:
 if self._Header is None:
 self._GetHeaderInfo()
@@ -147,7 +150,8 @@ class DecBuildData(PackageBuildClassObject):
 return self._Guid
 
 ## Retrieve package version
-def _GetVersion(self):
+@property
+def Version(self):
 if self._Version is None:
 if self._Header is None:
 self._GetHeaderInfo()
@@ -156,7 +160,8 @@ class DecBuildData(PackageBuildClassObject):
 return self._Version
 
 ## Retrieve protocol definitions (name/value pairs)
-def _GetProtocol(self):
+@property
+def Protocols(self):
 if self._Protocols is None:
 #
 # tdict is a special kind of dict, used for selecting correct
@@ -198,7 +203,8 @@ class DecBuildData(PackageBuildClassObject):
 return self._Protocols
 
 ## Retrieve PPI definitions (name/value pairs)
-def _GetPpi(self):
+@property
+def Ppis(self):
 if self._Ppis is None:
 #
 # tdict is a special kind of dict, used for selecting correct
@@ -240,7 +246,8 @@ class DecBuildData(PackageBuildClassObject):
 return self._Ppis
 
 ## Retrieve GUID definitions (name/value pairs)
-def _GetGuid(self):
+@property
+def Guids(self):
 if self._Guids is None:
 #
 # tdict is a special kind of dict, used for selecting correct
@@ -282,7 +289,8 @@ class DecBuildData(PackageBuildClassObject):
 return self._Guids
 
 ## Retrieve public include paths declared in this 

[edk2] [PATCH v2 4/9] BaseTools: refactor class properties

2018-09-10 Thread Jaben Carsey
use decorators and auto cache those that were cached manually
remove properties never used

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/Misc.py | 90 +---
 1 file changed, 38 insertions(+), 52 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index 2cf9574326d5..372fdf01805b 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -38,6 +38,7 @@ from Common.LongFilePathSupport import OpenLongFilePath as 
open
 from Common.MultipleWorkspace import MultipleWorkspace as mws
 import uuid
 from CommonDataClass.Exceptions import BadExpression
+from Common.caching import cached_property
 import subprocess
 ## Regular expression used to find out place holders in string template
 gPlaceholderPattern = re.compile("\$\{([^$()\s]+)\}", re.MULTILINE | 
re.UNICODE)
@@ -1719,8 +1720,6 @@ class PathClass(object):
 self.ToolCode = ToolCode
 self.ToolChainFamily = ToolChainFamily
 
-self._Key = None
-
 ## Convert the object of this class to a string
 #
 #  Convert member Path of the class to a string
@@ -1773,12 +1772,12 @@ class PathClass(object):
 def __hash__(self):
 return hash(self.Path)
 
-def _GetFileKey(self):
-if self._Key is None:
-self._Key = self.Path.upper()   # + self.ToolChainFamily + 
self.TagName + self.ToolCode + self.Target
-return self._Key
+@cached_property
+def Key(self):
+return self.Path.upper()
 
-def _GetTimeStamp(self):
+@property
+def TimeStamp(self):
 return os.stat(self.Path)[8]
 
 def Validate(self, Type='', CaseSensitive=True):
@@ -1817,9 +1816,6 @@ class PathClass(object):
 self.Path = os.path.join(RealRoot, RealFile)
 return ErrorCode, ErrorInfo
 
-Key = property(_GetFileKey)
-TimeStamp = property(_GetTimeStamp)
-
 ## Parse PE image to get the required PE informaion.
 #
 class PeImageClass():
@@ -1929,8 +1925,8 @@ class DefaultStore():
 for sid, name in self.DefaultStores.values():
 if sid == minid:
 return name
+
 class SkuClass():
-
 DEFAULT = 0
 SINGLE = 1
 MULTIPLE =2
@@ -1951,8 +1947,8 @@ class SkuClass():
 self.SkuIdSet = []
 self.SkuIdNumberSet = []
 self.SkuData = SkuIds
-self.__SkuInherit = {}
-self.__SkuIdentifier = SkuIdentifier
+self._SkuInherit = {}
+self._SkuIdentifier = SkuIdentifier
 if SkuIdentifier == '' or SkuIdentifier is None:
 self.SkuIdSet = ['DEFAULT']
 self.SkuIdNumberSet = ['0U']
@@ -1976,7 +1972,7 @@ class SkuClass():
 EdkLogger.error("build", PARAMETER_INVALID,
 ExtraData="SKU-ID [%s] is not supported by the 
platform. [Valid SKU-ID: %s]"
   % (each, " | ".join(SkuIds.keys(
-if self.SkuUsageType != self.SINGLE:
+if self.SkuUsageType != SkuClass.SINGLE:
 self.AvailableSkuIds.update({'DEFAULT':0, 'COMMON':0})
 if self.SkuIdSet:
 GlobalData.gSkuids = (self.SkuIdSet)
@@ -1990,11 +1986,11 @@ class SkuClass():
 GlobalData.gSkuids.sort()
 
 def GetNextSkuId(self, skuname):
-if not self.__SkuInherit:
-self.__SkuInherit = {}
+if not self._SkuInherit:
+self._SkuInherit = {}
 for item in self.SkuData.values():
-self.__SkuInherit[item[1]]=item[2] if item[2] else "DEFAULT"
-return self.__SkuInherit.get(skuname, "DEFAULT")
+self._SkuInherit[item[1]]=item[2] if item[2] else "DEFAULT"
+return self._SkuInherit.get(skuname, "DEFAULT")
 
 def GetSkuChain(self, sku):
 if sku == "DEFAULT":
@@ -2024,55 +2020,45 @@ class SkuClass():
 
 return skuorder
 
-def __SkuUsageType(self):
-
-if self.__SkuIdentifier.upper() == "ALL":
+@property
+def SkuUsageType(self):
+if self._SkuIdentifier.upper() == "ALL":
 return SkuClass.MULTIPLE
 
 if len(self.SkuIdSet) == 1:
 if self.SkuIdSet[0] == 'DEFAULT':
 return SkuClass.DEFAULT
-else:
-return SkuClass.SINGLE
-elif len(self.SkuIdSet) == 2:
-if 'DEFAULT' in self.SkuIdSet:
-return SkuClass.SINGLE
-else:
-return SkuClass.MULTIPLE
-else:
-return SkuClass.MULTIPLE
+return SkuClass.SINGLE
+if len(self.SkuIdSet) == 2 and 'DEFAULT' in self.SkuIdSet:
+return SkuClass.SINGLE
+return SkuClass.MULTIPLE
+
 def DumpSkuIdArrary(self):
-
+if self.SkuUsageType == SkuClass.SINGLE:
+return "{0x0}"
 ArrayStrList = []
-if 

[edk2] [PATCH v2 9/9] BaseTools: refactor to cache InfBuildData data

2018-09-10 Thread Jaben Carsey
use Common.caching and auto cache properties and functions of InfBuildData

Cc: Liming Gao 
Cc: Yonghong Zhu 
Cc: Bob Feng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Workspace/InfBuildData.py | 840 +---
 1 file changed, 389 insertions(+), 451 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py 
b/BaseTools/Source/Python/Workspace/InfBuildData.py
index 0016cd30ce7a..44d44d24eb6b 100644
--- a/BaseTools/Source/Python/Workspace/InfBuildData.py
+++ b/BaseTools/Source/Python/Workspace/InfBuildData.py
@@ -13,14 +13,14 @@
 #
 
 from __future__ import absolute_import
-from Common.StringUtils import *
 from Common.DataType import *
 from Common.Misc import *
+from Common.caching import cached_property, cached_class_function
 from types import *
 from .MetaFileParser import *
 from collections import OrderedDict
-
 from Workspace.BuildClassObject import ModuleBuildClassObject, 
LibraryClassObject, PcdClassObject
+
 ## Module build information from INF file
 #
 #  This class is used to retrieve information stored in database and convert 
them
@@ -77,9 +77,9 @@ class InfBuildData(ModuleBuildClassObject):
 }
 
 
-## Constructor of DscBuildData
+## Constructor of InfBuildData
 #
-#  Initialize object of DscBuildData
+#  Initialize object of InfBuildData
 #
 #   @param  FilePathThe path of platform description file
 #   @param  RawData The raw data of DSC file
@@ -97,10 +97,37 @@ class InfBuildData(ModuleBuildClassObject):
 self._Target = Target
 self._Toolchain = Toolchain
 self._Platform = TAB_COMMON
-self._SourceOverridePath = None
 if FilePath.Key in GlobalData.gOverrideDir:
 self._SourceOverridePath = GlobalData.gOverrideDir[FilePath.Key]
-self._Clear()
+else:
+self._SourceOverridePath = None
+self._TailComments = None
+self._BaseName = None
+self._DxsFile = None
+self._ModuleType = None
+self._ComponentType = None
+self._BuildType = None
+self._Guid = None
+self._Version = None
+self._PcdIsDriver = None
+self._BinaryModule = None
+self._Shadow = None
+self._MakefileName = None
+self._CustomMakefile = None
+self._Specification = None
+self._LibraryClass = None
+self._ModuleEntryPointList = None
+self._ModuleUnloadImageList = None
+self._ConstructorList = None
+self._DestructorList = None
+self._Defs = OrderedDict()
+self._ProtocolComments = None
+self._PpiComments = None
+self._GuidsUsedByPcd = OrderedDict()
+self._GuidComments = None
+self._PcdComments = None
+self._BuildOptions = None
+self._DependencyFileList = None
 
 ## XXX[key] = value
 def __setitem__(self, key, value):
@@ -114,89 +141,39 @@ class InfBuildData(ModuleBuildClassObject):
 def __contains__(self, key):
 return key in self._PROPERTY_
 
-## Set all internal used members of InfBuildData to None
-def _Clear(self):
-self._HeaderComments = None
-self._TailComments = None
-self._Header_   = None
-self._AutoGenVersion= None
-self._BaseName  = None
-self._DxsFile   = None
-self._ModuleType= None
-self._ComponentType = None
-self._BuildType = None
-self._Guid  = None
-self._Version   = None
-self._PcdIsDriver   = None
-self._BinaryModule  = None
-self._Shadow= None
-self._MakefileName  = None
-self._CustomMakefile= None
-self._Specification = None
-self._LibraryClass  = None
-self._ModuleEntryPointList  = None
-self._ModuleUnloadImageList = None
-self._ConstructorList   = None
-self._DestructorList= None
-self._Defs  = OrderedDict()
-self._Binaries  = None
-self._Sources   = None
-self._LibraryClasses= None
-self._Libraries = None
-self._Protocols = None
-self._ProtocolComments  = None
-self._Ppis  = None
-self._PpiComments   = None
-self._Guids = None
-self._GuidsUsedByPcd= OrderedDict()
-self._GuidComments  = None
-self._Includes  = None
-self._Packages  = None
-self._Pcds  = None
-self._PcdComments   = None
-self._BuildOptions  = None
-self._Depex = None
-

[edk2] [PATCH v2 3/9] BaseTools: AutoGen - refactor class properties

2018-09-10 Thread Jaben Carsey
use function decorators

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/GenMake.py | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 35ee98c82bb1..b4377eef17d7 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -455,7 +455,8 @@ cleanlib:
 self.FfsOutputFileList = []
 
 # Compose a dict object containing information used to do replacement in 
template
-def _CreateTemplateDict(self):
+@property
+def _TemplateDict(self):
 if self._FileType not in self._SEP_:
 EdkLogger.error("build", PARAMETER_INVALID, "Invalid Makefile type 
[%s]" % self._FileType,
 ExtraData="[%s]" % str(self._AutoGenObject))
@@ -1095,8 +1096,6 @@ cleanlib:
 
 return DependencyList
 
-_TemplateDict = property(_CreateTemplateDict)
-
 ## CustomMakefile class
 #
 #  This class encapsules makefie and its generation for module. It uses 
template to generate
@@ -1205,7 +1204,8 @@ ${BEGIN}\t-@${create_directory_command}\n${END}\
 self.IntermediateDirectoryList = ["$(DEBUG_DIR)", "$(OUTPUT_DIR)"]
 
 # Compose a dict object containing information used to do replacement in 
template
-def _CreateTemplateDict(self):
+@property
+def _TemplateDict(self):
 Separator = self._SEP_[self._FileType]
 MyAgo = self._AutoGenObject
 if self._FileType not in MyAgo.CustomMakefile:
@@ -1278,8 +1278,6 @@ ${BEGIN}\t-@${create_directory_command}\n${END}\
 
 return MakefileTemplateDict
 
-_TemplateDict = property(_CreateTemplateDict)
-
 ## PlatformMakefile class
 #
 #  This class encapsules makefie and its generation for platform. It uses
@@ -1396,7 +1394,8 @@ cleanlib:
 self.LibraryMakeCommandList = []
 
 # Compose a dict object containing information used to do replacement in 
template
-def _CreateTemplateDict(self):
+@property
+def _TemplateDict(self):
 Separator = self._SEP_[self._FileType]
 
 MyAgo = self._AutoGenObject
@@ -1481,8 +1480,6 @@ cleanlib:
 DirList.append(os.path.join(self._AutoGenObject.BuildDir, 
LibraryAutoGen.BuildDir))
 return DirList
 
-_TemplateDict = property(_CreateTemplateDict)
-
 ## TopLevelMakefile class
 #
 #  This class encapsules makefie and its generation for entrance makefile. It
@@ -1502,7 +1499,8 @@ class TopLevelMakefile(BuildFile):
 self.IntermediateDirectoryList = []
 
 # Compose a dict object containing information used to do replacement in 
template
-def _CreateTemplateDict(self):
+@property
+def _TemplateDict(self):
 Separator = self._SEP_[self._FileType]
 
 # any platform autogen object is ok because we just need common 
information
@@ -1622,8 +1620,6 @@ class TopLevelMakefile(BuildFile):
 DirList.append(os.path.join(self._AutoGenObject.BuildDir, 
LibraryAutoGen.BuildDir))
 return DirList
 
-_TemplateDict = property(_CreateTemplateDict)
-
 # This acts like the main() function for the script, unless it is 'import'ed 
into another script.
 if __name__ == '__main__':
 pass
-- 
2.16.2.windows.1

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


[edk2] [PATCH v2 1/9] BaseTools: Refactor PlatformAutoGen

2018-09-10 Thread Jaben Carsey
use decorators for property and automatic caching
remove circular dependency between some APIs

Cc: Liming Gao 
Cc: Yonghong Zhu 
Cc: Bob Feng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 587 +---
 1 file changed, 264 insertions(+), 323 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 95370d182157..726b1c98f5bc 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1046,41 +1046,13 @@ class PlatformAutoGen(AutoGen):
 # get the original module/package/platform objects
 self.BuildDatabase = Workspace.BuildDatabase
 self.DscBuildDataObj = Workspace.Platform
-self._GuidDict = Workspace._GuidDict
 
 # flag indicating if the makefile/C-code file has been created or not
 self.IsMakeFileCreated  = False
-self.IsCodeFileCreated  = False
 
-self._Platform   = None
-self._Name   = None
-self._Guid   = None
-self._Version= None
-
-self._BuildRule = None
-self._SourceDir = None
-self._BuildDir = None
-self._OutputDir = None
-self._FvDir = None
-self._MakeFileDir = None
-self._FdfFile = None
-
-self._PcdTokenNumber = None# (TokenCName, TokenSpaceGuidCName) : 
GeneratedTokenNumber
 self._DynamicPcdList = None# [(TokenCName1, TokenSpaceGuidCName1), 
(TokenCName2, TokenSpaceGuidCName2), ...]
 self._NonDynamicPcdList = None # [(TokenCName1, TokenSpaceGuidCName1), 
(TokenCName2, TokenSpaceGuidCName2), ...]
-self._NonDynamicPcdDict = {}
 
-self._ToolDefinitions = None
-self._ToolDefFile = None  # toolcode : tool path
-self._ToolChainFamily = None
-self._BuildRuleFamily = None
-self._BuildOption = None  # toolcode : option
-self._EdkBuildOption = None   # edktoolcode : option
-self._EdkIIBuildOption = None # edkiitoolcode : option
-self._PackageList = None
-self._ModuleAutoGenList  = None
-self._LibraryAutoGenList = None
-self._BuildCommand = None
 self._AsBuildInfList = []
 self._AsBuildModuleList = []
 
@@ -1100,6 +1072,7 @@ class PlatformAutoGen(AutoGen):
 
 return True
 
+@cached_class_function
 def __repr__(self):
 return "%s [%s]" % (self.MetaFile, self.Arch)
 
@@ -,19 +1084,18 @@ class PlatformAutoGen(AutoGen):
 #   @param  CreateModuleCodeFileFlag indicating if creating 
module's
 #   autogen code file or not
 #
+@cached_class_function
 def CreateCodeFile(self, CreateModuleCodeFile=False):
 # only module has code to be greated, so do nothing if 
CreateModuleCodeFile is False
-if self.IsCodeFileCreated or not CreateModuleCodeFile:
+if not CreateModuleCodeFile:
 return
 
 for Ma in self.ModuleAutoGenList:
 Ma.CreateCodeFile(True)
 
-# don't do this twice
-self.IsCodeFileCreated = True
-
 ## Generate Fds Command
-def _GenFdsCommand(self):
+@cached_property
+def GenFdsCommand(self):
 return self.Workspace.GenFdsCommand
 
 ## Create makefile for the platform and mdoules in it
@@ -1185,7 +1157,6 @@ class PlatformAutoGen(AutoGen):
 LibAuto.ConstPcd[key] = FixedAtBuildPcds[key]
 
 def CollectVariables(self, DynamicPcdSet):
-
 VpdRegionSize = 0
 VpdRegionBase = 0
 if self.Workspace.FdfFile:
@@ -1197,7 +1168,6 @@ class PlatformAutoGen(AutoGen):
 VpdRegionBase = FdRegion.Offset
 break
 
-
 VariableInfo = VariableMgr(self.DscBuildDataObj._GetDefaultStores(), 
self.DscBuildDataObj._GetSkuIds())
 VariableInfo.SetVpdRegionMaxSize(VpdRegionSize)
 VariableInfo.SetVpdRegionOffset(VpdRegionBase)
@@ -1250,7 +1220,6 @@ class PlatformAutoGen(AutoGen):
 #  This interface should be invoked explicitly when platform action is 
created.
 #
 def CollectPlatformDynamicPcds(self):
-
 for key in self.Platform.Pcds:
 for SinglePcd in GlobalData.MixedPcd:
 if (self.Platform.Pcds[key].TokenCName, 
self.Platform.Pcds[key].TokenSpaceGuidCName) == SinglePcd:
@@ -1683,308 +1652,318 @@ class PlatformAutoGen(AutoGen):
 EdkLogger.error("Build", FILE_NOT_FOUND, "Fail to find 
third-party BPDG tool to process VPD PCDs. BPDG Guid tool need to be defined in 
tools_def.txt and VPD_TOOL_GUID need to be provided in DSC file.")
 
 ## Return the platform build data object
-def _GetPlatform(self):
-if self._Platform is None:
-self._Platform = self.BuildDatabase[self.MetaFile, self.Arch, 
self.BuildTarget, self.ToolChain]
-   

[edk2] [PATCH v2 8/9] BaseTools: refactor to not overcreate ModuleAutoGen objects

2018-09-10 Thread Jaben Carsey
currently created for 3 different purposes and saved once.
this makes it created once and saved and then referenced.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Cc: Bob Feng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 58 +---
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 35dab75785eb..4e9fc54dbaf3 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1085,21 +1085,19 @@ class PlatformAutoGen(AutoGen):
 def GenFdsCommand(self):
 return self.Workspace.GenFdsCommand
 
-## Create makefile for the platform and mdoules in it
+## Create makefile for the platform and modules in it
 #
 #   @param  CreateModuleMakeFileFlag indicating if the makefile for
 #   modules will be created as well
 #
 def CreateMakeFile(self, CreateModuleMakeFile=False, FfsCommand = {}):
 if CreateModuleMakeFile:
-for ModuleFile in self.Platform.Modules:
-Ma = ModuleAutoGen(self.Workspace, ModuleFile, 
self.BuildTarget,
-   self.ToolChain, self.Arch, self.MetaFile)
-if (ModuleFile.File, self.Arch) in FfsCommand:
-Ma.CreateMakeFile(True, FfsCommand[ModuleFile.File, 
self.Arch])
+for Ma in self._MaList:
+key = (Ma.MetaFile.File, self.Arch)
+if key in FfsCommand:
+Ma.CreateMakeFile(True, FfsCommand[key])
 else:
 Ma.CreateMakeFile(True)
-#Ma.CreateAsBuiltInf()
 
 # no need to create makefile for the platform more than once
 if self.IsMakeFileCreated:
@@ -1231,16 +1229,12 @@ class PlatformAutoGen(AutoGen):
 for InfName in self._AsBuildInfList:
 InfName = mws.join(self.WorkspaceDir, InfName)
 FdfModuleList.append(os.path.normpath(InfName))
-for F in self.Platform.Modules.keys():
-M = ModuleAutoGen(self.Workspace, F, self.BuildTarget, 
self.ToolChain, self.Arch, self.MetaFile)
-#GuidValue.update(M.Guids)
-
-self.Platform.Modules[F].M = M
-
+for M in self._MaList:
+#F is the Module for which M is the module autogen
 for PcdFromModule in M.ModulePcdList + M.LibraryPcdList:
 # make sure that the "VOID*" kind of datum has MaxDatumSize set
 if PcdFromModule.DatumType == TAB_VOID and not 
PcdFromModule.MaxDatumSize:
-NoDatumTypePcdList.add("%s.%s [%s]" % 
(PcdFromModule.TokenSpaceGuidCName, PcdFromModule.TokenCName, F))
+NoDatumTypePcdList.add("%s.%s [%s]" % 
(PcdFromModule.TokenSpaceGuidCName, PcdFromModule.TokenCName, M.MetaFile))
 
 # Check the PCD from Binary INF or Source INF
 if M.IsBinaryModule == True:
@@ -1250,7 +1244,7 @@ class PlatformAutoGen(AutoGen):
 PcdFromModule.IsFromDsc = (PcdFromModule.TokenCName, 
PcdFromModule.TokenSpaceGuidCName) in self.Platform.Pcds
 
 if PcdFromModule.Type in PCD_DYNAMIC_TYPE_SET or 
PcdFromModule.Type in PCD_DYNAMIC_EX_TYPE_SET:
-if F.Path not in FdfModuleList:
+if M.MetaFile.Path not in FdfModuleList:
 # If one of the Source built modules listed in the DSC 
is not listed
 # in FDF modules, and the INF lists a PCD can only use 
the PcdsDynamic
 # access method (it is only listed in the DEC file 
that declares the
@@ -1930,19 +1924,25 @@ class PlatformAutoGen(AutoGen):
 TokenNumber += 1
 return RetVal
 
+@cached_property
+def _MaList(self):
+for ModuleFile in self.Platform.Modules:
+Ma = ModuleAutoGen(
+  self.Workspace,
+  ModuleFile,
+  self.BuildTarget,
+  self.ToolChain,
+  self.Arch,
+  self.MetaFile
+  )
+self.Platform.Modules[ModuleFile].M = Ma
+return [x.M for x in self.Platform.Modules.values()]
+
 ## Summarize ModuleAutoGen objects of all modules to be built for this 
platform
 @cached_property
 def ModuleAutoGenList(self):
 RetVal = []
-for ModuleFile in self.Platform.Modules:
-Ma = ModuleAutoGen(
-self.Workspace,
-ModuleFile,
-self.BuildTarget,
-self.ToolChain,
-self.Arch,
-self.MetaFile
-)
+for Ma in self._MaList:
 if Ma not in RetVal:
 RetVal.append(Ma)
 return RetVal
@@ 

[edk2] [PATCH v2 7/9] BaseTools: Don't save unused workspace data

2018-09-10 Thread Jaben Carsey
  FlexibleFieldName was never used not set.
  DefinitionPosition (file and line number) are recalculated
and never used outside the function.  remove the saving of the
data.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Workspace/BuildClassObject.py | 7 ---
 BaseTools/Source/Python/Workspace/DecBuildData.py | 7 ---
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py 
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index e7259b2d3d50..f4ffd05324f4 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -66,7 +66,6 @@ class PcdClassObject(object):
 self.DscDefaultValue = Value
 self.PcdValueFromComm = ""
 self.PcdValueFromFdf = ""
-self.DefinitionPosition = ("","")
 
 ## Get the maximum number of bytes
 def GetPcdMaxSize(self):
@@ -168,7 +167,6 @@ class StructurePcd(PcdClassObject):
 self.DefaultValues = OrderedDict()
 self.PcdMode = None
 self.SkuOverrideValues = OrderedDict()
-self.FlexibleFieldName = None
 self.StructName = None
 self.PcdDefineLineNo = 0
 self.PkgPath = ""
@@ -200,9 +198,6 @@ class StructurePcd(PcdClassObject):
 def SetPcdMode (self, PcdMode):
 self.PcdMode = PcdMode
 
-def SetFlexibleFieldName (self, FlexibleFieldName):
-self.FlexibleFieldName = FlexibleFieldName
-
 def copy(self, PcdObject):
 self.TokenCName = PcdObject.TokenCName if PcdObject.TokenCName else 
self.TokenCName
 self.TokenSpaceGuidCName = PcdObject.TokenSpaceGuidCName if 
PcdObject.TokenSpaceGuidCName else PcdObject.TokenSpaceGuidCName
@@ -224,7 +219,6 @@ class StructurePcd(PcdClassObject):
 self.DscRawValue = PcdObject.DscRawValue if PcdObject.DscRawValue else 
self.DscRawValue
 self.PcdValueFromComm = PcdObject.PcdValueFromComm if 
PcdObject.PcdValueFromComm else self.PcdValueFromComm
 self.PcdValueFromFdf = PcdObject.PcdValueFromFdf if 
PcdObject.PcdValueFromFdf else self.PcdValueFromFdf
-self.DefinitionPosition = PcdObject.DefinitionPosition if 
PcdObject.DefinitionPosition else self.DefinitionPosition
 if isinstance(PcdObject, StructurePcd):
 self.StructuredPcdIncludeFile = PcdObject.StructuredPcdIncludeFile 
if PcdObject.StructuredPcdIncludeFile else self.StructuredPcdIncludeFile
 self.PackageDecs = PcdObject.PackageDecs if PcdObject.PackageDecs 
else self.PackageDecs
@@ -233,7 +227,6 @@ class StructurePcd(PcdClassObject):
 self.DefaultFromDSC=None
 self.DefaultValueFromDec = PcdObject.DefaultValueFromDec if 
PcdObject.DefaultValueFromDec else self.DefaultValueFromDec
 self.SkuOverrideValues = PcdObject.SkuOverrideValues if 
PcdObject.SkuOverrideValues else self.SkuOverrideValues
-self.FlexibleFieldName = PcdObject.FlexibleFieldName if 
PcdObject.FlexibleFieldName else self.FlexibleFieldName
 self.StructName = PcdObject.DatumType if PcdObject.DatumType else 
self.StructName
 self.PcdDefineLineNo = PcdObject.PcdDefineLineNo if 
PcdObject.PcdDefineLineNo else self.PcdDefineLineNo
 self.PkgPath = PcdObject.PkgPath if PcdObject.PkgPath else 
self.PkgPath
diff --git a/BaseTools/Source/Python/Workspace/DecBuildData.py 
b/BaseTools/Source/Python/Workspace/DecBuildData.py
index 1f74e898f2ef..31ee13eca91f 100644
--- a/BaseTools/Source/Python/Workspace/DecBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DecBuildData.py
@@ -410,6 +410,7 @@ class DecBuildData(PackageBuildClassObject):
 if not (PcdCName, TokenSpaceGuid) in PcdSet:
 PcdSet.append((PcdCName, TokenSpaceGuid))
 
+DefinitionPosition = {}
 for PcdCName, TokenSpaceGuid in PcdSet:
 #
 # limit the ARCH to self._Arch, if no self._Arch found, tdict
@@ -436,7 +437,7 @@ class DecBuildData(PackageBuildClassObject):
 list(validlists),
 list(expressions)
 )
-PcdObj.DefinitionPosition = (self.MetaFile.File, LineNo)
+DefinitionPosition[PcdObj] = (self.MetaFile.File, LineNo)
 if "." in TokenSpaceGuid:
 StrPcdSet.append((PcdObj, LineNo))
 else:
@@ -449,10 +450,10 @@ class DecBuildData(PackageBuildClassObject):
 for pcd in Pcds.values():
 if pcd.DatumType not in [TAB_UINT8, TAB_UINT16, TAB_UINT32, 
TAB_UINT64, TAB_VOID, "BOOLEAN"]:
 if StructPattern.match(pcd.DatumType) is None:
-EdkLogger.error('build', FORMAT_INVALID, "DatumType only 
support BOOLEAN, UINT8, UINT16, UINT32, UINT64, VOID* or a valid struct name.", 

[edk2] [PATCH v2 6/9] BaseTools: refactor Build Database objects

2018-09-10 Thread Jaben Carsey
1) use namedtuple instead of custom class when apropriate
2) rename collections.OrderedDict to OrderedDict since we import it already

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Workspace/BuildClassObject.py | 32 ++--
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py 
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index 88465c59ea30..e7259b2d3d50 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -11,8 +11,8 @@
 # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
 
+from collections import OrderedDict, namedtuple
 from Common.DataType import *
-import collections
 
 ## PcdClassObject
 #
@@ -165,17 +165,17 @@ class StructurePcd(PcdClassObject):
 self.StructuredPcdIncludeFile = [] if StructuredPcdIncludeFile is None 
else StructuredPcdIncludeFile
 self.PackageDecs = Packages
 self.DefaultStoreName = [default_store]
-self.DefaultValues = collections.OrderedDict()
+self.DefaultValues = OrderedDict()
 self.PcdMode = None
-self.SkuOverrideValues = collections.OrderedDict()
+self.SkuOverrideValues = OrderedDict()
 self.FlexibleFieldName = None
 self.StructName = None
 self.PcdDefineLineNo = 0
 self.PkgPath = ""
 self.DefaultValueFromDec = ""
 self.ValueChain = set()
-self.PcdFieldValueFromComm = collections.OrderedDict()
-self.PcdFieldValueFromFdf = collections.OrderedDict()
+self.PcdFieldValueFromComm = OrderedDict()
+self.PcdFieldValueFromFdf = OrderedDict()
 def __repr__(self):
 return self.TypeName
 
@@ -189,9 +189,9 @@ class StructurePcd(PcdClassObject):
 self.DefaultValueFromDec = DefaultValue
 def AddOverrideValue (self, FieldName, Value, SkuName, DefaultStoreName, 
FileName="", LineNo=0):
 if SkuName not in self.SkuOverrideValues:
-self.SkuOverrideValues[SkuName] = collections.OrderedDict()
+self.SkuOverrideValues[SkuName] = OrderedDict()
 if DefaultStoreName not in self.SkuOverrideValues[SkuName]:
-self.SkuOverrideValues[SkuName][DefaultStoreName] = 
collections.OrderedDict()
+self.SkuOverrideValues[SkuName][DefaultStoreName] = OrderedDict()
 if FieldName in self.SkuOverrideValues[SkuName][DefaultStoreName]:
 del self.SkuOverrideValues[SkuName][DefaultStoreName][FieldName]
 self.SkuOverrideValues[SkuName][DefaultStoreName][FieldName] = 
[Value.strip(), FileName, LineNo]
@@ -241,21 +241,7 @@ class StructurePcd(PcdClassObject):
 self.PcdFieldValueFromComm = PcdObject.PcdFieldValueFromComm if 
PcdObject.PcdFieldValueFromComm else self.PcdFieldValueFromComm
 self.PcdFieldValueFromFdf = PcdObject.PcdFieldValueFromFdf if 
PcdObject.PcdFieldValueFromFdf else self.PcdFieldValueFromFdf
 
-## LibraryClassObject
-#
-# This Class defines LibraryClassObject used in BuildDatabase
-#
-# @param object:  Inherited from object class
-# @param Name:Input value for LibraryClassName, default is None
-# @param SupModList:  Input value for SupModList, default is []
-#
-# @var LibraryClass:  To store value for LibraryClass
-# @var SupModList:To store value for SupModList
-#
-class LibraryClassObject(object):
-def __init__(self, Name = None, SupModList = []):
-self.LibraryClass = Name
-self.SupModList = SupModList
+LibraryClassObject = namedtuple('LibraryClassObject', 
['LibraryClass','SupModList'], verbose=False)
 
 ## ModuleBuildClassObject
 #
@@ -323,7 +309,7 @@ class ModuleBuildClassObject(object):
 
 self.Binaries= []
 self.Sources = []
-self.LibraryClasses  = collections.OrderedDict()
+self.LibraryClasses  = OrderedDict()
 self.Libraries   = []
 self.Protocols   = []
 self.Ppis= []
-- 
2.16.2.windows.1

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


[edk2] [PATCH v1 1/1] BaseTools\GenFds: remove extra content

2018-09-10 Thread Jaben Carsey
remove uncalled functions
remove extra blank lines
remove commented out code

Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/GenFds/FdfParser.py | 56 
 1 file changed, 56 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 7e1be659fca5..1f7d59bb51b0 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -1120,28 +1120,6 @@ class FdfParser:
 else:
 return False
 
-def __GetNextOp(self):
-# Skip leading spaces, if exist.
-self.__SkipWhiteSpace()
-if self.__EndOfFile():
-return False
-# Record the token start position, the position of the first non-space 
char.
-StartPos = self.CurrentOffsetWithinLine
-while not self.__EndOfLine():
-TempChar = self.__CurrentChar()
-# Try to find the end char that is not a space
-if not str(TempChar).isspace():
-self.__GetOneChar()
-else:
-break
-else:
-return False
-
-if StartPos != self.CurrentOffsetWithinLine:
-self.__Token = self.__CurrentLine()[StartPos : 
self.CurrentOffsetWithinLine]
-return True
-else:
-return False
 ## __GetNextGuid() method
 #
 #   Get next token unit before a seperator
@@ -1247,28 +1225,6 @@ class FdfParser:
 self.__UndoToken()
 return False
 
-## __GetNextPcdName() method
-#
-#   Get next PCD token space C name and PCD C name pair before a seperator
-#   If found, the decimal data is put into self.__Token
-#
-#   @param  selfThe object pointer
-#   @retval Tuple   PCD C name and PCD token space C name pair
-#
-def __GetNextPcdName(self):
-if not self.__GetNextWord():
-raise Warning("expected format of 
.", self.FileName, self.CurrentLineNumber)
-pcdTokenSpaceCName = self.__Token
-
-if not self.__IsToken( "."):
-raise Warning("expected format of 
.", self.FileName, self.CurrentLineNumber)
-
-if not self.__GetNextWord():
-raise Warning("expected format of 
.", self.FileName, self.CurrentLineNumber)
-pcdCName = self.__Token
-
-return (pcdCName, pcdTokenSpaceCName)
-
 def __GetNextPcdSettings(self):
 if not self.__GetNextWord():
 raise Warning("expected format of 
.", self.FileName, self.CurrentLineNumber)
@@ -3681,7 +3637,6 @@ class FdfParser:
   ModuleType.upper() + \
   '.'+ \
   TemplateName.upper() ] = RuleObj
-#self.Profile.RuleList.append(rule)
 return True
 
 ## __GetModuleType() method
@@ -4139,7 +4094,6 @@ class FdfParser:
 #   @retval False   Not able to find section statement
 #
 def __GetRuleEncapsulationSection(self, Rule):
-
 if self.__IsKeyword( "COMPRESS"):
 Type = "PI_STD"
 if self.__IsKeyword("PI_STD") or self.__IsKeyword("PI_NONE"):
@@ -4207,7 +4161,6 @@ class FdfParser:
 #   @retval False   Not able to find a VTF
 #
 def __GetVtf(self):
-
 if not self.__GetNextToken():
 return False
 
@@ -4279,7 +4232,6 @@ class FdfParser:
 #   @retval False   Not able to find a component
 #
 def __GetComponentStatement(self, VtfObj):
-
 if not self.__IsKeyword("COMP_NAME"):
 return False
 
@@ -4413,7 +4365,6 @@ class FdfParser:
 #   @retval False   Not able to find a OptionROM
 #
 def __GetOptionRom(self):
-
 if not self.__GetNextToken():
 return False
 
@@ -4454,7 +4405,6 @@ class FdfParser:
 #   @retval False   Not able to find inf statement
 #
 def __GetOptRomInfStatement(self, Obj):
-
 if not self.__IsKeyword( "INF"):
 return False
 
@@ -4557,7 +4507,6 @@ class FdfParser:
 #   @retval False   Not able to find FILE statement
 #
 def __GetOptRomFileStatement(self, Obj):
-
 if not self.__IsKeyword( "FILE"):
 return False
 
@@ -4592,7 +4541,6 @@ class FdfParser:
 #   @retval CapList List of Capsule in FD
 #
 def __GetCapInFd (self, FdName):
-
 CapList = []
 if FdName.upper() in self.Profile.FdDict:
 FdObj = self.Profile.FdDict[FdName.upper()]
@@ -4615,7 +4563,6 @@ class FdfParser:
 #   @param  RefFvList   referenced FV by section
 #
 def __GetReferencedFdCapTuple(self, CapObj, RefFdList = [], RefFvList = 
[]):
-
 for CapsuleDataObj in CapObj.CapsuleDataList :
 if hasattr(CapsuleDataObj, 'FvName') and CapsuleDataObj.FvName is 
not None and 

Re: [edk2] [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set

2018-09-10 Thread Ni, Ruiyu
Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, September 11, 2018 10:04 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Ni, Ruiyu ; Wang,
> Jian J ; Wang, Fei1 
> Subject: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is
> set
> 
> When the HSEE in the USBCMD bit is a '1' and the HSE bit in the USBSTS
> register is a '1', the xHC shall assert out-of-band error signaling to the 
> host
> and assert the SERR# pin.
> To prevent masking any potential issues with SERR, this patch is to set
> USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable Bit is
> set.
> 
> Cc: Ruiyu Ni 
> Cc: Jian J Wang 
> Cc: Fei1 Wang 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 41
> ++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> index 5f0736a516b6..89f073e1d83f 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> @@ -587,6 +587,39 @@ XhcIsSysError (
>  }
> 
>  /**
> +  Set USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable
> Bit is set.
> +
> +  The USBCMD HSEE Bit will be reset to default 0 by USBCMD Host Controller
> Reset(HCRST).
> +  This function is to set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set.
> +
> +  @param XhcThe XHCI Instance.
> +
> +**/
> +VOID
> +XhcSetHsee (
> +  IN USB_XHCI_INSTANCE  *Xhc
> +  )
> +{
> +  EFI_STATUSStatus;
> +  EFI_PCI_IO_PROTOCOL   *PciIo;
> +  UINT16XhciCmd;
> +
> +  PciIo = Xhc->PciIo;
> +  Status = PciIo->Pci.Read (
> +PciIo,
> +EfiPciIoWidthUint16,
> +PCI_COMMAND_OFFSET,
> +sizeof (XhciCmd),
> +
> +);
> +  if (!EFI_ERROR (Status)) {
> +if ((XhciCmd & EFI_PCI_COMMAND_SERR) != 0) {
> +  XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_HSEE);
> +}
> +  }
> +}
> +
> +/**
>Reset the XHCI host controller.
> 
>@param  Xhc  The XHCI Instance.
> @@ -628,6 +661,14 @@ XhcResetHC (
>  //
>  gBS->Stall (XHC_1_MILLISECOND);
>  Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET,
> XHC_USBCMD_RESET, FALSE, Timeout);
> +
> +if (!EFI_ERROR (Status)) {
> +  //
> +  // The USBCMD HSEE Bit will be reset to default 0 by USBCMD HCRST.
> +  // Set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set.
> +  //
> +  XhcSetHsee (Xhc);
> +}
>}
> 
>return Status;
> --
> 2.7.0.windows.1

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


[edk2] [Patch] BaseTools: Report error for incorrect hex value format

2018-09-10 Thread Yonghong Zhu
From: zhijufan 

The case is user use 0x1} as a hex value for Pcd, it directly cause
tool report traceback info. This patch add more error info.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/Common/Misc.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index 2cf9574..430bf6b 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -1412,11 +1412,14 @@ def ParseFieldValue (Value):
 if Value.startswith('DEVICE_PATH(') and Value.endswith(')'):
 Value = Value.replace("DEVICE_PATH(", '').rstrip(')')
 Value = Value.strip().strip('"')
 return ParseDevPathValue(Value)
 if Value.lower().startswith('0x'):
-Value = int(Value, 16)
+try:
+Value = int(Value, 16)
+except:
+raise BadExpression("invalid hex value: %s" % Value)
 if Value == 0:
 return 0, 1
 return Value, (Value.bit_length() + 7) / 8
 if Value[0].isdigit():
 Value = int(Value, 10)
-- 
2.6.1.windows.1

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


[edk2] [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set

2018-09-10 Thread Star Zeng
When the HSEE in the USBCMD bit is a ‘1’ and the HSE bit in the USBSTS
register is a ‘1’, the xHC shall assert out-of-band error signaling to
the host and assert the SERR# pin.
To prevent masking any potential issues with SERR, this patch is to set
USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable Bit is
set.

Cc: Ruiyu Ni 
Cc: Jian J Wang 
Cc: Fei1 Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 41 ++
 1 file changed, 41 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
index 5f0736a516b6..89f073e1d83f 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
@@ -587,6 +587,39 @@ XhcIsSysError (
 }
 
 /**
+  Set USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable Bit is 
set.
+
+  The USBCMD HSEE Bit will be reset to default 0 by USBCMD Host Controller 
Reset(HCRST).
+  This function is to set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set.
+
+  @param XhcThe XHCI Instance.
+
+**/
+VOID
+XhcSetHsee (
+  IN USB_XHCI_INSTANCE  *Xhc
+  )
+{
+  EFI_STATUSStatus;
+  EFI_PCI_IO_PROTOCOL   *PciIo;
+  UINT16XhciCmd;
+
+  PciIo = Xhc->PciIo;
+  Status = PciIo->Pci.Read (
+PciIo,
+EfiPciIoWidthUint16,
+PCI_COMMAND_OFFSET,
+sizeof (XhciCmd),
+
+);
+  if (!EFI_ERROR (Status)) {
+if ((XhciCmd & EFI_PCI_COMMAND_SERR) != 0) {
+  XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_HSEE);
+}
+  }
+}
+
+/**
   Reset the XHCI host controller.
 
   @param  Xhc  The XHCI Instance.
@@ -628,6 +661,14 @@ XhcResetHC (
 //
 gBS->Stall (XHC_1_MILLISECOND);
 Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_RESET, FALSE, 
Timeout);
+
+if (!EFI_ERROR (Status)) {
+  //
+  // The USBCMD HSEE Bit will be reset to default 0 by USBCMD HCRST.
+  // Set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set.
+  //
+  XhcSetHsee (Xhc);
+}
   }
 
   return Status;
-- 
2.7.0.windows.1

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


Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

2018-09-10 Thread Ni, Ruiyu
The reason I didn't remove the GCC version is because Liming told me that there 
is a XCODE issue which prevents using NASM for library.

Thanks/Ray

> -Original Message-
> From: Ni, Ruiyu
> Sent: Tuesday, September 11, 2018 10:25 AM
> To: Kinney, Michael D ; edk2-
> de...@lists.01.org
> Cc: Yao, Jiewen ; Gao, Liming
> 
> Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix
> Interlocked[De|In]crement return value
> 
> The NASM does the exactly same as the MSFT intrinsic.
> I'd like to remove them because I was almost lost in so many versions of
> Interlocked[De|In]crement.
> I'd like to merge them to one version.
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Tuesday, September 11, 2018 12:39 AM
> > To: Ni, Ruiyu ; edk2-devel@lists.01.org; Kinney,
> > Michael D 
> > Cc: Yao, Jiewen ; Gao, Liming
> > 
> > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix
> > Interlocked[De|In]crement return value
> >
> > Ray,
> >
> > Why are we removing the use of intrinsics from MSFT builds?  We should
> > keep those for more efficient code generation if the intrinsic has the
> > correct behavior.
> >
> > Thanks,
> >
> > Mike
> >
> > > -Original Message-
> > > From: Ni, Ruiyu
> > > Sent: Monday, September 10, 2018 3:06 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Yao, Jiewen ; Gao, Liming
> > > ; Kinney, Michael D
> > > 
> > > Subject: [PATCH] MdePkg/SynchronizationLib: fix
> > > Interlocked[De|In]crement return value
> > >
> > > Today's InterlockedIncrement()/InterlockedDecrement()
> > > guarantees to
> > > perform atomic increment/decrement but doesn't guarantee the return
> > > value equals to the new value.
> > >
> > > The patch fixes the behavior to use "XADD" instruction to guarantee
> > > the return value equals to the new value.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ruiyu Ni 
> > > Cc: Jiewen Yao 
> > > Cc: Liming Gao 
> > > Cc: Michael D Kinney 
> > > ---
> > >  MdePkg/Include/Library/SynchronizationLib.h|  6
> > > +--
> > >  .../BaseSynchronizationLib.inf | 18
> > > -
> > >  .../BaseSynchronizationLibInternals.h  |  6
> > > +--
> > >  .../BaseSynchronizationLib/Ia32/GccInline.c| 18
> > > -
> > >  .../Ia32/InterlockedDecrement.c| 42
> > > 
> > >  .../Ia32/InterlockedDecrement.nasm | 10
> > > ++---
> > >  .../Ia32/InterlockedIncrement.c| 43
> > > 
> > >  .../Ia32/InterlockedIncrement.nasm |  7
> > > ++--
> > >  .../BaseSynchronizationLib/Synchronization.c   |  6
> > > +--
> > >  .../BaseSynchronizationLib/SynchronizationGcc.c|  6
> > > +--
> > >  .../BaseSynchronizationLib/SynchronizationMsc.c|  6
> > > +--
> > >  .../Library/BaseSynchronizationLib/X64/GccInline.c | 19
> > > +
> > >  .../X64/InterlockedDecrement.c | 46
> > > --
> > >  .../X64/InterlockedDecrement.nasm  |  8
> > > ++--
> > >  .../X64/InterlockedIncrement.c | 46
> > > --
> > >  .../X64/InterlockedIncrement.nasm  |  5
> > > ++-
> > >  16 files changed, 52 insertions(+), 240 deletions(-)  delete mode
> > > 100644 MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDe
> > > crement.c
> > >  delete mode 100644
> > > MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIn
> > > crement.c
> > >  delete mode 100644
> > > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDec
> > > rement.c
> > >  delete mode 100644
> > > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedInc
> > > rement.c
> > >
> > > diff --git a/MdePkg/Include/Library/SynchronizationLib.h
> > > b/MdePkg/Include/Library/SynchronizationLib.h
> > > index da69f6ff5e..ce3bce04f5 100644
> > > --- a/MdePkg/Include/Library/SynchronizationLib.h
> > > +++ b/MdePkg/Include/Library/SynchronizationLib.h
> > > @@ -144,8 +144,7 @@ ReleaseSpinLock (
> > >
> > >Performs an atomic increment of the 32-bit unsigned integer
> > > specified by
> > >Value and returns the incremented value. The increment operation
> > > must be
> > > -  performed using MP safe mechanisms. The state of the return value
> > > is not
> > > -  guaranteed to be MP safe.
> > > +  performed using MP safe mechanisms.
> > >
> > >If Value is NULL, then ASSERT().
> > >
> > > @@ -166,8 +165,7 @@ InterlockedIncrement (
> > >
> > >Performs an atomic decrement of the 32-bit unsigned integer
> > > specified by
> > >Value and returns the decremented value. The decrement operation
> > > must be
> > > -  performed using MP safe mechanisms. The state of the return value
> > > is not
> > > -  guaranteed to be MP safe.
> > > +  performed using MP safe mechanisms.
> > >
> > >If Value is NULL, then ASSERT().
> > >
> > > diff --git
> > > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> > > ionLib.inf

Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

2018-09-10 Thread Ni, Ruiyu
The NASM does the exactly same as the MSFT intrinsic.
I'd like to remove them because I was almost lost in so many versions of 
Interlocked[De|In]crement.
I'd like to merge them to one version.

Thanks/Ray

> -Original Message-
> From: Kinney, Michael D
> Sent: Tuesday, September 11, 2018 12:39 AM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org; Kinney, Michael
> D 
> Cc: Yao, Jiewen ; Gao, Liming
> 
> Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix
> Interlocked[De|In]crement return value
> 
> Ray,
> 
> Why are we removing the use of intrinsics from MSFT builds?  We should
> keep those for more efficient code generation if the intrinsic has the correct
> behavior.
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Monday, September 10, 2018 3:06 AM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen ; Gao, Liming
> > ; Kinney, Michael D 
> > Subject: [PATCH] MdePkg/SynchronizationLib: fix
> > Interlocked[De|In]crement return value
> >
> > Today's InterlockedIncrement()/InterlockedDecrement()
> > guarantees to
> > perform atomic increment/decrement but doesn't guarantee the return
> > value equals to the new value.
> >
> > The patch fixes the behavior to use "XADD" instruction to guarantee
> > the return value equals to the new value.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ruiyu Ni 
> > Cc: Jiewen Yao 
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > ---
> >  MdePkg/Include/Library/SynchronizationLib.h|  6
> > +--
> >  .../BaseSynchronizationLib.inf | 18
> > -
> >  .../BaseSynchronizationLibInternals.h  |  6
> > +--
> >  .../BaseSynchronizationLib/Ia32/GccInline.c| 18
> > -
> >  .../Ia32/InterlockedDecrement.c| 42
> > 
> >  .../Ia32/InterlockedDecrement.nasm | 10
> > ++---
> >  .../Ia32/InterlockedIncrement.c| 43
> > 
> >  .../Ia32/InterlockedIncrement.nasm |  7
> > ++--
> >  .../BaseSynchronizationLib/Synchronization.c   |  6
> > +--
> >  .../BaseSynchronizationLib/SynchronizationGcc.c|  6
> > +--
> >  .../BaseSynchronizationLib/SynchronizationMsc.c|  6
> > +--
> >  .../Library/BaseSynchronizationLib/X64/GccInline.c | 19
> > +
> >  .../X64/InterlockedDecrement.c | 46
> > --
> >  .../X64/InterlockedDecrement.nasm  |  8
> > ++--
> >  .../X64/InterlockedIncrement.c | 46
> > --
> >  .../X64/InterlockedIncrement.nasm  |  5
> > ++-
> >  16 files changed, 52 insertions(+), 240 deletions(-)  delete mode
> > 100644 MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDe
> > crement.c
> >  delete mode 100644
> > MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIn
> > crement.c
> >  delete mode 100644
> > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDec
> > rement.c
> >  delete mode 100644
> > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedInc
> > rement.c
> >
> > diff --git a/MdePkg/Include/Library/SynchronizationLib.h
> > b/MdePkg/Include/Library/SynchronizationLib.h
> > index da69f6ff5e..ce3bce04f5 100644
> > --- a/MdePkg/Include/Library/SynchronizationLib.h
> > +++ b/MdePkg/Include/Library/SynchronizationLib.h
> > @@ -144,8 +144,7 @@ ReleaseSpinLock (
> >
> >Performs an atomic increment of the 32-bit unsigned integer
> > specified by
> >Value and returns the incremented value. The increment operation
> > must be
> > -  performed using MP safe mechanisms. The state of the return value
> > is not
> > -  guaranteed to be MP safe.
> > +  performed using MP safe mechanisms.
> >
> >If Value is NULL, then ASSERT().
> >
> > @@ -166,8 +165,7 @@ InterlockedIncrement (
> >
> >Performs an atomic decrement of the 32-bit unsigned integer
> > specified by
> >Value and returns the decremented value. The decrement operation
> > must be
> > -  performed using MP safe mechanisms. The state of the return value
> > is not
> > -  guaranteed to be MP safe.
> > +  performed using MP safe mechanisms.
> >
> >If Value is NULL, then ASSERT().
> >
> > diff --git
> > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> > ionLib.inf
> > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> > ionLib.inf
> > index 0be1d4331f..b971cd138d 100755
> > ---
> > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> > ionLib.inf
> > +++
> > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat
> > ionLib.inf
> > @@ -34,9 +34,9 @@ [Sources.IA32]
> >Ia32/InterlockedCompareExchange64.c | MSFT
> >Ia32/InterlockedCompareExchange32.c | MSFT
> >Ia32/InterlockedCompareExchange16.c | MSFT
> > -  Ia32/InterlockedDecrement.c | MSFT
> > -  Ia32/InterlockedIncrement.c | MSFT
> > -  SynchronizationMsc.c  | MSFT
> > +  Ia32/InterlockedDecrement.nasm | MSFT
> > + Ia32/InterlockedIncrement.nasm | MSFT  

[edk2] [PATCH] UefiCpuPkg/CpuMpPei: suppress compiler complaining

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1166

Cc: Dandan Bi 
Cc: Hao A Wu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 UefiCpuPkg/CpuMpPei/CpuPaging.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index bcb942a8e5..a63421a1af 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -517,7 +517,7 @@ GetStackBase (
   IN OUT VOID *Buffer
   )
 {
-  EFI_PHYSICAL_ADDRESSStackBase;
+  volatile EFI_PHYSICAL_ADDRESS   StackBase;
 
   StackBase = (EFI_PHYSICAL_ADDRESS)(UINTN)
   StackBase += BASE_4KB;
@@ -554,6 +554,8 @@ SetupStackGuardPage (
   MpInitLibGetNumberOfProcessors(, NULL);
   MpInitLibWhoAmI ();
   for (Index = 0; Index < NumberOfProcessors; ++Index) {
+StackBase = 0;
+
 if (Index == Bsp) {
   Hob.Raw = GetHobList ();
   while ((Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob.Raw)) 
!= NULL) {
@@ -570,12 +572,19 @@ SetupStackGuardPage (
   //
   MpInitLibStartupThisAP(GetStackBase, Index, NULL, 0, (VOID *), 
NULL);
 }
-//
-// Set Guard page at stack base address.
-//
-ConvertMemoryPageAttributes(StackBase, EFI_PAGE_SIZE, 0);
-DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n",
-(UINT64)StackBase, (UINT64)Index));
+
+if (StackBase == 0) {
+  DEBUG ((DEBUG_ERROR, "Stack base address was not found for [cpu%lu]!\n",
+  (UINT64)Index));
+  ASSERT(StackBase != 0);
+} else {
+  //
+  // Set Guard page at stack base address.
+  //
+  ConvertMemoryPageAttributes(StackBase, EFI_PAGE_SIZE, 0);
+  DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n",
+  (UINT64)StackBase, (UINT64)Index));
+}
   }
 
   //
-- 
2.16.2.windows.1

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


[edk2] [PATCH 1/5] MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Since the stack memory is allocated as EfiBootServicesData, its NX protection
can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid confusing
in setting related PCDs, PcdSetNxForStack will be expired. Instead, If BIT4
of PcdDxeNxMemoryProtectionPolicy is set, the DxeIpl will set NX bit in page
table entries mapping the stack memory.

Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c   |  6 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 +-
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  3 ++-
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |  2 +-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 +++---
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
index 176d361f19..d44b845b76 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
@@ -45,7 +45,11 @@ HandOffToDxeCore (
   BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
   ASSERT (BaseOfStack != NULL);
 
-  if (PcdGetBool (PcdSetNxForStack)) {
+  //
+  // Set stack to non-executable, if EfiBootServicesData type of memory is
+  // set for NX protection.
+  //
+  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) {
 Status = ArmSetMemoryRegionNoExec ((UINTN)BaseOfStack, STACK_SIZE);
 ASSERT_EFI_ERROR (Status);
   }
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index fd82657404..44b6ea84ff 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -116,7 +116,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
 
 [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack   ## 
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## 
SOMETIMES_CONSUMES
 
 [Depex]
   gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index d28baa3615..854078e6dd 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -245,7 +245,8 @@ ToBuildPageTable (
 return TRUE;
   }
 
-  if (PcdGetBool (PcdSetNxForStack) && IsExecuteDisableBitAvailable ()) {
+  if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0 &&
+  IsExecuteDisableBitAvailable ()) {
 return TRUE;
   }
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
index 81efcfe93d..eb53bc9417 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
@@ -94,7 +94,7 @@ HandOffToDxeCore (
 // Set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE
 // for the DxeIpl and the DxeCore are both X64.
 //
-ASSERT (PcdGetBool (PcdSetNxForStack) == FALSE);
+ASSERT (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) == 0);
 ASSERT (PcdGetBool (PcdCpuStackGuard) == FALSE);
   }
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c 
b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 496e219913..27e9d6955d 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -152,7 +152,11 @@ ToSplitPageTable (
 }
   }
 
-  if (PcdGetBool (PcdSetNxForStack)) {
+  //
+  // Set stack to non-executable, if EfiBootServicesData type of memory is
+  // set for NX protection.
+  //
+  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) {
 if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) {
   return TRUE;
 }
@@ -314,7 +318,11 @@ Split2MPageTo4K (
   PageTableEntry->Bits.Present = 1;
 }
 
-if (PcdGetBool (PcdSetNxForStack)
+//
+// Set stack to non-executable, if EfiBootServicesData type of memory is
+// set for NX protection.
+//
+if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0
 && (PhysicalAddress4K >= StackBase)
 && (PhysicalAddress4K < StackBase + StackSize)) {
   //
@@ -755,7 +763,7 @@ CreateIdentityMappingPageTables (
   //
   EnablePageTableProtection ((UINTN)PageMap, TRUE);
 
-  if (PcdGetBool (PcdSetNxForStack)) {
+  if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) {
 EnableExecuteDisableBit ();
   }
 
-- 
2.16.2.windows.1

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


[edk2] [PATCH 2/5] OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Since PcdSetNxForStack is expired, remove related config code.
PcdDxeNxMemoryProtectionPolicy cannot be used as dynamic PCD.
There's no need to add it in code to replace PcdSetNxForStack.

Cc: Laszlo Ersek 
Cc: Star Zeng 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 OvmfPkg/PlatformPei/Platform.c  | 1 -
 OvmfPkg/PlatformPei/PlatformPei.inf | 1 -
 2 files changed, 2 deletions(-)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 5a78668126..6627d236e0 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -340,7 +340,6 @@ NoexecDxeInitialization (
   )
 {
   UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdPropertiesTableEnable);
-  UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdSetNxForStack);
 }
 
 VOID
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 9c5ad9961c..ef5dcb7693 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -96,7 +96,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
-- 
2.16.2.windows.1

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


[edk2] [PATCH 4/5] ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Since PcdSetNxForStack is expired, remove all references. The default
setting of PcdDxeNxMemoryProtectionPolicy has covered PcdSetNxForStack.

Cc: Laszlo Ersek 
Cc: Star Zeng 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 5 -
 1 file changed, 5 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 70a0ac4d78..d87ae5a32d 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -383,11 +383,6 @@
   #
   
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
 
-  #
-  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
 [PcdsFixedAtBuild.ARM]
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
 
-- 
2.16.2.windows.1

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


[edk2] [PATCH 0/5] expire the use of PcdSetNxForStack

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Since the stack memory is allocated as EfiBootServicesData, its NX protection
can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid confusing
in setting related PCDs, PcdSetNxForStack will be expired. Instead, If BIT4
of PcdDxeNxMemoryProtectionPolicy is set, the DxeIpl will set NX bit in page
table entries mapping the stack memory.

Jian J Wang (5):
  MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack
  OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack
  OvmfPkg: expire the use of PcdSetNxForStack
  ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack
  MdeModulePkg: expire PcdSetNxForStack

 ArmVirtPkg/ArmVirt.dsc.inc   |  5 -
 MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c   |  6 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 +-
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  3 ++-
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |  2 +-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 +++---
 MdeModulePkg/MdeModulePkg.dec| 10 +-
 MdeModulePkg/MdeModulePkg.uni| 10 +-
 OvmfPkg/OvmfPkgIa32.dsc  |  1 -
 OvmfPkg/OvmfPkgIa32X64.dsc   |  1 -
 OvmfPkg/OvmfPkgX64.dsc   |  1 -
 OvmfPkg/PlatformPei/Platform.c   |  1 -
 OvmfPkg/PlatformPei/PlatformPei.inf  |  1 -
 13 files changed, 22 insertions(+), 35 deletions(-)

-- 
2.16.2.windows.1

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


[edk2] [PATCH 3/5] OvmfPkg: expire the use of PcdSetNxForStack

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Since PcdSetNxForStack is expired, remove related references in dsc file.

Cc: Laszlo Ersek 
Cc: Star Zeng 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 OvmfPkg/OvmfPkgIa32.dsc| 1 -
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 -
 OvmfPkg/OvmfPkgX64.dsc | 1 -
 3 files changed, 3 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 9f07e75050..1eaefbfd6e 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -559,7 +559,6 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
   # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a4eaeb808c..6f0424c862 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -567,7 +567,6 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
   # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index aa3efc5e73..da6b1293e3 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -566,7 +566,6 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
   # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
-- 
2.16.2.windows.1

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


[edk2] [PATCH 5/5] MdeModulePkg: expire PcdSetNxForStack

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Since the stack memory is allocated as EfiBootServicesData, its NX protection
can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid confusing
in setting related PCDs, PcdSetNxForStack will be expired. Set BIT4 of
PcdDxeNxMemoryProtectionPolicy if NX protection is needed for stack.

Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/MdeModulePkg.dec | 10 +-
 MdeModulePkg/MdeModulePkg.uni | 10 +-
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 74a699cbb7..b1f208909c 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1320,6 +1320,7 @@
   #
   # NOTE: User must NOT set NX protection for EfiLoaderCode / 
EfiBootServicesCode / EfiRuntimeServicesCode. 
   #   User MUST set the same NX protection for EfiBootServicesData and 
EfiConventionalMemory. 
+  #   Stack is allocated as type of EfiBootServicesData. Enable NX 
protection for it will also enable NX protection for stack. 
   #
   # e.g. 0x7FD5 can be used for all memory except Code. 
   # e.g. 0x7BD4 can be used for all memory except Code and ACPINVS/Reserved. 

@@ -1886,15 +1887,6 @@
   # @Prompt Default Creator Revision for ACPI table creation.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|0x0113|UINT32|0x30001038
 
-  ## Indicates if to set NX for stack.
-  #  For the DxeIpl and the DxeCore are both X64, set NX for stack feature 
also require PcdDxeIplBuildPageTables be TRUE.
-  #  For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode 
is FALSE), set NX for stack feature also require
-  #  IA32 PAE is supported and Execute Disable Bit is available.
-  #   TRUE  - to set NX for stack.
-  #   FALSE - Not to set NX for stack.
-  # @Prompt Set NX for stack.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE|BOOLEAN|0x0001006f
-
   ## This PCD specifies the PCI-based SD/MMC host controller mmio base address.
   # Define the mmio base address of the pci-based SD/MMC host controller. If 
there are multiple SD/MMC
   # host controllers, their mmio base addresses are calculated one by one from 
this base address.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 080b8a62c0..6b26b21f00 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -339,15 +339,6 @@
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialRegisterStride_HELP  
#language en-US "The number of bytes between registers in serial device.  The 
default is 1 byte."
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSetNxForStack_PROMPT  #language 
en-US "Set NX for stack"
-
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSetNxForStack_HELP  #language 
en-US "Indicates if to set NX for stack."
-   
   "For the DxeIpl and the DxeCore are both X64, set NX for stack feature also 
require PcdDxeIplBuildPageTables be TRUE."
-   
   "For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is 
FALSE), set NX for stack feature also require"
-   
   "IA32 PAE is supported and Execute Disable Bit is available."
-   
   "TRUE  - to set NX for stack."
-   
   "FALSE - Not to set NX for stack."
-
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiS3Enable_PROMPT  #language 
en-US "ACPI S3 Enable"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiS3Enable_HELP  #language 
en-US "Indicates if ACPI S3 will be enabled."
@@ -1129,6 +1120,7 @@

 "\n"

 "NOTE: User must NOT set NX protection for EfiLoaderCode / 
EfiBootServicesCode / EfiRuntimeServicesCode. \n"

 "User MUST set the same NX protection for EfiBootServicesData 
and EfiConventionalMemory. \n"
+   
 "Stack is allocated as type of EfiBootServicesData. Enable NX 
protection for it will also enable NX protection for stack. \n"

 "\n"

 "e.g. 0x7FD5 can be used for all 

Re: [edk2] [PATCH 0/5] expire the use of PcdSetNxForStack

2018-09-10 Thread Yao, Jiewen
Thanks to simply the PCD usage.

Reviewed-by: jiewen@intel.com


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org]
> Sent: Tuesday, September 11, 2018 1:17 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/5] expire the use of PcdSetNxForStack
> 
> BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116
> 
> Since the stack memory is allocated as EfiBootServicesData, its NX
> protection
> can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid
> confusing
> in setting related PCDs, PcdSetNxForStack will be expired. Instead, If BIT4
> of PcdDxeNxMemoryProtectionPolicy is set, the DxeIpl will set NX bit in page
> table entries mapping the stack memory.
> 
> Jian J Wang (5):
>   MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack
>   OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack
>   OvmfPkg: expire the use of PcdSetNxForStack
>   ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack
>   MdeModulePkg: expire PcdSetNxForStack
> 
>  ArmVirtPkg/ArmVirt.dsc.inc   |  5 -
>  MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c   |  6 +-
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 +-
>  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  3 ++-
>  MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |  2 +-
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14
> +++---
>  MdeModulePkg/MdeModulePkg.dec| 10 +-
>  MdeModulePkg/MdeModulePkg.uni| 10 +-
>  OvmfPkg/OvmfPkgIa32.dsc  |  1 -
>  OvmfPkg/OvmfPkgIa32X64.dsc   |  1 -
>  OvmfPkg/OvmfPkgX64.dsc   |  1 -
>  OvmfPkg/PlatformPei/Platform.c   |  1 -
>  OvmfPkg/PlatformPei/PlatformPei.inf  |  1 -
>  13 files changed, 22 insertions(+), 35 deletions(-)
> 
> --
> 2.16.2.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