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

[Tom Rini]

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...

> >>> 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.  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.

-- 
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/38ac22f5/attachment.sig>