public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: devel@edk2.groups.io, gilbert.chen@hpe.com
Cc: Palmer Dabbelt <palmer@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
Date: Wed, 2 Oct 2019 18:04:54 +0100	[thread overview]
Message-ID: <20191002170454.GE25504@bivouac.eciton.net> (raw)
In-Reply-To: <20190919035131.4700-6-gilbert.chen@hpe.com>

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@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/FirmwareContextProcessorSpecificLib.c
>  create mode 100644 Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareContextProcessorSpecificLib.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/FirmwareContextProcessorSpecificLib.c b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareContextProcessorSpecificLib.c
> new file mode 100644
> index 00000000..4d4c51dc
> --- /dev/null
> +++ b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareContextProcessorSpecificLib.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..

> +  )
> +{
> +  //
> +  // 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/FirmwareContextProcessorSpecificLib.inf b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareContextProcessorSpecificLib.inf
> new file mode 100644
> index 00000000..ff841c3e
> --- /dev/null
> +++ b/Platform/RiscV/Library/FirmwareContextProcessorSpecificLib/FirmwareContextProcessorSpecificLib.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

  reply	other threads:[~2019-10-02 17:04 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-19  3:51 [plaforms/devel-riscv-v2 PATCHv2 00/14] Add SiFive U500 VC707 FPGA Platform Gilbert Chen
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 01/14] Silicon/SiFive: Initial version of SiFive silicon package Gilbert Chen
2019-10-01  0:41   ` [edk2-devel] " Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 02/14] Silicon/SiFive: Add library module of SiFive RISC-V cores Gilbert Chen
2019-10-01 21:14   ` [edk2-devel] " Leif Lindholm
2019-10-16  1:36     ` Abner Chang
2019-10-17 10:33       ` Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 03/14] platforms/RiscV: Initial version of RISC-V platform package Gilbert Chen
2019-10-02  9:07   ` [edk2-devel] " Leif Lindholm
2019-10-15 15:24     ` Abner Chang
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 04/14] RiscV/Include: Initial version of header files in " Gilbert Chen
2019-10-02 16:46   ` [edk2-devel] " Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 05/14] RiscV/Library: Initial version of libraries introduced " Gilbert Chen
2019-10-02 17:04   ` Leif Lindholm [this message]
2019-10-15 15:26     ` [edk2-devel] " Abner Chang
2019-10-18  5:23     ` Abner Chang
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 06/14] RiscV/Universal: Initial version of common RISC-V SEC module Gilbert Chen
2019-10-02 19:43   ` [edk2-devel] " Leif Lindholm
2019-10-15 15:27     ` Abner Chang
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 07/14] RiscV/SiFive: Initial version of SiFive U500 platform package Gilbert Chen
2019-10-02 20:16   ` [edk2-devel] " Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 08/14] U500Pkg/Include: Header files of SiFive U500 platform Gilbert Chen
2019-10-02 21:00   ` [edk2-devel] " Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 09/14] U500Pkg/Library: Initial version of PlatformBootManagerLib Gilbert Chen
2019-10-02 22:02   ` [edk2-devel] " Leif Lindholm
2019-10-18  6:23     ` Abner Chang
2019-10-21 14:51       ` Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 10/14] U500Pkg/Library: Library instances of U500 platform library Gilbert Chen
2019-10-03 16:32   ` [edk2-devel] " Leif Lindholm
2019-10-17  2:21     ` Abner Chang
2019-10-17  7:44       ` Abner Chang
2019-10-17 11:19         ` Leif Lindholm
2019-10-17 16:09           ` Abner Chang
2019-10-17 16:38             ` Leif Lindholm
2019-10-18  5:24               ` Abner Chang
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 11/14] U500Pkg/RamFvbServiceruntimeDxe: FVB driver for EFI variable Gilbert Chen
2019-10-03 16:58   ` [edk2-devel] " Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 12/14] U500Pkg/TimerDxe: Platform Timer DXE driver Gilbert Chen
2019-10-03 17:30   ` [edk2-devel] " Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 13/14] U500Pkg/PlatformPei: Platform initialization PEIM Gilbert Chen
2019-10-03 17:38   ` [edk2-devel] " Leif Lindholm
2019-09-19  3:51 ` [plaforms/devel-riscv-v2 PATCHv2 14/14] Platforms: Readme file updates Gilbert Chen
2019-10-03 17:45   ` [edk2-devel] " Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191002170454.GE25504@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox