RISCV_EFI_BOOT_PROTOCOL support in linux
v4 cover RISCV_EFI_BOOT_PROTOCOL support in linux
This patch adds support for getting the boot hart ID using new RISCV_EFI_BOOT_PROTOCOL in linux efi stub. While there is an existing solution of passing the boot hart ID through Device Tree, it doesn't work for ACPI. Hence an EFI protocol protocol is recommended which works for both DT and ACPI based platforms.
The spec of this new protocol post public review (Ratification-ready) is available at:
https://github.com/riscv-non-isa/riscv-uefi/releases/download/1.0.0/RISCV_UEFI_PROTOCOL-spec.pdf
This is tested in qemu with both u-boot and edk2.
Changes since V3:
- Rebased, no code changes.
- cover letter updated with link to spec version after public review
Changes since V2:
- Updated error message
Changes since V1:
- Rebased to get the "Fix get_boot_hartid_from_fdt() return value" patch
- Removed mixed_mode member
- Separated return value and status.
Sunil V L (1):
riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)
Signature · 1 line
2.25.1
PATCH v4 1/1 riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
This patch adds the support for getting the boot hart ID in Linux EFI stub using RISCV_EFI_BOOT_PROTOCOL. This protocol is preferred method over existing DT based solution since it works irrespective of DT or ACPI.
The specification of the protocol is hosted at:
https://github.com/riscv-non-isa/riscv-uefi
--- drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)
drivers/firmware/efi/libstub/efistub.h +7 -0
@@ -720,6 +720,13 @@ union efi_tcg2_protocol { } mixed_mode; }; +struct riscv_efi_boot_protocol { +u64 revision; + + efi_status_t (__efiapi * get_boot_hartid)(struct riscv_efi_boot_protocol *this, + size_t *boot_hartid); +}; +typedef union efi_load_file_protocol efi_load_file_protocol_t ;typedef union efi_load_file_protocol efi_load_file2_protocol_t ;
drivers/firmware/efi/libstub/riscv-stub.c +24 -5
@@ -23,7 +23,7 @@typedef void __noreturn (*jump_kernel_func )(unsigned int ,unsigned long ); -static u32 hartid; +static size_t hartid;static int get_boot_hartid_from_fdt (void ) {
@@ -47,14 +47,33 @@ static int get_boot_hartid_from_fdt(void)return 0 ; } +static efi_status_t get_boot_hartid_from_efi (void ) +{ +efi_guid_t boot_protocol_guid =RISCV_EFI_BOOT_PROTOCOL_GUID ; +efi_status_t status; +struct riscv_efi_boot_protocol *boot_protocol; + + status =efi_bs_call (locate_protocol, &boot_protocol_guid,NULL , + (void **)&boot_protocol); +if (status ==EFI_SUCCESS ) { + status =efi_call_proto (boot_protocol, + get_boot_hartid, &hartid); + } +return status; +} +efi_status_t check_platform_features (void ) {int ret; +efi_status_t status; - ret =get_boot_hartid_from_fdt (); -if (ret) { -efi_err ("/chosen/boot-hartid missing or invalid!\n" ); -return EFI_UNSUPPORTED ; + status =get_boot_hartid_from_efi (); +if (status !=EFI_SUCCESS ) { + ret =get_boot_hartid_from_fdt (); +if (ret) { +efi_err ("Failed to get boot hartid!\n" ); +return EFI_UNSUPPORTED ; + } }return EFI_SUCCESS ; }
include/linux/efi.h +1 -0
@@ -380,6 +380,7 @@ void efi_native_runtime_setup(void);#define EFI_CONSOLE_OUT_DEVICE_GUID EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4, 0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)#define APPLE_PROPERTIES_PROTOCOL_GUID EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb, 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)#define EFI_TCG2_PROTOCOL_GUID EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) +#define RISCV_EFI_BOOT_PROTOCOL_GUID EFI_GUID(0xccd15fec, 0x6f73, 0x4eec, 0x83, 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf)#define EFI_LOAD_FILE_PROTOCOL_GUID EFI_GUID(0x56ec3091, 0x954c, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)#define EFI_LOAD_FILE2_PROTOCOL_GUID EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)#define EFI_RT_PROPERTIES_TABLE_GUID EFI_GUID(0xeb66918a, 0x7eef, 0x402a, 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9)
Signature · 1 line
2.25.1
Ard Biesheuvel
maintainerRe: [PATCH V4 1/1] riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
Quoted 23 lines · Sunil V L
This patch adds the support for getting the boot hart ID in Linux EFI stub using
RISCV_EFI_BOOT_PROTOCOL. This protocol is preferred method over existing DT based solution since it works irrespective of DT or ACPI.The specification of the protocol is hosted at:
https://github.com/riscv-non-isa/riscv-uefiSigned-off-by Sunil V L <[email protected]>Acked-by Palmer Dabbelt <[email protected]>Reviewed-by Heinrich Schuchardt <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)drivers/firmware/efi/libstub/efistub.h +0
-0
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 727
Quoted 8 lines
drivers/firmware/efi/libstub/efistub.h +5
-0@@ -720,6 +720,13 @@ union efi_tcg2_protocol { } mixed_mode; }; +struct riscv_efi_boot_protocol { +u64 revision; + + efi_status_t (__efiapi * get_boot_hartid)(struct riscv_efi_boot_protocol *this, + size_t *boot_hartid);
size_t is not a EFI type, and your spec uses UINTN here, which is equivalent to 'unsigned long'. However, jump_kernel_func() takes a unsigned int for the hartid. Please clean this up.
Quoted 10 lines
drivers/firmware/efi/libstub/efistub.h +2
-0+}; +typedef union efi_load_file_protocol efi_load_file_protocol_t ;typedef union efi_load_file_protocol efi_load_file2_protocol_t ;drivers/firmware/efi/libstub/riscv-stub.c +0
-0
drivers/firmware/efi/libstub/riscv-stub.c line 27
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +1
-1@@ -23,7 +23,7 @@typedef void __noreturn (*jump_kernel_func )(unsigned int ,unsigned long ); -static u32 hartid; +static size_t hartid;
and here
Quoted 53 lines
drivers/firmware/efi/libstub/riscv-stub.c +23
-4static int get_boot_hartid_from_fdt (void ) {@@ -47,14 +47,33 @@ static int get_boot_hartid_from_fdt(void)return 0 ; } +static efi_status_t get_boot_hartid_from_efi (void ) +{ +efi_guid_t boot_protocol_guid =RISCV_EFI_BOOT_PROTOCOL_GUID ; +efi_status_t status; +struct riscv_efi_boot_protocol *boot_protocol; + + status =efi_bs_call (locate_protocol, &boot_protocol_guid,NULL , + (void **)&boot_protocol); +if (status ==EFI_SUCCESS ) { + status =efi_call_proto (boot_protocol, + get_boot_hartid, &hartid); + } +return status; +} +efi_status_t check_platform_features (void ) {int ret; +efi_status_t status; - ret =get_boot_hartid_from_fdt (); -if (ret) { -efi_err ("/chosen/boot-hartid missing or invalid!\n" ); -return EFI_UNSUPPORTED ; + status =get_boot_hartid_from_efi (); +if (status !=EFI_SUCCESS ) { + ret =get_boot_hartid_from_fdt (); +if (ret) { +efi_err ("Failed to get boot hartid!\n" ); +return EFI_UNSUPPORTED ; + } }return EFI_SUCCESS ; }include/linux/efi.h +1
-1@@ -380,6 +380,7 @@ void efi_native_runtime_setup(void);#define EFI_CONSOLE_OUT_DEVICE_GUID EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4, 0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)#define APPLE_PROPERTIES_PROTOCOL_GUID EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb, 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)#define EFI_TCG2_PROTOCOL_GUID EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) +#define RISCV_EFI_BOOT_PROTOCOL_GUID EFI_GUID(0xccd15fec, 0x6f73, 0x4eec, 0x83, 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf)#define EFI_LOAD_FILE_PROTOCOL_GUID EFI_GUID(0x56ec3091, 0x954c, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)#define EFI_LOAD_FILE2_PROTOCOL_GUID EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)#define EFI_RT_PROPERTIES_TABLE_GUID EFI_GUID(0xeb66918a, 0x7eef, 0x402a, 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9) --
Quoted 2 lines
2.25.1
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH V4 1/1] riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
Quoted 27 lines · Ard Biesheuvel
Quoted 26 lines · Sunil V L
This patch adds the support for getting the boot hart ID in Linux EFI stub using
RISCV_EFI_BOOT_PROTOCOL. This protocol is preferred method over existing DT based solution since it works irrespective of DT or ACPI.The specification of the protocol is hosted at:
https://github.com/riscv-non-isa/riscv-uefiSigned-off-by Sunil V L <[email protected]>Acked-by Palmer Dabbelt <[email protected]>Reviewed-by Heinrich Schuchardt <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)drivers/firmware/efi/libstub/efistub.h +0
-0@@ -720,6 +720,13 @@ union efi_tcg2_protocol { } mixed_mode; };
Quoted 10 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 727
Quoted 5 lines
drivers/firmware/efi/libstub/efistub.h +5
-0+struct riscv_efi_boot_protocol { +u64 revision; + + efi_status_t (__efiapi * get_boot_hartid)(struct riscv_efi_boot_protocol *this, + size_t *boot_hartid);
size_tis not a EFI type, and your spec uses UINTN here, which is equivalent to 'unsigned long'. However,jump_kernel_func()takes a unsigned int for the hartid. Please clean this up.
Thank you Ard. Yes, hartid can be of native length. Have fixed this and sent V5. Please check.
Thanks!
Sunil
Quoted 75 lines
Quoted 11 lines
drivers/firmware/efi/libstub/efistub.h +2
-0+}; +typedef union efi_load_file_protocol efi_load_file_protocol_t ;typedef union efi_load_file_protocol efi_load_file2_protocol_t ;drivers/firmware/efi/libstub/riscv-stub.c +0
-0drivers/firmware/efi/libstub/riscv-stub.c line 27
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +1
-1@@ -23,7 +23,7 @@typedef void __noreturn (*jump_kernel_func )(unsigned int ,unsigned long ); -static u32 hartid; +static size_t hartid;and here
Quoted 55 lines
drivers/firmware/efi/libstub/riscv-stub.c +23
-4static int get_boot_hartid_from_fdt (void ) {@@ -47,14 +47,33 @@ static int get_boot_hartid_from_fdt(void)return 0 ; } +static efi_status_t get_boot_hartid_from_efi (void ) +{ +efi_guid_t boot_protocol_guid =RISCV_EFI_BOOT_PROTOCOL_GUID ; +efi_status_t status; +struct riscv_efi_boot_protocol *boot_protocol; + + status =efi_bs_call (locate_protocol, &boot_protocol_guid,NULL , + (void **)&boot_protocol); +if (status ==EFI_SUCCESS ) { + status =efi_call_proto (boot_protocol, + get_boot_hartid, &hartid); + } +return status; +} +efi_status_t check_platform_features (void ) {int ret; +efi_status_t status; - ret =get_boot_hartid_from_fdt (); -if (ret) { -efi_err ("/chosen/boot-hartid missing or invalid!\n" ); -return EFI_UNSUPPORTED ; + status =get_boot_hartid_from_efi (); +if (status !=EFI_SUCCESS ) { + ret =get_boot_hartid_from_fdt (); +if (ret) { +efi_err ("Failed to get boot hartid!\n" ); +return EFI_UNSUPPORTED ; + } }return EFI_SUCCESS ; }include/linux/efi.h +1
-1@@ -380,6 +380,7 @@ void efi_native_runtime_setup(void);#define EFI_CONSOLE_OUT_DEVICE_GUID EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4, 0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)#define APPLE_PROPERTIES_PROTOCOL_GUID EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb, 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)#define EFI_TCG2_PROTOCOL_GUID EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) +#define RISCV_EFI_BOOT_PROTOCOL_GUID EFI_GUID(0xccd15fec, 0x6f73, 0x4eec, 0x83, 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf)#define EFI_LOAD_FILE_PROTOCOL_GUID EFI_GUID(0x56ec3091, 0x954c, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)#define EFI_LOAD_FILE2_PROTOCOL_GUID EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)#define EFI_RT_PROPERTIES_TABLE_GUID EFI_GUID(0xeb66918a, 0x7eef, 0x402a, 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9) --2.25.1
Re: [PATCH V4 1/1] riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
In reply to Ard BiesheuvelQuoted 27 lines · Ard Biesheuvel
Quoted 26 lines · Sunil V L
This patch adds the support for getting the boot hart ID in Linux EFI stub using
RISCV_EFI_BOOT_PROTOCOL. This protocol is preferred method over existing DT based solution since it works irrespective of DT or ACPI.The specification of the protocol is hosted at:
https://github.com/riscv-non-isa/riscv-uefiSigned-off-by Sunil V L <[email protected]>Acked-by Palmer Dabbelt <[email protected]>Reviewed-by Heinrich Schuchardt <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)drivers/firmware/efi/libstub/efistub.h +0
-0@@ -720,6 +720,13 @@ union efi_tcg2_protocol { } mixed_mode; };
Quoted 9 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 727
Quoted 5 lines
drivers/firmware/efi/libstub/efistub.h +5
-0+struct riscv_efi_boot_protocol { +u64 revision; + + efi_status_t (__efiapi * get_boot_hartid)(struct riscv_efi_boot_protocol *this, + size_t *boot_hartid);
size_tis not a EFI type, and your spec uses UINTN here, which is equivalent to 'unsigned long'. However,jump_kernel_func()takes a unsigned int for the hartid. Please clean this up.
unsigned long and size_t have the same number of bits. This seems to be a question of taste where we should follow the maintainer.
jump_kernel_func() assuming boot hart ID to be an unsigned int is not in line with the RISC-V ISA which allows to use all xlen bits.
Quoted 27 lines
Quoted 12 lines
drivers/firmware/efi/libstub/efistub.h +2
-0+}; +typedef union efi_load_file_protocol efi_load_file_protocol_t ;typedef union efi_load_file_protocol efi_load_file2_protocol_t ;drivers/firmware/efi/libstub/riscv-stub.c +0
-0drivers/firmware/efi/libstub/riscv-stub.c line 27
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +1
-1@@ -23,7 +23,7 @@typedef void __noreturn (*jump_kernel_func )(unsigned int ,unsigned long ); -static u32 hartid; +static size_t hartid;and here
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +0
-0static int get_boot_hartid_from_fdt (void ) {@@ -47,14 +47,33 @@ static int get_boot_hartid_from_fdt(void)return 0 ; }
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c static int get_boot_hartid_from_fdt(void) · line 60
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c +11
-0+static efi_status_t get_boot_hartid_from_efi (void ) +{ +efi_guid_t boot_protocol_guid =RISCV_EFI_BOOT_PROTOCOL_GUID ; +efi_status_t status; +struct riscv_efi_boot_protocol *boot_protocol; + + status =efi_bs_call (locate_protocol, &boot_protocol_guid,NULL , + (void **)&boot_protocol); +if (status ==EFI_SUCCESS ) { + status =efi_call_proto (boot_protocol, + get_boot_hartid, &hartid);
A lot of the kernel code seems to be unfit to handle hart IDs exceeding INT_MAX (e.g. sbi_cpu_is_stopped()). Until this is fixed we have to treat hartid > INT_MAX as an error.
Best regards
Heinrich
Quoted 38 lines
Quoted 38 lines
drivers/firmware/efi/libstub/riscv-stub.c +12
-4+ } +return status; +} +efi_status_t check_platform_features (void ) {int ret; +efi_status_t status; - ret =get_boot_hartid_from_fdt (); -if (ret) { -efi_err ("/chosen/boot-hartid missing or invalid!\n" ); -return EFI_UNSUPPORTED ; + status =get_boot_hartid_from_efi (); +if (status !=EFI_SUCCESS ) { + ret =get_boot_hartid_from_fdt (); +if (ret) { +efi_err ("Failed to get boot hartid!\n" ); +return EFI_UNSUPPORTED ; + } }return EFI_SUCCESS ; }include/linux/efi.h +1
-1@@ -380,6 +380,7 @@ void efi_native_runtime_setup(void);#define EFI_CONSOLE_OUT_DEVICE_GUID EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4, 0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)#define APPLE_PROPERTIES_PROTOCOL_GUID EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb, 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)#define EFI_TCG2_PROTOCOL_GUID EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) +#define RISCV_EFI_BOOT_PROTOCOL_GUID EFI_GUID(0xccd15fec, 0x6f73, 0x4eec, 0x83, 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf)#define EFI_LOAD_FILE_PROTOCOL_GUID EFI_GUID(0x56ec3091, 0x954c, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)#define EFI_LOAD_FILE2_PROTOCOL_GUID EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)#define EFI_RT_PROPERTIES_TABLE_GUID EFI_GUID(0xeb66918a, 0x7eef, 0x402a, 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9) --2.25.1
Ard Biesheuvel
maintainerRe: [PATCH V4 1/1] riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
Quoted 41 lines · Heinrich Schuchardt
Quoted 27 lines · Ard Biesheuvel
Quoted 26 lines · Sunil V L
This patch adds the support for getting the boot hart ID in Linux EFI stub using
RISCV_EFI_BOOT_PROTOCOL. This protocol is preferred method over existing DT based solution since it works irrespective of DT or ACPI.The specification of the protocol is hosted at:
https://github.com/riscv-non-isa/riscv-uefiSigned-off-by Sunil V L <[email protected]>Acked-by Palmer Dabbelt <[email protected]>Reviewed-by Heinrich Schuchardt <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)drivers/firmware/efi/libstub/efistub.h +0
-0@@ -720,6 +720,13 @@ union efi_tcg2_protocol { } mixed_mode; };Quoted 9 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 727
Quoted 5 lines
drivers/firmware/efi/libstub/efistub.h +5
-0+struct riscv_efi_boot_protocol { +u64 revision; + + efi_status_t (__efiapi * get_boot_hartid)(struct riscv_efi_boot_protocol *this, + size_t *boot_hartid);
size_tis not a EFI type, and your spec uses UINTN here, which is equivalent to 'unsigned long'. However,jump_kernel_func()takes a unsigned int for the hartid. Please clean this up.
Quoted 3 lines
unsigned long and
size_thave the same number of bits. This seems to be a question of taste where we should follow the maintainer.
We use unsigned long wherever the UEFI spec uses UINTN. This is not a matter of taste, really.
Quoted 30 lines
jump_kernel_func()assuming boot hart ID to be an unsigned int is not in line with the RISC-V ISA which allows to use all xlen bits.Quoted 27 lines
Quoted 12 lines
drivers/firmware/efi/libstub/efistub.h +2
-0+}; +typedef union efi_load_file_protocol efi_load_file_protocol_t ;typedef union efi_load_file_protocol efi_load_file2_protocol_t ;drivers/firmware/efi/libstub/riscv-stub.c +0
-0drivers/firmware/efi/libstub/riscv-stub.c line 27
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +1
-1@@ -23,7 +23,7 @@typedef void __noreturn (*jump_kernel_func )(unsigned int ,unsigned long ); -static u32 hartid; +static size_t hartid;and here
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +0
-0static int get_boot_hartid_from_fdt (void ) {@@ -47,14 +47,33 @@ static int get_boot_hartid_from_fdt(void)return 0 ; }
Quoted 16 lines
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c static int get_boot_hartid_from_fdt(void) · line 60
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c +11
-0+static efi_status_t get_boot_hartid_from_efi (void ) +{ +efi_guid_t boot_protocol_guid =RISCV_EFI_BOOT_PROTOCOL_GUID ; +efi_status_t status; +struct riscv_efi_boot_protocol *boot_protocol; + + status =efi_bs_call (locate_protocol, &boot_protocol_guid,NULL , + (void **)&boot_protocol); +if (status ==EFI_SUCCESS ) { + status =efi_call_proto (boot_protocol, + get_boot_hartid, &hartid);A lot of the kernel code seems to be unfit to handle hart IDs exceeding
INT_MAX(e.g.sbi_cpu_is_stopped()). Until this is fixed we have to treat hartid >INT_MAXas an error.
This is an issue in the core kernel code, not in the EFI stub. As you pointed out, the ISA implies UINTN / unsigned long here, and if the startup code cannot deal with that, it can be fixed separately.
Re: [PATCH V4 1/1] riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
Quoted 98 lines · Ard Biesheuvel
Quoted 41 lines · Heinrich Schuchardt
Quoted 27 lines · Ard Biesheuvel
Quoted 26 lines · Sunil V L
This patch adds the support for getting the boot hart ID in Linux EFI stub using
RISCV_EFI_BOOT_PROTOCOL. This protocol is preferred method over existing DT based solution since it works irrespective of DT or ACPI.The specification of the protocol is hosted at:
https://github.com/riscv-non-isa/riscv-uefiSigned-off-by Sunil V L <[email protected]>Acked-by Palmer Dabbelt <[email protected]>Reviewed-by Heinrich Schuchardt <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)drivers/firmware/efi/libstub/efistub.h +0
-0@@ -720,6 +720,13 @@ union efi_tcg2_protocol { } mixed_mode; };Quoted 9 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 727
Quoted 5 lines
drivers/firmware/efi/libstub/efistub.h +5
-0+struct riscv_efi_boot_protocol { +u64 revision; + + efi_status_t (__efiapi * get_boot_hartid)(struct riscv_efi_boot_protocol *this, + size_t *boot_hartid);
size_tis not a EFI type, and your spec uses UINTN here, which is equivalent to 'unsigned long'. However,jump_kernel_func()takes a unsigned int for the hartid. Please clean this up.Quoted 3 lines
unsigned long and
size_thave the same number of bits. This seems to be a question of taste where we should follow the maintainer.We use unsigned long wherever the UEFI spec uses UINTN. This is not a matter of taste, really.
Quoted 30 lines
jump_kernel_func()assuming boot hart ID to be an unsigned int is not in line with the RISC-V ISA which allows to use all xlen bits.Quoted 27 lines
Quoted 12 lines
drivers/firmware/efi/libstub/efistub.h +2
-0+}; +typedef union efi_load_file_protocol efi_load_file_protocol_t ;typedef union efi_load_file_protocol efi_load_file2_protocol_t ;drivers/firmware/efi/libstub/riscv-stub.c +0
-0drivers/firmware/efi/libstub/riscv-stub.c line 27
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +1
-1@@ -23,7 +23,7 @@typedef void __noreturn (*jump_kernel_func )(unsigned int ,unsigned long ); -static u32 hartid; +static size_t hartid;and here
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +0
-0static int get_boot_hartid_from_fdt (void ) {@@ -47,14 +47,33 @@ static int get_boot_hartid_from_fdt(void)return 0 ; }Quoted 16 lines
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c static int get_boot_hartid_from_fdt(void) · line 60
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c +11
-0+static efi_status_t get_boot_hartid_from_efi (void ) +{ +efi_guid_t boot_protocol_guid =RISCV_EFI_BOOT_PROTOCOL_GUID ; +efi_status_t status; +struct riscv_efi_boot_protocol *boot_protocol; + + status =efi_bs_call (locate_protocol, &boot_protocol_guid,NULL , + (void **)&boot_protocol); +if (status ==EFI_SUCCESS ) { + status =efi_call_proto (boot_protocol, + get_boot_hartid, &hartid);A lot of the kernel code seems to be unfit to handle hart IDs exceeding
INT_MAX(e.g.sbi_cpu_is_stopped()). Until this is fixed we have to treat hartid >INT_MAXas an error.
Quoted 3 lines
This is an issue in the core kernel code, not in the EFI stub. As you pointed out, the ISA implies UINTN / unsigned long here, and if the startup code cannot deal with that, it can be fixed separately.
It was kept as unsigned int because hartid > INT_MAX is very unlikely to happen. But I agree that we should just follow the spec(allowing XLEN bits for hartid) and
change "unsigned int" to "unsigned long" wherever hartid is concerned.
As the hartid is changed to unsigned long, get_boot_hartid_from_fdt should be fixed as well.
Currently, it is using fdt32_to_cpu.
--
Regards,
Atish
Re: [PATCH V4 1/1] riscv/efi_stub: Add support for RISCV_EFI_BOOT_PROTOCOL
Quoted 103 lines · Atish Patra
Quoted 98 lines · Ard Biesheuvel
Quoted 41 lines · Heinrich Schuchardt
Quoted 27 lines · Ard Biesheuvel
Quoted 26 lines · Sunil V L
This patch adds the support for getting the boot hart ID in Linux EFI stub using
RISCV_EFI_BOOT_PROTOCOL. This protocol is preferred method over existing DT based solution since it works irrespective of DT or ACPI.The specification of the protocol is hosted at:
https://github.com/riscv-non-isa/riscv-uefiSigned-off-by Sunil V L <[email protected]>Acked-by Palmer Dabbelt <[email protected]>Reviewed-by Heinrich Schuchardt <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 7 ++++++ drivers/firmware/efi/libstub/riscv-stub.c | 29 +++++++++++++++++++---- include/linux/efi.h | 1 + 3 files changed, 32 insertions(+), 5 deletions(-)drivers/firmware/efi/libstub/efistub.h +0
-0@@ -720,6 +720,13 @@ union efi_tcg2_protocol { } mixed_mode; };Quoted 9 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 727
Quoted 5 lines
drivers/firmware/efi/libstub/efistub.h +5
-0+struct riscv_efi_boot_protocol { +u64 revision; + + efi_status_t (__efiapi * get_boot_hartid)(struct riscv_efi_boot_protocol *this, + size_t *boot_hartid);
size_tis not a EFI type, and your spec uses UINTN here, which is equivalent to 'unsigned long'. However,jump_kernel_func()takes a unsigned int for the hartid. Please clean this up.Quoted 3 lines
unsigned long and
size_thave the same number of bits. This seems to be a question of taste where we should follow the maintainer.We use unsigned long wherever the UEFI spec uses UINTN. This is not a matter of taste, really.
Quoted 30 lines
jump_kernel_func()assuming boot hart ID to be an unsigned int is not in line with the RISC-V ISA which allows to use all xlen bits.Quoted 27 lines
Quoted 12 lines
drivers/firmware/efi/libstub/efistub.h +2
-0+}; +typedef union efi_load_file_protocol efi_load_file_protocol_t ;typedef union efi_load_file_protocol efi_load_file2_protocol_t ;drivers/firmware/efi/libstub/riscv-stub.c +0
-0drivers/firmware/efi/libstub/riscv-stub.c line 27
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +1
-1@@ -23,7 +23,7 @@typedef void __noreturn (*jump_kernel_func )(unsigned int ,unsigned long ); -static u32 hartid; +static size_t hartid;and here
Quoted 6 lines
drivers/firmware/efi/libstub/riscv-stub.c +0
-0static int get_boot_hartid_from_fdt (void ) {@@ -47,14 +47,33 @@ static int get_boot_hartid_from_fdt(void)return 0 ; }Quoted 16 lines
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c static int get_boot_hartid_from_fdt(void) · line 60
Quoted 11 lines
drivers/firmware/efi/libstub/riscv-stub.c +11
-0+static efi_status_t get_boot_hartid_from_efi (void ) +{ +efi_guid_t boot_protocol_guid =RISCV_EFI_BOOT_PROTOCOL_GUID ; +efi_status_t status; +struct riscv_efi_boot_protocol *boot_protocol; + + status =efi_bs_call (locate_protocol, &boot_protocol_guid,NULL , + (void **)&boot_protocol); +if (status ==EFI_SUCCESS ) { + status =efi_call_proto (boot_protocol, + get_boot_hartid, &hartid);A lot of the kernel code seems to be unfit to handle hart IDs exceeding
INT_MAX(e.g.sbi_cpu_is_stopped()). Until this is fixed we have to treat hartid >INT_MAXas an error.Quoted 3 lines
This is an issue in the core kernel code, not in the EFI stub. As you pointed out, the ISA implies UINTN / unsigned long here, and if the startup code cannot deal with that, it can be fixed separately.
Quoted 8 lines
It was kept as unsigned int because hartid >
INT_MAXis very unlikely to happen. But I agree that we should just follow the spec(allowing XLEN bits for hartid) and
change "unsigned int" to "unsigned long" wherever hartid is concerned.As the hartid is changed to unsigned long,
get_boot_hartid_from_fdtshould be fixed as well.
Currently, it is usingfdt32_to_cpu.
I am working on a separate patch series to modify these instances. Will send soon for your feedback.
Thanks
Sunil
Quoted 6 lines
--
Regards,
Atish