> -----Original Message-----
> From: Sunil V L <suni...@ventanamicro.com>
> Sent: Friday, January 21, 2022 10:48 PM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.ch...@hpe.com>
> Cc: Schaefer, Daniel (ROM Janitor) <daniel.schae...@hpe.com>
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH 09/14] RISC-
> V/ProcessorPkg: Address Core CI ECC errors.
>
> On Fri, Jan 21, 2022 at 04:48:43PM +0800, Abner Chang wrote:
> > Signed-off-by: Abner Chang <abner.ch...@hpe.com>
> > Cc: Daniel Schaefer <daniel.schae...@hpe.com>
> > Cc: Sunil V L <suni...@ventanamicro.com>
> > ---
> > .../RISC-V/ProcessorPkg/RiscVProcessorPkg.dec | 2 ++
> > .../RISC-V/ProcessorPkg/RiscVProcessorPkg.dsc | 4 ++--
> > .../RiscVFirmwareContextSbiLib.inf | 2 +-
> > .../Include/Library/MachineModeTimerLib.h | 15 +++++++++++++
> > .../Include/Library/RiscVPlatformTimerLib.h | 21 +++++++++++++++++++
> > .../ProcessorPkg/Include/OpensbiTypes.h | 6 +++---
> > .../Include/ProcessorSpecificHobData.h | 8 +++----
> > .../CpuExceptionHandlerLib.h | 2 +-
> > .../Universal/SmbiosDxe/RiscVSmbiosDxe.c | 12 +++++------
> > .../RISC-V/ProcessorPkg/RiscVProcessorPkg.uni | 18 +++++++++++++++-
> > 10 files changed, 72 insertions(+), 18 deletions(-)
> > create mode 100644 Silicon/RISC-
> V/ProcessorPkg/Include/Library/MachineModeTimerLib.h
> > create mode 100644 Silicon/RISC-
> V/ProcessorPkg/Include/Library/RiscVPlatformTimerLib.h
> >
> > diff --git a/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dec
> b/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dec
> > index 9c8b57cce3..045fc55212 100644
> > --- a/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dec
> > +++ b/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.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/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dsc
> b/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dsc
> > index 563b9e7088..0591cd6a6c 100644
> > --- a/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dsc
> > +++ b/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dsc
> > @@ -1,11 +1,11 @@
> > -#/** @file
> > +## @file
> > # RISC-V processor package.
> > #
> > # Copyright (c) 2021, Hewlett Packard Enterprise Development LP. All rights
> reserved.<BR>
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > -#**/
> > +#
> >
> >
> ##########################################################
> ######################
> > #
> > diff --git a/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContext
> SbiLib.inf b/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContext
> SbiLib.inf
> > index 168b705453..0edf781149 100644
> > --- a/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContext
> SbiLib.inf
> > +++ b/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVFirmwareContextSbiLib/RiscVFirmwareContext
> SbiLib.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 change the GUID?
It is duplicated with another INF file.
>
> > MODULE_TYPE = PEIM
> > VERSION_STRING = 1.0
> > LIBRARY_CLASS = RiscVFirmwareContextLib|PEIM PEI_CORE
> > diff --git a/Silicon/RISC-
> V/ProcessorPkg/Include/Library/MachineModeTimerLib.h b/Silicon/RISC-
> V/ProcessorPkg/Include/Library/MachineModeTimerLib.h
> > new file mode 100644
> > index 0000000000..a27391cca3
> > --- /dev/null
> > +++ b/Silicon/RISC-
> V/ProcessorPkg/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/Silicon/RISC-
> V/ProcessorPkg/Include/Library/RiscVPlatformTimerLib.h b/Silicon/RISC-
> V/ProcessorPkg/Include/Library/RiscVPlatformTimerLib.h
> > new file mode 100644
> > index 0000000000..dcd8734eb5
> > --- /dev/null
> > +++ b/Silicon/RISC-
> V/ProcessorPkg/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/Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h
> b/Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h
> > index bbf74e2a82..8a6ea97708 100644
> > --- a/Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h
> > +++ b/Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h
> > @@ -44,8 +44,8 @@ typedef UINT64 virtual_size_t;
> > typedef UINT64 physical_addr_t;
> > typedef UINT64 physical_size_t;
> >
> > -#define true TRUE
> > -#define false FALSE
> > +#define true TRUE
> > +#define false FALSE
> >
> > #define __packed __attribute__((packed))
> > #define __noreturn __attribute__((noreturn))
> > @@ -70,7 +70,7 @@ typedef UINT64 physical_size_t;
> > const typeof(((type *)0)->member) * __mptr = (ptr); \
> > (type *)((char *)__mptr - offsetof(type, member)); })
> >
> > -#define array_size(x) (sizeof(x) / sizeof((x)[0]))
> > +#define array_size(x) (sizeof(x) / sizeof((x)[0]))
> >
> > #define CLAMP(a, lo, hi) MIN(MAX(a, lo), hi)
> > #define ROUNDUP(a, b) ((((a)-1) / (b) + 1) * (b))
> > diff --git a/Silicon/RISC-
> V/ProcessorPkg/Include/ProcessorSpecificHobData.h b/Silicon/RISC-
> V/ProcessorPkg/Include/ProcessorSpecificHobData.h
> > index 2f5847e53e..97285289f7 100644
> > --- a/Silicon/RISC-V/ProcessorPkg/Include/ProcessorSpecificHobData.h
> > +++ b/Silicon/RISC-V/ProcessorPkg/Include/ProcessorSpecificHobData.h
> > @@ -1,7 +1,7 @@
> > /** @file
> > Definition of Processor Specific Data HOB.
> >
> > - Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights
> reserved.<BR>
> > + Copyright (c) 2019-2022, Hewlett Packard Enterprise Development LP. All
> rights reserved.<BR>
> >
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -24,7 +24,7 @@
> > /// RISC-V processor specific data HOB
> > ///
> > typedef struct {
> > - EFI_GUID ParentPrcessorGuid;
> > + EFI_GUID ParentProcessorGuid;
> > UINTN ParentProcessorUid;
> > EFI_GUID CoreGuid;
> > VOID *Context; // The additional information of this core
> > which
> > @@ -37,7 +37,7 @@ typedef struct {
> > /// RISC-V SMBIOS type 4 (Processor) GUID data HOB
> > ///
> > typedef struct {
> > - EFI_GUID PrcessorGuid;
> > + EFI_GUID ProcessorGuid;
> > UINTN ProcessorUid;
> > SMBIOS_TABLE_TYPE4 SmbiosType4Processor;
> > UINT16 EndingZero;
> > @@ -75,7 +75,7 @@ typedef struct {
> > /// RISC-V SMBIOS type 7 (Cache) GUID data HOB
> > ///
> > typedef struct {
> > - EFI_GUID PrcessorGuid;
> > + EFI_GUID ProcessorGuid;
> > UINTN ProcessorUid;
> > SMBIOS_TABLE_TYPE7 SmbiosType7Cache;
> > UINT16 EndingZero;
> > diff --git a/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h
> b/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h
> > index 3e480e9b09..b316510020 100644
> > --- a/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h
> > +++ b/Silicon/RISC-
> V/ProcessorPkg/Library/RiscVExceptionLib/CpuExceptionHandlerLib.h
> > @@ -1,4 +1,4 @@
> > -/**@file
> > +/** @file
> >
> > RISC-V Exception Handler library definition file.
> >
> > diff --git a/Silicon/RISC-
> V/ProcessorPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.c b/Silicon/RISC-
> V/ProcessorPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.c
> > index b30f9d7f6a..14f62c4036 100644
> > --- a/Silicon/RISC-V/ProcessorPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.c
> > +++ b/Silicon/RISC-
> V/ProcessorPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > RISC-V generic SMBIOS DXE driver to build up SMBIOS type 4, type 7 and
> type 44 records.
> >
> > - Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights
> reserved.<BR>
> > + Copyright (c) 2019-2022, Hewlett Packard Enterprise Development LP. All
> rights reserved.<BR>
> >
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -33,7 +33,7 @@ BuildSmbiosType7 (
> > EFI_STATUS Status;
> > SMBIOS_HANDLE Handle;
> >
> > - if (!CompareGuid (&Type4HobData->PrcessorGuid, &Type7DataHob-
> >PrcessorGuid) ||
> > + if (!CompareGuid (&Type4HobData->ProcessorGuid, &Type7DataHob-
> >ProcessorGuid) ||
> > Type4HobData->ProcessorUid != Type7DataHob->ProcessorUid) {
> > return EFI_INVALID_PARAMETER;
> > }
> > @@ -48,7 +48,7 @@ BuildSmbiosType7 (
> > return Status;
> > }
> > DEBUG ((DEBUG_INFO, "SMBIOS Type 7 was added. SMBIOS Handle:
> 0x%x\n", Handle));
> > - DEBUG ((DEBUG_VERBOSE, " Cache belone to processor GUID: %g\n",
> &Type7DataHob->PrcessorGuid));
> > + DEBUG ((DEBUG_VERBOSE, " Cache belone to processor GUID: %g\n",
> &Type7DataHob->ProcessorGuid));
> > DEBUG ((DEBUG_VERBOSE, " Cache belone processor UID: %d\n",
> Type7DataHob->ProcessorUid));
> > DEBUG ((DEBUG_VERBOSE, "
> ==============================\n"));
> > DEBUG ((DEBUG_VERBOSE, " Socket Designation: %d\n",
> Type7DataHob->SmbiosType7Cache.SocketDesignation));
> > @@ -90,7 +90,7 @@ BuildSmbiosType4 (
> > EFI_STATUS Status;
> >
> > DEBUG ((DEBUG_INFO, "Building Type 4.\n"));
> > - DEBUG ((DEBUG_INFO, " Processor GUID: %g\n", &Type4HobData-
> >PrcessorGuid));
> > + DEBUG ((DEBUG_INFO, " Processor GUID: %g\n", &Type4HobData-
> >ProcessorGuid));
> > DEBUG ((DEBUG_INFO, " Processor UUID: %d\n", Type4HobData-
> >ProcessorUid));
> >
> > Type4HobData->SmbiosType4Processor.L1CacheHandle =
> RISC_V_CACHE_INFO_NOT_PROVIDED;
> > @@ -193,7 +193,7 @@ BuildSmbiosType44 (
> > EFI_STATUS Status;
> >
> > DEBUG ((DEBUG_INFO, "Building Type 44 for...\n"));
> > - DEBUG ((DEBUG_VERBOSE, " Processor GUID: %g\n", &Type4HobData-
> >PrcessorGuid));
> > + DEBUG ((DEBUG_VERBOSE, " Processor GUID: %g\n",
> &Type4HobData->ProcessorGuid));
> > DEBUG ((DEBUG_VERBOSE, " Processor UUID: %d\n", Type4HobData-
> >ProcessorUid));
> >
> > GuidHob = (EFI_HOB_GUID_TYPE *)GetFirstGuidHob ((EFI_GUID
> *)PcdGetPtr(PcdProcessorSpecificDataGuidHobGuid));
> > @@ -206,7 +206,7 @@ BuildSmbiosType44 (
> > //
> > do {
> > ProcessorSpecificData = (RISC_V_PROCESSOR_SPECIFIC_HOB_DATA
> *)GET_GUID_HOB_DATA (GuidHob);
> > - if (!CompareGuid (&ProcessorSpecificData->ParentPrcessorGuid,
> &Type4HobData->PrcessorGuid) ||
> > + if (!CompareGuid (&ProcessorSpecificData->ParentProcessorGuid,
> &Type4HobData->ProcessorGuid) ||
> > ProcessorSpecificData->ParentProcessorUid != Type4HobData-
> >ProcessorUid) {
> > GuidHob = GetNextGuidHob((EFI_GUID
> *)PcdGetPtr(PcdProcessorSpecificDataGuidHobGuid),
> GET_NEXT_HOB(GuidHob));
> > if (GuidHob == NULL) {
> > diff --git a/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.uni
> b/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.uni
> > index 83da92fe40..e743f033fb 100644
> > --- a/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.uni
> > +++ b/Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.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_PR
> OMPT #language en-US "RISC-V SMBIOS Data HOB GUID"
> > +#string
> STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosGuidHobGuid_HEL
> P #language en-US "This is the GUID definition of HOB that passes
> RISC-
> V SMBIOS"
> > +
> > "Data to DXE phase."
>
> Don't we need an extra space for these multiline strings? i.e. between
> "SMBIOS" and "Data".
Yes, will fix it.
>
> > +#string
> STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosType4GuidHobGui
> d_PROMPT #language en-US "RISC-V SMBIOS Type 4 Data HOB GUID"
> > +#string
> STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosType4GuidHobGui
> d_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_PcdProcessorSmbiosType7GuidHobGui
> d_PROMPT #language en-US "RISC-V SMBIOS Type 7 Data HOB GUID"
> > +#string
> STR_gUefiRiscVPkgTokenSpaceGuid_PcdProcessorSmbiosType7GuidHobGui
> d_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_PcdRiscVMachineTimerTickInNanoSeco
> nd_PROMPT #language en-US "RISC-V Machine Mode Timer Duration"
> > +#string
> STR_gUefiRiscVPkgTokenSpaceGuid_PcdRiscVMachineTimerTickInNanoSeco
> nd_HELP #language en-US "RISC-V Machine Mode Timer Duration in
> nanosecond."
> > +#string
> STR_gUefiRiscVPkgTokenSpaceGuid_PcdRiscVMachineTimerFrequencyInHer
> z_PROMPT #language en-US "RISC-V Machine Mode Timer frequency."
> > +#string
> STR_gUefiRiscVPkgTokenSpaceGuid_PcdRiscVMachineTimerFrequencyInHer
> z_HELP #language en-US "RISC-V Machine Mode Timer frequency in Herz"
>
> Should this Hertz instead of Herz?
Sure.
Thanks
Abner
>
> Regards
> Sunil
> > +
> > --
> > 2.31.1
> >
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85944): https://edk2.groups.io/g/devel/message/85944
Mute This Topic: https://groups.io/mt/88579974/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
