Re: [PATCH] EmulatorPkg: Fix bugs about MiscBootServices

["Ni, Ray" <ray.ni@intel.com>]

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