From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1E5D7FA3744 for ; Mon, 31 Oct 2022 13:41:49 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1opV2I-0003p2-G5; Mon, 31 Oct 2022 09:41:06 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1opV2G-0003om-NA for qemu-devel@nongnu.org; Mon, 31 Oct 2022 09:41:04 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1opV22-0004bd-F4 for qemu-devel@nongnu.org; Mon, 31 Oct 2022 09:41:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667223649; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Frpv+2g/HMF/0CWoJM+VNVYMD6p61Y+EME+QiXRHO+o=; b=XyOdTg1GR0e2CHtZfIuhctjdat5HaS6AkouJbsYGoScttTMXDaSqOEygx1b5pEWhPPvMq4 nzQFDGIIUto9ZhzjeHpdv59v9HJLKGEDStMDxL6OIYXxJ9NYHnl+q7NbT+TQu4fH6jGBJg W2NrZlIXX2qmwkBJ5h9+uf3qI0C4as0= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-583-crFgYB1vPfm83V0w0IZbDw-1; Mon, 31 Oct 2022 09:40:47 -0400 X-MC-Unique: crFgYB1vPfm83V0w0IZbDw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2BB1E85A5B6; Mon, 31 Oct 2022 13:40:47 +0000 (UTC) Received: from localhost (unknown [10.39.193.198]) by smtp.corp.redhat.com (Postfix) with ESMTP id 83ECA111E40B; Mon, 31 Oct 2022 13:40:46 +0000 (UTC) Date: Mon, 31 Oct 2022 13:40:45 +0000 From: "Richard W.M. Jones" To: Daniel =?iso-8859-1?Q?P=2E_Berrang=E9?= Cc: qemu-devel@nongnu.org, Laurent Vivier , Richard Henderson , Ani Sinha , Igor Mammedov , "Michael S. Tsirkin" , Thomas Huth , Paolo Bonzini , Eduardo Habkost , Marcel Apfelbaum Subject: Re: [PATCH 4/4] hw/isa: enable TCO watchdog reboot pin strap by default Message-ID: <20221031134045.GJ7636@redhat.com> References: <20221031131934.425448-1-berrange@redhat.com> <20221031131934.425448-5-berrange@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221031131934.425448-5-berrange@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 Received-SPF: pass client-ip=170.10.129.124; envelope-from=rjones@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -30 X-Spam_score: -3.1 X-Spam_bar: --- X-Spam_report: (-3.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-1.048, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Qemu-devel" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org+qemu-devel=archiver.kernel.org@nongnu.org On Mon, Oct 31, 2022 at 01:19:34PM +0000, Daniel P. Berrangé wrote: > The TCO watchdog implementation default behaviour from POV of the > guest OS relies on the initial values for two I/O ports: > > * TCO1_CNT == 0x0 > > Since bit 11 (TCO Timer Halt) is clear, the watchdog state > is considered to be initially running > > * GCS == 0x20 > > Since bit 5 (No Reboot) is set, the watchdog will not trigger > when the timer expires > > This is a safe default, because the No Reboot bit will prevent the > watchdog from triggering if the guest OS is unaware of its existance, > or is slow in configuring it. When a Linux guest initializes the TCO > watchdog, it will attempt to clear the "No Reboot" flag, and read the > value back. If the clear was honoured, the driver will treat this as > an indicator that the watchdog is functional and create the geust Typo: "guest" > watchdog device. > > QEMU implements a second "no reboot" flag, however, via pin straps > which overrides the behaviour of the guest controlled "no reboot" > flag: > > commit 5add35bec1e249bb5345a47008c8f298d4760be4 > Author: Paulo Alcantara > Date: Sun Jun 28 14:58:58 2015 -0300 > > ich9: implement strap SPKR pin logic > > This second 'noreboot' pin was defaulted to high, which also inhibits > triggering of the requested watchdog actions, unless QEMU is launched > with the magic flag "-global ICH9-LPC.noreboot=false". > > This is a bad default as we are exposing a watchdog to every guest OS > using the q35 machine type, but preventing it from actually doing what > it is designed to do. What is worse is that the guest OS and its apps > have no way to know that the watchdog is never going to fire, due to > this second 'noreboot' pin. > > If a guest OS had no watchdog device at all, then apps whose operation > and/or data integrity relies on a watchdog can refuse to launch, and > alert the administrator of the problematic deployment. With Q35 machines > unconditionally exposing a watchdog though, apps will think their > deployment is correct but in fact have no protection at all. > > This patch flips the default of the second 'no reboot' flag, so that > configured watchdog actions will be honoured out of the box for the > 7.2 Q35 machine type onwards, if the guest enables use of the watchdog. > > Signed-off-by: Daniel P. Berrangé Add Fixes: or some other reference to the BZs? We have a few! https://bugzilla.redhat.com/show_bug.cgi?id=2136889 https://bugzilla.redhat.com/show_bug.cgi?id=2080207 https://bugzilla.redhat.com/show_bug.cgi?id=2137346 (libvirt) > hw/i386/pc.c | 4 +++- > hw/isa/lpc_ich9.c | 2 +- > tests/qtest/tco-test.c | 2 +- > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 3e86083db3..e814f62fc6 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -108,7 +108,9 @@ > { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\ > { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, }, > > -GlobalProperty pc_compat_7_1[] = {}; > +GlobalProperty pc_compat_7_1[] = { > + { "ICH9-LPC", "noreboot", "true" }, > +}; > const size_t pc_compat_7_1_len = G_N_ELEMENTS(pc_compat_7_1); > > GlobalProperty pc_compat_7_0[] = {}; > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index 66062a344c..f9ce2ee1dc 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -789,7 +789,7 @@ static const VMStateDescription vmstate_ich9_lpc = { > }; > > static Property ich9_lpc_properties[] = { > - DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true), > + DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, false), > DEFINE_PROP_BOOL("smm-compat", ICH9LPCState, pm.smm_compat, false), > DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features, > ICH9_LPC_SMI_F_BROADCAST_BIT, true), > diff --git a/tests/qtest/tco-test.c b/tests/qtest/tco-test.c > index 254f735370..caabcac6e5 100644 > --- a/tests/qtest/tco-test.c > +++ b/tests/qtest/tco-test.c > @@ -60,7 +60,7 @@ static void test_init(TestData *d) > QTestState *qs; > > qs = qtest_initf("-machine q35 %s %s", > - d->noreboot ? "" : "-global ICH9-LPC.noreboot=false", > + d->noreboot ? "-global ICH9-LPC.noreboot=true" : "", > !d->args ? "" : d->args); > qtest_irq_intercept_in(qs, "ioapic"); Acked-by: Richard W.M. Jones Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW