On 9/25/20 8:06 AM, Chang, Abner (HPS SW/FW Technologist) wrote:
> 
> 
>> -----Original Message-----
>> From: Heinrich Schuchardt [mailto:[email protected]]
>> Sent: Thursday, September 24, 2020 8:43 PM
>> To: Ard Biesheuvel <[email protected]>
>> Cc: Chang, Abner (HPS SW/FW Technologist) <[email protected]>;
>> [email protected]; Atish Patra <[email protected]>; Rick
>> Chen <[email protected]>; Atish Patra <[email protected]>; Grant
>> Likely <[email protected]>
>> Subject: Re: EBBR: RISC-V handoff to OS
>>
>> On 24.09.20 14:05, Ard Biesheuvel wrote:
>>> On Thu, 24 Sep 2020 at 08:07, Heinrich Schuchardt <[email protected]>
>> wrote:
>>>>
>>>> On 9/24/20 5:35 AM, Chang, Abner (HPS SW/FW Technologist) wrote:
>>>>> Ok Heinrich, thanks for the clarification. I also read through this thread
>> again and understand the intention you would like to have some sentences
>> in the UEFI spec.
>>>>>
>>>>> I realize that we are trying to put sentences in the RISC-V calling
>> convention in UEFI spec to support the specific UEFI firmware
>> implementation of using gp/tp.
>>>>> Uboot UEFI firmware uses tp in HART to record the HART ID and uses gp
>> in the boot-HART to maintain the global data structure of uboot.
>>>>
>>>> tp is only used to store information on the secondary harts which
>>>> does not enter the UEFI payload. So for U-Boot the only problematic
>>>> register is gp.
> If you are using opensbi solution,  you should consider to use firmware 
> context mechanism because that is designed for any firmware solutions to 
> maintain its own data structure for any purposes.

I will have a look at it. Thanks for the link below.

