Re: [edk2-devel] [PATCH v2 23/30] OvmfPkg/LoongArchVirt: Add PeiServiceTablePointerLib

["Laszlo Ersek" <lersek@redhat.com>]

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 (#111139): https://edk2.groups.io/g/devel/message/111139
Mute This Topic: https://groups.io/mt/102413901/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-