[edk2-devel] [PATCH 1/5] PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe: rename LibRtcVirtualNotifyEvent

Laszlo Ersek posted 5 patches 2 years, 4 months ago
[edk2-devel] [PATCH 1/5] PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe: rename LibRtcVirtualNotifyEvent
Posted by Laszlo Ersek 2 years, 4 months ago
PcatRealTimeClockRuntimeDxe seems to have copied the interface name
LibRtcVirtualNotifyEvent() from EmbeddedPkg's RealTimeClockLib class.
That's not right, for two reasons:

- PcatRealTimeClockRuntimeDxe doesn't consume "EmbeddedPkg.dec" in the
  first place,

- in EmbeddedPkg, the RealTimeClockLib class API
  LibRtcVirtualNotifyEvent() is about to be eliminated (it's a bogus API).

Rename the LibRtcVirtualNotifyEvent() function to VirtualNotifyEvent(),
and make it static.

Tested with booting OVMF.

Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4564
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
index b600df487952..ca0cad9b0198 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
@@ -122,9 +122,10 @@ PcRtcEfiSetWakeupTime (
   @param[in]    Event   The Event that is being processed
   @param[in]    Context Event Context
 **/
+STATIC
 VOID
 EFIAPI
-LibRtcVirtualNotifyEvent (
+VirtualNotifyEvent (
   IN EFI_EVENT  Event,
   IN VOID       *Context
   )
@@ -220,7 +221,7 @@ InitializePcRtc (
     Status = gBS->CreateEventEx (
                     EVT_NOTIFY_SIGNAL,
                     TPL_NOTIFY,
-                    LibRtcVirtualNotifyEvent,
+                    VirtualNotifyEvent,
                     NULL,
                     &gEfiEventVirtualAddressChangeGuid,
                     &mVirtualAddrChangeEvent



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109562): https://edk2.groups.io/g/devel/message/109562
Mute This Topic: https://groups.io/mt/101914676/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 1/5] PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe: rename LibRtcVirtualNotifyEvent
Posted by Sami Mujawar 2 years, 4 months ago
Hi Laszlo,

Thank you for this patch. 
These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Tested by booting a Linux guest using Kvmtool guest firmware (ArmVirtPkg/ArmVirtKvmTool.dsc) for AArch64.
Tested-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 12/10/2023, 10:25, "devel@edk2.groups.io <mailto:devel@edk2.groups.io> on behalf of Laszlo Ersek via groups.io" <devel@edk2.groups.io <mailto:devel@edk2.groups.io> on behalf of lersek=redhat.com@groups.io <mailto:redhat.com@groups.io>> wrote:


PcatRealTimeClockRuntimeDxe seems to have copied the interface name
LibRtcVirtualNotifyEvent() from EmbeddedPkg's RealTimeClockLib class.
That's not right, for two reasons:


- PcatRealTimeClockRuntimeDxe doesn't consume "EmbeddedPkg.dec" in the
first place,


- in EmbeddedPkg, the RealTimeClockLib class API
LibRtcVirtualNotifyEvent() is about to be eliminated (it's a bogus API).


Rename the LibRtcVirtualNotifyEvent() function to VirtualNotifyEvent(),
and make it static.


Tested with booting OVMF.


Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com>>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4564 <https://bugzilla.tianocore.org/show_bug.cgi?id=4564>
Signed-off-by: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>
---
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)


diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
index b600df487952..ca0cad9b0198 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
@@ -122,9 +122,10 @@ PcRtcEfiSetWakeupTime (
@param[in] Event The Event that is being processed
@param[in] Context Event Context
**/
+STATIC
VOID
EFIAPI
-LibRtcVirtualNotifyEvent (
+VirtualNotifyEvent (
IN EFI_EVENT Event,
IN VOID *Context
)
@@ -220,7 +221,7 @@ InitializePcRtc (
Status = gBS->CreateEventEx (
EVT_NOTIFY_SIGNAL,
TPL_NOTIFY,
- LibRtcVirtualNotifyEvent,
+ VirtualNotifyEvent,
NULL,
&gEfiEventVirtualAddressChangeGuid,
&mVirtualAddrChangeEvent
















-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109573): https://edk2.groups.io/g/devel/message/109573
Mute This Topic: https://groups.io/mt/101914676/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 1/5] PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe: rename LibRtcVirtualNotifyEvent
Posted by Laszlo Ersek 2 years, 3 months ago
Hi Sami,

On 10/12/23 14:33, Sami Mujawar wrote:
> Hi Laszlo,
> 
> Thank you for this patch. 
> These changes look good to me.
> 
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> 
> Tested by booting a Linux guest using Kvmtool guest firmware (ArmVirtPkg/ArmVirtKvmTool.dsc) for AArch64.
> Tested-by: Sami Mujawar <sami.mujawar@arm.com>

your response is under a PcAtChipsetPkg patch -- that's not used in
"ArmVirtPkg/ArmVirtKvmTool.dsc".

Which patches did you mean your Tested-by for?

Thanks!
Laszlo

> 
> Regards,
> 
> Sami Mujawar
> 
> On 12/10/2023, 10:25, "devel@edk2.groups.io <mailto:devel@edk2.groups.io> on behalf of Laszlo Ersek via groups.io" <devel@edk2.groups.io <mailto:devel@edk2.groups.io> on behalf of lersek=redhat.com@groups.io <mailto:redhat.com@groups.io>> wrote:
> 
> 
> PcatRealTimeClockRuntimeDxe seems to have copied the interface name
> LibRtcVirtualNotifyEvent() from EmbeddedPkg's RealTimeClockLib class.
> That's not right, for two reasons:
> 
> 
> - PcatRealTimeClockRuntimeDxe doesn't consume "EmbeddedPkg.dec" in the
> first place,
> 
> 
> - in EmbeddedPkg, the RealTimeClockLib class API
> LibRtcVirtualNotifyEvent() is about to be eliminated (it's a bogus API).
> 
> 
> Rename the LibRtcVirtualNotifyEvent() function to VirtualNotifyEvent(),
> and make it static.
> 
> 
> Tested with booting OVMF.
> 
> 
> Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com>>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4564 <https://bugzilla.tianocore.org/show_bug.cgi?id=4564>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>
> ---
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> index b600df487952..ca0cad9b0198 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> @@ -122,9 +122,10 @@ PcRtcEfiSetWakeupTime (
> @param[in] Event The Event that is being processed
> @param[in] Context Event Context
> **/
> +STATIC
> VOID
> EFIAPI
> -LibRtcVirtualNotifyEvent (
> +VirtualNotifyEvent (
> IN EFI_EVENT Event,
> IN VOID *Context
> )
> @@ -220,7 +221,7 @@ InitializePcRtc (
> Status = gBS->CreateEventEx (
> EVT_NOTIFY_SIGNAL,
> TPL_NOTIFY,
> - LibRtcVirtualNotifyEvent,
> + VirtualNotifyEvent,
> NULL,
> &gEfiEventVirtualAddressChangeGuid,
> &mVirtualAddrChangeEvent
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109750): https://edk2.groups.io/g/devel/message/109750
Mute This Topic: https://groups.io/mt/101914676/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 1/5] PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe: rename LibRtcVirtualNotifyEvent
Posted by Sami Mujawar 2 years, 3 months ago
Hi Laszlo,

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 18/10/2023, 14:42, "Laszlo Ersek" <lersek@redhat.com <mailto:lersek@redhat.com>> wrote:


Hi Sami,


On 10/12/23 14:33, Sami Mujawar wrote:
> Hi Laszlo,
> 
> Thank you for this patch. 
> These changes look good to me.
> 
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>
> 
> Tested by booting a Linux guest using Kvmtool guest firmware (ArmVirtPkg/ArmVirtKvmTool.dsc) for AArch64.
> Tested-by: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>


your response is under a PcAtChipsetPkg patch -- that's not used in
"ArmVirtPkg/ArmVirtKvmTool.dsc".


Which patches did you mean your Tested-by for?
[SAMI] Sorry for not being clear. PcatRealTimeClockRuntimeDxe is used by the Kvmtool guest firmware, 
please see https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtKvmTool.dsc#L285-L288
I have applied this patch and tested that it works for the Kvmtool guest firmware.
[/SAMI]

Thanks!
Laszlo


> 
> Regards,
> 
> Sami Mujawar
> 
> On 12/10/2023, 10:25, "devel@edk2.groups.io <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>> on behalf of Laszlo Ersek via groups.io" <devel@edk2.groups.io <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>> on behalf of lersek=redhat.com@groups.io <mailto:redhat.com@groups.io> <mailto:redhat.com@groups.io <mailto:redhat.com@groups.io>>> wrote:
> 
> 
> PcatRealTimeClockRuntimeDxe seems to have copied the interface name
> LibRtcVirtualNotifyEvent() from EmbeddedPkg's RealTimeClockLib class.
> That's not right, for two reasons:
> 
> 
> - PcatRealTimeClockRuntimeDxe doesn't consume "EmbeddedPkg.dec" in the
> first place,
> 
> 
> - in EmbeddedPkg, the RealTimeClockLib class API
> LibRtcVirtualNotifyEvent() is about to be eliminated (it's a bogus API).
> 
> 
> Rename the LibRtcVirtualNotifyEvent() function to VirtualNotifyEvent(),
> and make it static.
> 
> 
> Tested with booting OVMF.
> 
> 
> Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com> <mailto:ray.ni@intel.com <mailto:ray.ni@intel.com>>>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4564 <https://bugzilla.tianocore.org/show_bug.cgi?id=4564> <https://bugzilla.tianocore.org/show_bug.cgi?id=4564> <https://bugzilla.tianocore.org/show_bug.cgi?id=4564&gt;>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com> <mailto:lersek@redhat.com <mailto:lersek@redhat.com>>>
> ---
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> index b600df487952..ca0cad9b0198 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> @@ -122,9 +122,10 @@ PcRtcEfiSetWakeupTime (
> @param[in] Event The Event that is being processed
> @param[in] Context Event Context
> **/
> +STATIC
> VOID
> EFIAPI
> -LibRtcVirtualNotifyEvent (
> +VirtualNotifyEvent (
> IN EFI_EVENT Event,
> IN VOID *Context
> )
> @@ -220,7 +221,7 @@ InitializePcRtc (
> Status = gBS->CreateEventEx (
> EVT_NOTIFY_SIGNAL,
> TPL_NOTIFY,
> - LibRtcVirtualNotifyEvent,
> + VirtualNotifyEvent,
> NULL,
> &gEfiEventVirtualAddressChangeGuid,
> &mVirtualAddrChangeEvent
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109763): https://edk2.groups.io/g/devel/message/109763
Mute This Topic: https://groups.io/mt/101914676/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 1/5] PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe: rename LibRtcVirtualNotifyEvent
Posted by Laszlo Ersek 2 years, 3 months ago
On 10/18/23 17:15, Sami Mujawar wrote:
> Hi Laszlo,
> 
> Please see my response inline marked [SAMI].
> 
> Regards,
> 
> Sami Mujawar
> 
> On 18/10/2023, 14:42, "Laszlo Ersek" <lersek@redhat.com <mailto:lersek@redhat.com>> wrote:
> 
> 
> Hi Sami,
> 
> 
> On 10/12/23 14:33, Sami Mujawar wrote:
>> Hi Laszlo,
>>
>> Thank you for this patch. 
>> These changes look good to me.
>>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>
>>
>> Tested by booting a Linux guest using Kvmtool guest firmware (ArmVirtPkg/ArmVirtKvmTool.dsc) for AArch64.
>> Tested-by: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>
> 
> 
> your response is under a PcAtChipsetPkg patch -- that's not used in
> "ArmVirtPkg/ArmVirtKvmTool.dsc".
> 
> 
> Which patches did you mean your Tested-by for?
> [SAMI] Sorry for not being clear. PcatRealTimeClockRuntimeDxe is used by the Kvmtool guest firmware, 
> please see https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtKvmTool.dsc#L285-L288
> I have applied this patch and tested that it works for the Kvmtool guest firmware.
> [/SAMI]

And right you are! Wow... that's unexpected of ArmVirtPkg content!

Thanks!
Laszlo

> 
> Thanks!
> Laszlo
> 
> 
>>
>> Regards,
>>
>> Sami Mujawar
>>
>> On 12/10/2023, 10:25, "devel@edk2.groups.io <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>> on behalf of Laszlo Ersek via groups.io" <devel@edk2.groups.io <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>> on behalf of lersek=redhat.com@groups.io <mailto:redhat.com@groups.io> <mailto:redhat.com@groups.io <mailto:redhat.com@groups.io>>> wrote:
>>
>>
>> PcatRealTimeClockRuntimeDxe seems to have copied the interface name
>> LibRtcVirtualNotifyEvent() from EmbeddedPkg's RealTimeClockLib class.
>> That's not right, for two reasons:
>>
>>
>> - PcatRealTimeClockRuntimeDxe doesn't consume "EmbeddedPkg.dec" in the
>> first place,
>>
>>
>> - in EmbeddedPkg, the RealTimeClockLib class API
>> LibRtcVirtualNotifyEvent() is about to be eliminated (it's a bogus API).
>>
>>
>> Rename the LibRtcVirtualNotifyEvent() function to VirtualNotifyEvent(),
>> and make it static.
>>
>>
>> Tested with booting OVMF.
>>
>>
>> Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com> <mailto:ray.ni@intel.com <mailto:ray.ni@intel.com>>>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4564 <https://bugzilla.tianocore.org/show_bug.cgi?id=4564> <https://bugzilla.tianocore.org/show_bug.cgi?id=4564> <https://bugzilla.tianocore.org/show_bug.cgi?id=4564&gt;>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com> <mailto:lersek@redhat.com <mailto:lersek@redhat.com>>>
>> ---
>> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
>> index b600df487952..ca0cad9b0198 100644
>> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
>> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
>> @@ -122,9 +122,10 @@ PcRtcEfiSetWakeupTime (
>> @param[in] Event The Event that is being processed
>> @param[in] Context Event Context
>> **/
>> +STATIC
>> VOID
>> EFIAPI
>> -LibRtcVirtualNotifyEvent (
>> +VirtualNotifyEvent (
>> IN EFI_EVENT Event,
>> IN VOID *Context
>> )
>> @@ -220,7 +221,7 @@ InitializePcRtc (
>> Status = gBS->CreateEventEx (
>> EVT_NOTIFY_SIGNAL,
>> TPL_NOTIFY,
>> - LibRtcVirtualNotifyEvent,
>> + VirtualNotifyEvent,
>> NULL,
>> &gEfiEventVirtualAddressChangeGuid,
>> &mVirtualAddrChangeEvent
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> 
>>
>>
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109764): https://edk2.groups.io/g/devel/message/109764
Mute This Topic: https://groups.io/mt/101914676/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-