[PATCH] hw/arm: virt: Add SBSA watchdog

Jan Kiszka posted 1 patch 1 year, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/4ad779bc-09f6-4041-d671-624fd0e22cf9@web.de
Maintainers: Peter Maydell <peter.maydell@linaro.org>
hw/arm/Kconfig        |  1 +
hw/arm/virt.c         | 35 +++++++++++++++++++++++++++++++++++
include/hw/arm/virt.h |  3 +++
3 files changed, 39 insertions(+)
[PATCH] hw/arm: virt: Add SBSA watchdog
Posted by Jan Kiszka 1 year, 12 months ago
From: Jan Kiszka <jan.kiszka@siemens.com>

The virt machine lacks a watchdog so far while the sbsa-ref has a simple
model that is also supported by Linux and even U-Boot. Let's take it to
allow, e.g., system integration tests of A/B software update under
watchdog monitoring.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

An alternative could be sp805. However, QEMU has no support for that
device yet.

 hw/arm/Kconfig        |  1 +
 hw/arm/virt.c         | 35 +++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |  3 +++
 3 files changed, 39 insertions(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 97f3b38019..8df04a3533 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -29,6 +29,7 @@ config ARM_VIRT
     select ACPI_APEI
     select ACPI_VIOT
     select VIRTIO_MEM_SUPPORTED
+    select WDT_SBSA

 config CHEETAH
     bool
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f94278935f..a7b85438a8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -78,6 +78,7 @@
 #include "hw/virtio/virtio-mem-pci.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
+#include "hw/watchdog/sbsa_gwdt.h"
 #include "qemu/guest-random.h"

 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
@@ -154,6 +155,8 @@ static const MemMapEntry base_memmap[] = {
     [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
     [VIRT_PVTIME] =             { 0x090a0000, 0x00010000 },
     [VIRT_SECURE_GPIO] =        { 0x090b0000, 0x00001000 },
+    [VIRT_GWDT_CONTROL] =       { 0x090c0000, 0x00001000 },
+    [VIRT_GWDT_REFRESH] =       { 0x090d0000, 0x00001000 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -190,6 +193,7 @@ static const int a15irqmap[] = {
     [VIRT_GPIO] = 7,
     [VIRT_SECURE_UART] = 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 */
@@ -902,6 +906,35 @@ static void create_rtc(const VirtMachineState *vms)
     g_free(nodename);
 }

+static void create_wdt(const VirtMachineState *vms)
+{
+    hwaddr cbase = vms->memmap[VIRT_GWDT_CONTROL].base;
+    hwaddr csize = vms->memmap[VIRT_GWDT_CONTROL].size;
+    hwaddr rbase = vms->memmap[VIRT_GWDT_REFRESH].base;
+    hwaddr rsize = vms->memmap[VIRT_GWDT_REFRESH].size;
+    DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
+    SysBusDevice *s = SYS_BUS_DEVICE(dev);
+    int irq = vms->irqmap[VIRT_GWDT_WS0];
+    const char compat[] = "arm,sbsa-gwdt";
+    MachineState *ms = MACHINE(vms);
+    char *nodename;
+
+    sysbus_realize_and_unref(s, &error_fatal);
+    sysbus_mmio_map(s, 1, cbase);
+    sysbus_mmio_map(s, 0, rbase);
+    sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
+
+    nodename = g_strdup_printf("/watchdog@%" PRIx64, cbase);
+    qemu_fdt_add_subnode(ms->fdt, nodename);
+    qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
+                                 2, cbase, 2, csize, 2, rbase, 2, rsize);
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    g_free(nodename);
+}
+
 static DeviceState *gpio_key_dev;
 static void virt_powerdown_req(Notifier *n, void *opaque)
 {
@@ -2240,6 +2273,8 @@ static void machvirt_init(MachineState *machine)

     create_rtc(vms);

+    create_wdt(vms);
+
     create_pcie(vms);

     if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 15feabac63..7f295f771e 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -86,6 +86,9 @@ enum {
     VIRT_ACPI_GED,
     VIRT_NVDIMM_ACPI,
     VIRT_PVTIME,
+    VIRT_GWDT_WS0,
+    VIRT_GWDT_CONTROL,
+    VIRT_GWDT_REFRESH,
     VIRT_LOWMEMMAP_LAST,
 };

--
2.34.1
Re: [PATCH] hw/arm: virt: Add SBSA watchdog
Posted by Peter Maydell 1 year, 11 months ago
On Sun, 1 May 2022 at 19:07, Jan Kiszka <jan.kiszka@web.de> wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> The virt machine lacks a watchdog so far while the sbsa-ref has a simple
> model that is also supported by Linux and even U-Boot. Let's take it to
> allow, e.g., system integration tests of A/B software update under
> watchdog monitoring.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

The virt board has a PCI bus, and QEMU has a model of a
PCI watchdog device -- the i6300esb. Can you use that?

In general I much prefer it if we can support use cases in
virt via pluggable PCI devices rather than hard-wired MMIO
devices -- it means that only users who want the functionality
need to have the exposed attack surface area of the device
present in their VM.

thanks
-- PMM
Re: [PATCH] hw/arm: virt: Add SBSA watchdog
Posted by Jan Kiszka 1 year, 11 months ago
On 05.05.22 10:40, Peter Maydell wrote:
> On Sun, 1 May 2022 at 19:07, Jan Kiszka <jan.kiszka@web.de> wrote:
>>
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> The virt machine lacks a watchdog so far while the sbsa-ref has a simple
>> model that is also supported by Linux and even U-Boot. Let's take it to
>> allow, e.g., system integration tests of A/B software update under
>> watchdog monitoring.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The virt board has a PCI bus, and QEMU has a model of a
> PCI watchdog device -- the i6300esb. Can you use that?

While one could do that, it implies using completely alien Intel x86
host controller bits on this platform. And, thus, would require changes
in firmware (e.g. adding a driver to U-Boot).

> 
> In general I much prefer it if we can support use cases in
> virt via pluggable PCI devices rather than hard-wired MMIO
> devices -- it means that only users who want the functionality
> need to have the exposed attack surface area of the device
> present in their VM.

Valid point - would making this opt-in via a machine feature be sufficient?

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux
Re: [PATCH] hw/arm: virt: Add SBSA watchdog
Posted by Peter Maydell 1 year, 11 months ago
On Thu, 5 May 2022 at 11:17, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 05.05.22 10:40, Peter Maydell wrote:
> > On Sun, 1 May 2022 at 19:07, Jan Kiszka <jan.kiszka@web.de> wrote:
> >>
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> The virt machine lacks a watchdog so far while the sbsa-ref has a simple
> >> model that is also supported by Linux and even U-Boot. Let's take it to
> >> allow, e.g., system integration tests of A/B software update under
> >> watchdog monitoring.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >
> > The virt board has a PCI bus, and QEMU has a model of a
> > PCI watchdog device -- the i6300esb. Can you use that?
>
> While one could do that, it implies using completely alien Intel x86
> host controller bits on this platform. And, thus, would require changes
> in firmware (e.g. adding a driver to U-Boot).

> > In general I much prefer it if we can support use cases in
> > virt via pluggable PCI devices rather than hard-wired MMIO
> > devices -- it means that only users who want the functionality
> > need to have the exposed attack surface area of the device
> > present in their VM.
>
> Valid point - would making this opt-in via a machine feature be sufficient?

I really strongly prefer the PCI device solution. If you don't
like the i6300esb you could write a model of some other
PCI device.

thanks
-- PMM