On Fri, 2014-11-21 at 15:01 -0800, Jordan Justen wrote:
> 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,
Chen
>
> 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
------------------------------------------------------------------------------
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
