EFI_RISCV_FIRMWARE_CONTEXT

[Warkentin, Andrei]

I am (finally) looking at fixing the PI spec language around EFI_PEI_SERVICES access.

EFI_RISCV_FIRMWARE_CONTEXT is currently defined as:

typedef struct {
UINT64 BootHartId;
VOID *PeiServiceTable; // PEI Service table
VOID *PrePiHobList; // Pre PI Hob List
UINT64 FlattenedDeviceTree; // Pointer to Flattened Device tree
} EFI_RISCV_FIRMWARE_CONTEXT;

How about rolling PrePiHobLib ad PeiServiceTable into a union? They are mutually exclusive with each other.

typedef struct {
UINT64 BootHartId;
union {
VOID *PeiServiceTable; // PEI Service table
VOID *PrePiHobList; // Pre PI Hob List
} PreDxe;
UINT64 FlattenedDeviceTree; // Pointer to Flattened Device tree
} EFI_RISCV_FIRMWARE_CONTEXT;

Also I see GetFirmwareContext/SetFirmwareContext is in the SBI lib which is...not an obvious place for it. Shall we break it out into a separate library?

A

[Warkentin, Andrei]

Actually, PrePiHobList in the PI Spec would be an ugly wart - the PI spec doesn't talk about a PEI-less phase at all. So in fact we could combine both PeiServiceTable and PrePiHobList into a VOID *single PreDxeContext.

Thoughts?

[Sunil V L]

Hi Andrei,

Make sense to me. I agree it doesn't belong to SbiLib. Better to move to
BaseLib since anyway RiscVGetSupervisorScratch() is defined in BaseLib?

[Warkentin, Andrei]

I've done even better.

I've entirely removed EFI_RISCV_FIRMWARE_CONTEXT. EFI_RISCV_FIRMWARE_CONTEXT isn't really "EFI-style", there's no precedent for it, and the stuff in it (DT ptr and BootHartId) don't have to be there.
EFI_RISCV_FIRMWARE_CONTEXT::FlattenedDeviceTree was only used in the Sec
EFI_RISCV_FIRWAMRE_CONTEXT::BootHartId was only use in CpuDxe (and it may as well be exposed via a HOB).

So now SSCRATCH is used for PrePiHobList (and in PEI, SSCRATCH would be used for the PEI services pointer). This matches AArch64 behavior and makes the PI spec changes infinitely simpler over w/e crazy stuff it lists today.

https://github.com/andreiw/edk2-rv-wip/tree/remove_riscv_fw_ctx

Please take a look, and if this looks okay I'll clean it up an post to edk2.

> --
> You received this message because you are subscribed to the Google Groups
> "RISC-V Firmware Exchange" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to
> fw-exchange...@riscv.org.
> To view this discussion on the web visit
> https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/54f6ac03-24ab-
> ea83-5537-d9ec6c03dbc1%40ventanamicro.com.

[Sunil V L]

Yes, that's even better. Do we need PI spec to get updated (or ECR
approved) for using CPU GUID in edk2?

[Warkentin, Andrei]

Yeah this all got prompted by PI spec cleanup. So I'll clean up the patches and file a PI spec change as well.

[Warkentin, Andrei]

So I've started looking at edk2-platforms and got an major anxiety attack over all the EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT craziness and the weird Platform/RISC-V/PlatformPkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.c handling.

The worst part is that there must be folks out there who are copying this design (so I've heard, anyway). And as they start upstreaming, we're going to be in a world of pain.

It's almost easier to just delete all the edk2-platform bits ☹, and add a separate PEI-enabled target to RiscVVirt just to show the proper SEC->PEI->DXE flow, using PPIs for passing all the RISC-V "extras" from SEC to PEI and using SSCRATCH only for PEI Services Table (I've filed a PIWG issue for that cleanup - https://mantis.uefi.org/mantis/view.php?id=2397)

Any thoughts?

A

> exchange/PH8PR11MB6856A9B1205E606D3A38BB218329A%40PH8PR11MB685
> 6.namprd11.prod.outlook.com.

[Sunil V L]

Yeah, it is bit messy :-(

The challenge in edk2-platforms is because of its design of starting in
M-mode. It needs to adhere to both OpenSBI calling convention and  EFI
calling convention between SEC and PEI phase. OpenSBI launches the next
S-mode stage only with two arguments (boot hart ID and FDT). But Pei
expects EFI_SEC_PEI_HAND_OFF and PpiList. This probably makes it
difficult to remove this structure entirely.

[Aaron Durbin]

On Fri, Jul 14, 2023 at 12:53 AM Sunil V L <sun...@ventanamicro.com> wrote:

Yeah, it is bit messy :-(

The challenge in edk2-platforms is because of its design of starting in
M-mode. It needs to adhere to both OpenSBI calling convention and  EFI

It doesn't have to start in M-mode, correct? Are we just trying to formfit something that may not be the best approach/solution?

 

To view this discussion on the web visit https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/944f29f9-7f43-c2a0-ecc9-9ecd6ce94147%40ventanamicro.com.

[Sunil V L]

On 14/07/23 20:53, Aaron Durbin wrote:

On Fri, Jul 14, 2023 at 12:53 AM Sunil V L <sun...@ventanamicro.com> wrote:

Yeah, it is bit messy :-(

The challenge in edk2-platforms is because of its design of starting in
M-mode. It needs to adhere to both OpenSBI calling convention and  EFI

It doesn't have to start in M-mode, correct? Are we just trying to formfit something that may not be the best approach/solution?

Correct. But the original EDK2 port for Sifive U5* boards(which exists in edk2-platforms) always assumed to start in M-mode by integrating OpenSBI in EDK2 as a library. For qemu virt machine, we changed to payload design where EDK2 will start in S-mode as a payload of previous stage OpenSBI.

[Warkentin, Andrei]

It needs to be reworked so that OpenSBI doesn't directly call PEI then, but a SEC stub that passes the correct parameters. The PEI entry signature can't randomly be changed like that.

A

> https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/944f29f9-7f43-
> c2a0-ecc9-9ecd6ce94147%40ventanamicro.com.

[Warkentin, Andrei]

I can see either of these approaches being adopted, depending on the scenario:

 

H/M-Mode OpenSBI -> S-Mode PrePi -> DXE

M-Mode OpenSBI -> S-Mode SEC -> S-Mode PEI -> DXE

M-Mode SEC + OpenSBI -> M/S-Mode PEI -> DXE

 

It would be beneficial to have well-maintained examples of these. The issue with the U5 port is some bad choices made that are not in line with the Tiano design overall.

 

I just realized that Qemu can be used for the U5 cleanup work…

 

A

To view this discussion on the web visit https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/9d32b9b1-aded-9bf1-8d0c-cc254cfb5adc%40ventanamicro.com.

[Dhaval Sharma]

Universal Payload (one of our RISE projects) is something closer to H/M-Mode OpenSBI -> S-Mode PrePi -> DXE. But I am not sure what do you mean by PrePi in this case.

[Dhaval Sharma]

Also wanted to add one more point reg removal of EFI_RISCV_FIRMWARE_CONTEXT that I faced recently.

One assumption it made was there is already a contract between PI and UEFI stages (without explicity spec in place). This causes problem on a config which does not have PI phase but only UEFI (Universal Payload) for obvious reasons. Andrei's implementation should address that well. Thanks Andrei!

=D

[Warkentin, Andrei]

I better hurry up with those patches for review (at least for RiscVVirt).

 

A

 

To view this discussion on the web visit https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/22ab10e0-54e2-4a20-8f05-1ec5e9c3b273n%40riscv.org.