* [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