From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=r6oeEntL; spf=pass (domain: linaro.org, ip: 209.85.221.67, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by groups.io with SMTP; Wed, 02 Oct 2019 10:04:59 -0700 Received: by mail-wr1-f67.google.com with SMTP id q9so1747508wrm.8 for ; Wed, 02 Oct 2019 10:04:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=yQby5E187/llMFVU13yB4GfZ10CNM65kVWTBhWZIBaI=; b=r6oeEntL7IVugOc/uSHK/wP9TAzm/lIt6+xsv9G/e6MsWu6ab1brJCNJqermPVZF7c gMYjKkNQ9JhCI+8+h2C3N2Rvw88XvmerMGeTdnr3ox+IiV6RrLQdUuYAyHRYjx1y3NsH eN7KpC0SWYWPv9UJrIntR0PARApKRZ+5XkN9f7bcx8k32oEF7+vbX2zyadROYNGaWw7r msjKFS1bYzOzDsAEHd3eTQO7ZtIaerOCYp0GGWwgfzGg70M7VSSd8HGTpny5pgSB1Qsv gIUOeeIBNv13dccU/ahuPqm+JxZ5r8irlg0txkqnMHO2SOE3oM6G0l/WTFLFdQx55I45 qB6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=yQby5E187/llMFVU13yB4GfZ10CNM65kVWTBhWZIBaI=; b=qt4Dzmkk+/Wjzd3l75uMUPIfJu8n8n49ThFUtgeLUEndZp1NAampeyGkdnJLb5IR9L fZH+ADs0w2bGMYNyqpv1sGtokyDJ4/AZNKhz3Fi5A/wvTVDmGtlMrqGLLmyipUWNOO+5 TRzS8jMweRsla5t56lw+SFAZ9BJV4dSFJ/s+MJITbvogaEraFoSjVQ3fU7xgSvBI+tD+ 9aPJwITdCJLyPe+scsY96jDvxdtPyAhIwEOD4q1XD1XXuKpMe00OVnuCHgyqSCqQxLcc AzgsCldanhi2jESb5xO9Cwj4DKZleXExcNHjflKuQAHptUOq9GMSziKwkLc54runVetW EdmA== X-Gm-Message-State: APjAAAU0JWPN056bIMZJlPRb8kLyLrF+i6DIm922rlps54tJiKApR78V Ux/IsZGAsrp4liVXtG+w3hJleHR7eH4= X-Google-Smtp-Source: APXvYqxYHM4e7uWTuH/dGu1Sz5ZeQYomc6kaWNy6rZ5LBeTL+jzga4hBbZk/49hCScNbCO5H1BAMIg== X-Received: by 2002:adf:e84c:: with SMTP id d12mr3418756wrn.373.1570035897110; Wed, 02 Oct 2019 10:04:57 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id t6sm11726820wmf.8.2019.10.02.10.04.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Oct 2019 10:04:56 -0700 (PDT) Date: Wed, 2 Oct 2019 18:04:54 +0100 From: "Leif Lindholm" To: devel@edk2.groups.io, gilbert.chen@hpe.com Cc: Palmer Dabbelt Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 05/14] RiscV/Library: Initial version of libraries introduced in RISC-V platform package Message-ID: <20191002170454.GE25504@bivouac.eciton.net> References: <20190919035131.4700-1-gilbert.chen@hpe.com> <20190919035131.4700-6-gilbert.chen@hpe.com> MIME-Version: 1.0 In-Reply-To: <20190919035131.4700-6-gilbert.chen@hpe.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > --- > .../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.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +// > +// The package level header files this module uses > +// > +#include > + > +// > +// The Library classes this module consumes > +// > +#include > +#include > +#include Please sort these includes alphabetically. > + > +#include > +#include > +#include Please sort these includes alphabetically. > +#include > +#include > +#include > + > +/** > + 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.
> +# > +# 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