[PATCH 2/2] watchdog: add watchdog behavior configuration

[Heinrich Schuchardt]

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 <trini at konsulko.com>
>>>>
>>>> On Wed, Sep 23, 2020 at 07:01:54PM +0200, Mark Kettenis wrote:
>>>>>> From: Michael Walle <michael at 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