On Tue, Sep 22, 2020 at 11:51 PM Chang, Abner (HPS SW/FW Technologist) <[email protected]> wrote: > > > > > -----Original Message----- > > From: Heinrich Schuchardt [mailto:[email protected]] > > Sent: Wednesday, September 23, 2020 2:18 PM > > To: Chang, Abner (HPS SW/FW Technologist) <[email protected]>; > > [email protected]; Atish Patra <[email protected]> > > Cc: Rick Chen <[email protected]>; Atish Patra <[email protected]>; > > Grant Likely <[email protected]>; Ard Biesheuvel <[email protected]> > > Subject: Re: EBBR: RISC-V handoff to OS > > > > On 9/23/20 7:24 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > > > > > > >> -----Original Message----- > > >> From: Heinrich Schuchardt [mailto:[email protected]] > > >> Sent: Tuesday, September 22, 2020 5:30 PM > > >> To: [email protected]; Chang, Abner (HPS SW/FW > > >> Technologist) <[email protected]>; Atish Patra > > >> <[email protected]> > > >> Cc: [email protected]; Rick Chen > > >> <[email protected]>; Atish Patra <[email protected]>; Grant Likely > > >> <[email protected]>; Ard Biesheuvel <[email protected]> > > >> Subject: RE: EBBR: RISC-V handoff to OS > > >> > > >> Am 22. September 2020 08:30:57 MESZ schrieb "Chang, Abner (HPS > > SW/FW > > >> Technologist)" <[email protected]>: > > >>> > > >>> > > >>>> -----Original Message----- > > >>>> From: Atish Patra [mailto:[email protected]] > > >>>> Sent: Tuesday, September 22, 2020 2:08 PM > > >>>> To: Chang, Abner (HPS SW/FW Technologist) <[email protected]> > > >>>> Cc: Heinrich Schuchardt <[email protected]>; Atish Patra > > >>>> <[email protected]>; [email protected]; Grant > > >>> Likely > > >>>> <[email protected]>; Ard Biesheuvel <[email protected]>; Rick > > Chen > > >>>> <[email protected]> > > >>>> Subject: Re: EBBR: RISC-V handoff to OS > > >>>> > > >>>> On Mon, Sep 21, 2020 at 6:11 PM Chang, Abner (HPS SW/FW > > >>>> Technologist) <[email protected]> wrote: > > >>>>> > > >>>>> > > >>>>> > > >>>>>> -----Original Message----- > > >>>>>> From: Atish Patra [mailto:[email protected]] > > >>>>>> Sent: Tuesday, September 22, 2020 2:28 AM > > >>>>>> To: Heinrich Schuchardt <[email protected]> > > >>>>>> Cc: Atish Patra <[email protected]>; > > >>>>>> [email protected]; > > >>>>>> Grant Likely <[email protected]>; Ard Biesheuvel > > >>>>>> <[email protected]>; Rick Chen <[email protected]>; Chang, > > Abner > > >>>> (HPS > > >>>>>> SW/FW Technologist) <[email protected]> > > >>>>>> Subject: Re: EBBR: RISC-V handoff to OS > > >>>>>> > > >>>>>> On Mon, Sep 21, 2020 at 9:23 AM Heinrich Schuchardt > > >>>>>> <[email protected]> wrote: > > >>>>>>> > > >>>>>>> Hello Atish, > > >>>>>>> > > >>>>>>> the UEFI spec has this sentence: > > >>>>>>> > > >>>>>>> "When UEFI firmware handoff control to OS, the RISC-V is > > >>> operated > > >>>>>>> in machine-mode privilege." (M-mode is the equivalent to EL3 in > > >>> ARM). > > >>>>> Yes, this is a pretty old section in UEFI spec even before OpenSBI > > >>> as I can > > >>>> remember and haven't been updated to sync with latest status RISC-V > > >>> works. > > >>>>> > > >>>>>>> > > >>>>>>> This does not make any sense to me when using a secure > > >>> execution > > >>>>>>> environement (SEE) like OpenSBI. > > >>>>>>> > > >>>>>>> The hand-off should occur in S-Mode if the CPU supports it and > > >>>>>>> only in M-Mode when the CPU only supports M-mode. > > >>>>>>> > > >>>>>> +Abner > > >>>>>> > > >>>>>> Yes. Abner has already submitted an ECR for this & few other > > >>> RISC-V > > >>>>>> related changes to UEFI spec. I am not sure about the current > > >>> status > > >>>> though. > > >>>>>> > > >>>>>> @Abner: Do you know the latest status ? > > >>>>> Yes, the ECR was submitted to USWG for review, however the > > meeting > > >>>>> canceled often recently and the process goes slow. I will keep > > >>>>> following up > > >>>>> > > >>>>>> Maybe you also attach the latest ECR here for a broader review. > > >>>>> I may not allowed to public ECR here unless all people in the > > >>> mailing > > >>>>> list are the members of UEFI org… but the sentence we revised in > > >>> ECR > > >>>>> is as below, > > >>>>> > > >>>>> "When UEFI firmware handoff control to Supervisor mode OS, RISC-V > > >>> boot > > >>>> hart must be operated in Supervisor mode, and the memory > > addressing > > >>>> must be operated in Bare mode which is no memory address > > >>>> translation > > >>> or > > >>>> protection through the virtual page table entry." > > >>>>> > > >>>> > > >>>> Thanks. > > >>>> > > >>>>> We didn’t mention hand-off to M-Mode if CPU only support M-Mode > > >>>> because we only verified edk2 RISC-V port in S-Mode with OpenSBI, > > >>>> but didn’t try it on M-Mode even though the code is ready there. > > >>>>> > > >>>> > > >>>> That's correct. We can't run UEFI applications run time services in > > >>> M-mode as > > >>>> it requires virtual memory in current setup. > > >>>> I think it is better to keep that way there is a specific demand > > >>>> and > > >>> value in > > >>>> running UEFI applications in M-mode. > > >>>> > > >>>>>> > > >>>>>>> We should prescribe this in the EBBR and somehow get the UEFI > > >>> spec > > >>>>>>> fixed afterwards. > > >>>>>>> > > >>>>>> > > >>>>>> I will add it to the RISC-V EBBR PR (haven't sent it yet). > > >>>>>> > > >>>>>>> An other issue is the calling convention. Chapter "2.3.7.3 > > >>>>>>> Detailed Calling Convention" does not describe which registers > > >>> are > > >>>>>>> saved by the UEFI payload's entry point and restored by the > > >>>>>>> payload before calling the UEFI API or returning to the UEFI > > >>>>>>> payload. This concerns especially registers gp (x3) and tp > > >>> (x4). > > >>>>>>> > > >>>>>>> Into the EBBR or UEFI spec we should put a link to the "RISC-V > > >>> ELF > > >>>>>>> psABI specification" > > >>>>>>> > > >>> https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf > > >>>>>>> .md which is referenced by "The RISC-V Instruction Set Manual". > > >>>>>>> > > >>>>>>> From the "RISC-V ELF psABI specification" one might conclude > > >>> that > > >>>>>>> the UEFI payload should not be allowed to change gp and tp > > >>> before > > >>>>>>> calling > > >>>>>>> ExitBootServices() or SetVirtualAddressMap() whichever occurs > > >>> last. > > >>>>>>> > > >>>>>> > > >>>>>> Agreed. Thanks for pointing this out. I think this should go into > > >>>>>> the UEFI spec instead of EBBR spec. Any suggestions ? > > >>>>> To have this external link is good, I will add this link in ECR. > > >>>>>> > > >>>> > > >>>> Thanks again :). As per Heinrich's suggestion, it would also be > > >>>> good > > >>> to add > > >>>> some text about the state of gp & tp. > > >>> > > >>> Ok. I add some text, copied from psABI spec. :) > > >>> > > >>> "........ Registers tp and gp should not be modified in the > > >>> function, > > >> > > >> I would not know what "in the function" refers to here. > > >> > > >> We have a value of tp and gp passed from the firmware to the > > >> entry-point of the firmware. We need a definition describing what > > >> assumptions the firmware may or may not make about the value of tp > > >> and gp up to ExitBootServices and in SetVirtualAddressMap. I would > > >> not assume gp and tp to have any firmware related value afterwards. > > > Ok, I got your point. But do we mention "value of tp and gp passed from > > the firmware to the entry-point of the firmware" somewhere in the > > particular spec? What is the spec of values you pass in tp/gp to the entry > > point of firmware? > > > > > >> > > >> It would be perfectly ok if the spec said that the firmware should > > >> make no assumptions about the value of these registers > > > I don’t think firmware has assumption of value in these registers > > > because the calling convention doesn't use these two regs. Maybe the > > > assembly code use these two regs for some purpose but firmware has to > > > preserve it. The revised version in below, > > > > > >> but has to keep the values > > >> intact during an API call. > > >> > > >> The important thing is that we have a clear definition of the > > >> interface between the firmware and the payload. > > > > > > How about this, > > > "Registers tp and gp are unallocatable and not preserved across the > > function calls, UEFI firmware implementation of RISC-V platform should keep > > the values in these two registers intact during entire UEFI firmware boot > > process, UEFI boot service and runtime service. See “Link to UEFI > > Specification-Related Document” on https://uefi.org/uefi under the heading > > “RISC-V EFL psABI Specification” for the more descriptions of RISC-V calling > > convention." > > > > Current situation in U-Boot > > =========================== > > > > With U-Boot we currently have the following boot flows: > > > > U-Boot -> payload > > OpenSBI -> payload > > U-Boot SPL -> OpenSBI -> U-Boot -> payload > > > > Upon entry U-Boot overwrites tp with the HART id on all HARTs and the boot > > HART's gp with a pointer to U-Boot's global data structure. > > > > At boot time when the payload makes an API call U-Boot replaces the > > payload's gp by its own value upon entry and restores the payload's gp > > before returning to the payload. > > > > At runtime UBoot neither touches gp nor tp. > > > > Interrupt handling > > ================== > > > > Before ExitBootServices() handling of interrupts and exceptions is the task > > of > > the firmware. As both may rely on the values of tp and gp the payload should > > not overwrite them. > > > > When the operating system takes over it also has gets the responsibility for > > handling interrupts and exceptions and will set its own values of tp and gp. > > > > Some operating systems call SetVirtualAddressMap() after ExitBootServices(), > > others don't. On Linux this is controlled by kernel command line parameter > > efi=novamap. > > > > Conclusion > > ========== > > > > I would suggest the following rules: > > > > Until ExitBootServices() returns to the payload the firmware is the owner of > > registers gp and tp. The payload MUST NOT alter these values. > > > > This implies that when handling EVT_SIGNAL_EXIT_BOOT_SERVICES events > > the payload MUST NOT alter gp and tp. > > > > After ExitBootServices() the payload is the owner of registers gp and tp. > > The > > firmware MUST NOT alter these values. > > The firmware solution you mentioned above is the payload solution with either > uboot or opensbi. However, edk2 firmware solution is not the same. RISC-V > edk2 firmware solution uses OpenSBI as the library. Thus we don't have to > mention specific firmware solution in UEFI spec. > I think the final sentence already covered all use cases you mentioned above, > that includes the EFI POST time and the UEFI service exposed to user such as > EFI boot service and EFI runtime service. We don’t have to mention specific > rule in the spec. I revised it again and add EFI Management Mode service. > > "Registers tp and gp are unallocatable and not preserved across the function > calls, UEFI firmware implementation of RISC-V platform should keep the values > in these two registers intact during entire UEFI firmware boot process, EFI > boot services, EFI runtime services and EFI management mode services. See > “Link to UEFI Specification-Related Document” on https://uefi.org/uefi under > the heading “RISC-V EFL psABI Specification” for the more descriptions of > RISC-V calling convention."
I agree with Henerich. Additional clarification is required for ownership of the gp & tp. UEFI firmware implementation should preserve the gp & tp at all times. But EFI payload shouldn't alter them during the UEFI firmware boot process, EFI boot services. > > > > > > Best regards > > > > Heinrich > > > > >> > > >>> because signal handlers may rely upon their values. See “Link to > > >>> UEFI Specification-Related Document” on https://uefi.org/uefi under > > >>> the heading “RISC-V EFL psABI Specification” for the more > > >>> descriptions of RISC-V calling convention." > > >>> > > >>> > > >>> I will upload this update to USWG once Heinrich agrees with above. > > >>> > > >>>> > > >>>>>>> Due to this missing clarification U-Boot is currently saving gp > > >>>>>>> before calling the entry point of the payload and restores it > > >>> on > > >>>>>>> reentry or on entry of an API call. Nothing is done for tp. > > >>>>>>> -- Regards, Atish _______________________________________________ boot-architecture mailing list [email protected] https://lists.linaro.org/mailman/listinfo/boot-architecture
