* [PATCH] EmulatorPkg: Fix bugs about MiscBootServices
@ 2019-05-24 8:17 Zhiguang Liu
2019-05-24 9:08 ` Ni, Ray
0 siblings, 1 reply; 3+ messages in thread
From: Zhiguang Liu @ 2019-05-24 8:17 UTC (permalink / raw)
To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1757
This patch fix two issues.
1.
The previous code "NanoSecondDelay (MicroSeconds * 1000)"
may cause data overflow.
2.
Delete some code in
/EmulatorPkg/ResetRuntimeDxe/Reset.c
which may cause Tpl problems when invoked by watchdog.
I think it is ok to delete these code, because it will
be more like what NT32 does.
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
---
EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c | 14 +++++++++++++-
EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c | 14 +++++++++++++-
EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c | 14 +++++++++++++-
EmulatorPkg/ResetRuntimeDxe/Reset.c | 24 ------------------------
4 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
index c331cbba9c..813963de7b 100644
--- a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
+++ b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
@@ -33,7 +33,19 @@ MicroSecondDelay (
IN UINTN MicroSeconds
)
{
- return NanoSecondDelay (MicroSeconds * 1000);
+ UINTN Remainder;
+ UINTN Counter;
+ UINTN Index;
+ Counter = (UINTN) DivU64x32Remainder (
+ MultU64x32 (MicroSeconds, 1000),
+ 0xffffffff,
+ &Remainder
+ );
+ for (Index = 0; Index < Counter; Index++) {
+ NanoSecondDelay (0xffffffff);
+ }
+ NanoSecondDelay (Remainder);
+ return MicroSeconds;
}
diff --git a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
index 14cae4214c..590ce55fae 100644
--- a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
+++ b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
@@ -64,7 +64,19 @@ MicroSecondDelay (
IN UINTN MicroSeconds
)
{
- return NanoSecondDelay (MicroSeconds * 1000);
+ UINTN Remainder;
+ UINTN Counter;
+ UINTN Index;
+ Counter = (UINTN) DivU64x32Remainder (
+ MultU64x32 (MicroSeconds, 1000),
+ 0xffffffff,
+ &Remainder
+ );
+ for (Index = 0; Index < Counter; Index++) {
+ NanoSecondDelay (0xffffffff);
+ }
+ NanoSecondDelay (Remainder);
+ return MicroSeconds;
}
diff --git a/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c b/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
index cce46fb366..dcac32a51f 100644
--- a/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
+++ b/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
@@ -30,7 +30,19 @@ MicroSecondDelay (
IN UINTN MicroSeconds
)
{
- return NanoSecondDelay (MicroSeconds * 1000);
+ UINTN Remainder;
+ UINTN Counter;
+ UINTN Index;
+ Counter = (UINTN) DivU64x32Remainder (
+ MultU64x32 (MicroSeconds, 1000),
+ 0xffffffff,
+ &Remainder
+ );
+ for (Index = 0; Index < Counter; Index++) {
+ NanoSecondDelay (0xffffffff);
+ }
+ NanoSecondDelay (Remainder);
+ return MicroSeconds;
}
/**
diff --git a/EmulatorPkg/ResetRuntimeDxe/Reset.c b/EmulatorPkg/ResetRuntimeDxe/Reset.c
index 19504825c9..9439f9ccff 100644
--- a/EmulatorPkg/ResetRuntimeDxe/Reset.c
+++ b/EmulatorPkg/ResetRuntimeDxe/Reset.c
@@ -29,30 +29,6 @@ EmuResetSystem (
IN VOID *ResetData OPTIONAL
)
{
- EFI_STATUS Status;
- UINTN HandleCount;
- EFI_HANDLE *HandleBuffer;
- UINTN Index;
-
- //
- // Disconnect all
- //
- Status = gBS->LocateHandleBuffer (
- AllHandles,
- NULL,
- NULL,
- &HandleCount,
- &HandleBuffer
- );
- if (!EFI_ERROR (Status)) {
- for (Index = 0; Index < HandleCount; Index++) {
- Status = gBS->DisconnectController (HandleBuffer[Index], NULL, NULL);
- }
-
- gBS->FreePool (HandleBuffer);
- }
-
-
//
// Discard ResetType, always return 0 as exit code
//
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] EmulatorPkg: Fix bugs about MiscBootServices
2019-05-24 8:17 [PATCH] EmulatorPkg: Fix bugs about MiscBootServices Zhiguang Liu
@ 2019-05-24 9:08 ` Ni, Ray
2019-05-27 0:31 ` Zhiguang Liu
0 siblings, 1 reply; 3+ messages in thread
From: Ni, Ray @ 2019-05-24 9:08 UTC (permalink / raw)
To: Liu, Zhiguang, devel@edk2.groups.io; +Cc: Justen, Jordan L, Andrew Fish
Zhiguang,
For the first issue, where does the fix come from?
Or you did the implementation yourself?
I ask this because I want to make sure the code is
from an existing tested implementation.
Thanks,
Ray
> -----Original Message-----
> From: Liu, Zhiguang
> Sent: Friday, May 24, 2019 4:18 PM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Andrew Fish
> <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH] EmulatorPkg: Fix bugs about MiscBootServices
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1757
>
> This patch fix two issues.
> 1.
> The previous code "NanoSecondDelay (MicroSeconds * 1000)"
> may cause data overflow.
> 2.
> Delete some code in
> /EmulatorPkg/ResetRuntimeDxe/Reset.c
> which may cause Tpl problems when invoked by watchdog.
> I think it is ok to delete these code, because it will
> be more like what NT32 does.
>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
> EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c | 14
> +++++++++++++-
> EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c | 14 +++++++++++++-
> EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c | 14 +++++++++++++-
> EmulatorPkg/ResetRuntimeDxe/Reset.c | 24 ------------------------
> 4 files changed, 39 insertions(+), 27 deletions(-)
>
> diff --git a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> index c331cbba9c..813963de7b 100644
> --- a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> +++ b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> @@ -33,7 +33,19 @@ MicroSecondDelay (
> IN UINTN MicroSeconds
> )
> {
> - return NanoSecondDelay (MicroSeconds * 1000);
> + UINTN Remainder;
> + UINTN Counter;
> + UINTN Index;
> + Counter = (UINTN) DivU64x32Remainder (
> + MultU64x32 (MicroSeconds, 1000),
> + 0xffffffff,
> + &Remainder
> + );
> + for (Index = 0; Index < Counter; Index++) {
> + NanoSecondDelay (0xffffffff);
> + }
> + NanoSecondDelay (Remainder);
> + return MicroSeconds;
> }
>
>
> diff --git a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> index 14cae4214c..590ce55fae 100644
> --- a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> +++ b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> @@ -64,7 +64,19 @@ MicroSecondDelay (
> IN UINTN MicroSeconds
> )
> {
> - return NanoSecondDelay (MicroSeconds * 1000);
> + UINTN Remainder;
> + UINTN Counter;
> + UINTN Index;
> + Counter = (UINTN) DivU64x32Remainder (
> + MultU64x32 (MicroSeconds, 1000),
> + 0xffffffff,
> + &Remainder
> + );
> + for (Index = 0; Index < Counter; Index++) {
> + NanoSecondDelay (0xffffffff);
> + }
> + NanoSecondDelay (Remainder);
> + return MicroSeconds;
> }
>
>
> diff --git a/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
> b/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
> index cce46fb366..dcac32a51f 100644
> --- a/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
> +++ b/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
> @@ -30,7 +30,19 @@ MicroSecondDelay (
> IN UINTN MicroSeconds
> )
> {
> - return NanoSecondDelay (MicroSeconds * 1000);
> + UINTN Remainder;
> + UINTN Counter;
> + UINTN Index;
> + Counter = (UINTN) DivU64x32Remainder (
> + MultU64x32 (MicroSeconds, 1000),
> + 0xffffffff,
> + &Remainder
> + );
> + for (Index = 0; Index < Counter; Index++) {
> + NanoSecondDelay (0xffffffff);
> + }
> + NanoSecondDelay (Remainder);
> + return MicroSeconds;
> }
>
> /**
> diff --git a/EmulatorPkg/ResetRuntimeDxe/Reset.c
> b/EmulatorPkg/ResetRuntimeDxe/Reset.c
> index 19504825c9..9439f9ccff 100644
> --- a/EmulatorPkg/ResetRuntimeDxe/Reset.c
> +++ b/EmulatorPkg/ResetRuntimeDxe/Reset.c
> @@ -29,30 +29,6 @@ EmuResetSystem (
> IN VOID *ResetData OPTIONAL
> )
> {
> - EFI_STATUS Status;
> - UINTN HandleCount;
> - EFI_HANDLE *HandleBuffer;
> - UINTN Index;
> -
> - //
> - // Disconnect all
> - //
> - Status = gBS->LocateHandleBuffer (
> - AllHandles,
> - NULL,
> - NULL,
> - &HandleCount,
> - &HandleBuffer
> - );
> - if (!EFI_ERROR (Status)) {
> - for (Index = 0; Index < HandleCount; Index++) {
> - Status = gBS->DisconnectController (HandleBuffer[Index], NULL, NULL);
> - }
> -
> - gBS->FreePool (HandleBuffer);
> - }
> -
> -
> //
> // Discard ResetType, always return 0 as exit code
> //
> --
> 2.21.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] EmulatorPkg: Fix bugs about MiscBootServices
2019-05-24 9:08 ` Ni, Ray
@ 2019-05-27 0:31 ` Zhiguang Liu
0 siblings, 0 replies; 3+ messages in thread
From: Zhiguang Liu @ 2019-05-27 0:31 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io; +Cc: Justen, Jordan L, Andrew Fish
Hello, Ray
I did the implementation myself.
I just found there did be the same function in
\Omap35xxPkg\Library\Omap35xxTimerLib\TimerLib.c
In old version.
Should I use the existing code like following
\Omap35xxPkg\Library\Omap35xxTimerLib\TimerLib.c
UINTN
EFIAPI
MicroSecondDelay (
IN UINTN MicroSeconds
)
{
UINT64 NanoSeconds;
NanoSeconds = MultU64x32(MicroSeconds, 1000);
while (NanoSeconds > (UINTN)-1) {
NanoSecondDelay((UINTN)-1);
NanoSeconds -= (UINTN)-1;
}
NanoSecondDelay(NanoSeconds);
return MicroSeconds;
}
Best Regards,
Zhiguang Liu
>-----Original Message-----
>From: Ni, Ray
>Sent: Friday, May 24, 2019 5:08 PM
>To: Liu, Zhiguang <zhiguang.liu@intel.com>; devel@edk2.groups.io
>Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Andrew Fish
><afish@apple.com>
>Subject: RE: [PATCH] EmulatorPkg: Fix bugs about MiscBootServices
>
>Zhiguang,
>For the first issue, where does the fix come from?
>Or you did the implementation yourself?
>
>I ask this because I want to make sure the code is from an existing tested
>implementation.
>
>Thanks,
>Ray
>
>> -----Original Message-----
>> From: Liu, Zhiguang
>> Sent: Friday, May 24, 2019 4:18 PM
>> To: devel@edk2.groups.io
>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Andrew Fish
>> <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
>> Subject: [PATCH] EmulatorPkg: Fix bugs about MiscBootServices
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1757
>>
>> This patch fix two issues.
>> 1.
>> The previous code "NanoSecondDelay (MicroSeconds * 1000)"
>> may cause data overflow.
>> 2.
>> Delete some code in
>> /EmulatorPkg/ResetRuntimeDxe/Reset.c
>> which may cause Tpl problems when invoked by watchdog.
>> I think it is ok to delete these code, because it will
>> be more like what NT32 does.
>>
>> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Andrew Fish <afish@apple.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> ---
>> EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c | 14
>> +++++++++++++-
>> EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c | 14 +++++++++++++-
>> EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c | 14 +++++++++++++-
>> EmulatorPkg/ResetRuntimeDxe/Reset.c | 24 ------------------------
>> 4 files changed, 39 insertions(+), 27 deletions(-)
>>
>> diff --git a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
>> b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
>> index c331cbba9c..813963de7b 100644
>> --- a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
>> +++ b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
>> @@ -33,7 +33,19 @@ MicroSecondDelay (
>> IN UINTN MicroSeconds
>> )
>> {
>> - return NanoSecondDelay (MicroSeconds * 1000);
>> + UINTN Remainder;
>> + UINTN Counter;
>> + UINTN Index;
>> + Counter = (UINTN) DivU64x32Remainder (
>> + MultU64x32 (MicroSeconds, 1000),
>> + 0xffffffff,
>> + &Remainder
>> + );
>> + for (Index = 0; Index < Counter; Index++) {
>> + NanoSecondDelay (0xffffffff);
>> + }
>> + NanoSecondDelay (Remainder);
>> + return MicroSeconds;
>> }
>>
>>
>> diff --git a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
>> b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
>> index 14cae4214c..590ce55fae 100644
>> --- a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
>> +++ b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
>> @@ -64,7 +64,19 @@ MicroSecondDelay (
>> IN UINTN MicroSeconds
>> )
>> {
>> - return NanoSecondDelay (MicroSeconds * 1000);
>> + UINTN Remainder;
>> + UINTN Counter;
>> + UINTN Index;
>> + Counter = (UINTN) DivU64x32Remainder (
>> + MultU64x32 (MicroSeconds, 1000),
>> + 0xffffffff,
>> + &Remainder
>> + );
>> + for (Index = 0; Index < Counter; Index++) {
>> + NanoSecondDelay (0xffffffff);
>> + }
>> + NanoSecondDelay (Remainder);
>> + return MicroSeconds;
>> }
>>
>>
>> diff --git a/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
>> b/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
>> index cce46fb366..dcac32a51f 100644
>> --- a/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
>> +++ b/EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c
>> @@ -30,7 +30,19 @@ MicroSecondDelay (
>> IN UINTN MicroSeconds
>> )
>> {
>> - return NanoSecondDelay (MicroSeconds * 1000);
>> + UINTN Remainder;
>> + UINTN Counter;
>> + UINTN Index;
>> + Counter = (UINTN) DivU64x32Remainder (
>> + MultU64x32 (MicroSeconds, 1000),
>> + 0xffffffff,
>> + &Remainder
>> + );
>> + for (Index = 0; Index < Counter; Index++) {
>> + NanoSecondDelay (0xffffffff);
>> + }
>> + NanoSecondDelay (Remainder);
>> + return MicroSeconds;
>> }
>>
>> /**
>> diff --git a/EmulatorPkg/ResetRuntimeDxe/Reset.c
>> b/EmulatorPkg/ResetRuntimeDxe/Reset.c
>> index 19504825c9..9439f9ccff 100644
>> --- a/EmulatorPkg/ResetRuntimeDxe/Reset.c
>> +++ b/EmulatorPkg/ResetRuntimeDxe/Reset.c
>> @@ -29,30 +29,6 @@ EmuResetSystem (
>> IN VOID *ResetData OPTIONAL
>> )
>> {
>> - EFI_STATUS Status;
>> - UINTN HandleCount;
>> - EFI_HANDLE *HandleBuffer;
>> - UINTN Index;
>> -
>> - //
>> - // Disconnect all
>> - //
>> - Status = gBS->LocateHandleBuffer (
>> - AllHandles,
>> - NULL,
>> - NULL,
>> - &HandleCount,
>> - &HandleBuffer
>> - );
>> - if (!EFI_ERROR (Status)) {
>> - for (Index = 0; Index < HandleCount; Index++) {
>> - Status = gBS->DisconnectController (HandleBuffer[Index], NULL, NULL);
>> - }
>> -
>> - gBS->FreePool (HandleBuffer);
>> - }
>> -
>> -
>> //
>> // Discard ResetType, always return 0 as exit code
>> //
>> --
>> 2.21.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-27 0:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-24 8:17 [PATCH] EmulatorPkg: Fix bugs about MiscBootServices Zhiguang Liu
2019-05-24 9:08 ` Ni, Ray
2019-05-27 0:31 ` Zhiguang Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox