[PATCH 0/2] efi/reboot: Enable platform specific reset on arm64

[PATCH 0/2] efi/reboot: Enable platform specific reset on arm64

[Sumit Garg]

From: Sumit Garg <sumit.garg@oss.qualcomm.com>

Vendor/platform specific reset types are common on arm64 platforms as
can be seen from this patch-set [1]. EFI runtime ResetSystem service
provide an alternate method known as EFI_RESET_PLATFORM_SPECIFIC [2].
It should be able to handle these reset scenarios where the platform
specific UEFI implementation will decode the reboot command as reset
data passed as a parameter.

On UEFI systems, it is rather a better abstracted interface for the OS
to use EFI platform specific runtime reset method rather than overriding
PSCI reset method with DT based overrides on arm64 platforms.

[1] https://lore.kernel.org/all/20251109-arm-psci-system_reset2-vendor-reboots-v17-0-46e085bca4cc@oss.qualcomm.com/
[2] https://uefi.org/specs/UEFI/2.10/08_Services_Runtime_Services.html?highlight=resetsystem#resetsystem

Sumit Garg (2):
  efi/reboot: Add support for EFI_RESET_PLATFORM_SPECIFIC
  arm64: efi: Pass reboot cmd parameter to efi_reboot()

 arch/arm64/kernel/process.c   |  2 +-
 drivers/firmware/efi/reboot.c | 25 +++++++++++++++----------
 include/linux/efi.h           |  5 +++--
 3 files changed, 19 insertions(+), 13 deletions(-)

-- 
2.48.1

[PATCH 1/2] efi/reboot: Add support for EFI_RESET_PLATFORM_SPECIFIC

[Sumit Garg]

From: Sumit Garg <sumit.garg@oss.qualcomm.com>

UEFI specification provides support for EfiResetPlatformSpecific reset
type as follows:

"
ResetSystem:

Calling this interface with ResetType of EfiResetPlatformSpecific
causes a system-wide reset. The exact type of the reset is defined
by the EFI_GUID that follows the Null-terminated Unicode string passed
into ResetData. If the platform does not recognize the EFI_GUID in
ResetData the platform must pick a supported reset type to perform.
The platform may optionally log the parameters from any non-normal
reset that occurs.
"

Lets use the ResetData to pass the platform specific reboot command
issued and leave it's interpretation to UEFI implementation following
the specification.

Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
---
 drivers/firmware/efi/reboot.c | 25 +++++++++++++++----------
 include/linux/efi.h           |  5 +++--
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/efi/reboot.c b/drivers/firmware/efi/reboot.c
index ceae84c19d22..23a2fc68e9c9 100644
--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -10,7 +10,7 @@ static struct sys_off_handler *efi_sys_off_handler;
 
 int efi_reboot_quirk_mode = -1;
 
-void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
+void efi_reboot(enum reboot_mode reboot_mode, const char *data)
 {
 	const char *str[] = { "cold", "warm", "shutdown", "platform" };
 	int efi_mode, cap_reset_mode;
@@ -18,14 +18,18 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_RESET_SYSTEM))
 		return;
 
