diff mbox series

[1/2] efi: Add dependency on M-mode for RISC-V

Message ID 20200928160837.457324-1-seanga2@gmail.com
State Rejected, archived
Delegated to: Heinrich Schuchardt
Headers show
Series [1/2] efi: Add dependency on M-mode for RISC-V | expand

Commit Message

Sean Anderson Sept. 28, 2020, 4:08 p.m. UTC 
From section 2.3.7 of the UEFI specification:

> RISC-V UEFI will only be executed in machine mode. The machine mode has
> the highest privilege and this mode is the only mandatory privilege level
> for RISC-V platforms; all other privilege levels are optional depending
> on the platform requirements. Machine mode is the first mode entered at
> the power-on reset. This level is used in UEFI for low-level access to a
> hardware platform.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 lib/efi_loader/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

Comments

Heinrich Schuchardt Sept. 28, 2020, 4:27 p.m. UTC | #1 
On 28.09.20 18:08, Sean Anderson wrote:
>>From section 2.3.7 of the UEFI specification:
>
>> RISC-V UEFI will only be executed in machine mode. The machine mode has
>> the highest privilege and this mode is the only mandatory privilege level
>> for RISC-V platforms; all other privilege levels are optional depending
>> on the platform requirements. Machine mode is the first mode entered at
>> the power-on reset. This level is used in UEFI for low-level access to a
>> hardware platform.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  lib/efi_loader/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index bad1a29ba8..2197c84bf3 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -10,6 +10,8 @@ config EFI_LOADER
>  	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
>  	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
>  	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
> +	# RISC-V UEFI must run in machine mode
> +	depends on !RISCV || RISCV_M_MODE
>  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
>  	select LIB_UUID
>  	select HAVE_BLOCK_DEVICE
>

Hello Sean,

the specification is wrong. A change request has been made to correct
the specification. We should not change U-Boot here.

See the discussion in this thread:
https://lists.linaro.org/pipermail/boot-architecture/2020-September/001481.html

Best regards

Heinrich
Sean Anderson Sept. 28, 2020, 5:15 p.m. UTC | #2 
On 9/28/20 12:27 PM, Heinrich Schuchardt wrote:
> On 28.09.20 18:08, Sean Anderson wrote:
>> >From section 2.3.7 of the UEFI specification:
>>
>>> RISC-V UEFI will only be executed in machine mode. The machine mode has
>>> the highest privilege and this mode is the only mandatory privilege level
>>> for RISC-V platforms; all other privilege levels are optional depending
>>> on the platform requirements. Machine mode is the first mode entered at
>>> the power-on reset. This level is used in UEFI for low-level access to a
>>> hardware platform.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>  lib/efi_loader/Kconfig | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index bad1a29ba8..2197c84bf3 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -10,6 +10,8 @@ config EFI_LOADER
>>  	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
>>  	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
>>  	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
>> +	# RISC-V UEFI must run in machine mode
>> +	depends on !RISCV || RISCV_M_MODE
>>  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
>>  	select LIB_UUID
>>  	select HAVE_BLOCK_DEVICE
>>
> 
> Hello Sean,
> 
> the specification is wrong. A change request has been made to correct
> the specification. We should not change U-Boot here.
> 
> See the discussion in this thread:
> https://lists.linaro.org/pipermail/boot-architecture/2020-September/001481.html
> 
> Best regards
> 
> Heinrich
> 

Thanks for linking that. I'm glad some of these issues are being addressed. However, I am concerned about your suggestion

> * Don't trust the values of tp and gp when entered
>   from the payloads world.

Because tp *must* be trusted in S-Mode...

Of course, all this could be averted by adding an sbi_get_mhartid function...

