Re: [edk2-devel] [PATCH v2 23/30] OvmfPkg/LoongArchVirt: Add PeiServiceTablePointerLib
On 11/6/23 04:30, Chao Li wrote:
> Use a register to save PeiServiceTable pointer. This Library provides
> PeiServiceTable pointer saveing and retrieval serivces.
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4584
>
> Cc: Ard Biesheuvel
> Cc: Jiewen Yao
> Cc: Jordan Justen
> Cc: Gerd Hoffmann
> Signed-off-by: Chao Li
> Co-authored-by: Xianglai Li
> Co-authored-by: Bibo Mao
> ---
> .../PeiServicesTablePointer.c | 75 +++
> .../PeiServicesTablePointerLib.inf| 31
> 2 files changed, 106 insertions(+)
> create mode 100644
> OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
> create mode 100644
> OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
Why is this under OvmfPkg?
Laszlo
>
> diff --git
> a/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
>
> b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
> new file mode 100644
> index 00..dcbcb50131
> --- /dev/null
> +++
> b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
> @@ -0,0 +1,75 @@
> +/** @file
> + PEI Services Table Pointer Library.
> +
> + Copyright (c) 2023 Loongson Technology Corporation Limited. All rights
> reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + Caches a pointer PEI Services Table.
> +
> + Caches the pointer to the PEI Services Table specified by
> PeiServicesTablePointer
> + in a platform specific manner.
> +
> + If PeiServicesTablePointer is NULL, then ASSERT ().
> +
> + @paramPeiServicesTablePointer The address of PeiServices pointer.
> +**/
> +VOID
> +EFIAPI
> +SetPeiServicesTablePointer (
> + IN CONST EFI_PEI_SERVICES **PeiServicesTablePointer
> + )
> +{
> + CsrWrite (LOONGARCH_CSR_KS0, (UINTN)PeiServicesTablePointer);
> +}
> +
> +/**
> + Retrieves the cached value of the PEI Services Table pointer.
> +
> + Returns the cached value of the PEI Services Table pointer in a CPU
> specific manner
> + as specified in the CPU binding section of the Platform Initialization
> Pre-EFI
> + Initialization Core Interface Specification.
> +
> + If the cached PEI Services Table pointer is NULL, then ASSERT ().
> +
> + @return The pointer to PeiServices.
> +**/
> +CONST EFI_PEI_SERVICES **
> +EFIAPI
> +GetPeiServicesTablePointer (
> + VOID
> + )
> +{
> + return (CONST EFI_PEI_SERVICES **)(CsrRead (LOONGARCH_CSR_KS0));
> +}
> +
> +/**
> +Perform CPU specific actions required to migrate the PEI Services Table
> +pointer from temporary RAM to permanent RAM.
> +
> +For IA32 CPUs, the PEI Services Table pointer is stored in the 4 bytes
> +immediately preceding the Interrupt Descriptor Table (IDT) in memory.
> +For X64 CPUs, the PEI Services Table pointer is stored in the 8 bytes
> +immediately preceding the Interrupt Descriptor Table (IDT) in memory.
> +For Itanium and ARM CPUs, a the PEI Services Table Pointer is stored in
> +a dedicated CPU register. This means that there is no memory storage
> +associated with storing the PEI Services Table pointer, so no additional
> +migration actions are required for Itanium or ARM CPUs.
> +**/
> +VOID
> +EFIAPI
> +MigratePeiServicesTablePointer (
> + VOID
> + )
> +{
> + return;
> +}
> diff --git
> a/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
>
> b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
> new file mode 100644
> index 00..274cb2f781
> --- /dev/null
> +++
> b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
> @@ -0,0 +1,31 @@
> +## @file
> +# PEI Services Table Pointer Library.
> +#
> +# Copyright (c) 2023 Loongson Technology Corporation Limited. All rights
> reserved.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +[Defines]
> + INF_VERSION= 0x00010005
> + BASE_NAME = PeiServicesTablePointerLib
> + FILE_GUID = 098B0DB0-AD01-8886-D409-90CBC7E89154
> + MODULE_TYPE= PEIM
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = PeiServicesTablePointerLib|PEIM PEI_CORE
> SEC
> +
> +#
> +# VALID_ARCHITECTURES = LOONGARCH64
> +#
> +
> +[Sources]
> + PeiServicesTablePointer.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + DebugLib
> +
> +[Pcd]
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110932): https://edk2.groups.io/g/devel/message/110932
Mute This Topic: https://groups.io/mt/102413901/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.co
Re: [edk2-devel] [PATCH v2 23/30] OvmfPkg/LoongArchVirt: Add PeiServiceTablePointerLib
Hi Laszlo,
This is a good question, same as the previous email, some ARCH don't
require running code on memory during PEI phase or other reason can not
used the MdePkg version, so this pointer will be saved by some register,
I saw the ArmPkg version also uses a register to save this pointer.
If I move ArmPkg version PeiServiceTablePointer to MdePkg and the method
of getting and saving the pointer use a new library which related
architecture, other word, the API of PeiServiceTablePointer has not
changed, adding a new library that PeiServiceTablePointer depends on,
the real saving and reading method completed in the new library. Do you
think this way is better?
Thanks,
Chao
在 2023/11/9 06:22, Laszlo Ersek 写道:
On 11/6/23 04:30, Chao Li wrote:
Use a register to save PeiServiceTable pointer. This Library provides
PeiServiceTable pointer saveing and retrieval serivces.
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4584
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Jordan Justen
Cc: Gerd Hoffmann
Signed-off-by: Chao Li
Co-authored-by: Xianglai Li
Co-authored-by: Bibo Mao
---
.../PeiServicesTablePointer.c | 75 +++
.../PeiServicesTablePointerLib.inf| 31
2 files changed, 106 insertions(+)
create mode 100644
OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
create mode 100644
OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
Why is this under OvmfPkg?
Laszlo
diff --git
a/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
new file mode 100644
index 00..dcbcb50131
--- /dev/null
+++
b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointer.c
@@ -0,0 +1,75 @@
+/** @file
+ PEI Services Table Pointer Library.
+
+ Copyright (c) 2023 Loongson Technology Corporation Limited. All rights
reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include
+#include
+#include
+#include
+#include
+
+/**
+ Caches a pointer PEI Services Table.
+
+ Caches the pointer to the PEI Services Table specified by
PeiServicesTablePointer
+ in a platform specific manner.
+
+ If PeiServicesTablePointer is NULL, then ASSERT ().
+
+ @paramPeiServicesTablePointer The address of PeiServices pointer.
+**/
+VOID
+EFIAPI
+SetPeiServicesTablePointer (
+ IN CONST EFI_PEI_SERVICES **PeiServicesTablePointer
+ )
+{
+ CsrWrite (LOONGARCH_CSR_KS0, (UINTN)PeiServicesTablePointer);
+}
+
+/**
+ Retrieves the cached value of the PEI Services Table pointer.
+
+ Returns the cached value of the PEI Services Table pointer in a CPU specific
manner
+ as specified in the CPU binding section of the Platform Initialization
Pre-EFI
+ Initialization Core Interface Specification.
+
+ If the cached PEI Services Table pointer is NULL, then ASSERT ().
+
+ @return The pointer to PeiServices.
+**/
+CONST EFI_PEI_SERVICES **
+EFIAPI
+GetPeiServicesTablePointer (
+ VOID
+ )
+{
+ return (CONST EFI_PEI_SERVICES **)(CsrRead (LOONGARCH_CSR_KS0));
+}
+
+/**
+Perform CPU specific actions required to migrate the PEI Services Table
+pointer from temporary RAM to permanent RAM.
+
+For IA32 CPUs, the PEI Services Table pointer is stored in the 4 bytes
+immediately preceding the Interrupt Descriptor Table (IDT) in memory.
+For X64 CPUs, the PEI Services Table pointer is stored in the 8 bytes
+immediately preceding the Interrupt Descriptor Table (IDT) in memory.
+For Itanium and ARM CPUs, a the PEI Services Table Pointer is stored in
+a dedicated CPU register. This means that there is no memory storage
+associated with storing the PEI Services Table pointer, so no additional
+migration actions are required for Itanium or ARM CPUs.
+**/
+VOID
+EFIAPI
+MigratePeiServicesTablePointer (
+ VOID
+ )
+{
+ return;
+}
diff --git
a/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
new file mode 100644
index 00..274cb2f781
--- /dev/null
+++
b/OvmfPkg/LoongArchVirt/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
@@ -0,0 +1,31 @@
+## @file
+# PEI Services Table Pointer Library.
+#
+# Copyright (c) 2023 Loongson Technology Corporation Limited. All rights
reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+[Defines]
+ INF_VERSION= 0x00010005
+ BASE_NAME = PeiServicesTablePointerLib
+ FILE_GUID = 098B0DB0-AD01-8886-D409-90CBC7E89154
+ MODULE_TYPE= PEIM
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PeiServicesTablePointerLib|PEIM PEI_CORE SEC
+
+#
+# VALID_ARCHITECTURES = LOONGARCH64
+#
+
+[Sources]
+ PeiServicesTablePointer.c
+
+[Packages]
+ MdePkg/MdeP
Re: [edk2-devel] [PATCH v2 23/30] OvmfPkg/LoongArchVirt: Add PeiServiceTablePointerLib
On 11/10/23 07:44, Chao Li wrote: > Hi Laszlo, > > This is a good question, same as the previous email, some ARCH don't > require running code on memory during PEI phase or other reason can not > used the MdePkg version, so this pointer will be saved by some register, > I saw the ArmPkg version also uses a register to save this pointer. > > If I move ArmPkg version PeiServiceTablePointer to MdePkg and the method > of getting and saving the pointer use a new library which related > architecture, other word, the API of PeiServiceTablePointer has not > changed, adding a new library that PeiServiceTablePointer depends on, > the real saving and reading method completed in the new library. Do you > think this way is better? Right. I think you need a loongarch-specific library instance of PeiServicesTablePointerLib under MdePkg. Presumably, if / when you enable edk2 on *physical* loongarch platforms, you're going to need the same library instance. And therefore it should not be under OvmfPkg, but MdePkg. In fact, now that I'm reading the comments in "MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c", it seems that the architecture bindings in the Platform Init specification actually document how the PEI services table pointer is supposed to be stored before system memory becomes available (IIUC). The latest version of the PI spec that I have is 1.8. In Volume I, there is a section titled "I-9.4 PEI Services Table Retrieval". It considers the following architectures: X86, x64, Itanium, ARM, AArch64, RISC-V. I think that in the longer term, you should file a PIWG Mantis ticket for extending this section, with an Engineering Change Request where you describe how this mechanism should work on loongarch. And then the implementation certainly belongs to MdePkg. (Note that I'm *not* saying you should first update the specification and then implement it in edk2 -- that's not my point. My point is that *eventually*, this mechanism will become a part of the public loongarch bindings, and therefore you can already start introducing it under MdePkg, in my opinion anyway.) Note that "prior art" is inconsistent here, regarding edk2 locations, because, as you say, the arm/aarch64 implementation lives under ArmPkg -- but it's quite unlikely IMO that a LoongArchPkg top-level directory would be introduced. In the longer term, we might want to consolidate all PeiServicesTablePointerLib instances under MdePkg. Then your question is, IIUC, whether to reuse any portion of the ArmPkg implementation. I don't think that's necessary; the library is so small, there is effectively *nothing generic* in it. Put differently, all of it is architecture specific. Thus I think you can just add a totally self-contained loongarch instance. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39): https://edk2.groups.io/g/devel/message/39 Mute This Topic: https://groups.io/mt/102413901/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 23/30] OvmfPkg/LoongArchVirt: Add PeiServiceTablePointerLib
Hi Laszlo, I see, I will look into this PI SPEC and try to implement it in MdePkg first and then to update the SPEC. Thanks, Chao On 2023/11/13 19:02, Laszlo Ersek wrote: On 11/10/23 07:44, Chao Li wrote: Hi Laszlo, This is a good question, same as the previous email, some ARCH don't require running code on memory during PEI phase or other reason can not used the MdePkg version, so this pointer will be saved by some register, I saw the ArmPkg version also uses a register to save this pointer. If I move ArmPkg version PeiServiceTablePointer to MdePkg and the method of getting and saving the pointer use a new library which related architecture, other word, the API of PeiServiceTablePointer has not changed, adding a new library that PeiServiceTablePointer depends on, the real saving and reading method completed in the new library. Do you think this way is better? Right. I think you need a loongarch-specific library instance of PeiServicesTablePointerLib under MdePkg. Presumably, if / when you enable edk2 on *physical* loongarch platforms, you're going to need the same library instance. And therefore it should not be under OvmfPkg, but MdePkg. In fact, now that I'm reading the comments in "MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c", it seems that the architecture bindings in the Platform Init specification actually document how the PEI services table pointer is supposed to be stored before system memory becomes available (IIUC). The latest version of the PI spec that I have is 1.8. In Volume I, there is a section titled "I-9.4 PEI Services Table Retrieval". It considers the following architectures: X86, x64, Itanium, ARM, AArch64, RISC-V. I think that in the longer term, you should file a PIWG Mantis ticket for extending this section, with an Engineering Change Request where you describe how this mechanism should work on loongarch. And then the implementation certainly belongs to MdePkg. (Note that I'm *not* saying you should first update the specification and then implement it in edk2 -- that's not my point. My point is that *eventually*, this mechanism will become a part of the public loongarch bindings, and therefore you can already start introducing it under MdePkg, in my opinion anyway.) Note that "prior art" is inconsistent here, regarding edk2 locations, because, as you say, the arm/aarch64 implementation lives under ArmPkg -- but it's quite unlikely IMO that a LoongArchPkg top-level directory would be introduced. In the longer term, we might want to consolidate all PeiServicesTablePointerLib instances under MdePkg. Then your question is, IIUC, whether to reuse any portion of the ArmPkg implementation. I don't think that's necessary; the library is so small, there is effectively *nothing generic* in it. Put differently, all of it is architecture specific. Thus I think you can just add a totally self-contained loongarch instance. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#89): https://edk2.groups.io/g/devel/message/89 Mute This Topic: https://groups.io/mt/102413901/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
