Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 23 Apr 2022 10:36:56 +0100
Message-ID: <877d7gcf9j.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Sebastian Ene <sebastianene@google.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org,
        Derek Kiernan <derek.kiernan@xilinx.com>,
        Dragan Cvetic <dragan.cvetic@xilinx.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org,
        qperret@google.com, will@kernel.org
Subject: Re: [PATCH  v2 0/2] Detect stalls on guest vCPUS
In-Reply-To: <YmPAoFjfGoBHbPjC@google.com>
References: <20220422141949.3456505-1-sebastianene@google.com>
        <YmOh5I7ZK08LlM6D@kroah.com>
        <YmPAoFjfGoBHbPjC@google.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Sat, 23 Apr 2022 10:02:24 +0100,
Sebastian Ene <sebastianene@google.com> wrote:
> 
> On Sat, Apr 23, 2022 at 08:51:16AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Apr 22, 2022 at 02:19:48PM +0000, Sebastian Ene wrote:
> > > This adds a mechanism to detect stalls on the guest vCPUS by creating a
> > > per CPU hrtimer which periodically 'pets' the host backend driver.
> > > 
> > > This device driver acts as a soft lockup detector by relying on the host
> > > backend driver to measure the elapesed time between subsequent 'pet' events.
> > > If the elapsed time doesn't match an expected value, the backend driver
> > > decides that the guest vCPU is locked and resets the guest. The host
> > > backend driver takes into account the time that the guest is not
> > > running. The communication with the backend driver is done through MMIO
> > > and the register layout of the virtual watchdog is described as part of
> > > the backend driver changes.
> > > 
> > > The host backend driver is implemented as part of:
> > > https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3548817
> > > 
> > > Changelog v2:
> > >  - move the driver to misc as this does not cope with watchdog core
> > >    subsystem
> 
> Hello Greg,
> 
> > 
> > Wait, why does it not cope with it?  That's not documented anywhere in
> > your patch that adds the driver.  In fact, most of the text here needs
> > to be in the changelog for the driver submission, not thrown away in the
> > 00/XX email that will never end up in the kernel tree.
> > 
> > thanks,
> > 
> > greg k-h
> 
> From the previous feedback that I received on this patch it seems that
> watchdog core is not intended to be used for this type of driver. This
> watchdog device tracks the elapsed time on a per-cpu basis,
> since KVM schedules vCPUs independently. Watchdog core is not intended
> to detect CPU stalls and the drivers don't have a notion of CPU.

I must say that I don't really get the objection against the watchdog
approach. OK, there is no userspace aspect to this.  But we already
use watchdogs for more than just userspace (reboot is one of the major
use cases).

There already are per-CPU watchdog in the tree: see how the
fsl-ls208xa platform has one SP805 per CPU (8 of them in total). As
far as I can tell, there was no objection to this. So what is special
about this one?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.