From: "Abner Chang" <abner.chang@hpe.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
"Chen, Gilbert" <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: Tue, 15 Oct 2019 15:26:21 +0000 [thread overview]
Message-ID: <CS1PR8401MB1192658383893D6929717494FF930@CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20191002170454.GE25504@bivouac.eciton.net>
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Leif Lindholm
> Sent: Thursday, October 3, 2019 1:05 AM
> To: devel@edk2.groups.io; Chen, Gilbert <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
>
> 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/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
>
>
next prev parent reply other threads:[~2019-10-15 15:26 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 ` [edk2-devel] " Leif Lindholm
2019-10-15 15:26 ` Abner Chang [this message]
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=CS1PR8401MB1192658383893D6929717494FF930@CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM \
--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