On 27/05/2015 02:29, Paulo Alcantara wrote:
This interface provides some registers within a 32-byte range and can be acessed through PCI-to-LPC bridge interface (PMBASE + 0x60).
It's commonly used as a watchdog timer to detect system lockups through SMIs that are generated -- if TCO_EN bit is set -- on every timeout. If NO_REBOOT bit is not set in GCS (General Control and Status register), the system will be resetted upon second timeout if TCO_RLD register wasn't previously written to prevent timeout.
This patch adds support to TCO watchdog logic and few other features like mapping NMIs to SMIs (NMI2SMI_EN bit), system intruder detection, etc. are not implemented yet.
Signed-off-by: Paulo Alcantara pcacjr@zytor.com
Hi,
the main issue here is lack of migration support. You need to add a new subsection to vmstate_ich9_pm that is migrated if the registers are different from the default values.
Other comments inline.
diff --git a/hw/acpi/tco.c b/hw/acpi/tco.c new file mode 100644 index 0000000..3a44a95 --- /dev/null +++ b/hw/acpi/tco.c @@ -0,0 +1,188 @@ +/*
- QEMU ICH9 TCO emulation
- Copyright (c) 2015 Paulo Alcantara pcacjr@zytor.com
- Permission is hereby granted, free of charge, to any person obtaining a copy
- of this software and associated documentation files (the "Software"), to deal
- in the Software without restriction, including without limitation the rights
- to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- copies of the Software, and to permit persons to whom the Software is
- furnished to do so, subject to the following conditions:
- The above copyright notice and this permission notice shall be included in
- all copies or substantial portions of the Software.
- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- THE SOFTWARE.
- */
+#include "qemu-common.h" +#include "sysemu/watchdog.h" +#include "hw/i386/ich9.h"
+#include "hw/acpi/tco.h"
+//#define DEBUG
+#ifdef DEBUG +#define TCO_DEBUG(fmt, ...) \
- do { \
fprintf(stderr, "%s "fmt, __func__, ## __VA_ARGS__); \- } while (0)
+#else +#define TCO_DEBUG(fmt, ...) do { } while (0) +#endif
+static QEMUTimer *tco_timer; +static unsigned int timeouts_no;
These must not be globals. Instead, add them to TCOIORegs.
+static inline void tco_timer_reload(void) +{
- int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- timer_del(tco_timer);
- timer_mod(tco_timer, now + tco_ticks_per_sec());
No need for del+mod. Also, these are not tco_ticks_per_sec() but rather TCO_TICK_NSEC. It can be a #define instead of a function.
For a little extra challenge, you could the base value when the timer was started inside TCOIORegs. The timer can be made to expire directly after 0.6*TCO_TMR seconds; you can use the base value and the current time to compute the value of TCO_RLD. Something like:
case TCO_RLD: rld = tr->tco.rld; /* base_clock is set to -1 in tco_timer_stop, reloaded in * tco_timer_reload. */ if (base_clock != -1) { elapsed = (qemu_get_clock(QEMU_CLOCK_VIRTUAL) - base_clock) / TCO_TICK_NSEC; rld -= MIN(elapsed, rld); }
This makes it possible for QEMU to sleep most of the time when the guest is idle, instead of waking up every 0.6 seconds. But feel free to work on this afterwards.
+void acpi_pm_tco_init(TCOIORegs *tr) +{
- *tr = TCO_IO_REGS_DEFAULTS_INIT();
Just inline the macro definition here.
+void acpi_pm_tco_ioport_writew(TCOIORegs *tr, uint32_t addr, uint32_t val) +{
- switch (addr) {
- case TCO_RLD:
timeouts_no = 0;if (can_start_tco_timer(tr)) {tr->tco.rld = tr->tco.tmr;tco_timer_reload();} else {tr->tco.rld = val;}break;- case TCO_DAT_IN:
tr->tco.din = val;tr->tco.sts1 |= SW_TCO_SMI;ich9_generate_smi();break;- case TCO_DAT_OUT:
tr->tco.dout = val;tr->tco.sts1 |= TCO_INT_STS;/* TODO: cause an interrupt, as selected by the TCO_INT_SEL bits */break;- case TCO1_STS:
tr->tco.sts1 = val & TCO1_STS_MASK;break;- case TCO2_STS:
tr->tco.sts2 = val & TCO2_STS_MASK;break;- case TCO1_CNT:
val &= TCO1_CNT_MASK;/* TCO_LOCK bit cannot be changed once set */tr->tco.cnt1 = (val & ~TCO_LOCK) | (tr->tco.cnt1 & TCO_LOCK);
This means that TCO_LOCK is never set in tr->tco.cnt1, I think? If I'm correct, it should be covered by tests.
if (can_start_tco_timer(tr)) {tr->tco.rld = tr->tco.tmr;tco_timer_reload();} else {tco_timer_stop();}break;
diff --git a/include/hw/acpi/tco.h b/include/hw/acpi/tco.h new file mode 100644 index 0000000..700532c --- /dev/null +++ b/include/hw/acpi/tco.h @@ -0,0 +1,139 @@ +/*
- QEMU ICH9 TCO emulation
- Copyright (c) 2015 Paulo Alcantara pcacjr@zytor.com
- Permission is hereby granted, free of charge, to any person obtaining a copy
- of this software and associated documentation files (the "Software"), to deal
- in the Software without restriction, including without limitation the rights
- to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- copies of the Software, and to permit persons to whom the Software is
- furnished to do so, subject to the following conditions:
- The above copyright notice and this permission notice shall be included in
- all copies or substantial portions of the Software.
- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- THE SOFTWARE.
- */
+#ifndef HW_ACPI_TCO_H +#define HW_ACPI_TCO_H
+#include "qemu/typedefs.h" +#include "qemu-common.h"
+/* TCO I/O register offsets */ +enum {
- TCO_RLD = 0x00,
- TCO_DAT_IN = 0x02,
- TCO_DAT_OUT = 0x03,
- TCO1_STS = 0x04,
- TCO2_STS = 0x06,
- TCO1_CNT = 0x08,
- TCO2_CNT = 0x0a,
- TCO_MESSAGE1 = 0x0c,
- TCO_MESSAGE2 = 0x0d,
- TCO_WDCNT = 0x0e,
- SW_IRQ_GEN = 0x10,
- TCO_TMR = 0x12,
+};
+/* TCO I/O register defaults */ +enum {
- TCO_RLD_DEFAULT = 0x0000,
- TCO_DAT_IN_DEFAULT = 0x00,
- TCO_DAT_OUT_DEFAULT = 0x00,
- TCO1_STS_DEFAULT = 0x0000,
- TCO2_STS_DEFAULT = 0x0000,
- TCO1_CNT_DEFAULT = 0x0000,
- TCO2_CNT_DEFAULT = 0x0008,
- TCO_MESSAGE1_DEFAULT = 0x00,
- TCO_MESSAGE2_DEFAULT = 0x00,
- TCO_WDCNT_DEFAULT = 0x00,
- TCO_TMR_DEFAULT = 0x0004,
- SW_IRQ_GEN_DEFAULT = 0x03,
+};
+/* TCO I/O register control/status bits */ +enum {
- SW_TCO_SMI = (1 << 1),
- TCO_INT_STS = (1 << 2),
- TCO_LOCK = (1 << 12),
- TCO_TMR_HLT = (1 << 11),
- TCO_TIMEOUT = (1 << 3),
- TCO_SECOND_TO_STS = (1 << 1),
- TCO_BOOT_STS = (1 << 2),
+};
+/* TCO I/O registers mask bits */ +enum {
- TCO_RLD_MASK = 0x3ff,
- TCO1_STS_MASK = 0xe870,
- TCO2_STS_MASK = 0xfff8,
- TCO1_CNT_MASK = 0xfeff,
- TCO_TMR_MASK = 0x3ff,
+};
+typedef struct TCOIORegs {
- struct {
uint16_t rld;uint8_t din;uint8_t dout;uint16_t sts1;uint16_t sts2;uint16_t cnt1;uint16_t cnt2;uint8_t msg1;uint8_t msg2;uint8_t wdcnt;uint16_t tmr;- } tco;
- uint8_t sw_irq_gen;
+} TCOIORegs;
+#define TCO_IO_REGS_DEFAULTS_INIT() \
- (TCOIORegs) { \
.tco = { \.rld = TCO_RLD_DEFAULT, \.din = TCO_DAT_IN_DEFAULT, \.dout = TCO_DAT_OUT_DEFAULT, \.sts1 = TCO1_STS_DEFAULT, \.sts2 = TCO2_STS_DEFAULT, \.cnt1 = TCO1_CNT_DEFAULT, \.cnt2 = TCO2_CNT_DEFAULT, \.msg1 = TCO_MESSAGE1_DEFAULT, \.msg2 = TCO_MESSAGE2_DEFAULT, \.wdcnt = TCO_WDCNT_DEFAULT, \.tmr = TCO_TMR_DEFAULT, \}, \.sw_irq_gen = SW_IRQ_GEN_DEFAULT \- }
No need for this definition. This and the *_DEFAULT definitions as well should be in the .c file.
+/* tco.c */ +void acpi_pm_tco_init(TCOIORegs *tr); +uint32_t acpi_pm_tco_ioport_readw(TCOIORegs *tr, uint32_t addr); +void acpi_pm_tco_ioport_writew(TCOIORegs *tr, uint32_t addr, uint32_t val);
+/* As per ICH9 spec, the internal timer has an error of ~0.6s on every tick */ +static inline int64_t tco_ticks_per_sec(void) +{
- return 600000000LL;
+}
+static inline int is_valid_tco_time(uint32_t val) +{
- /* values of 0 or 1 will be ignored by ICH */
- return val > 1;
+}
+static inline int can_start_tco_timer(TCOIORegs *tr) +{
- return !(tr->tco.cnt1 & TCO_TMR_HLT) && is_valid_tco_time(tr->tco.tmr);
+}
These three inlines should be in the .c file.
Don't be discouraged by the comments. It's good work!
Paolo
+#endif /* HW_ACPI_TCO_H */ diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index f4e522c..f41cca6 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -20,6 +20,9 @@ PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin); void ich9_lpc_pm_init(PCIDevice *pci_lpc); I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
+void ich9_generate_smi(void); +void ich9_generate_nmi(void);
#define ICH9_CC_SIZE (16 * 1024) /* 16KB */
#define TYPE_ICH9_LPC_DEVICE "ICH9-LPC" @@ -156,6 +159,8 @@ Object *ich9_lpc_find(void); #define ICH9_LPC_RCBA_BA_MASK Q35_MASK(32, 31, 14) #define ICH9_LPC_RCBA_EN 0x1 #define ICH9_LPC_RCBA_DEFAULT 0x0 +#define ICH9_LPC_RCBA_GCS 0x3410 +#define ICH9_LPC_RCBA_GCS_NO_REBOOT (1 << 5)
#define ICH9_LPC_PIC_NUM_PINS 16 #define ICH9_LPC_IOAPIC_NUM_PINS 24 @@ -180,7 +185,10 @@ Object *ich9_lpc_find(void); #define ICH9_PMIO_GPE0_LEN 16 #define ICH9_PMIO_SMI_EN 0x30 #define ICH9_PMIO_SMI_EN_APMC_EN (1 << 5) +#define ICH9_PMIO_SMI_EN_TCO_EN (1 << 13) #define ICH9_PMIO_SMI_STS 0x34 +#define ICH9_PMIO_TCO_RLD 0x60 +#define ICH9_PMIO_TCO_LEN 32
/* FADT ACPI_ENABLE/ACPI_DISABLE */ #define ICH9_APM_ACPI_ENABLE 0x2