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, <mich...@walle.cc> wrote:
> >>>
> >>> Am 2020-09-15 09:44, schrieb Rayagonda Kokatanur:
> >>> > On Tue, Sep 15, 2020 at 12:56 PM Michael Walle <mich...@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?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. 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. -- Tom
signature.asc
Description: PGP signature
