Re: Review request: New EFI_RISCV_BOOT_PROTOCOL


Heinrich Schuchardt
 

On 1/11/22 02:07, Chang, Abner (HPS SW/FW Technologist) wrote:
Hi Sunil,
Instead of having a spec for EFI_RISCV_BOOT_PROTOCOL specifically, I suggest to have a RISC-V EFI Protocols Specification. This spec accommodates EFI_RISCV_BOOT_PROTOCOL, the future EFI protocols for RISC-V platform, and those we had implemented in edk2.
Which protocols in EDK II do you refer to?

"git grep -n RISC edk2/ | grep PRO" yields no result.

Once we have agreed on the EFI_RISCV_BOOT_PROTOCOL we should create an issue in bugzilla.tianocore.org and create a Mantis entry to get it merged into the UEFI specification.

Best regards

Heinrich

Thanks
Abner

-----Original Message-----
From: Heinrich Schuchardt <heinrich.schuchardt@...>
Sent: Tuesday, January 11, 2022 1:28 AM
To: Sunil V L <sunilvl@...>
Cc: tech-unixplatformspec@...; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>; Anup Patel
<apatel@...>; Atish Patra <atishp@...>; Jessica
Clarke <jrtc27@...>
Subject: Re: Review request: New EFI_RISCV_BOOT_PROTOCOL

On 1/10/22 18:02, Sunil V L wrote:
Hi All,

As we discussed in the Platform HSC meeting today, here is the document
which details a new RISC-V specific EFI protocol.

https://github.com/riscv-non-isa/riscv-
uefi/releases/download/0.1/EFI_RISCV_BOOT_PROTOCOL.pdf

Currently, the main use case of this protocol is to pass the boot hartid to
the OS. But this can be extended in future if required. A PoC has been
developed using EDK2 and Linux.

More details of this requirement and alternatives discussed are available at
http://lists.infradead.org/pipermail/linux-riscv/2021-December/010604.html.

I request your review and will be great if you provide the feedback by
01/17.

Thanks!
Sunil


Dear Sunil,

thank you for drafting the protocol specification.

The interface of a protocol may change from version to version.
Therefore I understand why there must be a path to convey this
information. But using a function like
EFI_RISCV_BOOT_PROTOCOL.GetProtocolVersion() makes accessing this
information unnecessarily complicated. Instead consider adding a version
field as first element of the interface like many other UEFI protocols
do. This will also decrease the implementation size. For alignment
reasons make this field UINT64. Other protocols call such a field
"Revision". Please, provide a define for the current version. E.g.

#define EFI_RISCV_BOOT_PROTOCOL_REVISION 0x00010000
#define EFI_RISCV_BOOT_PROTOCOL_LATEST_VERSION \
EFI_RISCV_BOOT_PROTOCOL_REVISION

Function EFI_RISCV_BOOT_PROTOCOL.GetBootHartId() looks ok to me and is
well described.

Best regards

Heinrich

Join tech-unixplatformspec@lists.riscv.org to automatically receive all group messages.