From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) by mx.groups.io with SMTP id smtpd.web10.2328.1686071420274430303 for ; Tue, 06 Jun 2023 10:10:20 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@ventanamicro.com header.s=google header.b=atxnfmJ4; spf=pass (domain: ventanamicro.com, ip: 209.85.210.181, mailfrom: sunilvl@ventanamicro.com) Received: by mail-pf1-f181.google.com with SMTP id d2e1a72fcca58-650c89c7e4fso6514627b3a.0 for ; Tue, 06 Jun 2023 10:10:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1686071420; x=1688663420; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=zXMNywBLK75gzmpz+3pdqKYN24ZGpNiQcqyZJEM03vw=; b=atxnfmJ4o1mtQTHPOyyVVRI8e/yXJjyWutYfeBFYe5Pp7Q66izH9cKWJSj55Q3YM3b v6cBjCj+O93saap8X2pj+13H6u6xq3I80CbsRX/TKM4rflD+hX+1SJwvDwpytjjhwJX5 G5m5zl6zk53Yp6e6wSAQhBNGPPkrdvpSK8sYQdcdNAVIuN0bl+jTXH5iYBYcwYmiOkYB TnWa62PZaDdddpY/ztzF+N8ZqfjMaJjMIZZ6DrdZfUm/XoK4w6hQnj/MwwkCUMM4qi9l qMiPwpxkDtfiaYYeBNz9RChCd0E1G51lcDoh0veP+KKx10Pljtz/VXz5A1j22tPa3Etn d3qQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686071420; x=1688663420; h=in-reply-to:content-transfer-encoding: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=zXMNywBLK75gzmpz+3pdqKYN24ZGpNiQcqyZJEM03vw=; b=ZUtdMs5sFxNkIhG9O2FSzTp63x6aqWm9R7LLY6kj93je0ja4dS7OUrLvZk5gjL8vXo mcuppsJmw7I2g0ZNMs6atoaDQYfgmUENOc2UADrKxg3uDxAc5WQ5gCCQN9t2pOW8M1C6 gqzWRTTMJcIDmsc9HFMPe9/HOosePi6YKP6JzXh16byS9paez/5kZAvAvgiMHyMUQFDB Y0yPxgGBtuSJGEs47Ppe6Stloqyg0m0Z/KRoyxSJIm/ZLTJV6YkjchiAMLHL61jeB5hU MDSfB1d5F24lWMKjlCXndQKgS8Mb5GUD5ardJGNpVJ3CqN9iog3x9huP0DYlzAd7mfOw BnnQ== X-Gm-Message-State: AC+VfDwe/IeCwmWw8VuqoXW63fA7EM+q8e5SMLP+0TldEORqg1b+nL/I Tt1LscN1Ej+h66S8RJvRP17llA== X-Google-Smtp-Source: ACHHUZ4QmkNZU4w3bSt+FHej3vzspOsOOTUwe3P1BBb8Azb3tbPBkXQUellofpscH9V8+WG4WuA4JA== X-Received: by 2002:a05:6a00:1883:b0:646:9232:df6 with SMTP id x3-20020a056a00188300b0064692320df6mr3797400pfh.33.1686071419663; Tue, 06 Jun 2023 10:10:19 -0700 (PDT) Return-Path: Received: from sunil-laptop ([106.51.186.3]) by smtp.gmail.com with ESMTPSA id 15-20020aa7924f000000b0066199088a2dsm30432pfp.193.2023.06.06.10.10.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Jun 2023 10:10:19 -0700 (PDT) Date: Tue, 6 Jun 2023 22:40:12 +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: Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Tue, Jun 06, 2023 at 10:02:08AM -0700, Tuan Phan wrote: > On Tue, Jun 6, 2023 at 3:27 AM Sunil V L wrote: > > > 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? > > > => That is the result of running crutinize tool with edk2 config. Should I > leave it as before? > I have not used crutinize tool. But this change looks unnecessary. I would use uncrustify tool locally for any formatting fixes since the same tool gets used in CI tests also. > > > > Otherwise LGTM. Thanks for the fix! > > > > Reviewed-by: Sunil V L > >