* [PATCH 01/11] acpi: add API to build WDAT instructions
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-16 8:12 ` Ani Sinha
2026-02-06 13:14 ` [PATCH 02/11] machine: add "acpi-watchdog" property Igor Mammedov
` (11 subsequent siblings)
12 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
Add definitions for WDAT[1] actions/instructions
and build_append_wdat_ins() API to build table entries.
1)
"Hardware Watchdog Timers Design Specification"
https://uefi.org/acpi 'Watchdog Action Table (WDAT)'
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/wdat.h | 118 +++++++++++++++++++++++++++++++++++++++++
hw/acpi/aml-build.c | 14 +++++
2 files changed, 132 insertions(+)
create mode 100644 include/hw/acpi/wdat.h
diff --git a/include/hw/acpi/wdat.h b/include/hw/acpi/wdat.h
new file mode 100644
index 0000000000..89803eef4b
--- /dev/null
+++ b/include/hw/acpi/wdat.h
@@ -0,0 +1,118 @@
+/*
+ * Watchdog Action Table (WDAT) definitions
+ *
+ * Copyright Red Hat, Inc. 2026
+ * Author(s): Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef QEMU_HW_ACPI_WDAT_H
+#define QEMU_HW_ACPI_WDAT_H
+
+#include "hw/acpi/acpi-defs.h"
+
+/*
+ * Watchdog actions as described in
+ * "Hardware Watchdog Timers Design Specification"
+ * for link to spec see https://uefi.org/acpi
+ * 'Watchdog Action Table (WDAT)'
+ */
+typedef enum {
+ /*
+ * Restarts the watchdog timer's countdown. This action is
+ * required.
+ */
+ WDAT_ACTION_RESET = 0x1,
+ /*
+ * Returns the current countdown value of the watchdog hardware
+ * (in count intervals).
+ */
+ WDAT_ACTION_QUERY_CURRENT_COUNTDOWN_PERIOD = 0x4,
+ /*
+ * Returns the countdown value the watchdog hardware is
+ * configured to use when reset (in count intervals).
+ */
+ WDAT_ACTION_QUERY_COUNTDOWN_PERIOD = 0x5,
+ /*
+ * Sets the countdown value (in count intervals) to be used when
+ * the watchdog timer is reset. This action is required if
+ * WDAT_ACTION_RESET does not explicitly write a new
+ * countdown value to a register during a reset. Otherwise, this
+ * action is optional.
+ */
+ WDAT_ACTION_SET_COUNTDOWN_PERIOD = 0x6,
+ /*
+ * Determines if the watchdog hardware is currently in enabled/
+ * running state. The same result must occur when performed from
+ * both from enabled/stopped state and enabled/running state. If
+ * the watchdog hardware is disabled, results are indeterminate.
+ * This action is required.
+ */
+ WDAT_ACTION_QUERY_RUNNING_STATE = 0x8,
+ /*
+ * Starts the watchdog, if not already in running state. If the
+ * watchdog hardware is disabled, results are indeterminate.
+ * This action is required.
+ */
+ WDAT_ACTION_SET_RUNNING_STATE = 0x9,
+ /*
+ * Determines if the watchdog hardware is currently in enabled/
+ * stopped state. The same result must occur when performed from
+ * both the enabled/stopped state and enabled/running state. If
+ * the watchdog hardware is disabled, results are indeterminate.
+ * This action is required.
+ */
+ WDAT_ACTION_QUERY_STOPPED_STATE = 0xA,
+ /*
+ * Stops the watchdog, if not already in stopped state. If the
+ * watchdog hardware is disabled, results are indeterminate.
+ * This action is required.
+ */
+ WDAT_ACTION_SET_STOPPED_STATE = 0xB,
+ /*
+ * Determines if the watchdog hardware is configured to perform a
+ * reboot when the watchdog is fired.
+ */
+ WDAT_ACTION_QUERY_REBOOT = 0x10,
+ /*
+ * Configures the watchdog hardware to perform a reboot when it
+ * is fired.
+ */
+ WDAT_ACTION_SET_REBOOT = 0x11,
+ /*
+ * Determines if the watchdog hardware is configured to perform a
+ * system shutdown when fired.
+ */
+ WDAT_ACTION_QUERY_SHUTDOWN = 0x12,
+ /*
+ * Configures the watchdog hardware to perform a system shutdown
+ * when fired.
+ */
+ WDAT_ACTION_SET_SHUTDOWN = 0x13,
+ /*
+ * Determines if the current boot was caused by the watchdog
+ * firing. The boot status is required to be set if the watchdog
+ * fired and caused a reboot. It is recommended that the
+ * Watchdog Status be set if the watchdog fired and caused a
+ * shutdown. This action is required.
+ */
+ WDAT_ACTION_QUERY_WATCHDOG_STATUS = 0x20,
+ /*
+ * Sets the watchdog's boot status to the default value. This
+ * action is required.
+ */
+ WDAT_ACTION_SET_WATCHDOG_STATUS = 0x21,
+} WDATAction;
+
+#define WDAT_INS_READ_VALUE 0x0
+#define WDAT_INS_READ_COUNTDOWN 0x1
+#define WDAT_INS_WRITE_VALUE 0x2
+#define WDAT_INS_WRITE_COUNTDOWN 0x3
+#define WDAT_INS_PRESERVE_REGISTER 0x80
+
+void build_append_wdat_ins(GArray *table_data,
+ WDATAction action, uint8_t flags,
+ struct AcpiGenericAddress as,
+ uint32_t val, uint32_t mask);
+
+#endif /* QEMU_HW_ACPI_WDAT_H */
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index ea1c415b21..9f9ce28dcc 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -32,6 +32,7 @@
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_bridge.h"
#include "qemu/cutils.h"
+#include "hw/acpi/wdat.h"
static GArray *build_alloc_array(void)
{
@@ -2647,3 +2648,16 @@ Aml *aml_error_device(void)
return dev;
}
+
+void build_append_wdat_ins(GArray *table_data,
+ WDATAction action, uint8_t flags,
+ struct AcpiGenericAddress as,
+ uint32_t val, uint32_t mask)
+{
+ build_append_int_noprefix(table_data, action, 1); /* Watchdog Action */
+ build_append_int_noprefix(table_data, flags, 1); /* Instruction Flags */
+ build_append_int_noprefix(table_data, 0, 2); /* Reserved */
+ build_append_gas_from_struct(table_data, &as); /* Register Region */
+ build_append_int_noprefix(table_data, val, 4); /* Value */
+ build_append_int_noprefix(table_data, mask, 4); /* Mask */
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 01/11] acpi: add API to build WDAT instructions
2026-02-06 13:14 ` [PATCH 01/11] acpi: add API to build WDAT instructions Igor Mammedov
@ 2026-02-16 8:12 ` Ani Sinha
0 siblings, 0 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-16 8:12 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:45 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> Add definitions for WDAT[1] actions/instructions
> and build_append_wdat_ins() API to build table entries.
>
> 1)
> "Hardware Watchdog Timers Design Specification"
> https://uefi.org/acpi 'Watchdog Action Table (WDAT)'
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
> ---
> include/hw/acpi/wdat.h | 118 +++++++++++++++++++++++++++++++++++++++++
> hw/acpi/aml-build.c | 14 +++++
> 2 files changed, 132 insertions(+)
> create mode 100644 include/hw/acpi/wdat.h
>
> diff --git a/include/hw/acpi/wdat.h b/include/hw/acpi/wdat.h
> new file mode 100644
> index 0000000000..89803eef4b
> --- /dev/null
> +++ b/include/hw/acpi/wdat.h
> @@ -0,0 +1,118 @@
> +/*
> + * Watchdog Action Table (WDAT) definitions
> + *
> + * Copyright Red Hat, Inc. 2026
> + * Author(s): Igor Mammedov <imammedo@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QEMU_HW_ACPI_WDAT_H
> +#define QEMU_HW_ACPI_WDAT_H
> +
> +#include "hw/acpi/acpi-defs.h"
> +
> +/*
> + * Watchdog actions as described in
> + * "Hardware Watchdog Timers Design Specification"
> + * for link to spec see https://uefi.org/acpi
> + * 'Watchdog Action Table (WDAT)'
> + */
> +typedef enum {
> + /*
> + * Restarts the watchdog timer's countdown. This action is
> + * required.
> + */
> + WDAT_ACTION_RESET = 0x1,
> + /*
> + * Returns the current countdown value of the watchdog hardware
> + * (in count intervals).
> + */
> + WDAT_ACTION_QUERY_CURRENT_COUNTDOWN_PERIOD = 0x4,
> + /*
> + * Returns the countdown value the watchdog hardware is
> + * configured to use when reset (in count intervals).
> + */
> + WDAT_ACTION_QUERY_COUNTDOWN_PERIOD = 0x5,
> + /*
> + * Sets the countdown value (in count intervals) to be used when
> + * the watchdog timer is reset. This action is required if
> + * WDAT_ACTION_RESET does not explicitly write a new
> + * countdown value to a register during a reset. Otherwise, this
> + * action is optional.
> + */
> + WDAT_ACTION_SET_COUNTDOWN_PERIOD = 0x6,
> + /*
> + * Determines if the watchdog hardware is currently in enabled/
> + * running state. The same result must occur when performed from
> + * both from enabled/stopped state and enabled/running state. If
> + * the watchdog hardware is disabled, results are indeterminate.
> + * This action is required.
> + */
> + WDAT_ACTION_QUERY_RUNNING_STATE = 0x8,
> + /*
> + * Starts the watchdog, if not already in running state. If the
> + * watchdog hardware is disabled, results are indeterminate.
> + * This action is required.
> + */
> + WDAT_ACTION_SET_RUNNING_STATE = 0x9,
> + /*
> + * Determines if the watchdog hardware is currently in enabled/
> + * stopped state. The same result must occur when performed from
> + * both the enabled/stopped state and enabled/running state. If
> + * the watchdog hardware is disabled, results are indeterminate.
> + * This action is required.
> + */
> + WDAT_ACTION_QUERY_STOPPED_STATE = 0xA,
> + /*
> + * Stops the watchdog, if not already in stopped state. If the
> + * watchdog hardware is disabled, results are indeterminate.
> + * This action is required.
> + */
> + WDAT_ACTION_SET_STOPPED_STATE = 0xB,
> + /*
> + * Determines if the watchdog hardware is configured to perform a
> + * reboot when the watchdog is fired.
> + */
> + WDAT_ACTION_QUERY_REBOOT = 0x10,
> + /*
> + * Configures the watchdog hardware to perform a reboot when it
> + * is fired.
> + */
> + WDAT_ACTION_SET_REBOOT = 0x11,
> + /*
> + * Determines if the watchdog hardware is configured to perform a
> + * system shutdown when fired.
> + */
> + WDAT_ACTION_QUERY_SHUTDOWN = 0x12,
> + /*
> + * Configures the watchdog hardware to perform a system shutdown
> + * when fired.
> + */
> + WDAT_ACTION_SET_SHUTDOWN = 0x13,
> + /*
> + * Determines if the current boot was caused by the watchdog
> + * firing. The boot status is required to be set if the watchdog
> + * fired and caused a reboot. It is recommended that the
> + * Watchdog Status be set if the watchdog fired and caused a
> + * shutdown. This action is required.
> + */
> + WDAT_ACTION_QUERY_WATCHDOG_STATUS = 0x20,
> + /*
> + * Sets the watchdog's boot status to the default value. This
> + * action is required.
> + */
> + WDAT_ACTION_SET_WATCHDOG_STATUS = 0x21,
> +} WDATAction;
> +
> +#define WDAT_INS_READ_VALUE 0x0
> +#define WDAT_INS_READ_COUNTDOWN 0x1
> +#define WDAT_INS_WRITE_VALUE 0x2
> +#define WDAT_INS_WRITE_COUNTDOWN 0x3
> +#define WDAT_INS_PRESERVE_REGISTER 0x80
> +
> +void build_append_wdat_ins(GArray *table_data,
> + WDATAction action, uint8_t flags,
> + struct AcpiGenericAddress as,
> + uint32_t val, uint32_t mask);
> +
> +#endif /* QEMU_HW_ACPI_WDAT_H */
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ea1c415b21..9f9ce28dcc 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -32,6 +32,7 @@
> #include "hw/pci/pci_bus.h"
> #include "hw/pci/pci_bridge.h"
> #include "qemu/cutils.h"
> +#include "hw/acpi/wdat.h"
>
> static GArray *build_alloc_array(void)
> {
> @@ -2647,3 +2648,16 @@ Aml *aml_error_device(void)
>
> return dev;
> }
> +
> +void build_append_wdat_ins(GArray *table_data,
> + WDATAction action, uint8_t flags,
> + struct AcpiGenericAddress as,
> + uint32_t val, uint32_t mask)
> +{
> + build_append_int_noprefix(table_data, action, 1); /* Watchdog Action */
> + build_append_int_noprefix(table_data, flags, 1); /* Instruction Flags */
> + build_append_int_noprefix(table_data, 0, 2); /* Reserved */
> + build_append_gas_from_struct(table_data, &as); /* Register Region */
> + build_append_int_noprefix(table_data, val, 4); /* Value */
> + build_append_int_noprefix(table_data, mask, 4); /* Mask */
> +}
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 02/11] machine: add "acpi-watchdog" property
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
2026-02-06 13:14 ` [PATCH 01/11] acpi: add API to build WDAT instructions Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-16 8:23 ` Ani Sinha
2026-02-26 17:08 ` Philippe Mathieu-Daudé
2026-02-06 13:14 ` [PATCH 03/11] x86: q35: generate WDAT ACPI table Igor Mammedov
` (10 subsequent siblings)
12 siblings, 2 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
It will be used by following code to enable ACPI watchdog.
Initial support will bring it to Q35 and arm/virt machines using
respective iTCO and GWDT watchdogs as hardware backend.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/core/boards.h | 1 +
hw/core/machine.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/include/hw/core/boards.h b/include/hw/core/boards.h
index 07f8938752..25577c3aac 100644
--- a/include/hw/core/boards.h
+++ b/include/hw/core/boards.h
@@ -447,6 +447,7 @@ struct MachineState {
struct NVDIMMState *nvdimms_state;
struct NumaState *numa_state;
bool acpi_spcr_enabled;
+ bool acpi_watchdog;
};
/*
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6411e68856..4ca808fb73 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -499,6 +499,20 @@ static void machine_set_usb(Object *obj, bool value, Error **errp)
ms->usb_disabled = !value;
}
+static bool machine_get_acpi_watchdog(Object *obj, Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+
+ return ms->acpi_watchdog;
+}
+
+static void machine_set_acpi_watchdog(Object *obj, bool value, Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+
+ ms->acpi_watchdog = value;
+}
+
static bool machine_get_graphics(Object *obj, Error **errp)
{
MachineState *ms = MACHINE(obj);
@@ -1257,6 +1271,12 @@ static void machine_class_init(ObjectClass *oc, const void *data)
NULL, NULL);
object_class_property_set_description(oc, "memory",
"Memory size configuration");
+
+ object_class_property_add_bool(oc, "acpi-watchdog",
+ machine_get_acpi_watchdog, machine_set_acpi_watchdog);
+ object_class_property_set_description(oc, "acpi-watchdog",
+ "Set on/off to enable/disable ACPI watchdog."
+ " Default: off (ACPI watchdog is disabled).");
}
static void machine_class_base_init(ObjectClass *oc, const void *data)
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 02/11] machine: add "acpi-watchdog" property
2026-02-06 13:14 ` [PATCH 02/11] machine: add "acpi-watchdog" property Igor Mammedov
@ 2026-02-16 8:23 ` Ani Sinha
2026-02-26 17:08 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-16 8:23 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:44 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> It will be used by following code to enable ACPI watchdog.
> Initial support will bring it to Q35 and arm/virt machines using
> respective iTCO and GWDT watchdogs as hardware backend.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
> ---
> include/hw/core/boards.h | 1 +
> hw/core/machine.c | 20 ++++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/include/hw/core/boards.h b/include/hw/core/boards.h
> index 07f8938752..25577c3aac 100644
> --- a/include/hw/core/boards.h
> +++ b/include/hw/core/boards.h
> @@ -447,6 +447,7 @@ struct MachineState {
> struct NVDIMMState *nvdimms_state;
> struct NumaState *numa_state;
> bool acpi_spcr_enabled;
> + bool acpi_watchdog;
> };
>
> /*
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 6411e68856..4ca808fb73 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -499,6 +499,20 @@ static void machine_set_usb(Object *obj, bool value, Error **errp)
> ms->usb_disabled = !value;
> }
>
> +static bool machine_get_acpi_watchdog(Object *obj, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> +
> + return ms->acpi_watchdog;
> +}
> +
> +static void machine_set_acpi_watchdog(Object *obj, bool value, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> +
> + ms->acpi_watchdog = value;
> +}
> +
> static bool machine_get_graphics(Object *obj, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
> @@ -1257,6 +1271,12 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> NULL, NULL);
> object_class_property_set_description(oc, "memory",
> "Memory size configuration");
> +
> + object_class_property_add_bool(oc, "acpi-watchdog",
> + machine_get_acpi_watchdog, machine_set_acpi_watchdog);
> + object_class_property_set_description(oc, "acpi-watchdog",
> + "Set on/off to enable/disable ACPI watchdog."
> + " Default: off (ACPI watchdog is disabled).");
> }
>
> static void machine_class_base_init(ObjectClass *oc, const void *data)
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 02/11] machine: add "acpi-watchdog" property
2026-02-06 13:14 ` [PATCH 02/11] machine: add "acpi-watchdog" property Igor Mammedov
2026-02-16 8:23 ` Ani Sinha
@ 2026-02-26 17:08 ` Philippe Mathieu-Daudé
2026-02-27 8:23 ` Igor Mammedov
1 sibling, 1 reply; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-02-26 17:08 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, zhao1.liu,
rad, leif.lindholm
Hi Igor,
On 6/2/26 14:14, Igor Mammedov wrote:
> It will be used by following code to enable ACPI watchdog.
> Initial support will bring it to Q35 and arm/virt machines using
> respective iTCO and GWDT watchdogs as hardware backend.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/core/boards.h | 1 +
> hw/core/machine.c | 20 ++++++++++++++++++++
> 2 files changed, 21 insertions(+)
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 6411e68856..4ca808fb73 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -499,6 +499,20 @@ static void machine_set_usb(Object *obj, bool value, Error **errp)
> ms->usb_disabled = !value;
> }
>
> +static bool machine_get_acpi_watchdog(Object *obj, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> +
> + return ms->acpi_watchdog;
> +}
> +
> +static void machine_set_acpi_watchdog(Object *obj, bool value, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> +
> + ms->acpi_watchdog = value;
> +}
> +
> static bool machine_get_graphics(Object *obj, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
> @@ -1257,6 +1271,12 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> NULL, NULL);
> object_class_property_set_description(oc, "memory",
> "Memory size configuration");
> +
> + object_class_property_add_bool(oc, "acpi-watchdog",
> + machine_get_acpi_watchdog, machine_set_acpi_watchdog);
> + object_class_property_set_description(oc, "acpi-watchdog",
> + "Set on/off to enable/disable ACPI watchdog."
> + " Default: off (ACPI watchdog is disabled).");
Should we check for CONFIG_ACPI somewhere?
> }
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 02/11] machine: add "acpi-watchdog" property
2026-02-26 17:08 ` Philippe Mathieu-Daudé
@ 2026-02-27 8:23 ` Igor Mammedov
0 siblings, 0 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-27 8:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, mst, anisinha, pbonzini, peter.maydell,
shannon.zhaosl, zhao1.liu, rad, leif.lindholm
On Thu, 26 Feb 2026 18:08:09 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> Hi Igor,
>
> On 6/2/26 14:14, Igor Mammedov wrote:
> > It will be used by following code to enable ACPI watchdog.
> > Initial support will bring it to Q35 and arm/virt machines using
> > respective iTCO and GWDT watchdogs as hardware backend.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > include/hw/core/boards.h | 1 +
> > hw/core/machine.c | 20 ++++++++++++++++++++
> > 2 files changed, 21 insertions(+)
>
>
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 6411e68856..4ca808fb73 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -499,6 +499,20 @@ static void machine_set_usb(Object *obj, bool value, Error **errp)
> > ms->usb_disabled = !value;
> > }
> >
> > +static bool machine_get_acpi_watchdog(Object *obj, Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > +
> > + return ms->acpi_watchdog;
> > +}
> > +
> > +static void machine_set_acpi_watchdog(Object *obj, bool value, Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > +
> > + ms->acpi_watchdog = value;
> > +}
> > +
> > static bool machine_get_graphics(Object *obj, Error **errp)
> > {
> > MachineState *ms = MACHINE(obj);
> > @@ -1257,6 +1271,12 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> > NULL, NULL);
> > object_class_property_set_description(oc, "memory",
> > "Memory size configuration");
> > +
> > + object_class_property_add_bool(oc, "acpi-watchdog",
> > + machine_get_acpi_watchdog, machine_set_acpi_watchdog);
> > + object_class_property_set_description(oc, "acpi-watchdog",
> > + "Set on/off to enable/disable ACPI watchdog."
> > + " Default: off (ACPI watchdog is disabled).");
>
> Should we check for CONFIG_ACPI somewhere?
I do check it when I build wdat specific files.
Anyways, this part has been reworked see my reply in
8/11 thread with new more generic 'watchdog' option.
>
> > }
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 03/11] x86: q35: generate WDAT ACPI table
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
2026-02-06 13:14 ` [PATCH 01/11] acpi: add API to build WDAT instructions Igor Mammedov
2026-02-06 13:14 ` [PATCH 02/11] machine: add "acpi-watchdog" property Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-16 10:51 ` Ani Sinha
2026-02-06 13:14 ` [PATCH 04/11] tests: acpi: x86/q35: whitelist new WDAT table Igor Mammedov
` (9 subsequent siblings)
12 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
It will generate WDAT table [1] customized for TCO watchdog.
This allows Windows guests (Windows Server 2008/Vista+) to
use TCO watchdog using built-in generic driver, which
aleviates need to install vendor specific drivers.
Given that enabling it might change guest behaviour
(both Windows/Linux) the feature is disabled by default.
Users that need it can enable the feature with
following CLI option.
-machine 'acpi-watchdog=on'
1)
"Hardware Watchdog Timers Design Specification"
https://uefi.org/acpi 'Watchdog Action Table (WDAT)'
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/wdat-ich9.h | 15 +++++++
hw/acpi/meson.build | 3 +-
hw/acpi/wdat-ich9-stub.c | 15 +++++++
hw/acpi/wdat-ich9.c | 87 +++++++++++++++++++++++++++++++++++++
hw/i386/acpi-build.c | 12 +++++
5 files changed, 131 insertions(+), 1 deletion(-)
create mode 100644 include/hw/acpi/wdat-ich9.h
create mode 100644 hw/acpi/wdat-ich9-stub.c
create mode 100644 hw/acpi/wdat-ich9.c
diff --git a/include/hw/acpi/wdat-ich9.h b/include/hw/acpi/wdat-ich9.h
new file mode 100644
index 0000000000..60a2a6a112
--- /dev/null
+++ b/include/hw/acpi/wdat-ich9.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright Red Hat, Inc. 2026
+ * Author(s): Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef QEMU_HW_ACPI_WDAT_ICH9_H
+#define QEMU_HW_ACPI_WDAT_ICH9_H
+
+#include "hw/acpi/aml-build.h"
+
+void build_ich9_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
+ const char *oem_table_id, uint64_t tco_base);
+
+#endif /* QEMU_HW_ACPI_WDAT_ICH9_H */
diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index 56b5d1ec96..ba974b2f0f 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -24,7 +24,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_PCI_BRIDGE', if_true: files('pci-bridge.c'))
acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_true: files('pcihp.c'))
acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_false: files('acpi-pci-hotplug-stub.c'))
acpi_ss.add(when: 'CONFIG_ACPI_VIOT', if_true: files('viot.c'))
-acpi_ss.add(when: 'CONFIG_ACPI_ICH9', if_true: files('ich9.c', 'ich9_tco.c', 'ich9_timer.c'))
+acpi_ss.add(when: 'CONFIG_ACPI_ICH9', if_true: files('ich9.c', 'ich9_tco.c', 'ich9_timer.c', 'wdat-ich9.c'))
acpi_ss.add(when: 'CONFIG_ACPI_ERST', if_true: files('erst.c'))
acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
@@ -33,6 +33,7 @@ if have_tpm
endif
system_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))
system_ss.add(when: 'CONFIG_ACPI_PCI_BRIDGE', if_false: files('pci-bridge-stub.c'))
+system_ss.add(when: 'CONFIG_ACPI_ICH9', if_false: files('wdat-ich9-stub.c'))
system_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
system_ss.add(when: 'CONFIG_GHES_CPER', if_true: files('ghes_cper.c'))
system_ss.add(when: 'CONFIG_GHES_CPER', if_false: files('ghes_cper_stub.c'))
diff --git a/hw/acpi/wdat-ich9-stub.c b/hw/acpi/wdat-ich9-stub.c
new file mode 100644
index 0000000000..7c30ef3558
--- /dev/null
+++ b/hw/acpi/wdat-ich9-stub.c
@@ -0,0 +1,15 @@
+/*
+ * Copyright Red Hat, Inc. 2026
+ * Author(s): Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/wdat-ich9.h"
+
+void build_ich9_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
+ const char *oem_table_id, uint64_t tco_base)
+{
+ g_assert_not_reached();
+}
diff --git a/hw/acpi/wdat-ich9.c b/hw/acpi/wdat-ich9.c
new file mode 100644
index 0000000000..b1930dc71d
--- /dev/null
+++ b/hw/acpi/wdat-ich9.c
@@ -0,0 +1,87 @@
+/*
+ * TCO Watchdog Action Table (WDAT)
+ *
+ * Copyright Red Hat, Inc. 2026
+ * Author(s): Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/wdat.h"
+#include "hw/acpi/wdat-ich9.h"
+#include "hw/southbridge/ich9.h"
+
+#define TCO_REG(base, reg_offset, reg_width) { .space_id = AML_AS_SYSTEM_IO, \
+ .address = base + reg_offset, .bit_width = reg_width, \
+ .access_width = AML_WORD_ACC, };
+
+/*
+ * "Hardware Watchdog Timers Design Specification"
+ * https://uefi.org/acpi 'Watchdog Action Table (WDAT)'
+ *
+ * ICH9 specific implementation.
+ */
+void build_ich9_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
+ const char *oem_table_id, uint64_t tco_base)
+{
+ AcpiTable table = { .sig = "WDAT", .rev = 1, .oem_id = oem_id,
+ .oem_table_id = oem_table_id };
+ struct AcpiGenericAddress tco_rld = TCO_REG(tco_base, 0x0, 16);
+ struct AcpiGenericAddress tco2_sts = TCO_REG(tco_base, 0x6, 16);
+ struct AcpiGenericAddress tco1_cnt = TCO_REG(tco_base, 0x8, 16);
+ struct AcpiGenericAddress tco_tmr = TCO_REG(tco_base, 0x12, 16);
+
+ acpi_table_begin(&table, table_data);
+ build_append_int_noprefix(table_data, 0x20, 4); /* Watchdog Header Length */
+ build_append_int_noprefix(table_data, 0xff, 2); /* PCI Segment */
+ build_append_int_noprefix(table_data, 0xff, 1); /* PCI Bus Number */
+ build_append_int_noprefix(table_data, 0xff, 1); /* PCI Device Number */
+ build_append_int_noprefix(table_data, 0xff, 1); /* PCI Function Number */
+ build_append_int_noprefix(table_data, 0, 3); /* Reserved */
+ build_append_int_noprefix(table_data, 0x258, 4);/* Timer Period, ms */
+ build_append_int_noprefix(table_data, 0x3ff, 4);/* Maximum Count */
+ build_append_int_noprefix(table_data, 0x4, 4); /* Minimum Count */
+ /*
+ * WATCHDOG_ENABLED & WATCHDOG_STOPPED_IN_SLEEP_STATE
+ */
+ build_append_int_noprefix(table_data, 0x81, 1); /* Watchdog Flags */
+ build_append_int_noprefix(table_data, 0, 3); /* Reserved */
+ /*
+ * watchdog instruction entries
+ */
+ build_append_int_noprefix(table_data, 10, 4);
+ /* Action table */
+ build_append_wdat_ins(table_data, WDAT_ACTION_RESET,
+ WDAT_INS_WRITE_VALUE,
+ tco_rld, 0x1, 0x1ff);
+ build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_RUNNING_STATE,
+ WDAT_INS_READ_VALUE,
+ tco1_cnt, 0x0, 0x800);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_RUNNING_STATE,
+ WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
+ tco1_cnt, 0, 0x800);
+ build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_STOPPED_STATE,
+ WDAT_INS_READ_VALUE,
+ tco1_cnt, 0x800, 0x800);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_STOPPED_STATE,
+ WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
+ tco1_cnt, 0x800, 0x800);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_COUNTDOWN_PERIOD,
+ WDAT_INS_WRITE_COUNTDOWN,
+ tco_tmr, 0x0, 0x3FF);
+ build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_COUNTDOWN_PERIOD,
+ WDAT_INS_READ_COUNTDOWN,
+ tco_tmr, 0x0, 0x3FF);
+ build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_WATCHDOG_STATUS,
+ WDAT_INS_READ_VALUE,
+ tco2_sts, 0x2, 0x2);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_WATCHDOG_STATUS,
+ WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
+ tco2_sts, 0x2, 0x2);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_WATCHDOG_STATUS,
+ WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
+ tco2_sts, 0x4, 0x4);
+
+ acpi_table_end(linker, &table);
+}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 19c62362e3..abb3e936fd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -77,6 +77,7 @@
#include "hw/acpi/hmat.h"
#include "hw/acpi/viot.h"
+#include "hw/acpi/wdat-ich9.h"
#include CONFIG_DEVICES
@@ -109,6 +110,7 @@ typedef struct AcpiPmInfo {
uint16_t cpu_hp_io_base;
uint16_t pcihp_io_base;
uint16_t pcihp_io_len;
+ uint64_t tco_io_base;
} AcpiPmInfo;
typedef struct AcpiMiscInfo {
@@ -203,6 +205,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
pm->pcihp_io_len = 0;
pm->smi_on_cpuhp = false;
pm->smi_on_cpu_unplug = false;
+ pm->tco_io_base = 0;
assert(obj);
init_common_fadt_data(machine, obj, &pm->fadt);
@@ -224,6 +227,8 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
!!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT));
pm->smi_on_cpu_unplug =
!!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT));
+ pm->tco_io_base = object_property_get_uint(obj, ACPI_PM_PROP_PM_IO_BASE,
+ NULL) + ICH9_PMIO_TCO_RLD;
}
pm->pcihp_io_base =
object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
@@ -2079,6 +2084,13 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
acpi_add_table(table_offsets, tables_blob);
build_waet(tables_blob, tables->linker, x86ms->oem_id, x86ms->oem_table_id);
+ if (machine->acpi_watchdog) {
+ g_assert(pm.tco_io_base);
+ acpi_add_table(table_offsets, tables_blob);
+ build_ich9_wdat(tables_blob, tables->linker, x86ms->oem_id,
+ x86ms->oem_table_id, pm.tco_io_base);
+ }
+
/* Add tables supplied by user (if any) */
for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
unsigned len = acpi_table_len(u);
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 03/11] x86: q35: generate WDAT ACPI table
2026-02-06 13:14 ` [PATCH 03/11] x86: q35: generate WDAT ACPI table Igor Mammedov
@ 2026-02-16 10:51 ` Ani Sinha
0 siblings, 0 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-16 10:51 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:45 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> It will generate WDAT table [1] customized for TCO watchdog.
> This allows Windows guests (Windows Server 2008/Vista+) to
> use TCO watchdog using built-in generic driver, which
> aleviates need to install vendor specific drivers.
^^^^^^^^^^
typo.
>
> Given that enabling it might change guest behaviour
> (both Windows/Linux) the feature is disabled by default.
>
> Users that need it can enable the feature with
> following CLI option.
> -machine 'acpi-watchdog=on'
>
> 1)
> "Hardware Watchdog Timers Design Specification"
> https://uefi.org/acpi 'Watchdog Action Table (WDAT)'
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Other than below,
Reviewed-by: Ani Sinha <anisinha@redhat.com>
> ---
> include/hw/acpi/wdat-ich9.h | 15 +++++++
> hw/acpi/meson.build | 3 +-
> hw/acpi/wdat-ich9-stub.c | 15 +++++++
> hw/acpi/wdat-ich9.c | 87 +++++++++++++++++++++++++++++++++++++
> hw/i386/acpi-build.c | 12 +++++
> 5 files changed, 131 insertions(+), 1 deletion(-)
> create mode 100644 include/hw/acpi/wdat-ich9.h
> create mode 100644 hw/acpi/wdat-ich9-stub.c
> create mode 100644 hw/acpi/wdat-ich9.c
>
> diff --git a/include/hw/acpi/wdat-ich9.h b/include/hw/acpi/wdat-ich9.h
> new file mode 100644
> index 0000000000..60a2a6a112
> --- /dev/null
> +++ b/include/hw/acpi/wdat-ich9.h
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright Red Hat, Inc. 2026
> + * Author(s): Igor Mammedov <imammedo@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QEMU_HW_ACPI_WDAT_ICH9_H
> +#define QEMU_HW_ACPI_WDAT_ICH9_H
> +
> +#include "hw/acpi/aml-build.h"
> +
> +void build_ich9_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
> + const char *oem_table_id, uint64_t tco_base);
> +
> +#endif /* QEMU_HW_ACPI_WDAT_ICH9_H */
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index 56b5d1ec96..ba974b2f0f 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -24,7 +24,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_PCI_BRIDGE', if_true: files('pci-bridge.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_true: files('pcihp.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_false: files('acpi-pci-hotplug-stub.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_VIOT', if_true: files('viot.c'))
> -acpi_ss.add(when: 'CONFIG_ACPI_ICH9', if_true: files('ich9.c', 'ich9_tco.c', 'ich9_timer.c'))
> +acpi_ss.add(when: 'CONFIG_ACPI_ICH9', if_true: files('ich9.c', 'ich9_tco.c', 'ich9_timer.c', 'wdat-ich9.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_ERST', if_true: files('erst.c'))
> acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
> acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
> @@ -33,6 +33,7 @@ if have_tpm
> endif
> system_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))
> system_ss.add(when: 'CONFIG_ACPI_PCI_BRIDGE', if_false: files('pci-bridge-stub.c'))
> +system_ss.add(when: 'CONFIG_ACPI_ICH9', if_false: files('wdat-ich9-stub.c'))
> system_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
> system_ss.add(when: 'CONFIG_GHES_CPER', if_true: files('ghes_cper.c'))
> system_ss.add(when: 'CONFIG_GHES_CPER', if_false: files('ghes_cper_stub.c'))
> diff --git a/hw/acpi/wdat-ich9-stub.c b/hw/acpi/wdat-ich9-stub.c
> new file mode 100644
> index 0000000000..7c30ef3558
> --- /dev/null
> +++ b/hw/acpi/wdat-ich9-stub.c
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright Red Hat, Inc. 2026
> + * Author(s): Igor Mammedov <imammedo@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/acpi/wdat-ich9.h"
> +
> +void build_ich9_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
> + const char *oem_table_id, uint64_t tco_base)
> +{
> + g_assert_not_reached();
> +}
> diff --git a/hw/acpi/wdat-ich9.c b/hw/acpi/wdat-ich9.c
> new file mode 100644
> index 0000000000..b1930dc71d
> --- /dev/null
> +++ b/hw/acpi/wdat-ich9.c
> @@ -0,0 +1,87 @@
> +/*
> + * TCO Watchdog Action Table (WDAT)
> + *
> + * Copyright Red Hat, Inc. 2026
> + * Author(s): Igor Mammedov <imammedo@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/acpi/wdat.h"
> +#include "hw/acpi/wdat-ich9.h"
> +#include "hw/southbridge/ich9.h"
> +
> +#define TCO_REG(base, reg_offset, reg_width) { .space_id = AML_AS_SYSTEM_IO, \
> + .address = base + reg_offset, .bit_width = reg_width, \
> + .access_width = AML_WORD_ACC, };
> +
> +/*
> + * "Hardware Watchdog Timers Design Specification"
> + * https://uefi.org/acpi 'Watchdog Action Table (WDAT)'
> + *
> + * ICH9 specific implementation.
> + */
> +void build_ich9_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
> + const char *oem_table_id, uint64_t tco_base)
> +{
> + AcpiTable table = { .sig = "WDAT", .rev = 1, .oem_id = oem_id,
> + .oem_table_id = oem_table_id };
> + struct AcpiGenericAddress tco_rld = TCO_REG(tco_base, 0x0, 16);
> + struct AcpiGenericAddress tco2_sts = TCO_REG(tco_base, 0x6, 16);
> + struct AcpiGenericAddress tco1_cnt = TCO_REG(tco_base, 0x8, 16);
> + struct AcpiGenericAddress tco_tmr = TCO_REG(tco_base, 0x12, 16);
> +
> + acpi_table_begin(&table, table_data);
> + build_append_int_noprefix(table_data, 0x20, 4); /* Watchdog Header Length */
> + build_append_int_noprefix(table_data, 0xff, 2); /* PCI Segment */
> + build_append_int_noprefix(table_data, 0xff, 1); /* PCI Bus Number */
> + build_append_int_noprefix(table_data, 0xff, 1); /* PCI Device Number */
> + build_append_int_noprefix(table_data, 0xff, 1); /* PCI Function Number */
> + build_append_int_noprefix(table_data, 0, 3); /* Reserved */
> + build_append_int_noprefix(table_data, 0x258, 4);/* Timer Period, ms */
> + build_append_int_noprefix(table_data, 0x3ff, 4);/* Maximum Count */
> + build_append_int_noprefix(table_data, 0x4, 4); /* Minimum Count */
How are you getting the values for the above three (timer period, min
count and max count)? A comment will help here.
> + /*
> + * WATCHDOG_ENABLED & WATCHDOG_STOPPED_IN_SLEEP_STATE
> + */
> + build_append_int_noprefix(table_data, 0x81, 1); /* Watchdog Flags */
> + build_append_int_noprefix(table_data, 0, 3); /* Reserved */
> + /*
> + * watchdog instruction entries
> + */
> + build_append_int_noprefix(table_data, 10, 4);
It might help to add a comment that we are adding 10 (decimal 10 even
though other values above are in hex) instruction entries below.
> + /* Action table */
> + build_append_wdat_ins(table_data, WDAT_ACTION_RESET,
> + WDAT_INS_WRITE_VALUE,
> + tco_rld, 0x1, 0x1ff);
> + build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_RUNNING_STATE,
> + WDAT_INS_READ_VALUE,
> + tco1_cnt, 0x0, 0x800);
> + build_append_wdat_ins(table_data, WDAT_ACTION_SET_RUNNING_STATE,
> + WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
> + tco1_cnt, 0, 0x800);
> + build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_STOPPED_STATE,
> + WDAT_INS_READ_VALUE,
> + tco1_cnt, 0x800, 0x800);
> + build_append_wdat_ins(table_data, WDAT_ACTION_SET_STOPPED_STATE,
> + WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
> + tco1_cnt, 0x800, 0x800);
> + build_append_wdat_ins(table_data, WDAT_ACTION_SET_COUNTDOWN_PERIOD,
> + WDAT_INS_WRITE_COUNTDOWN,
> + tco_tmr, 0x0, 0x3FF);
> + build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_COUNTDOWN_PERIOD,
> + WDAT_INS_READ_COUNTDOWN,
> + tco_tmr, 0x0, 0x3FF);
> + build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_WATCHDOG_STATUS,
> + WDAT_INS_READ_VALUE,
> + tco2_sts, 0x2, 0x2);
> + build_append_wdat_ins(table_data, WDAT_ACTION_SET_WATCHDOG_STATUS,
> + WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
> + tco2_sts, 0x2, 0x2);
> + build_append_wdat_ins(table_data, WDAT_ACTION_SET_WATCHDOG_STATUS,
> + WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
> + tco2_sts, 0x4, 0x4);
> +
> + acpi_table_end(linker, &table);
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 19c62362e3..abb3e936fd 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -77,6 +77,7 @@
>
> #include "hw/acpi/hmat.h"
> #include "hw/acpi/viot.h"
> +#include "hw/acpi/wdat-ich9.h"
>
> #include CONFIG_DEVICES
>
> @@ -109,6 +110,7 @@ typedef struct AcpiPmInfo {
> uint16_t cpu_hp_io_base;
> uint16_t pcihp_io_base;
> uint16_t pcihp_io_len;
> + uint64_t tco_io_base;
> } AcpiPmInfo;
>
> typedef struct AcpiMiscInfo {
> @@ -203,6 +205,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
> pm->pcihp_io_len = 0;
> pm->smi_on_cpuhp = false;
> pm->smi_on_cpu_unplug = false;
> + pm->tco_io_base = 0;
>
> assert(obj);
> init_common_fadt_data(machine, obj, &pm->fadt);
> @@ -224,6 +227,8 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
> !!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT));
> pm->smi_on_cpu_unplug =
> !!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT));
> + pm->tco_io_base = object_property_get_uint(obj, ACPI_PM_PROP_PM_IO_BASE,
> + NULL) + ICH9_PMIO_TCO_RLD;
> }
> pm->pcihp_io_base =
> object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> @@ -2079,6 +2084,13 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> acpi_add_table(table_offsets, tables_blob);
> build_waet(tables_blob, tables->linker, x86ms->oem_id, x86ms->oem_table_id);
>
> + if (machine->acpi_watchdog) {
> + g_assert(pm.tco_io_base);
> + acpi_add_table(table_offsets, tables_blob);
> + build_ich9_wdat(tables_blob, tables->linker, x86ms->oem_id,
> + x86ms->oem_table_id, pm.tco_io_base);
> + }
> +
> /* Add tables supplied by user (if any) */
> for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
> unsigned len = acpi_table_len(u);
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 04/11] tests: acpi: x86/q35: whitelist new WDAT table
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (2 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 03/11] x86: q35: generate WDAT ACPI table Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-16 9:50 ` Ani Sinha
2026-02-06 13:14 ` [PATCH 05/11] tests: acpi: x86/q35: add WDAT table test case Igor Mammedov
` (8 subsequent siblings)
12 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 +
tests/data/acpi/x86/q35/WDAT.wdat | 0
2 files changed, 1 insertion(+)
create mode 100644 tests/data/acpi/x86/q35/WDAT.wdat
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..14b680debe 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/x86/q35/WDAT.wdat",
diff --git a/tests/data/acpi/x86/q35/WDAT.wdat b/tests/data/acpi/x86/q35/WDAT.wdat
new file mode 100644
index 0000000000..e69de29bb2
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 04/11] tests: acpi: x86/q35: whitelist new WDAT table
2026-02-06 13:14 ` [PATCH 04/11] tests: acpi: x86/q35: whitelist new WDAT table Igor Mammedov
@ 2026-02-16 9:50 ` Ani Sinha
0 siblings, 0 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-16 9:50 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:45 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Acked-by: Ani Sinha <anisinha@redhat.com>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 1 +
> tests/data/acpi/x86/q35/WDAT.wdat | 0
> 2 files changed, 1 insertion(+)
> create mode 100644 tests/data/acpi/x86/q35/WDAT.wdat
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..14b680debe 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,2 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/x86/q35/WDAT.wdat",
> diff --git a/tests/data/acpi/x86/q35/WDAT.wdat b/tests/data/acpi/x86/q35/WDAT.wdat
> new file mode 100644
> index 0000000000..e69de29bb2
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 05/11] tests: acpi: x86/q35: add WDAT table test case
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (3 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 04/11] tests: acpi: x86/q35: whitelist new WDAT table Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-16 9:51 ` Ani Sinha
2026-02-06 13:14 ` [PATCH 06/11] tests: acpi: x86/q35: update expected WDAT blob Igor Mammedov
` (7 subsequent siblings)
12 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index e489d94331..25345dd035 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -2511,6 +2511,18 @@ static void test_acpi_isapc_smbios_legacy(void)
free_test_data(&data);
}
+static void test_acpi_q35_wdat(void)
+{
+ test_data data = {
+ .machine = MACHINE_Q35,
+ .arch = "x86",
+ .variant = ".wdat",
+ };
+
+ test_acpi_one("-machine acpi-watchdog=on", &data);
+ free_test_data(&data);
+}
+
static void test_oem_fields(test_data *data)
{
int i;
@@ -2805,6 +2817,7 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/q35/cxl", test_acpi_q35_cxl);
#endif
qtest_add_func("acpi/q35/slic", test_acpi_q35_slic);
+ qtest_add_func("acpi/q35/wdat", test_acpi_q35_wdat);
}
if (qtest_has_machine("microvm")) {
qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 05/11] tests: acpi: x86/q35: add WDAT table test case
2026-02-06 13:14 ` [PATCH 05/11] tests: acpi: x86/q35: add WDAT table test case Igor Mammedov
@ 2026-02-16 9:51 ` Ani Sinha
0 siblings, 0 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-16 9:51 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:45 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index e489d94331..25345dd035 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -2511,6 +2511,18 @@ static void test_acpi_isapc_smbios_legacy(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_q35_wdat(void)
> +{
> + test_data data = {
> + .machine = MACHINE_Q35,
> + .arch = "x86",
> + .variant = ".wdat",
> + };
> +
> + test_acpi_one("-machine acpi-watchdog=on", &data);
> + free_test_data(&data);
> +}
> +
> static void test_oem_fields(test_data *data)
> {
> int i;
> @@ -2805,6 +2817,7 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/q35/cxl", test_acpi_q35_cxl);
> #endif
> qtest_add_func("acpi/q35/slic", test_acpi_q35_slic);
> + qtest_add_func("acpi/q35/wdat", test_acpi_q35_wdat);
> }
> if (qtest_has_machine("microvm")) {
> qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 06/11] tests: acpi: x86/q35: update expected WDAT blob
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (4 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 05/11] tests: acpi: x86/q35: add WDAT table test case Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-17 5:34 ` Ani Sinha
2026-02-06 13:14 ` [PATCH 07/11] arm: add tracing events to sbsa_gwdt Igor Mammedov
` (6 subsequent siblings)
12 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
replace blank table with a new one:
+[000h 0000 4] Signature : "WDAT" [Watchdog Action Table]
+[004h 0004 4] Table Length : 00000134
+[008h 0008 1] Revision : 01
+[009h 0009 1] Checksum : 31
+[00Ah 0010 6] Oem ID : "BOCHS "
+[010h 0016 8] Oem Table ID : "BXPC "
+[018h 0024 4] Oem Revision : 00000001
+[01Ch 0028 4] Asl Compiler ID : "BXPC"
+[020h 0032 4] Asl Compiler Revision : 00000001
+
+[024h 0036 4] Header Length : 00000020
+[028h 0040 2] PCI Segment : 00FF
+[02Ah 0042 1] PCI Bus : FF
+[02Bh 0043 1] PCI Device : FF
+[02Ch 0044 1] PCI Function : FF
+[02Dh 0045 3] Reserved : 000000
+[030h 0048 4] Timer Period : 00000258
+[034h 0052 4] Max Count : 000003FF
+[038h 0056 4] Min Count : 00000004
+[03Ch 0060 1] Flags (decoded below) : 81
+ Enabled : 1
+ Stopped When Asleep : 1
+[03Dh 0061 3] Reserved : 000000
+[040h 0064 4] Watchdog Entry Count : 0000000A
+
+[044h 0068 1] Watchdog Action : 01
+[045h 0069 1] Instruction : 02
+[046h 0070 2] Reserved : 0000
+
+[048h 0072 12] Register Region : [Generic Address Structure]
+[048h 0072 1] Space ID : 01 [SystemIO]
+[049h 0073 1] Bit Width : 10
+[04Ah 0074 1] Bit Offset : 00
+[04Bh 0075 1] Encoded Access Width : 02 [Word Access:16]
+[04Ch 0076 8] Address : 0000000000000660
+
+[054h 0084 4] Value : 00000001
+[058h 0088 4] Register Mask : 000001FF
+
+[05Ch 0092 1] Watchdog Action : 08
+[05Dh 0093 1] Instruction : 00
+[05Eh 0094 2] Reserved : 0000
+
+[060h 0096 12] Register Region : [Generic Address Structure]
+[060h 0096 1] Space ID : 01 [SystemIO]
+[061h 0097 1] Bit Width : 10
+[062h 0098 1] Bit Offset : 00
+[063h 0099 1] Encoded Access Width : 02 [Word Access:16]
+[064h 0100 8] Address : 0000000000000668
+
+[06Ch 0108 4] Value : 00000000
+[070h 0112 4] Register Mask : 00000800
+
+[074h 0116 1] Watchdog Action : 09
+[075h 0117 1] Instruction : 82
+[076h 0118 2] Reserved : 0000
+
+[078h 0120 12] Register Region : [Generic Address Structure]
+[078h 0120 1] Space ID : 01 [SystemIO]
+[079h 0121 1] Bit Width : 10
+[07Ah 0122 1] Bit Offset : 00
+[07Bh 0123 1] Encoded Access Width : 02 [Word Access:16]
+[07Ch 0124 8] Address : 0000000000000668
+
+[084h 0132 4] Value : 00000000
+[088h 0136 4] Register Mask : 00000800
+
+[08Ch 0140 1] Watchdog Action : 0A
+[08Dh 0141 1] Instruction : 00
+[08Eh 0142 2] Reserved : 0000
+
+[090h 0144 12] Register Region : [Generic Address Structure]
+[090h 0144 1] Space ID : 01 [SystemIO]
+[091h 0145 1] Bit Width : 10
+[092h 0146 1] Bit Offset : 00
+[093h 0147 1] Encoded Access Width : 02 [Word Access:16]
+[094h 0148 8] Address : 0000000000000668
+
+[09Ch 0156 4] Value : 00000800
+[0A0h 0160 4] Register Mask : 00000800
+
+[0A4h 0164 1] Watchdog Action : 0B
+[0A5h 0165 1] Instruction : 82
+[0A6h 0166 2] Reserved : 0000
+
+[0A8h 0168 12] Register Region : [Generic Address Structure]
+[0A8h 0168 1] Space ID : 01 [SystemIO]
+[0A9h 0169 1] Bit Width : 10
+[0AAh 0170 1] Bit Offset : 00
+[0ABh 0171 1] Encoded Access Width : 02 [Word Access:16]
+[0ACh 0172 8] Address : 0000000000000668
+
+[0B4h 0180 4] Value : 00000800
+[0B8h 0184 4] Register Mask : 00000800
+
+[0BCh 0188 1] Watchdog Action : 06
+[0BDh 0189 1] Instruction : 03
+[0BEh 0190 2] Reserved : 0000
+
+[0C0h 0192 12] Register Region : [Generic Address Structure]
+[0C0h 0192 1] Space ID : 01 [SystemIO]
+[0C1h 0193 1] Bit Width : 10
+[0C2h 0194 1] Bit Offset : 00
+[0C3h 0195 1] Encoded Access Width : 02 [Word Access:16]
+[0C4h 0196 8] Address : 0000000000000672
+
+[0CCh 0204 4] Value : 00000000
+[0D0h 0208 4] Register Mask : 000003FF
+
+[0D4h 0212 1] Watchdog Action : 05
+[0D5h 0213 1] Instruction : 01
+[0D6h 0214 2] Reserved : 0000
+
+[0D8h 0216 12] Register Region : [Generic Address Structure]
+[0D8h 0216 1] Space ID : 01 [SystemIO]
+[0D9h 0217 1] Bit Width : 10
+[0DAh 0218 1] Bit Offset : 00
+[0DBh 0219 1] Encoded Access Width : 02 [Word Access:16]
+[0DCh 0220 8] Address : 0000000000000672
+
+[0E4h 0228 4] Value : 00000000
+[0E8h 0232 4] Register Mask : 000003FF
+
+[0ECh 0236 1] Watchdog Action : 20
+[0EDh 0237 1] Instruction : 00
+[0EEh 0238 2] Reserved : 0000
+
+[0F0h 0240 12] Register Region : [Generic Address Structure]
+[0F0h 0240 1] Space ID : 01 [SystemIO]
+[0F1h 0241 1] Bit Width : 10
+[0F2h 0242 1] Bit Offset : 00
+[0F3h 0243 1] Encoded Access Width : 02 [Word Access:16]
+[0F4h 0244 8] Address : 0000000000000666
+
+[0FCh 0252 4] Value : 00000002
+[100h 0256 4] Register Mask : 00000002
+
+[104h 0260 1] Watchdog Action : 21
+[105h 0261 1] Instruction : 82
+[106h 0262 2] Reserved : 0000
+
+[108h 0264 12] Register Region : [Generic Address Structure]
+[108h 0264 1] Space ID : 01 [SystemIO]
+[109h 0265 1] Bit Width : 10
+[10Ah 0266 1] Bit Offset : 00
+[10Bh 0267 1] Encoded Access Width : 02 [Word Access:16]
+[10Ch 0268 8] Address : 0000000000000666
+
+[114h 0276 4] Value : 00000002
+[118h 0280 4] Register Mask : 00000002
+
+[11Ch 0284 1] Watchdog Action : 21
+[11Dh 0285 1] Instruction : 82
+[11Eh 0286 2] Reserved : 0000
+
+[120h 0288 12] Register Region : [Generic Address Structure]
+[120h 0288 1] Space ID : 01 [SystemIO]
+[121h 0289 1] Bit Width : 10
+[122h 0290 1] Bit Offset : 00
+[123h 0291 1] Encoded Access Width : 02 [Word Access:16]
+[124h 0292 8] Address : 0000000000000666
+
+[12Ch 0300 4] Value : 00000004
+[130h 0304 4] Register Mask : 00000004
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
tests/data/acpi/x86/q35/WDAT.wdat | Bin 0 -> 308 bytes
2 files changed, 1 deletion(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 14b680debe..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/x86/q35/WDAT.wdat",
diff --git a/tests/data/acpi/x86/q35/WDAT.wdat b/tests/data/acpi/x86/q35/WDAT.wdat
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..e1e056b390e51c49f11f7b0d35e4e7c55d8067b1 100644
GIT binary patch
literal 308
zcmaivF$w}f3`JkEx{TV)5#kX9FW|0)oq~c}T8Xtc(2GYhPO+$9AY^#?`OH6WH>b6L
zth#-_J6bm!dNVTt;vLff$K8QJ!5f$W4`2?6f@Jj4QYvm-AjK}Dx5`T;m_2{<;x*QX
lL4w7nzVfzt``kzV@waK@$$at15&T;93Z<9T{~wRc(JwJ97XbhO
literal 0
HcmV?d00001
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 06/11] tests: acpi: x86/q35: update expected WDAT blob
2026-02-06 13:14 ` [PATCH 06/11] tests: acpi: x86/q35: update expected WDAT blob Igor Mammedov
@ 2026-02-17 5:34 ` Ani Sinha
0 siblings, 0 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-17 5:34 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:45 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> replace blank table with a new one:
>
> +[000h 0000 4] Signature : "WDAT" [Watchdog Action Table]
> +[004h 0004 4] Table Length : 00000134
> +[008h 0008 1] Revision : 01
> +[009h 0009 1] Checksum : 31
> +[00Ah 0010 6] Oem ID : "BOCHS "
> +[010h 0016 8] Oem Table ID : "BXPC "
> +[018h 0024 4] Oem Revision : 00000001
> +[01Ch 0028 4] Asl Compiler ID : "BXPC"
> +[020h 0032 4] Asl Compiler Revision : 00000001
> +
> +[024h 0036 4] Header Length : 00000020
> +[028h 0040 2] PCI Segment : 00FF
> +[02Ah 0042 1] PCI Bus : FF
> +[02Bh 0043 1] PCI Device : FF
> +[02Ch 0044 1] PCI Function : FF
> +[02Dh 0045 3] Reserved : 000000
> +[030h 0048 4] Timer Period : 00000258
> +[034h 0052 4] Max Count : 000003FF
> +[038h 0056 4] Min Count : 00000004
> +[03Ch 0060 1] Flags (decoded below) : 81
> + Enabled : 1
> + Stopped When Asleep : 1
> +[03Dh 0061 3] Reserved : 000000
> +[040h 0064 4] Watchdog Entry Count : 0000000A
> +
> +[044h 0068 1] Watchdog Action : 01
> +[045h 0069 1] Instruction : 02
> +[046h 0070 2] Reserved : 0000
> +
> +[048h 0072 12] Register Region : [Generic Address Structure]
> +[048h 0072 1] Space ID : 01 [SystemIO]
> +[049h 0073 1] Bit Width : 10
> +[04Ah 0074 1] Bit Offset : 00
> +[04Bh 0075 1] Encoded Access Width : 02 [Word Access:16]
> +[04Ch 0076 8] Address : 0000000000000660
> +
> +[054h 0084 4] Value : 00000001
> +[058h 0088 4] Register Mask : 000001FF
> +
> +[05Ch 0092 1] Watchdog Action : 08
> +[05Dh 0093 1] Instruction : 00
> +[05Eh 0094 2] Reserved : 0000
> +
> +[060h 0096 12] Register Region : [Generic Address Structure]
> +[060h 0096 1] Space ID : 01 [SystemIO]
> +[061h 0097 1] Bit Width : 10
> +[062h 0098 1] Bit Offset : 00
> +[063h 0099 1] Encoded Access Width : 02 [Word Access:16]
> +[064h 0100 8] Address : 0000000000000668
> +
> +[06Ch 0108 4] Value : 00000000
> +[070h 0112 4] Register Mask : 00000800
> +
> +[074h 0116 1] Watchdog Action : 09
> +[075h 0117 1] Instruction : 82
> +[076h 0118 2] Reserved : 0000
> +
> +[078h 0120 12] Register Region : [Generic Address Structure]
> +[078h 0120 1] Space ID : 01 [SystemIO]
> +[079h 0121 1] Bit Width : 10
> +[07Ah 0122 1] Bit Offset : 00
> +[07Bh 0123 1] Encoded Access Width : 02 [Word Access:16]
> +[07Ch 0124 8] Address : 0000000000000668
> +
> +[084h 0132 4] Value : 00000000
> +[088h 0136 4] Register Mask : 00000800
> +
> +[08Ch 0140 1] Watchdog Action : 0A
> +[08Dh 0141 1] Instruction : 00
> +[08Eh 0142 2] Reserved : 0000
> +
> +[090h 0144 12] Register Region : [Generic Address Structure]
> +[090h 0144 1] Space ID : 01 [SystemIO]
> +[091h 0145 1] Bit Width : 10
> +[092h 0146 1] Bit Offset : 00
> +[093h 0147 1] Encoded Access Width : 02 [Word Access:16]
> +[094h 0148 8] Address : 0000000000000668
> +
> +[09Ch 0156 4] Value : 00000800
> +[0A0h 0160 4] Register Mask : 00000800
> +
> +[0A4h 0164 1] Watchdog Action : 0B
> +[0A5h 0165 1] Instruction : 82
> +[0A6h 0166 2] Reserved : 0000
> +
> +[0A8h 0168 12] Register Region : [Generic Address Structure]
> +[0A8h 0168 1] Space ID : 01 [SystemIO]
> +[0A9h 0169 1] Bit Width : 10
> +[0AAh 0170 1] Bit Offset : 00
> +[0ABh 0171 1] Encoded Access Width : 02 [Word Access:16]
> +[0ACh 0172 8] Address : 0000000000000668
> +
> +[0B4h 0180 4] Value : 00000800
> +[0B8h 0184 4] Register Mask : 00000800
> +
> +[0BCh 0188 1] Watchdog Action : 06
> +[0BDh 0189 1] Instruction : 03
> +[0BEh 0190 2] Reserved : 0000
> +
> +[0C0h 0192 12] Register Region : [Generic Address Structure]
> +[0C0h 0192 1] Space ID : 01 [SystemIO]
> +[0C1h 0193 1] Bit Width : 10
> +[0C2h 0194 1] Bit Offset : 00
> +[0C3h 0195 1] Encoded Access Width : 02 [Word Access:16]
> +[0C4h 0196 8] Address : 0000000000000672
> +
> +[0CCh 0204 4] Value : 00000000
> +[0D0h 0208 4] Register Mask : 000003FF
> +
> +[0D4h 0212 1] Watchdog Action : 05
> +[0D5h 0213 1] Instruction : 01
> +[0D6h 0214 2] Reserved : 0000
> +
> +[0D8h 0216 12] Register Region : [Generic Address Structure]
> +[0D8h 0216 1] Space ID : 01 [SystemIO]
> +[0D9h 0217 1] Bit Width : 10
> +[0DAh 0218 1] Bit Offset : 00
> +[0DBh 0219 1] Encoded Access Width : 02 [Word Access:16]
> +[0DCh 0220 8] Address : 0000000000000672
> +
> +[0E4h 0228 4] Value : 00000000
> +[0E8h 0232 4] Register Mask : 000003FF
> +
> +[0ECh 0236 1] Watchdog Action : 20
> +[0EDh 0237 1] Instruction : 00
> +[0EEh 0238 2] Reserved : 0000
> +
> +[0F0h 0240 12] Register Region : [Generic Address Structure]
> +[0F0h 0240 1] Space ID : 01 [SystemIO]
> +[0F1h 0241 1] Bit Width : 10
> +[0F2h 0242 1] Bit Offset : 00
> +[0F3h 0243 1] Encoded Access Width : 02 [Word Access:16]
> +[0F4h 0244 8] Address : 0000000000000666
> +
> +[0FCh 0252 4] Value : 00000002
> +[100h 0256 4] Register Mask : 00000002
> +
> +[104h 0260 1] Watchdog Action : 21
> +[105h 0261 1] Instruction : 82
> +[106h 0262 2] Reserved : 0000
> +
> +[108h 0264 12] Register Region : [Generic Address Structure]
> +[108h 0264 1] Space ID : 01 [SystemIO]
> +[109h 0265 1] Bit Width : 10
> +[10Ah 0266 1] Bit Offset : 00
> +[10Bh 0267 1] Encoded Access Width : 02 [Word Access:16]
> +[10Ch 0268 8] Address : 0000000000000666
> +
> +[114h 0276 4] Value : 00000002
> +[118h 0280 4] Register Mask : 00000002
> +
> +[11Ch 0284 1] Watchdog Action : 21
> +[11Dh 0285 1] Instruction : 82
> +[11Eh 0286 2] Reserved : 0000
> +
> +[120h 0288 12] Register Region : [Generic Address Structure]
> +[120h 0288 1] Space ID : 01 [SystemIO]
> +[121h 0289 1] Bit Width : 10
> +[122h 0290 1] Bit Offset : 00
> +[123h 0291 1] Encoded Access Width : 02 [Word Access:16]
> +[124h 0292 8] Address : 0000000000000666
> +
> +[12Ch 0300 4] Value : 00000004
> +[130h 0304 4] Register Mask : 00000004
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
looks OK from a quick look.
Acked-by: Ani Sinha <anisinha@redhat.com>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 1 -
> tests/data/acpi/x86/q35/WDAT.wdat | Bin 0 -> 308 bytes
> 2 files changed, 1 deletion(-)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index 14b680debe..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,2 +1 @@
> /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/x86/q35/WDAT.wdat",
> diff --git a/tests/data/acpi/x86/q35/WDAT.wdat b/tests/data/acpi/x86/q35/WDAT.wdat
> index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..e1e056b390e51c49f11f7b0d35e4e7c55d8067b1 100644
> GIT binary patch
> literal 308
> zcmaivF$w}f3`JkEx{TV)5#kX9FW|0)oq~c}T8Xtc(2GYhPO+$9AY^#?`OH6WH>b6L
> zth#-_J6bm!dNVTt;vLff$K8QJ!5f$W4`2?6f@Jj4QYvm-AjK}Dx5`T;m_2{<;x*QX
> lL4w7nzVfzt``kzV@waK@$$at15&T;93Z<9T{~wRc(JwJ97XbhO
>
> literal 0
> HcmV?d00001
>
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 07/11] arm: add tracing events to sbsa_gwdt
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (5 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 06/11] tests: acpi: x86/q35: update expected WDAT blob Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-16 10:22 ` Ani Sinha
2026-02-06 13:14 ` [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table Igor Mammedov
` (5 subsequent siblings)
12 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/watchdog/sbsa_gwdt.c | 8 ++++++++
hw/watchdog/trace-events | 9 +++++++++
2 files changed, 17 insertions(+)
diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 7ade5c6f18..d159e61c34 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -24,6 +24,7 @@
#include "migration/vmstate.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "trace.h"
static const VMStateDescription vmstate_sbsa_gwdt = {
.name = "sbsa-gwdt",
@@ -62,6 +63,7 @@ static uint64_t sbsa_gwdt_rread(void *opaque, hwaddr addr, unsigned int size)
qemu_log_mask(LOG_GUEST_ERROR, "bad address in refresh frame read :"
" 0x%x\n", (int)addr);
}
+ trace_sbsa_gwdt_refresh_read(addr, ret);
return ret;
}
@@ -93,6 +95,7 @@ static uint64_t sbsa_gwdt_read(void *opaque, hwaddr addr, unsigned int size)
qemu_log_mask(LOG_GUEST_ERROR, "bad address in control frame read :"
" 0x%x\n", (int)addr);
}
+ trace_sbsa_gwdt_control_read(addr, ret);
return ret;
}
@@ -127,6 +130,7 @@ static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
unsigned size) {
SBSA_GWDTState *s = SBSA_GWDT(opaque);
+ trace_sbsa_gwdt_refresh_write(offset, data);
if (offset == SBSA_GWDT_WRR) {
s->wcs &= ~(SBSA_GWDT_WCS_WS0 | SBSA_GWDT_WCS_WS1);
@@ -141,6 +145,7 @@ static void sbsa_gwdt_write(void *opaque, hwaddr offset, uint64_t data,
unsigned size) {
SBSA_GWDTState *s = SBSA_GWDT(opaque);
+ trace_sbsa_gwdt_control_write(offset, data);
switch (offset) {
case SBSA_GWDT_WCS:
s->wcs = data & SBSA_GWDT_WCS_EN;
@@ -180,6 +185,7 @@ static void wdt_sbsa_gwdt_reset(DeviceState *dev)
{
SBSA_GWDTState *s = SBSA_GWDT(dev);
+ trace_sbsa_gwdt_reset();
timer_del(s->timer);
s->wcs = 0;
@@ -196,10 +202,12 @@ static void sbsa_gwdt_timer_sysinterrupt(void *opaque)
if (!(s->wcs & SBSA_GWDT_WCS_WS0)) {
s->wcs |= SBSA_GWDT_WCS_WS0;
+ trace_sbsa_gwdt_ws0_asserted();
sbsa_gwdt_update_timer(s, TIMEOUT_REFRESH);
qemu_set_irq(s->irq, 1);
} else {
s->wcs |= SBSA_GWDT_WCS_WS1;
+ trace_sbsa_gwdt_ws1_asserted();
qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n");
/*
* Reset the watchdog only if the guest gets notified about
diff --git a/hw/watchdog/trace-events b/hw/watchdog/trace-events
index ad3be1e9bd..5be430fb9e 100644
--- a/hw/watchdog/trace-events
+++ b/hw/watchdog/trace-events
@@ -33,3 +33,12 @@ spapr_watchdog_expired(uint64_t num, unsigned action) "num=%" PRIu64 " action=%u
# watchdog.c
watchdog_perform_action(unsigned int action) "action=%u"
watchdog_set_action(unsigned int action) "action=%u"
+
+#sbsa_gwdt.c
+sbsa_gwdt_refresh_read(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] -> 0x%" PRIu32
+sbsa_gwdt_refresh_write(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] <- 0x%" PRIu32
+sbsa_gwdt_control_read(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] -> 0x%" PRIu32
+sbsa_gwdt_control_write(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] <- 0x%" PRIu32
+sbsa_gwdt_ws0_asserted(void) "WS0 signal is asserted"
+sbsa_gwdt_ws1_asserted(void) "WS1 signal is asserted"
+sbsa_gwdt_reset(void) "reset watchdog to defaut state"
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 07/11] arm: add tracing events to sbsa_gwdt
2026-02-06 13:14 ` [PATCH 07/11] arm: add tracing events to sbsa_gwdt Igor Mammedov
@ 2026-02-16 10:22 ` Ani Sinha
0 siblings, 0 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-16 10:22 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:45 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Other than what mentioned below,
Reviewed-by: Ani Sinha <anisinha@redhat.com>
> ---
> hw/watchdog/sbsa_gwdt.c | 8 ++++++++
> hw/watchdog/trace-events | 9 +++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
> index 7ade5c6f18..d159e61c34 100644
> --- a/hw/watchdog/sbsa_gwdt.c
> +++ b/hw/watchdog/sbsa_gwdt.c
> @@ -24,6 +24,7 @@
> #include "migration/vmstate.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> +#include "trace.h"
>
> static const VMStateDescription vmstate_sbsa_gwdt = {
> .name = "sbsa-gwdt",
> @@ -62,6 +63,7 @@ static uint64_t sbsa_gwdt_rread(void *opaque, hwaddr addr, unsigned int size)
> qemu_log_mask(LOG_GUEST_ERROR, "bad address in refresh frame read :"
> " 0x%x\n", (int)addr);
> }
> + trace_sbsa_gwdt_refresh_read(addr, ret);
> return ret;
> }
>
> @@ -93,6 +95,7 @@ static uint64_t sbsa_gwdt_read(void *opaque, hwaddr addr, unsigned int size)
> qemu_log_mask(LOG_GUEST_ERROR, "bad address in control frame read :"
> " 0x%x\n", (int)addr);
> }
> + trace_sbsa_gwdt_control_read(addr, ret);
> return ret;
> }
>
> @@ -127,6 +130,7 @@ static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
> unsigned size) {
> SBSA_GWDTState *s = SBSA_GWDT(opaque);
>
> + trace_sbsa_gwdt_refresh_write(offset, data);
^^^^^
this is 64 bit.
> if (offset == SBSA_GWDT_WRR) {
> s->wcs &= ~(SBSA_GWDT_WCS_WS0 | SBSA_GWDT_WCS_WS1);
>
> @@ -141,6 +145,7 @@ static void sbsa_gwdt_write(void *opaque, hwaddr offset, uint64_t data,
> unsigned size) {
> SBSA_GWDTState *s = SBSA_GWDT(opaque);
>
> + trace_sbsa_gwdt_control_write(offset, data);
Same here ...
> switch (offset) {
> case SBSA_GWDT_WCS:
> s->wcs = data & SBSA_GWDT_WCS_EN;
> @@ -180,6 +185,7 @@ static void wdt_sbsa_gwdt_reset(DeviceState *dev)
> {
> SBSA_GWDTState *s = SBSA_GWDT(dev);
>
> + trace_sbsa_gwdt_reset();
> timer_del(s->timer);
>
> s->wcs = 0;
> @@ -196,10 +202,12 @@ static void sbsa_gwdt_timer_sysinterrupt(void *opaque)
>
> if (!(s->wcs & SBSA_GWDT_WCS_WS0)) {
> s->wcs |= SBSA_GWDT_WCS_WS0;
> + trace_sbsa_gwdt_ws0_asserted();
> sbsa_gwdt_update_timer(s, TIMEOUT_REFRESH);
> qemu_set_irq(s->irq, 1);
> } else {
> s->wcs |= SBSA_GWDT_WCS_WS1;
> + trace_sbsa_gwdt_ws1_asserted();
> qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n");
> /*
> * Reset the watchdog only if the guest gets notified about
> diff --git a/hw/watchdog/trace-events b/hw/watchdog/trace-events
> index ad3be1e9bd..5be430fb9e 100644
> --- a/hw/watchdog/trace-events
> +++ b/hw/watchdog/trace-events
> @@ -33,3 +33,12 @@ spapr_watchdog_expired(uint64_t num, unsigned action) "num=%" PRIu64 " action=%u
> # watchdog.c
> watchdog_perform_action(unsigned int action) "action=%u"
> watchdog_set_action(unsigned int action) "action=%u"
> +
> +#sbsa_gwdt.c
> +sbsa_gwdt_refresh_read(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] -> 0x%" PRIu32
> +sbsa_gwdt_refresh_write(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] <- 0x%" PRIu32
^^^^^^^
This needs to be 64 bit.
> +sbsa_gwdt_control_read(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] -> 0x%" PRIu32
> +sbsa_gwdt_control_write(uint64_t addr, uint32_t value) "[0x%" PRIu64 "] <- 0x%" PRIu32
Same here .
> +sbsa_gwdt_ws0_asserted(void) "WS0 signal is asserted"
> +sbsa_gwdt_ws1_asserted(void) "WS1 signal is asserted"
> +sbsa_gwdt_reset(void) "reset watchdog to defaut state"
^^^^^^
typo.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (6 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 07/11] arm: add tracing events to sbsa_gwdt Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-18 19:08 ` Peter Maydell
2026-02-06 13:14 ` [PATCH 09/11] tests: acpi: arm/virt: whitelist new WDAT table Igor Mammedov
` (4 subsequent siblings)
12 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
Add SBSA generic watchdog to virt machine type with
all necessary wiring for ACPI watchdog. Which includes
setting its frequency to 1KHz (max that WDAT is able to handle).
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/wdat-gwdt.h | 19 ++++++++
include/hw/arm/virt.h | 3 ++
hw/acpi/meson.build | 2 +
hw/acpi/wdat-gwdt-stub.c | 16 +++++++
hw/acpi/wdat-gwdt.c | 92 +++++++++++++++++++++++++++++++++++++
hw/arm/Kconfig | 1 +
hw/arm/virt-acpi-build.c | 16 +++++++
hw/arm/virt.c | 26 +++++++++++
8 files changed, 175 insertions(+)
create mode 100644 include/hw/acpi/wdat-gwdt.h
create mode 100644 hw/acpi/wdat-gwdt-stub.c
create mode 100644 hw/acpi/wdat-gwdt.c
diff --git a/include/hw/acpi/wdat-gwdt.h b/include/hw/acpi/wdat-gwdt.h
new file mode 100644
index 0000000000..42339e031e
--- /dev/null
+++ b/include/hw/acpi/wdat-gwdt.h
@@ -0,0 +1,19 @@
+/*
+ * GWDT Watchdog Action Table (WDAT) definition
+ *
+ * Copyright Red Hat, Inc. 2026
+ * Author(s): Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef QEMU_HW_ACPI_WDAT_GWDT_H
+#define QEMU_HW_ACPI_WDAT_GWDT_H
+
+#include "hw/acpi/aml-build.h"
+#include "hw/watchdog/sbsa_gwdt.h"
+
+void build_gwdt_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
+ const char *oem_table_id, uint64_t rbase, uint64_t cbase,
+ uint64_t freq);
+
+#endif /* QEMU_HW_ACPI_WDAT_GWDT_H */
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 3b382bdf49..d47bb8a72d 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -82,6 +82,9 @@ enum {
VIRT_NVDIMM_ACPI,
VIRT_PVTIME,
VIRT_ACPI_PCIHP,
+ VIRT_GWDT_WS0,
+ VIRT_GWDT_REFRESH,
+ VIRT_GWDT_CONTROL,
VIRT_LOWMEMMAP_LAST,
};
diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index ba974b2f0f..200b525ae1 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -28,12 +28,14 @@ acpi_ss.add(when: 'CONFIG_ACPI_ICH9', if_true: files('ich9.c', 'ich9_tco.c', 'ic
acpi_ss.add(when: 'CONFIG_ACPI_ERST', if_true: files('erst.c'))
acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
+acpi_ss.add(when: 'CONFIG_WDT_SBSA', if_true: files('wdat-gwdt.c'))
if have_tpm
acpi_ss.add(files('tpm.c'))
endif
system_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))
system_ss.add(when: 'CONFIG_ACPI_PCI_BRIDGE', if_false: files('pci-bridge-stub.c'))
system_ss.add(when: 'CONFIG_ACPI_ICH9', if_false: files('wdat-ich9-stub.c'))
+system_ss.add(when: 'CONFIG_WDT_SBSA', if_false: files('wdat-gwdt-stub.c'))
system_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
system_ss.add(when: 'CONFIG_GHES_CPER', if_true: files('ghes_cper.c'))
system_ss.add(when: 'CONFIG_GHES_CPER', if_false: files('ghes_cper_stub.c'))
diff --git a/hw/acpi/wdat-gwdt-stub.c b/hw/acpi/wdat-gwdt-stub.c
new file mode 100644
index 0000000000..4d43783f70
--- /dev/null
+++ b/hw/acpi/wdat-gwdt-stub.c
@@ -0,0 +1,16 @@
+/*
+ * Copyright Red Hat, Inc. 2026
+ * Author(s): Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/wdat-gwdt.h"
+
+void build_gwdt_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
+ const char *oem_table_id, uint64_t rbase, uint64_t cbase,
+ uint64_t freq)
+{
+ g_assert_not_reached();
+}
diff --git a/hw/acpi/wdat-gwdt.c b/hw/acpi/wdat-gwdt.c
new file mode 100644
index 0000000000..226ba3f01e
--- /dev/null
+++ b/hw/acpi/wdat-gwdt.c
@@ -0,0 +1,92 @@
+/*
+ * SBSA GWDT Watchdog Action Table (WDAT)
+ *
+ * Copyright Red Hat, Inc. 2026
+ * Author(s): Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/wdat-gwdt.h"
+#include "hw/acpi/wdat.h"
+#include "hw/watchdog/sbsa_gwdt.h"
+
+#define GWDT_REG(base, reg_offset, reg_width) { \
+ .space_id = AML_AS_SYSTEM_MEMORY, \
+ .address = base + reg_offset, .bit_width = reg_width, \
+ .access_width = AML_DWORD_ACC };
+
+/*
+ * "Hardware Watchdog Timers Design Specification"
+ * https://uefi.org/acpi 'Watchdog Action Table (WDAT)'
+ */
+void build_gwdt_wdat(GArray *table_data, BIOSLinker *linker, const char *oem_id,
+ const char *oem_table_id, uint64_t rbase, uint64_t cbase,
+ uint64_t freq)
+{
+ AcpiTable table = { .sig = "WDAT", .rev = 1, .oem_id = oem_id,
+ .oem_table_id = oem_table_id };
+
+ struct AcpiGenericAddress wrr = GWDT_REG(rbase, 0x0, 32);
+ struct AcpiGenericAddress wor_l = GWDT_REG(cbase, SBSA_GWDT_WOR, 32);
+ struct AcpiGenericAddress wcs = GWDT_REG(cbase, SBSA_GWDT_WCS, 32);
+
+ acpi_table_begin(&table, table_data);
+ build_append_int_noprefix(table_data, 0x20, 4); /* Watchdog Header Length */
+ build_append_int_noprefix(table_data, 0xff, 2); /* PCI Segment */
+ build_append_int_noprefix(table_data, 0xff, 1); /* PCI Bus Number */
+ build_append_int_noprefix(table_data, 0xff, 1); /* PCI Device Number */
+ build_append_int_noprefix(table_data, 0xff, 1); /* PCI Function Number */
+ build_append_int_noprefix(table_data, 0, 3); /* Reserved */
+ /*
+ * WDAT spec suports only 1KHz or more coarse watchdog timer,
+ * Set resolution to minimum supported 1ms.
+ * Before starting watchdog Windows set countdown value to 5min.
+ */
+ g_assert(freq <= 1000);
+ build_append_int_noprefix(table_data, 1, 4);/* Timer Period, ms */
+ /*
+ * Needs to be more than 4min, otherwise Windows 11 won't start watchdog.
+ * Set max to limits to arbitrary max 10min and min to 5sec.
+ */
+ build_append_int_noprefix(table_data, 600 * freq, 4);/* Maximum Count */
+ build_append_int_noprefix(table_data, 5 * freq, 4); /* Minimum Count */
+ /*
+ * WATCHDOG_ENABLED
+ */
+ build_append_int_noprefix(table_data, 0x81, 1); /* Watchdog Flags */
+ build_append_int_noprefix(table_data, 0, 3); /* Reserved */
+ /*
+ * watchdog instruction entries
+ */
+ build_append_int_noprefix(table_data, 8, 4);
+ /* Action table */
+ build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_RUNNING_STATE,
+ WDAT_INS_READ_VALUE,
+ wcs, 0x1, 0x1);
+ build_append_wdat_ins(table_data, WDAT_ACTION_RESET,
+ WDAT_INS_WRITE_VALUE,
+ wrr, 0x1, 0x7);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_COUNTDOWN_PERIOD,
+ WDAT_INS_WRITE_COUNTDOWN,
+ wor_l, 0, 0xffffffff);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_RUNNING_STATE,
+ WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
+ wcs, 1, 0x00000001);
+ build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_STOPPED_STATE,
+ WDAT_INS_READ_VALUE,
+ wcs, 0x0, 0x00000001);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_STOPPED_STATE,
+ WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
+ wcs, 0x0, 0x00000001);
+ build_append_wdat_ins(table_data, WDAT_ACTION_QUERY_WATCHDOG_STATUS,
+ WDAT_INS_READ_VALUE,
+ wcs, 0x4, 0x00000004);
+ build_append_wdat_ins(table_data, WDAT_ACTION_SET_WATCHDOG_STATUS,
+ WDAT_INS_WRITE_VALUE | WDAT_INS_PRESERVE_REGISTER,
+ wrr, 0x4, 0x4);
+
+ acpi_table_end(linker, &table);
+}
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index c66c452737..1222efadd1 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -36,6 +36,7 @@ config ARM_VIRT
select VIRTIO_MEM_SUPPORTED
select ACPI_CXL
select ACPI_HMAT
+ select WDT_SBSA
config CUBIEBOARD
bool
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index c145678185..51a1c040a4 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -64,6 +64,7 @@
#include "hw/virtio/virtio-acpi.h"
#include "target/arm/cpu.h"
#include "target/arm/multiprocessing.h"
+#include "hw/acpi/wdat-gwdt.h"
#define ARM_SPI_BASE 32
@@ -1270,6 +1271,21 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
+ acpi_add_table(table_offsets, tables_blob);
+ if (ms->acpi_watchdog) {
+ uint64_t freq;
+
+ freq = object_property_get_uint(
+ object_resolve_type_unambiguous(TYPE_WDT_SBSA, &error_abort),
+ "clock-frequency", &error_abort);
+
+ build_gwdt_wdat(tables_blob, tables->linker,
+ vms->oem_id, vms->oem_table_id,
+ vms->memmap[VIRT_GWDT_REFRESH].base,
+ vms->memmap[VIRT_GWDT_CONTROL].base,
+ freq);
+ }
+
if (!vmc->no_cpu_topology) {
acpi_add_table(table_offsets, tables_blob);
build_pptt(tables_blob, tables->linker, ms,
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 390845c503..caf5700ed2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -93,6 +93,7 @@
#include "hw/cxl/cxl.h"
#include "hw/cxl/cxl_host.h"
#include "qemu/guest-random.h"
+#include "hw/watchdog/sbsa_gwdt.h"
static GlobalProperty arm_virt_compat[] = {
{ TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
@@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
[VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
[VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
[VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
+ [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
+ [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
[VIRT_MMIO] = { 0x0a000000, 0x00000200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
[VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
@@ -245,12 +248,32 @@ static const int a15irqmap[] = {
[VIRT_GPIO] = 7,
[VIRT_UART1] = 8,
[VIRT_ACPI_GED] = 9,
+ [VIRT_GWDT_WS0] = 10,
[VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
[VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
[VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
[VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
};
+static void create_wdt(const VirtMachineState *vms)
+{
+ hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
+ hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
+ int irq = vms->irqmap[VIRT_GWDT_WS0];
+ DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
+ SysBusDevice *s = SYS_BUS_DEVICE(dev);
+
+ /*
+ * Set watchdog tick freq to 1Kz as it's the max WDAT driver
+ * is able to handle.
+ */
+ qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
+ sysbus_realize_and_unref(s, &error_fatal);
+ sysbus_mmio_map(s, 0, rbase);
+ sysbus_mmio_map(s, 1, cbase);
+ sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
+}
+
static void create_randomness(MachineState *ms, const char *node)
{
struct {
@@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
vms->highmem_ecam &= (!firmware_loaded || aarch64);
create_rtc(vms);
+ if (machine->acpi_watchdog) {
+ create_wdt(vms);
+ }
create_pcie(vms);
create_cxl_host_reg_region(vms);
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-06 13:14 ` [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table Igor Mammedov
@ 2026-02-18 19:08 ` Peter Maydell
2026-02-19 12:17 ` Igor Mammedov
0 siblings, 1 reply; 47+ messages in thread
From: Peter Maydell @ 2026-02-18 19:08 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <imammedo@redhat.com> wrote:
>
> Add SBSA generic watchdog to virt machine type with
> all necessary wiring for ACPI watchdog. Which includes
> setting its frequency to 1KHz (max that WDAT is able to handle).
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 390845c503..caf5700ed2 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -93,6 +93,7 @@
> #include "hw/cxl/cxl.h"
> #include "hw/cxl/cxl_host.h"
> #include "qemu/guest-random.h"
> +#include "hw/watchdog/sbsa_gwdt.h"
>
> static GlobalProperty arm_virt_compat[] = {
> { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> @@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
> [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
> [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
> [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
> + [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
> + [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> @@ -245,12 +248,32 @@ static const int a15irqmap[] = {
> [VIRT_GPIO] = 7,
> [VIRT_UART1] = 8,
> [VIRT_ACPI_GED] = 9,
> + [VIRT_GWDT_WS0] = 10,
> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
> [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> };
>
> +static void create_wdt(const VirtMachineState *vms)
> +{
> + hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
> + hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
> + int irq = vms->irqmap[VIRT_GWDT_WS0];
> + DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
> + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> +
> + /*
> + * Set watchdog tick freq to 1Kz as it's the max WDAT driver
> + * is able to handle.
> + */
> + qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
> + sysbus_realize_and_unref(s, &error_fatal);
> + sysbus_mmio_map(s, 0, rbase);
> + sysbus_mmio_map(s, 1, cbase);
> + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
> +}
Please can you also add support for exposing this device
in the device tree ?
> +
> static void create_randomness(MachineState *ms, const char *node)
> {
> struct {
> @@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
> vms->highmem_ecam &= (!firmware_loaded || aarch64);
>
> create_rtc(vms);
> + if (machine->acpi_watchdog) {
> + create_wdt(vms);
> + }
Can we have a command line option name that isn't ACPI
specific, please? There's nothing inherent to ACPI about
"I would like a watchdog device".
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-18 19:08 ` Peter Maydell
@ 2026-02-19 12:17 ` Igor Mammedov
2026-02-19 13:00 ` Peter Maydell
2026-02-25 15:19 ` Daniel P. Berrangé
0 siblings, 2 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-19 12:17 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm, Daniel P. Berrangé
On Wed, 18 Feb 2026 19:08:36 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:
> On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > Add SBSA generic watchdog to virt machine type with
> > all necessary wiring for ACPI watchdog. Which includes
> > setting its frequency to 1KHz (max that WDAT is able to handle).
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
>
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 390845c503..caf5700ed2 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -93,6 +93,7 @@
> > #include "hw/cxl/cxl.h"
> > #include "hw/cxl/cxl_host.h"
> > #include "qemu/guest-random.h"
> > +#include "hw/watchdog/sbsa_gwdt.h"
> >
> > static GlobalProperty arm_virt_compat[] = {
> > { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> > @@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
> > [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
> > [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
> > [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
> > + [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
> > + [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
> > [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> > [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> > @@ -245,12 +248,32 @@ static const int a15irqmap[] = {
> > [VIRT_GPIO] = 7,
> > [VIRT_UART1] = 8,
> > [VIRT_ACPI_GED] = 9,
> > + [VIRT_GWDT_WS0] = 10,
> > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> > [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> > [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
> > [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> > };
> >
> > +static void create_wdt(const VirtMachineState *vms)
> > +{
> > + hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
> > + hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
> > + int irq = vms->irqmap[VIRT_GWDT_WS0];
> > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
> > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > +
> > + /*
> > + * Set watchdog tick freq to 1Kz as it's the max WDAT driver
> > + * is able to handle.
> > + */
> > + qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
> > + sysbus_realize_and_unref(s, &error_fatal);
> > + sysbus_mmio_map(s, 0, rbase);
> > + sysbus_mmio_map(s, 1, cbase);
> > + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
> > +}
>
> Please can you also add support for exposing this device
> in the device tree ?
It's possible,
but we probably should not enable it if acpi variant was requested,
to avoid confusion on guest side.
For Windows it doesn't really mater, for linux it does.
on x86 linux guest uses a quirk to disable native iTCO watchdog
in favor of WDAT one if later is present.
I assume quirk is not desirable so we should expose only a preferred
variant.
>
> > +
> > static void create_randomness(MachineState *ms, const char *node)
> > {
> > struct {
> > @@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
> > vms->highmem_ecam &= (!firmware_loaded || aarch64);
> >
> > create_rtc(vms);
> > + if (machine->acpi_watchdog) {
> > + create_wdt(vms);
> > + }
>
> Can we have a command line option name that isn't ACPI
> specific, please? There's nothing inherent to ACPI about
> "I would like a watchdog device".
that is specifically asking for ACPI flavor being used/configured.
acpi specific option conflates 2 things:
1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
2. how to expose it on firmware level (in this case ACPI WDAT table)
other option, I've considered was
-device some_wd[,fw=dt|acpi|none]
but then 'fw' is not exactly device property but rather a machine one
(we can 'abuse' it/use as a proxy of cause).
However it doesn't work for boards that have builtin watchdog.
I've used device property for x86 Q35 TCO watchdog and
then using -global to enable it as workaround in previous version.
Ugly but doable workaround. It puts a burden on users to learn how
to configure it for each support watchdog/board combo.
It's nightmare to discover/maintain though.
Hence in this version, I've used a generic machine property approach
that exposes feature in uniform way across boards.
enum might work, but ...
machine.watchdog = [acpi | what else could we put here???]
I'm not married to a way how we expose/configure it and will rewrite
it to something that works for majority.
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-19 12:17 ` Igor Mammedov
@ 2026-02-19 13:00 ` Peter Maydell
2026-02-19 14:55 ` Igor Mammedov
2026-02-25 15:19 ` Daniel P. Berrangé
1 sibling, 1 reply; 47+ messages in thread
From: Peter Maydell @ 2026-02-19 13:00 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm, Daniel P. Berrangé
On Thu, 19 Feb 2026 at 12:17, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 18 Feb 2026 19:08:36 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
> > Please can you also add support for exposing this device
> > in the device tree ?
>
> It's possible,
> but we probably should not enable it if acpi variant was requested,
> to avoid confusion on guest side.
Why? Almost every other device on this board we advertise
via DTB for device tree guests and via ACPI for ACPI guests.
(The exceptions are things like the CXL handling that really
only has an ACPI representation and can't be described in the DTB.)
> For Windows it doesn't really mater, for linux it does.
> on x86 linux guest uses a quirk to disable native iTCO watchdog
> in favor of WDAT one if later is present.
> I assume quirk is not desirable so we should expose only a preferred
> variant.
For this Arm board there is no watchdog except the one you're
adding in this patch, though.
> > Can we have a command line option name that isn't ACPI
> > specific, please? There's nothing inherent to ACPI about
> > "I would like a watchdog device".
>
> that is specifically asking for ACPI flavor being used/configured.
> acpi specific option conflates 2 things:
> 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
> 2. how to expose it on firmware level (in this case ACPI WDAT table)
Right, and I think that's the wrong way to approach this,
at least for the virt board. We should either always create
or allow the user to ask us to create a watchdog device.
And then we should do what we do for all the other devices,
which is expose it via both the mechanisms that we have
for telling the guest about hardware (ACPI and DTB).
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-19 13:00 ` Peter Maydell
@ 2026-02-19 14:55 ` Igor Mammedov
2026-02-19 16:04 ` Peter Maydell
0 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-19 14:55 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm, Daniel P. Berrangé
On Thu, 19 Feb 2026 13:00:13 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:
> On Thu, 19 Feb 2026 at 12:17, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Wed, 18 Feb 2026 19:08:36 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > > Please can you also add support for exposing this device
> > > in the device tree ?
> >
> > It's possible,
> > but we probably should not enable it if acpi variant was requested,
> > to avoid confusion on guest side.
>
> Why? Almost every other device on this board we advertise
> via DTB for device tree guests and via ACPI for ACPI guests.
If we expose both guest may try to load both drivers causing conflict or misbehavior.
(
that's what x86 solved by quirk on guest side preferring WDAT watchdog
instead of native one.
)
I can try and see what arm kernel would do in presence if both
but that's not the point.
(my guess is that only 1st loaded driver would succeed, the rest
would fail on claimed resources)
One should consider plain GWDT whatchdog as a different device
when compared to WDAT one.
The later one is basically an synthetic watchdog with it's own driver.
From guest pov they are different devices (using the same registers/irqs).
exposing only one watchdog in fw (DT or ACPI), lets user pick a preferred one
without need to hack guest kernel with a quirk.
> (The exceptions are things like the CXL handling that really
> only has an ACPI representation and can't be described in the DTB.)
> > For Windows it doesn't really mater, for linux it does.
> > on x86 linux guest uses a quirk to disable native iTCO watchdog
> > in favor of WDAT one if later is present.
> > I assume quirk is not desirable so we should expose only a preferred
> > variant.
>
> For this Arm board there is no watchdog except the one you're
> adding in this patch, though.
>
> > > Can we have a command line option name that isn't ACPI
> > > specific, please? There's nothing inherent to ACPI about
> > > "I would like a watchdog device".
> >
> > that is specifically asking for ACPI flavor being used/configured.
> > acpi specific option conflates 2 things:
> > 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
> > 2. how to expose it on firmware level (in this case ACPI WDAT table)
>
> Right, and I think that's the wrong way to approach this,
> at least for the virt board. We should either always create
> or allow the user to ask us to create a watchdog device.
> And then we should do what we do for all the other devices,
> which is expose it via both the mechanisms that we have
> for telling the guest about hardware (ACPI and DTB).
Looks like I wasn't clear, WDAT is not a description
of the hardware watchdog. It's a 'abstract watchdog device',
that uses a set IO/MMIO operations from WDAT table to manage
whatever hardware watchdog board provides. Guest uses WDAT
specific driver to manage it.
Windows and Linux[1] use it without need for hardware specific driver.
All hardware specific details are encoded in WDAT table provided by board.
q35 would provide its own WDAT for underling TCO watchdog
while virt-arm would provide GWDT specific WDAT.
From guest pov it's the same WDAT watchdog, and guest doesn't
need any other watchdog drivers beside WDAT one.
machine.watchdog = (on|off) will handle/expose native watchdog whatever it is.
but with this user won't be able to pick wdat/acpi watchdog.
would following enum be acceptable?
machine.watchdog {
"on" or "native",
"off",
"acpi" or "wdat"
}
1) https://lkml.iu.edu/hypermail/linux/kernel/1609.1/04003.html
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-19 14:55 ` Igor Mammedov
@ 2026-02-19 16:04 ` Peter Maydell
2026-02-23 9:28 ` Igor Mammedov
2026-02-25 15:11 ` Daniel P. Berrangé
0 siblings, 2 replies; 47+ messages in thread
From: Peter Maydell @ 2026-02-19 16:04 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm, Daniel P. Berrangé
On Thu, 19 Feb 2026 at 14:55, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Thu, 19 Feb 2026 13:00:13 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > On Thu, 19 Feb 2026 at 12:17, Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > On Wed, 18 Feb 2026 19:08:36 +0000
> > > Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > > > Please can you also add support for exposing this device
> > > > in the device tree ?
> > >
> > > It's possible,
> > > but we probably should not enable it if acpi variant was requested,
> > > to avoid confusion on guest side.
> >
> > Why? Almost every other device on this board we advertise
> > via DTB for device tree guests and via ACPI for ACPI guests.
> If we expose both guest may try to load both drivers causing conflict or misbehavior.
Huh? A guest will do one of:
1 read the ACPI tables, and load the driver based on the ACPI data
2 read the DTB, and load the driver based on the DTB
Nothing tries to read both at once, or it would get totally
confused.
> I can try and see what arm kernel would do in presence if both
> but that's not the point.
We know already that this works fine, because almost every
piece of hardware in the virt board is described in both
the dtb and the ACPI tables.
> One should consider plain GWDT whatchdog as a different device
> when compared to WDAT one.
> The later one is basically an synthetic watchdog with it's own driver.
> From guest pov they are different devices (using the same registers/irqs).
But there is only one piece of hardware here, right?
I don't have an opinion on what the best way to tell the
guest about that in the ACPI tables is. I just want us
to report the presence of the watchdog in both DTB and ACPI,
so that it is usable whichever the guest is using.
> > > > Can we have a command line option name that isn't ACPI
> > > > specific, please? There's nothing inherent to ACPI about
> > > > "I would like a watchdog device".
> > >
> > > that is specifically asking for ACPI flavor being used/configured.
> > > acpi specific option conflates 2 things:
> > > 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
> > > 2. how to expose it on firmware level (in this case ACPI WDAT table)
> >
> > Right, and I think that's the wrong way to approach this,
> > at least for the virt board. We should either always create
> > or allow the user to ask us to create a watchdog device.
> > And then we should do what we do for all the other devices,
> > which is expose it via both the mechanisms that we have
> > for telling the guest about hardware (ACPI and DTB).
>
> Looks like I wasn't clear, WDAT is not a description
> of the hardware watchdog. It's a 'abstract watchdog device',
> that uses a set IO/MMIO operations from WDAT table to manage
> whatever hardware watchdog board provides. Guest uses WDAT
> specific driver to manage it.
Does the user need to care about how we report the hardware in
the ACPI tables? Is there not a way we can report it that works
for all guests?
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-19 16:04 ` Peter Maydell
@ 2026-02-23 9:28 ` Igor Mammedov
2026-02-25 15:11 ` Daniel P. Berrangé
1 sibling, 0 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-23 9:28 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm, Daniel P. Berrangé
On Thu, 19 Feb 2026 16:04:37 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:
> On Thu, 19 Feb 2026 at 14:55, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Thu, 19 Feb 2026 13:00:13 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > > On Thu, 19 Feb 2026 at 12:17, Igor Mammedov <imammedo@redhat.com> wrote:
> > > >
> > > > On Wed, 18 Feb 2026 19:08:36 +0000
> > > > Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > > > Please can you also add support for exposing this device
> > > > > in the device tree ?
> > > >
> > > > It's possible,
> > > > but we probably should not enable it if acpi variant was requested,
> > > > to avoid confusion on guest side.
> > >
> > > Why? Almost every other device on this board we advertise
> > > via DTB for device tree guests and via ACPI for ACPI guests.
>
> > If we expose both guest may try to load both drivers causing conflict or misbehavior.
>
> Huh? A guest will do one of:
> 1 read the ACPI tables, and load the driver based on the ACPI data
> 2 read the DTB, and load the driver based on the DTB
>
> Nothing tries to read both at once, or it would get totally
> confused.
>
> > I can try and see what arm kernel would do in presence if both
> > but that's not the point.
>
> We know already that this works fine, because almost every
> piece of hardware in the virt board is described in both
> the dtb and the ACPI tables.
>
> > One should consider plain GWDT whatchdog as a different device
> > when compared to WDAT one.
> > The later one is basically an synthetic watchdog with it's own driver.
> > From guest pov they are different devices (using the same registers/irqs).
>
> But there is only one piece of hardware here, right?
To some degree only (GWDT is wired to use different frequency
depending if it's configured for native or WDAT usecase)
> I don't have an opinion on what the best way to tell the
> guest about that in the ACPI tables is. I just want us
> to report the presence of the watchdog in both DTB and ACPI,
> so that it is usable whichever the guest is using.
correct way would be for QEMU to ship following:
1. DTB + watchdog descriptor in GTDT ACPI table for native watchdog
for that one, GWDT is wired at 1GHz resolution (same as SBSA one)
for current amr/virt machine type
or alternatively
2. WDAT table describing abstract watchdog
for this one GWDT is wired to 1KHz resolution (limitation if MS spec)
watchdog in GTDT is likely to cause conflict with WDAT watchdog timer.
Also given that DTB/GTDT spec doesn't provide explicit timer resolution,
native driver is not going to work correctly with 1Kz gwdt variant,
since it assumes wdt resolution is at 'System Counter clock frequency'
according to linux driver.
On top of current WADT impl, I can add #1 option on respin
if that is what you are asking for
(or just a DTB variant if that is sufficient for acceptance).
> > > > > Can we have a command line option name that isn't ACPI
> > > > > specific, please? There's nothing inherent to ACPI about
> > > > > "I would like a watchdog device".
> > > >
> > > > that is specifically asking for ACPI flavor being used/configured.
> > > > acpi specific option conflates 2 things:
> > > > 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
> > > > 2. how to expose it on firmware level (in this case ACPI WDAT table)
> > >
> > > Right, and I think that's the wrong way to approach this,
> > > at least for the virt board. We should either always create
> > > or allow the user to ask us to create a watchdog device.
> > > And then we should do what we do for all the other devices,
> > > which is expose it via both the mechanisms that we have
> > > for telling the guest about hardware (ACPI and DTB).
> >
> > Looks like I wasn't clear, WDAT is not a description
> > of the hardware watchdog. It's a 'abstract watchdog device',
> > that uses a set IO/MMIO operations from WDAT table to manage
> > whatever hardware watchdog board provides. Guest uses WDAT
> > specific driver to manage it.
>
> Does the user need to care about how we report the hardware in
> the ACPI tables? Is there not a way we can report it that works
> for all guests?
it's more about letting user to pick what 'hardware' to use,
this is usually done in firmware setup GUI on bare metal.
In this case QEMU is playing the role of firmware knob.
I don't see how it would work for any guest without changing
guest behavior/configuration.
supported cases would be:
q35:
- off: not supported
- native: default
- Linux: present and in use by TCO driver but not running unless guest enables service,
- on Windows, present but not in use)
- wdat:
- Linux: same as 'native' but in use by different driver (wdat_wdt)
- Windows: present/in use by inbox driver, and armed/running by default on OS startup
not manageable by user on guest side.
arm/virt:
- off: default
- native: DTB/GTDT pair exposed
- Linux/Windows: I need to check what both would do with it.
- wdat:
- should be the same as q35 above
So far WDAT the only known way to start watchdog in Windows without having
install native drivers (or in absence of such also write/maintain them),
this series describes watchdog as an abstract one and helps in an effort
to avoid writing/maintaining multitude of drivers Windows drivers and
a means to manage them on guest side. It also lets us to change underlying
hardware transparently without breaking guest if we decide to change
used watchdog hw later on.
That's what VMWare lets users to configure and this series tries to close
this feature gap.
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-19 16:04 ` Peter Maydell
2026-02-23 9:28 ` Igor Mammedov
@ 2026-02-25 15:11 ` Daniel P. Berrangé
1 sibling, 0 replies; 47+ messages in thread
From: Daniel P. Berrangé @ 2026-02-25 15:11 UTC (permalink / raw)
To: Peter Maydell
Cc: Igor Mammedov, qemu-devel, mst, anisinha, pbonzini,
shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Thu, Feb 19, 2026 at 04:04:37PM +0000, Peter Maydell wrote:
> On Thu, 19 Feb 2026 at 14:55, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Thu, 19 Feb 2026 13:00:13 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > > On Thu, 19 Feb 2026 at 12:17, Igor Mammedov <imammedo@redhat.com> wrote:
> > > >
> > > > On Wed, 18 Feb 2026 19:08:36 +0000
> > > > Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > > > Please can you also add support for exposing this device
> > > > > in the device tree ?
> > > >
> > > > It's possible,
> > > > but we probably should not enable it if acpi variant was requested,
> > > > to avoid confusion on guest side.
> > >
> > > Why? Almost every other device on this board we advertise
> > > via DTB for device tree guests and via ACPI for ACPI guests.
>
> > If we expose both guest may try to load both drivers causing conflict or misbehavior.
>
> Huh? A guest will do one of:
> 1 read the ACPI tables, and load the driver based on the ACPI data
> 2 read the DTB, and load the driver based on the DTB
>
> Nothing tries to read both at once, or it would get totally
> confused.
>
> > I can try and see what arm kernel would do in presence if both
> > but that's not the point.
>
> We know already that this works fine, because almost every
> piece of hardware in the virt board is described in both
> the dtb and the ACPI tables.
The watchdog is different though as the ACPI WDAT table is not
describing the underlying piece of hardware. Instead it is
describing an interface for controlling watchdog functionality,
such that the guest does not have any awareness of what the
underlying physical impl is.
This feels like a pretty awful characteristic of the ACPI WDAT
design, as it forces guests to do hacks like the quirk added to
iTCO :-(
If such hacks aren't possible then it forces the even worse
behaviour of QEMU mgmt apps needing to decide between multiple
different watchdog configs depending on what they know about
guest OS support :-(
Bit of a no-win scenario.
> > One should consider plain GWDT whatchdog as a different device
> > when compared to WDAT one.
> > The later one is basically an synthetic watchdog with it's own driver.
> > From guest pov they are different devices (using the same registers/irqs).
>
> But there is only one piece of hardware here, right?
QEMU knows that, but the guest OS does not. It cloud be one piece of
hardware in which case only 1 exposed device can be activated, or it
could be two distinct pieces of hardware both of which can be used
independently.
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-19 12:17 ` Igor Mammedov
2026-02-19 13:00 ` Peter Maydell
@ 2026-02-25 15:19 ` Daniel P. Berrangé
2026-02-26 12:56 ` Igor Mammedov
1 sibling, 1 reply; 47+ messages in thread
From: Daniel P. Berrangé @ 2026-02-25 15:19 UTC (permalink / raw)
To: Igor Mammedov
Cc: Peter Maydell, qemu-devel, mst, anisinha, pbonzini,
shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Thu, Feb 19, 2026 at 01:17:51PM +0100, Igor Mammedov wrote:
> On Wed, 18 Feb 2026 19:08:36 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > Add SBSA generic watchdog to virt machine type with
> > > all necessary wiring for ACPI watchdog. Which includes
> > > setting its frequency to 1KHz (max that WDAT is able to handle).
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> >
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 390845c503..caf5700ed2 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -93,6 +93,7 @@
> > > #include "hw/cxl/cxl.h"
> > > #include "hw/cxl/cxl_host.h"
> > > #include "qemu/guest-random.h"
> > > +#include "hw/watchdog/sbsa_gwdt.h"
> > >
> > > static GlobalProperty arm_virt_compat[] = {
> > > { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> > > @@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
> > > [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
> > > [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
> > > [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
> > > + [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
> > > + [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
> > > [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> > > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> > > [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> > > @@ -245,12 +248,32 @@ static const int a15irqmap[] = {
> > > [VIRT_GPIO] = 7,
> > > [VIRT_UART1] = 8,
> > > [VIRT_ACPI_GED] = 9,
> > > + [VIRT_GWDT_WS0] = 10,
> > > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> > > [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> > > [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
> > > [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> > > };
> > >
> > > +static void create_wdt(const VirtMachineState *vms)
> > > +{
> > > + hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
> > > + hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
> > > + int irq = vms->irqmap[VIRT_GWDT_WS0];
> > > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
> > > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > > +
> > > + /*
> > > + * Set watchdog tick freq to 1Kz as it's the max WDAT driver
> > > + * is able to handle.
> > > + */
> > > + qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
> > > + sysbus_realize_and_unref(s, &error_fatal);
> > > + sysbus_mmio_map(s, 0, rbase);
> > > + sysbus_mmio_map(s, 1, cbase);
> > > + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
> > > +}
> >
> > Please can you also add support for exposing this device
> > in the device tree ?
>
> It's possible,
> but we probably should not enable it if acpi variant was requested,
> to avoid confusion on guest side.
>
> For Windows it doesn't really mater, for linux it does.
> on x86 linux guest uses a quirk to disable native iTCO watchdog
> in favor of WDAT one if later is present.
> I assume quirk is not desirable so we should expose only a preferred
> variant.
>
> >
> > > +
> > > static void create_randomness(MachineState *ms, const char *node)
> > > {
> > > struct {
> > > @@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
> > > vms->highmem_ecam &= (!firmware_loaded || aarch64);
> > >
> > > create_rtc(vms);
> > > + if (machine->acpi_watchdog) {
> > > + create_wdt(vms);
> > > + }
> >
> > Can we have a command line option name that isn't ACPI
> > specific, please? There's nothing inherent to ACPI about
> > "I would like a watchdog device".
>
> that is specifically asking for ACPI flavor being used/configured.
> acpi specific option conflates 2 things:
> 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
> 2. how to expose it on firmware level (in this case ACPI WDAT table)
>
> other option, I've considered was
> -device some_wd[,fw=dt|acpi|none]
> but then 'fw' is not exactly device property but rather a machine one
> (we can 'abuse' it/use as a proxy of cause).
> However it doesn't work for boards that have builtin watchdog.
Right, if there was a case where we were already using -device to
create the watchdog, then I would suggest having a simple
'wadt=on|off' property as standard for any watchdog wanting that
ACPI abstraction wrapper.
That could (hypothetically) be the case if we had chosen to add
WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
eg for Q35, "-machine q35,itco-wadt=on|off"
for virt, '-machine virt,gwdt-wadt=on|off'
Q35 is easier as we rely on the guest quirks to disable direct
access via both paths.
With GWDT we need to decide if gwdt-wadt=on should imply DTB
disabled, or if we just expose both & rely on a guest quirk
or worst case, need a separate property to toggle DTB too.
> I've used device property for x86 Q35 TCO watchdog and
> then using -global to enable it as workaround in previous version.
> Ugly but doable workaround. It puts a burden on users to learn how
> to configure it for each support watchdog/board combo.
> It's nightmare to discover/maintain though.
>
> Hence in this version, I've used a generic machine property approach
> that exposes feature in uniform way across boards.
>
> enum might work, but ...
> machine.watchdog = [acpi | what else could we put here???]
>
> I'm not married to a way how we expose/configure it and will rewrite
> it to something that works for majority.
>
> > thanks
> > -- PMM
> >
>
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-25 15:19 ` Daniel P. Berrangé
@ 2026-02-26 12:56 ` Igor Mammedov
2026-02-27 7:24 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-26 12:56 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Peter Maydell, qemu-devel, mst, anisinha, pbonzini,
shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm,
Markus Armbruster
On Wed, 25 Feb 2026 15:19:56 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Thu, Feb 19, 2026 at 01:17:51PM +0100, Igor Mammedov wrote:
> > On Wed, 18 Feb 2026 19:08:36 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > > On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <imammedo@redhat.com> wrote:
> > > >
> > > > Add SBSA generic watchdog to virt machine type with
> > > > all necessary wiring for ACPI watchdog. Which includes
> > > > setting its frequency to 1KHz (max that WDAT is able to handle).
> > > >
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > >
> > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > > index 390845c503..caf5700ed2 100644
> > > > --- a/hw/arm/virt.c
> > > > +++ b/hw/arm/virt.c
> > > > @@ -93,6 +93,7 @@
> > > > #include "hw/cxl/cxl.h"
> > > > #include "hw/cxl/cxl_host.h"
> > > > #include "qemu/guest-random.h"
> > > > +#include "hw/watchdog/sbsa_gwdt.h"
> > > >
> > > > static GlobalProperty arm_virt_compat[] = {
> > > > { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> > > > @@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
> > > > [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
> > > > [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
> > > > [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
> > > > + [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
> > > > + [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
> > > > [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> > > > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> > > > [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> > > > @@ -245,12 +248,32 @@ static const int a15irqmap[] = {
> > > > [VIRT_GPIO] = 7,
> > > > [VIRT_UART1] = 8,
> > > > [VIRT_ACPI_GED] = 9,
> > > > + [VIRT_GWDT_WS0] = 10,
> > > > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> > > > [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> > > > [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
> > > > [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> > > > };
> > > >
> > > > +static void create_wdt(const VirtMachineState *vms)
> > > > +{
> > > > + hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
> > > > + hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
> > > > + int irq = vms->irqmap[VIRT_GWDT_WS0];
> > > > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
> > > > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > > > +
> > > > + /*
> > > > + * Set watchdog tick freq to 1Kz as it's the max WDAT driver
> > > > + * is able to handle.
> > > > + */
> > > > + qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
> > > > + sysbus_realize_and_unref(s, &error_fatal);
> > > > + sysbus_mmio_map(s, 0, rbase);
> > > > + sysbus_mmio_map(s, 1, cbase);
> > > > + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
> > > > +}
> > >
> > > Please can you also add support for exposing this device
> > > in the device tree ?
> >
> > It's possible,
> > but we probably should not enable it if acpi variant was requested,
> > to avoid confusion on guest side.
> >
> > For Windows it doesn't really mater, for linux it does.
> > on x86 linux guest uses a quirk to disable native iTCO watchdog
> > in favor of WDAT one if later is present.
> > I assume quirk is not desirable so we should expose only a preferred
> > variant.
> >
> > >
> > > > +
> > > > static void create_randomness(MachineState *ms, const char *node)
> > > > {
> > > > struct {
> > > > @@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
> > > > vms->highmem_ecam &= (!firmware_loaded || aarch64);
> > > >
> > > > create_rtc(vms);
> > > > + if (machine->acpi_watchdog) {
> > > > + create_wdt(vms);
> > > > + }
> > >
> > > Can we have a command line option name that isn't ACPI
> > > specific, please? There's nothing inherent to ACPI about
> > > "I would like a watchdog device".
> >
> > that is specifically asking for ACPI flavor being used/configured.
> > acpi specific option conflates 2 things:
> > 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
> > 2. how to expose it on firmware level (in this case ACPI WDAT table)
> >
> > other option, I've considered was
> > -device some_wd[,fw=dt|acpi|none]
> > but then 'fw' is not exactly device property but rather a machine one
> > (we can 'abuse' it/use as a proxy of cause).
> > However it doesn't work for boards that have builtin watchdog.
>
> Right, if there was a case where we were already using -device to
> create the watchdog, then I would suggest having a simple
> 'wadt=on|off' property as standard for any watchdog wanting that
> ACPI abstraction wrapper.
>
> That could (hypothetically) be the case if we had chosen to add
> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
>
> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
>
> eg for Q35, "-machine q35,itco-wadt=on|off"
>
> for virt, '-machine virt,gwdt-wadt=on|off'
CCin Markus since it touches QAPI stuff.
For respin, I've went ahead with a bit more generic approach
(as a user, I wouldn't really care what kind of built-in watchdog board ships,
and having 1 property instead total instead of machine specific ones makes it
easier for mgmt layer to deal with).
here is current idea:
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6411e68856..a0c6719b58 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
NULL, NULL);
object_class_property_set_description(oc, "memory",
"Memory size configuration");
+
+ object_class_property_add_enum(oc, "watchdog", "WatchdogType",
+ &WatchdogType_lookup,
+ machine_get_watchdog, machine_set_watchdog);
+ object_class_property_set_description(oc, "watchdog",
+ "Use: auto (watchdog is configured according board defaults),"
+ " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
+ " Default: auto");
}
diff --git a/qapi/machine-common.json b/qapi/machine-common.json
index 92e84dfb14..7f5e10340f 100644
--- a/qapi/machine-common.json
+++ b/qapi/machine-common.json
@@ -9,6 +9,23 @@
# Common machine types
# ********************
##
+##
+# @WatchdogType:
+#
+# On board watchdog configuration.
+#
+# @auto: Watchdog is configured according to board defaults.
+#
+# @off: Watchdog disabled (ARM).
+#
+# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
+#
+# @wdat: ACPI WDAT watchdog.
+#
+# Since: 10.2
+##
+{ 'enum': 'WatchdogType',
+ 'data': [ 'auto', 'off', 'native', 'wdat' ] }
> Q35 is easier as we rely on the guest quirks to disable direct
> access via both paths.
>
> With GWDT we need to decide if gwdt-wadt=on should imply DTB
> disabled, or if we just expose both & rely on a guest quirk
> or worst case, need a separate property to toggle DTB too.
>
> > I've used device property for x86 Q35 TCO watchdog and
> > then using -global to enable it as workaround in previous version.
> > Ugly but doable workaround. It puts a burden on users to learn how
> > to configure it for each support watchdog/board combo.
> > It's nightmare to discover/maintain though.
> >
> > Hence in this version, I've used a generic machine property approach
> > that exposes feature in uniform way across boards.
> >
> > enum might work, but ...
> > machine.watchdog = [acpi | what else could we put here???]
> >
> > I'm not married to a way how we expose/configure it and will rewrite
> > it to something that works for majority.
> >
> > > thanks
> > > -- PMM
> > >
> >
>
> With regards,
> Daniel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-26 12:56 ` Igor Mammedov
@ 2026-02-27 7:24 ` Markus Armbruster
2026-02-27 9:01 ` Daniel P. Berrangé
2026-02-27 9:42 ` Igor Mammedov
0 siblings, 2 replies; 47+ messages in thread
From: Markus Armbruster @ 2026-02-27 7:24 UTC (permalink / raw)
To: Igor Mammedov
Cc: Daniel P. Berrangé, Peter Maydell, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
Igor Mammedov <imammedo@redhat.com> writes:
> On Wed, 25 Feb 2026 15:19:56 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
>
>> On Thu, Feb 19, 2026 at 01:17:51PM +0100, Igor Mammedov wrote:
>> > On Wed, 18 Feb 2026 19:08:36 +0000
>> > Peter Maydell <peter.maydell@linaro.org> wrote:
>> >
>> > > On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <imammedo@redhat.com> wrote:
>> > > >
>> > > > Add SBSA generic watchdog to virt machine type with
>> > > > all necessary wiring for ACPI watchdog. Which includes
>> > > > setting its frequency to 1KHz (max that WDAT is able to handle).
>> > > >
>> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> > > > ---
>> > >
>> > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> > > > index 390845c503..caf5700ed2 100644
>> > > > --- a/hw/arm/virt.c
>> > > > +++ b/hw/arm/virt.c
>> > > > @@ -93,6 +93,7 @@
>> > > > #include "hw/cxl/cxl.h"
>> > > > #include "hw/cxl/cxl_host.h"
>> > > > #include "qemu/guest-random.h"
>> > > > +#include "hw/watchdog/sbsa_gwdt.h"
>> > > >
>> > > > static GlobalProperty arm_virt_compat[] = {
>> > > > { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
>> > > > @@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
>> > > > [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
>> > > > [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
>> > > > [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
>> > > > + [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
>> > > > + [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
>> > > > [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
>> > > > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>> > > > [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
>> > > > @@ -245,12 +248,32 @@ static const int a15irqmap[] = {
>> > > > [VIRT_GPIO] = 7,
>> > > > [VIRT_UART1] = 8,
>> > > > [VIRT_ACPI_GED] = 9,
>> > > > + [VIRT_GWDT_WS0] = 10,
>> > > > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>> > > > [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>> > > > [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
>> > > > [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>> > > > };
>> > > >
>> > > > +static void create_wdt(const VirtMachineState *vms)
>> > > > +{
>> > > > + hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
>> > > > + hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
>> > > > + int irq = vms->irqmap[VIRT_GWDT_WS0];
>> > > > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
>> > > > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
>> > > > +
>> > > > + /*
>> > > > + * Set watchdog tick freq to 1Kz as it's the max WDAT driver
>> > > > + * is able to handle.
>> > > > + */
>> > > > + qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
>> > > > + sysbus_realize_and_unref(s, &error_fatal);
>> > > > + sysbus_mmio_map(s, 0, rbase);
>> > > > + sysbus_mmio_map(s, 1, cbase);
>> > > > + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
>> > > > +}
>> > >
>> > > Please can you also add support for exposing this device
>> > > in the device tree ?
>> >
>> > It's possible,
>> > but we probably should not enable it if acpi variant was requested,
>> > to avoid confusion on guest side.
>> >
>> > For Windows it doesn't really mater, for linux it does.
>> > on x86 linux guest uses a quirk to disable native iTCO watchdog
>> > in favor of WDAT one if later is present.
>> > I assume quirk is not desirable so we should expose only a preferred
>> > variant.
>> >
>> > >
>> > > > +
>> > > > static void create_randomness(MachineState *ms, const char *node)
>> > > > {
>> > > > struct {
>> > > > @@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
>> > > > vms->highmem_ecam &= (!firmware_loaded || aarch64);
>> > > >
>> > > > create_rtc(vms);
>> > > > + if (machine->acpi_watchdog) {
>> > > > + create_wdt(vms);
>> > > > + }
>> > >
>> > > Can we have a command line option name that isn't ACPI
>> > > specific, please? There's nothing inherent to ACPI about
>> > > "I would like a watchdog device".
>> >
>> > that is specifically asking for ACPI flavor being used/configured.
>> > acpi specific option conflates 2 things:
>> > 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
>> > 2. how to expose it on firmware level (in this case ACPI WDAT table)
>> >
>> > other option, I've considered was
>> > -device some_wd[,fw=dt|acpi|none]
>> > but then 'fw' is not exactly device property but rather a machine one
>> > (we can 'abuse' it/use as a proxy of cause).
>> > However it doesn't work for boards that have builtin watchdog.
>>
>> Right, if there was a case where we were already using -device to
>> create the watchdog, then I would suggest having a simple
>> 'wadt=on|off' property as standard for any watchdog wanting that
>> ACPI abstraction wrapper.
>>
>> That could (hypothetically) be the case if we had chosen to add
>> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
>>
>> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
>>
>> eg for Q35, "-machine q35,itco-wadt=on|off"
>>
>> for virt, '-machine virt,gwdt-wadt=on|off'
>
> CCin Markus since it touches QAPI stuff.
>
> For respin, I've went ahead with a bit more generic approach
> (as a user, I wouldn't really care what kind of built-in watchdog board ships,
> and having 1 property instead total instead of machine specific ones makes it
> easier for mgmt layer to deal with).
>
> here is current idea:
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 6411e68856..a0c6719b58 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> NULL, NULL);
> object_class_property_set_description(oc, "memory",
> "Memory size configuration");
> +
> + object_class_property_add_enum(oc, "watchdog", "WatchdogType",
> + &WatchdogType_lookup,
> + machine_get_watchdog, machine_set_watchdog);
> + object_class_property_set_description(oc, "watchdog",
> + "Use: auto (watchdog is configured according board defaults),"
> + " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
> + " Default: auto");
> }
Adds the property to abstract base type "machine", i.e. every machine
has it.
Fundamentally assumes there is at most one onboard watchdog of interest.
I'm lacking context, please bear with me: who or what is going to set
this property, and for what purpose?
> diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> index 92e84dfb14..7f5e10340f 100644
> --- a/qapi/machine-common.json
> +++ b/qapi/machine-common.json
> @@ -9,6 +9,23 @@
> # Common machine types
> # ********************
> ##
> +##
> +# @WatchdogType:
> +#
> +# On board watchdog configuration.
> +#
> +# @auto: Watchdog is configured according to board defaults.
As far as I can tell, we don't document "boards" (machine types)
anywhere, let alone "board defaults". Not this patch's fault, of
course. The meaning of @auto remains unclear.
> +#
> +# @off: Watchdog disabled (ARM).
> +#
> +# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
> +#
> +# @wdat: ACPI WDAT watchdog.
> +#
> +# Since: 10.2
11.0
> +##
> +{ 'enum': 'WatchdogType',
> + 'data': [ 'auto', 'off', 'native', 'wdat' ] }
What do managament applications need to know about onboard watchdog
configuration?
Do they need to know what @auto means for the machine type at hand?
Do they ever need to pick a value? Do they need to know which values
work with the machine type then?
>> Q35 is easier as we rely on the guest quirks to disable direct
>> access via both paths.
>>
>> With GWDT we need to decide if gwdt-wadt=on should imply DTB
>> disabled, or if we just expose both & rely on a guest quirk
>> or worst case, need a separate property to toggle DTB too.
>>
>> > I've used device property for x86 Q35 TCO watchdog and
>> > then using -global to enable it as workaround in previous version.
>> > Ugly but doable workaround. It puts a burden on users to learn how
>> > to configure it for each support watchdog/board combo.
>> > It's nightmare to discover/maintain though.
Our unsolved "how to configure onboard devices" problem bites again.
-global can be pressed into service when there's just once instance of
the device type. The drawbacks you describe are real. I want to see
less of it, not more.
>> > Hence in this version, I've used a generic machine property approach
>> > that exposes feature in uniform way across boards.
We've done that before. It works, but I don't think it scale up to a
complete solution (I understand you are not after a complete solution
here, and that's okay).
For instance, machine pc-q35-FOO has two onboard cfi.pflash01 devices.
To enable configuration of their block backends, we alias their "drive"
properties as machine properties "pflash0" and "pflash1". Their other
properties remain inaccessible.
Adding a few alias properties to the machine type is workable. A bit ad
hoc, perhaps. Bothersome to document, so we largely don't.
Adding alias properties for everything anyone could possibly want to
configure feels unworkable.
This is *not* an objection to solving the problem at hand with a machine
property.
>> > enum might work, but ...
>> > machine.watchdog = [acpi | what else could we put here???]
>> >
>> > I'm not married to a way how we expose/configure it and will rewrite
>> > it to something that works for majority.
>> >
>> > > thanks
>> > > -- PMM
>> > >
>> >
>>
>> With regards,
>> Daniel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 7:24 ` Markus Armbruster
@ 2026-02-27 9:01 ` Daniel P. Berrangé
2026-02-27 10:01 ` Igor Mammedov
2026-02-27 9:42 ` Igor Mammedov
1 sibling, 1 reply; 47+ messages in thread
From: Daniel P. Berrangé @ 2026-02-27 9:01 UTC (permalink / raw)
To: Markus Armbruster
Cc: Igor Mammedov, Peter Maydell, qemu-devel, mst, anisinha, pbonzini,
shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Fri, Feb 27, 2026 at 08:24:42AM +0100, Markus Armbruster wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
>
> > On Wed, 25 Feb 2026 15:19:56 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >>
> >> Right, if there was a case where we were already using -device to
> >> create the watchdog, then I would suggest having a simple
> >> 'wadt=on|off' property as standard for any watchdog wanting that
> >> ACPI abstraction wrapper.
> >>
> >> That could (hypothetically) be the case if we had chosen to add
> >> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
> >>
> >> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
> >>
> >> eg for Q35, "-machine q35,itco-wadt=on|off"
> >>
> >> for virt, '-machine virt,gwdt-wadt=on|off'
> >
> > CCin Markus since it touches QAPI stuff.
> >
> > For respin, I've went ahead with a bit more generic approach
> > (as a user, I wouldn't really care what kind of built-in watchdog board ships,
> > and having 1 property instead total instead of machine specific ones makes it
> > easier for mgmt layer to deal with).
> >
> > here is current idea:
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 6411e68856..a0c6719b58 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> > NULL, NULL);
> > object_class_property_set_description(oc, "memory",
> > "Memory size configuration");
> > +
> > + object_class_property_add_enum(oc, "watchdog", "WatchdogType",
> > + &WatchdogType_lookup,
> > + machine_get_watchdog, machine_set_watchdog);
> > + object_class_property_set_description(oc, "watchdog",
> > + "Use: auto (watchdog is configured according board defaults),"
> > + " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
> > + " Default: auto");
> > }
>
> Adds the property to abstract base type "machine", i.e. every machine
> has it.
>
> Fundamentally assumes there is at most one onboard watchdog of interest.
>
> I'm lacking context, please bear with me: who or what is going to set
> this property, and for what purpose?
>
> > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> > index 92e84dfb14..7f5e10340f 100644
> > --- a/qapi/machine-common.json
> > +++ b/qapi/machine-common.json
> > @@ -9,6 +9,23 @@
> > # Common machine types
> > # ********************
> > ##
> > +##
> > +# @WatchdogType:
> > +#
> > +# On board watchdog configuration.
> > +#
> > +# @auto: Watchdog is configured according to board defaults.
>
> As far as I can tell, we don't document "boards" (machine types)
> anywhere, let alone "board defaults". Not this patch's fault, of
> course. The meaning of @auto remains unclear.
>
> > +#
> > +# @off: Watchdog disabled (ARM).
> > +#
> > +# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
> > +#
> > +# @wdat: ACPI WDAT watchdog.
> > +#
> > +# Since: 10.2
>
> 11.0
>
> > +##
> > +{ 'enum': 'WatchdogType',
> > + 'data': [ 'auto', 'off', 'native', 'wdat' ] }
>
> What do managament applications need to know about onboard watchdog
> configuration?
>
> Do they need to know what @auto means for the machine type at hand?
>
> Do they ever need to pick a value? Do they need to know which values
> work with the machine type then?
Libvirt generally wants to work with & express the exact hardware
and not rely on "auto" settings in QEMU.
So in terms of the built-in watchdog for Q35 we currently express
this explicitly via
<watchdog model='itco' />
Adding "WADT" is not expressing a new piece of hardware, just a
configuration choice of the iTCO hardware. So from that POV in
libvirt I anticipate expressing it as
<watchdog model='itco' wadt='yes|no'/>
IMHO, similar reasoning works at the QEMU level - WADT is not a
new type of watchdog hardware, just a config choice supported
with certain specific QEMU watchdogs. So I'd prefer something
closer to what I suggested in the mail that Igor replies to,
where we have a simple boolean flag to control use of WADT,
and other distinct properties if we need other controls on the
watchdog, instead of trying overload multiple distinct settings
into one enum.
For Arm we would similarly want to express the choice of
hardware directly as
<watchdog model='gwdt'/>
regardless of whether the GWDT ends up being something we have to
add manually with -device, or something built-in to the machine.
> >> Q35 is easier as we rely on the guest quirks to disable direct
> >> access via both paths.
> >>
> >> With GWDT we need to decide if gwdt-wadt=on should imply DTB
> >> disabled, or if we just expose both & rely on a guest quirk
> >> or worst case, need a separate property to toggle DTB too.
> >>
> >> > I've used device property for x86 Q35 TCO watchdog and
> >> > then using -global to enable it as workaround in previous version.
> >> > Ugly but doable workaround. It puts a burden on users to learn how
> >> > to configure it for each support watchdog/board combo.
> >> > It's nightmare to discover/maintain though.
>
> Our unsolved "how to configure onboard devices" problem bites again.
>
> -global can be pressed into service when there's just once instance of
> the device type. The drawbacks you describe are real. I want to see
> less of it, not more.
IIRC, the only place we use -global with watchdogs today is to
set "-global ICH9-LPC.noreboot=off" which was merely to workaround
a historical QEMU bug which exposed a guest watchdog and then
inexplicably made it not work by having it ignore the guest
trigger event. Newer machine types fixed that broken default, but
libvirt keeps setting things explicitly since that's easier than
making machine version specific choices.
I'm not sure we need -global to support the WADT features, as that
seems like it can just a -machine property instead.
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 9:01 ` Daniel P. Berrangé
@ 2026-02-27 10:01 ` Igor Mammedov
2026-02-27 10:18 ` Daniel P. Berrangé
0 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-27 10:01 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Markus Armbruster, Peter Maydell, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Fri, 27 Feb 2026 09:01:25 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Fri, Feb 27, 2026 at 08:24:42AM +0100, Markus Armbruster wrote:
> > Igor Mammedov <imammedo@redhat.com> writes:
> >
> > > On Wed, 25 Feb 2026 15:19:56 +0000
> > > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >>
> > >> Right, if there was a case where we were already using -device to
> > >> create the watchdog, then I would suggest having a simple
> > >> 'wadt=on|off' property as standard for any watchdog wanting that
> > >> ACPI abstraction wrapper.
> > >>
> > >> That could (hypothetically) be the case if we had chosen to add
> > >> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
> > >>
> > >> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
> > >>
> > >> eg for Q35, "-machine q35,itco-wadt=on|off"
> > >>
> > >> for virt, '-machine virt,gwdt-wadt=on|off'
> > >
> > > CCin Markus since it touches QAPI stuff.
> > >
> > > For respin, I've went ahead with a bit more generic approach
> > > (as a user, I wouldn't really care what kind of built-in watchdog board ships,
> > > and having 1 property instead total instead of machine specific ones makes it
> > > easier for mgmt layer to deal with).
> > >
> > > here is current idea:
> > >
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index 6411e68856..a0c6719b58 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> > > NULL, NULL);
> > > object_class_property_set_description(oc, "memory",
> > > "Memory size configuration");
> > > +
> > > + object_class_property_add_enum(oc, "watchdog", "WatchdogType",
> > > + &WatchdogType_lookup,
> > > + machine_get_watchdog, machine_set_watchdog);
> > > + object_class_property_set_description(oc, "watchdog",
> > > + "Use: auto (watchdog is configured according board defaults),"
> > > + " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
> > > + " Default: auto");
> > > }
> >
> > Adds the property to abstract base type "machine", i.e. every machine
> > has it.
> >
> > Fundamentally assumes there is at most one onboard watchdog of interest.
> >
> > I'm lacking context, please bear with me: who or what is going to set
> > this property, and for what purpose?
> >
> > > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> > > index 92e84dfb14..7f5e10340f 100644
> > > --- a/qapi/machine-common.json
> > > +++ b/qapi/machine-common.json
> > > @@ -9,6 +9,23 @@
> > > # Common machine types
> > > # ********************
> > > ##
> > > +##
> > > +# @WatchdogType:
> > > +#
> > > +# On board watchdog configuration.
> > > +#
> > > +# @auto: Watchdog is configured according to board defaults.
> >
> > As far as I can tell, we don't document "boards" (machine types)
> > anywhere, let alone "board defaults". Not this patch's fault, of
> > course. The meaning of @auto remains unclear.
> >
> > > +#
> > > +# @off: Watchdog disabled (ARM).
> > > +#
> > > +# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
> > > +#
> > > +# @wdat: ACPI WDAT watchdog.
> > > +#
> > > +# Since: 10.2
> >
> > 11.0
> >
> > > +##
> > > +{ 'enum': 'WatchdogType',
> > > + 'data': [ 'auto', 'off', 'native', 'wdat' ] }
> >
> > What do managament applications need to know about onboard watchdog
> > configuration?
> >
> > Do they need to know what @auto means for the machine type at hand?
> >
> > Do they ever need to pick a value? Do they need to know which values
> > work with the machine type then?
>
> Libvirt generally wants to work with & express the exact hardware
> and not rely on "auto" settings in QEMU.
>
> So in terms of the built-in watchdog for Q35 we currently express
> this explicitly via
>
> <watchdog model='itco' />
not specifying above libvirt doesn't make itco go away on qemu side,
it's still there regardless of above.
but that said, I can work with it and add
q35.wdat -> itco.wdat alias to toggle it
> Adding "WADT" is not expressing a new piece of hardware, just a
> configuration choice of the iTCO hardware. So from that POV in
> libvirt I anticipate expressing it as
>
> <watchdog model='itco' wadt='yes|no'/>
>
> IMHO, similar reasoning works at the QEMU level - WADT is not a
> new type of watchdog hardware, just a config choice supported
> with certain specific QEMU watchdogs. So I'd prefer something
> closer to what I suggested in the mail that Igor replies to,
> where we have a simple boolean flag to control use of WADT,
> and other distinct properties if we need other controls on the
> watchdog, instead of trying overload multiple distinct settings
> into one enum.
>
> For Arm we would similarly want to express the choice of
> hardware directly as
>
> <watchdog model='gwdt'/>
for gwdt it's more than just adding extra ACPI table,
with wdat knob, watchdog entry in GTDT table should be disabled
and gwdt itself should be wired to use different freq
(in the end it's different hw that happens to use the same registers/
protocol).
if we make it built-in, we would need in addition to virt.wadt alias
also have virt.watchdog=[on|off] to manage creation.
> regardless of whether the GWDT ends up being something we have to
> add manually with -device, or something built-in to the machine.
Peter,
is it possible/acceptable to add sbsa-gwdt to virt board with
-device. (technically I can use generic hotplug hooks[1] to wire
it up to the board)
1)
virt_machine_device_*plug_cb()
> > >> Q35 is easier as we rely on the guest quirks to disable direct
> > >> access via both paths.
> > >>
> > >> With GWDT we need to decide if gwdt-wadt=on should imply DTB
> > >> disabled, or if we just expose both & rely on a guest quirk
> > >> or worst case, need a separate property to toggle DTB too.
> > >>
> > >> > I've used device property for x86 Q35 TCO watchdog and
> > >> > then using -global to enable it as workaround in previous version.
> > >> > Ugly but doable workaround. It puts a burden on users to learn how
> > >> > to configure it for each support watchdog/board combo.
> > >> > It's nightmare to discover/maintain though.
> >
> > Our unsolved "how to configure onboard devices" problem bites again.
> >
> > -global can be pressed into service when there's just once instance of
> > the device type. The drawbacks you describe are real. I want to see
> > less of it, not more.
>
> IIRC, the only place we use -global with watchdogs today is to
> set "-global ICH9-LPC.noreboot=off" which was merely to workaround
> a historical QEMU bug which exposed a guest watchdog and then
> inexplicably made it not work by having it ignore the guest
> trigger event. Newer machine types fixed that broken default, but
> libvirt keeps setting things explicitly since that's easier than
> making machine version specific choices.
>
> I'm not sure we need -global to support the WADT features, as that
> seems like it can just a -machine property instead.
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 10:01 ` Igor Mammedov
@ 2026-02-27 10:18 ` Daniel P. Berrangé
2026-02-27 11:41 ` Igor Mammedov
0 siblings, 1 reply; 47+ messages in thread
From: Daniel P. Berrangé @ 2026-02-27 10:18 UTC (permalink / raw)
To: Igor Mammedov
Cc: Markus Armbruster, Peter Maydell, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Fri, Feb 27, 2026 at 11:01:25AM +0100, Igor Mammedov wrote:
> On Fri, 27 Feb 2026 09:01:25 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> > On Fri, Feb 27, 2026 at 08:24:42AM +0100, Markus Armbruster wrote:
> > > Igor Mammedov <imammedo@redhat.com> writes:
> > >
> > > > On Wed, 25 Feb 2026 15:19:56 +0000
> > > > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >>
> > > >> Right, if there was a case where we were already using -device to
> > > >> create the watchdog, then I would suggest having a simple
> > > >> 'wadt=on|off' property as standard for any watchdog wanting that
> > > >> ACPI abstraction wrapper.
> > > >>
> > > >> That could (hypothetically) be the case if we had chosen to add
> > > >> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
> > > >>
> > > >> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
> > > >>
> > > >> eg for Q35, "-machine q35,itco-wadt=on|off"
> > > >>
> > > >> for virt, '-machine virt,gwdt-wadt=on|off'
> > > >
> > > > CCin Markus since it touches QAPI stuff.
> > > >
> > > > For respin, I've went ahead with a bit more generic approach
> > > > (as a user, I wouldn't really care what kind of built-in watchdog board ships,
> > > > and having 1 property instead total instead of machine specific ones makes it
> > > > easier for mgmt layer to deal with).
> > > >
> > > > here is current idea:
> > > >
> > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > index 6411e68856..a0c6719b58 100644
> > > > --- a/hw/core/machine.c
> > > > +++ b/hw/core/machine.c
> > > > @@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> > > > NULL, NULL);
> > > > object_class_property_set_description(oc, "memory",
> > > > "Memory size configuration");
> > > > +
> > > > + object_class_property_add_enum(oc, "watchdog", "WatchdogType",
> > > > + &WatchdogType_lookup,
> > > > + machine_get_watchdog, machine_set_watchdog);
> > > > + object_class_property_set_description(oc, "watchdog",
> > > > + "Use: auto (watchdog is configured according board defaults),"
> > > > + " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
> > > > + " Default: auto");
> > > > }
> > >
> > > Adds the property to abstract base type "machine", i.e. every machine
> > > has it.
> > >
> > > Fundamentally assumes there is at most one onboard watchdog of interest.
> > >
> > > I'm lacking context, please bear with me: who or what is going to set
> > > this property, and for what purpose?
> > >
> > > > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> > > > index 92e84dfb14..7f5e10340f 100644
> > > > --- a/qapi/machine-common.json
> > > > +++ b/qapi/machine-common.json
> > > > @@ -9,6 +9,23 @@
> > > > # Common machine types
> > > > # ********************
> > > > ##
> > > > +##
> > > > +# @WatchdogType:
> > > > +#
> > > > +# On board watchdog configuration.
> > > > +#
> > > > +# @auto: Watchdog is configured according to board defaults.
> > >
> > > As far as I can tell, we don't document "boards" (machine types)
> > > anywhere, let alone "board defaults". Not this patch's fault, of
> > > course. The meaning of @auto remains unclear.
> > >
> > > > +#
> > > > +# @off: Watchdog disabled (ARM).
> > > > +#
> > > > +# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
> > > > +#
> > > > +# @wdat: ACPI WDAT watchdog.
> > > > +#
> > > > +# Since: 10.2
> > >
> > > 11.0
> > >
> > > > +##
> > > > +{ 'enum': 'WatchdogType',
> > > > + 'data': [ 'auto', 'off', 'native', 'wdat' ] }
> > >
> > > What do managament applications need to know about onboard watchdog
> > > configuration?
> > >
> > > Do they need to know what @auto means for the machine type at hand?
> > >
> > > Do they ever need to pick a value? Do they need to know which values
> > > work with the machine type then?
> >
> > Libvirt generally wants to work with & express the exact hardware
> > and not rely on "auto" settings in QEMU.
> >
> > So in terms of the built-in watchdog for Q35 we currently express
> > this explicitly via
> >
> > <watchdog model='itco' />
>
> not specifying above libvirt doesn't make itco go away on qemu side,
> it's still there regardless of above.
That is fine. Libvirt will synthesize a <watchdog> device to
represent & track the built-in default hardware, if the mgmt
app did not specify it explicitly.
> > Adding "WADT" is not expressing a new piece of hardware, just a
> > configuration choice of the iTCO hardware. So from that POV in
> > libvirt I anticipate expressing it as
> >
> > <watchdog model='itco' wadt='yes|no'/>
> >
> > IMHO, similar reasoning works at the QEMU level - WADT is not a
> > new type of watchdog hardware, just a config choice supported
> > with certain specific QEMU watchdogs. So I'd prefer something
> > closer to what I suggested in the mail that Igor replies to,
> > where we have a simple boolean flag to control use of WADT,
> > and other distinct properties if we need other controls on the
> > watchdog, instead of trying overload multiple distinct settings
> > into one enum.
> >
> > For Arm we would similarly want to express the choice of
> > hardware directly as
> >
> > <watchdog model='gwdt'/>
> for gwdt it's more than just adding extra ACPI table,
> with wdat knob, watchdog entry in GTDT table should be disabled
> and gwdt itself should be wired to use different freq
> (in the end it's different hw that happens to use the same registers/
> protocol).
That doesn't really sound that different to me. If enabling WADT
implies disabling GTDT that's ok, likewise if Peter wants DT to be
available in parallel that's ok too. We can cope with modelling
either approach in libvirt
> if we make it built-in, we would need in addition to virt.wadt alias
> also have virt.watchdog=[on|off] to manage creation.
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 10:18 ` Daniel P. Berrangé
@ 2026-02-27 11:41 ` Igor Mammedov
0 siblings, 0 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-27 11:41 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Markus Armbruster, Peter Maydell, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Fri, 27 Feb 2026 10:18:25 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Fri, Feb 27, 2026 at 11:01:25AM +0100, Igor Mammedov wrote:
> > On Fri, 27 Feb 2026 09:01:25 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > > On Fri, Feb 27, 2026 at 08:24:42AM +0100, Markus Armbruster wrote:
> > > > Igor Mammedov <imammedo@redhat.com> writes:
> > > >
> > > > > On Wed, 25 Feb 2026 15:19:56 +0000
> > > > > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > >>
> > > > >> Right, if there was a case where we were already using -device to
> > > > >> create the watchdog, then I would suggest having a simple
> > > > >> 'wadt=on|off' property as standard for any watchdog wanting that
> > > > >> ACPI abstraction wrapper.
> > > > >>
> > > > >> That could (hypothetically) be the case if we had chosen to add
> > > > >> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
> > > > >>
> > > > >> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
> > > > >>
> > > > >> eg for Q35, "-machine q35,itco-wadt=on|off"
> > > > >>
> > > > >> for virt, '-machine virt,gwdt-wadt=on|off'
> > > > >
> > > > > CCin Markus since it touches QAPI stuff.
> > > > >
> > > > > For respin, I've went ahead with a bit more generic approach
> > > > > (as a user, I wouldn't really care what kind of built-in watchdog board ships,
> > > > > and having 1 property instead total instead of machine specific ones makes it
> > > > > easier for mgmt layer to deal with).
> > > > >
> > > > > here is current idea:
> > > > >
> > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > > index 6411e68856..a0c6719b58 100644
> > > > > --- a/hw/core/machine.c
> > > > > +++ b/hw/core/machine.c
> > > > > @@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> > > > > NULL, NULL);
> > > > > object_class_property_set_description(oc, "memory",
> > > > > "Memory size configuration");
> > > > > +
> > > > > + object_class_property_add_enum(oc, "watchdog", "WatchdogType",
> > > > > + &WatchdogType_lookup,
> > > > > + machine_get_watchdog, machine_set_watchdog);
> > > > > + object_class_property_set_description(oc, "watchdog",
> > > > > + "Use: auto (watchdog is configured according board defaults),"
> > > > > + " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
> > > > > + " Default: auto");
> > > > > }
> > > >
> > > > Adds the property to abstract base type "machine", i.e. every machine
> > > > has it.
> > > >
> > > > Fundamentally assumes there is at most one onboard watchdog of interest.
> > > >
> > > > I'm lacking context, please bear with me: who or what is going to set
> > > > this property, and for what purpose?
> > > >
> > > > > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> > > > > index 92e84dfb14..7f5e10340f 100644
> > > > > --- a/qapi/machine-common.json
> > > > > +++ b/qapi/machine-common.json
> > > > > @@ -9,6 +9,23 @@
> > > > > # Common machine types
> > > > > # ********************
> > > > > ##
> > > > > +##
> > > > > +# @WatchdogType:
> > > > > +#
> > > > > +# On board watchdog configuration.
> > > > > +#
> > > > > +# @auto: Watchdog is configured according to board defaults.
> > > >
> > > > As far as I can tell, we don't document "boards" (machine types)
> > > > anywhere, let alone "board defaults". Not this patch's fault, of
> > > > course. The meaning of @auto remains unclear.
> > > >
> > > > > +#
> > > > > +# @off: Watchdog disabled (ARM).
> > > > > +#
> > > > > +# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
> > > > > +#
> > > > > +# @wdat: ACPI WDAT watchdog.
> > > > > +#
> > > > > +# Since: 10.2
> > > >
> > > > 11.0
> > > >
> > > > > +##
> > > > > +{ 'enum': 'WatchdogType',
> > > > > + 'data': [ 'auto', 'off', 'native', 'wdat' ] }
> > > >
> > > > What do managament applications need to know about onboard watchdog
> > > > configuration?
> > > >
> > > > Do they need to know what @auto means for the machine type at hand?
> > > >
> > > > Do they ever need to pick a value? Do they need to know which values
> > > > work with the machine type then?
> > >
> > > Libvirt generally wants to work with & express the exact hardware
> > > and not rely on "auto" settings in QEMU.
> > >
> > > So in terms of the built-in watchdog for Q35 we currently express
> > > this explicitly via
> > >
> > > <watchdog model='itco' />
> >
> > not specifying above libvirt doesn't make itco go away on qemu side,
> > it's still there regardless of above.
>
> That is fine. Libvirt will synthesize a <watchdog> device to
> represent & track the built-in default hardware, if the mgmt
> app did not specify it explicitly.
>
>
>
> > > Adding "WADT" is not expressing a new piece of hardware, just a
> > > configuration choice of the iTCO hardware. So from that POV in
> > > libvirt I anticipate expressing it as
> > >
> > > <watchdog model='itco' wadt='yes|no'/>
> > >
> > > IMHO, similar reasoning works at the QEMU level - WADT is not a
> > > new type of watchdog hardware, just a config choice supported
> > > with certain specific QEMU watchdogs. So I'd prefer something
> > > closer to what I suggested in the mail that Igor replies to,
> > > where we have a simple boolean flag to control use of WADT,
> > > and other distinct properties if we need other controls on the
> > > watchdog, instead of trying overload multiple distinct settings
> > > into one enum.
> > >
> > > For Arm we would similarly want to express the choice of
> > > hardware directly as
> > >
> > > <watchdog model='gwdt'/>
> > for gwdt it's more than just adding extra ACPI table,
> > with wdat knob, watchdog entry in GTDT table should be disabled
> > and gwdt itself should be wired to use different freq
> > (in the end it's different hw that happens to use the same registers/
> > protocol).
>
> That doesn't really sound that different to me. If enabling WADT
> implies disabling GTDT that's ok, likewise if Peter wants DT to be
> available in parallel that's ok too. We can cope with modelling
> either approach in libvirt
with wdat enabled not only GTDT part but also a corresponding DT node should
be hidden from guest as they assume gwdt running at different frequency.
As for exposing both GTDT along with DT node when wdat is disabled,
quick test shows that it works for linux and Windows (if one considers
unusable Windows driver as works).
Both pickup GTDT definition just fine and
with ACPI disabled, linux guest fallbacks to DT and uses DT node
to create watchdog device node.
>
> > if we make it built-in, we would need in addition to virt.wadt alias
> > also have virt.watchdog=[on|off] to manage creation.
>
>
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 7:24 ` Markus Armbruster
2026-02-27 9:01 ` Daniel P. Berrangé
@ 2026-02-27 9:42 ` Igor Mammedov
2026-02-27 12:10 ` Markus Armbruster
1 sibling, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-27 9:42 UTC (permalink / raw)
To: Markus Armbruster
Cc: Daniel P. Berrangé, Peter Maydell, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Fri, 27 Feb 2026 08:24:42 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
>
> > On Wed, 25 Feb 2026 15:19:56 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> >> On Thu, Feb 19, 2026 at 01:17:51PM +0100, Igor Mammedov wrote:
> >> > On Wed, 18 Feb 2026 19:08:36 +0000
> >> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >> >
> >> > > On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <imammedo@redhat.com> wrote:
> >> > > >
> >> > > > Add SBSA generic watchdog to virt machine type with
> >> > > > all necessary wiring for ACPI watchdog. Which includes
> >> > > > setting its frequency to 1KHz (max that WDAT is able to handle).
> >> > > >
> >> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >> > > > ---
> >> > >
> >> > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> > > > index 390845c503..caf5700ed2 100644
> >> > > > --- a/hw/arm/virt.c
> >> > > > +++ b/hw/arm/virt.c
> >> > > > @@ -93,6 +93,7 @@
> >> > > > #include "hw/cxl/cxl.h"
> >> > > > #include "hw/cxl/cxl_host.h"
> >> > > > #include "qemu/guest-random.h"
> >> > > > +#include "hw/watchdog/sbsa_gwdt.h"
> >> > > >
> >> > > > static GlobalProperty arm_virt_compat[] = {
> >> > > > { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> >> > > > @@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
> >> > > > [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
> >> > > > [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
> >> > > > [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
> >> > > > + [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
> >> > > > + [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
> >> > > > [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> >> > > > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> >> > > > [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> >> > > > @@ -245,12 +248,32 @@ static const int a15irqmap[] = {
> >> > > > [VIRT_GPIO] = 7,
> >> > > > [VIRT_UART1] = 8,
> >> > > > [VIRT_ACPI_GED] = 9,
> >> > > > + [VIRT_GWDT_WS0] = 10,
> >> > > > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> >> > > > [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> >> > > > [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
> >> > > > [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> >> > > > };
> >> > > >
> >> > > > +static void create_wdt(const VirtMachineState *vms)
> >> > > > +{
> >> > > > + hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
> >> > > > + hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
> >> > > > + int irq = vms->irqmap[VIRT_GWDT_WS0];
> >> > > > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
> >> > > > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> >> > > > +
> >> > > > + /*
> >> > > > + * Set watchdog tick freq to 1Kz as it's the max WDAT driver
> >> > > > + * is able to handle.
> >> > > > + */
> >> > > > + qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
> >> > > > + sysbus_realize_and_unref(s, &error_fatal);
> >> > > > + sysbus_mmio_map(s, 0, rbase);
> >> > > > + sysbus_mmio_map(s, 1, cbase);
> >> > > > + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
> >> > > > +}
> >> > >
> >> > > Please can you also add support for exposing this device
> >> > > in the device tree ?
> >> >
> >> > It's possible,
> >> > but we probably should not enable it if acpi variant was requested,
> >> > to avoid confusion on guest side.
> >> >
> >> > For Windows it doesn't really mater, for linux it does.
> >> > on x86 linux guest uses a quirk to disable native iTCO watchdog
> >> > in favor of WDAT one if later is present.
> >> > I assume quirk is not desirable so we should expose only a preferred
> >> > variant.
> >> >
> >> > >
> >> > > > +
> >> > > > static void create_randomness(MachineState *ms, const char *node)
> >> > > > {
> >> > > > struct {
> >> > > > @@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
> >> > > > vms->highmem_ecam &= (!firmware_loaded || aarch64);
> >> > > >
> >> > > > create_rtc(vms);
> >> > > > + if (machine->acpi_watchdog) {
> >> > > > + create_wdt(vms);
> >> > > > + }
> >> > >
> >> > > Can we have a command line option name that isn't ACPI
> >> > > specific, please? There's nothing inherent to ACPI about
> >> > > "I would like a watchdog device".
> >> >
> >> > that is specifically asking for ACPI flavor being used/configured.
> >> > acpi specific option conflates 2 things:
> >> > 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
> >> > 2. how to expose it on firmware level (in this case ACPI WDAT table)
> >> >
> >> > other option, I've considered was
> >> > -device some_wd[,fw=dt|acpi|none]
> >> > but then 'fw' is not exactly device property but rather a machine one
> >> > (we can 'abuse' it/use as a proxy of cause).
> >> > However it doesn't work for boards that have builtin watchdog.
> >>
> >> Right, if there was a case where we were already using -device to
> >> create the watchdog, then I would suggest having a simple
> >> 'wadt=on|off' property as standard for any watchdog wanting that
> >> ACPI abstraction wrapper.
> >>
> >> That could (hypothetically) be the case if we had chosen to add
> >> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
> >>
> >> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
> >>
> >> eg for Q35, "-machine q35,itco-wadt=on|off"
> >>
> >> for virt, '-machine virt,gwdt-wadt=on|off'
> >
> > CCin Markus since it touches QAPI stuff.
> >
> > For respin, I've went ahead with a bit more generic approach
> > (as a user, I wouldn't really care what kind of built-in watchdog board ships,
> > and having 1 property instead total instead of machine specific ones makes it
> > easier for mgmt layer to deal with).
> >
> > here is current idea:
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 6411e68856..a0c6719b58 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
> > NULL, NULL);
> > object_class_property_set_description(oc, "memory",
> > "Memory size configuration");
> > +
> > + object_class_property_add_enum(oc, "watchdog", "WatchdogType",
> > + &WatchdogType_lookup,
> > + machine_get_watchdog, machine_set_watchdog);
> > + object_class_property_set_description(oc, "watchdog",
> > + "Use: auto (watchdog is configured according board defaults),"
> > + " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
> > + " Default: auto");
> > }
>
> Adds the property to abstract base type "machine", i.e. every machine
> has it.
in past (which might be different from now 'policies'), we used to add
a property to abstract machine when there is more than 1 board/target
that consumes it. This follows that trend.
If that is not desirable now, I can move it specific boards at the cost
of some boiler-plate code duplication.
Boards of interest in this series are arm/virt and q35
> Fundamentally assumes there is at most one onboard watchdog of interest.
there might be, hence default 'auto' to describe various boards default/
implicit watchdog selection.
User might set it but is doesn't make much sense to configure already
default state.
> I'm lacking context, please bear with me: who or what is going to set
> this property, and for what purpose?
it's for users/mgmt apps that wish to explicitly optin to use watchdog.
majority users don't need it (and can leave it at default 'auto' or 'off' state)
but depending on usecase, watchdog presence can be a mandatory requirement.
and here comes a choice for a layer that knows what guest OS supports
to pick a suitable watchdog.
- native: requires from user upfront knowledge what built-in watchdog is
and if it's supported by guest
- wdat: abstracts user from need to know what built-in watchdog is
as still requires user to know is guest supports it
As of today guest drivers support is in following state:
TCO watchdog on Q35 (linux: supported, Windows: not supported)
sbsa-gwdt watchdog in arm world - (linux: supported, Windows: broken)
wdat abstract watchdog (Q35 and arm) - (linux: supported, Windows: supported)
peddling points in favor of wdat (MS and my to some extent)
From above selection, wdat is widely supported already for 10 years,
and while it's not ideal/limited than purpose built drivers
and relies on less that ideal MS spec.
It lets use out of box drivers (both Linux and Windows) without need
to install ones on guest side.
From QEMU maintenance pov, wdat abstraction uses under the hood
native watchdog, but guest OS doesn't care as all it does in wdat driver
is executing a series of mmio instructions as specified in WDAT ACPI table.
And QEMU can transparently change hidden native watchdog to
another one without need to reconfigure guest OS.
With deprecation and old machine types being removed we can gradually
phase out undesirable watchdog impl and replace it with what
we would fancy in that moment.
> > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> > index 92e84dfb14..7f5e10340f 100644
> > --- a/qapi/machine-common.json
> > +++ b/qapi/machine-common.json
> > @@ -9,6 +9,23 @@
> > # Common machine types
> > # ********************
> > ##
> > +##
> > +# @WatchdogType:
> > +#
> > +# On board watchdog configuration.
> > +#
> > +# @auto: Watchdog is configured according to board defaults.
>
> As far as I can tell, we don't document "boards" (machine types)
> anywhere, let alone "board defaults". Not this patch's fault, of
> course. The meaning of @auto remains unclear.
would mentioning q35 and arm/virt here be of any help?
Suggestions how to deal with it?
>
> > +#
> > +# @off: Watchdog disabled (ARM).
> > +#
> > +# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
> > +#
> > +# @wdat: ACPI WDAT watchdog.
> > +#
> > +# Since: 10.2
>
> 11.0
>
> > +##
> > +{ 'enum': 'WatchdogType',
> > + 'data': [ 'auto', 'off', 'native', 'wdat' ] }
>
> What do managament applications need to know about onboard watchdog
> configuration?
given it emulates firmware knob (with qemu acting as fw),
if watchdog is needed, mgmt would need to pick either native
or wdat one based on what works for guest OS.
> Do they need to know what @auto means for the machine type at hand?
I don't think they would need to know what 'auto' means.
> Do they ever need to pick a value? Do they need to know which values
> work with the machine type then?
tentatively answer to both questions is yes. not impl. yet though)
valid states per board would be:
q35 { auto -> native, wdat, off - not supported }
arm/virt {auto -> off, native, wdat, off }
other boards might have different mapping.
Easiest way to check for this is duplicated per board 'watchdog' property
with per board setter that verifies setting.
Or in case a of genric machine property, a callback to let boards
add their own logic in generic property setter.
> >> Q35 is easier as we rely on the guest quirks to disable direct
> >> access via both paths.
> >>
> >> With GWDT we need to decide if gwdt-wadt=on should imply DTB
> >> disabled, or if we just expose both & rely on a guest quirk
> >> or worst case, need a separate property to toggle DTB too.
> >>
> >> > I've used device property for x86 Q35 TCO watchdog and
> >> > then using -global to enable it as workaround in previous version.
> >> > Ugly but doable workaround. It puts a burden on users to learn how
> >> > to configure it for each support watchdog/board combo.
> >> > It's nightmare to discover/maintain though.
>
> Our unsolved "how to configure onboard devices" problem bites again.
>
> -global can be pressed into service when there's just once instance of
> the device type. The drawbacks you describe are real. I want to see
> less of it, not more.
>
> >> > Hence in this version, I've used a generic machine property approach
> >> > that exposes feature in uniform way across boards.
>
> We've done that before. It works, but I don't think it scale up to a
> complete solution (I understand you are not after a complete solution
> here, and that's okay).
>
> For instance, machine pc-q35-FOO has two onboard cfi.pflash01 devices.
> To enable configuration of their block backends, we alias their "drive"
> properties as machine properties "pflash0" and "pflash1". Their other
> properties remain inaccessible.
>
> Adding a few alias properties to the machine type is workable. A bit ad
> hoc, perhaps. Bothersome to document, so we largely don't.
>
> Adding alias properties for everything anyone could possibly want to
> configure feels unworkable.
>
> This is *not* an objection to solving the problem at hand with a machine
> property.
using aliases might work for q35 since it doesn't have 'off' option,
i.e. we can add 'wdat' property to TCO watchdog and then let alias it
to q35 as watchdog property:
{ native -> tco.wdat = off,
wdat -> tco.wdat = on }
for arm/virt it would look like, adding wdat property to sbsa-gwdt watchdog
and then aliasing it machine's 'watchdog' property with following
mapping
{ off - don't create watchdog at all,
native -> sbsa-gwdt.wdat=off,
wdat -> sbsa-gwdt.wdat=on }
it doesn't seem like a workable approach
> >> > enum might work, but ...
> >> > machine.watchdog = [acpi | what else could we put here???]
> >> >
> >> > I'm not married to a way how we expose/configure it and will rewrite
> >> > it to something that works for majority.
> >> >
> >> > > thanks
> >> > > -- PMM
> >> > >
> >> >
> >>
> >> With regards,
> >> Daniel
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 9:42 ` Igor Mammedov
@ 2026-02-27 12:10 ` Markus Armbruster
2026-02-27 13:14 ` Peter Maydell
0 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2026-02-27 12:10 UTC (permalink / raw)
To: Igor Mammedov
Cc: Daniel P. Berrangé, Peter Maydell, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
Igor Mammedov <imammedo@redhat.com> writes:
> On Fri, 27 Feb 2026 08:24:42 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Igor Mammedov <imammedo@redhat.com> writes:
>>
>> > On Wed, 25 Feb 2026 15:19:56 +0000
>> > Daniel P. Berrangé <berrange@redhat.com> wrote:
>> >
>> >> On Thu, Feb 19, 2026 at 01:17:51PM +0100, Igor Mammedov wrote:
>> >> > On Wed, 18 Feb 2026 19:08:36 +0000
>> >> > Peter Maydell <peter.maydell@linaro.org> wrote:
>> >> >
>> >> > > On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <imammedo@redhat.com> wrote:
>> >> > > >
>> >> > > > Add SBSA generic watchdog to virt machine type with
>> >> > > > all necessary wiring for ACPI watchdog. Which includes
>> >> > > > setting its frequency to 1KHz (max that WDAT is able to handle).
>> >> > > >
>> >> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> >> > > > ---
>> >> > >
>> >> > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> >> > > > index 390845c503..caf5700ed2 100644
>> >> > > > --- a/hw/arm/virt.c
>> >> > > > +++ b/hw/arm/virt.c
>> >> > > > @@ -93,6 +93,7 @@
>> >> > > > #include "hw/cxl/cxl.h"
>> >> > > > #include "hw/cxl/cxl_host.h"
>> >> > > > #include "qemu/guest-random.h"
>> >> > > > +#include "hw/watchdog/sbsa_gwdt.h"
>> >> > > >
>> >> > > > static GlobalProperty arm_virt_compat[] = {
>> >> > > > { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
>> >> > > > @@ -194,6 +195,8 @@ static const MemMapEntry base_memmap[] = {
>> >> > > > [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
>> >> > > > [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
>> >> > > > [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
>> >> > > > + [VIRT_GWDT_REFRESH] = { 0x090d0000, 0x00001000 },
>> >> > > > + [VIRT_GWDT_CONTROL] = { 0x090d1000, 0x00001000 },
>> >> > > > [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
>> >> > > > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>> >> > > > [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
>> >> > > > @@ -245,12 +248,32 @@ static const int a15irqmap[] = {
>> >> > > > [VIRT_GPIO] = 7,
>> >> > > > [VIRT_UART1] = 8,
>> >> > > > [VIRT_ACPI_GED] = 9,
>> >> > > > + [VIRT_GWDT_WS0] = 10,
>> >> > > > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>> >> > > > [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>> >> > > > [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
>> >> > > > [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>> >> > > > };
>> >> > > >
>> >> > > > +static void create_wdt(const VirtMachineState *vms)
>> >> > > > +{
>> >> > > > + hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
>> >> > > > + hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
>> >> > > > + int irq = vms->irqmap[VIRT_GWDT_WS0];
>> >> > > > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
>> >> > > > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
>> >> > > > +
>> >> > > > + /*
>> >> > > > + * Set watchdog tick freq to 1Kz as it's the max WDAT driver
>> >> > > > + * is able to handle.
>> >> > > > + */
>> >> > > > + qdev_prop_set_uint64(dev, "clock-frequency", 1000 /* 1KHz */);
>> >> > > > + sysbus_realize_and_unref(s, &error_fatal);
>> >> > > > + sysbus_mmio_map(s, 0, rbase);
>> >> > > > + sysbus_mmio_map(s, 1, cbase);
>> >> > > > + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
>> >> > > > +}
>> >> > >
>> >> > > Please can you also add support for exposing this device
>> >> > > in the device tree ?
>> >> >
>> >> > It's possible,
>> >> > but we probably should not enable it if acpi variant was requested,
>> >> > to avoid confusion on guest side.
>> >> >
>> >> > For Windows it doesn't really mater, for linux it does.
>> >> > on x86 linux guest uses a quirk to disable native iTCO watchdog
>> >> > in favor of WDAT one if later is present.
>> >> > I assume quirk is not desirable so we should expose only a preferred
>> >> > variant.
>> >> >
>> >> > >
>> >> > > > +
>> >> > > > static void create_randomness(MachineState *ms, const char *node)
>> >> > > > {
>> >> > > > struct {
>> >> > > > @@ -2515,6 +2538,9 @@ static void machvirt_init(MachineState *machine)
>> >> > > > vms->highmem_ecam &= (!firmware_loaded || aarch64);
>> >> > > >
>> >> > > > create_rtc(vms);
>> >> > > > + if (machine->acpi_watchdog) {
>> >> > > > + create_wdt(vms);
>> >> > > > + }
>> >> > >
>> >> > > Can we have a command line option name that isn't ACPI
>> >> > > specific, please? There's nothing inherent to ACPI about
>> >> > > "I would like a watchdog device".
>> >> >
>> >> > that is specifically asking for ACPI flavor being used/configured.
>> >> > acpi specific option conflates 2 things:
>> >> > 1. watchdog device creation (of the board choice) with properties tuned for WDAT usage
>> >> > 2. how to expose it on firmware level (in this case ACPI WDAT table)
>> >> >
>> >> > other option, I've considered was
>> >> > -device some_wd[,fw=dt|acpi|none]
>> >> > but then 'fw' is not exactly device property but rather a machine one
>> >> > (we can 'abuse' it/use as a proxy of cause).
>> >> > However it doesn't work for boards that have builtin watchdog.
>> >>
>> >> Right, if there was a case where we were already using -device to
>> >> create the watchdog, then I would suggest having a simple
>> >> 'wadt=on|off' property as standard for any watchdog wanting that
>> >> ACPI abstraction wrapper.
>> >>
>> >> That could (hypothetically) be the case if we had chosen to add
>> >> WADT suport backed by i6300esb, eg -device i6300esb,wadt=on|off
>> >>
>> >> For cases built-in to the machine type, I'd suggest <type>-wadt=on|off
>> >>
>> >> eg for Q35, "-machine q35,itco-wadt=on|off"
>> >>
>> >> for virt, '-machine virt,gwdt-wadt=on|off'
>> >
>> > CCin Markus since it touches QAPI stuff.
>> >
>> > For respin, I've went ahead with a bit more generic approach
>> > (as a user, I wouldn't really care what kind of built-in watchdog board ships,
>> > and having 1 property instead total instead of machine specific ones makes it
>> > easier for mgmt layer to deal with).
>> >
>> > here is current idea:
>> >
>> > diff --git a/hw/core/machine.c b/hw/core/machine.c
>> > index 6411e68856..a0c6719b58 100644
>> > --- a/hw/core/machine.c
>> > +++ b/hw/core/machine.c
>> > @@ -1257,6 +1271,14 @@ static void machine_class_init(ObjectClass *oc, const void *data)
>> > NULL, NULL);
>> > object_class_property_set_description(oc, "memory",
>> > "Memory size configuration");
>> > +
>> > + object_class_property_add_enum(oc, "watchdog", "WatchdogType",
>> > + &WatchdogType_lookup,
>> > + machine_get_watchdog, machine_set_watchdog);
>> > + object_class_property_set_description(oc, "watchdog",
>> > + "Use: auto (watchdog is configured according board defaults),"
>> > + " off (disabled), native (built-in watchdog), wdat (ACPI based watchdog)]. "
>> > + " Default: auto");
>> > }
>>
>> Adds the property to abstract base type "machine", i.e. every machine
>> has it.
>
> in past (which might be different from now 'policies'), we used to add
> a property to abstract machine when there is more than 1 board/target
> that consumes it. This follows that trend.
> If that is not desirable now, I can move it specific boards at the cost
> of some boiler-plate code duplication.
>
> Boards of interest in this series are arm/virt and q35
I didn't mean to object to adding it to all machines. I don't have an
opinion there :)
I figure when the board doesn't have a watchdog, then the only available
choices are 'off' and 'auto', which is 'auto' also 'off'.
>> Fundamentally assumes there is at most one onboard watchdog of interest.
>
> there might be, hence default 'auto' to describe various boards default/
> implicit watchdog selection.
> User might set it but is doesn't make much sense to configure already
> default state.
Defaulting to a valid value is good.
>> I'm lacking context, please bear with me: who or what is going to set
>> this property, and for what purpose?
>
> it's for users/mgmt apps that wish to explicitly optin to use watchdog.
> majority users don't need it (and can leave it at default 'auto' or 'off' state)
> but depending on usecase, watchdog presence can be a mandatory requirement.
>
> and here comes a choice for a layer that knows what guest OS supports
> to pick a suitable watchdog.
>
> - native: requires from user upfront knowledge what built-in watchdog is
> and if it's supported by guest
> - wdat: abstracts user from need to know what built-in watchdog is
> as still requires user to know is guest supports it
>
> As of today guest drivers support is in following state:
> TCO watchdog on Q35 (linux: supported, Windows: not supported)
> sbsa-gwdt watchdog in arm world - (linux: supported, Windows: broken)
> wdat abstract watchdog (Q35 and arm) - (linux: supported, Windows: supported)
>
> peddling points in favor of wdat (MS and my to some extent)
> From above selection, wdat is widely supported already for 10 years,
> and while it's not ideal/limited than purpose built drivers
> and relies on less that ideal MS spec.
> It lets use out of box drivers (both Linux and Windows) without need
> to install ones on guest side.
>
> From QEMU maintenance pov, wdat abstraction uses under the hood
> native watchdog, but guest OS doesn't care as all it does in wdat driver
> is executing a series of mmio instructions as specified in WDAT ACPI table.
> And QEMU can transparently change hidden native watchdog to
> another one without need to reconfigure guest OS.
> With deprecation and old machine types being removed we can gradually
> phase out undesirable watchdog impl and replace it with what
> we would fancy in that moment.
>
>> > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
>> > index 92e84dfb14..7f5e10340f 100644
>> > --- a/qapi/machine-common.json
>> > +++ b/qapi/machine-common.json
>> > @@ -9,6 +9,23 @@
>> > # Common machine types
>> > # ********************
>> > ##
>> > +##
>> > +# @WatchdogType:
>> > +#
>> > +# On board watchdog configuration.
>> > +#
>> > +# @auto: Watchdog is configured according to board defaults.
>>
>> As far as I can tell, we don't document "boards" (machine types)
>> anywhere, let alone "board defaults". Not this patch's fault, of
>> course. The meaning of @auto remains unclear.
>
> would mentioning q35 and arm/virt here be of any help?
> Suggestions how to deal with it?
The larger problem is that we generally fail at documenting device and
machine properties. I'm not asking you to boil that ocean :)
The narrow problem is providing guidance on watchdog configuration.
What and where?
Regarding where: you proposed to add a machine property of QAPI enum
type.
The property can have a description, and you give it one. Sadly, it's
basically invisible. As far as I know, the only way you can get at it
is qom-list-properties and such. I doubt anybody is going to read the
description there.
The QAPI enum type must have documentation. Goes into the "QEMU QMP
Reference Manual". Since it's not actually used in the QAPI schema (QOM
bypasses QAPI), it's not linked from anywhere, which makes it hard to
find. A future version of the doc generator may elide unused
definitions from this manual.
Other ideas?
Regarding what: read on.
>> > +#
>> > +# @off: Watchdog disabled (ARM).
>> > +#
>> > +# @native: Native watchdog (arm/virt: sbsa-gwdt, x86/q53: TCO)
>> > +#
>> > +# @wdat: ACPI WDAT watchdog.
>> > +#
>> > +# Since: 10.2
>>
>> 11.0
>>
>> > +##
>> > +{ 'enum': 'WatchdogType',
>> > + 'data': [ 'auto', 'off', 'native', 'wdat' ] }
>>
>> What do managament applications need to know about onboard watchdog
>> configuration?
>
> given it emulates firmware knob (with qemu acting as fw),
> if watchdog is needed, mgmt would need to pick either native
> or wdat one based on what works for guest OS.
>
>> Do they need to know what @auto means for the machine type at hand?
>
> I don't think they would need to know what 'auto' means.
>
>> Do they ever need to pick a value? Do they need to know which values
>> work with the machine type then?
>
> tentatively answer to both questions is yes. not impl. yet though)
> valid states per board would be:
> q35 { auto -> native, wdat, off - not supported }
> arm/virt {auto -> off, native, wdat, off }
>
> other boards might have different mapping.
So...
1. Which values other than @auto are actually supported depends on the
board.
2. Which of the supported values works best (or at all) depends on the
guest OS.
Configuring QEMU according to the guest OS's needs is the management
application's job.
Is there anything QEMU could do to help with this aspect of the job? Or
do management applications just need to hardcode the required
information?
> Easiest way to check for this is duplicated per board 'watchdog' property
> with per board setter that verifies setting.
> Or in case a of genric machine property, a callback to let boards
> add their own logic in generic property setter.
>
>> >> Q35 is easier as we rely on the guest quirks to disable direct
>> >> access via both paths.
>> >>
>> >> With GWDT we need to decide if gwdt-wadt=on should imply DTB
>> >> disabled, or if we just expose both & rely on a guest quirk
>> >> or worst case, need a separate property to toggle DTB too.
>> >>
>> >> > I've used device property for x86 Q35 TCO watchdog and
>> >> > then using -global to enable it as workaround in previous version.
>> >> > Ugly but doable workaround. It puts a burden on users to learn how
>> >> > to configure it for each support watchdog/board combo.
>> >> > It's nightmare to discover/maintain though.
>>
>> Our unsolved "how to configure onboard devices" problem bites again.
>>
>> -global can be pressed into service when there's just once instance of
>> the device type. The drawbacks you describe are real. I want to see
>> less of it, not more.
>>
>> >> > Hence in this version, I've used a generic machine property approach
>> >> > that exposes feature in uniform way across boards.
>>
>> We've done that before. It works, but I don't think it scale up to a
>> complete solution (I understand you are not after a complete solution
>> here, and that's okay).
>>
>> For instance, machine pc-q35-FOO has two onboard cfi.pflash01 devices.
>> To enable configuration of their block backends, we alias their "drive"
>> properties as machine properties "pflash0" and "pflash1". Their other
>> properties remain inaccessible.
>>
>> Adding a few alias properties to the machine type is workable. A bit ad
>> hoc, perhaps. Bothersome to document, so we largely don't.
>>
>> Adding alias properties for everything anyone could possibly want to
>> configure feels unworkable.
>>
>> This is *not* an objection to solving the problem at hand with a machine
>> property.
>
> using aliases might work for q35 since it doesn't have 'off' option,
> i.e. we can add 'wdat' property to TCO watchdog and then let alias it
> to q35 as watchdog property:
> { native -> tco.wdat = off,
> wdat -> tco.wdat = on }
>
> for arm/virt it would look like, adding wdat property to sbsa-gwdt watchdog
> and then aliasing it machine's 'watchdog' property with following
> mapping
> { off - don't create watchdog at all,
> native -> sbsa-gwdt.wdat=off,
> wdat -> sbsa-gwdt.wdat=on }
>
> it doesn't seem like a workable approach
I see you need more magic than a simple alias. I dislike magic, but I
don't have better ideas.
>> >> > enum might work, but ...
>> >> > machine.watchdog = [acpi | what else could we put here???]
>> >> >
>> >> > I'm not married to a way how we expose/configure it and will rewrite
>> >> > it to something that works for majority.
>> >> >
>> >> > > thanks
>> >> > > -- PMM
>> >> > >
>> >> >
>> >>
>> >> With regards,
>> >> Daniel
>>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 12:10 ` Markus Armbruster
@ 2026-02-27 13:14 ` Peter Maydell
2026-02-27 14:51 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Peter Maydell @ 2026-02-27 13:14 UTC (permalink / raw)
To: Markus Armbruster
Cc: Igor Mammedov, Daniel P. Berrangé, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Fri, 27 Feb 2026 at 12:10, Markus Armbruster <armbru@redhat.com> wrote:
> The larger problem is that we generally fail at documenting device and
> machine properties. I'm not asking you to boil that ocean :)
>
> The narrow problem is providing guidance on watchdog configuration.
> What and where?
>
> Regarding where: you proposed to add a machine property of QAPI enum
> type.
>
> The property can have a description, and you give it one. Sadly, it's
> basically invisible. As far as I know, the only way you can get at it
> is qom-list-properties and such. I doubt anybody is going to read the
> description there.
I think it also will appear if you run e.g.
qemu-system-arm -M virt,help
(which produces a list of all properties with their short descriptions)
For board-specific properties, we should be documenting these in
the manual page for the board. For instance "virt" does that here:
https://www.qemu.org/docs/master/system/arm/virt.html#machine-specific-options
For the global machine options, I don't think we have a place where we
document those. If we did, we should perhaps have each board's page
also link to the global-options page.
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 13:14 ` Peter Maydell
@ 2026-02-27 14:51 ` Markus Armbruster
2026-02-27 15:01 ` Peter Maydell
0 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2026-02-27 14:51 UTC (permalink / raw)
To: Peter Maydell
Cc: Igor Mammedov, Daniel P. Berrangé, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
Peter Maydell <peter.maydell@linaro.org> writes:
> On Fri, 27 Feb 2026 at 12:10, Markus Armbruster <armbru@redhat.com> wrote:
>> The larger problem is that we generally fail at documenting device and
>> machine properties. I'm not asking you to boil that ocean :)
>>
>> The narrow problem is providing guidance on watchdog configuration.
>> What and where?
>>
>> Regarding where: you proposed to add a machine property of QAPI enum
>> type.
>>
>> The property can have a description, and you give it one. Sadly, it's
>> basically invisible. As far as I know, the only way you can get at it
>> is qom-list-properties and such. I doubt anybody is going to read the
>> description there.
>
> I think it also will appear if you run e.g.
> qemu-system-arm -M virt,help
>
> (which produces a list of all properties with their short descriptions)
You're right!
> For board-specific properties, we should be documenting these in
> the manual page for the board. For instance "virt" does that here:
> https://www.qemu.org/docs/master/system/arm/virt.html#machine-specific-options
Every board should have such a page. Even if it's just a placeholder.
Placeholders would remind us where the gaps are.
> For the global machine options, I don't think we have a place where we
> document those. If we did, we should perhaps have each board's page
> also link to the global-options page.
We should do this.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 14:51 ` Markus Armbruster
@ 2026-02-27 15:01 ` Peter Maydell
2026-03-02 5:32 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Peter Maydell @ 2026-02-27 15:01 UTC (permalink / raw)
To: Markus Armbruster
Cc: Igor Mammedov, Daniel P. Berrangé, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
On Fri, 27 Feb 2026 at 14:51, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Fri, 27 Feb 2026 at 12:10, Markus Armbruster <armbru@redhat.com> wrote:
> >> The larger problem is that we generally fail at documenting device and
> >> machine properties. I'm not asking you to boil that ocean :)
> >>
> >> The narrow problem is providing guidance on watchdog configuration.
> >> What and where?
> >>
> >> Regarding where: you proposed to add a machine property of QAPI enum
> >> type.
> >>
> >> The property can have a description, and you give it one. Sadly, it's
> >> basically invisible. As far as I know, the only way you can get at it
> >> is qom-list-properties and such. I doubt anybody is going to read the
> >> description there.
> >
> > I think it also will appear if you run e.g.
> > qemu-system-arm -M virt,help
> >
> > (which produces a list of all properties with their short descriptions)
>
> You're right!
>
> > For board-specific properties, we should be documenting these in
> > the manual page for the board. For instance "virt" does that here:
> > https://www.qemu.org/docs/master/system/arm/virt.html#machine-specific-options
>
> Every board should have such a page. Even if it's just a placeholder.
> Placeholders would remind us where the gaps are.
I agree. For Arm I believe every board should already have a docs page,
even if a very minimal one. (Some closely related boards get
described on a single page that covers that whole family of boards.)
The full list of pages is here:
https://www.qemu.org/docs/master/system/target-arm.html
We follow the same pattern (but not always with complete coverage)
in most but not all the other target architectures.
I don't think there's currently any automatic way to get a list
of undocumented boards, because there isn't any tagging of the
boards that are documented except in as much as the board names
are listed in the title of the relevant pages.
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table
2026-02-27 15:01 ` Peter Maydell
@ 2026-03-02 5:32 ` Markus Armbruster
0 siblings, 0 replies; 47+ messages in thread
From: Markus Armbruster @ 2026-03-02 5:32 UTC (permalink / raw)
To: Peter Maydell
Cc: Igor Mammedov, Daniel P. Berrangé, qemu-devel, mst, anisinha,
pbonzini, shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
Peter Maydell <peter.maydell@linaro.org> writes:
> On Fri, 27 Feb 2026 at 14:51, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On Fri, 27 Feb 2026 at 12:10, Markus Armbruster <armbru@redhat.com> wrote:
>> >> The larger problem is that we generally fail at documenting device and
>> >> machine properties. I'm not asking you to boil that ocean :)
>> >>
>> >> The narrow problem is providing guidance on watchdog configuration.
>> >> What and where?
>> >>
>> >> Regarding where: you proposed to add a machine property of QAPI enum
>> >> type.
>> >>
>> >> The property can have a description, and you give it one. Sadly, it's
>> >> basically invisible. As far as I know, the only way you can get at it
>> >> is qom-list-properties and such. I doubt anybody is going to read the
>> >> description there.
>> >
>> > I think it also will appear if you run e.g.
>> > qemu-system-arm -M virt,help
>> >
>> > (which produces a list of all properties with their short descriptions)
>>
>> You're right!
>>
>> > For board-specific properties, we should be documenting these in
>> > the manual page for the board. For instance "virt" does that here:
>> > https://www.qemu.org/docs/master/system/arm/virt.html#machine-specific-options
>>
>> Every board should have such a page. Even if it's just a placeholder.
>> Placeholders would remind us where the gaps are.
>
> I agree. For Arm I believe every board should already have a docs page,
> even if a very minimal one. (Some closely related boards get
> described on a single page that covers that whole family of boards.)
> The full list of pages is here:
> https://www.qemu.org/docs/master/system/target-arm.html
>
> We follow the same pattern (but not always with complete coverage)
> in most but not all the other target architectures.
>
> I don't think there's currently any automatic way to get a list
> of undocumented boards, because there isn't any tagging of the
> boards that are documented except in as much as the board names
> are listed in the title of the relevant pages.
Alright, here's a first and incomplete attempt at the manual way.
docs/system/targets.rst "QEMU System Emulator Targets" lists target
pages docs/system/target-ARCH:
target-arm
target-avr
target-loongarch
target-m68k
target-mips
target-or1k
target-ppc
target-riscv
target-rx
target-s390x
target-sparc
target-sparc64
target-i386
target-xtensa
These appear to be mostly per "arch", not per target, i.e. there's just
target-mips, not -mips, -mipsel, -mips64, mips64el. Exception: -sparc
and -sparc64.
Checking against include/system/arch_init.h... We're missing
target-alpha
target-hexagon
target-hppa
target-microblaze
target-sh4
target-tricore
Some target pages list board pages under a "Board-specific
documentation" heading:
target-arm
arm/max78000
arm/integratorcp
arm/mps2
arm/musca
arm/realview
arm/sbsa
arm/versatile
arm/vexpress
arm/aspeed
arm/bananapi_m2u.rst
arm/b-l475e-iot01a.rst
arm/sabrelite
arm/highbank
arm/digic
arm/cubieboard
arm/emcraft-sf2
arm/exynos
arm/fby35
arm/musicpal
arm/kzm
arm/nrf
arm/nuvoton
arm/imx25-pdk
arm/mcimx6ul-evk
arm/mcimx7d-sabre
arm/imx8mp-evk
arm/orangepi
arm/raspi
arm/collie
arm/sx1
arm/stellaris
arm/stm32
arm/virt
arm/vmapple
arm/xenpvh
arm/xlnx-versal-virt
arm/xlnx-zynq
arm/xlnx-zcu102
target-i386
i386/pc
i386/microvm
i386/nitro-enclave
target-loongarch
loongarch/virt
target-or1k
or1k/or1k-sim
or1k/virt
target-ppc
ppc/amigang
ppc/embedded
ppc/powermac
ppc/powernv
ppc/ppce500
ppc/prep
ppc/pseries
target-riscv
riscv/microblaze-v-generic
riscv/microchip-icicle-kit
riscv/mips
riscv/shakti-c
riscv/sifive_u
riscv/virt
riscv/xiangshan-kunminghu
Pages may cover multiple machine types (e.g. ppc/amigang covers
amigaone, pegasos1, pegasos2, sam460ex). We might want to tweak titles
to help us find the board names more easily, so we can check for
completeness.
Target pages without a "Board-specific documentation" heading:
target-avr
target-m68k
target-mips
target-rx
target-s390x
target-sparc
target-sparc64
target-xtensa
Some of these do talk about boards, just in less greppable ways. We
might want to adjust them to better support completeness checking.
Worth pursuing this further?
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 09/11] tests: acpi: arm/virt: whitelist new WDAT table
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (7 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 08/11] arm: virt: create GWDT watchdog paired with WDAT ACPI table Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-06 13:14 ` [PATCH 10/11] tests: acpi: arm/virt: add WDAT table test case Igor Mammedov
` (3 subsequent siblings)
12 siblings, 0 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 +
tests/data/acpi/aarch64/virt/WDAT.wdat | 0
2 files changed, 1 insertion(+)
create mode 100644 tests/data/acpi/aarch64/virt/WDAT.wdat
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..135186b40b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/aarch64/virt/WDAT.wdat",
diff --git a/tests/data/acpi/aarch64/virt/WDAT.wdat b/tests/data/acpi/aarch64/virt/WDAT.wdat
new file mode 100644
index 0000000000..e69de29bb2
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 10/11] tests: acpi: arm/virt: add WDAT table test case
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (8 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 09/11] tests: acpi: arm/virt: whitelist new WDAT table Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-06 13:14 ` [PATCH 11/11] tests: acpi: arm/virt: update expected WDAT blob Igor Mammedov
` (2 subsequent siblings)
12 siblings, 0 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 25345dd035..c2d235e686 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -2252,6 +2252,25 @@ static void test_acpi_aarch64_virt_tcg_its_off(void)
free_test_data(&data);
}
+static void test_acpi_aarch64_virt_tcg_wdat(void)
+{
+ test_data data = {
+ .machine = "virt",
+ .arch = "aarch64",
+ .variant = ".wdat",
+ .tcg_only = true,
+ .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+ .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+ .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+ .ram_start = 0x40000000ULL,
+ .scan_len = 128ULL * MiB,
+ };
+
+ test_acpi_one("-cpu cortex-a57 "
+ "-machine acpi-watchdog=on", &data);
+ free_test_data(&data);
+}
+
static void test_acpi_q35_viot(void)
{
test_data data = {
@@ -2868,6 +2887,8 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/virt/smmuv3-dev",
test_acpi_aarch64_virt_smmuv3_dev);
}
+ qtest_add_func("acpi/virt/acpi-watchdog",
+ test_acpi_aarch64_virt_tcg_wdat);
}
} else if (strcmp(arch, "riscv64") == 0) {
if (has_tcg && qtest_has_device("virtio-blk-pci")) {
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 11/11] tests: acpi: arm/virt: update expected WDAT blob
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (9 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 10/11] tests: acpi: arm/virt: add WDAT table test case Igor Mammedov
@ 2026-02-06 13:14 ` Igor Mammedov
2026-02-16 7:39 ` [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Ani Sinha
2026-02-18 19:10 ` Peter Maydell
12 siblings, 0 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-06 13:14 UTC (permalink / raw)
To: qemu-devel
Cc: mst, anisinha, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
replace blank table with a new one:
[000h 0000 4] Signature : "WDAT" [Watchdog Action Table]
[004h 0004 4] Table Length : 00000104
[008h 0008 1] Revision : 01
[009h 0009 1] Checksum : EB
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPC "
[018h 0024 4] Oem Revision : 00000001
[01Ch 0028 4] Asl Compiler ID : "BXPC"
[020h 0032 4] Asl Compiler Revision : 00000001
[024h 0036 4] Header Length : 00000020
[028h 0040 2] PCI Segment : 00FF
[02Ah 0042 1] PCI Bus : FF
[02Bh 0043 1] PCI Device : FF
[02Ch 0044 1] PCI Function : FF
[02Dh 0045 3] Reserved : 000000
[030h 0048 4] Timer Period : 00000001
[034h 0052 4] Max Count : 000927C0
[038h 0056 4] Min Count : 00001388
[03Ch 0060 1] Flags (decoded below) : 81
Enabled : 1
Stopped When Asleep : 1
[03Dh 0061 3] Reserved : 000000
[040h 0064 4] Watchdog Entry Count : 00000008
[044h 0068 1] Watchdog Action : 08
[045h 0069 1] Instruction : 00
[046h 0070 2] Reserved : 0000
[048h 0072 12] Register Region : [Generic Address Structure]
[048h 0072 1] Space ID : 00 [SystemMemory]
[049h 0073 1] Bit Width : 20
[04Ah 0074 1] Bit Offset : 00
[04Bh 0075 1] Encoded Access Width : 03 [DWord Access:32]
[04Ch 0076 8] Address : 00000000090D1000
[054h 0084 4] Value : 00000001
[058h 0088 4] Register Mask : 00000001
[05Ch 0092 1] Watchdog Action : 01
[05Dh 0093 1] Instruction : 02
[05Eh 0094 2] Reserved : 0000
[060h 0096 12] Register Region : [Generic Address Structure]
[060h 0096 1] Space ID : 00 [SystemMemory]
[061h 0097 1] Bit Width : 20
[062h 0098 1] Bit Offset : 00
[063h 0099 1] Encoded Access Width : 03 [DWord Access:32]
[064h 0100 8] Address : 00000000090D0000
[06Ch 0108 4] Value : 00000001
[070h 0112 4] Register Mask : 00000007
[074h 0116 1] Watchdog Action : 06
[075h 0117 1] Instruction : 03
[076h 0118 2] Reserved : 0000
[078h 0120 12] Register Region : [Generic Address Structure]
[078h 0120 1] Space ID : 00 [SystemMemory]
[079h 0121 1] Bit Width : 20
[07Ah 0122 1] Bit Offset : 00
[07Bh 0123 1] Encoded Access Width : 03 [DWord Access:32]
[07Ch 0124 8] Address : 00000000090D1008
[084h 0132 4] Value : 00000000
[088h 0136 4] Register Mask : FFFFFFFF
[08Ch 0140 1] Watchdog Action : 09
[08Dh 0141 1] Instruction : 82
[08Eh 0142 2] Reserved : 0000
[090h 0144 12] Register Region : [Generic Address Structure]
[090h 0144 1] Space ID : 00 [SystemMemory]
[091h 0145 1] Bit Width : 20
[092h 0146 1] Bit Offset : 00
[093h 0147 1] Encoded Access Width : 03 [DWord Access:32]
[094h 0148 8] Address : 00000000090D1000
[09Ch 0156 4] Value : 00000001
[0A0h 0160 4] Register Mask : 00000001
[0A4h 0164 1] Watchdog Action : 0A
[0A5h 0165 1] Instruction : 00
[0A6h 0166 2] Reserved : 0000
[0A8h 0168 12] Register Region : [Generic Address Structure]
[0A8h 0168 1] Space ID : 00 [SystemMemory]
[0A9h 0169 1] Bit Width : 20
[0AAh 0170 1] Bit Offset : 00
[0ABh 0171 1] Encoded Access Width : 03 [DWord Access:32]
[0ACh 0172 8] Address : 00000000090D1000
[0B4h 0180 4] Value : 00000000
[0B8h 0184 4] Register Mask : 00000001
[0BCh 0188 1] Watchdog Action : 0B
[0BDh 0189 1] Instruction : 82
[0BEh 0190 2] Reserved : 0000
[0C0h 0192 12] Register Region : [Generic Address Structure]
[0C0h 0192 1] Space ID : 00 [SystemMemory]
[0C1h 0193 1] Bit Width : 20
[0C2h 0194 1] Bit Offset : 00
[0C3h 0195 1] Encoded Access Width : 03 [DWord Access:32]
[0C4h 0196 8] Address : 00000000090D1000
[0CCh 0204 4] Value : 00000000
[0D0h 0208 4] Register Mask : 00000001
[0D4h 0212 1] Watchdog Action : 20
[0D5h 0213 1] Instruction : 00
[0D6h 0214 2] Reserved : 0000
[0D8h 0216 12] Register Region : [Generic Address Structure]
[0D8h 0216 1] Space ID : 00 [SystemMemory]
[0D9h 0217 1] Bit Width : 20
[0DAh 0218 1] Bit Offset : 00
[0DBh 0219 1] Encoded Access Width : 03 [DWord Access:32]
[0DCh 0220 8] Address : 00000000090D1000
[0E4h 0228 4] Value : 00000004
[0E8h 0232 4] Register Mask : 00000004
[0ECh 0236 1] Watchdog Action : 21
[0EDh 0237 1] Instruction : 82
[0EEh 0238 2] Reserved : 0000
[0F0h 0240 12] Register Region : [Generic Address Structure]
[0F0h 0240 1] Space ID : 00 [SystemMemory]
[0F1h 0241 1] Bit Width : 20
[0F2h 0242 1] Bit Offset : 00
[0F3h 0243 1] Encoded Access Width : 03 [DWord Access:32]
[0F4h 0244 8] Address : 00000000090D0000
[0FCh 0252 4] Value : 00000004
[100h 0256 4] Register Mask : 00000004
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
tests/data/acpi/aarch64/virt/WDAT.wdat | Bin 0 -> 260 bytes
2 files changed, 1 deletion(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 135186b40b..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/aarch64/virt/WDAT.wdat",
diff --git a/tests/data/acpi/aarch64/virt/WDAT.wdat b/tests/data/acpi/aarch64/virt/WDAT.wdat
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..bac7a101c3d349e0b9555963526d5e497af27c67 100644
GIT binary patch
literal 260
zcmZ9GK?=e^3`O5`itQ+Z;05B!bJ)6Yr=XzMv8x`d_vs-bX){<F2uVIKpZ~sIUS)tV
zHuv`Sl;-njMMMH%yG;QURR!!G&WZjAU!e9zh#d9_XN(KQ^c>-@SHLFDdhNWGs%q+-
cU-@heSom9+|8B)+-SRH^5~e<7-t%Pr2hLL!0RR91
literal 0
HcmV?d00001
--
2.47.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (10 preceding siblings ...)
2026-02-06 13:14 ` [PATCH 11/11] tests: acpi: arm/virt: update expected WDAT blob Igor Mammedov
@ 2026-02-16 7:39 ` Ani Sinha
2026-02-16 8:46 ` Mohamed Mediouni
2026-02-18 9:29 ` Igor Mammedov
2026-02-18 19:10 ` Peter Maydell
12 siblings, 2 replies; 47+ messages in thread
From: Ani Sinha @ 2026-02-16 7:39 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, Feb 6, 2026 at 6:44 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> Windows doesn't ship built-in TCO watchdog driver, and users are
> forced to install vendor specific driver(s) if such exists.
> However OS provides a generic watchdog driver that uses
> ACPI WDAT table [1] to abstract actual hardware behind it.
> The same applies to ARM version of Windows.
>
> This series adds
> * WDAT table tailored for TCO watchdog that Q35 already
> has as part of ICH9 chipset and a knob to enable it.
> * SBSA GWDT to arm/vit machine with WDAT table tailored
> for it and necessary tweaks to GWDT to make usable
> with WDAT driver.
>
> Guest OS support is present since Windows Vista and Linux since 2016[2].
I am confused. The linux documentation for arm64 says WDAT is not supported
https://www.kernel.org/doc/html/latest/arch/arm64/acpi_object_usage.html
>
> One can test it launching VM with following options:
> -machine acpi-watchdog=on
> to trace access to TCO registers one can add to CLI:
> -trace "tco_*"
> to trace access to GWDT registers one can add to CLI:
> -trace "sbsa_gwdt*"
>
> To verify that guest uses WDAT (both Windows and Linux would use it if present),
> * on linux make sure that watchdog service is configured/enabled
> (use wdctl to check what watchdog device to use and confirm that it's
> wadt based one) and then trigger kernel panic with command:
> echo c > /proc/sysrq-trigger
> * on Windows kill svchost process, running cmd in admin mode:
> taskkill /f /im svchost.exe
>
> Tested with WS2025 & RHEL9.6 & Fedore 43 (aarch64)
>
> git tree at: https://gitlab.com/imammedo/qemu/-/commits/gwdt_v2
>
> 1) http://msdn.microsoft.com/en-us/windows/hardware/gg463320.aspx
> 2) https://lwn.net/Articles/700524/
>
> PS:
> previous revision (q35 only):
> https://patchwork.ozlabs.org/project/qemu-devel/cover/20251030123750.136175-1-imammedo@redhat.com/
>
> Also CCing SBSA folks to see if they are interested in adding it to their
> reference machine
>
> Igor Mammedov (11):
> acpi: add API to build WDAT instructions
> machine: add "acpi-watchdog" property
> x86: q35: generate WDAT ACPI table
> tests: acpi: x86/q35: whitelist new WDAT table
> tests: acpi: x86/q35: add WDAT table test case
> tests: acpi: x86/q35: update expected WDAT blob
> arm: add tracing events to sbsa_gwdt
> arm: virt: create GWDT watchdog paired with WDAT ACPI table
> tests: acpi: arm/virt: whitelist new WDAT table
> tests: acpi: arm/virt: add WDAT table test case
> tests: acpi: arm/virt: update expected WDAT blob
>
> include/hw/acpi/wdat-gwdt.h | 19 ++++
> include/hw/acpi/wdat-ich9.h | 15 ++++
> include/hw/acpi/wdat.h | 118 +++++++++++++++++++++++++
> include/hw/arm/virt.h | 3 +
> include/hw/core/boards.h | 1 +
> hw/acpi/aml-build.c | 14 +++
> hw/acpi/meson.build | 5 +-
> hw/acpi/wdat-gwdt-stub.c | 16 ++++
> hw/acpi/wdat-gwdt.c | 92 +++++++++++++++++++
> hw/acpi/wdat-ich9-stub.c | 15 ++++
> hw/acpi/wdat-ich9.c | 87 ++++++++++++++++++
> hw/arm/Kconfig | 1 +
> hw/arm/virt-acpi-build.c | 16 ++++
> hw/arm/virt.c | 26 ++++++
> hw/core/machine.c | 20 +++++
> hw/i386/acpi-build.c | 12 +++
> hw/watchdog/sbsa_gwdt.c | 8 ++
> hw/watchdog/trace-events | 9 ++
> tests/data/acpi/aarch64/virt/WDAT.wdat | Bin 0 -> 260 bytes
> tests/data/acpi/x86/q35/WDAT.wdat | Bin 0 -> 308 bytes
> tests/qtest/bios-tables-test.c | 34 +++++++
> 21 files changed, 510 insertions(+), 1 deletion(-)
> create mode 100644 include/hw/acpi/wdat-gwdt.h
> create mode 100644 include/hw/acpi/wdat-ich9.h
> create mode 100644 include/hw/acpi/wdat.h
> create mode 100644 hw/acpi/wdat-gwdt-stub.c
> create mode 100644 hw/acpi/wdat-gwdt.c
> create mode 100644 hw/acpi/wdat-ich9-stub.c
> create mode 100644 hw/acpi/wdat-ich9.c
> create mode 100644 tests/data/acpi/aarch64/virt/WDAT.wdat
> create mode 100644 tests/data/acpi/x86/q35/WDAT.wdat
>
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards
2026-02-16 7:39 ` [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Ani Sinha
@ 2026-02-16 8:46 ` Mohamed Mediouni
2026-02-18 9:29 ` Igor Mammedov
1 sibling, 0 replies; 47+ messages in thread
From: Mohamed Mediouni @ 2026-02-16 8:46 UTC (permalink / raw)
To: Ani Sinha
Cc: Igor Mammedov, qemu-devel, mst, pbonzini, peter.maydell,
shannon.zhaosl, philmd, zhao1.liu, rad, leif.lindholm
[-- Attachment #1: Type: text/plain, Size: 4664 bytes --]
> On 16. Feb 2026, at 08:39, Ani Sinha <anisinha@redhat.com> wrote:
>
> On Fri, Feb 6, 2026 at 6:44 PM Igor Mammedov <imammedo@redhat.com> wrote:
>>
>> Windows doesn't ship built-in TCO watchdog driver, and users are
>> forced to install vendor specific driver(s) if such exists.
>> However OS provides a generic watchdog driver that uses
>> ACPI WDAT table [1] to abstract actual hardware behind it.
>> The same applies to ARM version of Windows.
>>
>> This series adds
>> * WDAT table tailored for TCO watchdog that Q35 already
>> has as part of ICH9 chipset and a knob to enable it.
>> * SBSA GWDT to arm/vit machine with WDAT table tailored
>> for it and necessary tweaks to GWDT to make usable
>> with WDAT driver.
>>
>> Guest OS support is present since Windows Vista and Linux since 2016[2].
>
> I am confused. The linux documentation for arm64 says WDAT is not supported
> https://www.kernel.org/doc/html/latest/arch/arm64/acpi_object_usage.html
Hi,
It’s outdated documentation, support for it has been added in https://lkml.iu.edu/hypermail/linux/kernel/1609.1/04003.html
>
>>
>> One can test it launching VM with following options:
>> -machine acpi-watchdog=on
>> to trace access to TCO registers one can add to CLI:
>> -trace "tco_*"
>> to trace access to GWDT registers one can add to CLI:
>> -trace "sbsa_gwdt*"
>>
>> To verify that guest uses WDAT (both Windows and Linux would use it if present),
>> * on linux make sure that watchdog service is configured/enabled
>> (use wdctl to check what watchdog device to use and confirm that it's
>> wadt based one) and then trigger kernel panic with command:
>> echo c > /proc/sysrq-trigger
>> * on Windows kill svchost process, running cmd in admin mode:
>> taskkill /f /im svchost.exe
>>
>> Tested with WS2025 & RHEL9.6 & Fedore 43 (aarch64)
>>
>> git tree at: https://gitlab.com/imammedo/qemu/-/commits/gwdt_v2
>>
>> 1) http://msdn.microsoft.com/en-us/windows/hardware/gg463320.aspx
>> 2) https://lwn.net/Articles/700524/
>>
>> PS:
>> previous revision (q35 only):
>> https://patchwork.ozlabs.org/project/qemu-devel/cover/20251030123750.136175-1-imammedo@redhat.com/
>>
>> Also CCing SBSA folks to see if they are interested in adding it to their
>> reference machine
>>
>> Igor Mammedov (11):
>> acpi: add API to build WDAT instructions
>> machine: add "acpi-watchdog" property
>> x86: q35: generate WDAT ACPI table
>> tests: acpi: x86/q35: whitelist new WDAT table
>> tests: acpi: x86/q35: add WDAT table test case
>> tests: acpi: x86/q35: update expected WDAT blob
>> arm: add tracing events to sbsa_gwdt
>> arm: virt: create GWDT watchdog paired with WDAT ACPI table
>> tests: acpi: arm/virt: whitelist new WDAT table
>> tests: acpi: arm/virt: add WDAT table test case
>> tests: acpi: arm/virt: update expected WDAT blob
>>
>> include/hw/acpi/wdat-gwdt.h | 19 ++++
>> include/hw/acpi/wdat-ich9.h | 15 ++++
>> include/hw/acpi/wdat.h | 118 +++++++++++++++++++++++++
>> include/hw/arm/virt.h | 3 +
>> include/hw/core/boards.h | 1 +
>> hw/acpi/aml-build.c | 14 +++
>> hw/acpi/meson.build | 5 +-
>> hw/acpi/wdat-gwdt-stub.c | 16 ++++
>> hw/acpi/wdat-gwdt.c | 92 +++++++++++++++++++
>> hw/acpi/wdat-ich9-stub.c | 15 ++++
>> hw/acpi/wdat-ich9.c | 87 ++++++++++++++++++
>> hw/arm/Kconfig | 1 +
>> hw/arm/virt-acpi-build.c | 16 ++++
>> hw/arm/virt.c | 26 ++++++
>> hw/core/machine.c | 20 +++++
>> hw/i386/acpi-build.c | 12 +++
>> hw/watchdog/sbsa_gwdt.c | 8 ++
>> hw/watchdog/trace-events | 9 ++
>> tests/data/acpi/aarch64/virt/WDAT.wdat | Bin 0 -> 260 bytes
>> tests/data/acpi/x86/q35/WDAT.wdat | Bin 0 -> 308 bytes
>> tests/qtest/bios-tables-test.c | 34 +++++++
>> 21 files changed, 510 insertions(+), 1 deletion(-)
>> create mode 100644 include/hw/acpi/wdat-gwdt.h
>> create mode 100644 include/hw/acpi/wdat-ich9.h
>> create mode 100644 include/hw/acpi/wdat.h
>> create mode 100644 hw/acpi/wdat-gwdt-stub.c
>> create mode 100644 hw/acpi/wdat-gwdt.c
>> create mode 100644 hw/acpi/wdat-ich9-stub.c
>> create mode 100644 hw/acpi/wdat-ich9.c
>> create mode 100644 tests/data/acpi/aarch64/virt/WDAT.wdat
>> create mode 100644 tests/data/acpi/x86/q35/WDAT.wdat
>>
>> --
>> 2.47.3
>>
>
>
[-- Attachment #2: Type: text/html, Size: 7220 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards
2026-02-16 7:39 ` [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Ani Sinha
2026-02-16 8:46 ` Mohamed Mediouni
@ 2026-02-18 9:29 ` Igor Mammedov
1 sibling, 0 replies; 47+ messages in thread
From: Igor Mammedov @ 2026-02-18 9:29 UTC (permalink / raw)
To: Ani Sinha
Cc: qemu-devel, mst, pbonzini, peter.maydell, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Mon, 16 Feb 2026 13:09:30 +0530
Ani Sinha <anisinha@redhat.com> wrote:
> On Fri, Feb 6, 2026 at 6:44 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > Windows doesn't ship built-in TCO watchdog driver, and users are
> > forced to install vendor specific driver(s) if such exists.
> > However OS provides a generic watchdog driver that uses
> > ACPI WDAT table [1] to abstract actual hardware behind it.
> > The same applies to ARM version of Windows.
> >
> > This series adds
> > * WDAT table tailored for TCO watchdog that Q35 already
> > has as part of ICH9 chipset and a knob to enable it.
> > * SBSA GWDT to arm/vit machine with WDAT table tailored
> > for it and necessary tweaks to GWDT to make usable
> > with WDAT driver.
> >
> > Guest OS support is present since Windows Vista and Linux since 2016[2].
>
> I am confused. The linux documentation for arm64 says WDAT is not supported
> https://www.kernel.org/doc/html/latest/arch/arm64/acpi_object_usage.html
IT probably should be fixed,
the WDAT support builds and actually works
and it's no different than x86 in respect that it's Windows specific ACPI table.
Justification for using WDAT on ARM is the same as x86,
reuse a more generic driver that hw supports for the sake
of out of box Windows driver, instead of writing it for each
watchdog impl.
Yep, underlying hw watchdog abstracted by WDAT is limited
but if it is sufficient for typical usecase, there is not need to
for guest to ship hw specific driver.
From QEMU pov, WDAT allows us to change used hw watchdog
without breaking guests. (i.e. if we decide to use another
watchdog, we can do so by starting from new machine type and
slowly phase out previously used one)
Thanks for review,
I'll respin series with your comments addressed.
>
> >
> > One can test it launching VM with following options:
> > -machine acpi-watchdog=on
> > to trace access to TCO registers one can add to CLI:
> > -trace "tco_*"
> > to trace access to GWDT registers one can add to CLI:
> > -trace "sbsa_gwdt*"
> >
> > To verify that guest uses WDAT (both Windows and Linux would use it if present),
> > * on linux make sure that watchdog service is configured/enabled
> > (use wdctl to check what watchdog device to use and confirm that it's
> > wadt based one) and then trigger kernel panic with command:
> > echo c > /proc/sysrq-trigger
> > * on Windows kill svchost process, running cmd in admin mode:
> > taskkill /f /im svchost.exe
> >
> > Tested with WS2025 & RHEL9.6 & Fedore 43 (aarch64)
> >
> > git tree at: https://gitlab.com/imammedo/qemu/-/commits/gwdt_v2
> >
> > 1) http://msdn.microsoft.com/en-us/windows/hardware/gg463320.aspx
> > 2) https://lwn.net/Articles/700524/
> >
> > PS:
> > previous revision (q35 only):
> > https://patchwork.ozlabs.org/project/qemu-devel/cover/20251030123750.136175-1-imammedo@redhat.com/
> >
> > Also CCing SBSA folks to see if they are interested in adding it to their
> > reference machine
> >
> > Igor Mammedov (11):
> > acpi: add API to build WDAT instructions
> > machine: add "acpi-watchdog" property
> > x86: q35: generate WDAT ACPI table
> > tests: acpi: x86/q35: whitelist new WDAT table
> > tests: acpi: x86/q35: add WDAT table test case
> > tests: acpi: x86/q35: update expected WDAT blob
> > arm: add tracing events to sbsa_gwdt
> > arm: virt: create GWDT watchdog paired with WDAT ACPI table
> > tests: acpi: arm/virt: whitelist new WDAT table
> > tests: acpi: arm/virt: add WDAT table test case
> > tests: acpi: arm/virt: update expected WDAT blob
> >
> > include/hw/acpi/wdat-gwdt.h | 19 ++++
> > include/hw/acpi/wdat-ich9.h | 15 ++++
> > include/hw/acpi/wdat.h | 118 +++++++++++++++++++++++++
> > include/hw/arm/virt.h | 3 +
> > include/hw/core/boards.h | 1 +
> > hw/acpi/aml-build.c | 14 +++
> > hw/acpi/meson.build | 5 +-
> > hw/acpi/wdat-gwdt-stub.c | 16 ++++
> > hw/acpi/wdat-gwdt.c | 92 +++++++++++++++++++
> > hw/acpi/wdat-ich9-stub.c | 15 ++++
> > hw/acpi/wdat-ich9.c | 87 ++++++++++++++++++
> > hw/arm/Kconfig | 1 +
> > hw/arm/virt-acpi-build.c | 16 ++++
> > hw/arm/virt.c | 26 ++++++
> > hw/core/machine.c | 20 +++++
> > hw/i386/acpi-build.c | 12 +++
> > hw/watchdog/sbsa_gwdt.c | 8 ++
> > hw/watchdog/trace-events | 9 ++
> > tests/data/acpi/aarch64/virt/WDAT.wdat | Bin 0 -> 260 bytes
> > tests/data/acpi/x86/q35/WDAT.wdat | Bin 0 -> 308 bytes
> > tests/qtest/bios-tables-test.c | 34 +++++++
> > 21 files changed, 510 insertions(+), 1 deletion(-)
> > create mode 100644 include/hw/acpi/wdat-gwdt.h
> > create mode 100644 include/hw/acpi/wdat-ich9.h
> > create mode 100644 include/hw/acpi/wdat.h
> > create mode 100644 hw/acpi/wdat-gwdt-stub.c
> > create mode 100644 hw/acpi/wdat-gwdt.c
> > create mode 100644 hw/acpi/wdat-ich9-stub.c
> > create mode 100644 hw/acpi/wdat-ich9.c
> > create mode 100644 tests/data/acpi/aarch64/virt/WDAT.wdat
> > create mode 100644 tests/data/acpi/x86/q35/WDAT.wdat
> >
> > --
> > 2.47.3
> >
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards
2026-02-06 13:14 [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Igor Mammedov
` (11 preceding siblings ...)
2026-02-16 7:39 ` [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards Ani Sinha
@ 2026-02-18 19:10 ` Peter Maydell
2026-02-19 12:27 ` Igor Mammedov
12 siblings, 1 reply; 47+ messages in thread
From: Peter Maydell @ 2026-02-18 19:10 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Fri, 6 Feb 2026 at 13:14, Igor Mammedov <imammedo@redhat.com> wrote:
>
> Windows doesn't ship built-in TCO watchdog driver, and users are
> forced to install vendor specific driver(s) if such exists.
> However OS provides a generic watchdog driver that uses
> ACPI WDAT table [1] to abstract actual hardware behind it.
> The same applies to ARM version of Windows.
>
> This series adds
> * WDAT table tailored for TCO watchdog that Q35 already
> has as part of ICH9 chipset and a knob to enable it.
> * SBSA GWDT to arm/vit machine with WDAT table tailored
> for it and necessary tweaks to GWDT to make usable
> with WDAT driver.
> Also CCing SBSA folks to see if they are interested in adding it to their
> reference machine
The sbsa-ref machine already implements an sbsa-gwdt watchdog...
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards
2026-02-18 19:10 ` Peter Maydell
@ 2026-02-19 12:27 ` Igor Mammedov
2026-02-19 14:05 ` Peter Maydell
0 siblings, 1 reply; 47+ messages in thread
From: Igor Mammedov @ 2026-02-19 12:27 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Wed, 18 Feb 2026 19:10:02 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:
> On Fri, 6 Feb 2026 at 13:14, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > Windows doesn't ship built-in TCO watchdog driver, and users are
> > forced to install vendor specific driver(s) if such exists.
> > However OS provides a generic watchdog driver that uses
> > ACPI WDAT table [1] to abstract actual hardware behind it.
> > The same applies to ARM version of Windows.
> >
> > This series adds
> > * WDAT table tailored for TCO watchdog that Q35 already
> > has as part of ICH9 chipset and a knob to enable it.
> > * SBSA GWDT to arm/vit machine with WDAT table tailored
> > for it and necessary tweaks to GWDT to make usable
> > with WDAT driver.
>
> > Also CCing SBSA folks to see if they are interested in adding it to their
> > reference machine
>
> The sbsa-ref machine already implements an sbsa-gwdt watchdog...
yep, but not exposed as WDAT one. Hence the question.
It also would need clock-frequency tweak if we are expose it via ACPI.
PS:
I don't even see DT mapping for it,
looks like sbsa-gwdt is just wired for the sake of compliance with the spec.
Well sbsa-ref specific firmware could just opencode it's mapping instead
of getting it from QEMU.
>
> -- PMM
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 00/11] Introduce ACPI watchdog for Q35 and arm/virt boards
2026-02-19 12:27 ` Igor Mammedov
@ 2026-02-19 14:05 ` Peter Maydell
0 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2026-02-19 14:05 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mst, anisinha, pbonzini, shannon.zhaosl, philmd,
zhao1.liu, rad, leif.lindholm
On Thu, 19 Feb 2026 at 12:27, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 18 Feb 2026 19:10:02 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
> > The sbsa-ref machine already implements an sbsa-gwdt watchdog...
>
> yep, but not exposed as WDAT one. Hence the question.
> It also would need clock-frequency tweak if we are expose it via ACPI.
>
> PS:
> I don't even see DT mapping for it,
> looks like sbsa-gwdt is just wired for the sake of compliance with the spec.
> Well sbsa-ref specific firmware could just opencode it's mapping instead
> of getting it from QEMU.
The way the sbsa-ref board works is that it's supposed to be a reference
system and software stack that behaves like a real hardware system would.
So the "dtb" is not a proper DTB, it's just a way to tell the firmware
a few key details (that on real hardware it would probably interrogate
a BMC to find out). The firmware knows the memory map and what devices
are present, and the ACPI tables to pass to the guest are entirely
handled by the firmware, in the same way they are on real hardware.
QEMU's ACPI-fragment handling mechanisms are not used at all.
(This is noted in docs/system/arm/sbsa.rst.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread