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

[Michael Walle]

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?

-michael