-	switch (reboot_mode) {
-	case REBOOT_WARM:
-	case REBOOT_SOFT:
-		efi_mode = EFI_RESET_WARM;
-		break;
-	default:
-		efi_mode = EFI_RESET_COLD;
-		break;
+	if (data) {
+		efi_mode = EFI_RESET_PLATFORM_SPECIFIC;
+	} else {
+		switch (reboot_mode) {
+		case REBOOT_WARM:
+		case REBOOT_SOFT:
+			efi_mode = EFI_RESET_WARM;
+			break;
+		default:
+			efi_mode = EFI_RESET_COLD;
+			break;
+		}
 	}
 
 	/*
@@ -43,7 +47,8 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
 		efi_mode = cap_reset_mode;
 	}
 
-	efi.reset_system(efi_mode, EFI_SUCCESS, 0, NULL);
+	efi.reset_system(efi_mode, EFI_SUCCESS, sizeof(data),
+			 (efi_char16_t *)data);
 }
 
 bool __weak efi_poweroff_required(void)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a98cc39e7aaa..5324db1518b6 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -256,6 +256,7 @@ typedef union efi_boot_services efi_boot_services_t;
 #define EFI_RESET_COLD 0
 #define EFI_RESET_WARM 1
 #define EFI_RESET_SHUTDOWN 2
+#define EFI_RESET_PLATFORM_SPECIFIC 3
 
 /*
  * EFI Runtime Services table
@@ -874,7 +875,7 @@ static inline bool efi_enabled(int feature)
 {
 	return test_bit(feature, &efi.flags) != 0;
 }
-extern void efi_reboot(enum reboot_mode reboot_mode, const char *__unused);
+extern void efi_reboot(enum reboot_mode reboot_mode, const char *data);
 
 bool __pure __efi_soft_reserve_enabled(void);
 
@@ -895,7 +896,7 @@ static inline bool efi_enabled(int feature)
 	return false;
 }
 static inline void
-efi_reboot(enum reboot_mode reboot_mode, const char *__unused) {}
+efi_reboot(enum reboot_mode reboot_mode, const char *data) {}
 
 static inline bool efi_soft_reserve_enabled(void)
 {
-- 
2.48.1

[PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Sumit Garg]

From: Sumit Garg <sumit.garg@oss.qualcomm.com>

EFI ResetSystem runtime service allows for platform specific reset type
allowing the OS to pass reset data for the UEFI implementation to take
corresponding action. So lets pass the reboot cmd parameter for the EFI
driver to determine whether it's a platform specific reset requested or
not.

Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
---
 arch/arm64/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index fba7ca102a8c..51784986c568 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -136,7 +136,7 @@ void machine_restart(char *cmd)
 	 * ResetSystem().
 	 */
 	if (efi_enabled(EFI_RUNTIME_SERVICES))
-		efi_reboot(reboot_mode, NULL);
+		efi_reboot(reboot_mode, cmd);
 
 	/* Now call the architecture specific reboot code. */
 	do_kernel_restart(cmd);
-- 
2.48.1

Re: [PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Ard Biesheuvel]

On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
>
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>
> EFI ResetSystem runtime service allows for platform specific reset type
> allowing the OS to pass reset data for the UEFI implementation to take
> corresponding action. So lets pass the reboot cmd parameter for the EFI
> driver to determine whether it's a platform specific reset requested or
> not.
>
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---
>  arch/arm64/kernel/process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index fba7ca102a8c..51784986c568 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -136,7 +136,7 @@ void machine_restart(char *cmd)
>          * ResetSystem().
>          */
>         if (efi_enabled(EFI_RUNTIME_SERVICES))
> -               efi_reboot(reboot_mode, NULL);
> +               efi_reboot(reboot_mode, cmd);
>

I agree with the general principle. However, there are already
existing callers of kernel_restart() that would end up passing a
random string to efi_reboot(), resulting in platform specific reset
with undefined result.

E.g.,

$ git grep kernel_restart\(\"
drivers/md/dm-verity-target.c:          kernel_restart("dm-verity
device corrupted");
drivers/md/dm-verity-target.c:  kernel_restart("dm-verity device has
I/O error");
drivers/memory/emif.c:                  kernel_restart("SDRAM
Over-temp Emergency restart");


>         /* Now call the architecture specific reboot code. */
>         do_kernel_restart(cmd);
> --
> 2.48.1
>

Re: [PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Sumit Garg]

On Fri, Nov 14, 2025 at 10:26:03AM +0100, Ard Biesheuvel wrote:
> On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
> >
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >
> > EFI ResetSystem runtime service allows for platform specific reset type
> > allowing the OS to pass reset data for the UEFI implementation to take
> > corresponding action. So lets pass the reboot cmd parameter for the EFI
> > driver to determine whether it's a platform specific reset requested or
> > not.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> >  arch/arm64/kernel/process.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > index fba7ca102a8c..51784986c568 100644
> > --- a/arch/arm64/kernel/process.c
> > +++ b/arch/arm64/kernel/process.c
> > @@ -136,7 +136,7 @@ void machine_restart(char *cmd)
> >          * ResetSystem().
> >          */
> >         if (efi_enabled(EFI_RUNTIME_SERVICES))
> > -               efi_reboot(reboot_mode, NULL);
> > +               efi_reboot(reboot_mode, cmd);
> >
> 
> I agree with the general principle. However, there are already
> existing callers of kernel_restart() that would end up passing a
> random string to efi_reboot(), resulting in platform specific reset
> with undefined result.

Yeah true but the UEFI spec says:

"If the platform does not recognize the EFI_GUID in ResetData the platform
must pick a supported reset type to perform. The platform may optionally
log the parameters from any non-normal reset that occurs."

So, in these cases the UEFI implementation can fallback to normal reset
optionally logging the reset data being passed. Does that sounds
reasonable to you?

-Sumit

> 
> E.g.,
> 
> $ git grep kernel_restart\(\"
> drivers/md/dm-verity-target.c:          kernel_restart("dm-verity
> device corrupted");
> drivers/md/dm-verity-target.c:  kernel_restart("dm-verity device has
> I/O error");
> drivers/memory/emif.c:                  kernel_restart("SDRAM
> Over-temp Emergency restart");
> 
> 
> >         /* Now call the architecture specific reboot code. */
> >         do_kernel_restart(cmd);
> > --
> > 2.48.1
> >

Re: [PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Ard Biesheuvel]

On Fri, 14 Nov 2025 at 10:31, Sumit Garg <sumit.garg@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 10:26:03AM +0100, Ard Biesheuvel wrote:
> > On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
> > >
> > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > >
> > > EFI ResetSystem runtime service allows for platform specific reset type
> > > allowing the OS to pass reset data for the UEFI implementation to take
> > > corresponding action. So lets pass the reboot cmd parameter for the EFI
> > > driver to determine whether it's a platform specific reset requested or
> > > not.
> > >
> > > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > ---
> > >  arch/arm64/kernel/process.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > > index fba7ca102a8c..51784986c568 100644
> > > --- a/arch/arm64/kernel/process.c
> > > +++ b/arch/arm64/kernel/process.c
> > > @@ -136,7 +136,7 @@ void machine_restart(char *cmd)
> > >          * ResetSystem().
> > >          */
> > >         if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > -               efi_reboot(reboot_mode, NULL);
> > > +               efi_reboot(reboot_mode, cmd);
> > >
> >
> > I agree with the general principle. However, there are already
> > existing callers of kernel_restart() that would end up passing a
> > random string to efi_reboot(), resulting in platform specific reset
> > with undefined result.
>
> Yeah true but the UEFI spec says:
>
> "If the platform does not recognize the EFI_GUID in ResetData the platform
> must pick a supported reset type to perform. The platform may optionally
> log the parameters from any non-normal reset that occurs."
>
> So, in these cases the UEFI implementation can fallback to normal reset
> optionally logging the reset data being passed. Does that sounds
> reasonable to you?
>

What the UEFI spec says might deviate from how real platforms in the
field will behave when being passed a reset type that nobody ever
tried passing before.

Re: [PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Ard Biesheuvel]

On Fri, 14 Nov 2025 at 10:33, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 14 Nov 2025 at 10:31, Sumit Garg <sumit.garg@kernel.org> wrote:
> >
> > On Fri, Nov 14, 2025 at 10:26:03AM +0100, Ard Biesheuvel wrote:
> > > On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
> > > >
> > > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > >
> > > > EFI ResetSystem runtime service allows for platform specific reset type
> > > > allowing the OS to pass reset data for the UEFI implementation to take
> > > > corresponding action. So lets pass the reboot cmd parameter for the EFI
> > > > driver to determine whether it's a platform specific reset requested or
> > > > not.
> > > >
> > > > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > > ---
> > > >  arch/arm64/kernel/process.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > > > index fba7ca102a8c..51784986c568 100644
> > > > --- a/arch/arm64/kernel/process.c
> > > > +++ b/arch/arm64/kernel/process.c
> > > > @@ -136,7 +136,7 @@ void machine_restart(char *cmd)
> > > >          * ResetSystem().
> > > >          */
> > > >         if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > > -               efi_reboot(reboot_mode, NULL);
> > > > +               efi_reboot(reboot_mode, cmd);
> > > >
> > >
> > > I agree with the general principle. However, there are already
> > > existing callers of kernel_restart() that would end up passing a
> > > random string to efi_reboot(), resulting in platform specific reset
> > > with undefined result.
> >
> > Yeah true but the UEFI spec says:
> >
> > "If the platform does not recognize the EFI_GUID in ResetData the platform
> > must pick a supported reset type to perform. The platform may optionally
> > log the parameters from any non-normal reset that occurs."
> >
> > So, in these cases the UEFI implementation can fallback to normal reset
> > optionally logging the reset data being passed. Does that sounds
> > reasonable to you?
> >
>
> What the UEFI spec says might deviate from how real platforms in the
> field will behave when being passed a reset type that nobody ever
> tried passing before.

Also, the GUID is expected to follow an unbounded NULL terminated
UTF-16 string in memory, so we could easily cause a crash by doing
this if \0\0 doesn't appear in the memory following the string.

Re: [PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Sumit Garg]

On Fri, Nov 14, 2025 at 10:35:33AM +0100, Ard Biesheuvel wrote:
> On Fri, 14 Nov 2025 at 10:33, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 14 Nov 2025 at 10:31, Sumit Garg <sumit.garg@kernel.org> wrote:
> > >
> > > On Fri, Nov 14, 2025 at 10:26:03AM +0100, Ard Biesheuvel wrote:
> > > > On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
> > > > >
> > > > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > > >
> > > > > EFI ResetSystem runtime service allows for platform specific reset type
> > > > > allowing the OS to pass reset data for the UEFI implementation to take
> > > > > corresponding action. So lets pass the reboot cmd parameter for the EFI
> > > > > driver to determine whether it's a platform specific reset requested or
> > > > > not.
> > > > >
> > > > > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > > > ---
> > > > >  arch/arm64/kernel/process.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > > > > index fba7ca102a8c..51784986c568 100644
> > > > > --- a/arch/arm64/kernel/process.c
> > > > > +++ b/arch/arm64/kernel/process.c
> > > > > @@ -136,7 +136,7 @@ void machine_restart(char *cmd)
> > > > >          * ResetSystem().
> > > > >          */
> > > > >         if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > > > -               efi_reboot(reboot_mode, NULL);
> > > > > +               efi_reboot(reboot_mode, cmd);
> > > > >
> > > >
> > > > I agree with the general principle. However, there are already
> > > > existing callers of kernel_restart() that would end up passing a
> > > > random string to efi_reboot(), resulting in platform specific reset
> > > > with undefined result.
> > >
> > > Yeah true but the UEFI spec says:
> > >
> > > "If the platform does not recognize the EFI_GUID in ResetData the platform
> > > must pick a supported reset type to perform. The platform may optionally
> > > log the parameters from any non-normal reset that occurs."
> > >
> > > So, in these cases the UEFI implementation can fallback to normal reset
> > > optionally logging the reset data being passed. Does that sounds
> > > reasonable to you?
> > >
> >
> > What the UEFI spec says might deviate from how real platforms in the
> > field will behave when being passed a reset type that nobody ever
> > tried passing before.

I suppose from OS point of view, we need to follow the UEFI
specification. However, there will be scope for quirks later if the real
world problems occur. Currently, in case of EFI reboot we are just
ignoring the reboot cmd parameter.

If you have in mind any sanity checks we should do here then feel free
to propose and I can try to implement them.

> 
> Also, the GUID is expected to follow an unbounded NULL terminated
> UTF-16 string in memory, so we could easily cause a crash by doing
> this if \0\0 doesn't appear in the memory following the string.

Okay I see, would following change on top of this patchset address this
concern?

--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -5,6 +5,7 @@
  */
 #include <linux/efi.h>
 #include <linux/reboot.h>
+#include <linux/ucs2_string.h>

 static struct sys_off_handler *efi_sys_off_handler;

@@ -14,11 +15,18 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *data)
 {
        const char *str[] = { "cold", "warm", "shutdown", "platform" };
        int efi_mode, cap_reset_mode;
+       unsigned long reset_data_sz = 0;
+       efi_char16_t *reset_data = NULL;

        if (!efi_rt_services_supported(EFI_RT_SUPPORTED_RESET_SYSTEM))
                return;

        if (data) {
+               reset_data_sz = ucs2_strlen(data) * sizeof(efi_char16_t);
+               reset_data = kzalloc(reset_data_sz + 2, GFP_KERNEL);
+               memcpy(reset_data, data, reset_data_sz);
+               reset_data_sz += 2;
+
                efi_mode = EFI_RESET_PLATFORM_SPECIFIC;
        } else {
                switch (reboot_mode) {
@@ -47,8 +55,7 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *data)
                efi_mode = cap_reset_mode;
        }

-       efi.reset_system(efi_mode, EFI_SUCCESS, sizeof(data),
-                        (efi_char16_t *)data);
+       efi.reset_system(efi_mode, EFI_SUCCESS, reset_data_sz, reset_data);
 }

 bool __weak efi_poweroff_required(void)

-Sumit

Re: [PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Ard Biesheuvel]

On Fri, 14 Nov 2025 at 13:16, Sumit Garg <sumit.garg@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 10:35:33AM +0100, Ard Biesheuvel wrote:
> > On Fri, 14 Nov 2025 at 10:33, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Fri, 14 Nov 2025 at 10:31, Sumit Garg <sumit.garg@kernel.org> wrote:
> > > >
> > > > On Fri, Nov 14, 2025 at 10:26:03AM +0100, Ard Biesheuvel wrote:
> > > > > On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
> > > > > >
> > > > > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > > > >
> > > > > > EFI ResetSystem runtime service allows for platform specific reset type
> > > > > > allowing the OS to pass reset data for the UEFI implementation to take
> > > > > > corresponding action. So lets pass the reboot cmd parameter for the EFI
> > > > > > driver to determine whether it's a platform specific reset requested or
> > > > > > not.
> > > > > >
> > > > > > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > > > > ---
> > > > > >  arch/arm64/kernel/process.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > > > > > index fba7ca102a8c..51784986c568 100644
> > > > > > --- a/arch/arm64/kernel/process.c
> > > > > > +++ b/arch/arm64/kernel/process.c
> > > > > > @@ -136,7 +136,7 @@ void machine_restart(char *cmd)
> > > > > >          * ResetSystem().
> > > > > >          */
> > > > > >         if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > > > > -               efi_reboot(reboot_mode, NULL);
> > > > > > +               efi_reboot(reboot_mode, cmd);
> > > > > >
> > > > >
> > > > > I agree with the general principle. However, there are already
> > > > > existing callers of kernel_restart() that would end up passing a
> > > > > random string to efi_reboot(), resulting in platform specific reset
> > > > > with undefined result.
> > > >
> > > > Yeah true but the UEFI spec says:
> > > >
> > > > "If the platform does not recognize the EFI_GUID in ResetData the platform
> > > > must pick a supported reset type to perform. The platform may optionally
> > > > log the parameters from any non-normal reset that occurs."
> > > >
> > > > So, in these cases the UEFI implementation can fallback to normal reset
> > > > optionally logging the reset data being passed. Does that sounds
> > > > reasonable to you?
> > > >
> > >
> > > What the UEFI spec says might deviate from how real platforms in the
> > > field will behave when being passed a reset type that nobody ever
> > > tried passing before.
>
> I suppose from OS point of view, we need to follow the UEFI
> specification. However, there will be scope for quirks later if the real
> world problems occur. Currently, in case of EFI reboot we are just
> ignoring the reboot cmd parameter.
>
> If you have in mind any sanity checks we should do here then feel free
> to propose and I can try to implement them.
>
> >
> > Also, the GUID is expected to follow an unbounded NULL terminated
> > UTF-16 string in memory, so we could easily cause a crash by doing
> > this if \0\0 doesn't appear in the memory following the string.
>
> Okay I see, would following change on top of this patchset address this
> concern?
>
> --- a/drivers/firmware/efi/reboot.c
> +++ b/drivers/firmware/efi/reboot.c
> @@ -5,6 +5,7 @@
>   */
>  #include <linux/efi.h>
>  #include <linux/reboot.h>
> +#include <linux/ucs2_string.h>
>
>  static struct sys_off_handler *efi_sys_off_handler;
>
> @@ -14,11 +15,18 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *data)
>  {
>         const char *str[] = { "cold", "warm", "shutdown", "platform" };
>         int efi_mode, cap_reset_mode;
> +       unsigned long reset_data_sz = 0;
> +       efi_char16_t *reset_data = NULL;
>
>         if (!efi_rt_services_supported(EFI_RT_SUPPORTED_RESET_SYSTEM))
>                 return;
>
>         if (data) {
> +               reset_data_sz = ucs2_strlen(data) * sizeof(efi_char16_t);

You can't just run ucs2_strlen() on an arbitrary buffer.

> +               reset_data = kzalloc(reset_data_sz + 2, GFP_KERNEL);
> +               memcpy(reset_data, data, reset_data_sz);
> +               reset_data_sz += 2;
> +

What happened to the GUID? It comes after the UTF-16 string, no?

>                 efi_mode = EFI_RESET_PLATFORM_SPECIFIC;
>         } else {
>                 switch (reboot_mode) {
> @@ -47,8 +55,7 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *data)
>                 efi_mode = cap_reset_mode;
>         }
>
> -       efi.reset_system(efi_mode, EFI_SUCCESS, sizeof(data),
> -                        (efi_char16_t *)data);
> +       efi.reset_system(efi_mode, EFI_SUCCESS, reset_data_sz, reset_data);
>  }
>

I think the main issue here is tying machine_restart(), which takes a
u8[] argument, to efi_reboot(), which takes a (u16[]) + L"\0" + GUID
buffer. So the change to efi_reboot() looks fine to me, we just cannot
call it directly from machine_restart() as you are suggesting.

Re: [PATCH 1/2] efi/reboot: Add support for EFI_RESET_PLATFORM_SPECIFIC

[Konrad Dybcio]

On 11/14/25 9:50 AM, Sumit Garg wrote:
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> 
> UEFI specification provides support for EfiResetPlatformSpecific reset
> type as follows:
> 
> "
> ResetSystem:
> 
> Calling this interface with ResetType of EfiResetPlatformSpecific
> causes a system-wide reset. The exact type of the reset is defined
> by the EFI_GUID that follows the Null-terminated Unicode string passed
> into ResetData. If the platform does not recognize the EFI_GUID in
> ResetData the platform must pick a supported reset type to perform.
> The platform may optionally log the parameters from any non-normal
> reset that occurs.
> "
> 
> Lets use the ResetData to pass the platform specific reboot command
> issued and leave it's interpretation to UEFI implementation following
> the specification.
> 
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 1/2] efi/reboot: Add support for EFI_RESET_PLATFORM_SPECIFIC

[Ard Biesheuvel]

On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
>
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>
> UEFI specification provides support for EfiResetPlatformSpecific reset
> type as follows:
>
> "
> ResetSystem:
>
> Calling this interface with ResetType of EfiResetPlatformSpecific
> causes a system-wide reset. The exact type of the reset is defined
> by the EFI_GUID that follows the Null-terminated Unicode string passed
> into ResetData. If the platform does not recognize the EFI_GUID in
> ResetData the platform must pick a supported reset type to perform.
> The platform may optionally log the parameters from any non-normal
> reset that occurs.
> "
>
> Lets use the ResetData to pass the platform specific reboot command
> issued and leave it's interpretation to UEFI implementation following
> the specification.
>
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---
>  drivers/firmware/efi/reboot.c | 25 +++++++++++++++----------
>  include/linux/efi.h           |  5 +++--
>  2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/firmware/efi/reboot.c b/drivers/firmware/efi/reboot.c
> index ceae84c19d22..23a2fc68e9c9 100644
> --- a/drivers/firmware/efi/reboot.c
> +++ b/drivers/firmware/efi/reboot.c
> @@ -10,7 +10,7 @@ static struct sys_off_handler *efi_sys_off_handler;
>
>  int efi_reboot_quirk_mode = -1;
>
> -void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
> +void efi_reboot(enum reboot_mode reboot_mode, const char *data)
>  {
>         const char *str[] = { "cold", "warm", "shutdown", "platform" };
>         int efi_mode, cap_reset_mode;
> @@ -18,14 +18,18 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
>         if (!efi_rt_services_supported(EFI_RT_SUPPORTED_RESET_SYSTEM))
>                 return;
>
> -       switch (reboot_mode) {
> -       case REBOOT_WARM:
> -       case REBOOT_SOFT:
> -               efi_mode = EFI_RESET_WARM;
> -               break;
> -       default:
> -               efi_mode = EFI_RESET_COLD;
> -               break;
> +       if (data) {
> +               efi_mode = EFI_RESET_PLATFORM_SPECIFIC;
> +       } else {
> +               switch (reboot_mode) {
> +               case REBOOT_WARM:
> +               case REBOOT_SOFT:
> +                       efi_mode = EFI_RESET_WARM;
> +                       break;
> +               default:
> +                       efi_mode = EFI_RESET_COLD;
> +                       break;
> +               }
>         }
>
>         /*
> @@ -43,7 +47,8 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
>                 efi_mode = cap_reset_mode;
>         }
>
> -       efi.reset_system(efi_mode, EFI_SUCCESS, 0, NULL);
> +       efi.reset_system(efi_mode, EFI_SUCCESS, sizeof(data),

You cannot use sizeof() here - it will return the size of a pointer to
const char.


> +                        (efi_char16_t *)data);
>  }
>
>  bool __weak efi_poweroff_required(void)
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index a98cc39e7aaa..5324db1518b6 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -256,6 +256,7 @@ typedef union efi_boot_services efi_boot_services_t;
>  #define EFI_RESET_COLD 0
>  #define EFI_RESET_WARM 1
>  #define EFI_RESET_SHUTDOWN 2
> +#define EFI_RESET_PLATFORM_SPECIFIC 3
>
>  /*
>   * EFI Runtime Services table
> @@ -874,7 +875,7 @@ static inline bool efi_enabled(int feature)
>  {
>         return test_bit(feature, &efi.flags) != 0;
>  }
> -extern void efi_reboot(enum reboot_mode reboot_mode, const char *__unused);
> +extern void efi_reboot(enum reboot_mode reboot_mode, const char *data);
>
>  bool __pure __efi_soft_reserve_enabled(void);
>
> @@ -895,7 +896,7 @@ static inline bool efi_enabled(int feature)
>         return false;
>  }
>  static inline void
> -efi_reboot(enum reboot_mode reboot_mode, const char *__unused) {}
> +efi_reboot(enum reboot_mode reboot_mode, const char *data) {}
>
>  static inline bool efi_soft_reserve_enabled(void)
>  {
> --
> 2.48.1
>

Re: [PATCH 2/2] arm64: efi: Pass reboot cmd parameter to efi_reboot()

[Sumit Garg]

On Fri, Nov 14, 2025 at 04:47:18PM +0100, Ard Biesheuvel wrote:
> On Fri, 14 Nov 2025 at 13:16, Sumit Garg <sumit.garg@kernel.org> wrote:
> >
> > On Fri, Nov 14, 2025 at 10:35:33AM +0100, Ard Biesheuvel wrote:
> > > On Fri, 14 Nov 2025 at 10:33, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Fri, 14 Nov 2025 at 10:31, Sumit Garg <sumit.garg@kernel.org> wrote:
> > > > >
> > > > > On Fri, Nov 14, 2025 at 10:26:03AM +0100, Ard Biesheuvel wrote:
> > > > > > On Fri, 14 Nov 2025 at 09:51, Sumit Garg <sumit.garg@kernel.org> wrote:
> > > > > > >
> > > > > > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > > > > >
> > > > > > > EFI ResetSystem runtime service allows for platform specific reset type
> > > > > > > allowing the OS to pass reset data for the UEFI implementation to take
> > > > > > > corresponding action. So lets pass the reboot cmd parameter for the EFI
> > > > > > > driver to determine whether it's a platform specific reset requested or
> > > > > > > not.
> > > > > > >
> > > > > > > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > > > > > ---
> > > > > > >  arch/arm64/kernel/process.c | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > > > > > > index fba7ca102a8c..51784986c568 100644
> > > > > > > --- a/arch/arm64/kernel/process.c
> > > > > > > +++ b/arch/arm64/kernel/process.c
> > > > > > > @@ -136,7 +136,7 @@ void machine_restart(char *cmd)
> > > > > > >          * ResetSystem().
> > > > > > >          */
> > > > > > >         if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > > > > > -               efi_reboot(reboot_mode, NULL);
> > > > > > > +               efi_reboot(reboot_mode, cmd);
> > > > > > >
> > > > > >
> > > > > > I agree with the general principle. However, there are already
> > > > > > existing callers of kernel_restart() that would end up passing a
> > > > > > random string to efi_reboot(), resulting in platform specific reset
> > > > > > with undefined result.
> > > > >
> > > > > Yeah true but the UEFI spec says:
> > > > >
> > > > > "If the platform does not recognize the EFI_GUID in ResetData the platform
> > > > > must pick a supported reset type to perform. The platform may optionally
> > > > > log the parameters from any non-normal reset that occurs."
> > > > >
> > > > > So, in these cases the UEFI implementation can fallback to normal reset
> > > > > optionally logging the reset data being passed. Does that sounds
> > > > > reasonable to you?
> > > > >
> > > >
> > > > What the UEFI spec says might deviate from how real platforms in the
> > > > field will behave when being passed a reset type that nobody ever
> > > > tried passing before.
> >
> > I suppose from OS point of view, we need to follow the UEFI
> > specification. However, there will be scope for quirks later if the real
> > world problems occur. Currently, in case of EFI reboot we are just
> > ignoring the reboot cmd parameter.
> >
> > If you have in mind any sanity checks we should do here then feel free
> > to propose and I can try to implement them.
> >
> > >
> > > Also, the GUID is expected to follow an unbounded NULL terminated
> > > UTF-16 string in memory, so we could easily cause a crash by doing
> > > this if \0\0 doesn't appear in the memory following the string.
> >
> > Okay I see, would following change on top of this patchset address this
> > concern?
> >
> > --- a/drivers/firmware/efi/reboot.c
> > +++ b/drivers/firmware/efi/reboot.c
> > @@ -5,6 +5,7 @@
> >   */
> >  #include <linux/efi.h>
> >  #include <linux/reboot.h>
> > +#include <linux/ucs2_string.h>
> >
> >  static struct sys_off_handler *efi_sys_off_handler;
> >
> > @@ -14,11 +15,18 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *data)
> >  {
> >         const char *str[] = { "cold", "warm", "shutdown", "platform" };
> >         int efi_mode, cap_reset_mode;
> > +       unsigned long reset_data_sz = 0;
> > +       efi_char16_t *reset_data = NULL;
> >
> >         if (!efi_rt_services_supported(EFI_RT_SUPPORTED_RESET_SYSTEM))
> >                 return;
> >
> >         if (data) {
> > +               reset_data_sz = ucs2_strlen(data) * sizeof(efi_char16_t);
> 
> You can't just run ucs2_strlen() on an arbitrary buffer.
> 
> > +               reset_data = kzalloc(reset_data_sz + 2, GFP_KERNEL);
> > +               memcpy(reset_data, data, reset_data_sz);
> > +               reset_data_sz += 2;
> > +
> 
> What happened to the GUID? It comes after the UTF-16 string, no?

Ah, I missed putting the GUID here.

> 
> >                 efi_mode = EFI_RESET_PLATFORM_SPECIFIC;
> >         } else {
> >                 switch (reboot_mode) {
> > @@ -47,8 +55,7 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *data)
> >                 efi_mode = cap_reset_mode;
> >         }
> >
> > -       efi.reset_system(efi_mode, EFI_SUCCESS, sizeof(data),
> > -                        (efi_char16_t *)data);
> > +       efi.reset_system(efi_mode, EFI_SUCCESS, reset_data_sz, reset_data);
> >  }
> >
> 
> I think the main issue here is tying machine_restart(), which takes a
> u8[] argument, to efi_reboot(), which takes a (u16[]) + L"\0" + GUID
> buffer. So the change to efi_reboot() looks fine to me, we just cannot
> call it directly from machine_restart() as you are suggesting.

It mostly looks like the concerns you are highlighing are related to
random commands being passed to UEFI platform specific reset API. I
suppose this can be addressed using following allow list (based on
analysis done in patch-set [1]) for platform specific reset types. Your
views?

static const efi_platform_reset_type_t platform_reset_types[] = {
        {EFI_RESET_BOOTLOADER_GUID,                     L"bootloader"           },
        {EFI_RESET_DM_VERITY_GUID,                      L"dm-verity-device-corrupted"   },
        {EFI_RESET_EDL_GUID,                            L"edl"                  },
        {EFI_RESET_FASTBOOT_GUID,                       L"fastboot"             },
        {EFI_RESET_LOADER_GUID,                         L"loader"               },
        {EFI_RESET_REBOOT_AB_UPDATE_GUID,               L"reboot-ab-update"     },
        {EFI_RESET_RECOVERY_GUID,                       L"recovery"             },
        {EFI_RESET_RESCUE_GUID,                         L"rescue"               },
        {EFI_RESET_SHUTDOWN_THERMAL_GUID,               L"shutdown-thermal"     },
        {EFI_RESET_SHUTDOWN_THERMAL_BATTERY_GUID,       L"shutdown-thermal-battery"     },
}

[1] https://lore.kernel.org/all/20251109-arm-psci-system_reset2-vendor-reboots-v17-0-46e085bca4cc@oss.qualcomm.com/

-Sumit