[PATCH 0/6] Fix race condition and add event protocol

[PATCH 0/6] Fix race condition and add event protocol

[Gao, Zhichao]

There is a race condition in CoreWaitForEvent function:
If an interrupt happens between CheckEvent and gIdleLoopEvent,
there would be a event pending during cpu sleep. 
So it is required to check the gEventPending with the interrupt
disabled.
Implement a gEfiCpu2ProtocolGuid to fix that. The protocol include
one interface to enable interrupt and put the cpu to sleep.

Add a event protocol gEdkiiCommonEventProtocolGuid to support
all TPL event. It is require for PI drivers that use HW interrput.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

Sean Brogan (5):
  MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event
  MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare
  MdePkg/BaseLib: Implement EnableInterruptsAndSleep
  MdePkg: Add gEfiCpu2ProtocolGuid and header file
  MdeModulePkg/DxeMain: Implement common event protocol

Zhichao Gao (1):
  UefiCpuPkg/CpuDxe: Implement Cpu2 protocol

 MdeModulePkg/Core/Dxe/DxeMain.h               |  64 ++++++++++-
 MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c       |  22 ++++
 .../Core/Dxe/DxeMain/DxeProtocolNotify.c      |   1 +
 MdeModulePkg/Core/Dxe/Event/Event.c           | 102 ++++++++++++++++--
 MdeModulePkg/Core/Dxe/Event/Event.h           |   2 +-
 MdeModulePkg/Include/Protocol/CommonEvent.h   |  18 ++++
 MdeModulePkg/MdeModulePkg.dec                 |   3 +
 MdePkg/Include/Library/BaseLib.h              |  11 ++
 MdePkg/Include/Protocol/Cpu2.h                |  43 ++++++++
 .../Library/BaseLib/Ia32/EnableInterrupts.c   |  18 +++-
 .../BaseLib/Ia32/EnableInterrupts.nasm        |  15 ++-
 .../Library/BaseLib/X64/EnableInterrupts.nasm |  15 ++-
 MdePkg/MdePkg.dec                             |   3 +
 UefiCpuPkg/CpuDxe/CpuDxe.c                    |  40 ++++++-
 UefiCpuPkg/CpuDxe/CpuDxe.h                    |  15 +++
 UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   3 +-
 17 files changed, 358 insertions(+), 18 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/CommonEvent.h
 create mode 100644 MdePkg/Include/Protocol/Cpu2.h

-- 
2.21.0.windows.1

[PATCH 1/6] MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event

[Gao, Zhichao]

From: Sean Brogan <sean.brogan@microsoft.com>

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

Add common event protocol to support all TPL event:
Add the guid and header file of it.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/Include/Protocol/CommonEvent.h | 18 ++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec               |  3 +++
 2 files changed, 21 insertions(+)
 create mode 100644 MdeModulePkg/Include/Protocol/CommonEvent.h

diff --git a/MdeModulePkg/Include/Protocol/CommonEvent.h b/MdeModulePkg/Include/Protocol/CommonEvent.h
new file mode 100644
index 0000000000..176be14101
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/CommonEvent.h
@@ -0,0 +1,18 @@
+/** @file
+  Common event protocol would provide a service of event.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#ifndef __COMMON_EVENT_H__
+#define __COMMON_EVENT_H__
+
+#include <Uefi.h>
+
+typedef struct {
+  EFI_CREATE_EVENT_EX       CreateEventEx;
+  EFI_WAIT_FOR_EVENT        WaitForEvent;
+} EDKII_COMMON_EVENT_PROTOCOL;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 0a9fcddecc..ff4337f235 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -630,6 +630,9 @@
   ## Include/Protocol/PeCoffImageEmulator.h
   gEdkiiPeCoffImageEmulatorProtocolGuid = { 0x96f46153, 0x97a7, 0x4793, { 0xac, 0xc1, 0xfa, 0x19, 0xbf, 0x78, 0xea, 0x97 } }
 
+  ## Include/Protocole/CommonEvent.h
+  gEdkiiCommonEventProtocolGuid = { 0x8bfb3718, 0xdb10, 0x4cc1, { 0xa9, 0xf5, 0x84, 0xae, 0xce, 0xce, 0x99, 0x55 } }
+
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
-- 
2.21.0.windows.1

[PATCH 2/6] MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare

[Gao, Zhichao]

From: Sean Brogan <sean.brogan@microsoft.com>

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

Add EnableInterruptsAndSleep function.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/Include/Library/BaseLib.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index ebd7dd274c..8ab2c12328 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -5033,6 +5033,17 @@ LongJump (
   );
 
 
+/**
+  Enables CPU interrupts and then waits for an interrupt to arrive.
+
+**/
+VOID
+EFIAPI
+EnableInterruptsAndSleep (
+  VOID
+  );
+
+
 /**
   Enables CPU interrupts.
 
-- 
2.21.0.windows.1

[PATCH 3/6] MdePkg/BaseLib: Implement EnableInterruptsAndSleep

[Gao, Zhichao]

From: Sean Brogan <sean.brogan@microsoft.com>

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

Implement EnableInterruptsAndSleep.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/Library/BaseLib/Ia32/EnableInterrupts.c | 18 +++++++++++++++++-
 .../Library/BaseLib/Ia32/EnableInterrupts.nasm | 15 ++++++++++++++-
 .../Library/BaseLib/X64/EnableInterrupts.nasm  | 15 ++++++++++++++-
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.c b/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.c
index bc03144c42..c2a84342b1 100644
--- a/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.c
+++ b/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.c
@@ -1,7 +1,7 @@
 /** @file
   EnableInterrupts function
 
-  Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -24,3 +24,19 @@ EnableInterrupts (
   }
 }
 
+/**
+  Enables CPU interrupts and then waits for an interrupt to arrive.
+
+**/
+VOID
+EFIAPI
+EnableInterruptsAndSleep (
+  VOID
+  )
+{
+  _asm {
+    sti
+    hlt
+  }
+}
+
diff --git a/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.nasm b/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.nasm
index 979e708207..86902f61b6 100644
--- a/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.nasm
+++ b/MdePkg/Library/BaseLib/Ia32/EnableInterrupts.nasm
@@ -1,6 +1,6 @@
 ;------------------------------------------------------------------------------
 ;
-; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Module Name:
@@ -29,3 +29,16 @@ ASM_PFX(EnableInterrupts):
     sti
     ret
 
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; EnableInterruptsAndSleep (
+;   VOID
+;   );
+;------------------------------------------------------------------------------
+global ASM_PFX(EnableInterruptsAndSleep)
+ASM_PFX(EnableInterruptsAndSleep):
+    sti
+    hlt
+    ret
+
diff --git a/MdePkg/Library/BaseLib/X64/EnableInterrupts.nasm b/MdePkg/Library/BaseLib/X64/EnableInterrupts.nasm
index 6057afd626..de8f2ad434 100644
--- a/MdePkg/Library/BaseLib/X64/EnableInterrupts.nasm
+++ b/MdePkg/Library/BaseLib/X64/EnableInterrupts.nasm
@@ -1,6 +1,6 @@
 ;------------------------------------------------------------------------------
 ;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Module Name:
@@ -30,3 +30,16 @@ ASM_PFX(EnableInterrupts):
     sti
     ret
 
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; EnableInterruptsAndSleep (
+;   VOID
+;   );
+;------------------------------------------------------------------------------
+global ASM_PFX(EnableInterruptsAndSleep)
+ASM_PFX(EnableInterruptsAndSleep):
+    sti
+    hlt
+    ret
+
-- 
2.21.0.windows.1

[PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file

[Gao, Zhichao]

From: Sean Brogan <sean.brogan@microsoft.com>

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

Add gEfiCpu2ProtocolGuid to MdePkg.dec.
Add the header file of Cpu2 protocol: it has one interface
to enable interrupt and put cpu to sleep to wait for an
interrupt.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/Include/Protocol/Cpu2.h | 43 ++++++++++++++++++++++++++++++++++
 MdePkg/MdePkg.dec              |  3 +++
 2 files changed, 46 insertions(+)
 create mode 100644 MdePkg/Include/Protocol/Cpu2.h

diff --git a/MdePkg/Include/Protocol/Cpu2.h b/MdePkg/Include/Protocol/Cpu2.h
new file mode 100644
index 0000000000..cacd948140
--- /dev/null
+++ b/MdePkg/Include/Protocol/Cpu2.h
@@ -0,0 +1,43 @@
+/** @file
+  CPU2 Protocol
+
+  This code abstracts the DXE core from processor implementation details.
+
+  Copyright (c) 2006 - 2018, Microsoft Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __PROTOCOL_CPU2_H__
+#define __PROTOCOL_CPU2_H__
+
+#include <Uefi.h>
+
+typedef struct _EFI_CPU2_PROTOCOL   EFI_CPU2_PROTOCOL;
+
+
+/**
+  This function enables CPU interrupts and then waits for an interrupt to arrive.
+
+  @param  This                  The EFI_CPU2_PROTOCOL instance.
+
+  @retval EFI_SUCCESS           Interrupts are enabled on the processor.
+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled on the processor.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT)(
+  IN EFI_CPU2_PROTOCOL              *This
+  );
+
+//
+// The EFI_CPU2_PROTOCOL is used to abstract processor-specific functions from the DXE
+// Foundation.
+//
+struct _EFI_CPU2_PROTOCOL {
+  EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT            EnableAndWaitForInterrupt;
+};
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6c563375ee..e8c6939849 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1803,6 +1803,9 @@
   ## Include/Protocol/ShellDynamicCommand.h
   gEfiShellDynamicCommandProtocolGuid  = { 0x3c7200e9, 0x005f, 0x4ea4, {0x87, 0xde, 0xa3, 0xdf, 0xac, 0x8a, 0x27, 0xc3 }}
 
+  ## Include/Protocol/Cpu2.h
+  gEfiCpu2ProtocolGuid                 = { 0x55198405, 0x26C0, 0x4765, {0x8B, 0x7D, 0xBE, 0x1D, 0xF5, 0xF9, 0x97, 0x12 }}
+
 #
 # [Error.gEfiMdePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
-- 
2.21.0.windows.1

[PATCH 5/6] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol

[Gao, Zhichao]

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

Implement Cp2 protocol: it has one interface to enable the
interrupt and put cpu to sleep and wait for an interrupt.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 UefiCpuPkg/CpuDxe/CpuDxe.c   | 40 +++++++++++++++++++++++++++++++++++-
 UefiCpuPkg/CpuDxe/CpuDxe.h   | 15 ++++++++++++++
 UefiCpuPkg/CpuDxe/CpuDxe.inf |  3 ++-
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index 7d7270e10b..0d0cdf6f86 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -1,7 +1,7 @@
 /** @file
   CPU DXE Module to produce CPU ARCH Protocol.
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -18,6 +18,7 @@
 //
 BOOLEAN                   InterruptState = FALSE;
 EFI_HANDLE                mCpuHandle = NULL;
+EFI_HANDLE                mCpu2Handle = NULL;
 BOOLEAN                   mIsFlushingGCD;
 BOOLEAN                   mIsAllocatingPageTable = FALSE;
 UINT64                    mValidMtrrAddressMask;
@@ -96,6 +97,10 @@ EFI_CPU_ARCH_PROTOCOL  gCpu = {
   4                           // DmaBufferAlignment
 };
 
+EFI_CPU2_PROTOCOL   gCpu2 = {
+  CpuEnableAndWaitForInterrupt
+};
+
 //
 // CPU Arch Protocol Functions
 //
@@ -499,6 +504,28 @@ CpuSetMemoryAttributes (
   return AssignMemoryPageAttributes (NULL, BaseAddress, Length, MemoryAttributes, NULL);
 }
 
+//
+// CPU2 Protocol Functions
+//
+/**
+  This function enables CPU interrupts and then waits for an interrupt to arrive.
+
+  @param  This                  The EFI_CPU2_PROTOCOL instance.
+
+  @retval EFI_SUCCESS           Interrupts are enabled on the processor.
+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled on the processor.
+
+**/
+EFI_STATUS
+CpuEnableAndWaitForInterrupt (
+  IN EFI_CPU2_PROTOCOL              *This
+  )
+{
+  EnableInterruptsAndSleep ();
+
+  return EFI_SUCCESS;
+}
+
 /**
   Initializes the valid bits mask and valid address mask for MTRRs.
 
@@ -1211,6 +1238,17 @@ InitializeCpu (
                   );
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // Install CPU2 Protocol
+  //
+  Status = gBS->InstallMultipleProtocolInterfaces (
+                  &mCpu2Handle,
+                  &gEfiCpu2ProtocolGuid,
+                  &gCpu2,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+
   InitializeMpSupport ();
 
   return Status;
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
index b029be430b..8698ff78eb 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.h
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
@@ -12,6 +12,7 @@
 #include <PiDxe.h>
 
 #include <Protocol/Cpu.h>
+#include <Protocol/Cpu2.h>
 #include <Protocol/MpService.h>
 #include <Register/Msr.h>
 
@@ -305,6 +306,20 @@ PageFaultExceptionHandler (
   IN EFI_SYSTEM_CONTEXT   SystemContext
   );
 
+/**
+  This function enables CPU interrupts and then waits for an interrupt to arrive.
+
+  @param  This                  The EFI_CPU2_PROTOCOL instance.
+
+  @retval EFI_SUCCESS           Interrupts are enabled on the processor.
+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled on the processor.
+
+**/
+EFI_STATUS
+CpuEnableAndWaitForInterrupt (
+  IN EFI_CPU2_PROTOCOL              *This
+  );
+
 extern BOOLEAN mIsAllocatingPageTable;
 extern UINTN   mNumberOfProcessors;
 
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf
index 57381dbc85..d4ff562d89 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
@@ -1,7 +1,7 @@
 ## @file
 #  CPU driver installs CPU Architecture Protocol and CPU MP protocol.
 #
-#  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
 #  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -62,6 +62,7 @@
   gEfiCpuArchProtocolGuid                       ## PRODUCES
   gEfiMpServiceProtocolGuid                     ## PRODUCES
   gEfiSmmBase2ProtocolGuid                      ## SOMETIMES_CONSUMES
+  gEfiCpu2ProtocolGuid                          ## PRODUCES
 
 [Guids]
   gIdleLoopEventGuid                            ## CONSUMES           ## Event
-- 
2.21.0.windows.1

[PATCH 6/6] MdeModulePkg/DxeMain: Implement common event protocol

[Gao, Zhichao]

From: Sean Brogan <sean.brogan@microsoft.com>

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

If an interrupt happens between CheckEvent and gIdleLoopEvent,
there would be a event pending during cpu sleep. So it is
required to check the gEventPending with the interrupt disabled.

Implement a protocol gEdkiiCommonEventProtocolGuid to support
all TPL event for PI drivers that use HW interrput. It has two
interface, one is to create event and the other one is to wait for
event.

Add 'volatile' specifier for gEfiCurrentTpl and gEventPending
because they may be changed out of the context (interrupt context).

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/Core/Dxe/DxeMain.h               |  64 ++++++++++-
 MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c       |  22 ++++
 .../Core/Dxe/DxeMain/DxeProtocolNotify.c      |   1 +
 MdeModulePkg/Core/Dxe/Event/Event.c           | 102 ++++++++++++++++--
 MdeModulePkg/Core/Dxe/Event/Event.h           |   2 +-
 6 files changed, 179 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 6a64852730..355f66bc44 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -38,6 +38,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Protocol/Security2.h>
 #include <Protocol/Reset.h>
 #include <Protocol/Cpu.h>
+#include <Protocol/Cpu2.h>
 #include <Protocol/Metronome.h>
 #include <Protocol/FirmwareVolumeBlock.h>
 #include <Protocol/Capsule.h>
@@ -47,6 +48,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Protocol/HiiPackageList.h>
 #include <Protocol/SmmBase2.h>
 #include <Protocol/PeCoffImageEmulator.h>
+#include <Protocol/CommonEvent.h>
 #include <Guid/MemoryTypeInformation.h>
 #include <Guid/FirmwareFileSystem2.h>
 #include <Guid/FirmwareFileSystem3.h>
@@ -274,10 +276,11 @@ extern EFI_METRONOME_ARCH_PROTOCOL              *gMetronome;
 extern EFI_TIMER_ARCH_PROTOCOL                  *gTimer;
 extern EFI_SECURITY_ARCH_PROTOCOL               *gSecurity;
 extern EFI_SECURITY2_ARCH_PROTOCOL              *gSecurity2;
+extern EFI_CPU2_PROTOCOL                        *gCpu2;
 extern EFI_BDS_ARCH_PROTOCOL                    *gBds;
 extern EFI_SMM_BASE2_PROTOCOL                   *gSmmBase2;
 
-extern EFI_TPL                                  gEfiCurrentTpl;
+extern volatile EFI_TPL                         gEfiCurrentTpl;
 
 extern EFI_GUID                                 *gDxeCoreFileName;
 extern EFI_LOADED_IMAGE_PROTOCOL                *gDxeCoreLoadedImage;
@@ -1714,6 +1717,65 @@ CoreCheckEvent (
   );
 
 
+/**
+  Stops execution until an event is signaled.
+
+  Support all TPL.
+
+  @param  NumberOfEvents         The number of events in the UserEvents array
+  @param  UserEvents             An array of EFI_EVENT
+  @param  UserIndex              Pointer to the index of the event which
+                                 satisfied the wait condition
+
+  @retval EFI_SUCCESS            The event indicated by Index was signaled.
+  @retval EFI_INVALID_PARAMETER  The event indicated by Index has a notification
+                                 function or Event was not a valid type
+  @retval EFI_UNSUPPORTED        The current TPL is not TPL_APPLICATION
+
+**/
+EFI_STATUS
+EFIAPI
+CommonWaitForEvent (
+  IN UINTN        NumberOfEvents,
+  IN EFI_EVENT    *UserEvents,
+  OUT UINTN       *UserIndex
+  );
+
+
+/**
+  Creates an event in a group.
+
+  Support all TPL.
+
+  @param  Type                   The type of event to create and its mode and
+                                 attributes
+  @param  NotifyTpl              The task priority level of event notifications
+  @param  NotifyFunction         Pointer to the events notification function
+  @param  NotifyContext          Pointer to the notification functions context;
+                                 corresponds to parameter "Context" in the
+                                 notification function
+  @param  EventGroup             GUID for EventGroup if NULL act the same as
+                                 gBS->CreateEvent().
+  @param  Event                  Pointer to the newly created event if the call
+                                 succeeds; undefined otherwise
+
+  @retval EFI_SUCCESS            The event structure was created
+  @retval EFI_INVALID_PARAMETER  One of the parameters has an invalid value
+  @retval EFI_OUT_OF_RESOURCES   The event could not be allocated
+
+**/
+EFI_STATUS
+EFIAPI
+CommonCreateEventEx (
+  IN UINT32                   Type,
+  IN EFI_TPL                  NotifyTpl,
+  IN EFI_EVENT_NOTIFY         NotifyFunction, OPTIONAL
+  IN CONST VOID               *NotifyContext, OPTIONAL
+  IN CONST EFI_GUID           *EventGroup,    OPTIONAL
+  OUT EFI_EVENT               *Event
+  );
+
+
 /**
   Adds reserved memory, system memory, or memory-mapped I/O resources to the
   global coherency domain of the processor.
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 61161bee28..d758ee431e 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -171,6 +171,7 @@
   gEfiVariableArchProtocolGuid                  ## CONSUMES
   gEfiCapsuleArchProtocolGuid                   ## CONSUMES
   gEfiWatchdogTimerArchProtocolGuid             ## CONSUMES
+  gEdkiiCommonEventProtocolGuid                 ## SOMETIMES_CONSUMES
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdLoadFixAddressBootTimeCodePageNumber    ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 514d1aa75a..0e6a4b2ec5 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -13,12 +13,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 EFI_HANDLE                                mDecompressHandle = NULL;
 
+//
+//  Common event protocol
+//
+EFI_HANDLE                                mCommonEventHalde = NULL;
+
 //
 // DXE Core globals for Architecture Protocols
 //
 EFI_SECURITY_ARCH_PROTOCOL        *gSecurity      = NULL;
 EFI_SECURITY2_ARCH_PROTOCOL       *gSecurity2     = NULL;
 EFI_CPU_ARCH_PROTOCOL             *gCpu           = NULL;
+EFI_CPU2_PROTOCOL                 *gCpu2          = NULL;
 EFI_METRONOME_ARCH_PROTOCOL       *gMetronome     = NULL;
 EFI_TIMER_ARCH_PROTOCOL           *gTimer         = NULL;
 EFI_BDS_ARCH_PROTOCOL             *gBds           = NULL;
@@ -211,6 +217,14 @@ EFI_DECOMPRESS_PROTOCOL  gEfiDecompress = {
   DxeMainUefiDecompress
 };
 
+//
+// Common event protocol
+//
+EDKII_COMMON_EVENT_PROTOCOL gEdkiiCommonEvent = {
+  CommonCreateEventEx,
+  CommonWaitForEvent
+};
+
 //
 // For Loading modules at fixed address feature, the configuration table is to cache the top address below which to load
 // Runtime code&boot time code
@@ -475,6 +489,14 @@ DxeMain (
   //
   CoreNotifyOnProtocolInstallation ();
 
+  Status = CoreInstallProtocolInterface (
+              &mCommonEventHalde,
+              &gEdkiiCommonEventProtocolGuid,
+              EFI_NATIVE_INTERFACE,
+              &gEdkiiCommonEvent
+              );
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Produce Firmware Volume Protocols, one for each FV in the HOB list.
   //
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c
index 29a55d02e6..77af8185b8 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c
@@ -41,6 +41,7 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY  mArchProtocols[] = {
 EFI_CORE_PROTOCOL_NOTIFY_ENTRY  mOptionalProtocols[] = {
   { &gEfiSecurity2ArchProtocolGuid,        (VOID **)&gSecurity2,     NULL, NULL, FALSE },
   { &gEfiSmmBase2ProtocolGuid,             (VOID **)&gSmmBase2,      NULL, NULL, FALSE },
+  { &gEdkiiCommonEventProtocolGuid,        (VOID **)&gCpu2,          NULL, NULL, FALSE },
   { NULL,                                  (VOID **)NULL,            NULL, NULL, FALSE }
 };
 
diff --git a/MdeModulePkg/Core/Dxe/Event/Event.c b/MdeModulePkg/Core/Dxe/Event/Event.c
index 21db38aaf0..d271d2faf3 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.c
+++ b/MdeModulePkg/Core/Dxe/Event/Event.c
@@ -14,7 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 ///
 /// gEfiCurrentTpl - Current Task priority level
 ///
-EFI_TPL  gEfiCurrentTpl = TPL_APPLICATION;
+volatile EFI_TPL  gEfiCurrentTpl = TPL_APPLICATION;
 
 ///
 /// gEventQueueLock - Protects the event queues
@@ -29,7 +29,7 @@ LIST_ENTRY      gEventQueue[TPL_HIGH_LEVEL + 1];
 ///
 /// gEventPending - A bitmask of the EventQueues that are pending
 ///
-UINTN           gEventPending = 0;
+volatile UINTN  gEventPending = 0;
 
 ///
 /// gEventSignalQueue - A list of events to signal based on EventGroup type
@@ -658,16 +658,44 @@ CoreWaitForEvent (
   OUT UINTN       *UserIndex
   )
 {
-  EFI_STATUS      Status;
-  UINTN           Index;
-
-  //
+   //
   // Can only WaitForEvent at TPL_APPLICATION
   //
   if (gEfiCurrentTpl != TPL_APPLICATION) {
     return EFI_UNSUPPORTED;
   }
 
+  return CommonWaitForEvent (NumberOfEvents, UserEvents, UserIndex);
+}
+
+
+/**
+  Stops execution until an event is signaled.
+
+  Support all TPL.
+
+  @param  NumberOfEvents         The number of events in the UserEvents array
+  @param  UserEvents             An array of EFI_EVENT
+  @param  UserIndex              Pointer to the index of the event which
+                                 satisfied the wait condition
+
+  @retval EFI_SUCCESS            The event indicated by Index was signaled.
+  @retval EFI_INVALID_PARAMETER  The event indicated by Index has a notification
+                                 function or Event was not a valid type
+  @retval EFI_UNSUPPORTED        The current TPL is not TPL_APPLICATION
+
+**/
+EFI_STATUS
+EFIAPI
+CommonWaitForEvent (
+  IN UINTN        NumberOfEvents,
+  IN EFI_EVENT    *UserEvents,
+  OUT UINTN       *UserIndex
+  )
+{
+  EFI_STATUS      Status;
+  UINTN           Index;
+
   if (NumberOfEvents == 0) {
     return EFI_INVALID_PARAMETER;
   }
@@ -678,7 +706,7 @@ CoreWaitForEvent (
 
   for(;;) {
 
-    for(Index = 0; Index < NumberOfEvents; Index++) {
+    for (Index = 0; Index < NumberOfEvents; Index++) {
 
       Status = CoreCheckEvent (UserEvents[Index]);
 
@@ -693,14 +721,66 @@ CoreWaitForEvent (
       }
     }
 
-    //
-    // Signal the Idle event
-    //
-    CoreSignalEvent (gIdleLoopEvent);
+    if (gCpu != NULL && gCpu2 != NULL) {
+      //
+      // None of the events checked above are ready/signaled yet,
+      // disable interrupts before checking for all pending events
+      //
+      gCpu->DisableInterrupt (gCpu);
+
+      if (((UINTN) HighBitSet64 (gEventPending)) > gEfiCurrentTpl) {
+        //
+        // There are pending events, enable interrupts for these events to be processed
+        //
+        gCpu->EnableInterrupt (gCpu);
+      } else {
+        //
+        // No events are pending, enable interrupts, sleep the CPU and wait for an interrupt
+        //
+        gCpu2->EnableAndWaitForInterrupt (gCpu2);
+      }
+    }
   }
 }
 
 
+/**
+  Creates an event in a group.
+
+  Support all TPL.
+
+  @param  Type                   The type of event to create and its mode and
+                                 attributes
+  @param  NotifyTpl              The task priority level of event notifications
+  @param  NotifyFunction         Pointer to the events notification function
+  @param  NotifyContext          Pointer to the notification functions context;
+                                 corresponds to parameter "Context" in the
+                                 notification function
+  @param  EventGroup             GUID for EventGroup if NULL act the same as
+                                 gBS->CreateEvent().
+  @param  Event                  Pointer to the newly created event if the call
+                                 succeeds; undefined otherwise
+
+  @retval EFI_SUCCESS            The event structure was created
+  @retval EFI_INVALID_PARAMETER  One of the parameters has an invalid value
+  @retval EFI_OUT_OF_RESOURCES   The event could not be allocated
+
+**/
+EFI_STATUS
+EFIAPI
+CommonCreateEventEx (
+  IN UINT32                   Type,
+  IN EFI_TPL                  NotifyTpl,
+  IN EFI_EVENT_NOTIFY         NotifyFunction, OPTIONAL
+  IN CONST VOID               *NotifyContext, OPTIONAL
+  IN CONST EFI_GUID           *EventGroup,    OPTIONAL
+  OUT EFI_EVENT               *Event
+  )
+{
+  return CoreCreateEventInternal (Type, NotifyTpl, NotifyFunction, NotifyContext, EventGroup, Event);
+}
+
+
 /**
   Closes an event and frees the event structure.
 
diff --git a/MdeModulePkg/Core/Dxe/Event/Event.h b/MdeModulePkg/Core/Dxe/Event/Event.h
index 8141c5003e..050df26ec9 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.h
+++ b/MdeModulePkg/Core/Dxe/Event/Event.h
@@ -12,7 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 
 #define VALID_TPL(a)            ((a) <= TPL_HIGH_LEVEL)
-extern  UINTN                   gEventPending;
+extern  volatile UINTN          gEventPending;
 
 ///
 /// Set if Event is part of an event group
-- 
2.21.0.windows.1

Re: [PATCH 0/6] Fix race condition and add event protocol

[Ni, Ray]

Zhichao,
Did your detailed patch commit message describe the consequence of the race condition? (I haven't checked.)
If no, could you please describe in detail about the consequence?

Thanks,
Ray

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Friday, May 24, 2019 1:05 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH 0/6] Fix race condition and add event protocol
> 
> There is a race condition in CoreWaitForEvent function:
> If an interrupt happens between CheckEvent and gIdleLoopEvent, there
> would be a event pending during cpu sleep.
> So it is required to check the gEventPending with the interrupt disabled.
> Implement a gEfiCpu2ProtocolGuid to fix that. The protocol include one
> interface to enable interrupt and put the cpu to sleep.
> 
> Add a event protocol gEdkiiCommonEventProtocolGuid to support all TPL
> event. It is require for PI drivers that use HW interrput.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> 
> Sean Brogan (5):
>   MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event
>   MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare
>   MdePkg/BaseLib: Implement EnableInterruptsAndSleep
>   MdePkg: Add gEfiCpu2ProtocolGuid and header file
>   MdeModulePkg/DxeMain: Implement common event protocol
> 
> Zhichao Gao (1):
>   UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
> 
>  MdeModulePkg/Core/Dxe/DxeMain.h               |  64 ++++++++++-
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c       |  22 ++++
>  .../Core/Dxe/DxeMain/DxeProtocolNotify.c      |   1 +
>  MdeModulePkg/Core/Dxe/Event/Event.c           | 102 ++++++++++++++++--
>  MdeModulePkg/Core/Dxe/Event/Event.h           |   2 +-
>  MdeModulePkg/Include/Protocol/CommonEvent.h   |  18 ++++
>  MdeModulePkg/MdeModulePkg.dec                 |   3 +
>  MdePkg/Include/Library/BaseLib.h              |  11 ++
>  MdePkg/Include/Protocol/Cpu2.h                |  43 ++++++++
>  .../Library/BaseLib/Ia32/EnableInterrupts.c   |  18 +++-
>  .../BaseLib/Ia32/EnableInterrupts.nasm        |  15 ++-
>  .../Library/BaseLib/X64/EnableInterrupts.nasm |  15 ++-
>  MdePkg/MdePkg.dec                             |   3 +
>  UefiCpuPkg/CpuDxe/CpuDxe.c                    |  40 ++++++-
>  UefiCpuPkg/CpuDxe/CpuDxe.h                    |  15 +++
>  UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   3 +-
>  17 files changed, 358 insertions(+), 18 deletions(-)  create mode 100644
> MdeModulePkg/Include/Protocol/CommonEvent.h
>  create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> 
> --
> 2.21.0.windows.1

Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file

[Liming Gao]

Zhichao:
  Which spec defines Cpu2 protocol?

Thanks
Liming
>-----Original Message-----
>From: Gao, Zhichao
>Sent: Friday, May 24, 2019 1:05 PM
>To: devel@edk2.groups.io
>Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Michael
>Turner <Michael.Turner@microsoft.com>; Bret Barkelew
><Bret.Barkelew@microsoft.com>
>Subject: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
>
>From: Sean Brogan <sean.brogan@microsoft.com>
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
>
>Add gEfiCpu2ProtocolGuid to MdePkg.dec.
>Add the header file of Cpu2 protocol: it has one interface
>to enable interrupt and put cpu to sleep to wait for an
>interrupt.
>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Sean Brogan <sean.brogan@microsoft.com>
>Cc: Michael Turner <Michael.Turner@microsoft.com>
>Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
>Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>---
> MdePkg/Include/Protocol/Cpu2.h | 43
>++++++++++++++++++++++++++++++++++
> MdePkg/MdePkg.dec              |  3 +++
> 2 files changed, 46 insertions(+)
> create mode 100644 MdePkg/Include/Protocol/Cpu2.h
>
>diff --git a/MdePkg/Include/Protocol/Cpu2.h
>b/MdePkg/Include/Protocol/Cpu2.h
>new file mode 100644
>index 0000000000..cacd948140
>--- /dev/null
>+++ b/MdePkg/Include/Protocol/Cpu2.h
>@@ -0,0 +1,43 @@
>+/** @file
>+  CPU2 Protocol
>+
>+  This code abstracts the DXE core from processor implementation details.
>+
>+  Copyright (c) 2006 - 2018, Microsoft Corporation. All rights reserved.<BR>
>+
>+  SPDX-License-Identifier: BSD-2-Clause-Patent
>+
>+**/
>+
>+#ifndef __PROTOCOL_CPU2_H__
>+#define __PROTOCOL_CPU2_H__
>+
>+#include <Uefi.h>
>+
>+typedef struct _EFI_CPU2_PROTOCOL   EFI_CPU2_PROTOCOL;
>+
>+
>+/**
>+  This function enables CPU interrupts and then waits for an interrupt to
>arrive.
>+
>+  @param  This                  The EFI_CPU2_PROTOCOL instance.
>+
>+  @retval EFI_SUCCESS           Interrupts are enabled on the processor.
>+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled on the
>processor.
>+
>+**/
>+typedef
>+EFI_STATUS
>+(EFIAPI *EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT)(
>+  IN EFI_CPU2_PROTOCOL              *This
>+  );
>+
>+//
>+// The EFI_CPU2_PROTOCOL is used to abstract processor-specific functions
>from the DXE
>+// Foundation.
>+//
>+struct _EFI_CPU2_PROTOCOL {
>+  EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT
>EnableAndWaitForInterrupt;
>+};
>+
>+#endif
>diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>index 6c563375ee..e8c6939849 100644
>--- a/MdePkg/MdePkg.dec
>+++ b/MdePkg/MdePkg.dec
>@@ -1803,6 +1803,9 @@
>   ## Include/Protocol/ShellDynamicCommand.h
>   gEfiShellDynamicCommandProtocolGuid  = { 0x3c7200e9, 0x005f, 0x4ea4,
>{0x87, 0xde, 0xa3, 0xdf, 0xac, 0x8a, 0x27, 0xc3 }}
>
>+  ## Include/Protocol/Cpu2.h
>+  gEfiCpu2ProtocolGuid                 = { 0x55198405, 0x26C0, 0x4765, {0x8B, 0x7D,
>0xBE, 0x1D, 0xF5, 0xF9, 0x97, 0x12 }}
>+
> #
> # [Error.gEfiMdePkgTokenSpaceGuid]
> #   0x80000001 | Invalid value provided.
>--
>2.21.0.windows.1

Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file

[Gao, Zhichao]

I have found it in UEFP spec and PI spec. It is not defined in these specs.
So I think maybe no spec has already defined it.

Thanks,
Zhichao

> -----Original Message-----
> From: Gao, Liming
> Sent: Friday, May 24, 2019 1:20 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
> 
> Zhichao:
>   Which spec defines Cpu2 protocol?
> 
> Thanks
> Liming
> >-----Original Message-----
> >From: Gao, Zhichao
> >Sent: Friday, May 24, 2019 1:05 PM
> >To: devel@edk2.groups.io
> >Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> >Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> ><Bret.Barkelew@microsoft.com>
> >Subject: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
> >
> >From: Sean Brogan <sean.brogan@microsoft.com>
> >
> >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
> >
> >Add gEfiCpu2ProtocolGuid to MdePkg.dec.
> >Add the header file of Cpu2 protocol: it has one interface to enable
> >interrupt and put cpu to sleep to wait for an interrupt.
> >
> >Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >Cc: Liming Gao <liming.gao@intel.com>
> >Cc: Sean Brogan <sean.brogan@microsoft.com>
> >Cc: Michael Turner <Michael.Turner@microsoft.com>
> >Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> >---
> > MdePkg/Include/Protocol/Cpu2.h | 43
> >++++++++++++++++++++++++++++++++++
> > MdePkg/MdePkg.dec              |  3 +++
> > 2 files changed, 46 insertions(+)
> > create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> >
> >diff --git a/MdePkg/Include/Protocol/Cpu2.h
> >b/MdePkg/Include/Protocol/Cpu2.h new file mode 100644 index
> >0000000000..cacd948140
> >--- /dev/null
> >+++ b/MdePkg/Include/Protocol/Cpu2.h
> >@@ -0,0 +1,43 @@
> >+/** @file
> >+  CPU2 Protocol
> >+
> >+  This code abstracts the DXE core from processor implementation details.
> >+
> >+  Copyright (c) 2006 - 2018, Microsoft Corporation. All rights
> >+ reserved.<BR>
> >+
> >+  SPDX-License-Identifier: BSD-2-Clause-Patent
> >+
> >+**/
> >+
> >+#ifndef __PROTOCOL_CPU2_H__
> >+#define __PROTOCOL_CPU2_H__
> >+
> >+#include <Uefi.h>
> >+
> >+typedef struct _EFI_CPU2_PROTOCOL   EFI_CPU2_PROTOCOL;
> >+
> >+
> >+/**
> >+  This function enables CPU interrupts and then waits for an interrupt
> >+to
> >arrive.
> >+
> >+  @param  This                  The EFI_CPU2_PROTOCOL instance.
> >+
> >+  @retval EFI_SUCCESS           Interrupts are enabled on the processor.
> >+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled on the
> >processor.
> >+
> >+**/
> >+typedef
> >+EFI_STATUS
> >+(EFIAPI *EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT)(
> >+  IN EFI_CPU2_PROTOCOL              *This
> >+  );
> >+
> >+//
> >+// The EFI_CPU2_PROTOCOL is used to abstract processor-specific
> >+functions
> >from the DXE
> >+// Foundation.
> >+//
> >+struct _EFI_CPU2_PROTOCOL {
> >+  EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT
> >EnableAndWaitForInterrupt;
> >+};
> >+
> >+#endif
> >diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> >6c563375ee..e8c6939849 100644
> >--- a/MdePkg/MdePkg.dec
> >+++ b/MdePkg/MdePkg.dec
> >@@ -1803,6 +1803,9 @@
> >   ## Include/Protocol/ShellDynamicCommand.h
> >   gEfiShellDynamicCommandProtocolGuid  = { 0x3c7200e9, 0x005f, 0x4ea4,
> >{0x87, 0xde, 0xa3, 0xdf, 0xac, 0x8a, 0x27, 0xc3 }}
> >
> >+  ## Include/Protocol/Cpu2.h
> >+  gEfiCpu2ProtocolGuid                 = { 0x55198405, 0x26C0, 0x4765, {0x8B,
> 0x7D,
> >0xBE, 0x1D, 0xF5, 0xF9, 0x97, 0x12 }}
> >+
> > #
> > # [Error.gEfiMdePkgTokenSpaceGuid]
> > #   0x80000001 | Invalid value provided.
> >--
> >2.21.0.windows.1

Re: [edk2-devel] [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file

[Yao, Jiewen]

EFI_CPU_IO2_PROCOL is defined in PI spec Volume 5 Chapter 15 CPU I/O Protocol.

Thank you
Yao jiewen

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Gao, Zhichao
> Sent: Thursday, May 23, 2019 10:28 PM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid
> and header file
> 
> I have found it in UEFP spec and PI spec. It is not defined in these specs.
> So I think maybe no spec has already defined it.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Gao, Liming
> > Sent: Friday, May 24, 2019 1:20 PM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Michael Turner
> > <Michael.Turner@microsoft.com>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header
> file
> >
> > Zhichao:
> >   Which spec defines Cpu2 protocol?
> >
> > Thanks
> > Liming
> > >-----Original Message-----
> > >From: Gao, Zhichao
> > >Sent: Friday, May 24, 2019 1:05 PM
> > >To: devel@edk2.groups.io
> > >Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> > ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> > >Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> > ><Bret.Barkelew@microsoft.com>
> > >Subject: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
> > >
> > >From: Sean Brogan <sean.brogan@microsoft.com>
> > >
> > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
> > >
> > >Add gEfiCpu2ProtocolGuid to MdePkg.dec.
> > >Add the header file of Cpu2 protocol: it has one interface to enable
> > >interrupt and put cpu to sleep to wait for an interrupt.
> > >
> > >Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > >Cc: Liming Gao <liming.gao@intel.com>
> > >Cc: Sean Brogan <sean.brogan@microsoft.com>
> > >Cc: Michael Turner <Michael.Turner@microsoft.com>
> > >Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > >Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > >---
> > > MdePkg/Include/Protocol/Cpu2.h | 43
> > >++++++++++++++++++++++++++++++++++
> > > MdePkg/MdePkg.dec              |  3 +++
> > > 2 files changed, 46 insertions(+)
> > > create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> > >
> > >diff --git a/MdePkg/Include/Protocol/Cpu2.h
> > >b/MdePkg/Include/Protocol/Cpu2.h new file mode 100644 index
> > >0000000000..cacd948140
> > >--- /dev/null
> > >+++ b/MdePkg/Include/Protocol/Cpu2.h
> > >@@ -0,0 +1,43 @@
> > >+/** @file
> > >+  CPU2 Protocol
> > >+
> > >+  This code abstracts the DXE core from processor implementation
> details.
> > >+
> > >+  Copyright (c) 2006 - 2018, Microsoft Corporation. All rights
> > >+ reserved.<BR>
> > >+
> > >+  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >+
> > >+**/
> > >+
> > >+#ifndef __PROTOCOL_CPU2_H__
> > >+#define __PROTOCOL_CPU2_H__
> > >+
> > >+#include <Uefi.h>
> > >+
> > >+typedef struct _EFI_CPU2_PROTOCOL   EFI_CPU2_PROTOCOL;
> > >+
> > >+
> > >+/**
> > >+  This function enables CPU interrupts and then waits for an interrupt
> > >+to
> > >arrive.
> > >+
> > >+  @param  This                  The EFI_CPU2_PROTOCOL
> instance.
> > >+
> > >+  @retval EFI_SUCCESS           Interrupts are enabled on the
> processor.
> > >+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled
> on the
> > >processor.
> > >+
> > >+**/
> > >+typedef
> > >+EFI_STATUS
> > >+(EFIAPI *EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT)(
> > >+  IN EFI_CPU2_PROTOCOL              *This
> > >+  );
> > >+
> > >+//
> > >+// The EFI_CPU2_PROTOCOL is used to abstract processor-specific
> > >+functions
> > >from the DXE
> > >+// Foundation.
> > >+//
> > >+struct _EFI_CPU2_PROTOCOL {
> > >+  EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT
> > >EnableAndWaitForInterrupt;
> > >+};
> > >+
> > >+#endif
> > >diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> > >6c563375ee..e8c6939849 100644
> > >--- a/MdePkg/MdePkg.dec
> > >+++ b/MdePkg/MdePkg.dec
> > >@@ -1803,6 +1803,9 @@
> > >   ## Include/Protocol/ShellDynamicCommand.h
> > >   gEfiShellDynamicCommandProtocolGuid  = { 0x3c7200e9, 0x005f,
> 0x4ea4,
> > >{0x87, 0xde, 0xa3, 0xdf, 0xac, 0x8a, 0x27, 0xc3 }}
> > >
> > >+  ## Include/Protocol/Cpu2.h
> > >+  gEfiCpu2ProtocolGuid                 = { 0x55198405, 0x26C0,
> 0x4765, {0x8B,
> > 0x7D,
> > >0xBE, 0x1D, 0xF5, 0xF9, 0x97, 0x12 }}
> > >+
> > > #
> > > # [Error.gEfiMdePkgTokenSpaceGuid]
> > > #   0x80000001 | Invalid value provided.
> > >--
> > >2.21.0.windows.1
> 
> 
> 

Re: [edk2-devel] [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file

[Yao, Jiewen]

Sorry, it is CPU2 not CPU_IO2.

Right, I did not find CPU2 protocol in PI spec today.


> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, May 23, 2019 10:35 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid
> and header file
> 
> EFI_CPU_IO2_PROCOL is defined in PI spec Volume 5 Chapter 15 CPU I/O
> Protocol.
> 
> Thank you
> Yao jiewen
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Gao, Zhichao
> > Sent: Thursday, May 23, 2019 10:28 PM
> > To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
> > Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Michael Turner
> > <Michael.Turner@microsoft.com>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > Subject: Re: [edk2-devel] [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid
> > and header file
> >
> > I have found it in UEFP spec and PI spec. It is not defined in these specs.
> > So I think maybe no spec has already defined it.
> >
> > Thanks,
> > Zhichao
> >
> > > -----Original Message-----
> > > From: Gao, Liming
> > > Sent: Friday, May 24, 2019 1:20 PM
> > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > > Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>; Michael Turner
> > > <Michael.Turner@microsoft.com>; Bret Barkelew
> > > <Bret.Barkelew@microsoft.com>
> > > Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header
> > file
> > >
> > > Zhichao:
> > >   Which spec defines Cpu2 protocol?
> > >
> > > Thanks
> > > Liming
> > > >-----Original Message-----
> > > >From: Gao, Zhichao
> > > >Sent: Friday, May 24, 2019 1:05 PM
> > > >To: devel@edk2.groups.io
> > > >Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> > > ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> > > >Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> > > ><Bret.Barkelew@microsoft.com>
> > > >Subject: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header
> file
> > > >
> > > >From: Sean Brogan <sean.brogan@microsoft.com>
> > > >
> > > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
> > > >
> > > >Add gEfiCpu2ProtocolGuid to MdePkg.dec.
> > > >Add the header file of Cpu2 protocol: it has one interface to enable
> > > >interrupt and put cpu to sleep to wait for an interrupt.
> > > >
> > > >Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > >Cc: Liming Gao <liming.gao@intel.com>
> > > >Cc: Sean Brogan <sean.brogan@microsoft.com>
> > > >Cc: Michael Turner <Michael.Turner@microsoft.com>
> > > >Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > >Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > >---
> > > > MdePkg/Include/Protocol/Cpu2.h | 43
> > > >++++++++++++++++++++++++++++++++++
> > > > MdePkg/MdePkg.dec              |  3 +++
> > > > 2 files changed, 46 insertions(+)
> > > > create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> > > >
> > > >diff --git a/MdePkg/Include/Protocol/Cpu2.h
> > > >b/MdePkg/Include/Protocol/Cpu2.h new file mode 100644 index
> > > >0000000000..cacd948140
> > > >--- /dev/null
> > > >+++ b/MdePkg/Include/Protocol/Cpu2.h
> > > >@@ -0,0 +1,43 @@
> > > >+/** @file
> > > >+  CPU2 Protocol
> > > >+
> > > >+  This code abstracts the DXE core from processor implementation
> > details.
> > > >+
> > > >+  Copyright (c) 2006 - 2018, Microsoft Corporation. All rights
> > > >+ reserved.<BR>
> > > >+
> > > >+  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >+
> > > >+**/
> > > >+
> > > >+#ifndef __PROTOCOL_CPU2_H__
> > > >+#define __PROTOCOL_CPU2_H__
> > > >+
> > > >+#include <Uefi.h>
> > > >+
> > > >+typedef struct _EFI_CPU2_PROTOCOL   EFI_CPU2_PROTOCOL;
> > > >+
> > > >+
> > > >+/**
> > > >+  This function enables CPU interrupts and then waits for an interrupt
> > > >+to
> > > >arrive.
> > > >+
> > > >+  @param  This                  The EFI_CPU2_PROTOCOL
> > instance.
> > > >+
> > > >+  @retval EFI_SUCCESS           Interrupts are enabled on the
> > processor.
> > > >+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled
> > on the
> > > >processor.
> > > >+
> > > >+**/
> > > >+typedef
> > > >+EFI_STATUS
> > > >+(EFIAPI *EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT)(
> > > >+  IN EFI_CPU2_PROTOCOL              *This
> > > >+  );
> > > >+
> > > >+//
> > > >+// The EFI_CPU2_PROTOCOL is used to abstract processor-specific
> > > >+functions
> > > >from the DXE
> > > >+// Foundation.
> > > >+//
> > > >+struct _EFI_CPU2_PROTOCOL {
> > > >+  EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT
> > > >EnableAndWaitForInterrupt;
> > > >+};
> > > >+
> > > >+#endif
> > > >diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> > > >6c563375ee..e8c6939849 100644
> > > >--- a/MdePkg/MdePkg.dec
> > > >+++ b/MdePkg/MdePkg.dec
> > > >@@ -1803,6 +1803,9 @@
> > > >   ## Include/Protocol/ShellDynamicCommand.h
> > > >   gEfiShellDynamicCommandProtocolGuid  = { 0x3c7200e9, 0x005f,
> > 0x4ea4,
> > > >{0x87, 0xde, 0xa3, 0xdf, 0xac, 0x8a, 0x27, 0xc3 }}
> > > >
> > > >+  ## Include/Protocol/Cpu2.h
> > > >+  gEfiCpu2ProtocolGuid                 = { 0x55198405, 0x26C0,
> > 0x4765, {0x8B,
> > > 0x7D,
> > > >0xBE, 0x1D, 0xF5, 0xF9, 0x97, 0x12 }}
> > > >+
> > > > #
> > > > # [Error.gEfiMdePkgTokenSpaceGuid]
> > > > #   0x80000001 | Invalid value provided.
> > > >--
> > > >2.21.0.windows.1
> >
> >
> > 

Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file

[Liming Gao]

If no spec defines it, it belongs to edk2 implementation. It should be placed into other placement and name be with Edkii prefix instead of Uefi.

>-----Original Message-----
>From: Gao, Zhichao
>Sent: Friday, May 24, 2019 1:28 PM
>To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
>Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; Michael Turner
><Michael.Turner@microsoft.com>; Bret Barkelew
><Bret.Barkelew@microsoft.com>
>Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
>
>I have found it in UEFP spec and PI spec. It is not defined in these specs.
>So I think maybe no spec has already defined it.
>
>Thanks,
>Zhichao
>
>> -----Original Message-----
>> From: Gao, Liming
>> Sent: Friday, May 24, 2019 1:20 PM
>> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
>> Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Michael Turner
>> <Michael.Turner@microsoft.com>; Bret Barkelew
>> <Bret.Barkelew@microsoft.com>
>> Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header
>file
>>
>> Zhichao:
>>   Which spec defines Cpu2 protocol?
>>
>> Thanks
>> Liming
>> >-----Original Message-----
>> >From: Gao, Zhichao
>> >Sent: Friday, May 24, 2019 1:05 PM
>> >To: devel@edk2.groups.io
>> >Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
>> ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
>> >Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
>> ><Bret.Barkelew@microsoft.com>
>> >Subject: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
>> >
>> >From: Sean Brogan <sean.brogan@microsoft.com>
>> >
>> >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
>> >
>> >Add gEfiCpu2ProtocolGuid to MdePkg.dec.
>> >Add the header file of Cpu2 protocol: it has one interface to enable
>> >interrupt and put cpu to sleep to wait for an interrupt.
>> >
>> >Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> >Cc: Liming Gao <liming.gao@intel.com>
>> >Cc: Sean Brogan <sean.brogan@microsoft.com>
>> >Cc: Michael Turner <Michael.Turner@microsoft.com>
>> >Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
>> >Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>> >---
>> > MdePkg/Include/Protocol/Cpu2.h | 43
>> >++++++++++++++++++++++++++++++++++
>> > MdePkg/MdePkg.dec              |  3 +++
>> > 2 files changed, 46 insertions(+)
>> > create mode 100644 MdePkg/Include/Protocol/Cpu2.h
>> >
>> >diff --git a/MdePkg/Include/Protocol/Cpu2.h
>> >b/MdePkg/Include/Protocol/Cpu2.h new file mode 100644 index
>> >0000000000..cacd948140
>> >--- /dev/null
>> >+++ b/MdePkg/Include/Protocol/Cpu2.h
>> >@@ -0,0 +1,43 @@
>> >+/** @file
>> >+  CPU2 Protocol
>> >+
>> >+  This code abstracts the DXE core from processor implementation details.
>> >+
>> >+  Copyright (c) 2006 - 2018, Microsoft Corporation. All rights
>> >+ reserved.<BR>
>> >+
>> >+  SPDX-License-Identifier: BSD-2-Clause-Patent
>> >+
>> >+**/
>> >+
>> >+#ifndef __PROTOCOL_CPU2_H__
>> >+#define __PROTOCOL_CPU2_H__
>> >+
>> >+#include <Uefi.h>
>> >+
>> >+typedef struct _EFI_CPU2_PROTOCOL   EFI_CPU2_PROTOCOL;
>> >+
>> >+
>> >+/**
>> >+  This function enables CPU interrupts and then waits for an interrupt
>> >+to
>> >arrive.
>> >+
>> >+  @param  This                  The EFI_CPU2_PROTOCOL instance.
>> >+
>> >+  @retval EFI_SUCCESS           Interrupts are enabled on the processor.
>> >+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled on the
>> >processor.
>> >+
>> >+**/
>> >+typedef
>> >+EFI_STATUS
>> >+(EFIAPI *EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT)(
>> >+  IN EFI_CPU2_PROTOCOL              *This
>> >+  );
>> >+
>> >+//
>> >+// The EFI_CPU2_PROTOCOL is used to abstract processor-specific
>> >+functions
>> >from the DXE
>> >+// Foundation.
>> >+//
>> >+struct _EFI_CPU2_PROTOCOL {
>> >+  EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT
>> >EnableAndWaitForInterrupt;
>> >+};
>> >+
>> >+#endif
>> >diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
>> >6c563375ee..e8c6939849 100644
>> >--- a/MdePkg/MdePkg.dec
>> >+++ b/MdePkg/MdePkg.dec
>> >@@ -1803,6 +1803,9 @@
>> >   ## Include/Protocol/ShellDynamicCommand.h
>> >   gEfiShellDynamicCommandProtocolGuid  = { 0x3c7200e9, 0x005f, 0x4ea4,
>> >{0x87, 0xde, 0xa3, 0xdf, 0xac, 0x8a, 0x27, 0xc3 }}
>> >
>> >+  ## Include/Protocol/Cpu2.h
>> >+  gEfiCpu2ProtocolGuid                 = { 0x55198405, 0x26C0, 0x4765, {0x8B,
>> 0x7D,
>> >0xBE, 0x1D, 0xF5, 0xF9, 0x97, 0x12 }}
>> >+
>> > #
>> > # [Error.gEfiMdePkgTokenSpaceGuid]
>> > #   0x80000001 | Invalid value provided.
>> >--
>> >2.21.0.windows.1

Re: [PATCH 0/6] Fix race condition and add event protocol

[Gao, Zhichao]

Sorry I didn't add the BZ link.
https://bugzilla.tianocore.org/show_bug.cgi?id=1400 

"If an interrupt happens between CheckEvent and gIdleLoopEvent, there would be a event pending during cpu sleep."

Here is the wait for event flow:

Result = CheckEvent
(interrupt happens here, and set the gEventPending)
Result == FALSE
Ilde event (put cpu to sleep)(there are events in pending list and they should be handle, but the cpu is hlt and cannot handle it)

Solution is here:
Result = CheckEvent
(interrupt happens)
Disable interrupt
PendingResult = Check gEventPending
If gEventPending == TRUE {
  Enable interrupt and goto CheckEvent to handle
} else {
  Enable interrupt and hlt (this should be 'atomic' so that no race condition would happen)
}

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, May 24, 2019 1:18 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Sean Brogan <sean.brogan@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: RE: [PATCH 0/6] Fix race condition and add event protocol
> 
> Zhichao,
> Did your detailed patch commit message describe the consequence of the
> race condition? (I haven't checked.) If no, could you please describe in detail
> about the consequence?
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao
> > Sent: Friday, May 24, 2019 1:05 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan
> > <sean.brogan@microsoft.com>; Michael Turner
> > <Michael.Turner@microsoft.com>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo
> > Ersek <lersek@redhat.com>
> > Subject: [PATCH 0/6] Fix race condition and add event protocol
> >
> > There is a race condition in CoreWaitForEvent function:
> > If an interrupt happens between CheckEvent and gIdleLoopEvent, there
> > would be a event pending during cpu sleep.
> > So it is required to check the gEventPending with the interrupt disabled.
> > Implement a gEfiCpu2ProtocolGuid to fix that. The protocol include one
> > interface to enable interrupt and put the cpu to sleep.
> >
> > Add a event protocol gEdkiiCommonEventProtocolGuid to support all TPL
> > event. It is require for PI drivers that use HW interrput.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming gao <liming.gao@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> >
> > Sean Brogan (5):
> >   MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event
> >   MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare
> >   MdePkg/BaseLib: Implement EnableInterruptsAndSleep
> >   MdePkg: Add gEfiCpu2ProtocolGuid and header file
> >   MdeModulePkg/DxeMain: Implement common event protocol
> >
> > Zhichao Gao (1):
> >   UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
> >
> >  MdeModulePkg/Core/Dxe/DxeMain.h               |  64 ++++++++++-
> >  MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
> >  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c       |  22 ++++
> >  .../Core/Dxe/DxeMain/DxeProtocolNotify.c      |   1 +
> >  MdeModulePkg/Core/Dxe/Event/Event.c           | 102 ++++++++++++++++-
> -
> >  MdeModulePkg/Core/Dxe/Event/Event.h           |   2 +-
> >  MdeModulePkg/Include/Protocol/CommonEvent.h   |  18 ++++
> >  MdeModulePkg/MdeModulePkg.dec                 |   3 +
> >  MdePkg/Include/Library/BaseLib.h              |  11 ++
> >  MdePkg/Include/Protocol/Cpu2.h                |  43 ++++++++
> >  .../Library/BaseLib/Ia32/EnableInterrupts.c   |  18 +++-
> >  .../BaseLib/Ia32/EnableInterrupts.nasm        |  15 ++-
> >  .../Library/BaseLib/X64/EnableInterrupts.nasm |  15 ++-
> >  MdePkg/MdePkg.dec                             |   3 +
> >  UefiCpuPkg/CpuDxe/CpuDxe.c                    |  40 ++++++-
> >  UefiCpuPkg/CpuDxe/CpuDxe.h                    |  15 +++
> >  UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   3 +-
> >  17 files changed, 358 insertions(+), 18 deletions(-)  create mode
> > 100644 MdeModulePkg/Include/Protocol/CommonEvent.h
> >  create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> >
> > --
> > 2.21.0.windows.1

Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file

[Gao, Zhichao]

I would move the definition to MdeModulePkg and change the prefix.

Thanks,
Zhichao

> -----Original Message-----
> From: Gao, Liming
> Sent: Friday, May 24, 2019 2:02 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
> 
> If no spec defines it, it belongs to edk2 implementation. It should be placed
> into other placement and name be with Edkii prefix instead of Uefi.
> 
> >-----Original Message-----
> >From: Gao, Zhichao
> >Sent: Friday, May 24, 2019 1:28 PM
> >To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
> >Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> ><michael.d.kinney@intel.com>; Michael Turner
> ><Michael.Turner@microsoft.com>; Bret Barkelew
> ><Bret.Barkelew@microsoft.com>
> >Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header
> >file
> >
> >I have found it in UEFP spec and PI spec. It is not defined in these specs.
> >So I think maybe no spec has already defined it.
> >
> >Thanks,
> >Zhichao
> >
> >> -----Original Message-----
> >> From: Gao, Liming
> >> Sent: Friday, May 24, 2019 1:20 PM
> >> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> >> Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Michael Turner
> >> <Michael.Turner@microsoft.com>; Bret Barkelew
> >> <Bret.Barkelew@microsoft.com>
> >> Subject: RE: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header
> >file
> >>
> >> Zhichao:
> >>   Which spec defines Cpu2 protocol?
> >>
> >> Thanks
> >> Liming
> >> >-----Original Message-----
> >> >From: Gao, Zhichao
> >> >Sent: Friday, May 24, 2019 1:05 PM
> >> >To: devel@edk2.groups.io
> >> >Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> >> ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> >> >Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> >> ><Bret.Barkelew@microsoft.com>
> >> >Subject: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header
> >> >file
> >> >
> >> >From: Sean Brogan <sean.brogan@microsoft.com>
> >> >
> >> >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
> >> >
> >> >Add gEfiCpu2ProtocolGuid to MdePkg.dec.
> >> >Add the header file of Cpu2 protocol: it has one interface to enable
> >> >interrupt and put cpu to sleep to wait for an interrupt.
> >> >
> >> >Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >> >Cc: Liming Gao <liming.gao@intel.com>
> >> >Cc: Sean Brogan <sean.brogan@microsoft.com>
> >> >Cc: Michael Turner <Michael.Turner@microsoft.com>
> >> >Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >> >Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> >> >---
> >> > MdePkg/Include/Protocol/Cpu2.h | 43
> >> >++++++++++++++++++++++++++++++++++
> >> > MdePkg/MdePkg.dec              |  3 +++
> >> > 2 files changed, 46 insertions(+)
> >> > create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> >> >
> >> >diff --git a/MdePkg/Include/Protocol/Cpu2.h
> >> >b/MdePkg/Include/Protocol/Cpu2.h new file mode 100644 index
> >> >0000000000..cacd948140
> >> >--- /dev/null
> >> >+++ b/MdePkg/Include/Protocol/Cpu2.h
> >> >@@ -0,0 +1,43 @@
> >> >+/** @file
> >> >+  CPU2 Protocol
> >> >+
> >> >+  This code abstracts the DXE core from processor implementation
> details.
> >> >+
> >> >+  Copyright (c) 2006 - 2018, Microsoft Corporation. All rights
> >> >+ reserved.<BR>
> >> >+
> >> >+  SPDX-License-Identifier: BSD-2-Clause-Patent
> >> >+
> >> >+**/
> >> >+
> >> >+#ifndef __PROTOCOL_CPU2_H__
> >> >+#define __PROTOCOL_CPU2_H__
> >> >+
> >> >+#include <Uefi.h>
> >> >+
> >> >+typedef struct _EFI_CPU2_PROTOCOL   EFI_CPU2_PROTOCOL;
> >> >+
> >> >+
> >> >+/**
> >> >+  This function enables CPU interrupts and then waits for an
> >> >+interrupt to
> >> >arrive.
> >> >+
> >> >+  @param  This                  The EFI_CPU2_PROTOCOL instance.
> >> >+
> >> >+  @retval EFI_SUCCESS           Interrupts are enabled on the processor.
> >> >+  @retval EFI_DEVICE_ERROR      Interrupts could not be enabled on the
> >> >processor.
> >> >+
> >> >+**/
> >> >+typedef
> >> >+EFI_STATUS
> >> >+(EFIAPI *EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT)(
> >> >+  IN EFI_CPU2_PROTOCOL              *This
> >> >+  );
> >> >+
> >> >+//
> >> >+// The EFI_CPU2_PROTOCOL is used to abstract processor-specific
> >> >+functions
> >> >from the DXE
> >> >+// Foundation.
> >> >+//
> >> >+struct _EFI_CPU2_PROTOCOL {
> >> >+  EFI_CPU_ENABLE_AND_WAIT_FOR_INTERRUPT
> >> >EnableAndWaitForInterrupt;
> >> >+};
> >> >+
> >> >+#endif
> >> >diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> >> >6c563375ee..e8c6939849 100644
> >> >--- a/MdePkg/MdePkg.dec
> >> >+++ b/MdePkg/MdePkg.dec
> >> >@@ -1803,6 +1803,9 @@
> >> >   ## Include/Protocol/ShellDynamicCommand.h
> >> >   gEfiShellDynamicCommandProtocolGuid  = { 0x3c7200e9, 0x005f,
> >> >0x4ea4, {0x87, 0xde, 0xa3, 0xdf, 0xac, 0x8a, 0x27, 0xc3 }}
> >> >
> >> >+  ## Include/Protocol/Cpu2.h
> >> >+  gEfiCpu2ProtocolGuid                 = { 0x55198405, 0x26C0, 0x4765, {0x8B,
> >> 0x7D,
> >> >0xBE, 0x1D, 0xF5, 0xF9, 0x97, 0x12 }}
> >> >+
> >> > #
> >> > # [Error.gEfiMdePkgTokenSpaceGuid]
> >> > #   0x80000001 | Invalid value provided.
> >> >--
> >> >2.21.0.windows.1

Re: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol

[Laszlo Ersek]

On 05/24/19 07:04, Gao, Zhichao wrote:
> There is a race condition in CoreWaitForEvent function:
> If an interrupt happens between CheckEvent and gIdleLoopEvent,
> there would be a event pending during cpu sleep. 
> So it is required to check the gEventPending with the interrupt
> disabled.
> Implement a gEfiCpu2ProtocolGuid to fix that. The protocol include
> one interface to enable interrupt and put the cpu to sleep.
> 
> Add a event protocol gEdkiiCommonEventProtocolGuid to support
> all TPL event. It is require for PI drivers that use HW interrput.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>

Thanks for the CC; I'll let others review this series.

Thanks
Laszlo

> Sean Brogan (5):
>   MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event
>   MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare
>   MdePkg/BaseLib: Implement EnableInterruptsAndSleep
>   MdePkg: Add gEfiCpu2ProtocolGuid and header file
>   MdeModulePkg/DxeMain: Implement common event protocol
> 
> Zhichao Gao (1):
>   UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
> 
>  MdeModulePkg/Core/Dxe/DxeMain.h               |  64 ++++++++++-
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c       |  22 ++++
>  .../Core/Dxe/DxeMain/DxeProtocolNotify.c      |   1 +
>  MdeModulePkg/Core/Dxe/Event/Event.c           | 102 ++++++++++++++++--
>  MdeModulePkg/Core/Dxe/Event/Event.h           |   2 +-
>  MdeModulePkg/Include/Protocol/CommonEvent.h   |  18 ++++
>  MdeModulePkg/MdeModulePkg.dec                 |   3 +
>  MdePkg/Include/Library/BaseLib.h              |  11 ++
>  MdePkg/Include/Protocol/Cpu2.h                |  43 ++++++++
>  .../Library/BaseLib/Ia32/EnableInterrupts.c   |  18 +++-
>  .../BaseLib/Ia32/EnableInterrupts.nasm        |  15 ++-
>  .../Library/BaseLib/X64/EnableInterrupts.nasm |  15 ++-
>  MdePkg/MdePkg.dec                             |   3 +
>  UefiCpuPkg/CpuDxe/CpuDxe.c                    |  40 ++++++-
>  UefiCpuPkg/CpuDxe/CpuDxe.h                    |  15 +++
>  UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   3 +-
>  17 files changed, 358 insertions(+), 18 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Protocol/CommonEvent.h
>  create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> 

Re: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol

[Felix Polyudov]

I think there is a bigger problem with the idle event signaling by CoreWaitForEvent.
On every iteration CoreWaitForEvent checks user events, and, if none is signaled, the function signals special idle event.
This itself is not a problem. However, on many (most?) platforms CPU driver installs idle event handler that calls CpuSleep
(f.i. refer to IdleLoopEventCallback in UefiCpuPkg/CpuDxe/CpuDxe.c and ArmPkg/Drivers/CpuDxe/CpuDxe.c).
CpuSleep "places the CPU in a sleep state until an interrupt is received", which changes the nature of the WaitForEvent function arguably violating the UEFI specification.

According to the UEFI specification:
"The list of events in the Event array are evaluated in order from first to last, and this evaluation is repeated until an event is signaled or an error is detected."

The sentence above implies continues evaluation (polling model).
An idle event callback that sends CPU to a sleep state, converts WaitForEvent from
a continues polling function into a poll-once-per-timer-period function (timer is typically the only enabled HW interrupt),
which reduces quality of service provided by WaitForEvent and can lead to missed events.

For example, UEFI application that reads key strokes using gBS->WaitForEvent(..)/ConIn->ReadKey(...) sequence
will be losing key strokes if they are coming faster than 18.2 keys per second on a system based on legacy timer.

So, should idle event support be removed from CoreWaitForEvent?

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gao, Zhichao
Sent: Friday, May 24, 2019 1:05 AM
To: devel@edk2.groups.io
Cc: Jian J Wang; Hao A Wu; Ray Ni; Star Zeng; Liming gao; Sean Brogan; Michael Turner; Bret Barkelew; Michael D Kinney; Eric Dong; Laszlo Ersek
Subject: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol

There is a race condition in CoreWaitForEvent function:
If an interrupt happens between CheckEvent and gIdleLoopEvent,
there would be a event pending during cpu sleep.
So it is required to check the gEventPending with the interrupt
disabled.
Implement a gEfiCpu2ProtocolGuid to fix that. The protocol include
one interface to enable interrupt and put the cpu to sleep.

Add a event protocol gEdkiiCommonEventProtocolGuid to support
all TPL event. It is require for PI drivers that use HW interrput.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

Sean Brogan (5):
  MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event
  MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare
  MdePkg/BaseLib: Implement EnableInterruptsAndSleep
  MdePkg: Add gEfiCpu2ProtocolGuid and header file
  MdeModulePkg/DxeMain: Implement common event protocol

Zhichao Gao (1):
  UefiCpuPkg/CpuDxe: Implement Cpu2 protocol

 MdeModulePkg/Core/Dxe/DxeMain.h               |  64 ++++++++++-
 MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c       |  22 ++++
 .../Core/Dxe/DxeMain/DxeProtocolNotify.c      |   1 +
 MdeModulePkg/Core/Dxe/Event/Event.c           | 102 ++++++++++++++++--
 MdeModulePkg/Core/Dxe/Event/Event.h           |   2 +-
 MdeModulePkg/Include/Protocol/CommonEvent.h   |  18 ++++
 MdeModulePkg/MdeModulePkg.dec                 |   3 +
 MdePkg/Include/Library/BaseLib.h              |  11 ++
 MdePkg/Include/Protocol/Cpu2.h                |  43 ++++++++
 .../Library/BaseLib/Ia32/EnableInterrupts.c   |  18 +++-
 .../BaseLib/Ia32/EnableInterrupts.nasm        |  15 ++-
 .../Library/BaseLib/X64/EnableInterrupts.nasm |  15 ++-
 MdePkg/MdePkg.dec                             |   3 +
 UefiCpuPkg/CpuDxe/CpuDxe.c                    |  40 ++++++-
 UefiCpuPkg/CpuDxe/CpuDxe.h                    |  15 +++
 UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   3 +-
 17 files changed, 358 insertions(+), 18 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/CommonEvent.h
 create mode 100644 MdePkg/Include/Protocol/Cpu2.h

--
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41302): https://edk2.groups.io/g/devel/message/41302
Mute This Topic: https://groups.io/mt/31741727/1498468
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [felixp@ami.com]
-=-=-=-=-=-=-=-=-=-=-=-


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends, Inc.  This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

Re: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol

[Liming Gao]

Felix:
  Sorry for the late response. 

> -----Original Message-----
> From: Felix Polyudov [mailto:Felixp@ami.com]
> Sent: Friday, May 24, 2019 8:53 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Andrew Fish (afish@apple.com) <afish@apple.com>
> Subject: RE: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol
> 
> I think there is a bigger problem with the idle event signaling by CoreWaitForEvent.
> On every iteration CoreWaitForEvent checks user events, and, if none is signaled, the function signals special idle event.
> This itself is not a problem. However, on many (most?) platforms CPU driver installs idle event handler that calls CpuSleep
> (f.i. refer to IdleLoopEventCallback in UefiCpuPkg/CpuDxe/CpuDxe.c and ArmPkg/Drivers/CpuDxe/CpuDxe.c).
> CpuSleep "places the CPU in a sleep state until an interrupt is received", which changes the nature of the WaitForEvent function arguably
> violating the UEFI specification.
> 
> According to the UEFI specification:
> "The list of events in the Event array are evaluated in order from first to last, and this evaluation is repeated until an event is signaled or an
> error is detected."
> 
> The sentence above implies continues evaluation (polling model).

Here is repeat. It is not identical to polling model. A check per timer period is also a repeat implementation. So, it follows UEFI spec.

> An idle event callback that sends CPU to a sleep state, converts WaitForEvent from
> a continues polling function into a poll-once-per-timer-period function (timer is typically the only enabled HW interrupt),
> which reduces quality of service provided by WaitForEvent and can lead to missed events.
> 
> For example, UEFI application that reads key strokes using gBS->WaitForEvent(..)/ConIn->ReadKey(...) sequence
> will be losing key strokes if they are coming faster than 18.2 keys per second on a system based on legacy timer.

This case may not be real case. Idle event is added since 2011 year @54cd17e9842d82dae3cd78686e05c4dc37a3540d. 
I don't get the real issue report. Have you meet with some real issue in the production?

> 
> So, should idle event support be removed from CoreWaitForEvent?
> 
I think we can still keep it. It is edk2 implementation.

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gao, Zhichao
> Sent: Friday, May 24, 2019 1:05 AM
> To: devel@edk2.groups.io
> Cc: Jian J Wang; Hao A Wu; Ray Ni; Star Zeng; Liming gao; Sean Brogan; Michael Turner; Bret Barkelew; Michael D Kinney; Eric Dong;
> Laszlo Ersek
> Subject: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol
> 
> There is a race condition in CoreWaitForEvent function:
> If an interrupt happens between CheckEvent and gIdleLoopEvent,
> there would be a event pending during cpu sleep.
> So it is required to check the gEventPending with the interrupt
> disabled.
> Implement a gEfiCpu2ProtocolGuid to fix that. The protocol include
> one interface to enable interrupt and put the cpu to sleep.
> 
> Add a event protocol gEdkiiCommonEventProtocolGuid to support
> all TPL event. It is require for PI drivers that use HW interrput.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> 
> Sean Brogan (5):
>   MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event
>   MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare
>   MdePkg/BaseLib: Implement EnableInterruptsAndSleep
>   MdePkg: Add gEfiCpu2ProtocolGuid and header file
>   MdeModulePkg/DxeMain: Implement common event protocol
> 
> Zhichao Gao (1):
>   UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
> 
>  MdeModulePkg/Core/Dxe/DxeMain.h               |  64 ++++++++++-
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c       |  22 ++++
>  .../Core/Dxe/DxeMain/DxeProtocolNotify.c      |   1 +
>  MdeModulePkg/Core/Dxe/Event/Event.c           | 102 ++++++++++++++++--
>  MdeModulePkg/Core/Dxe/Event/Event.h           |   2 +-
>  MdeModulePkg/Include/Protocol/CommonEvent.h   |  18 ++++
>  MdeModulePkg/MdeModulePkg.dec                 |   3 +
>  MdePkg/Include/Library/BaseLib.h              |  11 ++
>  MdePkg/Include/Protocol/Cpu2.h                |  43 ++++++++
>  .../Library/BaseLib/Ia32/EnableInterrupts.c   |  18 +++-
>  .../BaseLib/Ia32/EnableInterrupts.nasm        |  15 ++-
>  .../Library/BaseLib/X64/EnableInterrupts.nasm |  15 ++-
>  MdePkg/MdePkg.dec                             |   3 +
>  UefiCpuPkg/CpuDxe/CpuDxe.c                    |  40 ++++++-
>  UefiCpuPkg/CpuDxe/CpuDxe.h                    |  15 +++
>  UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   3 +-
>  17 files changed, 358 insertions(+), 18 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Protocol/CommonEvent.h
>  create mode 100644 MdePkg/Include/Protocol/Cpu2.h
> 
> --
> 2.21.0.windows.1
> 
> 
> 
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and proprietary to American Megatrends, Inc.  This communication is
> intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the
> intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the
> sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.