| Message ID | 4ad779bc-09f6-4041-d671-624fd0e22cf9@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm: virt: Add SBSA watchdog | expand |
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
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
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
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, };