--Sean
Heinrich Schuchardt Sept. 28, 2020, 6:24 p.m. UTC | #3 
On 28.09.20 19:15, Sean Anderson wrote:
> On 9/28/20 12:27 PM, Heinrich Schuchardt wrote:
>> On 28.09.20 18:08, Sean Anderson wrote:
>>> >From section 2.3.7 of the UEFI specification:
>>>
>>>> RISC-V UEFI will only be executed in machine mode. The machine mode has
>>>> the highest privilege and this mode is the only mandatory privilege level
>>>> for RISC-V platforms; all other privilege levels are optional depending
>>>> on the platform requirements. Machine mode is the first mode entered at
>>>> the power-on reset. This level is used in UEFI for low-level access to a
>>>> hardware platform.
>>>
>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>> ---
>>>
>>>  lib/efi_loader/Kconfig | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>> index bad1a29ba8..2197c84bf3 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -10,6 +10,8 @@ config EFI_LOADER
>>>  	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
>>>  	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
>>>  	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
>>> +	# RISC-V UEFI must run in machine mode
>>> +	depends on !RISCV || RISCV_M_MODE
>>>  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
>>>  	select LIB_UUID
>>>  	select HAVE_BLOCK_DEVICE
>>>
>>
>> Hello Sean,
>>
>> the specification is wrong. A change request has been made to correct
>> the specification. We should not change U-Boot here.
>>
>> See the discussion in this thread:
>> https://lists.linaro.org/pipermail/boot-architecture/2020-September/001481.html
>>
>> Best regards
>>
>> Heinrich
>>
>
> Thanks for linking that. I'm glad some of these issues are being addressed. However, I am concerned about your suggestion
>
>> * Don't trust the values of tp and gp when entered
>>   from the payloads world.
>
> Because tp *must* be trusted in S-Mode...
>
> Of course, all this could be averted by adding an sbi_get_mhartid function...

I thought only the secondary hart uses tp in U-Boot.

Best regards

Heinrich
Sean Anderson Sept. 28, 2020, 6:29 p.m. UTC | #4 
On 9/28/20 2:24 PM, Heinrich Schuchardt wrote:
> On 28.09.20 19:15, Sean Anderson wrote:
>> On 9/28/20 12:27 PM, Heinrich Schuchardt wrote:
>>> On 28.09.20 18:08, Sean Anderson wrote:
>>>> >From section 2.3.7 of the UEFI specification:
>>>>
>>>>> RISC-V UEFI will only be executed in machine mode. The machine mode has
>>>>> the highest privilege and this mode is the only mandatory privilege level
>>>>> for RISC-V platforms; all other privilege levels are optional depending
>>>>> on the platform requirements. Machine mode is the first mode entered at
>>>>> the power-on reset. This level is used in UEFI for low-level access to a
>>>>> hardware platform.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>> ---
>>>>
>>>>  lib/efi_loader/Kconfig | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>>> index bad1a29ba8..2197c84bf3 100644
>>>> --- a/lib/efi_loader/Kconfig
>>>> +++ b/lib/efi_loader/Kconfig
>>>> @@ -10,6 +10,8 @@ config EFI_LOADER
>>>>  	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
>>>>  	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
>>>>  	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
>>>> +	# RISC-V UEFI must run in machine mode
>>>> +	depends on !RISCV || RISCV_M_MODE
>>>>  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
>>>>  	select LIB_UUID
>>>>  	select HAVE_BLOCK_DEVICE
>>>>
>>>
>>> Hello Sean,
>>>
>>> the specification is wrong. A change request has been made to correct
>>> the specification. We should not change U-Boot here.
>>>
>>> See the discussion in this thread:
>>> https://lists.linaro.org/pipermail/boot-architecture/2020-September/001481.html
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>
>> Thanks for linking that. I'm glad some of these issues are being addressed. However, I am concerned about your suggestion
>>
>>> * Don't trust the values of tp and gp when entered
>>>   from the payloads world.
>>
>> Because tp *must* be trusted in S-Mode...
>>
>> Of course, all this could be averted by adding an sbi_get_mhartid function...
> 
> I thought only the secondary hart uses tp in U-Boot.
> 
> Best regards
> 
> Heinrich
> 

So UEFI is only ever ran on a single hart? I thought one of the
advantages of UEFI was that it could initialize devices in parallel.

--Sean
diff mbox series

Patch

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index bad1a29ba8..2197c84bf3 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -10,6 +10,8 @@  config EFI_LOADER
 	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
 	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
 	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
+	# RISC-V UEFI must run in machine mode
+	depends on !RISCV || RISCV_M_MODE
 	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
 	select LIB_UUID
 	select HAVE_BLOCK_DEVICE