On 2014-11-20 21:09:03, Chen Fan wrote: > Because TimeoutInMicrosecsond is a unsigned value, converting it to > signed value will cause the data region changed. so this patch fix > that. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Chen Fan <[email protected]> > --- > EmulatorPkg/CpuRuntimeDxe/MpService.c | 41 > ++++++++++++++++++++++++++--------- > 1 file changed, 31 insertions(+), 10 deletions(-) > > diff --git a/EmulatorPkg/CpuRuntimeDxe/MpService.c > b/EmulatorPkg/CpuRuntimeDxe/MpService.c > index d6dd984..db49a45 100644 > --- a/EmulatorPkg/CpuRuntimeDxe/MpService.c > +++ b/EmulatorPkg/CpuRuntimeDxe/MpService.c > @@ -111,8 +111,31 @@ GetNextBlockedNumber ( > return EFI_NOT_FOUND; > } > > +/** > + * Calculated and stalled the interval time by BSP to check whether > + * the APs have finished. > + * > + * @param[in] Timeout The time limit in microseconds for > + * APs to return from Procedure. > + * > + * @retval StallTime Time of execution stall. > +**/ > +UINTN > +CalculateAndStallInterval ( > + IN UINTN Timeout > + ) > +{ > + UINTN StallTime; > > + if (Timeout < gPollInterval && Timeout != 0) { > + StallTime = Timeout; > + } else { > + StallTime = gPollInterval; > + } > + gBS->Stall (StallTime); > > + return StallTime; > +}
I committed your series, but I think there is still a case where this
behaves (a little) unexpectedly. (But, not incorrectly.)
I think that if the timeout is set to unlimited (0), then the Timeout
variable will still be decremented by gPollInterval, except when the
value has nearly wrapped around.
On a 32-bit system, I think this will occur 0x100000000/1000000, or
about every 4294 seconds. So, pretty rarely, and on a 64-bit system it
is not worth even considering.
But, in that rare case, Timeout may be < gPollInterval, yet we should
expect to always stall gPollInterval microseconds in the case of an
unlimited timeout.
I don't think this rare case is actually handled incorectly by your
code, so I went ahead and committed your series.
Thanks for the contribution!
-Jordan
> /**
> This service retrieves the number of logical processor in the platform
> @@ -378,7 +401,7 @@ CpuMpServicesStartupAllAps (
> UINTN NextNumber;
> PROCESSOR_STATE APInitialState;
> PROCESSOR_STATE ProcessorState;
> - INTN Timeout;
> + UINTN Timeout;
>
>
> if (!IsBSP ()) {
> @@ -540,13 +563,12 @@ CpuMpServicesStartupAllAps (
> goto Done;
> }
>
> - if ((TimeoutInMicroseconds != 0) && (Timeout < 0)) {
> + if ((TimeoutInMicroseconds != 0) && (Timeout == 0)) {
> Status = EFI_TIMEOUT;
> goto Done;
> }
>
> - gBS->Stall (gPollInterval);
> - Timeout -= gPollInterval;
> + Timeout -= CalculateAndStallInterval (Timeout);
> }
>
> Done:
> @@ -659,7 +681,7 @@ CpuMpServicesStartupThisAP (
> OUT BOOLEAN *Finished OPTIONAL
> )
> {
> - INTN Timeout;
> + UINTN Timeout;
>
> if (!IsBSP ()) {
> return EFI_DEVICE_ERROR;
> @@ -717,12 +739,11 @@ CpuMpServicesStartupThisAP (
>
> gThread->MutexUnlock
> (gMPSystem.ProcessorData[ProcessorNumber].StateLock);
>
> - if ((TimeoutInMicroseconds != 0) && (Timeout < 0)) {
> + if ((TimeoutInMicroseconds != 0) && (Timeout == 0)) {
> return EFI_TIMEOUT;
> }
>
> - gBS->Stall (gPollInterval);
> - Timeout -= gPollInterval;
> + Timeout -= CalculateAndStallInterval (Timeout);
> }
>
> return EFI_SUCCESS;
> @@ -987,7 +1008,7 @@ CpuCheckAllAPsStatus (
> BOOLEAN Found;
>
> if (gMPSystem.TimeoutActive) {
> - gMPSystem.Timeout -= gPollInterval;
> + gMPSystem.Timeout -= CalculateAndStallInterval (gMPSystem.Timeout);
> }
>
> for (ProcessorNumber = 0; ProcessorNumber < gMPSystem.NumberOfProcessors;
> ProcessorNumber++) {
> @@ -1040,7 +1061,7 @@ CpuCheckAllAPsStatus (
> }
> }
>
> - if (gMPSystem.TimeoutActive && gMPSystem.Timeout < 0) {
> + if (gMPSystem.TimeoutActive && gMPSystem.Timeout == 0) {
> //
> // Timeout
> //
> --
> 1.9.3
>
>
> ------------------------------------------------------------------------------
> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> with Interactivity, Sharing, Native Excel Exports, App Integration & more
> Get technology previously reserved for billion-dollar corporations, FREE
> http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
signature.asc
Description: signature
------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
