> -----Original Message-----
> From: Chang, Abner (HPS SW/FW Technologist)
> Sent: Wednesday, September 23, 2020 10:55 PM
> To: 'Heinrich Schuchardt' <xypron.g...@gmx.de>; boot-
> architect...@lists.linaro.org; Atish Patra <ati...@atishpatra.org>
> Cc: Rick Chen <r...@andestech.com>; Atish Patra <atish.pa...@wdc.com>;
> Grant Likely <grant.lik...@arm.com>; Ard Biesheuvel <a...@kernel.org>
> Subject: RE: EBBR: RISC-V handoff to OS
> 
> 
> 
> > -----Original Message-----
> > From: Heinrich Schuchardt [mailto:xypron.g...@gmx.de]
> > Sent: Wednesday, September 23, 2020 3:42 PM
> > To: Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com>;
> > boot-architecture@lists.linaro.org; Atish Patra
> > <ati...@atishpatra.org>
> > Cc: Rick Chen <r...@andestech.com>; Atish Patra <atish.pa...@wdc.com>;
> > Grant Likely <grant.lik...@arm.com>; Ard Biesheuvel <a...@kernel.org>
> > Subject: Re: EBBR: RISC-V handoff to OS
> >
> > On 23.09.20 08:51, Chang, Abner (HPS SW/FW Technologist) wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Heinrich Schuchardt [mailto:xypron.g...@gmx.de]
> > >> Sent: Wednesday, September 23, 2020 2:18 PM
> > >> To: Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com>;
> > >> boot-architecture@lists.linaro.org; Atish Patra
> > >> <ati...@atishpatra.org>
> > >> Cc: Rick Chen <r...@andestech.com>; Atish Patra
> > >> <atish.pa...@wdc.com>; Grant Likely <grant.lik...@arm.com>; Ard
> > >> Biesheuvel <a...@kernel.org>
> > >> 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:xypron.g...@gmx.de]
> > >>>> Sent: Tuesday, September 22, 2020 5:30 PM
> > >>>> To: boot-architecture@lists.linaro.org; Chang, Abner (HPS SW/FW
> > >>>> Technologist) <abner.ch...@hpe.com>; Atish Patra
> > >>>> <ati...@atishpatra.org>
> > >>>> Cc: boot-architecture@lists.linaro.org; Rick Chen
> > >>>> <r...@andestech.com>; Atish Patra <atish.pa...@wdc.com>; Grant
> > >>>> Likely <grant.lik...@arm.com>; Ard Biesheuvel <a...@kernel.org>
> > >>>> Subject: RE: EBBR: RISC-V handoff to OS
> > >>>>
> > >>>> Am 22. September 2020 08:30:57 MESZ schrieb "Chang, Abner (HPS
> > >> SW/FW
> > >>>> Technologist)" <abner.ch...@hpe.com>:
> > >>>>>
> > >>>>>
> > >>>>>> -----Original Message-----
> > >>>>>> From: Atish Patra [mailto:ati...@atishpatra.org]
> > >>>>>> Sent: Tuesday, September 22, 2020 2:08 PM
> > >>>>>> To: Chang, Abner (HPS SW/FW Technologist)
> > <abner.ch...@hpe.com>
> > >>>>>> Cc: Heinrich Schuchardt <xypron.g...@gmx.de>; Atish Patra
> > >>>>>> <atish.pa...@wdc.com>; boot-architecture@lists.linaro.org;
> > >>>>>> Grant
> > >>>>> Likely
> > >>>>>> <grant.lik...@arm.com>; Ard Biesheuvel <a...@kernel.org>; Rick
> > >> Chen
> > >>>>>> <r...@andestech.com>
> > >>>>>> Subject: Re: EBBR: RISC-V handoff to OS
> > >>>>>>
> > >>>>>> On Mon, Sep 21, 2020 at 6:11 PM Chang, Abner (HPS SW/FW
> > >>>>>> Technologist) <abner.ch...@hpe.com> wrote:
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>> -----Original Message-----
> > >>>>>>>> From: Atish Patra [mailto:ati...@atishpatra.org]
> > >>>>>>>> Sent: Tuesday, September 22, 2020 2:28 AM
> > >>>>>>>> To: Heinrich Schuchardt <xypron.g...@gmx.de>
> > >>>>>>>> Cc: Atish Patra <atish.pa...@wdc.com>;
> > >>>>>>>> boot-architecture@lists.linaro.org;
> > >>>>>>>> Grant Likely <grant.lik...@arm.com>; Ard Biesheuvel
> > >>>>>>>> <a...@kernel.org>; Rick Chen <r...@andestech.com>; Chang,
> > >> Abner
> > >>>>>> (HPS
> > >>>>>>>> SW/FW Technologist) <abner.ch...@hpe.com>
> > >>>>>>>> Subject: Re: EBBR: RISC-V handoff to OS
> > >>>>>>>>
> > >>>>>>>> On Mon, Sep 21, 2020 at 9:23 AM Heinrich Schuchardt
> > >>>>>>>> <xypron.g...@gmx.de> 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-e
> > >>>>> lf
> > >>>>>>>>> .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.
> >
> > My references to U-Boot where not meant for inclusion in any spec but
> > as background information only.
> >
> > >
> > > "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."
> >
> > Please, have a look at RFC 2119 "Key words for use in RFCs to Indicate
> > Requirement Levels" [INVALID URI REMOVED
> >
> 3A__tools.ietf.org_html_rfc2119&d=DwIFaQ&c=C5b8zRQO1miGmBeVZ2LFW
> >
> g&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=M_APpMPr8z
> >
> v5FK0ys5Noq10akQQqL0eNc9vlZqmOkpw&s=DBCkr05bpuKrQWdyNn5bJSow
> > sLV7UDvDJIJc3nT4jd0&e= ]. Here "should"
> > has the meaning of "recommended" and may be ignored "in particular
> > circumstances".
> >
> > Did you mean SHOULD or MUST?
> MUST is better, to make UEFI firmware solution simple. UEFI edk2
> implementation could be a stand along firmware or to be built as the payload
> (we don’t have this implementation yet).
> >
> > "are unallocatable" is vague. It does not describe who is not allowed
> > to change the values. Is it the firmware or is it the payload?
> From UEFI perspective, UEFI firmware payload is not allowed to change the
> value. I will rephrase this sentence.
> >
> > "keep the values intact" is vague. It could mean that the firmware
> > SHOULD reset gp and tp to the values they had had before StartImage()
> > whenever an API call is made.
> >
> > We need a definition that cannot be misunderstood. In my definition it
> > is at least clear who is the owner of tp and gp.
> >
> > Please, consider that before ExitBootServices() many different
> > payloads may be loaded by the firmware: applications, boot service
> > drivers, runtime service drivers. This is why to me it does not make
> > sense to allow any other software but the firmware to define the values of
> tp and gp before ExitBootServices().
> 
> I am not against that  firmware to define the values of tp and gp. I think the
> "firmware" you mentioned here refer to uboot but not edk2 firmware.
> I am also fine to mention UEFI firmware should not (Must not) use gp and tp
> in spec. I would like to restrict using these two registers in UEFI firmware 
> no
> matter the RISC-V UEFI firmware implementation is a stand along system
> firmware or a payload for other firmware such as uboot/opensbi, this would
> be easier for us to maintain RISC-V edk2 port. Hope I don’t misunderstand
> the intention which you brought up here, which is the UEFI firmware payload
> must not altering the value of gp and tp, right?
> 
> However, I don’t want to see that we list the specific functions such as
> ExitBootServices, startImage, SetVirtualAddress in the UEFI spec, we can just
> give the strict criteria to UEFI firmware to not using gp/tp.
> Says UEFI firmware can't alter tp/gp values,
> -  After firmware enters UEFI firmware payload and before returning to
> firmware
> - In BS
> - In RTS
> - In SMM service
> These should cover all possibilities of gp/tp value changed in  UEFI firmware.
> We don’t even have to define the ownership of these registers in UEFI spec.
BTW, above is based on the edk2 UEFI implementation. I am not sure if other 
UEFI firmware implementation can avoid to use these registers.
> 
> >
> > 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.
> > >>>>>>>>>

_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture

Reply via email to