From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) by mx.groups.io with SMTP id smtpd.web10.5285.1686047255797952880 for ; Tue, 06 Jun 2023 03:27:35 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@ventanamicro.com header.s=google header.b=W/tGlzlu; spf=pass (domain: ventanamicro.com, ip: 209.85.214.175, mailfrom: sunilvl@ventanamicro.com) Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-1b04782fe07so31683705ad.3 for ; Tue, 06 Jun 2023 03:27:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1686047255; x=1688639255; 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=wp+k/Hf0RUxMyD8OvZBr94XGbkwvrtFf4fifxfVECBI=; b=W/tGlzluFnTmU8YUta81JtqpH7fBlsyqp9bD2DCV3D9VNPdJaFVUURYx7/bPU4DkEe 3LCQ8v/lruEpa2bGlzXFuOUhkCDAseR78os0pMoSFfCtAs5+403nAzHAH4+R7VpuIow9 RFvMFATgKFeufZ8p7pm5RqusDFGXLxlfdJxkBgyQNyz5ES0isB2MEBlGtSsuL5VFY7m5 RARkBv6sfv0oaU21mkjSwYlef5vk0+LjU1QqFX4YHfmeS7uAAvXSRb7CJcZ09ygiiXer Gox5xV0LOCUA03EN34ebAQ8AQICzDwUQXgSD4+8GtjRmc+Osi/i5JYrlqxHdED8X5Dcb QoCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686047255; x=1688639255; 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=wp+k/Hf0RUxMyD8OvZBr94XGbkwvrtFf4fifxfVECBI=; b=GAVcd4ePFNu5N3pczyQkZxp5BmxZ9LQuwqfwiS6KTeY+gf0xtQPxkcufyiMPyyjVfH 9sCNor3lvTlio/TTYl9sZG09lz765Ze0ToB11IMo56xAfT83sDEPZ5M/xq/3UYLU+Yu4 +FeV+aPsGdw1/49+QwrgQalm0XhpVGJ+KPfwb+7XYbe27dbuxbwMDpARhWLAE5BuKhD9 B1fpQqz+B+6FPWcDyVaXrzPa95wYtK1o0YjZy8yCaXMZPISlQYQRRuVn3umRzkfgamos /Jhfl/ut7iO30of3oFN1HoFGenYD/Pa52krSZYNy56lG1xd2SYk5lL0NS/vCAPAGxeBQ i8ZA== X-Gm-Message-State: AC+VfDyuYDMykoHpWPc+sV668F4HG2L8cG9u0yAhwI8OA7snJqdqzDHR cxQ9inGvSwJqmikRkIa/qkJctg== X-Google-Smtp-Source: ACHHUZ4hnqW3e61nLjOAgTrnSqpKKpifC3Ybh+UJP94EclUExrY8NVj5i+FNxzlS5FGtzLIBttiG5w== X-Received: by 2002:a17:902:d484:b0:1ad:dd21:2691 with SMTP id c4-20020a170902d48400b001addd212691mr1162317plg.10.1686047255280; Tue, 06 Jun 2023 03:27:35 -0700 (PDT) Return-Path: Received: from sunil-laptop ([106.51.186.3]) by smtp.gmail.com with ESMTPSA id z11-20020a1709028f8b00b001b1f991a4e2sm4693663plo.20.2023.06.06.03.27.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Jun 2023 03:27:34 -0700 (PDT) Date: Tue, 6 Jun 2023 15:57:29 +0530 From: "Sunil V L" To: Tuan Phan Cc: devel@edk2.groups.io, eric.dong@intel.com, ray.ni@intel.com, rahul1.kumar@intel.com, git@danielschaefer.me, abner.chang@amd.com, kraxel@redhat.com, andrei.warkentin@intel.com Subject: Re: [PATCH 2/2] UefiCpuPkg: RISC-V: TimerLib: Fix delay function to use 64-bit Message-ID: References: <20230526232518.7141-1-tphan@ventanamicro.com> <20230526232518.7141-2-tphan@ventanamicro.com> MIME-Version: 1.0 In-Reply-To: <20230526232518.7141-2-tphan@ventanamicro.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, May 26, 2023 at 04:25:18PM -0700, Tuan Phan wrote: > The timer compare register is 64-bit so simplifying the delay > function. > > Signed-off-by: Tuan Phan > --- > MdePkg/Include/Register/RiscV64/RiscVImpl.h | 1 - > .../BaseRiscV64CpuTimerLib/CpuTimerLib.c | 62 +++++++++---------- > 2 files changed, 28 insertions(+), 35 deletions(-) > > diff --git a/MdePkg/Include/Register/RiscV64/RiscVImpl.h b/MdePkg/Include/Register/RiscV64/RiscVImpl.h > index ee5c2ba60377..6997de6cc001 100644 > --- a/MdePkg/Include/Register/RiscV64/RiscVImpl.h > +++ b/MdePkg/Include/Register/RiscV64/RiscVImpl.h > @@ -20,6 +20,5 @@ > Name: > > #define ASM_FUNC(Name) _ASM_FUNC(ASM_PFX(Name), .text. ## Name) > -#define RISCV_TIMER_COMPARE_BITS 32 > > #endif > diff --git a/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c b/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c > index 9c8efc0f3530..57800177023c 100644 > --- a/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c > +++ b/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c > @@ -22,26 +22,19 @@ > @param Delay A period of time to delay in ticks. > > **/ > +STATIC > VOID > InternalRiscVTimerDelay ( > - IN UINT32 Delay > + IN UINT64 Delay > ) > { > - UINT32 Ticks; > - UINT32 Times; > - > - Times = Delay >> (RISCV_TIMER_COMPARE_BITS - 2); > - Delay &= ((1 << (RISCV_TIMER_COMPARE_BITS - 2)) - 1); > - do { > - // > - // The target timer count is calculated here > - // > - Ticks = RiscVReadTimer () + Delay; > - Delay = 1 << (RISCV_TIMER_COMPARE_BITS - 2); > - while (((Ticks - RiscVReadTimer ()) & (1 << (RISCV_TIMER_COMPARE_BITS - 1))) == 0) { > - CpuPause (); > - } > - } while (Times-- > 0); > + UINT64 Ticks; > + > + Ticks = RiscVReadTimer () + Delay; > + > + while (RiscVReadTimer () <= Ticks) { > + CpuPause (); > + } > } > > /** > @@ -61,14 +54,14 @@ MicroSecondDelay ( > ) > { > InternalRiscVTimerDelay ( > - (UINT32)DivU64x32 ( > - MultU64x32 ( > - MicroSeconds, > - PcdGet64 (PcdCpuCoreCrystalClockFrequency) > - ), > - 1000000u > - ) > - ); > + DivU64x32 ( > + MultU64x32 ( > + MicroSeconds, > + PcdGet64 (PcdCpuCoreCrystalClockFrequency) > + ), > + 1000000u > + ) > + ); > return MicroSeconds; > } > > @@ -89,14 +82,14 @@ NanoSecondDelay ( > ) > { > InternalRiscVTimerDelay ( > - (UINT32)DivU64x32 ( > - MultU64x32 ( > - NanoSeconds, > - PcdGet64 (PcdCpuCoreCrystalClockFrequency) > - ), > - 1000000000u > - ) > - ); > + DivU64x32 ( > + MultU64x32 ( > + NanoSeconds, > + PcdGet64 (PcdCpuCoreCrystalClockFrequency) > + ), > + 1000000000u > + ) > + ); > return NanoSeconds; > } > > @@ -147,8 +140,9 @@ GetPerformanceCounter ( > UINT64 > EFIAPI > GetPerformanceCounterProperties ( > - OUT UINT64 *StartValue, OPTIONAL > - OUT UINT64 *EndValue OPTIONAL > + OUT UINT64 *StartValue, > + OPTIONAL > + OUT UINT64 *EndValue OPTIONAL Hi Tuan, What is this change? The formatting doesn't look correct. Have you run CI tests? Otherwise LGTM. Thanks for the fix! Reviewed-by: Sunil V L