[PATCH 1/1] efi_loader: stop watchdogs in ExitBootServices()
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
+ /* Disable watchdogs */ + efi_set_watchdog(0); + if IS_ENABLED(CONFIG_WDT) + wdt_stop_all(); + if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE)) @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr); - - /* Give the payload some time to boot */ - efi_set_watchdog(0); - schedule(); out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule();
out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule();
out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule();
out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of 1. After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again. 2. EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
None of those is perfect, but I prefer the latter
Regards /Ilias
-- Tom
Hi,
On Tue, 31 Jan 2023 at 05:04, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule();
out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
None of those is perfect, but I prefer the latter
How does this work if U-Boot runs grub instead of Linux? Does grub update the watchdog?
Regards, Simon
On 1/31/23 15:16, Simon Glass wrote:
Hi,
On Tue, 31 Jan 2023 at 05:04, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
<snip />
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
None of those is perfect, but I prefer the latter
How does this work if U-Boot runs grub instead of Linux? Does grub update the watchdog?
U-Boot will reset the watchdog when the UEFI API is invoked to read the console or to access the network or block devices. Just grep for "efi_timer_check()".
Best regards
Heinrich
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule();
out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule();
out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Unrelated, but does anybody know who "the UEFI specification people" are and how to reach out?
Rasmus
On 2/1/23 09:32, Rasmus Villemoes wrote:
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule(); out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Unrelated, but does anybody know who "the UEFI specification people" are and how to reach out?
Rasmus
After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point.
If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen.
@Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU?
The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system.
Best regards
Heinrich
Hello Heinrich and all,
On Wed, 1 Feb 2023 at 10:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 2/1/23 09:32, Rasmus Villemoes wrote:
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
> The UEFI specification requires for ExitBootServices() that "the boot > services watchdog timer is disabled". We already disable the software > watchdog. We should additionally disable the hardware watchdogs. > > Reported-by: Andre Przywara andre.przywara@arm.com > Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com > --- > lib/efi_loader/efi_boottime.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index ba28989f36..71215af9d2 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -19,6 +19,7 @@ > #include <u-boot/crc.h> > #include <usb.h> > #include <watchdog.h> > +#include <wdt.h> > #include <asm/global_data.h> > #include <asm/setjmp.h> > #include <linux/libfdt_env.h> > @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, > list_del(&evt->link); > } > > + /* Disable watchdogs */ > + efi_set_watchdog(0); > + if IS_ENABLED(CONFIG_WDT) > + wdt_stop_all(); > + > if (!efi_st_keep_devices) { > bootm_disable_interrupts(); > if (IS_ENABLED(CONFIG_USB_DEVICE)) > @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, > > /* Recalculate CRC32 */ > efi_update_table_header_crc32(&systab.hdr); > - > - /* Give the payload some time to boot */ > - efi_set_watchdog(0); > - schedule(); > out: > if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { > if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Unrelated, but does anybody know who "the UEFI specification people" are and how to reach out?
Rasmus
After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point.
If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen.
@Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU?
I guess not all but there are some. Likely related to some chip specific fuse(s), once burnt a watchdog is initially kicked at reset and can't be stopped. We're indeed facing issues with hardware watchdogs timeout capabilities when booting EFI bootloaders or even kernels that may take time to download and install their watchdog driver as a kmod. Extending timeout capabilities looks like the only viable way to address the later case.
BR, Etienne
The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system.
Best regards
Heinrich
Hi,
On Thu, 2 Feb 2023 at 01:17, Etienne Carriere etienne.carriere@linaro.org wrote:
Hello Heinrich and all,
On Wed, 1 Feb 2023 at 10:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 2/1/23 09:32, Rasmus Villemoes wrote:
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote: > On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote: > >> The UEFI specification requires for ExitBootServices() that "the boot >> services watchdog timer is disabled". We already disable the software >> watchdog. We should additionally disable the hardware watchdogs. >> >> Reported-by: Andre Przywara andre.przywara@arm.com >> Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com >> --- >> lib/efi_loader/efi_boottime.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >> index ba28989f36..71215af9d2 100644 >> --- a/lib/efi_loader/efi_boottime.c >> +++ b/lib/efi_loader/efi_boottime.c >> @@ -19,6 +19,7 @@ >> #include <u-boot/crc.h> >> #include <usb.h> >> #include <watchdog.h> >> +#include <wdt.h> >> #include <asm/global_data.h> >> #include <asm/setjmp.h> >> #include <linux/libfdt_env.h> >> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >> list_del(&evt->link); >> } >> >> + /* Disable watchdogs */ >> + efi_set_watchdog(0); >> + if IS_ENABLED(CONFIG_WDT) >> + wdt_stop_all(); >> + >> if (!efi_st_keep_devices) { >> bootm_disable_interrupts(); >> if (IS_ENABLED(CONFIG_USB_DEVICE)) >> @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >> >> /* Recalculate CRC32 */ >> efi_update_table_header_crc32(&systab.hdr); >> - >> - /* Give the payload some time to boot */ >> - efi_set_watchdog(0); >> - schedule(); >> out: >> if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { >> if (ret != EFI_SUCCESS) > > I thought we had rejected going down this path since the UEFI spec is > unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Unrelated, but does anybody know who "the UEFI specification people" are and how to reach out?
Rasmus
After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point.
If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen.
@Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU?
I guess not all but there are some. Likely related to some chip specific fuse(s), once burnt a watchdog is initially kicked at reset and can't be stopped. We're indeed facing issues with hardware watchdogs timeout capabilities when booting EFI bootloaders or even kernels that may take time to download and install their watchdog driver as a kmod. Extending timeout capabilities looks like the only viable way to address the later case.
BR, Etienne
The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system.
Just to point out that this sort of thing is easier with the VBE approach, since the OS can tell U-Boot whether it supports a watchdog or not.
Regards, Simon
On Thu, Feb 02, 2023 at 10:12:07AM -0700, Simon Glass wrote:
Hi,
On Thu, 2 Feb 2023 at 01:17, Etienne Carriere etienne.carriere@linaro.org wrote:
Hello Heinrich and all,
On Wed, 1 Feb 2023 at 10:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 2/1/23 09:32, Rasmus Villemoes wrote:
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote: > On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote: >> On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote: >> >>> The UEFI specification requires for ExitBootServices() that "the boot >>> services watchdog timer is disabled". We already disable the software >>> watchdog. We should additionally disable the hardware watchdogs. >>> >>> Reported-by: Andre Przywara andre.przywara@arm.com >>> Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com >>> --- >>> lib/efi_loader/efi_boottime.c | 10 ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>> index ba28989f36..71215af9d2 100644 >>> --- a/lib/efi_loader/efi_boottime.c >>> +++ b/lib/efi_loader/efi_boottime.c >>> @@ -19,6 +19,7 @@ >>> #include <u-boot/crc.h> >>> #include <usb.h> >>> #include <watchdog.h> >>> +#include <wdt.h> >>> #include <asm/global_data.h> >>> #include <asm/setjmp.h> >>> #include <linux/libfdt_env.h> >>> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>> list_del(&evt->link); >>> } >>> >>> + /* Disable watchdogs */ >>> + efi_set_watchdog(0); >>> + if IS_ENABLED(CONFIG_WDT) >>> + wdt_stop_all(); >>> + >>> if (!efi_st_keep_devices) { >>> bootm_disable_interrupts(); >>> if (IS_ENABLED(CONFIG_USB_DEVICE)) >>> @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>> >>> /* Recalculate CRC32 */ >>> efi_update_table_header_crc32(&systab.hdr); >>> - >>> - /* Give the payload some time to boot */ >>> - efi_set_watchdog(0); >>> - schedule(); >>> out: >>> if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { >>> if (ret != EFI_SUCCESS) >> >> I thought we had rejected going down this path since the UEFI spec is >> unhelpfully wrong if it insists this? > > Because, to be clear, stopping hardware watchdogs is not to be done. The > one in-tree caller of wdt_stop_all is very questionable. You cannot > seriously stop a watchdog until someone else can hopefully resume it as > that violates the function of a hardware watchdog. A pure software > watchdog is one thing, and a hardware watchdog is another. I feel like > the most likely answer here is that someone needs to, still, push back > to the UEFI specification to get hardware watchdogs better understood > and handled, as it must never be stopped once started and if you cannot > reach the next stage in time, that's an engineering issue to resolve. My > first guess is that ExitBootServices should service the watchdog one > last time to ensure the largest window of time for the OS to take over > servicing of the watchdog. >
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Unrelated, but does anybody know who "the UEFI specification people" are and how to reach out?
Rasmus
After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point.
If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen.
@Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU?
I guess not all but there are some. Likely related to some chip specific fuse(s), once burnt a watchdog is initially kicked at reset and can't be stopped. We're indeed facing issues with hardware watchdogs timeout capabilities when booting EFI bootloaders or even kernels that may take time to download and install their watchdog driver as a kmod. Extending timeout capabilities looks like the only viable way to address the later case.
BR, Etienne
The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system.
Just to point out that this sort of thing is easier with the VBE approach, since the OS can tell U-Boot whether it supports a watchdog or not.
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
Hi Tom,
On Thu, 2 Feb 2023 at 10:22, Tom Rini trini@konsulko.com wrote:
On Thu, Feb 02, 2023 at 10:12:07AM -0700, Simon Glass wrote:
Hi,
On Thu, 2 Feb 2023 at 01:17, Etienne Carriere etienne.carriere@linaro.org wrote:
Hello Heinrich and all,
On Wed, 1 Feb 2023 at 10:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 2/1/23 09:32, Rasmus Villemoes wrote:
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote: > Hi all, > > On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote: >> On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote: >>> On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote: >>> >>>> The UEFI specification requires for ExitBootServices() that "the boot >>>> services watchdog timer is disabled". We already disable the software >>>> watchdog. We should additionally disable the hardware watchdogs. >>>> >>>> Reported-by: Andre Przywara andre.przywara@arm.com >>>> Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com >>>> --- >>>> lib/efi_loader/efi_boottime.c | 10 ++++++---- >>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>>> index ba28989f36..71215af9d2 100644 >>>> --- a/lib/efi_loader/efi_boottime.c >>>> +++ b/lib/efi_loader/efi_boottime.c >>>> @@ -19,6 +19,7 @@ >>>> #include <u-boot/crc.h> >>>> #include <usb.h> >>>> #include <watchdog.h> >>>> +#include <wdt.h> >>>> #include <asm/global_data.h> >>>> #include <asm/setjmp.h> >>>> #include <linux/libfdt_env.h> >>>> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>>> list_del(&evt->link); >>>> } >>>> >>>> + /* Disable watchdogs */ >>>> + efi_set_watchdog(0); >>>> + if IS_ENABLED(CONFIG_WDT) >>>> + wdt_stop_all(); >>>> + >>>> if (!efi_st_keep_devices) { >>>> bootm_disable_interrupts(); >>>> if (IS_ENABLED(CONFIG_USB_DEVICE)) >>>> @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>>> >>>> /* Recalculate CRC32 */ >>>> efi_update_table_header_crc32(&systab.hdr); >>>> - >>>> - /* Give the payload some time to boot */ >>>> - efi_set_watchdog(0); >>>> - schedule(); >>>> out: >>>> if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { >>>> if (ret != EFI_SUCCESS) >>> >>> I thought we had rejected going down this path since the UEFI spec is >>> unhelpfully wrong if it insists this? >> >> Because, to be clear, stopping hardware watchdogs is not to be done. The >> one in-tree caller of wdt_stop_all is very questionable. You cannot >> seriously stop a watchdog until someone else can hopefully resume it as >> that violates the function of a hardware watchdog. A pure software >> watchdog is one thing, and a hardware watchdog is another. I feel like >> the most likely answer here is that someone needs to, still, push back >> to the UEFI specification to get hardware watchdogs better understood >> and handled, as it must never be stopped once started and if you cannot >> reach the next stage in time, that's an engineering issue to resolve. My >> first guess is that ExitBootServices should service the watchdog one >> last time to ensure the largest window of time for the OS to take over >> servicing of the watchdog. >> > > There's two scenarios I can think of > 1. After U-Boot is done it can disable the hardware watchdog. > The kernel will go through the EFI-stub -> kernel proper -> watchdog > gets re-initialized. In that case you are *hoping* that device won't > hang in the efi-stub or until the wd is up again. > 2. EFI makes sure the hardware wd gets configured with the highest allowed > value. The efi-stub doesn't have any driver to refresh the wd, so we > will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Unrelated, but does anybody know who "the UEFI specification people" are and how to reach out?
Rasmus
After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point.
If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen.
@Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU?
I guess not all but there are some. Likely related to some chip specific fuse(s), once burnt a watchdog is initially kicked at reset and can't be stopped. We're indeed facing issues with hardware watchdogs timeout capabilities when booting EFI bootloaders or even kernels that may take time to download and install their watchdog driver as a kmod. Extending timeout capabilities looks like the only viable way to address the later case.
BR, Etienne
The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system.
Just to point out that this sort of thing is easier with the VBE approach, since the OS can tell U-Boot whether it supports a watchdog or not.
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
Regards, Simon
On 03/02/2023 03.15, Simon Glass wrote:
Hi Tom,
On Thu, 2 Feb 2023 at 10:22, Tom Rini trini@konsulko.com wrote:
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
_Every single_ custom piece of industrial (as opposed to consumer-grade) hardware I've worked on as a consultant has had an external, always-running, gpio-petted watchdog. It's simply just something that the hardware designers include, and in some cases that's even due to certification requirements. So an always-running, cannot-be-turned-off, watchdog is a real thing, in real hardware, and if specs don't account for that, well, the spec is just paper, and we can ignore it.
As for debugging and bringup, I've seen various solutions (depending on the actual watchdog chip). Usually there's some way to place a jumper that will either feed the watchdog from some, say, 32kHz output from an RTC or elsewhere, or place a jumper to pull up/pull down some enable/disable pin to the watchdog chip. IOW, when you have physical access to the PCB lying on your desk, you can disable the watchdog, but there's no way to do that in the field or in production.
Rasmus
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
_Every single_ custom piece of industrial (as opposed to consumer-grade) hardware I've worked on as a consultant has had an external, always-running, gpio-petted watchdog. It's simply just something that the hardware designers include, and in some cases that's even due to certification requirements. So an always-running, cannot-be-turned-off, watchdog is a real thing, in real hardware, and if specs don't account for that, well, the spec is just paper, and we can ignore it.
I agree. But on the other hand, you cannot assume or force the OS to have a watchdog driver in the general case - which is as I understand it - one goal of EFI.
Obviously, there are watchdogs that can be disabled and some which cannot. I don't want to argue about the advantages and disadvantages.
For watchdogs which cannot be turned off, we can't really do anything anyway after the handoff to the OS - except increasing its timeout if thats possible.
For watchdogs that can be disabled (and are enabled in u-boot of course), there seems to be two use-cases: (1) embedded EFI boot, that is you know exactly what you are booting, i.e. self compiled kernel with a watchdog driver (2) booting a general OS via EFI, think of a debian boot CD for example.
I agree, that for (1) the watchdog shouldn't be disabled. For (2) you cannot assume the booting OS has a driver for the watchdog, let it be an older version of a distribution which just haven't the SoC watchdog driver enabled or maybe because there is no driver for it at all (yet).
Is there a way, to have the watchdog disabled for case (2) while also having the possibity to use bootm/booti/bootz and keep the watchdog enabled? Basically I want the following:
(1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled)
The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board.
-michael
On 2/7/23 15:59, Michael Walle wrote:
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
_Every single_ custom piece of industrial (as opposed to consumer-grade) hardware I've worked on as a consultant has had an external, always-running, gpio-petted watchdog. It's simply just something that the hardware designers include, and in some cases that's even due to certification requirements. So an always-running, cannot-be-turned-off, watchdog is a real thing, in real hardware, and if specs don't account for that, well, the spec is just paper, and we can ignore it.
I agree. But on the other hand, you cannot assume or force the OS to have a watchdog driver in the general case - which is as I understand it - one goal of EFI.
Obviously, there are watchdogs that can be disabled and some which cannot. I don't want to argue about the advantages and disadvantages.
For watchdogs which cannot be turned off, we can't really do anything anyway after the handoff to the OS - except increasing its timeout if thats possible.
For watchdogs that can be disabled (and are enabled in u-boot of course), there seems to be two use-cases: (1) embedded EFI boot, that is you know exactly what you are booting, i.e. self compiled kernel with a watchdog driver (2) booting a general OS via EFI, think of a debian boot CD for example.
I agree, that for (1) the watchdog shouldn't be disabled. For (2) you cannot assume the booting OS has a driver for the watchdog, let it be an older version of a distribution which just haven't the SoC watchdog driver enabled or maybe because there is no driver for it at all (yet).
Is there a way, to have the watchdog disabled for case (2) while also having the possibity to use bootm/booti/bootz and keep the watchdog enabled? Basically I want the following:
(1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled)
The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board.
-michael
For SUNXI boards disabling CONFIG_WATCHDOG_AUTOSTART solved the problem with the very short maximum expiration time of the watchdog.
Best regards
Heinrich
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
_Every single_ custom piece of industrial (as opposed to consumer-grade) hardware I've worked on as a consultant has had an external, always-running, gpio-petted watchdog. It's simply just something that the hardware designers include, and in some cases that's even due to certification requirements. So an always-running, cannot-be-turned-off, watchdog is a real thing, in real hardware, and if specs don't account for that, well, the spec is just paper, and we can ignore it.
I agree. But on the other hand, you cannot assume or force the OS to have a watchdog driver in the general case - which is as I understand it - one goal of EFI.
Obviously, there are watchdogs that can be disabled and some which cannot. I don't want to argue about the advantages and disadvantages.
For watchdogs which cannot be turned off, we can't really do anything anyway after the handoff to the OS - except increasing its timeout if thats possible.
For watchdogs that can be disabled (and are enabled in u-boot of course), there seems to be two use-cases: (1) embedded EFI boot, that is you know exactly what you are booting, i.e. self compiled kernel with a watchdog driver (2) booting a general OS via EFI, think of a debian boot CD for example.
I agree, that for (1) the watchdog shouldn't be disabled. For (2) you cannot assume the booting OS has a driver for the watchdog, let it be an older version of a distribution which just haven't the SoC watchdog driver enabled or maybe because there is no driver for it at all (yet).
Is there a way, to have the watchdog disabled for case (2) while also having the possibity to use bootm/booti/bootz and keep the watchdog enabled? Basically I want the following:
(1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled)
The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board.
-michael
For SUNXI boards disabling CONFIG_WATCHDOG_AUTOSTART solved the problem with the very short maximum expiration time of the watchdog.
I can't follow you here. What "very short maximum expiration time"? With CONFIG_WATCHDOG_AUTOSTART disabled, the watchdog won't be kicked by u-boot, right? wdt->running will never be set to true and wdt_cyclic() will be a noop.
-michael
On 2/7/23 16:29, Michael Walle wrote:
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
_Every single_ custom piece of industrial (as opposed to consumer-grade) hardware I've worked on as a consultant has had an external, always-running, gpio-petted watchdog. It's simply just something that the hardware designers include, and in some cases that's even due to certification requirements. So an always-running, cannot-be-turned-off, watchdog is a real thing, in real hardware, and if specs don't account for that, well, the spec is just paper, and we can ignore it.
I agree. But on the other hand, you cannot assume or force the OS to have a watchdog driver in the general case - which is as I understand it - one goal of EFI.
Obviously, there are watchdogs that can be disabled and some which cannot. I don't want to argue about the advantages and disadvantages.
For watchdogs which cannot be turned off, we can't really do anything anyway after the handoff to the OS - except increasing its timeout if thats possible.
For watchdogs that can be disabled (and are enabled in u-boot of course), there seems to be two use-cases: (1) embedded EFI boot, that is you know exactly what you are booting, i.e. self compiled kernel with a watchdog driver (2) booting a general OS via EFI, think of a debian boot CD for example.
I agree, that for (1) the watchdog shouldn't be disabled. For (2) you cannot assume the booting OS has a driver for the watchdog, let it be an older version of a distribution which just haven't the SoC watchdog driver enabled or maybe because there is no driver for it at all (yet).
Is there a way, to have the watchdog disabled for case (2) while also having the possibity to use bootm/booti/bootz and keep the watchdog enabled? Basically I want the following:
(1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled)
The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board.
-michael
For SUNXI boards disabling CONFIG_WATCHDOG_AUTOSTART solved the problem with the very short maximum expiration time of the watchdog.
I can't follow you here. What "very short maximum expiration time"? With CONFIG_WATCHDOG_AUTOSTART disabled, the watchdog won't be kicked by u-boot, right? wdt->running will never be set to true and wdt_cyclic() will be a noop.
-michael
The sunxi boards failed to boot with CONFIG_WATCHDOG_AUTOSTART because 16s is too short for Linux to install a watchdog driver. With CONFIG_WATCHDOG_AUTOSTART=n the watchdog is not running and the boards boot.
Best regards
Heinrich
Basically I want the following:
(1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled)
The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board.
For SUNXI boards disabling CONFIG_WATCHDOG_AUTOSTART solved the problem with the very short maximum expiration time of the watchdog.
I can't follow you here. What "very short maximum expiration time"? With CONFIG_WATCHDOG_AUTOSTART disabled, the watchdog won't be kicked by u-boot, right? wdt->running will never be set to true and wdt_cyclic() will be a noop.
The sunxi boards failed to boot with CONFIG_WATCHDOG_AUTOSTART because 16s is too short for Linux to install a watchdog driver. With CONFIG_WATCHDOG_AUTOSTART=n the watchdog is not running and the boards boot.
But how does that help in my case?
-michael
On Thu, Feb 02, 2023 at 07:15:35PM -0700, Simon Glass wrote:
Hi Tom,
On Thu, 2 Feb 2023 at 10:22, Tom Rini trini@konsulko.com wrote:
On Thu, Feb 02, 2023 at 10:12:07AM -0700, Simon Glass wrote:
Hi,
On Thu, 2 Feb 2023 at 01:17, Etienne Carriere etienne.carriere@linaro.org wrote:
Hello Heinrich and all,
On Wed, 1 Feb 2023 at 10:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 2/1/23 09:32, Rasmus Villemoes wrote:
On 31/01/2023 16.07, Tom Rini wrote: > On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote: >> Hi all, >> >> On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote: >>> On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote: >>>> On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote: >>>> >>>>> The UEFI specification requires for ExitBootServices() that "the boot >>>>> services watchdog timer is disabled". We already disable the software >>>>> watchdog. We should additionally disable the hardware watchdogs. >>>>> >>>>> Reported-by: Andre Przywara andre.przywara@arm.com >>>>> Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com >>>>> --- >>>>> lib/efi_loader/efi_boottime.c | 10 ++++++---- >>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>>>> index ba28989f36..71215af9d2 100644 >>>>> --- a/lib/efi_loader/efi_boottime.c >>>>> +++ b/lib/efi_loader/efi_boottime.c >>>>> @@ -19,6 +19,7 @@ >>>>> #include <u-boot/crc.h> >>>>> #include <usb.h> >>>>> #include <watchdog.h> >>>>> +#include <wdt.h> >>>>> #include <asm/global_data.h> >>>>> #include <asm/setjmp.h> >>>>> #include <linux/libfdt_env.h> >>>>> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>>>> list_del(&evt->link); >>>>> } >>>>> >>>>> + /* Disable watchdogs */ >>>>> + efi_set_watchdog(0); >>>>> + if IS_ENABLED(CONFIG_WDT) >>>>> + wdt_stop_all(); >>>>> + >>>>> if (!efi_st_keep_devices) { >>>>> bootm_disable_interrupts(); >>>>> if (IS_ENABLED(CONFIG_USB_DEVICE)) >>>>> @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>>>> >>>>> /* Recalculate CRC32 */ >>>>> efi_update_table_header_crc32(&systab.hdr); >>>>> - >>>>> - /* Give the payload some time to boot */ >>>>> - efi_set_watchdog(0); >>>>> - schedule(); >>>>> out: >>>>> if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { >>>>> if (ret != EFI_SUCCESS) >>>> >>>> I thought we had rejected going down this path since the UEFI spec is >>>> unhelpfully wrong if it insists this? >>> >>> Because, to be clear, stopping hardware watchdogs is not to be done. The >>> one in-tree caller of wdt_stop_all is very questionable. You cannot >>> seriously stop a watchdog until someone else can hopefully resume it as >>> that violates the function of a hardware watchdog. A pure software >>> watchdog is one thing, and a hardware watchdog is another. I feel like >>> the most likely answer here is that someone needs to, still, push back >>> to the UEFI specification to get hardware watchdogs better understood >>> and handled, as it must never be stopped once started and if you cannot >>> reach the next stage in time, that's an engineering issue to resolve. My >>> first guess is that ExitBootServices should service the watchdog one >>> last time to ensure the largest window of time for the OS to take over >>> servicing of the watchdog. >>> >> >> There's two scenarios I can think of >> 1. After U-Boot is done it can disable the hardware watchdog. >> The kernel will go through the EFI-stub -> kernel proper -> watchdog >> gets re-initialized. In that case you are *hoping* that device won't >> hang in the efi-stub or until the wd is up again. >> 2. EFI makes sure the hardware wd gets configured with the highest allowed >> value. The efi-stub doesn't have any driver to refresh the wd, so we >> will again rely on the wd driver coming up and refreshing the timers. > > You cannot stop the hardware watchdog, period. I think in the previous > thread about this it was noted that some hardware watchdogs cannot be > disabled, it's not function that the watchdog supports. Someone needs to > go and talk with the UEFI specification people and get this addressed. > There is no sane path for "disable the hardware watchdog". >
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Unrelated, but does anybody know who "the UEFI specification people" are and how to reach out?
Rasmus
After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point.
If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen.
@Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU?
I guess not all but there are some. Likely related to some chip specific fuse(s), once burnt a watchdog is initially kicked at reset and can't be stopped. We're indeed facing issues with hardware watchdogs timeout capabilities when booting EFI bootloaders or even kernels that may take time to download and install their watchdog driver as a kmod. Extending timeout capabilities looks like the only viable way to address the later case.
BR, Etienne
The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system.
Just to point out that this sort of thing is easier with the VBE approach, since the OS can tell U-Boot whether it supports a watchdog or not.
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
Same way we have for the last 20+ years? This isn't a new thing by far.
Hi,
On Fri, 3 Feb 2023 at 08:51, Tom Rini trini@konsulko.com wrote:
On Thu, Feb 02, 2023 at 07:15:35PM -0700, Simon Glass wrote:
Hi Tom,
On Thu, 2 Feb 2023 at 10:22, Tom Rini trini@konsulko.com wrote:
On Thu, Feb 02, 2023 at 10:12:07AM -0700, Simon Glass wrote:
Hi,
On Thu, 2 Feb 2023 at 01:17, Etienne Carriere etienne.carriere@linaro.org wrote:
Hello Heinrich and all,
On Wed, 1 Feb 2023 at 10:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 2/1/23 09:32, Rasmus Villemoes wrote: > On 31/01/2023 16.07, Tom Rini wrote: >> On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote: >>> Hi all, >>> >>> On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote: >>>> On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote: >>>>> On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote: >>>>> >>>>>> The UEFI specification requires for ExitBootServices() that "the boot >>>>>> services watchdog timer is disabled". We already disable the software >>>>>> watchdog. We should additionally disable the hardware watchdogs. >>>>>> >>>>>> Reported-by: Andre Przywara andre.przywara@arm.com >>>>>> Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com >>>>>> --- >>>>>> lib/efi_loader/efi_boottime.c | 10 ++++++---- >>>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>>>>> index ba28989f36..71215af9d2 100644 >>>>>> --- a/lib/efi_loader/efi_boottime.c >>>>>> +++ b/lib/efi_loader/efi_boottime.c >>>>>> @@ -19,6 +19,7 @@ >>>>>> #include <u-boot/crc.h> >>>>>> #include <usb.h> >>>>>> #include <watchdog.h> >>>>>> +#include <wdt.h> >>>>>> #include <asm/global_data.h> >>>>>> #include <asm/setjmp.h> >>>>>> #include <linux/libfdt_env.h> >>>>>> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>>>>> list_del(&evt->link); >>>>>> } >>>>>> >>>>>> + /* Disable watchdogs */ >>>>>> + efi_set_watchdog(0); >>>>>> + if IS_ENABLED(CONFIG_WDT) >>>>>> + wdt_stop_all(); >>>>>> + >>>>>> if (!efi_st_keep_devices) { >>>>>> bootm_disable_interrupts(); >>>>>> if (IS_ENABLED(CONFIG_USB_DEVICE)) >>>>>> @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, >>>>>> >>>>>> /* Recalculate CRC32 */ >>>>>> efi_update_table_header_crc32(&systab.hdr); >>>>>> - >>>>>> - /* Give the payload some time to boot */ >>>>>> - efi_set_watchdog(0); >>>>>> - schedule(); >>>>>> out: >>>>>> if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { >>>>>> if (ret != EFI_SUCCESS) >>>>> >>>>> I thought we had rejected going down this path since the UEFI spec is >>>>> unhelpfully wrong if it insists this? >>>> >>>> Because, to be clear, stopping hardware watchdogs is not to be done. The >>>> one in-tree caller of wdt_stop_all is very questionable. You cannot >>>> seriously stop a watchdog until someone else can hopefully resume it as >>>> that violates the function of a hardware watchdog. A pure software >>>> watchdog is one thing, and a hardware watchdog is another. I feel like >>>> the most likely answer here is that someone needs to, still, push back >>>> to the UEFI specification to get hardware watchdogs better understood >>>> and handled, as it must never be stopped once started and if you cannot >>>> reach the next stage in time, that's an engineering issue to resolve. My >>>> first guess is that ExitBootServices should service the watchdog one >>>> last time to ensure the largest window of time for the OS to take over >>>> servicing of the watchdog. >>>> >>> >>> There's two scenarios I can think of >>> 1. After U-Boot is done it can disable the hardware watchdog. >>> The kernel will go through the EFI-stub -> kernel proper -> watchdog >>> gets re-initialized. In that case you are *hoping* that device won't >>> hang in the efi-stub or until the wd is up again. >>> 2. EFI makes sure the hardware wd gets configured with the highest allowed >>> value. The efi-stub doesn't have any driver to refresh the wd, so we >>> will again rely on the wd driver coming up and refreshing the timers. >> >> You cannot stop the hardware watchdog, period. I think in the previous >> thread about this it was noted that some hardware watchdogs cannot be >> disabled, it's not function that the watchdog supports. Someone needs to >> go and talk with the UEFI specification people and get this addressed. >> There is no sane path for "disable the hardware watchdog". >> > > Indeed. > > But I think one reasonable thing to do would be to say "ok, the payload > is now ready to assume responsibility, so on the U-Boot side we stop > _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering > them from the cyclic framework), even if the payload still performs > calls into U-Boot where we would otherwise use the opportunity to feed > the watchdog. And of course it's reasonable in that case to do one last > ping. Because it's also a recipe for disaster if, say, both the payload > and U-Boot toggles the same gpio or frobs the same SOC registers. > > Unrelated, but does anybody know who "the UEFI specification people" are > and how to reach out? > > Rasmus >
After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point.
If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen.
@Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU?
I guess not all but there are some. Likely related to some chip specific fuse(s), once burnt a watchdog is initially kicked at reset and can't be stopped. We're indeed facing issues with hardware watchdogs timeout capabilities when booting EFI bootloaders or even kernels that may take time to download and install their watchdog driver as a kmod. Extending timeout capabilities looks like the only viable way to address the later case.
BR, Etienne
The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system.
Just to point out that this sort of thing is easier with the VBE approach, since the OS can tell U-Boot whether it supports a watchdog or not.
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off.
I hope not, that sounds really risky. How would you debug such a platform?
Same way we have for the last 20+ years? This isn't a new thing by far.
I mean, when you pause U-Boot with your JTAG debugger, how does it avoid resetting and losing your progress? On-chip watchdogs handle this, but if it is something that is started externally to the chip and generates a reset signal for the system, then I don't see how it can work.
Regards, Simon
Date: Wed, 1 Feb 2023 09:32:54 +0100 From: Rasmus Villemoes rasmus.villemoes@prevas.dk
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
The UEFI specification requires for ExitBootServices() that "the boot services watchdog timer is disabled". We already disable the software watchdog. We should additionally disable the hardware watchdogs.
Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_boottime.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ba28989f36..71215af9d2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -19,6 +19,7 @@ #include <u-boot/crc.h> #include <usb.h> #include <watchdog.h> +#include <wdt.h> #include <asm/global_data.h> #include <asm/setjmp.h> #include <linux/libfdt_env.h> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, list_del(&evt->link); }
- /* Disable watchdogs */
- efi_set_watchdog(0);
- if IS_ENABLED(CONFIG_WDT)
wdt_stop_all();- if (!efi_st_keep_devices) { bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE))
@@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
/* Recalculate CRC32 */ efi_update_table_header_crc32(&systab.hdr);
- /* Give the payload some time to boot */
- efi_set_watchdog(0);
- schedule();
out: if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Well, for EFI the point where the handover happens is when the payload calls ExitBootServices(). After that point the payload "owns" the hardware exposed to it in the device tree. And the only way for the payload to call into U-Boot at that point is through EFI runtime services, which are not supposed to touch the hardware that is now under control of the payload. With the exception of the ResetSystem() EFI runtime service of course. But that call will never return and therefore souldn't lead to any conflicts. So nothing will pet the watchdog anymore.
The discussion is about what exactly should happen to the watchdog when ExitBootServices() gets called. The UEFI specification says that the (unspecified) watchdog should be disabled at that point. Which is something that makes no sense to many people with experience in embedded systems and may even be technically impossible on some hardware. So U-Boot doesn't follow the spec in this regard.
What U-Boot currently does actually works just fine in most cases. You just have to make sure that the payload takes over the petting of the watchdog in a timely fashion after calling ExitBootServices(). But that is not really different from what happens if you boot a Linux kernel using one of the "legacy" boot methods that U-Boot provides (e.g. using the "bootm" command).
The "problem" here really is that every 6 months somebody fails to include the watchdog driver in their Linux kernel, reads the UEFI spec, decides U-Boot is violating the spec and comes up with a diff to "fix" U-Boot. And then we have the same discussion again. The "solution" to that probablem is to get the UEFI spec changes to allow for U-Boot's behaviour. But nobody here cares enough about that to actually makes that happen.
On Wed, Feb 01, 2023 at 01:49:58PM +0100, Mark Kettenis wrote:
Date: Wed, 1 Feb 2023 09:32:54 +0100 From: Rasmus Villemoes rasmus.villemoes@prevas.dk
On 31/01/2023 16.07, Tom Rini wrote:
On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote:
Hi all,
On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote:
On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote:
On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:
> The UEFI specification requires for ExitBootServices() that "the boot > services watchdog timer is disabled". We already disable the software > watchdog. We should additionally disable the hardware watchdogs. > > Reported-by: Andre Przywara andre.przywara@arm.com > Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com > --- > lib/efi_loader/efi_boottime.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index ba28989f36..71215af9d2 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -19,6 +19,7 @@ > #include <u-boot/crc.h> > #include <usb.h> > #include <watchdog.h> > +#include <wdt.h> > #include <asm/global_data.h> > #include <asm/setjmp.h> > #include <linux/libfdt_env.h> > @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, > list_del(&evt->link); > } > > + /* Disable watchdogs */ > + efi_set_watchdog(0); > + if IS_ENABLED(CONFIG_WDT) > + wdt_stop_all(); > + > if (!efi_st_keep_devices) { > bootm_disable_interrupts(); > if (IS_ENABLED(CONFIG_USB_DEVICE)) > @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, > > /* Recalculate CRC32 */ > efi_update_table_header_crc32(&systab.hdr); > - > - /* Give the payload some time to boot */ > - efi_set_watchdog(0); > - schedule(); > out: > if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) { > if (ret != EFI_SUCCESS)
I thought we had rejected going down this path since the UEFI spec is unhelpfully wrong if it insists this?
Because, to be clear, stopping hardware watchdogs is not to be done. The one in-tree caller of wdt_stop_all is very questionable. You cannot seriously stop a watchdog until someone else can hopefully resume it as that violates the function of a hardware watchdog. A pure software watchdog is one thing, and a hardware watchdog is another. I feel like the most likely answer here is that someone needs to, still, push back to the UEFI specification to get hardware watchdogs better understood and handled, as it must never be stopped once started and if you cannot reach the next stage in time, that's an engineering issue to resolve. My first guess is that ExitBootServices should service the watchdog one last time to ensure the largest window of time for the OS to take over servicing of the watchdog.
There's two scenarios I can think of
- After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again.
- EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers.
You cannot stop the hardware watchdog, period. I think in the previous thread about this it was noted that some hardware watchdogs cannot be disabled, it's not function that the watchdog supports. Someone needs to go and talk with the UEFI specification people and get this addressed. There is no sane path for "disable the hardware watchdog".
Indeed.
But I think one reasonable thing to do would be to say "ok, the payload is now ready to assume responsibility, so on the U-Boot side we stop _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering them from the cyclic framework), even if the payload still performs calls into U-Boot where we would otherwise use the opportunity to feed the watchdog. And of course it's reasonable in that case to do one last ping. Because it's also a recipe for disaster if, say, both the payload and U-Boot toggles the same gpio or frobs the same SOC registers.
Well, for EFI the point where the handover happens is when the payload calls ExitBootServices(). After that point the payload "owns" the hardware exposed to it in the device tree. And the only way for the payload to call into U-Boot at that point is through EFI runtime services, which are not supposed to touch the hardware that is now under control of the payload. With the exception of the ResetSystem() EFI runtime service of course. But that call will never return and therefore souldn't lead to any conflicts. So nothing will pet the watchdog anymore.
The discussion is about what exactly should happen to the watchdog when ExitBootServices() gets called. The UEFI specification says that the (unspecified) watchdog should be disabled at that point. Which is something that makes no sense to many people with experience in embedded systems and may even be technically impossible on some hardware. So U-Boot doesn't follow the spec in this regard.
What U-Boot currently does actually works just fine in most cases. You just have to make sure that the payload takes over the petting of the watchdog in a timely fashion after calling ExitBootServices(). But that is not really different from what happens if you boot a Linux kernel using one of the "legacy" boot methods that U-Boot provides (e.g. using the "bootm" command).
The "problem" here really is that every 6 months somebody fails to include the watchdog driver in their Linux kernel, reads the UEFI spec, decides U-Boot is violating the spec and comes up with a diff to "fix" U-Boot. And then we have the same discussion again. The "solution" to that probablem is to get the UEFI spec changes to allow for U-Boot's behaviour. But nobody here cares enough about that to actually makes that happen.
Yes, this. And I am hoping that at this point I can motivate one of the many people that do already have contacts with, and accounts in the various UEFI forums and are otherwise familiar with the processes to get this addressed. There are groups that want UEFI to be a serious choice in embedded usage and fixing the spec to understand how hardware watchdogs are used in the embedded space for very good reason will benefit them. I'm quite happy to have U-Boot continue to violate this specific part of the specification until then.
participants (8)
-
Etienne Carriere -
Heinrich Schuchardt -
Ilias Apalodimas -
Mark Kettenis -
Michael Walle -
Rasmus Villemoes -
Simon Glass -
Tom Rini