On 10/05/2021 18:37, Atish Patra wrote:

This patch adds all the required content to make RISC-V EBBR compatible. The additional content is not a lot given that we just need to update the architecture specific sections for RISC-V. Rest of the document is ISA agnostic anyways.

Signed-off-by: Atish Patra atish.patra@wdc.com

Hi Atish,

I'm picking up this patch. There are some editorial changes that I'm making (line breaks, glossary organization, etc), but I don't need you to respin the patch. However, I have some comments questions before I push the result out.

--- a/source/chapter1-about.rst +++ b/source/chapter1-about.rst @@ -158,6 +158,8 @@ easily meet the stricter BBR requirements. By definition, all BBR compliant systems are also EBBR compliant, but the converse is not true.

+This specification is also referenced by RISC-V platform specification [RVPLTSPEC]_.

s/also //

I've dropped text that references RISC-V in comparison to ARM. The RISC-V wording is able to stand on it's own without reference. :-)

Conventions Used in this Document

@@ -181,14 +183,12 @@ This document uses the following terms and abbreviations.

.. glossary::

[...]

• SiP

•  Silicon Partner. In this document, the silicon manufacturer.
  

I've rearranged the glossary to put the generic terms at the top, and the architecture specifics below that. Also minor editorial so that all the terms are handled as glossary. (.. glossary:: is needed in each section)

diff --git a/source/chapter2-uefi.rst b/source/chapter2-uefi.rst index 53c962ab5c25..18a9b5404cba 100644 --- a/source/chapter2-uefi.rst +++ b/source/chapter2-uefi.rst @@ -209,7 +209,7 @@ Resident UEFI firmware might target a specific privilege level. In contrast, UEFI Loaded Images, such as third-party drivers and boot applications, must not contain any built-in assumptions that they are to be loaded at a given privilege level during boot time since they can, for example, -legitimately be loaded into either EL1 or EL2 on AArch64. +legitimately be loaded into either EL1 or EL2 on AArch64 and HS/VS/S mode on RISC-V.

AArch64 Exception Levels

@@ -232,6 +232,45 @@ UEFI-compliant Operating System. In this instance, the UEFI boot-time environment can be provided, as a virtualized service, by the hypervisor and not as part of the host firmware.

+RISC-V Privilege Levels +-----------------------

+Unlike AARCH64, RISC-V doesn't define dedicated privilege levels for hypervisor

s/Unlike AARCH64, //

+enabled platforms. The supervisor mode becomes HS mode where a hypervisor or a hosting-capable +operating system runs while the guest OS runs in virtual S mode (VS mode) as shown in below figures. +Resident UEFI firmware can be executed in M mode or S/HS mode during POST. However, +the UEFI images must be loaded in HS or VS mode if virtualization is available at OS load time.

editorial: I've reflowed most paragraphs to use semantic breaks (e.g., one sentence per line). It reduces the reflow changebars on future edits to the text.

+.. image:: images/riscv-sbi-intro1.png

• :align: center

+Figure1. RISC-V System without H-extension

+.. image:: images/riscv-sbi-intro2.png

• :align: center

+Figure2. RISC-V System with H-extension

I've dropped the images; I don't think they're necessary for spec language, but this can be debated.

In the mean time it doesn't have to hold up the rest of the changes.

+UEFI Boot at S mode +^^^^^^^^^^^^^^^^^^^^^^

+Most systems are expected to boot UEFI at S mode as the hypervisor extension [RVHYPSPEC]_ is +still in draft state.

+UEFI Boot at HS mode +^^^^^^^^^^^^^^^^^^^^

+Any platform with hypervisor extension enabled most likely to boot UEFI at HS mode, +to allow for the installation of a hypervisor or a virtualization aware Operating System.

+UEFI Boot at VS mode +^^^^^^^^^^^^^^^^^^^^^^

+Booting of UEFI at VS mode is employed within a hypervisor hosted Guest Operating System environment, +to allow the subsequent booting of a UEFI-compliant Operating System. +In this instance, the UEFI boot-time environment can be provided, as a +virtualized service, by the hypervisor and not as part of the host firmware.

• UEFI Boot Services

diff --git a/source/chapter3-secureworld.rst b/source/chapter3-secureworld.rst index 9c51bca24f33..d6bfbd322837 100644 --- a/source/chapter3-secureworld.rst +++ b/source/chapter3-secureworld.rst @@ -27,3 +27,15 @@ Platforms without EL3 must implement one of: However, the spin table protocol is strongly discouraged. Future versions of this specification will only allow PSCI, and PSCI should be implemented in all new designs.

+RISC-V Multiprocessor Startup Protocol +====================================== +The resident firmware in M mode or hypervisor running in HS mode must implement +and conform to at least SBI [RVSBISPEC]_ v0.2 with HART +State Management(HSM) extension for both RV32 and RV64. In addition to that, the +firmware must ensure the following condition before jumping to the UEFI +application.

+The device tree must contain a property named **boot-hartid** under the */chosen* +node. This property must indicate the id of the booting hart. The corresponding entry +in the ACPI tables has not been defined yet as ACPI support in RISC-V is under progress.

I'm just dropping the ACPI references. If the ACPI bindings aren't defined yet, then it doesn't make sense to reference them.

diff --git a/source/index.rst b/source/index.rst index 48318757f717..acd214d25323 100644 --- a/source/index.rst +++ b/source/index.rst @@ -1,5 +1,6 @@ .. EBBR Source Document Copyright Arm Limited, 2017-2019

• Copyright Western Digital Corporation or its affiliates, 2021

I don't think this is valid for a copyright message. In fact, the Arm line above it is also wrong. It should be specific to the copyright owner without the "or its affiliates". Is it okay by you if Idrop that last bit.

I'll fix the Arm line separately

 SPDX-License-Identifier: CC-BY-SA-4.0

#################################################### @@ -8,6 +9,8 @@ Embedded Base Boot Requirements (EBBR) Specification

Copyright © 2017-2019 Arm Limited and Contributors.

+Copyright © 2021 Western Digital Corporation or its affiliates.

Ditto

This work is licensed under the Creative Commons Attribution-ShareAlike 4.0 International License. To view a copy of this license, visit http://creativecommons.org/licenses/by-sa/4.0/ or send a letter to diff --git a/source/references.rst b/source/references.rst index 6d09d3c0aaa7..5b777a4d032f 100644 --- a/source/references.rst +++ b/source/references.rst @@ -29,3 +29,9 @@ .. [UEFI] `Unified Extensable Firmware Interface Specification v2.8 Errata A https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_A_Feb14.pdf`_, February 2020, `UEFI Forum http://www.uefi.org`_

+.. [RVPLTSPEC] `RISC-V Platform specification https://github.com/riscv/riscv-platform-specs`_

+.. [RVSBISPEC] `RISC-V Supervisor Binary Interface specification https://github.com/riscv/riscv-sbi-doc`_

+.. [RVHYPSPEC] `RISC-V ISA Hypervisor extension https://github.com/riscv/riscv-isa-manual/blob/master/src/hypervisor.tex`_

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.