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

Wed Sep 23 23:09:51 CEST 2020

On Wed, Sep 23, 2020 at 10:58:00PM +0200, Heinrich Schuchardt wrote:
> On 9/23/20 10:23 PM, Tom Rini wrote:
> > On Wed, Sep 23, 2020 at 10:01:29PM +0200, Heinrich Schuchardt wrote:
> >> 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().
> >
> > Well, that's why I go back and forth on having the EFI loader say it's
> > changing the current watchdog behavior.
> >
> > But just because EFI loader is enabled by default doesn't mean it gets
> > to set the overall default behavior for watchdogs.  It's an option in a
> > lot of cases where it's not actually used and a traditional method is
> > instead used.
> >
> >>>>>>> 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?
> >
> > Do we have a pure software watchdog?  I'm not 100% sure off-hand.
> >
> >>> 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.
> >
> > That's the non-DM function.  And looking at where we have drivers for
> > SoC watchdogs under drivers/watchdog/ I disagree with "most".  We keep
> > adding more, even.
> >
> >> There is no maintainer for drivers/watchdog/wdt-uclass.c.
> >
> > Yes, just general "Is DM, check with Simon".
> >
> >> doc/README.watchdog provides no description on the usage.
> >
> > Given the amount of docs you've been writing of late (thanks!) yes,
> > neither of us are surprised docs aren't in sync with implementation
> > right now.
> >
> >> We do not even have a routine to set a time out period in struct wdt_ops.
> >
> > Changing the timeout at run time?
> 
> That is what EFI_BOOT_SERVICES.SetWatchdogTimer() is about.
> 
> The UEFI application can set the watchdog timeout to a multiple of 1
> second (without upper limit) or disable it completely.
> 
> But this must not interfere with our serial drivers which are using
> hardware watchdogs. And now look at
> 
> include/configs/M5373EVB.h:27:
> #define CONFIG_WATCHDOG_TIMEOUT   3360
> /* timeout in ms, max is 3.36 sec */

Yes, but that's also not a UEFI-possible platform.  But even the i.MX
watchdog seems to have a 128 second timeout as the default/max.

> The current use of hardware watchdog does not fit well to UEFI.
> 
> A possible design would be a 100 Hz watchdog, to which U-Boot drivers
> can add listeners.

Maybe we need to step back and ask what UEFI says about how to handle
hardware watchdogs.  They can't be as rare as it seems right now in the
x86 server world.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200923/e6594820/attachment.sig>