Re: [edk2-devel] [PATCH 11/35] MdeModulePkg: document workaround for EFI_RUNTIME_EVENT_ENTRY PI spec bug

2019-09-18 Thread Dandan Bi 
Reviewed-by: Dandan Bi 

Thanks,
Dandan

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, September 18, 2019 3:49 AM
> To: edk2-devel-groups-io 
> Cc: Bi, Dandan ; Wu, Hao A ;
> Wang, Jian J ; Gao, Liming 
> Subject: [PATCH 11/35] MdeModulePkg: document workaround for
> EFI_RUNTIME_EVENT_ENTRY PI spec bug
> 
> The PI spec (v1.7) correctly specifies "EFI_RUNTIME_EVENT_ENTRY.Event" in
> natural language, but the field type in the structure definition itself is 
> wrong -
> - it should be EFI_EVENT, not (EFI_EVENT*).
> 
> This spec bug is likely unfixable for compatibility reasons, and so edk2 works
> it around already. We should clearly document the workaround.
> 
> Functionally, this patch is a no-op.
> 
> (I've also requested a non-normative (informative) clarification for the PI
> spec: .)
> 
> Cc: Dandan Bi 
> Cc: Hao A Wu 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> lightly tested, as these modules are part of the ArmVirt and/or OVMF
> platforms
> 
>  MdeModulePkg/Core/Dxe/Event/Event.c|  8 
>  MdeModulePkg/Core/RuntimeDxe/Runtime.c | 10 +-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Event/Event.c
> b/MdeModulePkg/Core/Dxe/Event/Event.c
> index 21db38aaf037..c83c572c8f84 100644
> --- a/MdeModulePkg/Core/Dxe/Event/Event.c
> +++ b/MdeModulePkg/Core/Dxe/Event/Event.c
> @@ -485,6 +485,14 @@ CoreCreateEventInternal (
>  IEvent->RuntimeData.NotifyTpl  = NotifyTpl;
>  IEvent->RuntimeData.NotifyFunction = NotifyFunction;
>  IEvent->RuntimeData.NotifyContext  = (VOID *) NotifyContext;
> +//
> +// Work around the bug in the Platform Init specification (v1.7), 
> reported
> +// as Mantis#2017: "EFI_RUNTIME_EVENT_ENTRY.Event" should have
> type
> +// EFI_EVENT, not (EFI_EVENT*). The PI spec documents the field
> correctly
> +// as "The EFI_EVENT returned by CreateEvent()", but the type of the
> field
> +// doesn't match the natural language description. Therefore we need an
> +// explicit cast here.
> +//
>  IEvent->RuntimeData.Event  = (EFI_EVENT *) IEvent;
>  InsertTailList (>EventHead, >RuntimeData.Link);
>}
> diff --git a/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> index c52b2b7ecf68..f7220a205d1e 100644
> --- a/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> +++ b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> @@ -285,8 +285,16 @@ RuntimeDriverSetVirtualAddressMap (
>for (Link = mRuntime.EventHead.ForwardLink; Link !=
>  Link = Link->ForwardLink) {
>  RuntimeEvent = BASE_CR (Link, EFI_RUNTIME_EVENT_ENTRY, Link);
>  if ((RuntimeEvent->Type & EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE) ==
> EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE) {
> +  //
> +  // Work around the bug in the Platform Init specification (v1.7),
> +  // reported as Mantis#2017: "EFI_RUNTIME_EVENT_ENTRY.Event"
> should have
> +  // type EFI_EVENT, not (EFI_EVENT*). The PI spec documents the field
> +  // correctly as "The EFI_EVENT returned by CreateEvent()", but the type
> +  // of the field doesn't match the natural language description. 
> Therefore
> +  // we need an explicit cast here.
> +  //
>RuntimeEvent->NotifyFunction (
> -  RuntimeEvent->Event,
> +  (EFI_EVENT) RuntimeEvent->Event,
>RuntimeEvent->NotifyContext
>);
>  }
> --
> 2.19.1.3.g30247aa5d201
> 


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

View/Reply Online (#47520): https://edk2.groups.io/g/devel/message/47520
Mute This Topic: https://groups.io/mt/34180212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 11/35] MdeModulePkg: document workaround for EFI_RUNTIME_EVENT_ENTRY PI spec bug

2019-09-17 Thread Laszlo Ersek 
The PI spec (v1.7) correctly specifies "EFI_RUNTIME_EVENT_ENTRY.Event" in
natural language, but the field type in the structure definition itself is
wrong -- it should be EFI_EVENT, not (EFI_EVENT*).

This spec bug is likely unfixable for compatibility reasons, and so edk2
works it around already. We should clearly document the workaround.

Functionally, this patch is a no-op.

(I've also requested a non-normative (informative) clarification for the
PI spec: .)

Cc: Dandan Bi 
Cc: Hao A Wu 
Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Laszlo Ersek 
---

Notes:
lightly tested, as these modules are part of the ArmVirt and/or OVMF
platforms

 MdeModulePkg/Core/Dxe/Event/Event.c|  8 
 MdeModulePkg/Core/RuntimeDxe/Runtime.c | 10 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Event/Event.c 
b/MdeModulePkg/Core/Dxe/Event/Event.c
index 21db38aaf037..c83c572c8f84 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.c
+++ b/MdeModulePkg/Core/Dxe/Event/Event.c
@@ -485,6 +485,14 @@ CoreCreateEventInternal (
 IEvent->RuntimeData.NotifyTpl  = NotifyTpl;
 IEvent->RuntimeData.NotifyFunction = NotifyFunction;
 IEvent->RuntimeData.NotifyContext  = (VOID *) NotifyContext;
+//
+// Work around the bug in the Platform Init specification (v1.7), reported
+// as Mantis#2017: "EFI_RUNTIME_EVENT_ENTRY.Event" should have type
+// EFI_EVENT, not (EFI_EVENT*). The PI spec documents the field correctly
+// as "The EFI_EVENT returned by CreateEvent()", but the type of the field
+// doesn't match the natural language description. Therefore we need an
+// explicit cast here.
+//
 IEvent->RuntimeData.Event  = (EFI_EVENT *) IEvent;
 InsertTailList (>EventHead, >RuntimeData.Link);
   }
diff --git a/MdeModulePkg/Core/RuntimeDxe/Runtime.c 
b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
index c52b2b7ecf68..f7220a205d1e 100644
--- a/MdeModulePkg/Core/RuntimeDxe/Runtime.c
+++ b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
@@ -285,8 +285,16 @@ RuntimeDriverSetVirtualAddressMap (
   for (Link = mRuntime.EventHead.ForwardLink; Link !=  
Link = Link->ForwardLink) {
 RuntimeEvent = BASE_CR (Link, EFI_RUNTIME_EVENT_ENTRY, Link);
 if ((RuntimeEvent->Type & EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE) == 
EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE) {
+  //
+  // Work around the bug in the Platform Init specification (v1.7),
+  // reported as Mantis#2017: "EFI_RUNTIME_EVENT_ENTRY.Event" should have
+  // type EFI_EVENT, not (EFI_EVENT*). The PI spec documents the field
+  // correctly as "The EFI_EVENT returned by CreateEvent()", but the type
+  // of the field doesn't match the natural language description. Therefore
+  // we need an explicit cast here.
+  //
   RuntimeEvent->NotifyFunction (
-  RuntimeEvent->Event,
+  (EFI_EVENT) RuntimeEvent->Event,
   RuntimeEvent->NotifyContext
   );
 }
-- 
2.19.1.3.g30247aa5d201



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

View/Reply Online (#47398): https://edk2.groups.io/g/devel/message/47398
Mute This Topic: https://groups.io/mt/34180212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-