> Content-Type: text/plain; charset=US-ASCII; > format=flowed > Date: Thu, 24 Sep 2020 10:20:30 +0200 > From: Michael Walle <mich...@walle.cc> > Cc: tr...@konsulko.com, xypron.g...@gmx.de, u-boot@lists.denx.de, > ag...@csgraf.de, ma...@denx.de, judge.pack...@gmail.com, s...@denx.de, > rayagonda.kokata...@broadcom.com > > Am 2020-09-24 10:10, schrieb Mark Kettenis: > >> Date: Thu, 24 Sep 2020 09:33:50 +0200 > >> From: Michael Walle <mich...@walle.cc> > >> > >> Am 2020-09-23 19:35, schrieb Tom Rini: > >> > On Wed, Sep 23, 2020 at 07:31:00PM +0200, Heinrich Schuchardt wrote: > >> >> On 9/23/20 7:14 PM, Tom Rini wrote: > >> >> > On Wed, Sep 23, 2020 at 07:01:54PM +0200, Mark Kettenis wrote: > >> >> >>> From: Michael Walle <mich...@walle.cc> > >> >> >>> Date: Wed, 23 Sep 2020 18:45:27 +0200 > >> >> >>> > >> >> >>> Let the user choose between three different behaviours of the > >> >> >>> watchdog: > >> >> >>> (1) Keep the watchdog disabled > >> >> >>> (2) Supervise u-boot > >> >> >>> (3) Supervise u-boot and the operating systen (default) > >> >> >>> > >> >> >>> Option (2) will disable the watchdog right before handing control > >> >> >>> to the > >> >> >>> operating system. This is useful when the OS is not aware of the > >> >> >>> watchdog. Option (3) doesn't disable the watchdog and assumes the OS > >> >> >>> will continue servicing. > >> >> >> > >> >> >> (3) can't be the default, at least for EFI > >> >> >> > >> >> >> The UEFI standard explicitly says that upon calling > >> >> >> ExitBootServices(), the watchdog timer is disabled. > >> >> >> > >> >> >> In general, you can't expect an OS to have support for a particular > >> >> >> watchdog timer. So (3) only makes sense in cases where U-Boot is > >> >> >> bundled with an OS image. > >> >> > > >> >> > We need to be careful here then. The current and historical / > >> >> > generally > >> >> > expected behavior is if we've enabled the watchdog we supervise it and > >> >> > leave it enabled for the OS. Given what UEFI requires I'd like to see > >> >> > that case handled with a print about disabling the watchdog so it's > >> >> > not > >> > >> I agree with "current and historical behavior" but not with "expected > >> behavior". > >> > >> I was thinking about something like > >> > >> +choice > >> + prompt "Watchdog behavior" > >> + default WATCHDOG_SUPERVISE_U_BOOT if EFI_LOADER > >> + default WATCHDOG_SUPERVISE_OS if !EFI_LOADER > >> + depends on WDT > >> > >> Unfortunately, EFI_LOADER is default y for any architecture != ARM. > >> Therefore, it is likely we are changing the behavior of some boards > >> and I agree this isn't what we want. > > > > I think you are misreading that. The following stanza: > > > > depends on OF_LIBFDT && ( \ > > ARM && (SYS_CPU = arm1136 || \ > > SYS_CPU = arm1176 || \ > > SYS_CPU = armv7 || \ > > SYS_CPU = armv8) || \ > > X86 || RISCV || SANDBOX) > > > > means that EFI_LOADER is onlu ever defined on (newish) ARM, X86, RISCV > > and SANDBOX. Which makes sense since there is no EFI calling > > convention defined for other architectures like MIPS and PPC. > > I've missed that, but that will just limit the search space. > Like how can we figure out what board has both EFI_LOADER=y and > WDT=y set? If there is no such board, then we can use the > defaults above. If there are such boards we will have to put > CONFIG_SUPERVISE_OS=y in their defconfig. > > If EFI_LOADER would have had no default y, then a simple > grep for EFI_LOADER=y (and WDT=y) would have been sufficient.
And this is where the conflict of interest surfaces. >From a "running a generic OS" standpoint of view we really want to have EFI_LOADER enabled on as many ARM/X86/RISCV boards as possible and want to discourage the sometimes heavy customization that some of the board vendors have done historically. Such customizations often break booting a generic OS or at least create inconsistencies tat are confusing to users of such a generic OS. And WDT=y pretty is such a customization. At the same time there obviously is the desire from some vendors to integrate U-Boot with an OS (which in practice probably always is a customized Linux kernel). This scenario typically uses "bootm" instead of the EFI loader and I believe that only in this context the historic watchdog behaviour makes sense. Therefore I think it makes sense to always disable any running hardware watchdog timer when starting an EFI payload, and reenable it if that payload returns to the bootloader. I don't think printing a message when doing so makes sense as users of the EFI loader expect any watchdog timer to be disabled, but printing a message that a hardware watchdog is being disabled before starting the EFI payload may be acceptable. Please note that very few ARM board configurations actually set WDT=y, so generic OS users may simply not have noticed issues with the current "policy" of leaving a hardware watchdog running when booting a generic OS.
