linux-kernel - Re: [PATCH] clocksource: timer-tegra186: Enable WDT at probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <458d9b45-2879-4dd5-b164-82d87fdb5ad8@nvidia.com>
Date: Thu, 3 Jul 2025 11:36:16 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Kartik Rajput <kkartik@...dia.com>, daniel.lezcano@...aro.org,
 tglx@...utronix.de, thierry.reding@...il.com, linux-kernel@...r.kernel.org,
 linux-tegra@...r.kernel.org
Subject: Re: [PATCH] clocksource: timer-tegra186: Enable WDT at probe



On 30/06/2025 12:01, Kartik Rajput wrote:
> Currently, if the system crashes or hangs during kernel boot before
> userspace initializes and configures the watchdog timer, then the
> watchdog won’t be able to recover the system as it’s not running. This
> becomes crucial during an over-the-air update, where if the newly
> updated kernel crashes on boot, the watchdog is needed to reset the
> device and boot into an alternative system partition. If the watchdog
> is disabled in such scenarios, it can lead to the system getting
> bricked.
> 
> Enable the WDT during driver probe to allow recovery from any crash/hang
> seen during early kernel boot. Also, disable interrupts once userspace
> starts pinging the watchdog.
> 
> Signed-off-by: Kartik Rajput <kkartik@...dia.com>
> ---
>   drivers/clocksource/timer-tegra186.c | 42 ++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
> index e5394f98a02e..59abb5dab8f1 100644
> --- a/drivers/clocksource/timer-tegra186.c
> +++ b/drivers/clocksource/timer-tegra186.c
> @@ -57,6 +57,8 @@
>   #define WDTUR 0x00c
>   #define  WDTUR_UNLOCK_PATTERN 0x0000c45a
>   
> +#define WDT_DEFAULT_TIMEOUT 120
> +
>   struct tegra186_timer_soc {
>   	unsigned int num_timers;
>   	unsigned int num_wdts;
> @@ -74,6 +76,7 @@ struct tegra186_wdt {
>   
>   	void __iomem *regs;
>   	unsigned int index;
> +	bool enable_irq;
>   	bool locked;
>   
>   	struct tegra186_tmr *tmr;
> @@ -174,6 +177,12 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt)
>   		value &= ~WDTCR_PERIOD_MASK;
>   		value |= WDTCR_PERIOD(1);
>   
> +		/* configure local interrupt for WDT petting */

It might be a bit clearer if this comment states ...

'If enable_irq is set then enable the watchdog IRQ for kernel petting, 
otherwise userspace is responsible for petting the watchdog.'

> +		if (wdt->enable_irq)
> +			value |= WDTCR_LOCAL_INT_ENABLE;
> +		else
> +			value &= ~WDTCR_LOCAL_INT_ENABLE;
> +
>   		/* enable system POR reset */
>   		value |= WDTCR_SYSTEM_POR_RESET_ENABLE;
>   
> @@ -205,6 +214,10 @@ static int tegra186_wdt_ping(struct watchdog_device *wdd)
>   {
>   	struct tegra186_wdt *wdt = to_tegra186_wdt(wdd);
>   
> +	/* Disable WDT interrupt once userspace takes over. */

Technically userspace is taking over at this point and so we should be 
more assertive here ...

'Disable the watchdog IRQ now userspace is taking over'

> +	if (wdt->enable_irq)
> +		wdt->enable_irq = false;
> +
>   	tegra186_wdt_disable(wdt);
>   	tegra186_wdt_enable(wdt);
>   
> @@ -315,6 +328,8 @@ static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra,
>   	if (value & WDTCR_LOCAL_INT_ENABLE)
>   		wdt->locked = true;
>   
> +	wdt->enable_irq = true;
> +
>   	source = value & WDTCR_TIMER_SOURCE_MASK;
>   
>   	wdt->tmr = tegra186_tmr_create(tegra, source);
> @@ -339,6 +354,13 @@ static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra,
>   		return ERR_PTR(err);
>   	}
>   
> +	/*
> +	 * Start the watchdog to recover the system if it crashes before
> +	 * userspace initialize the WDT.
> +	 */
> +	tegra186_wdt_set_timeout(&wdt->base, WDT_DEFAULT_TIMEOUT);
> +	tegra186_wdt_start(&wdt->base);
> +
>   	return wdt;
>   }
>   
> @@ -415,10 +437,21 @@ static int tegra186_timer_usec_init(struct tegra186_timer *tegra)
>   	return clocksource_register_hz(&tegra->usec, USEC_PER_SEC);
>   }
>   
> +static irqreturn_t tegra186_timer_irq(int irq, void *data)
> +{
> +	struct tegra186_timer *tegra = data;
> +
> +	tegra186_wdt_disable(tegra->wdt);
> +	tegra186_wdt_enable(tegra->wdt);
> +
> +	return IRQ_HANDLED;
> +}
> +
>   static int tegra186_timer_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct tegra186_timer *tegra;
> +	unsigned int irq;
>   	int err;
>   
>   	tegra = devm_kzalloc(dev, sizeof(*tegra), GFP_KERNEL);
> @@ -437,6 +470,15 @@ static int tegra186_timer_probe(struct platform_device *pdev)
>   	if (err < 0)
>   		return err;
>   
> +	irq = err;
> +
> +	err = devm_request_irq(dev, irq, tegra186_timer_irq, 0,
> +			       "tegra186-timer", tegra);
> +	if (err < 0) {
> +		dev_err(dev, "failed to request IRQ#%u: %d\n", irq, err);
> +		return err;
> +	}
> +
>   	/* create a watchdog using a preconfigured timer */
>   	tegra->wdt = tegra186_wdt_create(tegra, 0);
>   	if (IS_ERR(tegra->wdt)) {

-- 
nvpublic