On Mon, Jun 21, 2021 at 10:19 AM Grant Likely <grant.lik...@arm.com> wrote: > > > > 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.pa...@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. >
Thanks!! > > --- 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. :-) > Sure. > > 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. > Sure. > > + > > +.. 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. > Ok. Sure. I added them after Henerich suggested. > 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. > Sure. > > 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. > That's the copyright line we got from Western Digital lawyer because Western Digital has many subsidiaries (as a result of acquisitions) In fact, our group was part of HGST until recently. We have the same copyright in other open source projects (Kernel, OpenSBI, U-Boot) If you feel strongly about it, you can drop it. We can fix it in the future if somebody complains about it :) > 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. > _______________________________________________ > boot-architecture mailing list > boot-architecture@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/boot-architecture -- Regards, Atish _______________________________________________ boot-architecture mailing list boot-architecture@lists.linaro.org https://lists.linaro.org/mailman/listinfo/boot-architecture
