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

[Tom Rini]

On Thu, Sep 24, 2020 at 11:14:36PM +0200, Michael Walle wrote:
> Am 2020-09-24 22:58, schrieb Mark Kettenis:
> > > Am 2020-09-24 15:19, schrieb Tom Rini:
> > > > On Thu, Sep 24, 2020 at 09:33:50AM +0200, Michael Walle wrote:
> > > >> Am 2020-09-23 19:35, schrieb Tom Rini:
> > > [..]
> > > 
> > > >> > > Not printf(), maybe log_info().
> > > >> > >
> > > >> > > The disabling has to occur in ExitBootServices() (aka.
> > > >> > > efi_exit_boot_services()). Here we are in the middle of an executing
> > > >> > > UEFI application. Printing anything on the screen may mess up the
> > > >> > > output
> > > >> > > of the UEFI application.
> > > >> > >
> > > >> > > So, please, don't output anything.
> > > >> >
> > > >> > We need to find a good way to inform the user we're disabling their
> > > >> > watchdog.  Maybe before we fully jump in to UEFI note that it will be
> > > >> > disabled before entering the OS?  Or something a bit more generally
> > > >> > understood than ExitBootServices() having been called.  I don't know
> > > >> > _where_ the best place is, but I think it's important to inform the
> > > >> > user.
> > > >>
> > > >> The watchdog is only disabled in the "supervise u-boot" mode, why
> > > >> would we need to inform the user? It was the users choice to have
> > > >> the timer only enabled in u-boot.
> > > >>
> > > >> Or do you mean if for example the vendor chooses that option and
> > > >> in this case the user doesn't know anything about it? The mode
> > > >> is indicated in the "WDT:" output.
> > > >
> > > > I'm talking about the case where we say we've enabled the WDT to
> > > > supervise OS, but then bootefi something and have disabled the watchdog
> > > > (to meet UEFI requirements) but didn't tell the user we've turned off
> > > > the WDT that we had told them is on.
> > > 
> > > Ah, do you really want to have a different behavior between bootm and
> > > bootefi? Thats even more suprising IMHO.
> > 
> > I fear it is the only way to support both user communities.
> > 
> > > I had the following in mind:
> > > 
> > > +config WATCHDOG_SUPERVISE_OS
> > > +       bool "Supervise U-boot and operating system"
> > > +       help
> > > +         Upon U-Boot startup the first watchdog will be started
> > > automatically
> > > +         and kept running even after booting the operating system.
> > > +         Be aware, that the operating system needs to service the
> > > watchdog!
> > > +
> > > +         Additionally, this is not UEFI compliant because:
> > > +          - the timeout won't be set to 5 minutes before starting the
> > > OS and
> > > +          - the watchdog timer isn't stopped after the OS calls
> > > +            ExitBootServices().
> > 
> > That would be highly problematic:
> > 
> > * It is unfeasable for an EFI OS bootloader to include drivers for the
> >   hardware watchdog.  This means that the time available for the user
> >   to interact with the bootloader is limited by the timeout of the
> >   hardware watchdog.  Consider for example the case of full disk
> >   encryption where the user is supposed to enter a password before the
> >   OS can be loaded.  The hardware watchdog would limit the amount of
> >   time available to enter that password.  If the timeout is seconds
> >   rather than minutes the system becomes unusable.
> > 
> > * This requires the OS to have support for the hardware watchdog.
> >   Consider the case where a U-Boot with a default configuration has
> >   been programmed in SPI flash.  Without the hardware watchdog running
> >   you can install an arbitrary OS using an EFI bootable image.  With
> >   the hardware watchdog running an OS lacking support for the hardware
> >   watchdog would probably reboot in the middle of an install.  Even
> >   when the kernel that will be ultimately installed supports the
> >   hardware watchdog, the kernel used by the OS installer may not.  For
> >   example a Linux kernel where the watchdog timer driver is built as a
> >   module and that module isn't present on the install image.
> 
> You don't have to convince me, that having the HW watchdog enabled is
> bad in the EFI case. Having a debian installer without watchdog support
> was the reason I've started the old discussion thread and this patch.
> 
> Are there any objections to disable the HW watchdog unconditionally
> and printing a notice before we start an EFI image and possibly
> changing behavior for existing boards (if someone is actually using
> bootefi)? Tom? Heinrich?

The biggest concern I guess I have at this point is we have a lot of
boards where we don't have CONFIG_WDT set, but we do have a watchdog
driver available and in use and being frequently serviced.  See for
example mx6cuboxi.

To somewhat get back to the original problem, it feels like on the newer
NXP parts, watchdog support was implemented to the current frameworks,
DM, etc, and so this problem is more visibly exposed.  But on the 32bit
i.MX parts it's always there.  And a quick look at current'ish Linux
kernel defconfig, there's always the "IMX2_WDT" driver built in.  So I
don't know what's going on there off-hand.

-- 
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/20200924/53f29783/attachment.sig>