Re: [PATCH] x86: ich9: fix default value of 'No Reboot' bit in GCS

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Sep 23, 2025 at 10:40:51AM +0200, Igor Mammedov wrote:
> On Mon, 22 Sep 2025 15:24:09 +0100
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Mon, Sep 22, 2025 at 03:26:00PM +0200, Igor Mammedov wrote:
> > > [2] initialized 'No Reboot' bit to 1 by default. And due to quirk it happened
> > > to work with linux iTCO_wdt driver (which clears it on module load).
> > > 
> > > However spec [1] states:
> > > "
> > > R/W. This bit is set when the “No Reboot” strap (SPKR pin on
> > > ICH9) is sampled high on PWROK.
> > > "
> > > 
> > > So it should be set only when  '-global ICH9-LPC.noreboot=true' and cleared
> > > when it's false (which should be default).
> > > 
> > > Fix it to behave according to spec and set 'No Reboot' bit only when
> > > '-global ICH9-LPC.noreboot=true'.  
> > 
> > Is there a real-world problem you hit that is being solved by
> > this change, or is it just a theoretical spec compliance fix ?
> 
> I've stumbled upon it when implementing ACPI watchdog POC
> 
> https://gitlab.com/imammedo/qemu/-/commits/wadt_poc
> I'm not sure that watchdog table belongs to QEMU,
> but the ICH fix definitely is.

I've tested this as follows [1]

 $ make-tiny-image.py --kmod lpc_ich --kmod iTCO_wdt  --kmod i2c_i801
 $ qemu-system-x86_64 \
     -kernel /lib/modules/6.15.9-201.fc42.x86_64/vmlinuz \
     -initrd tiny-initrd.img \
     -append 'console=ttyS0 quiet' \
     -m 1000 \
     -display none \
     -serial stdio \
     -accel kvm \
     -M q35 \
     -global ICH9-LPC.noreboot=false \
     -watchdog-action poweroff \
     -trace ich9* -trace tco*
ich9_cc_read addr=0x3410 val=0x0 len=4
ich9_cc_write addr=0x3410 val=0x0 len=4
ich9_cc_read addr=0x3410 val=0x0 len=4
tco_io_write addr=0x4 val=0x8
tco_io_write addr=0x6 val=0x2
tco_io_write addr=0x6 val=0x4
tco_io_read addr=0x8 val=0x0
tco_io_read addr=0x12 val=0x4
tco_io_write addr=0x12 val=0x32
tco_io_read addr=0x12 val=0x32
tco_io_write addr=0x0 val=0x1
tco_timer_reload ticks=50 (30000 ms)
~ # mknod /dev/watchdog0 c 10 130
~ # cat /dev/watchdog0
tco_io_write addr=0x0 val=0x1
tco_timer_reload ticks=50 (30000 ms)
cat: read error: Invalid argument
[    5.646147] watchdog: watchdog0: watchdog did not stop!
tco_io_write addr=0x0 val=0x1
tco_timer_reload ticks=50 (30000 ms)
~ # tco_timer_expired timeouts_no=0 no_reboot=0/0
tco_timer_reload ticks=50 (30000 ms)
tco_timer_expired timeouts_no=1 no_reboot=0/0

And the same, but with ICH9-LPC.noreboot=true.

I see no functional change from Linux guest POV in either
scenario before/after this patch, so

  Tested-by: Daniel P. Berrangé <berrange@redhat.com>
  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel

[1] https://gitlab.com/berrange/tiny-vm-tools/-/blob/master/make-tiny-image.py
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|