On Tue, Feb 21, 2023 at 10:37:23AM +0530, Sunil V L via groups.io wrote:
> Hi Andrei,
>
> Thank you very much for fixing this!
>
> On Fri, Feb 17, 2023 at 10:31:49PM -0600, Andrei Warkentin wrote:
> > The TimerDxe implementation doesn't account for the physical
> > time passed due to timer handler execution or (perhaps even
> > more importantly) time spent with interrupts masked.
> >
> > Other implementations (e.g. like the Arm one) do. If the
> > timer tick is always incremented at a fixed rate, then
> > you can slow down UEFI's perception of time by running
> > long sections of code in a critical section.
> >
> > Cc: Sunil V L <suni...@ventanamicro.com>
> > Cc: Daniel Schaefer <g...@danielschaefer.me>
> > Signed-off-by: Andrei Warkentin <andrei.warken...@intel.com>
> > ---
> > UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c | 31 ++++++++++++--------
> > 1 file changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
> > b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
> > index 0ecefddf1f18..f65c64c296cd 100644
> > --- a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
> > +++ b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
> > @@ -41,6 +41,7 @@ STATIC EFI_TIMER_NOTIFY mTimerNotifyFunction;
> > // The current period of the timer interrupt
> > //
> > STATIC UINT64 mTimerPeriod = 0;
> > +STATIC UINT64 mLastPeriodStart = 0;
> >
> > /**
> > Timer Interrupt Handler.
> > @@ -55,26 +56,32 @@ TimerInterruptHandler (
> > IN EFI_SYSTEM_CONTEXT SystemContext
> > )
> > {
> > - EFI_TPL OriginalTPL;
> > - UINT64 RiscvTimer;
> > + EFI_TPL OriginalTPL;
> > + UINT64 PeriodStart = RiscVReadTimer ();
> >
> > OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> > if (mTimerNotifyFunction != NULL) {
> > - mTimerNotifyFunction (mTimerPeriod);
> > + //
> > + // For any number of reasons, the ticks could be coming
> > + // in slower than mTimerPeriod. For example, some code
> > + // is doing a *lot* of stuff inside an EFI_TPL_HIGH
> > + // critical section, and this should not cause the EFI
> > + // time to increment slower. So when we take an interrupt,
> > + // account for the actual time passed.
> > + //
> > + mTimerNotifyFunction (PeriodStart - mLastPeriodStart);
>
> Shouldn't we consider debug case like ARM does?
>
Hi Andrei,
Please ignore above comment. It is actually consistent with other
architectures.
Thanks,
Sunil
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100438): https://edk2.groups.io/g/devel/message/100438
Mute This Topic: https://groups.io/mt/97044196/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
