Re: [PATCH 73/79] RiscVPkg: Address Core CI ECC errors.

["Sunil V L" <sunilvl@ventanamicro.com>]

On Sat, Jan 08, 2022 at 12:14:14PM +0800, Abner Chang wrote:
> Signed-off-by: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> ---
>  RiscVPkg/RiscVPkg.dec                         |  2 ++
>  RiscVPkg/RiscVPkg.dsc                         |  4 ++--
>  .../RiscVFirmwareContextSbiLib.inf            |  2 +-
>  .../Include/Library/MachineModeTimerLib.h     | 15 +++++++++++++
>  .../Include/Library/RiscVPlatformTimerLib.h   | 21 +++++++++++++++++++
>  .../CpuExceptionHandlerLib.h                  |  2 +-
>  RiscVPkg/RiscVPkg.uni                         | 18 +++++++++++++++-
>  7 files changed, 59 insertions(+), 5 deletions(-)
>  create mode 100644 RiscVPkg/Include/Library/MachineModeTimerLib.h
>  create mode 100644 RiscVPkg/Include/Library/RiscVPlatformTimerLib.h
> 
> diff --git a/RiscVPkg/RiscVPkg.dec b/RiscVPkg/RiscVPkg.dec
> index f23d3c0135..1e2d2fac0d 100644
> --- a/RiscVPkg/RiscVPkg.dec
> +++ b/RiscVPkg/RiscVPkg.dec
> @@ -26,6 +26,8 @@
>    RiscVCpuLib|Include/Library/RiscVCpuLib.h
>    RiscVEdk2SbiLib|Include/Library/RiscVEdk2SbiLib.h
>    RiscVFirmwareContextLib|Include/Library/RiscVFirmwareContextLib.h
> +  RiscVPlatformTimerLib|Include/Library/RiscVPlatformTimerLib.h
> +  MachineModeTimerLib|Include/Library/MachineModeTimerLib.h
>  
>  [Guids]
>    gUefiRiscVPkgTokenSpaceGuid  = { 0x4261e9c8, 0x52c0, 0x4b34, { 0x85, 0x3d, 0x48, 0x46, 0xea, 0xd3, 0xb7, 0x2c}}
> diff --git a/RiscVPkg/RiscVPkg.dsc b/RiscVPkg/RiscVPkg.dsc
> index 7e8c8f599c..c1cd11a161 100644
> --- a/RiscVPkg/RiscVPkg.dsc
> +++ b/RiscVPkg/RiscVPkg.dsc
> @@ -1,11 +1,11 @@
> -#/** @file
> +## @file
>  # RISC-V package.
>  #
>  # Copyright (c) 2021, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> -#**/
> +#
>  
>  ################################################################################
>  #
> diff --git a/RiscVPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContextSbiLib.inf b/RiscVPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContextSbiLib.inf
> index db30d7e551..e3dbc05007 100644
> --- a/RiscVPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContextSbiLib.inf
> +++ b/RiscVPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContextSbiLib.inf
> @@ -12,7 +12,7 @@
>  [Defines]
>    INF_VERSION                    = 0x0001001b
>    BASE_NAME                      = RiscVFirmwareContextSbiLib
> -  FILE_GUID                      = 3709E048-6794-427A-B728-BFE3FFD6D461
> +  FILE_GUID                      = 308117C0-400A-79C5-6ED4-AB9763A202E5

Any reason to have new GUID?

