From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by mx.groups.io with SMTP id smtpd.web10.5250.1677059876264126399 for ; Wed, 22 Feb 2023 01:57:56 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@ventanamicro.com header.s=google header.b=nEGuU0cr; spf=pass (domain: ventanamicro.com, ip: 209.85.216.50, mailfrom: sunilvl@ventanamicro.com) Received: by mail-pj1-f50.google.com with SMTP id i1-20020a17090ad34100b00234463de251so7978943pjx.3 for ; Wed, 22 Feb 2023 01:57:56 -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=rxDVG1eett/5PJ6+pyCfmMnrKpv9A7jqf0eweCxkidE=; b=nEGuU0crl8wy8f3OFkFQ6TZ8LJ4FZjLYhcEhNikMYszr/gcVicLnKK+juvkMdDTSQJ Viv2RxrvYKT7lzrQ0j57Gaj/W3KLKPSaON6qQIr8j3btBISfdLieCNIW1aGwCBQQrZLV QLKv3h5vW80+MTDbEbNfjvICtJ6egejV3B1b90WAM6J3YKxA9HDo/isaRYT+F86x7pWj nzveQUnMZ4EAcX7neDW7vbv23ziqhqJHEGbplWgCqXShNMVqPAAAABHU/KppLVg7aPzL TRWOt3IUz/rx8pBQv4jN5ABDIW/w3QFobm+FHRAJf0Pgo6p6JruPxll8gwJG6jUzv3us ox2g== 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=rxDVG1eett/5PJ6+pyCfmMnrKpv9A7jqf0eweCxkidE=; b=kz3QYmcTJeJ7AgnKPfDgcS7CwPiagDd9f04ZOQV3G+K/yI3C5eRm1kNpE71dBK9y64 UTqGZ6JyyRwC9OvK4OTsJeyMkDNvAIsAyP125gd+qihDGm+Ewl/nAIVV9f56T2J2PVlW y+JN9JoSowg1YYL7VZP6OdBOYas7DrtNfK21j2p8b7MH6CU9UngGEwZmFXzg5JVnZ9vN RfNF+AHv3NXevwNCTSGLn3iPdCA6IUPgueUCDnHarEP/jMrpnkeWo/WXyb1M5V9vJApG s3nhSqqCFFuvrW+8j3HsWBEo6d9PNA8UPWO3K9gf0nIGAAYOayo4pgh0leXpULoCsqQq hUMQ== X-Gm-Message-State: AO0yUKWJph0QhhEKS5b/zlHIivnzl11lwT5Ee9HpfvJGGoNzzrOleAXi mHSsDcWwTI4Ei9Z9pssHD7D9iySYb30gx6sC X-Google-Smtp-Source: AK7set/iU4A99wdbD8xjNUlcTrJ+DVgqUzMXXNB817LD3NanY5UlnPzGO4p3IB26KBhif/SXgIrplw== X-Received: by 2002:a17:902:e5c9:b0:19c:355c:6eb5 with SMTP id u9-20020a170902e5c900b0019c355c6eb5mr10681577plf.30.1677059875177; Wed, 22 Feb 2023 01:57:55 -0800 (PST) Return-Path: Received: from sunil-laptop ([49.206.14.226]) by smtp.gmail.com with ESMTPSA id q16-20020a170902dad000b0019ac23cb6edsm7631837plx.181.2023.02.22.01.57.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Feb 2023 01:57:54 -0800 (PST) Date: Wed, 22 Feb 2023 15:27:50 +0530 From: "Sunil V L" To: devel@edk2.groups.io Cc: Andrei Warkentin , Daniel Schaefer Subject: Re: [edk2-devel] [edk2 1/1] RiscV64 Timer: fix tick duration accounting Message-ID: References: <20230218043149.1058-1-andrei.warkentin@intel.com> <1745BEA47127C0BE.676@groups.io> MIME-Version: 1.0 In-Reply-To: <1745BEA47127C0BE.676@groups.io> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > > 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? > Hi Andrei, Please ignore above comment. It is actually consistent with other architectures. Thanks, Sunil