linux-kernel - Re: [PATCH v3] watchdog: Add driver for Gunyah Watchdog

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: <6ceb8331-cc4a-4754-8b9e-dea887665efc@roeck-us.net>
Date: Tue, 28 Oct 2025 10:29:32 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Pavan Kondeti <pavan.kondeti@....qualcomm.com>
Cc: hrishabh.rajput@....qualcomm.com, Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>,
 Wim Van Sebroeck <wim@...ux-watchdog.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org,
 linux-watchdog@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Neil Armstrong <neil.armstrong@...aro.org>
Subject: Re: [PATCH v3] watchdog: Add driver for Gunyah Watchdog

On 10/28/25 09:40, Pavan Kondeti wrote:
> On Tue, Oct 28, 2025 at 09:06:12AM -0700, Guenter Roeck wrote:
>> On 10/28/25 02:35, Hrishabh Rajput via B4 Relay wrote:
>>> From: Hrishabh Rajput <hrishabh.rajput@....qualcomm.com>
>>>
>>> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
>>> through MMIO is not available on all platforms. Depending on the
>>> hypervisor configuration, the watchdog is either fully emulated or
>>> exposed via ARM's SMC Calling Conventions (SMCCC) through the Vendor
>>> Specific Hypervisor Service Calls space.
>>>
>>> When Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
>>> expect Qualcomm watchdog or ARM SBSA watchdog device to be present in
>>> the devicetree. If we detect either of the device nodes, we don't
>>> proceed ahead. Otherwise, we go ahead and invoke GUNYAH_WDT_STATUS SMC
>>> to initiate the discovery of the SMC-based watchdog.
>>>
>>> Add driver to support the SMC-based watchdog provided by the Gunyah
>>> Hypervisor. module_exit() is intentionally not implemented as this
>>> driver is intended to be a persistent module.
>>>
>>> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@....qualcomm.com>
>>> ---

...
>>> +	gunyah_wdt_dev = platform_device_register_simple(GUNYAH_WDT_DRV_NAME,
>>> +							 -1, NULL, 0);
>>
>> I did not follow the discussion around this, so I may be missing something.
>> If so, apologies.
>>
>> This is a highly unusual approach. What is the point of not instantiating
>> the watchdog device through devicetree and doing it in the init function
>> instead ? There should be a devicetree node which instantiates the device;
>> it should never be instantiated from the init function unless there _is_
>> no devicetree, which is obviously not the case here.
>>
>> Every other driver which supports devicetree has an .of_match_table
>> which triggers device instantiation. If the Gunyah watchdog can for
>> some reason not use that approach, its devicetree description should
>> be fixed. Instantiating the device from its init function because its
>> devicetree description is bad or missing is just wrong. It is even more
>> wrong to try to contact the hardware or embedded controller to figure out
>> if the device is there. This can have all kinds of negative impact on other
>> hardware.
>>
> The Gunyah WDT node gets overlayed by bootloader. We see that this
> overlay is failing w/ upstream device tree since the overlay has
> references to downstream code. Please see [1]. Hence we are trying to
> register the platform device dynamically.
> 

This is just wrong. Whatever happens downstream is not an upstream concern.
If an overlay is broken, fix it.

NACK to the current approach.

Guenter