>    MODULE_TYPE                    = PEIM
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = RiscVFirmwareContextLib|PEIM PEI_CORE
> diff --git a/RiscVPkg/Include/Library/MachineModeTimerLib.h b/RiscVPkg/Include/Library/MachineModeTimerLib.h
> new file mode 100644
> index 0000000000..a27391cca3
> --- /dev/null
> +++ b/RiscVPkg/Include/Library/MachineModeTimerLib.h
> @@ -0,0 +1,15 @@
> +/** @file
> +  RISC-V Machine Mode Timer Library Definition
> +
> +  Copyright (c) 2022, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef MACHINE_MODE_TIMER_LIB_H_
> +#define MACHINE_MODE_TIMER_LIB_H_
> +
> +UINT64
> +RiscVReadMachineTimerInterface (VOID);
> +
> +#endif
> diff --git a/RiscVPkg/Include/Library/RiscVPlatformTimerLib.h b/RiscVPkg/Include/Library/RiscVPlatformTimerLib.h
> new file mode 100644
> index 0000000000..dcd8734eb5
> --- /dev/null
> +++ b/RiscVPkg/Include/Library/RiscVPlatformTimerLib.h
> @@ -0,0 +1,21 @@
> +/** @file
> +  RISC-V Platform Timer library definitions.
> +
> +  Copyright (c) 2022, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef RISCV_PLATFORM_TIMER_LIB_H_
> +#define RISCV_PLATFORM_TIMER_LIB_H_
> +
> +UINT64
> +RiscVReadMachineTimer (VOID);
> +
> +VOID
> +RiscVSetMachineTimerCmp (UINT64);
> +
> +UINT64
> +RiscVReadMachineTimerCmp(VOID);
> +
> +#endif
> diff --git a/RiscVPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h b/RiscVPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h
> index 3e480e9b09..b316510020 100644
> --- a/RiscVPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h
> +++ b/RiscVPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h
> @@ -1,4 +1,4 @@
> -/**@file
> +/** @file
>  
>    RISC-V Exception Handler library definition file.
>  
> diff --git a/RiscVPkg/RiscVPkg.uni b/RiscVPkg/RiscVPkg.uni
> index 7e470eb531..d459272444 100644
> --- a/RiscVPkg/RiscVPkg.uni
> +++ b/RiscVPkg/RiscVPkg.uni
> @@ -8,6 +8,22 @@
>  // **/
>  
>  #string STR_PACKAGE_ABSTRACT            #language en-US "Provides UEFI compatible RISC-V processor modules and libraries"
> -
>  #string STR_PACKAGE_DESCRIPTION         #language en-US "This Package provides UEFI compatible RISC-V processor modules and libraries."
>  
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSpecificDataGuidHobGuid_PROMPT    #language en-US "Processor Specific Data HOB GUID"
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSpecificDataGuidHobGuid_HELP      #language en-US "This is the GUID definition of HOB that passes the "
> +                                                                                                      "processor specific data to DXE phase."
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosGuidHobGuid_PROMPT          #language en-US "RISC-V SMBIOS Data HOB GUID"
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosGuidHobGuid_HELP            #language en-US "This is the GUID definition of HOB that passes RISC-V SMBIOS"
> +                                                                                                      "Data to DXE phase."

I think space is required between multi line help string. Same comment
for other places.

> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosType4GuidHobGuid_PROMPT     #language en-US "RISC-V SMBIOS Type 4 Data HOB GUID"
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosType4GuidHobGuid_HELP       #language en-US "This is the GUID definition of HOB that passes RISC-V SMBIOS"
> +                                                                                      "Type 4 information to DXE phase for building up SMBIOS record."
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosType7GuidHobGuid_PROMPT     #language en-US "RISC-V SMBIOS Type 7 Data HOB GUID"
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosType7GuidHobGuid_HELP       #language en-US "This is the GUID definition of HOB that passes RISC-V SMBIOS"
> +                                                                                      "Type 7 information to DXE phase for building up SMBIOS record."
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdRiscVMachineTimerTickInNanoSecond_PROMPT   #language en-US "RISC-V Machine Mode Timer Duration"
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdRiscVMachineTimerTickInNanoSecond_HELP     #language en-US "RISC-V Machine Mode Timer Duration in nanosecond."
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdRiscVMachineTimerFrequencyInHerz_PROMPT    #language en-US "RISC-V Machine Mode Timer frequency."
> +#string STR_gUefiRiscVPkgTokenSpaceGuid_PcdRiscVMachineTimerFrequencyInHerz_HELP      #language en-US "RISC-V Machine Mode Timer frequency in Herz"

Should this Hertz instead of Herz?

Regards
Sunil
> +
> -- 
> 2.31.1
>