On 9/23/20 9:02 PM, Tom Rini wrote: > On Wed, Sep 23, 2020 at 08:51:53PM +0200, Heinrich Schuchardt wrote: >> On 9/23/20 7:53 PM, Mark Kettenis wrote: >>>> Date: Wed, 23 Sep 2020 13:14:09 -0400 >>>> From: Tom Rini <tr...@konsulko.com> >>>> >>>> On Wed, Sep 23, 2020 at 07:01:54PM +0200, Mark Kettenis wrote: >>>>>> From: Michael Walle <mich...@walle.cc> >>>>>> Date: Wed, 23 Sep 2020 18:45:27 +0200 >>>>>> >>>>>> Let the user choose between three different behaviours of the watchdog: >>>>>> (1) Keep the watchdog disabled >>>>>> (2) Supervise u-boot >>>>>> (3) Supervise u-boot and the operating systen (default) >>>>>> >>>>>> Option (2) will disable the watchdog right before handing control to the >>>>>> operating system. This is useful when the OS is not aware of the >>>>>> watchdog. Option (3) doesn't disable the watchdog and assumes the OS >>>>>> will continue servicing. >>>>> >>>>> (3) can't be the default, at least for EFI >>>>> >>>>> The UEFI standard explicitly says that upon calling >>>>> ExitBootServices(), the watchdog timer is disabled. >>>>> >> >> So 3) must not be allowed for CONFIG_EFI_LOADER=y which we have enabled >> on most boards. > > No, EFI is going to need to figure out how to deal with the watchdog. > See below...
Any active watchdog after ExitBootServices() is forbidden by the UEFI spec. 3) does not describe disabling at ExitBootServices(). > >>>>> In general, you can't expect an OS to have support for a particular >>>>> watchdog timer. So (3) only makes sense in cases where U-Boot is >>>>> bundled with an OS image. >>>> >>>> We need to be careful here then. The current and historical / generally >>>> expected behavior is if we've enabled the watchdog we supervise it and >>>> leave it enabled for the OS. Given what UEFI requires I'd like to see >>>> that case handled with a print about disabling the watchdog so it's not >>>> a surprise to the user. I say this because it's a surprise to me and I >>>> guess answers the question of "how does x86 handle this?" I had the >>>> other day. >>> >>> So the UEFI requirement actually is: >>> >>> * Before starting an EFI payload a watchdog timer is started to reset >>> the system after 5 minutes. >>> >>> * This watchdog timer is cancelled as soon as the EFI payload calls >>> ExitBootServices(). >> >> This requirement is currently fulfilled by a software watchdog in >> lib/efi_loader/efi_watchdog.c. We need an emulation because many boards >> don't offer a hardware watchdog in U-Boot. > > OK. > >>> The OpenBSD kernel in general does not supervise the watchdog unless >>> explicitly requested to do so by the user. What may happen is that >>> the driver for the hardware stops the watchdog timer when it attaches, >>> but (a) that is just a side-effect and (b) watchdog timer support >>> isn't implemented for all supported SoCs. >>> >>> The OpenBSD EFI bootloader does explicitly disable the watchdog timer >>> though by calling SetWatchdogTime() as soon as it starts. This is to >>> prevent an automatic reboot if you leave it sitting at the boot prompt >>> for more than 5 minutes. This is done across all our architectures >>> that support EFI, including amd64. So maybe that hides any >>> non-conforming behaviour. >>> >> >> SetWatchdogTime() resets the software watchdog implemented in >> lib/efi_loader/efi_watchdog.c. >> >> When an UEFI payload reads a key via the simple text input protocol or >> the extended simple text input protocol U-Boot calls efi_timer_check() >> which invokes WATCHDOG_RESET(). This is why hardware watchdogs don't >> kill an UEFI application waiting for keyboard input. > > I think we need to make things such that if there's a HW watchdog, our > EFI loader/services make use of it. And maybe just the Kconfig help > text ends up spelling out what the UEFI spec says happens with > watchdogs, so it's documented and expected behavior. There is a symbol CONFIG_WATCHDOG where mentioning UEFI might make sense. But I could not find any software watchdog implementation matching this symbol. Does it exist at all? > My concern is that > it's not obvious to the average user/developer what the behavior is > going to be in this case and that they'll expect a watchdog to fire in a > case where we've turned it off, and that's an important detail that's > just within the spec. > git grep -n hw_watchdog_reset tells us that most boards don't have a hardware watchdog. There is no maintainer for drivers/watchdog/wdt-uclass.c. doc/README.watchdog provides no description on the usage. We do not even have a routine to set a time out period in struct wdt_ops. Hardware watchdogs are already used by some serial drivers. So we cannot independently use them for UEFI. This does not look enticing. Best regards Heinrich
