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] [day] [month] [year] [list]

Message-ID: <cf5c0f82-56ce-4ced-986f-aae8a23e7166@quicinc.com>
Date: Tue, 28 Oct 2025 23:19:59 +0530
From: Pavan Kondeti <pavan.kondeti@....qualcomm.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Pavan Kondeti <pavan.kondeti@....qualcomm.com>,
        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 Tue, Oct 28, 2025 at 10:29:32AM -0700, Guenter Roeck wrote:
> 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.

The problem (watchdog availability to Linux) has been fixed on latest QC
devices where Gunyah (Hypervisor) emulate the MMIO. The watchdog device
has been added to device tree and it works with
drivers/watchdog/qcom-wdt.c drivers. 

However, we have older QC platforms like SM8550, SM8650, SM8750 etc
where watchdog is supported only through Gunyah SMC interface. we are
told it is better to expose this as a hypervisor interface, so we are
adding platfrom device in the code. 

Our goal is to enable the watchdog functionality on older Qualcomm
devices that are already supported in upstream. One of the main use cases
is to enable RAM dump collection upon kernel panic. With the
watchdog enabled, Linux fails to "pet" the watchdog during a panic and
device enter RAM dump collection mode.

> 
> NACK to the current approach.
> 

In hindsight, it would have been a lot better to add the watchdog device
directly from the qcom specific code and add the watchdog driver as a 
standard platform driver that probes the device using the
SMC interface. We plan to try this approach in v4 and will wait for
further feedback.

Thanks,
Pavan