On Tue, Sep 22, 2020 at 8:56 AM Michael Walle michael@walle.cc wrote:
Hi,
Am 2020-09-21 20:50, schrieb Tom Rini:
On Mon, Sep 21, 2020 at 08:29:00PM +0200, Heinrich Schuchardt wrote:
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@walle.cc wrote:
Am 2020-09-15 09:44, schrieb Rayagonda Kokatanur: > On Tue, Sep 15, 2020 at 12:56 PM Michael Walle <michael@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 neverWhich board are you referring to?
See the commit above. It is board/alliedtelesis/x530/x530.c. It might not use EFI, but I tried to use it as a blueprint to disable the watchdog by default and then noticed it won't work in the bootefi case (and I guess the 'go' case).
Yes that's the in-tree board we have that uses this although it's for different reasons (HW related). It doesn't use EFI. We had to add code for that board to disable the watchdog pre-boot to maintain compatibility with an old userland. Had we been starting from fresh we would have just made sure that the userland was able to service the watchdog (which we are doing for newer boards).
>> 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?
For EFI or ? What I'm saying is that the watchdog must be left running and not stopped, if we either:
- Came in to the world with the watchdog running AND were not specifically told to disable the watching.
- Came in to the world and were told to enable a watchdog.
My reason to start this thread was the fact that a watchdog is started by default in a generic way (i.e. initr_watchdog()) but there is _no_ way to disable it. I'm having a minimal board configuration and I want to be able to boot the debian-installer via EFI -> grub-efi -> d-i. The debian installer is not aware of any watchdog. Thus if u-boot leave it running, it might bite at very inconvenient times like half through the installation.
I'm fine with having a unified way to disable the watchdog per board, let it be a CONFIG_WDT_NO_START or a #define ENV "wdt-stop-pre-os", but it should work with bootm/booti/go/bootefi.
It's that first case with the AND I'm concerned with in general and this thread.
For the EFI case, I assume right now we aren't strictly adhering to the 5 minute rule, but I also assume there's some way for UEFI to tell us to call WATCHDOG_RESET() as needed.
EFI timers seems to be unrelated to the watchdog, right?
-michael