[edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 05/14] RiscV/Library: Initial version of libraries introduced in RISC-V platform package

Abner Chang abner.chang at hpe.com
Tue Oct 15 15:26:21 UTC 2019



> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> Leif Lindholm
> Sent: Thursday, October 3, 2019 1:05 AM
> To: devel at edk2.groups.io; Chen, Gilbert <gilbert.chen at hpe.com>
> Cc: Palmer Dabbelt <palmer at sifive.com>
> Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 05/14]
> RiscV/Library: Initial version of libraries introduced in RISC-V platform
> package
> 
> On Thu, Sep 19, 2019 at 11:51:22AM +0800, Gilbert Chen wrote:
> > FirmwareContextProcessorSpecificLib
> > - Common library to consume
> EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC
> >  and build up processor specific data HOB.
> >
> > RealTimClockLibNull
> > - NULL instance of Real Time Clock library.
> >
> > Signed-off-by: Gilbert Chen <gilbert.chen at hpe.com>
> > ---
> >  .../FirmwareContextProcessorSpecificLib.c          |  82 +++++++++
> >  .../FirmwareContextProcessorSpecificLib.inf        |  33 ++++
> >  .../RealTimeClockLibNull/RealTimeClockLibNull.c    | 204
> +++++++++++++++++++++
> >  .../RealTimeClockLibNull/RealTimeClockLibNull.inf  |  30 +++
> 
> I think you can replace this NULL RealTimeClockLib implementation with
> EmbeddedPkg/Library/VirtualRealTimeClockLib/ (which did not exist at the
> time of the original port).
> 
> >  4 files changed, 349 insertions(+)
> >  create mode 100644
> >
> Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareCon
> > textProcessorSpecificLib.c  create mode 100644
> >
> Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareCon
> > textProcessorSpecificLib.inf  create mode 100644
> > Platform/RiscV/Library/RealTimeClockLibNull/RealTimeClockLibNull.c
> >  create mode 100644
> > Platform/RiscV/Library/RealTimeClockLibNull/RealTimeClockLibNull.inf
> >
> > diff --git
> > a/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareC
> > ontextProcessorSpecificLib.c
> >
> b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareC
> > ontextProcessorSpecificLib.c
> > new file mode 100644
> > index 00000000..4d4c51dc
> > --- /dev/null
> > +++ b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/Firmw
> > +++ areContextProcessorSpecificLib.c
> > @@ -0,0 +1,82 @@
> > +/**@file
> > +  Common library to build upfirmware context processor-specific
> > +information
> > +
> > +  Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All
> > + rights reserved.<BR>
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +//
> > +// The package level header files this module uses // #include
> > +<PiPei.h>
> > +
> > +//
> > +// The Library classes this module consumes // #include
> > +<Library/DebugLib.h> #include <Library/PeiServicesLib.h> #include
> > +<Library/BaseMemoryLib.h>
> 
> Please sort these includes alphabetically.
> 
> > +
> > +#include <RiscV.h>
> > +#include <SmbiosProcessorSpecificData.h> #include
> > +<ProcessorSpecificDataHob.h>
> 
> Please sort these includes alphabetically.
> 
> > +#include <sbi/sbi_hart.h>
> > +#include <sbi/sbi.h>
> > +#include <sbi/SbiFirmwareContext.h>
> > +
> > +/**
> > +  Build up common firmware context processor-specific information
> > +
> > +  @param  FirmwareContextHartSpecific  Pointer to
> EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC
> > +  @param  ParentProcessorGuid          Pointer to GUID of Processor which
> contains this core
> > +  @param  ParentProcessorUid           Unique ID of pysical processor which
> owns this core.
> > +  @param  CoreGuid                     Pointer to GUID of core
> > +  @param  HartId                       Hart ID of this core.
> > +  @param  IsBootHart                   This is boot hart or not
> > +  @param  ProcessorSpecDataHob         Pointer to
> RISC_V_PROCESSOR_SPECIFIC_DATA_HOB
> > +
> > +  @return EFI_STATUS
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +CommonFirmwareContextHartSpecificInfo (
> > +  EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC
> > +*FirmwareContextHartSpecific,
> > +  EFI_GUID  *ParentProcessorGuid,
> > +  UINTN     ParentProcessorUid,
> > +  EFI_GUID  *CoreGuid,
> > +  UINTN     HartId,
> > +  BOOLEAN   IsBootHart,
> > +  RISC_V_PROCESSOR_SPECIFIC_DATA_HOB *ProcessorSpecDataHob
> 
> Spec is not a clear enough abbreviation, please use Specific.
> Or call it something else. If you keep the name, it certainly wouldn't hurt to
> use a local variable with a shorter name..

This will be RISC_V_PROCESSOR_SPECIFIC_HOB_DATA in v3 edk2-staging patches.
> 
> > +  )
> > +{
> > +  //
> > +  // Build up RISC_V_PROCESSOR_SPECIFIC_DATA_HOB.
> > +  //
> > +  CopyGuid (&ProcessorSpecDataHob->ParentPrcessorGuid,
> > +ParentProcessorGuid);
> > +  ProcessorSpecDataHob->ParentProcessorUid = ParentProcessorUid;
> > +  CopyGuid (&ProcessorSpecDataHob->CoreGuid, CoreGuid);
> > +  ProcessorSpecDataHob->Context = NULL;
> > +  ProcessorSpecDataHob->ProcessorSpecificData.Revision         =
> SMBIOS_RISC_V_PROCESSOR_SPECIFIC_DATA_REVISION;
> 
> 80 Characters is up to                                                      here ->
> These lines are way too long, please wrap them.
> 
> > +  ProcessorSpecDataHob->ProcessorSpecificData.Length           = sizeof
> (SMBIOS_RISC_V_PROCESSOR_SPECIFIC_DATA);
> > +  ProcessorSpecDataHob->ProcessorSpecificData.HartId.Value64_L =
> > +(UINT64)HartId;
> > +  ProcessorSpecDataHob->ProcessorSpecificData.HartId.Value64_H = 0;
> > +  ProcessorSpecDataHob->ProcessorSpecificData.BootHartId       =
> (UINT8)IsBootHart;
> > +  ProcessorSpecDataHob->ProcessorSpecificData.InstSetSupported =
> FirmwareContextHartSpecific->IsaExtensionSupported;
> > +  ProcessorSpecDataHob-
> >ProcessorSpecificData.PrivilegeModeSupported   =
> SMBIOS_RISC_V_PSD_MACHINE_MODE_SUPPORTED;
> > +  if ((ProcessorSpecDataHob->ProcessorSpecificData.InstSetSupported &
> RISC_V_ISA_SUPERVISOR_MODE_IMPLEMENTED) != 0) {
> > +
> > +ProcessorSpecDataHob->ProcessorSpecificData.PrivilegeModeSupported
> |=
> > +SMBIOS_RISC_V_PSD_SUPERVISOR_MODE_SUPPORTED;
> > +  }
> > +  if ((ProcessorSpecDataHob->ProcessorSpecificData.InstSetSupported &
> RISC_V_ISA_USER_MODE_IMPLEMENTED) != 0) {
> > +
> > +ProcessorSpecDataHob->ProcessorSpecificData.PrivilegeModeSupported
> |=
> > +SMBIOS_RISC_V_PSD_USER_MODE_SUPPORTED;
> > +  }
> > +
> > +ProcessorSpecDataHob-
> >ProcessorSpecificData.MachineVendorId.Value64_L
> > += FirmwareContextHartSpecific->MachineVendorId.Value64_L;
> > +
> > +ProcessorSpecDataHob-
> >ProcessorSpecificData.MachineVendorId.Value64_H
> > += FirmwareContextHartSpecific->MachineVendorId.Value64_H;
> > +  ProcessorSpecDataHob-
> >ProcessorSpecificData.MachineArchId.Value64_L
> > += FirmwareContextHartSpecific->MachineArchId.Value64_L;
> > +  ProcessorSpecDataHob-
> >ProcessorSpecificData.MachineArchId.Value64_H
> > += FirmwareContextHartSpecific->MachineArchId.Value64_H;
> > +  ProcessorSpecDataHob-
> >ProcessorSpecificData.MachineImplId.Value64_L
> > += FirmwareContextHartSpecific->MachineImplId.Value64_L;
> > +  ProcessorSpecDataHob-
> >ProcessorSpecificData.MachineImplId.Value64_H
> > += FirmwareContextHartSpecific->MachineImplId.Value64_H;
> > +  return EFI_SUCCESS;
> > +}
> > diff --git
> > a/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareC
> > ontextProcessorSpecificLib.inf
> >
> b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareC
> > ontextProcessorSpecificLib.inf
> > new file mode 100644
> > index 00000000..ff841c3e
> > --- /dev/null
> > +++ b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/Firmw
> > +++ areContextProcessorSpecificLib.inf
> > @@ -0,0 +1,33 @@
> > +#/** @file
> > +#
> > +#  Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All
> > +rights reserved.<BR> # #  SPDX-License-Identifier:
> > +BSD-2-Clause-Patent # #**/
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x00010005
> 
> Can we update to a more recent specification version?
> 
> > +  BASE_NAME                      = FirmwareContextProcessorSpecificLib
> > +  FILE_GUID                      = 8BEC9FD7-C554-403A-94F1-0EBBFD81A242
> > +  MODULE_TYPE                    = PEIM
> > +  VERSION_STRING                 = 1.0
> > +  LIBRARY_CLASS                  = FirmwareContextProcessorSpecificLib
> > +
> > +[Sources.common]
> > +  FirmwareContextProcessorSpecificLib.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> 
> Please sort the above alphabetically.
> 
> > +  RiscVPkg/RiscVPkg.dec
> > +  Silicon/SiFive/SiFive.dec
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  PcdLib
> > +  MemoryAllocationLib
> 
> Please sort the above alphabetically.
> 
> > +  PrintLib
> > +
> > +[Pcd]
> > +
> 
> /
>     Leif
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49013): https://edk2.groups.io/g/devel/message/49013
Mute This Topic: https://groups.io/mt/34196352/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list