* [PATCH 1/6] MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
@ 2019-05-24 5:04 ` Gao, Zhichao
2019-05-24 5:04 ` [PATCH 2/6] MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare Gao, Zhichao
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 5:04 UTC (permalink / raw)
To: devel
Cc: Sean Brogan, Jian J Wang, Hao A Wu, Ray Ni, Star Zeng, Liming gao,
Michael Turner, Bret Barkelew
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/6] MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
2019-05-24 5:04 ` [PATCH 1/6] MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event Gao, Zhichao
@ 2019-05-24 5:04 ` Gao, Zhichao
2019-05-24 5:04 ` [PATCH 3/6] MdePkg/BaseLib: Implement EnableInterruptsAndSleep Gao, Zhichao
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 5:04 UTC (permalink / raw)
To: devel
Cc: Sean Brogan, Michael D Kinney, Liming Gao, Michael Turner,
Bret Barkelew
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 3/6] MdePkg/BaseLib: Implement EnableInterruptsAndSleep
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
2019-05-24 5:04 ` [PATCH 1/6] MdeModulePkg: Add gEdkiiCommonEventProtocolGuid for event Gao, Zhichao
2019-05-24 5:04 ` [PATCH 2/6] MdePkg/BaseLib.h: Add EnableInterruptsAndSleep function declare Gao, Zhichao
@ 2019-05-24 5:04 ` Gao, Zhichao
2019-05-24 5:04 ` [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file Gao, Zhichao
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 5:04 UTC (permalink / raw)
To: devel
Cc: Sean Brogan, Michael D Kinney, Liming Gao, Michael Turner,
Bret Barkelew
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
` (2 preceding siblings ...)
2019-05-24 5:04 ` [PATCH 3/6] MdePkg/BaseLib: Implement EnableInterruptsAndSleep Gao, Zhichao
@ 2019-05-24 5:04 ` Gao, Zhichao
2019-05-24 5:20 ` Liming Gao
2019-05-24 5:04 ` [PATCH 5/6] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol Gao, Zhichao
` (4 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 5:04 UTC (permalink / raw)
To: devel
Cc: Sean Brogan, Michael D Kinney, Liming Gao, Michael Turner,
Bret Barkelew
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
2019-05-24 5:04 ` [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file Gao, Zhichao
@ 2019-05-24 5:20 ` Liming Gao
2019-05-24 5:27 ` Gao, Zhichao
0 siblings, 1 reply; 18+ messages in thread
From: Liming Gao @ 2019-05-24 5:20 UTC (permalink / raw)
To: Gao, Zhichao, devel@edk2.groups.io
Cc: Sean Brogan, Kinney, Michael D, Michael Turner, Bret Barkelew
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
2019-05-24 5:20 ` Liming Gao
@ 2019-05-24 5:27 ` Gao, Zhichao
2019-05-24 5:34 ` [edk2-devel] " Yao, Jiewen
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 5:27 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io
Cc: Sean Brogan, Kinney, Michael D, Michael Turner, Bret Barkelew
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [edk2-devel] [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
2019-05-24 5:27 ` Gao, Zhichao
@ 2019-05-24 5:34 ` Yao, Jiewen
2019-05-24 5:38 ` Yao, Jiewen
2019-05-24 6:02 ` Liming Gao
2 siblings, 0 replies; 18+ messages in thread
From: Yao, Jiewen @ 2019-05-24 5:34 UTC (permalink / raw)
To: devel@edk2.groups.io, Gao, Zhichao, Gao, Liming
Cc: Sean Brogan, Kinney, Michael D, Michael Turner, Bret Barkelew
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
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [edk2-devel] [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
2019-05-24 5:27 ` Gao, Zhichao
2019-05-24 5:34 ` [edk2-devel] " Yao, Jiewen
@ 2019-05-24 5:38 ` Yao, Jiewen
2019-05-24 6:02 ` Liming Gao
2 siblings, 0 replies; 18+ messages in thread
From: Yao, Jiewen @ 2019-05-24 5:38 UTC (permalink / raw)
To: devel@edk2.groups.io, Gao, Zhichao, Gao, Liming
Cc: Sean Brogan, Kinney, Michael D, Michael Turner, Bret Barkelew
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
> >
> >
> >
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
2019-05-24 5:27 ` Gao, Zhichao
2019-05-24 5:34 ` [edk2-devel] " Yao, Jiewen
2019-05-24 5:38 ` Yao, Jiewen
@ 2019-05-24 6:02 ` Liming Gao
2019-05-24 7:48 ` Gao, Zhichao
2 siblings, 1 reply; 18+ messages in thread
From: Liming Gao @ 2019-05-24 6:02 UTC (permalink / raw)
To: Gao, Zhichao, devel@edk2.groups.io
Cc: Sean Brogan, Kinney, Michael D, Michael Turner, Bret Barkelew
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file
2019-05-24 6:02 ` Liming Gao
@ 2019-05-24 7:48 ` Gao, Zhichao
0 siblings, 0 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 7:48 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io
Cc: Sean Brogan, Kinney, Michael D, Michael Turner, Bret Barkelew
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 5/6] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
` (3 preceding siblings ...)
2019-05-24 5:04 ` [PATCH 4/6] MdePkg: Add gEfiCpu2ProtocolGuid and header file Gao, Zhichao
@ 2019-05-24 5:04 ` Gao, Zhichao
2019-05-24 5:04 ` [PATCH 6/6] MdeModulePkg/DxeMain: Implement common event protocol Gao, Zhichao
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 5:04 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Laszlo Ersek, Liming Gao, Sean Brogan,
Michael Turner, Bret Barkelew
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 6/6] MdeModulePkg/DxeMain: Implement common event protocol
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
` (4 preceding siblings ...)
2019-05-24 5:04 ` [PATCH 5/6] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol Gao, Zhichao
@ 2019-05-24 5:04 ` Gao, Zhichao
2019-05-24 5:17 ` [PATCH 0/6] Fix race condition and add " Ni, Ray
` (2 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 5:04 UTC (permalink / raw)
To: devel
Cc: Sean Brogan, Jian J Wang, Hao A Wu, Ray Ni, Star Zeng, Liming gao,
Michael Turner, Bret Barkelew
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 0/6] Fix race condition and add event protocol
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
` (5 preceding siblings ...)
2019-05-24 5:04 ` [PATCH 6/6] MdeModulePkg/DxeMain: Implement common event protocol Gao, Zhichao
@ 2019-05-24 5:17 ` Ni, Ray
2019-05-24 7:47 ` Gao, Zhichao
2019-05-24 9:43 ` [edk2-devel] " Laszlo Ersek
2019-05-24 12:52 ` Felix Polyudov
8 siblings, 1 reply; 18+ messages in thread
From: Ni, Ray @ 2019-05-24 5:17 UTC (permalink / raw)
To: Gao, Zhichao, devel@edk2.groups.io
Cc: Wang, Jian J, Wu, Hao A, Zeng, Star, Gao, Liming, Sean Brogan,
Michael Turner, Bret Barkelew, Kinney, Michael D, Dong, Eric,
Laszlo Ersek
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 0/6] Fix race condition and add event protocol
2019-05-24 5:17 ` [PATCH 0/6] Fix race condition and add " Ni, Ray
@ 2019-05-24 7:47 ` Gao, Zhichao
0 siblings, 0 replies; 18+ messages in thread
From: Gao, Zhichao @ 2019-05-24 7:47 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
Cc: Wang, Jian J, Wu, Hao A, Zeng, Star, Gao, Liming, Sean Brogan,
Michael Turner, Bret Barkelew, Kinney, Michael D, Dong, Eric,
Laszlo Ersek
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
` (6 preceding siblings ...)
2019-05-24 5:17 ` [PATCH 0/6] Fix race condition and add " Ni, Ray
@ 2019-05-24 9:43 ` Laszlo Ersek
2019-05-24 12:52 ` Felix Polyudov
8 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2019-05-24 9:43 UTC (permalink / raw)
To: devel, zhichao.gao
Cc: Jian J Wang, Hao A Wu, Ray Ni, Star Zeng, Liming gao, Sean Brogan,
Michael Turner, Bret Barkelew, Michael D Kinney, Eric Dong
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
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol
2019-05-24 5:04 [PATCH 0/6] Fix race condition and add event protocol Gao, Zhichao
` (7 preceding siblings ...)
2019-05-24 9:43 ` [edk2-devel] " Laszlo Ersek
@ 2019-05-24 12:52 ` Felix Polyudov
2019-06-13 16:22 ` Liming Gao
8 siblings, 1 reply; 18+ messages in thread
From: Felix Polyudov @ 2019-05-24 12:52 UTC (permalink / raw)
To: devel@edk2.groups.io, 'zhichao.gao@intel.com'
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, Andrew Fish (afish@apple.com)
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.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [edk2-devel] [PATCH 0/6] Fix race condition and add event protocol
2019-05-24 12:52 ` Felix Polyudov
@ 2019-06-13 16:22 ` Liming Gao
0 siblings, 0 replies; 18+ messages in thread
From: Liming Gao @ 2019-06-13 16:22 UTC (permalink / raw)
To: Felix Polyudov, devel@edk2.groups.io, Gao, Zhichao
Cc: Wang, Jian J, Wu, Hao A, Ni, Ray, Zeng, Star, Sean Brogan,
Michael Turner, Bret Barkelew, Kinney, Michael D, Dong, Eric,
Laszlo Ersek, Andrew Fish (afish@apple.com)
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.
^ permalink raw reply [flat|nested] 18+ messages in thread