Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 02/14] Silicon/SiFive: Add library module of SiFive RISC-V cores
On Wed, Oct 16, 2019 at 01:36:07AM +, Chang, Abner (HPS SW/FW Technologist)
wrote:
> > -Original Message-
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Leif Lindholm
> > Sent: Wednesday, October 2, 2019 5:15 AM
> > To: devel@edk2.groups.io; Chen, Gilbert
> > Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 02/14]
> > Silicon/SiFive: Add library module of SiFive RISC-V cores
> >
> > On Thu, Sep 19, 2019 at 11:51:19AM +0800, Gilbert Chen wrote:
> > > Initial version of SiFive RISC-V core libraries. Library of each core
> > > creates processor core SMBIOS data hob for building SMBIOS records in
> > > DXE phase.
> >
> > So yes, this implementation needs to change.
> > These should all implement the same LibraryClass.
>
> No. It shouldn't be the same library class (If you were saying the
> same LibraryClass). RISC-V SoC could be the combination of different
> RISC-V cores, or even the cores from different vendors. This depends
> on how SoC vendor combine those IPs.
Ah, OK. Sorry, did not realise this aspect.
> Either U54 or E51 could be a standalone SoC, while U54MC is the
> combination of 4 x U54 core and one E51 core.
>
> U5MC under Platform/SiFive could be 1-8 U5 core and optionally
> support E5 core. This is the special case for U500 VC707 platform
> because the core number could be customized.
>
> > Also, U54 appears to be a simple superset of U51.
> U54 is a single core.
>
> >
> > What I would suggest is creating a
> > Silicon/SiFive/Library/SiFiveCoreInfoLib, which calls into a
> > SiFiveSoCCoreInfoLib in Silicon/SiFive//Library, providing the acual
> > SoC-
> > specific bits.
>
> Platform system firmware integrator just pull in the necessary core
> libraries from Silicon/{vendor} and invoke the function to create
> specific core bits.
>
> I think this implementation is well and flexible which has no need to change.
Oh, my criticism was that it was *too* flexible (with resulting code
overhead). I still feel the very small source differences, especially
between E51/U54, indicate that these could be implemented by the same
source file but different .inf files.
But this is not something that needs to be addressed before this patch
goes into -staging.
Regards,
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49178): https://edk2.groups.io/g/devel/message/49178
Mute This Topic: https://groups.io/mt/34196349/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 02/14] Silicon/SiFive: Add library module of SiFive RISC-V cores
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Leif Lindholm
> Sent: Wednesday, October 2, 2019 5:15 AM
> To: devel@edk2.groups.io; Chen, Gilbert
> Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 02/14]
> Silicon/SiFive: Add library module of SiFive RISC-V cores
>
> On Thu, Sep 19, 2019 at 11:51:19AM +0800, Gilbert Chen wrote:
> > Initial version of SiFive RISC-V core libraries. Library of each core
> > creates processor core SMBIOS data hob for building SMBIOS records in
> > DXE phase.
>
> So yes, this implementation needs to change.
> These should all implement the same LibraryClass.
No. It shouldn't be the same library class (If you were saying the same
LibraryClass). RISC-V SoC could be the combination of different RISC-V cores,
or even the cores from different vendors. This depends on how SoC vendor
combine those IPs.
Either U54 or E51 could be a standalone SoC, while U54MC is the combination of
4 x U54 core and one E51 core.
U5MC under Platform/SiFive could be 1-8 U5 core and optionally support E5 core.
This is the special case for U500 VC707 platform because the core number could
be customized.
> Also, U54 appears to be a simple superset of U51.
U54 is a single core.
>
> What I would suggest is creating a
> Silicon/SiFive/Library/SiFiveCoreInfoLib, which calls into a
> SiFiveSoCCoreInfoLib in Silicon/SiFive//Library, providing the acual SoC-
> specific bits.
Platform system firmware integrator just pull in the necessary core libraries
from Silicon/{vendor} and invoke the function to create specific core bits.
I think this implementation is well and flexible which has no need to change.
>
> /
> Leif
>
> > Signed-off-by: Gilbert Chen
> > ---
> > .../E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c| 242
> +
> > .../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 51
> > .../U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c| 294
> +
> > .../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 51
> > .../Library/PeiCoreInfoHobLib/CoreInfoHob.c| 185 +
> > .../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 50
> > 6 files changed, 873 insertions(+)
> > create mode 100644
> > Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > create mode 100644
> > Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf
> > create mode 100644
> > Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > create mode 100644
> > Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf
> > create mode 100644
> > Silicon/SiFive/U54MCCoreplex/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > create mode 100644
> >
> Silicon/SiFive/U54MCCoreplex/Library/PeiCoreInfoHobLib/PeiCoreInfoHobL
> > ib.inf
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49039): https://edk2.groups.io/g/devel/message/49039
Mute This Topic: https://groups.io/mt/34196349/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 02/14] Silicon/SiFive: Add library module of SiFive RISC-V cores
On Thu, Sep 19, 2019 at 11:51:19AM +0800, Gilbert Chen wrote: > Initial version of SiFive RISC-V core libraries. Library of each core > creates processor core SMBIOS data hob for building SMBIOS > records in DXE phase. So yes, this implementation needs to change. These should all implement the same LibraryClass. Also, U54 appears to be a simple superset of U51. What I would suggest is creating a Silicon/SiFive/Library/SiFiveCoreInfoLib, which calls into a SiFiveSoCCoreInfoLib in Silicon/SiFive//Library, providing the acual SoC-specific bits. / Leif > Signed-off-by: Gilbert Chen > --- > .../E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c| 242 + > .../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 51 > .../U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c| 294 > + > .../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 51 > .../Library/PeiCoreInfoHobLib/CoreInfoHob.c| 185 + > .../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 50 > 6 files changed, 873 insertions(+) > create mode 100644 Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c > create mode 100644 > Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf > create mode 100644 Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c > create mode 100644 > Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf > create mode 100644 > Silicon/SiFive/U54MCCoreplex/Library/PeiCoreInfoHobLib/CoreInfoHob.c > create mode 100644 > Silicon/SiFive/U54MCCoreplex/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48354): https://edk2.groups.io/g/devel/message/48354 Mute This Topic: https://groups.io/mt/34196349/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 02/14] Silicon/SiFive: Add library module of SiFive RISC-V cores
Initial version of SiFive RISC-V core libraries. Library of each core
creates processor core SMBIOS data hob for building SMBIOS
records in DXE phase.
Signed-off-by: Gilbert Chen
---
.../E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c| 242 +
.../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 51
.../U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c| 294 +
.../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 51
.../Library/PeiCoreInfoHobLib/CoreInfoHob.c| 185 +
.../PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf| 50
6 files changed, 873 insertions(+)
create mode 100644 Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c
create mode 100644
Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf
create mode 100644 Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
create mode 100644
Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf
create mode 100644
Silicon/SiFive/U54MCCoreplex/Library/PeiCoreInfoHobLib/CoreInfoHob.c
create mode 100644
Silicon/SiFive/U54MCCoreplex/Library/PeiCoreInfoHobLib/PeiCoreInfoHobLib.inf
diff --git a/Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c
b/Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c
new file mode 100644
index ..b7140b53
--- /dev/null
+++ b/Silicon/SiFive/E51/Library/PeiCoreInfoHobLib/CoreInfoHob.c
@@ -0,0 +1,242 @@
+/**@file
+ Build up platform processor 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
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ Function to build core specific information HOB. RISC-V SMBIOS DXE driver
collect
+ this information and build SMBIOS Type44.
+
+ @param ParentProcessorGuidParent processor od this core.
ParentProcessorGuid
+ could be the same as CoreGuid if one
processor has
+ only one core.
+ @param ParentProcessorUid Unique ID of pysical processor which owns
this core.
+ @param HartId Hart ID of this core.
+ @param IsBootHart TRUE means this is the boot HART.
+ @param GuidHobDataPointer to receive EFI_HOB_GUID_TYPE.
+
+ @return EFI_SUCCESS The PEIM initialized successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+CreateE51CoreProcessorSpecificDataHob (
+ IN EFI_GUID *ParentProcessorGuid,
+ IN UINTN ParentProcessorUid,
+ IN UINTN HartId,
+ IN BOOLEAN IsBootHart,
+ OUT RISC_V_PROCESSOR_SPECIFIC_DATA_HOB **GuidHobData
+ )
+{
+ RISC_V_PROCESSOR_SPECIFIC_DATA_HOB *CoreGuidHob;
+ EFI_GUID *ProcessorSpecDataHobGuid;
+ RISC_V_PROCESSOR_SPECIFIC_DATA_HOB ProcessorSpecDataHob;
+ struct sbi_scratch *ThisHartSbiScratch;
+ struct sbi_platform *ThisHartSbiPlatform;
+ EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *FirmwareContext;
+ EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC *FirmwareContextHartSpecific;
+
+ DEBUG ((DEBUG_INFO, "%a: Entry.\n", __FUNCTION__));
+
+ if (GuidHobData == NULL) {
+return EFI_INVALID_PARAMETER;
+ }
+
+ ThisHartSbiScratch = sbi_hart_id_to_scratch (sbi_scratch_thishart_ptr(),
(UINT32)HartId);
+ DEBUG ((DEBUG_INFO, "SBI Scratch is at 0x%x.\n", ThisHartSbiScratch));
+ ThisHartSbiPlatform = (struct sbi_platform
*)sbi_platform_ptr(ThisHartSbiScratch);
+ DEBUG ((DEBUG_INFO, "SBI platform is at 0x%x.\n", ThisHartSbiPlatform));
+ FirmwareContext = (EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT
*)ThisHartSbiPlatform->firmware_context;
+ DEBUG ((DEBUG_INFO, "Firmware Context is at 0x%x.\n", FirmwareContext));
+ FirmwareContextHartSpecific = FirmwareContext->HartSpecific[HartId];
+ DEBUG ((DEBUG_INFO, "Firmware Context Hart specific is at 0x%x.\n",
FirmwareContextHartSpecific));
+
+ //
+ // Build up RISC_V_PROCESSOR_SPECIFIC_DATA_HOB.
+ //
+ CommonFirmwareContextHartSpecificInfo (
+ FirmwareContextHartSpecific,
+ ParentProcessorGuid,
+ ParentProcessorUid,
+ (EFI_GUID *)PcdGetPtr (PcdSiFiveE51CoreGuid),
+ HartId,
+ IsBootHart,
+
+ );
+ ProcessorSpecDataHob.ProcessorSpecificData.MModeExcepDelegation.Value64_L
= TO_BE_FILLED;
+ ProcessorSpecDataHob.ProcessorSpecificData.MModeExcepDelegation.Value64_H
= TO_BE_FILLED;
+
ProcessorSpecDataHob.ProcessorSpecificData.MModeInterruptDelegation.Value64_L =
TO_BE_FILLED;
+
ProcessorSpecDataHob.ProcessorSpecificData.MModeInterruptDelegation.Value64_H =
TO_BE_FILLED;
+ ProcessorSpecDataHob.ProcessorSpecificData.HartXlen =
RegisterLen64;
+ ProcessorSpecDataHob.ProcessorSpecificData.MachineModeXlen =
RegisterLen64;
+
