On Mon, Jan 10, 2022 at 06:27:37PM +0100, Heinrich Schuchardt wrote:

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.

Hi Heinrich,
Thank you very much for your constant feedback and pointers while
drafting this spec.

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

This makes perfect sense. Will update the spec.

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

Jessica has given an input to change the parameter from UINT32 * to
UINTN * since mhartid can be of XLEN. I think this is also a great
feedback. Will incorporate it in the next update of the spec.

Thanks!
Sunil

Best regards

Heinrich