From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by mx.groups.io with SMTP id smtpd.web10.35533.1676956049512796596 for ; Mon, 20 Feb 2023 21:07:29 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@ventanamicro.com header.s=google header.b=MnhMh3di; spf=pass (domain: ventanamicro.com, ip: 209.85.214.177, mailfrom: sunilvl@ventanamicro.com) Received: by mail-pl1-f177.google.com with SMTP id q11so4072778plx.5 for ; Mon, 20 Feb 2023 21:07:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ncd7783sysPgCjLjD2025kuXcD0NVogdRVIn0PdCGOA=; b=MnhMh3ditsBBn1pqYo93C3Kccp1FmwqAQ14tWnfWPk42SGNi6hCDNSn48GZpruCzeW SVv3ZtSpL6ury7D6HETtB+cfDBWgV7pc1am/RzGBrQJvchP2/cJQVWYK12QDFVjlLdgg wSlgB8/9Gts4NX+XwD8UQguOyamrSv5O7Mo0Ldq8H5Lpu9j7TJdM50FzBQOLnSpeiyPf c9OvPLgiqJpho9zwZfFW8QQx2/IldoUREaMlZUdbLtrU2FP99o4BEkQjghOhnr55TV+U vV6+vumiI8hnPfuPI9TB9/fsS2EDWtROlS8OzLZrB1PMWfLlVCGp5hvT/++ANeLiKNeG Ujbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ncd7783sysPgCjLjD2025kuXcD0NVogdRVIn0PdCGOA=; b=xGK88/ZwImOZ4gTviU6CY+tm6ymGH9EMsHr6zm2xZfKWEGQ4ovgPOhAGf0lzI/TBh4 my/ArH4UYZFK+NPG3SM9i9vnS0hp2+/Ba5n2j/n3TBff/aRSoyQqajt+E+3GYeIHGt6z JnT3g1LQ16bFl4E3n3aDFb8Z79sehAKwSi5rxWAaQ2y65gb0g2ZygD4ULnJPcLpAw9sM eSH+8XuTGnUpGe+kgJPwmChox3wKEk5IaQgHbmLymiJgv2K+ukq4ofeA4OWnWkE9Z1YD PvCSmyfMbODfgAVXygnEBJ3smRocq1NaWchkOJLBHs50flIx7E7YFCEdE6x4TOH856j9 /nFA== X-Gm-Message-State: AO0yUKWBJnadBJ/eHhS6vDxZxiP2rmi5+aIk9VjD36TZi4l20x9A6NgO +6l7EDGHp+Uwv+o2f5kB4K1gEQ== X-Google-Smtp-Source: AK7set+DccfhH2/y6bHcZCRRsGbCVaKyg5IvDUq2IND+GAUd9C9xjxEKUUE872RIMvDnA7AFM4+3sQ== X-Received: by 2002:a05:6a20:1604:b0:bc:e2f6:8788 with SMTP id l4-20020a056a20160400b000bce2f68788mr3578223pzj.24.1676956048789; Mon, 20 Feb 2023 21:07:28 -0800 (PST) Return-Path: Received: from sunil-laptop ([49.206.14.226]) by smtp.gmail.com with ESMTPSA id k65-20020a633d44000000b004fb8732a2f9sm963633pga.88.2023.02.20.21.07.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Feb 2023 21:07:28 -0800 (PST) Date: Tue, 21 Feb 2023 10:37:23 +0530 From: "Sunil V L" To: Andrei Warkentin Cc: devel@edk2.groups.io, Daniel Schaefer Subject: Re: [edk2 1/1] RiscV64 Timer: fix tick duration accounting Message-ID: References: <20230218043149.1058-1-andrei.warkentin@intel.com> MIME-Version: 1.0 In-Reply-To: <20230218043149.1058-1-andrei.warkentin@intel.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > Cc: Daniel Schaefer > Signed-off-by: Andrei Warkentin > --- > 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? Also, looks like there are some uncrustify errors. Please run manually and fix them. https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#manual-usage---generate-file-list-via-stdin Thanks, Sunil