u-boot leaves watchdog enabled by default

[Heinrich Schuchardt]

On 9/21/20 7:30 PM, Tom Rini wrote:
> On Mon, Sep 21, 2020 at 11:01:37AM +0200, Stefan Roese wrote:
>> Hi Michael,
>> Hi Chris,
>>
>> On 15.09.20 12:44, Chris Packham wrote:
>>>
>>>
>>> On Tue, 15 Sep 2020, 7:54 PM Michael Walle, <michael at walle.cc> wrote:
>>>
>>>     Am 2020-09-15 09:44, schrieb Rayagonda Kokatanur:
>>>      > On Tue, Sep 15, 2020 at 12:56 PM Michael Walle <michael at walle.cc>
>>>      > wrote:
>>>      >>
>>>      >> Hi Stefan,
>>>      >>
>>>      >> it appears that since commit 06985289d45 ("watchdog: Implement
>>>     generic
>>>      >> watchdog_reset() version") - by default - the first watchdog is
>>>      >> started
>>>      >> unconditionally if CONFIG_WDT is set but never stopped before
>>>     booting
>>>      >> the operating system.
>>>      >>
>>>      >> Shouldn't it also be stopped uncondionally? What's worse is that on
>>>      >> one
>>>      >> board/arch the watchdog is stopped in arch_preboot_os() which is
>>>     never

Which board are you referring to?

>>>      >> called in the bootefi case. So even if I'd do a workaround and
>>>     stop it
>>>      >> manually in my board code, I couldn't do that consistently for
>>>      >> bootm/bootefi.
>>>      >>
>>>      >> Or am I missing something here?
>>>      >
>>>      > Define CONFIG_WATCHDOG.
>>>      > This takes care of resetting wdt.
>>>
>>>     Yes as along as you're inside the bootloader, but when u-boot hands
>>>     control over the OS the watchdog is not serviced anymore; which wouldn't
>>>     be a problem per se, but it is enabled unconditionally by u-boot.
>>>
>>>
>>> Just to add some data. At $dayjob we use this behaviour as a failsafe to
>>> make sure our userspace gets to a point where it is servicing the
>>> watchdog.
>>
>> Yes, this is exactly how this is supposed to work AFAIK.
>>
>> Michael, are you sure that the watchdog was disabled in U-Boot when
>> booting into the OS before this patch?
>>
>>> That said having a leave-wdt-running environment variable would work for
>>> our use case.
>>
>> I would rather use it the other way around. Something like "wdt-stop-
>> pre-os" to optionally stop the WDT before booting into the OS.
>>
>> Remark:
>> IMHO, if you don't use the WDT in the OS, it does not make much sense
>> to enable the WDT in U-Boot.
>
> Yes, we need to be very careful about making it so that a watchdog is
> disabled and not re-enabled before moving on for a whole bunch of
> reasons.  And the best option would be to just disable the watchdog if
> it won't be used while the device is running the OS.
>

The requirement of the UEFI specification is that if booting fails a
system should reset after five minutes by default. We ensure this in the
UEFI sub-system before ExitBootServices() using an EFI timer event.

In the UEFI sub-system we currently call in ExitBootServices():

        efi_set_watchdog(0); /* this disables the EFI timer */
        WATCHDOG_RESET();

Is there any requirement to do more?

Best regards

Heinrich