> 
>>>>
>>>
>>> Secondary CPUs could be used to invoke runtime services, though.
>>>
>>>>> The reason I can think of the reason to use tp/gp is because S-Mode
>> UEFI firmware (or S-mode payload?) is not able to get mhartid, so uboot
>> stores HART ID in tp. Because there is nowhere to maintain the global data
>> structure for uboot and the mscratch is occupied by opensbi, thus uboot uses
>> gp to maintain the pointer to global context.
>>>>> Because both tp and gp are non-privileged registers so uboot (or
>>>>> payload?) can get the information from those registers easily,
>>>>> right? (But is this a security hole that any software can hack
>>>>> tp/gp? However, this is the different topic, just ignore this)
>>>>
>>>> As said U-Boot restores value of gp when entered from UEFI via the API.
>>>>
>>>> We should do the same when serving interrupts and exceptions.
>>>>
>>>
>>> When?
>>>
>>> We should carefully consider the difference between before-EBS and
>>> after-EBS here:
>>>
>>> Before EBS:
>>> Interrupt/exception handlers are owned by the UEFI firmware (Uboot or
>>> EDK2). The OS or any other EFI payload must not interfere with this,
>>> and only use function call interfaces to interact with the system.
>>
>> There is a gap in U-Boot for interrupts/exceptions before
>> ExitBootServices() that I will have to close.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> After EBS:
>>> Interrupt/exception handlers are owned by the OS. The UEFI firmware
>>> can be entered on any CPU with interrupts enabled or disabled, and the
>>> UEFI firmware should not interfere with interrupt handling or
>>> exception vectors etc.
>>>
>>>
>>>
>>>> Concerning security we have to remember that an UEFI payload has the
>>>> same powers as the firmware itself. When it comes to hacking you have
>>>> lost once you execute a hostile payload. What we can try to guard
>>>> against to some degree are non-compliant payloads.
>>>>
>>>
>>> Agreed. Everything runs at the same permission level, so any software
>>> running under UEFI has the same powers as the UEFI firmware itself.
>>>
>>>>> Did we spec out above behavior in uboot RISC-V spec or the
>> implementation design guide at somewhere? I am not quite familiar with
>> uboot area though.
>>>>
>>>> The patch series
>>>>
>>>> doc: global data pointer
>>>> INVALID URI REMOVED
>> 3A__patchwork.ozlabs
>>>> .org_project_uboot_list_-3Fseries-
>> 3D202971&d=DwIFaQ&c=C5b8zRQO1miGmBe
>>>>
>> VZ2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=QMu
>> q0JhboDWbE
>>>> qbx6d4dZeXY2Pk0-PICJzRJKNT4wRY&s=UjTczoF1vjt5-
>> uUzkvB6QFgGIsCUpuTMrSjX
>>>> Zh8LbGA&e=
>>>>
>>>> starts closing the documentation gap in U-Boot.
>>>>
>>>>>
>>>>> Another problem comes to my mind is even though we save gp/tp upon
>> the entry of ExitBootService (or other UEFI APIs) and replaced with the
>> uboot ones, what if the interrupt/exception happens and delegated to the
>> payload event handler during ExitBootService (or other UEFI APIs)? The
>> ownership is messed up at this point, right?
>>>>
>>>> UEFI spec 2.8B chapter 2.3.7 "RISC-V Platforms" has:
>>>>
>>>> The processor is in the following execution mode during boot service:
>>>> The machine mode interrupt is enabled during boot service in UEFI.
>>>> Two kinds of interrupts are enabled, one is for timer interrupt and
>>>> another is software interrupt
>>>>
>>>> All other architectures except RISC-V describe the usage of runtime
>>>> services by the OS with the following sentences:
>>>>
>>>> For an operating system to use any runtime services, it must: Call
>>>> the runtime service functions, with the following conditions:
>>>> Interrupts may be disabled or enabled at the discretion of the caller.
>>>>
>>>> So the rule of thumb is: Until ExitBootServices() returns the
>>>> firmware serves interrupts and exceptions. After ExitBootServices()
>>>> returns it is the operating system.
>>>>
>>>> In the AAarch64 chapter I found:
>>>>
>>>> "An alternate execution environment can restore the firmware
>>>> environment before each UEFI call. However the possibility of
>>>> preemption may require the alternate execution-enabled application to
>>>> disable interrupts while the alternate execution environment is
>>>> active. It's legal for the alternate execution environment enabled
>>>> application to enable interrupts if the application catches the
>>>> interrupt and restores the EFI firmware environment prior to calling the
>> UEFI interrupt ISR."
>>>>
>>>> I am not aware of an implementation of such an "alternate execution
>>>> environment". But an alternative execution environment serving
>>>> interrupts while calling the UEFI API and relying on values that it
>>>> has put into gp would not work on current U-Boot.
>>>>
>>>>>
>>>>> In edk2 RISC-V port, we use "firmware_context" in sbi_platforms which
>> provided opensbi through mscratch to maintain the firmware specific context.
>> With this, firmware can maintain its own data structure without bothering
>> any other industrial specs. Because S-Mode firmware can't access to mhartid
>> and  the sbi-scratch which is stored in mscratch, so we have Edk2OpensbiLib
>> that registers the firmware specific sbi extension functions using "Firmware
>> Code Base Specific SBI Extension Space, Extension Ids 0x0A000000 through
>> 0x0AFFFFFF" defined in sbi spec. That is a separate spec for RISC-V edk2
>> firmware sbi extension function (This reminds me I forget to document it).
>> Then edk2 can get the necessary information using edk2 extension sbi call.
>>>>>
>>>>
>>>> Thank you for pointing me at Edk2OpensbiLib. Where can I find the code?
> Here it is,
> https://github.com/tianocore/edk2-platforms/tree/master/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib
> 
>>>>
>>>> I will have to keep in mind that U-Boot can be used both with and
>>>> without an SEE.
>>>>
>>>>> You would agree with that we don’t have to mention the gp/tp usage in
>> the calling convention just because of the uboot implementation. And we
>> don’t have  mention any connection/relationship between  gp/tp usage and
>> the specific UEFI firmware implementation. The sentence must be also
>> generic to other UEFI firmware solutions such as edk2 which doesn't care
>> about destroying gp/tp (so far).
>>>>
>>>> I fully agree that specifications precede implementations.
>>>>
>>>>>
>>>>> In the RISC-V assembly programmer's handbook in RISC-V spec, gp/tp
>> are not given either the caller or callee to save the value, so we can spec 
>> out
>> something in UEFI spec based on this calling convention.
>>>>
>>>> Jessica Clarke pointed me to the "RISC-V ELF psABI specification"
>>>> (https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.m
>>>> d) which defines the usage of tp and gp in the RISC-V world:
>>>>
>>>> "In the standard ABI, procedures should not modify the integer
>>>> registers tp and gp, because signal handlers may rely upon their values."
>>>>
>>>> So here the idea is that whoever serves interrupts and exceptions is
>>>> the owner of gp and tp.
>>>>
>>>> After ExitBootServices() signals are served by the operating system.
>>>> It takes hold of the values of tp and gp. See these lines of the Linux 
>>>> kernel:
>>>>
>>>> arch/riscv/kernel/entry.S:97:
>>>> la gp, __global_pointer$
>>>>
>>>> arch/riscv/kernel/head.S:253:
>>>> la tp, init_task
>>>>
>>>> Every time Linux enters kernel mode it again sets its own values of
>>>> gp and tp. This addresses the security problem that you mentioned above.
>>>>
>>>>>
>>>>> The sentence which I consider reasonable is , "In view of gp and tp
>>>>> registers are not assigned the specific owner to save and restore their
>> values in the calling convention (refer to "RISC-V assembly programmer's
>> handbook" section in RISC-V Unprivileged ISA specification), UEFI firmware
>> must not having assumption of owning the write access to these register at
>> any circumstances such as in EFI Boot service, EFI Runtime service, EFI
>> Management Mode service and in any UEFI firmware interfaces which may
>> invoked by the external firmware payload,  or altering the values of gp/tp
>> registers results in the interference in the asynchronous callback to the
>> external firmware payload . Whether and how to preserve gp and tp during
>> UEFI firmware environment is UEFI firmware implementation-specific. RISC-
>> V platform firmware integrator should refer to the corresponding design
>> specification of the particular UEFI firmware solution."

Thanks for this description which looks fine to me. Maybe a native
English speaker can clean it up a bit.

Best regards

Heinrich

>>>>>
>>>>
>>>> We should start with a reference of the "RISC-V ELF psABI specification"
>>>> to explain the usage of gp and tp.
>>>>
>>>> It seems to be necessary and sufficient for the firmware to follow:
>>>>
>>>> * Don't trust the values of tp and gp when entered
>>>>   from the payloads world.
>>>> * If you need to change them, restore the values before returning.
>>>> * Never touch tp and gp after ExitBootServices().
>>>>
>>>> The same should be observed by an interrupt or exception handler
>>>> provided by an "alternative execution environment".
>>>>
>>>> Best regards
> How about this,
>  
> In view of the statement "In the standard ABI, procedures should not modify 
> the integer registers tp and gp, because signal handlers may rely upon their 
> values." mentioned in RISC-V EFL psABI Specification (See “Link to UEFI 
> Specification-Related Document” on https://uefi.org/uefi under the heading 
> “RISC-V EFL psABI Specification”) and the RISC-V calling convention that gp 
> and tp registers are not assigned the specific owner to save and restore 
> their values (refer to "RISC-V assembly programmer's handbook"  section in  
> RISC-V Unprivileged ISA specification).  UEFI firmware must neither trusting 
> the values of tp and gp nor having assumption of owning the write access to 
> these register at any circumstances (Such as in EFI Boot service, EFI Runtime 
> service, EFI Management Mode service and any UEFI firmware interfaces which 
> may invoked by the EFI drivers, OS or external firmware payload). Preserve 
> the values in gp or tp register if UEFI firmware needs to change them and 
> never touches them after ExitBootServices(). Whether and how to preserve gp 
> and tp during UEFI firmware environment is UEFI firmware 
> implementation-specific. 
> 
> 
>>>>
>>>> Heinrich
>>>>
>>>>> I believe this initial sentence still needs some revises, please just 
>>>>> modify
>> above paragraph if you don't like it. We can keep discussing this  based on
>> above paragraph.
>>>>>
>>>>> Thanks
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Heinrich Schuchardt [mailto:[email protected]]
>>>>>> Sent: Thursday, September 24, 2020 2:20 AM
>>>>>> 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 5:01 PM, Chang, Abner (HPS SW/FW Technologist) wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Chang, Abner (HPS SW/FW Technologist)
>>>>>>>> Sent: Wednesday, September 23, 2020 10:55 PM
>>>>>>>> To: 'Heinrich Schuchardt' <[email protected]>; boot-
>>>>>>>> [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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Heinrich Schuchardt [mailto:[email protected]]
>>>>>>>>> Sent: Wednesday, September 23, 2020 3:42 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 23.09.20 08:51, Chang, Abner (HPS SW/FW Technologist) 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/ri
>>>>>>>>>>>>>> scv-
>>>>>>>>>>>>>> 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 refer to the firmware providing the UEFI API. U-Boot and EDK II
>>>>>> are alternative implementations.
>>>>>>
>>>>>>>> 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
>>>>>>
>>>>>> Both U-Boot and EDK II may be started by an SEE, e.g. OpenSBI.
>>>>>>
>>>>>>>> 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
>>>>>>
>>>>>> Everytime U-Boot code is entered during boot services the payloads
>>>>>> gp is saved. When returning to the payload the payloads value of
>>>>>> the gp is restored. So the payload cannot observe any change in gp.
>>>>>> At runtime U- Boot does not touch gp.
>>>>>>
>>>>>> I hope that is in line with your suggestion.
>>>>>>
>>>>>> Best regards
>>>>>>
>>>>>> Heinrich
>>>>>>
>>>>>>>> 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
> [email protected]
> https://lists.linaro.org/mailman/listinfo/boot-architecture
> 

_______________________________________________
boot-architecture mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/boot-architecture

Reply via email to