On Wed, 18 Feb 2026 19:08:36 +0000 Peter Maydell <[email protected]> wrote:
> On Fri, 6 Feb 2026 at 13:15, Igor Mammedov <[email protected]> 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 <[email protected]> > > --- > > > 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 >
