* [PATCH v5 0/2] Add watchdog support for SbsaQemu
@ 2020-10-13 15:16 Shashi Mallela
2020-10-13 15:16 ` [PATCH v5 1/2] hw/watchdog: Implement SBSA watchdog device Shashi Mallela
2020-10-13 15:16 ` [PATCH v5 2/2] hw/arm/sbsa-ref: add " Shashi Mallela
0 siblings, 2 replies; 8+ messages in thread
From: Shashi Mallela @ 2020-10-13 15:16 UTC (permalink / raw)
To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel
This patch series adds watchdog timer support for SbsaQemu platform.
The watchdog timer has been implemented first based on the generic
watchdog timer specifications from ARM BSA v0.9 and then used
in the SbsaQemu reference platform
Changes in v5:
- updated irq number of gwdt in sbsa-ref since the same
is being used by SMMU as per latest master merge
- addressed review comments
Shashi Mallela (2):
hw/watchdog: Implement SBSA watchdog device
hw/arm/sbsa-ref: add SBSA watchdog device
hw/arm/Kconfig | 1 +
hw/arm/sbsa-ref.c | 50 ++++
hw/watchdog/Kconfig | 4 +
hw/watchdog/meson.build | 1 +
hw/watchdog/wdt_sbsa_gwdt.c | 346 ++++++++++++++++++++++++++++
include/hw/watchdog/wdt_sbsa_gwdt.h | 70 ++++++
6 files changed, 472 insertions(+)
create mode 100644 hw/watchdog/wdt_sbsa_gwdt.c
create mode 100644 include/hw/watchdog/wdt_sbsa_gwdt.h
--
2.18.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] hw/watchdog: Implement SBSA watchdog device
2020-10-13 15:16 [PATCH v5 0/2] Add watchdog support for SbsaQemu Shashi Mallela
@ 2020-10-13 15:16 ` Shashi Mallela
2020-10-13 15:16 ` [PATCH v5 2/2] hw/arm/sbsa-ref: add " Shashi Mallela
1 sibling, 0 replies; 8+ messages in thread
From: Shashi Mallela @ 2020-10-13 15:16 UTC (permalink / raw)
To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel
Generic watchdog device model has been implemented as per ARM BSAv0.9
Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
hw/arm/Kconfig | 1 +
hw/watchdog/Kconfig | 4 +
hw/watchdog/meson.build | 1 +
hw/watchdog/wdt_sbsa_gwdt.c | 346 ++++++++++++++++++++++++++++
include/hw/watchdog/wdt_sbsa_gwdt.h | 70 ++++++
5 files changed, 422 insertions(+)
create mode 100644 hw/watchdog/wdt_sbsa_gwdt.c
create mode 100644 include/hw/watchdog/wdt_sbsa_gwdt.h
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index f303c6bead25..6b97e64595d3 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -210,6 +210,7 @@ config SBSA_REF
select PL031 # RTC
select PL061 # GPIO
select USB_EHCI_SYSBUS
+ select WDT_SBSA_GWDT
config SABRELITE
bool
diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
index 293209b291d6..ea9cadd66f22 100644
--- a/hw/watchdog/Kconfig
+++ b/hw/watchdog/Kconfig
@@ -17,3 +17,7 @@ config WDT_DIAG288
config WDT_IMX2
bool
+
+config WDT_SBSA_GWDT
+ bool
+ default y if SBSA_REF
diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
index 9b8725e64288..a9a23307acfe 100644
--- a/hw/watchdog/meson.build
+++ b/hw/watchdog/meson.build
@@ -5,3 +5,4 @@ softmmu_ss.add(when: 'CONFIG_WDT_IB700', if_true: files('wdt_ib700.c'))
softmmu_ss.add(when: 'CONFIG_WDT_DIAG288', if_true: files('wdt_diag288.c'))
softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('wdt_aspeed.c'))
softmmu_ss.add(when: 'CONFIG_WDT_IMX2', if_true: files('wdt_imx2.c'))
+softmmu_ss.add(when: 'CONFIG_WDT_SBSA_GWDT', if_true: files('wdt_sbsa_gwdt.c'))
diff --git a/hw/watchdog/wdt_sbsa_gwdt.c b/hw/watchdog/wdt_sbsa_gwdt.c
new file mode 100644
index 000000000000..1c2fe04db7c7
--- /dev/null
+++ b/hw/watchdog/wdt_sbsa_gwdt.c
@@ -0,0 +1,346 @@
+/*
+ * Generic watchdog device model for SBSA
+ *
+ * Copyright Linaro.org 2020
+ *
+ * Authors:
+ * Shashi Mallela <shashi.mallela@linaro.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version. See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/reset.h"
+#include "sysemu/watchdog.h"
+#include "hw/watchdog/wdt_sbsa_gwdt.h"
+#include "qemu/timer.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+
+static WatchdogTimerModel model = {
+ .wdt_name = TYPE_WDT_SBSA_GWDT,
+ .wdt_description = "sbsa_gwdt device for sbsa_ref platform",
+};
+
+static const VMStateDescription vmstate_sbsa_gwdt = {
+ .name = "vmstate_sbsa_gwdt",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_TIMER_PTR(timer, SBSA_GWDTState),
+ VMSTATE_BOOL(enabled, SBSA_GWDTState),
+ VMSTATE_BOOL(ws0, SBSA_GWDTState),
+ VMSTATE_BOOL(ws1, SBSA_GWDTState),
+ VMSTATE_UINT32(wrr, SBSA_GWDTState),
+ VMSTATE_UINT32(wcs, SBSA_GWDTState),
+ VMSTATE_UINT32(worl, SBSA_GWDTState),
+ VMSTATE_UINT32(woru, SBSA_GWDTState),
+ VMSTATE_UINT32(wcvl, SBSA_GWDTState),
+ VMSTATE_UINT32(wcvu, SBSA_GWDTState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static uint64_t sbsa_gwdt_rread(void *opaque, hwaddr addr, unsigned int size)
+{
+ uint32_t ret = 0;
+
+ if (addr == SBSA_GWDT_WRR) {
+ /* watch refresh read has no effect and returns 0 */
+ ret = 0;
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR, "bad address in refresh frame read :"
+ " 0x%x\n", (int)addr);
+ }
+ return ret;
+}
+
+static uint64_t sbsa_gwdt_read(void *opaque, hwaddr addr, unsigned int size)
+{
+ SBSA_GWDTState *s = SBSA_GWDT(opaque);
+ uint32_t ret = 0;
+
+ switch (addr) {
+ case SBSA_GWDT_WCS:
+ ret = s->wcs;
+ break;
+ case SBSA_GWDT_WOR:
+ ret = s->worl;
+ break;
+ case SBSA_GWDT_WORU:
+ ret = s->woru;
+ break;
+ case SBSA_GWDT_WCV:
+ ret = s->wcvl;
+ break;
+ case SBSA_GWDT_WCVU:
+ ret = s->wcvu;
+ break;
+ case SBSA_GWDT_W_IIDR:
+ ret = s->id;
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "bad address in control frame read :"
+ " 0x%x\n", (int)addr);
+ }
+ return ret;
+}
+
+static void sbsa_gwdt_update_timer(SBSA_GWDTState *s)
+{
+ uint64_t timeout = 0;
+
+ if (s->enabled) {
+ /*
+ * Extract the upper 16 bits from woru & 32 bits from worl
+ * registers to construct the 48 bit offset value
+ */
+ timeout = s->woru & SBSA_GWDT_WOR_MASK;
+ timeout <<= 32;
+ timeout |= s->worl;
+ timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, SBSA_TIMER_FREQ);
+ timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+ /* store the current timeout value into compare registers */
+ s->wcvu = timeout >> 32;
+ s->wcvl = timeout;
+
+ if (!s->ws0) {
+ timer_del(s->timer);
+ timer_mod(s->ptimer, timeout);
+ } else {
+ timer_del(s->ptimer);
+ timer_mod(s->timer, timeout);
+ }
+ } else {
+ timer_del(s->ptimer);
+ timer_del(s->timer);
+ }
+}
+
+static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
+ unsigned size) {
+ SBSA_GWDTState *s = SBSA_GWDT(opaque);
+
+ if (offset == SBSA_GWDT_WRR) {
+ s->wrr = data;
+ s->wcs &= ~SBSA_GWDT_WCS_WS0;
+ s->wcs &= ~SBSA_GWDT_WCS_WS1;
+ s->ws0 = false;
+ s->ws1 = false;
+ sbsa_gwdt_update_timer(s);
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR, "bad address in refresh frame write :"
+ " 0x%x\n", (int)offset);
+ }
+}
+
+static void sbsa_gwdt_write(void *opaque, hwaddr offset, uint64_t data,
+ unsigned size) {
+ SBSA_GWDTState *s = SBSA_GWDT(opaque);
+ bool enable;
+
+ switch (offset) {
+ case SBSA_GWDT_WCS:
+ enable = data & SBSA_GWDT_WCS_EN;
+ if (enable) {
+ s->wcs |= SBSA_GWDT_WCS_EN;
+ s->enabled = true;
+ } else {
+ s->wcs &= ~SBSA_GWDT_WCS_EN;
+ s->enabled = false;
+ }
+ s->ws0 = false;
+ s->ws1 = false;
+ s->wcs &= ~SBSA_GWDT_WCS_WS0;
+ s->wcs &= ~SBSA_GWDT_WCS_WS1;
+ sbsa_gwdt_update_timer(s);
+ break;
+
+ case SBSA_GWDT_WOR:
+ s->worl = data;
+ s->ws0 = false;
+ s->ws1 = false;
+ s->wcs &= ~SBSA_GWDT_WCS_WS0;
+ s->wcs &= ~SBSA_GWDT_WCS_WS1;
+ /*
+ * TODO:- setting woru to 0 and triggering update timer(below) is a
+ * temporary workaround to handle current linux driver which is
+ * based on earlier version of BSA specification.Once the linux
+ * driver is updated to BSA v0.9 will remove these next 2 lines.
+ */
+ s->woru = 0;
+ sbsa_gwdt_update_timer(s);
+ break;
+
+ case SBSA_GWDT_WORU:
+ s->woru = data;
+ s->ws0 = false;
+ s->ws1 = false;
+ s->wcs &= ~SBSA_GWDT_WCS_WS0;
+ s->wcs &= ~SBSA_GWDT_WCS_WS1;
+ sbsa_gwdt_update_timer(s);
+ break;
+
+ case SBSA_GWDT_WCV:
+ s->wcvl = data;
+ break;
+
+ case SBSA_GWDT_WCVU:
+ s->wcvu = data;
+ break;
+
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "bad address in control frame write :"
+ " 0x%x\n", (int)offset);
+ }
+ return;
+}
+
+static void wdt_sbsa_gwdt_reset(DeviceState *dev)
+{
+ SBSA_GWDTState *s = SBSA_GWDT(dev);
+
+ timer_del(s->ptimer);
+ timer_del(s->timer);
+
+ s->enabled = false;
+ s->ws0 = false;
+ s->ws1 = false;
+ s->wcs &= ~SBSA_GWDT_WCS_EN;
+ s->wcs &= ~SBSA_GWDT_WCS_WS0;
+ s->wcs &= ~SBSA_GWDT_WCS_WS1;
+ s->wcvl = 0;
+ s->wcvu = 0;
+ s->worl = 0;
+ s->woru = 0;
+ s->id = SBSA_GWDT_ID;
+}
+
+static void sbsa_gwdt_reset(void *opaque)
+{
+ DeviceState *sbsa_gwdt = opaque;
+
+ wdt_sbsa_gwdt_reset(sbsa_gwdt);
+}
+
+static void sbsa_gwdt_timer_sysinterrupt(void *opaque)
+{
+ SBSA_GWDTState *s = SBSA_GWDT(opaque);
+
+ s->wcs |= SBSA_GWDT_WCS_WS0;
+ s->ws0 = true;
+ qemu_set_irq(s->irq, 1);
+ sbsa_gwdt_update_timer(s);
+}
+
+static void sbsa_gwdt_timer_sysreset(void *dev)
+{
+ SBSA_GWDTState *s = SBSA_GWDT(dev);
+
+ s->wcs |= SBSA_GWDT_WCS_WS1;
+ s->ws1 = true;
+ qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n");
+ /*
+ * Reset the watchdog only if the guest gets notified about
+ * expiry. watchdog_perform_action() may temporarily relinquish
+ * the BQL; reset before triggering the action to avoid races with
+ * sbsa_gwdt instructions.
+ */
+ switch (get_watchdog_action()) {
+ case WATCHDOG_ACTION_DEBUG:
+ case WATCHDOG_ACTION_NONE:
+ case WATCHDOG_ACTION_PAUSE:
+ break;
+ default:
+ wdt_sbsa_gwdt_reset(dev);
+ }
+ watchdog_perform_action();
+}
+
+static const MemoryRegionOps sbsa_gwdt_rops = {
+ .read = sbsa_gwdt_rread,
+ .write = sbsa_gwdt_rwrite,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid.min_access_size = 4,
+ .valid.max_access_size = 4,
+ .valid.unaligned = false,
+};
+
+static const MemoryRegionOps sbsa_gwdt_ops = {
+ .read = sbsa_gwdt_read,
+ .write = sbsa_gwdt_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid.min_access_size = 4,
+ .valid.max_access_size = 4,
+ .valid.unaligned = false,
+};
+
+static void wdt_sbsa_gwdt_realize(DeviceState *dev, Error **errp)
+{
+ SBSA_GWDTState *s = SBSA_GWDT(dev);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+ memory_region_init_io(&s->rmmio, OBJECT(dev),
+ &sbsa_gwdt_rops, s,
+ "sbsa_gwdt.refresh",
+ SBSA_GWDT_RMMIO_SIZE);
+
+ memory_region_init_io(&s->cmmio, OBJECT(dev),
+ &sbsa_gwdt_ops, s,
+ "sbsa_gwdt.control",
+ SBSA_GWDT_CMMIO_SIZE);
+
+ sysbus_init_mmio(sbd, &s->rmmio);
+ sysbus_init_mmio(sbd, &s->cmmio);
+
+ sysbus_init_irq(sbd, &s->irq);
+
+ qemu_register_reset(sbsa_gwdt_reset, s);
+
+ s->ptimer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sbsa_gwdt_timer_sysinterrupt,
+ dev);
+ s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sbsa_gwdt_timer_sysreset,
+ dev);
+}
+
+static void wdt_sbsa_gwdt_unrealize(DeviceState *dev)
+{
+ SBSA_GWDTState *s = SBSA_GWDT(dev);
+
+ timer_del(s->ptimer);
+ timer_free(s->ptimer);
+
+ timer_del(s->timer);
+ timer_free(s->timer);
+}
+
+static void wdt_sbsa_gwdt_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = wdt_sbsa_gwdt_realize;
+ dc->unrealize = wdt_sbsa_gwdt_unrealize;
+ dc->reset = wdt_sbsa_gwdt_reset;
+ dc->hotpluggable = false;
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+ dc->vmsd = &vmstate_sbsa_gwdt;
+}
+
+static const TypeInfo wdt_sbsa_gwdt_info = {
+ .class_init = wdt_sbsa_gwdt_class_init,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .name = TYPE_WDT_SBSA_GWDT,
+ .instance_size = sizeof(SBSA_GWDTState),
+};
+
+static void wdt_sbsa_gwdt_register_types(void)
+{
+ watchdog_add_model(&model);
+ type_register_static(&wdt_sbsa_gwdt_info);
+}
+
+type_init(wdt_sbsa_gwdt_register_types)
diff --git a/include/hw/watchdog/wdt_sbsa_gwdt.h b/include/hw/watchdog/wdt_sbsa_gwdt.h
new file mode 100644
index 000000000000..70ba7abb3ace
--- /dev/null
+++ b/include/hw/watchdog/wdt_sbsa_gwdt.h
@@ -0,0 +1,70 @@
+#ifndef WDT_SBSA_GWDT_H
+#define WDT_SBSA_GWDT_H
+
+#include "qemu/bitops.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+
+#define TYPE_WDT_SBSA_GWDT "sbsa_gwdt"
+#define SBSA_GWDT(obj) \
+ OBJECT_CHECK(SBSA_GWDTState, (obj), TYPE_WDT_SBSA_GWDT)
+#define SBSA_GWDT_CLASS(klass) \
+ OBJECT_CLASS_CHECK(SBSA_GWDTClass, (klass), TYPE_WDT_SBSA_GWDT)
+#define SBSA_GWDT_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(SBSA_GWDTClass, (obj), TYPE_WDT_SBSA_GWDT)
+
+/* SBSA Generic Watchdog register definitions */
+/* refresh frame */
+#define SBSA_GWDT_WRR 0x000
+
+/* control frame */
+#define SBSA_GWDT_WCS 0x000
+#define SBSA_GWDT_WOR 0x008
+#define SBSA_GWDT_WORU 0x00C
+#define SBSA_GWDT_WCV 0x010
+#define SBSA_GWDT_WCVU 0x014
+
+/* Watchdog Interface Identification Register */
+#define SBSA_GWDT_W_IIDR 0xFCC
+
+/* Watchdog Control and Status Register Bits */
+#define SBSA_GWDT_WCS_EN BIT(0)
+#define SBSA_GWDT_WCS_WS0 BIT(1)
+#define SBSA_GWDT_WCS_WS1 BIT(2)
+
+#define SBSA_GWDT_WOR_MASK 0x0000FFFF
+
+/* Watchdog Interface Identification Register definition*/
+#define SBSA_GWDT_ID 0x1043B
+
+/* 2 Separate memory regions for each of refresh & control register frames */
+#define SBSA_GWDT_RMMIO_SIZE 0x1000
+#define SBSA_GWDT_CMMIO_SIZE 0x1000
+
+#define SBSA_TIMER_FREQ 62500000 /* Hz */
+
+typedef struct SBSA_GWDTState {
+ /* <private> */
+ SysBusDevice parent_obj;
+
+ /*< public >*/
+ MemoryRegion rmmio;
+ MemoryRegion cmmio;
+ qemu_irq irq;
+
+ QEMUTimer *ptimer, *timer;
+
+ uint32_t id;
+ uint32_t wrr;
+ uint32_t wcs;
+ uint32_t worl;
+ uint32_t woru;
+ uint32_t wcvl;
+ uint32_t wcvu;
+ bool enabled;
+ bool ws0, ws1;
+
+ /*< public >*/
+} SBSA_GWDTState;
+
+#endif /* WDT_SBSA_GWDT_H */
--
2.18.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
2020-10-13 15:16 [PATCH v5 0/2] Add watchdog support for SbsaQemu Shashi Mallela
2020-10-13 15:16 ` [PATCH v5 1/2] hw/watchdog: Implement SBSA watchdog device Shashi Mallela
@ 2020-10-13 15:16 ` Shashi Mallela
2020-10-14 9:31 ` Graeme Gregory
1 sibling, 1 reply; 8+ messages in thread
From: Shashi Mallela @ 2020-10-13 15:16 UTC (permalink / raw)
To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel
Included the newly implemented SBSA generic watchdog device model into
SBSA platform
Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
hw/arm/sbsa-ref.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 9c3a893bedfd..97ed41607119 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -30,6 +30,7 @@
#include "exec/hwaddr.h"
#include "kvm_arm.h"
#include "hw/arm/boot.h"
+#include "hw/arm/fdt.h"
#include "hw/block/flash.h"
#include "hw/boards.h"
#include "hw/ide/internal.h"
@@ -40,6 +41,7 @@
#include "hw/qdev-properties.h"
#include "hw/usb.h"
#include "hw/char/pl011.h"
+#include "hw/watchdog/wdt_sbsa_gwdt.h"
#include "net/net.h"
#include "qom/object.h"
@@ -64,6 +66,9 @@ enum {
SBSA_GIC_DIST,
SBSA_GIC_REDIST,
SBSA_SECURE_EC,
+ SBSA_GWDT,
+ SBSA_GWDT_REFRESH,
+ SBSA_GWDT_CONTROL,
SBSA_SMMU,
SBSA_UART,
SBSA_RTC,
@@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
[SBSA_GIC_DIST] = { 0x40060000, 0x00010000 },
[SBSA_GIC_REDIST] = { 0x40080000, 0x04000000 },
[SBSA_SECURE_EC] = { 0x50000000, 0x00001000 },
+ [SBSA_GWDT_REFRESH] = { 0x50010000, 0x00001000 },
+ [SBSA_GWDT_CONTROL] = { 0x50011000, 0x00001000 },
[SBSA_UART] = { 0x60000000, 0x00001000 },
[SBSA_RTC] = { 0x60010000, 0x00001000 },
[SBSA_GPIO] = { 0x60020000, 0x00001000 },
@@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
[SBSA_SECURE_UART_MM] = 9,
[SBSA_AHCI] = 10,
[SBSA_EHCI] = 11,
+ [SBSA_SMMU] = 12, /* ... to 15 */
+ [SBSA_GWDT] = 16,
};
static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
@@ -141,6 +150,30 @@ static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
return arm_cpu_mp_affinity(idx, clustersz);
}
+static void create_wdt_fdt(SBSAMachineState *sms)
+{
+ char *nodename;
+ const char compat[] = "arm,sbsa-gwdt";
+
+ hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
+ hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
+ int irq = sbsa_ref_irqmap[SBSA_GWDT];
+
+ nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
+ qemu_fdt_add_subnode(sms->fdt, nodename);
+
+ qemu_fdt_setprop(sms->fdt, nodename, "compatible",
+ compat, sizeof(compat));
+ qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
+ 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
+ 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
+ qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
+ GIC_FDT_IRQ_TYPE_PPI, irq,
+ GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+ qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
+ g_free(nodename);
+}
+
/*
* Firmware on this machine only uses ACPI table to load OS, these limited
* device tree nodes are just to let firmware know the info which varies from
@@ -219,6 +252,7 @@ static void create_fdt(SBSAMachineState *sms)
g_free(nodename);
}
+ create_wdt_fdt(sms);
}
#define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
@@ -447,6 +481,20 @@ static void create_rtc(const SBSAMachineState *sms)
sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq));
}
+static void create_wdt(const SBSAMachineState *sms)
+{
+ hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
+ hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
+ DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
+ SysBusDevice *s = SYS_BUS_DEVICE(dev);
+ int irq = sbsa_ref_irqmap[SBSA_GWDT];
+
+ 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(sms->gic, irq));
+}
+
static DeviceState *gpio_key_dev;
static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
{
@@ -730,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
create_rtc(sms);
+ create_wdt(sms);
+
create_gpio(sms);
create_ahci(sms);
--
2.18.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
2020-10-13 15:16 ` [PATCH v5 2/2] hw/arm/sbsa-ref: add " Shashi Mallela
@ 2020-10-14 9:31 ` Graeme Gregory
2020-10-14 17:04 ` Shashi Mallela
0 siblings, 1 reply; 8+ messages in thread
From: Graeme Gregory @ 2020-10-14 9:31 UTC (permalink / raw)
To: Shashi Mallela; +Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad
On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> Included the newly implemented SBSA generic watchdog device model into
> SBSA platform
>
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> ---
> hw/arm/sbsa-ref.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 9c3a893bedfd..97ed41607119 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -30,6 +30,7 @@
> #include "exec/hwaddr.h"
> #include "kvm_arm.h"
> #include "hw/arm/boot.h"
> +#include "hw/arm/fdt.h"
> #include "hw/block/flash.h"
> #include "hw/boards.h"
> #include "hw/ide/internal.h"
> @@ -40,6 +41,7 @@
> #include "hw/qdev-properties.h"
> #include "hw/usb.h"
> #include "hw/char/pl011.h"
> +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> #include "net/net.h"
> #include "qom/object.h"
>
> @@ -64,6 +66,9 @@ enum {
> SBSA_GIC_DIST,
> SBSA_GIC_REDIST,
> SBSA_SECURE_EC,
> + SBSA_GWDT,
> + SBSA_GWDT_REFRESH,
> + SBSA_GWDT_CONTROL,
> SBSA_SMMU,
> SBSA_UART,
> SBSA_RTC,
> @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> [SBSA_GIC_DIST] = { 0x40060000, 0x00010000 },
> [SBSA_GIC_REDIST] = { 0x40080000, 0x04000000 },
> [SBSA_SECURE_EC] = { 0x50000000, 0x00001000 },
> + [SBSA_GWDT_REFRESH] = { 0x50010000, 0x00001000 },
> + [SBSA_GWDT_CONTROL] = { 0x50011000, 0x00001000 },
> [SBSA_UART] = { 0x60000000, 0x00001000 },
> [SBSA_RTC] = { 0x60010000, 0x00001000 },
> [SBSA_GPIO] = { 0x60020000, 0x00001000 },
> @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> [SBSA_SECURE_UART_MM] = 9,
> [SBSA_AHCI] = 10,
> [SBSA_EHCI] = 11,
> + [SBSA_SMMU] = 12, /* ... to 15 */
> + [SBSA_GWDT] = 16,
> };
>
> static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> @@ -141,6 +150,30 @@ static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> return arm_cpu_mp_affinity(idx, clustersz);
> }
>
> +static void create_wdt_fdt(SBSAMachineState *sms)
> +{
> + char *nodename;
> + const char compat[] = "arm,sbsa-gwdt";
> +
> + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> +
> + nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
> + qemu_fdt_add_subnode(sms->fdt, nodename);
> +
> + qemu_fdt_setprop(sms->fdt, nodename, "compatible",
> + compat, sizeof(compat));
> + qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
> + 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
> + 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
> + qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
> + GIC_FDT_IRQ_TYPE_PPI, irq,
> + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> + qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
> + g_free(nodename);
> +}
> +
Is this actually used anywhere? I ask because SBSA-ref is not a FDT
booting machine and only uses FDT to transfer some dynamic info to
arm-tf/edk2 and is not a full description tree. Your ACPI patch in
edk2 certainly does not use this info.
Graeme
> /*
> * Firmware on this machine only uses ACPI table to load OS, these limited
> * device tree nodes are just to let firmware know the info which varies from
> @@ -219,6 +252,7 @@ static void create_fdt(SBSAMachineState *sms)
>
> g_free(nodename);
> }
> + create_wdt_fdt(sms);
> }
>
> #define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
> @@ -447,6 +481,20 @@ static void create_rtc(const SBSAMachineState *sms)
> sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq));
> }
>
> +static void create_wdt(const SBSAMachineState *sms)
> +{
> + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> + DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> +
> + 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(sms->gic, irq));
> +}
> +
> static DeviceState *gpio_key_dev;
> static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> {
> @@ -730,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
>
> create_rtc(sms);
>
> + create_wdt(sms);
> +
> create_gpio(sms);
>
> create_ahci(sms);
> --
> 2.18.4
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
2020-10-14 9:31 ` Graeme Gregory
@ 2020-10-14 17:04 ` Shashi Mallela
2020-10-15 14:10 ` Graeme Gregory
0 siblings, 1 reply; 8+ messages in thread
From: Shashi Mallela @ 2020-10-14 17:04 UTC (permalink / raw)
To: Graeme Gregory
Cc: Peter Maydell, Leif Lindholm, qemu-devel, qemu-arm,
Radosław Biernacki
[-- Attachment #1: Type: text/plain, Size: 5357 bytes --]
This was added as a placeholder for the virt requirement suggested by Maxim
earlier.Agreed that this fdt otherwise has no significance for sbsa-ref
platform nor is being used by ACPI table created for wdt.
-Shashi
On Wed, 14 Oct 2020 at 05:31, Graeme Gregory <graeme@nuviainc.com> wrote:
> On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> > Included the newly implemented SBSA generic watchdog device model into
> > SBSA platform
> >
> > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > ---
> > hw/arm/sbsa-ref.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index 9c3a893bedfd..97ed41607119 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -30,6 +30,7 @@
> > #include "exec/hwaddr.h"
> > #include "kvm_arm.h"
> > #include "hw/arm/boot.h"
> > +#include "hw/arm/fdt.h"
> > #include "hw/block/flash.h"
> > #include "hw/boards.h"
> > #include "hw/ide/internal.h"
> > @@ -40,6 +41,7 @@
> > #include "hw/qdev-properties.h"
> > #include "hw/usb.h"
> > #include "hw/char/pl011.h"
> > +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> > #include "net/net.h"
> > #include "qom/object.h"
> >
> > @@ -64,6 +66,9 @@ enum {
> > SBSA_GIC_DIST,
> > SBSA_GIC_REDIST,
> > SBSA_SECURE_EC,
> > + SBSA_GWDT,
> > + SBSA_GWDT_REFRESH,
> > + SBSA_GWDT_CONTROL,
> > SBSA_SMMU,
> > SBSA_UART,
> > SBSA_RTC,
> > @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > [SBSA_GIC_DIST] = { 0x40060000, 0x00010000 },
> > [SBSA_GIC_REDIST] = { 0x40080000, 0x04000000 },
> > [SBSA_SECURE_EC] = { 0x50000000, 0x00001000 },
> > + [SBSA_GWDT_REFRESH] = { 0x50010000, 0x00001000 },
> > + [SBSA_GWDT_CONTROL] = { 0x50011000, 0x00001000 },
> > [SBSA_UART] = { 0x60000000, 0x00001000 },
> > [SBSA_RTC] = { 0x60010000, 0x00001000 },
> > [SBSA_GPIO] = { 0x60020000, 0x00001000 },
> > @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> > [SBSA_SECURE_UART_MM] = 9,
> > [SBSA_AHCI] = 10,
> > [SBSA_EHCI] = 11,
> > + [SBSA_SMMU] = 12, /* ... to 15 */
> > + [SBSA_GWDT] = 16,
> > };
> >
> > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> > @@ -141,6 +150,30 @@ static uint64_t
> sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> > return arm_cpu_mp_affinity(idx, clustersz);
> > }
> >
> > +static void create_wdt_fdt(SBSAMachineState *sms)
> > +{
> > + char *nodename;
> > + const char compat[] = "arm,sbsa-gwdt";
> > +
> > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > +
> > + nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
> > + qemu_fdt_add_subnode(sms->fdt, nodename);
> > +
> > + qemu_fdt_setprop(sms->fdt, nodename, "compatible",
> > + compat, sizeof(compat));
> > + qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
> > + 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
> > + 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
> > + qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
> > + GIC_FDT_IRQ_TYPE_PPI, irq,
> > + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> > + qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
> > + g_free(nodename);
> > +}
> > +
>
> Is this actually used anywhere? I ask because SBSA-ref is not a FDT
> booting machine and only uses FDT to transfer some dynamic info to
> arm-tf/edk2 and is not a full description tree. Your ACPI patch in
> edk2 certainly does not use this info.
>
> Graeme
>
>
> > /*
> > * Firmware on this machine only uses ACPI table to load OS, these
> limited
> > * device tree nodes are just to let firmware know the info which
> varies from
> > @@ -219,6 +252,7 @@ static void create_fdt(SBSAMachineState *sms)
> >
> > g_free(nodename);
> > }
> > + create_wdt_fdt(sms);
> > }
> >
> > #define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
> > @@ -447,6 +481,20 @@ static void create_rtc(const SBSAMachineState *sms)
> > sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic,
> irq));
> > }
> >
> > +static void create_wdt(const SBSAMachineState *sms)
> > +{
> > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > +
> > + 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(sms->gic, irq));
> > +}
> > +
> > static DeviceState *gpio_key_dev;
> > static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> > {
> > @@ -730,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
> >
> > create_rtc(sms);
> >
> > + create_wdt(sms);
> > +
> > create_gpio(sms);
> >
> > create_ahci(sms);
> > --
> > 2.18.4
> >
> >
>
[-- Attachment #2: Type: text/html, Size: 7087 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
2020-10-14 17:04 ` Shashi Mallela
@ 2020-10-15 14:10 ` Graeme Gregory
2020-10-15 15:21 ` Maxim Uvarov
0 siblings, 1 reply; 8+ messages in thread
From: Graeme Gregory @ 2020-10-15 14:10 UTC (permalink / raw)
To: Shashi Mallela
Cc: Peter Maydell, Leif Lindholm, qemu-devel, qemu-arm,
Radosław Biernacki
On Wed, Oct 14, 2020 at 01:04:43PM -0400, Shashi Mallela wrote:
> This was added as a placeholder for the virt requirement suggested by Maxim
> earlier.Agreed that this fdt otherwise has no significance for sbsa-ref
> platform nor is being used by ACPI table created for wdt.
>
> -Shashi
>
> On Wed, 14 Oct 2020 at 05:31, Graeme Gregory <[1]graeme@nuviainc.com> wrote:
>
> On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> > Included the newly implemented SBSA generic watchdog device model into
> > SBSA platform
> >
> > Signed-off-by: Shashi Mallela <[2]shashi.mallela@linaro.org>
> > ---
> > hw/arm/sbsa-ref.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index 9c3a893bedfd..97ed41607119 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -30,6 +30,7 @@
> > #include "exec/hwaddr.h"
> > #include "kvm_arm.h"
> > #include "hw/arm/boot.h"
> > +#include "hw/arm/fdt.h"
> > #include "hw/block/flash.h"
> > #include "hw/boards.h"
> > #include "hw/ide/internal.h"
> > @@ -40,6 +41,7 @@
> > #include "hw/qdev-properties.h"
> > #include "hw/usb.h"
> > #include "hw/char/pl011.h"
> > +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> > #include "net/net.h"
> > #include "qom/object.h"
> >
> > @@ -64,6 +66,9 @@ enum {
> > SBSA_GIC_DIST,
> > SBSA_GIC_REDIST,
> > SBSA_SECURE_EC,
> > + SBSA_GWDT,
> > + SBSA_GWDT_REFRESH,
> > + SBSA_GWDT_CONTROL,
> > SBSA_SMMU,
> > SBSA_UART,
> > SBSA_RTC,
> > @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > [SBSA_GIC_DIST] = { 0x40060000, 0x00010000 },
> > [SBSA_GIC_REDIST] = { 0x40080000, 0x04000000 },
> > [SBSA_SECURE_EC] = { 0x50000000, 0x00001000 },
> > + [SBSA_GWDT_REFRESH] = { 0x50010000, 0x00001000 },
> > + [SBSA_GWDT_CONTROL] = { 0x50011000, 0x00001000 },
> > [SBSA_UART] = { 0x60000000, 0x00001000 },
> > [SBSA_RTC] = { 0x60010000, 0x00001000 },
> > [SBSA_GPIO] = { 0x60020000, 0x00001000 },
> > @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> > [SBSA_SECURE_UART_MM] = 9,
> > [SBSA_AHCI] = 10,
> > [SBSA_EHCI] = 11,
> > + [SBSA_SMMU] = 12, /* ... to 15 */
> > + [SBSA_GWDT] = 16,
> > };
I guess your patch was not based on master here? You should make sure
you are rebased to the latest version before sending.
> >
> > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> > @@ -141,6 +150,30 @@ static uint64_t sbsa_ref_cpu_mp_affinity
> (SBSAMachineState *sms, int idx)
> > return arm_cpu_mp_affinity(idx, clustersz);
> > }
> >
> > +static void create_wdt_fdt(SBSAMachineState *sms)
> > +{
> > + char *nodename;
> > + const char compat[] = "arm,sbsa-gwdt";
> > +
> > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > +
> > + nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
> > + qemu_fdt_add_subnode(sms->fdt, nodename);
> > +
> > + qemu_fdt_setprop(sms->fdt, nodename, "compatible",
> > + compat, sizeof(compat));
> > + qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
> > + 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
> > + 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
> > + qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
> > + GIC_FDT_IRQ_TYPE_PPI, irq,
> > + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> > + qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
> > + g_free(nodename);
> > +}
> > +
>
> Is this actually used anywhere? I ask because SBSA-ref is not a FDT
> booting machine and only uses FDT to transfer some dynamic info to
> arm-tf/edk2 and is not a full description tree. Your ACPI patch in
> edk2 certainly does not use this info.
>
From your reply above I would propose this hunk and the call below are
removed. If its needed to virt thats a seperate discussion which I think
was already happening in another thread.
FOSS style is for inline replies not top posting FYI
Graeme
> Graeme
>
>
> > /*
> > * Firmware on this machine only uses ACPI table to load OS, these
> limited
> > * device tree nodes are just to let firmware know the info which varies
> from
> > @@ -219,6 +252,7 @@ static void create_fdt(SBSAMachineState *sms)
> >
> > g_free(nodename);
> > }
> > + create_wdt_fdt(sms);
> > }
> >
> > #define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
> > @@ -447,6 +481,20 @@ static void create_rtc(const SBSAMachineState *sms)
> > sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic,
> irq));
> > }
> >
> > +static void create_wdt(const SBSAMachineState *sms)
> > +{
> > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > +
> > + 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(sms->gic, irq));
> > +}
> > +
> > static DeviceState *gpio_key_dev;
> > static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> > {
> > @@ -730,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
> >
> > create_rtc(sms);
> >
> > + create_wdt(sms);
> > +
> > create_gpio(sms);
> >
> > create_ahci(sms);
> > --
> > 2.18.4
> >
> >
>
>
> References:
>
> [1] mailto:graeme@nuviainc.com
> [2] mailto:shashi.mallela@linaro.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
2020-10-15 14:10 ` Graeme Gregory
@ 2020-10-15 15:21 ` Maxim Uvarov
2020-10-16 8:37 ` Graeme Gregory
0 siblings, 1 reply; 8+ messages in thread
From: Maxim Uvarov @ 2020-10-15 15:21 UTC (permalink / raw)
To: Graeme Gregory
Cc: Peter Maydell, Shashi Mallela, Radosław Biernacki,
QEMU Developers, qemu-arm, Leif Lindholm
On Thu, 15 Oct 2020 at 17:12, Graeme Gregory <graeme@nuviainc.com> wrote:
>
> On Wed, Oct 14, 2020 at 01:04:43PM -0400, Shashi Mallela wrote:
> > This was added as a placeholder for the virt requirement suggested by Maxim
> > earlier.Agreed that this fdt otherwise has no significance for sbsa-ref
> > platform nor is being used by ACPI table created for wdt.
> >
> > -Shashi
> >
> > On Wed, 14 Oct 2020 at 05:31, Graeme Gregory <[1]graeme@nuviainc.com> wrote:
> >
> > On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> > > Included the newly implemented SBSA generic watchdog device model into
> > > SBSA platform
> > >
> > > Signed-off-by: Shashi Mallela <[2]shashi.mallela@linaro.org>
> > > ---
> > > hw/arm/sbsa-ref.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 50 insertions(+)
> > >
> > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > > index 9c3a893bedfd..97ed41607119 100644
> > > --- a/hw/arm/sbsa-ref.c
> > > +++ b/hw/arm/sbsa-ref.c
> > > @@ -30,6 +30,7 @@
> > > #include "exec/hwaddr.h"
> > > #include "kvm_arm.h"
> > > #include "hw/arm/boot.h"
> > > +#include "hw/arm/fdt.h"
> > > #include "hw/block/flash.h"
> > > #include "hw/boards.h"
> > > #include "hw/ide/internal.h"
> > > @@ -40,6 +41,7 @@
> > > #include "hw/qdev-properties.h"
> > > #include "hw/usb.h"
> > > #include "hw/char/pl011.h"
> > > +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> > > #include "net/net.h"
> > > #include "qom/object.h"
> > >
> > > @@ -64,6 +66,9 @@ enum {
> > > SBSA_GIC_DIST,
> > > SBSA_GIC_REDIST,
> > > SBSA_SECURE_EC,
> > > + SBSA_GWDT,
> > > + SBSA_GWDT_REFRESH,
> > > + SBSA_GWDT_CONTROL,
> > > SBSA_SMMU,
> > > SBSA_UART,
> > > SBSA_RTC,
> > > @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > > [SBSA_GIC_DIST] = { 0x40060000, 0x00010000 },
> > > [SBSA_GIC_REDIST] = { 0x40080000, 0x04000000 },
> > > [SBSA_SECURE_EC] = { 0x50000000, 0x00001000 },
> > > + [SBSA_GWDT_REFRESH] = { 0x50010000, 0x00001000 },
> > > + [SBSA_GWDT_CONTROL] = { 0x50011000, 0x00001000 },
> > > [SBSA_UART] = { 0x60000000, 0x00001000 },
> > > [SBSA_RTC] = { 0x60010000, 0x00001000 },
> > > [SBSA_GPIO] = { 0x60020000, 0x00001000 },
> > > @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> > > [SBSA_SECURE_UART_MM] = 9,
> > > [SBSA_AHCI] = 10,
> > > [SBSA_EHCI] = 11,
> > > + [SBSA_SMMU] = 12, /* ... to 15 */
> > > + [SBSA_GWDT] = 16,
> > > };
>
> I guess your patch was not based on master here? You should make sure
> you are rebased to the latest version before sending.
>
> > >
> > > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> > > @@ -141,6 +150,30 @@ static uint64_t sbsa_ref_cpu_mp_affinity
> > (SBSAMachineState *sms, int idx)
> > > return arm_cpu_mp_affinity(idx, clustersz);
> > > }
> > >
> > > +static void create_wdt_fdt(SBSAMachineState *sms)
> > > +{
> > > + char *nodename;
> > > + const char compat[] = "arm,sbsa-gwdt";
> > > +
> > > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > > +
> > > + nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
> > > + qemu_fdt_add_subnode(sms->fdt, nodename);
> > > +
> > > + qemu_fdt_setprop(sms->fdt, nodename, "compatible",
> > > + compat, sizeof(compat));
> > > + qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
> > > + 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
> > > + 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
> > > + qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
> > > + GIC_FDT_IRQ_TYPE_PPI, irq,
> > > + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> > > + qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
> > > + g_free(nodename);
> > > +}
> > > +
> >
> > Is this actually used anywhere? I ask because SBSA-ref is not a FDT
> > booting machine and only uses FDT to transfer some dynamic info to
> > arm-tf/edk2 and is not a full description tree. Your ACPI patch in
> > edk2 certainly does not use this info.
> >
>
Greame, if sbsa-ref does not support non edk2 boot, i.e.
arm-tf/optee/uboot-uefi, then it's better to remove here and we can
add later for the virt machine.
> From your reply above I would propose this hunk and the call below are
> removed. If its needed to virt thats a seperate discussion which I think
> was already happening in another thread.
>
Yep, agree.
Maxim.
> FOSS style is for inline replies not top posting FYI
>
> Graeme
>
> > Graeme
> >
> >
> > > /*
> > > * Firmware on this machine only uses ACPI table to load OS, these
> > limited
> > > * device tree nodes are just to let firmware know the info which varies
> > from
> > > @@ -219,6 +252,7 @@ static void create_fdt(SBSAMachineState *sms)
> > >
> > > g_free(nodename);
> > > }
> > > + create_wdt_fdt(sms);
> > > }
> > >
> > > #define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
> > > @@ -447,6 +481,20 @@ static void create_rtc(const SBSAMachineState *sms)
> > > sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic,
> > irq));
> > > }
> > >
> > > +static void create_wdt(const SBSAMachineState *sms)
> > > +{
> > > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> > > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > > +
> > > + 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(sms->gic, irq));
> > > +}
> > > +
> > > static DeviceState *gpio_key_dev;
> > > static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> > > {
> > > @@ -730,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
> > >
> > > create_rtc(sms);
> > >
> > > + create_wdt(sms);
> > > +
> > > create_gpio(sms);
> > >
> > > create_ahci(sms);
> > > --
> > > 2.18.4
> > >
> > >
> >
> >
> > References:
> >
> > [1] mailto:graeme@nuviainc.com
> > [2] mailto:shashi.mallela@linaro.org
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
2020-10-15 15:21 ` Maxim Uvarov
@ 2020-10-16 8:37 ` Graeme Gregory
0 siblings, 0 replies; 8+ messages in thread
From: Graeme Gregory @ 2020-10-16 8:37 UTC (permalink / raw)
To: Maxim Uvarov
Cc: Peter Maydell, Shashi Mallela, Radosław Biernacki,
QEMU Developers, qemu-arm, Leif Lindholm
On Thu, Oct 15, 2020 at 06:21:09PM +0300, Maxim Uvarov wrote:
> On Thu, 15 Oct 2020 at 17:12, Graeme Gregory <graeme@nuviainc.com> wrote:
> >
> > On Wed, Oct 14, 2020 at 01:04:43PM -0400, Shashi Mallela wrote:
> > > This was added as a placeholder for the virt requirement suggested by Maxim
> > > earlier.Agreed that this fdt otherwise has no significance for sbsa-ref
> > > platform nor is being used by ACPI table created for wdt.
> > >
> > > -Shashi
> > >
> > > On Wed, 14 Oct 2020 at 05:31, Graeme Gregory <[1]graeme@nuviainc.com> wrote:
> > >
> > > On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> > > > Included the newly implemented SBSA generic watchdog device model into
> > > > SBSA platform
> > > >
> > > > Signed-off-by: Shashi Mallela <[2]shashi.mallela@linaro.org>
> > > > ---
> > > > hw/arm/sbsa-ref.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 50 insertions(+)
> > > >
> > > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > > > index 9c3a893bedfd..97ed41607119 100644
> > > > --- a/hw/arm/sbsa-ref.c
> > > > +++ b/hw/arm/sbsa-ref.c
> > > > @@ -30,6 +30,7 @@
> > > > #include "exec/hwaddr.h"
> > > > #include "kvm_arm.h"
> > > > #include "hw/arm/boot.h"
> > > > +#include "hw/arm/fdt.h"
> > > > #include "hw/block/flash.h"
> > > > #include "hw/boards.h"
> > > > #include "hw/ide/internal.h"
> > > > @@ -40,6 +41,7 @@
> > > > #include "hw/qdev-properties.h"
> > > > #include "hw/usb.h"
> > > > #include "hw/char/pl011.h"
> > > > +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> > > > #include "net/net.h"
> > > > #include "qom/object.h"
> > > >
> > > > @@ -64,6 +66,9 @@ enum {
> > > > SBSA_GIC_DIST,
> > > > SBSA_GIC_REDIST,
> > > > SBSA_SECURE_EC,
> > > > + SBSA_GWDT,
> > > > + SBSA_GWDT_REFRESH,
> > > > + SBSA_GWDT_CONTROL,
> > > > SBSA_SMMU,
> > > > SBSA_UART,
> > > > SBSA_RTC,
> > > > @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > > > [SBSA_GIC_DIST] = { 0x40060000, 0x00010000 },
> > > > [SBSA_GIC_REDIST] = { 0x40080000, 0x04000000 },
> > > > [SBSA_SECURE_EC] = { 0x50000000, 0x00001000 },
> > > > + [SBSA_GWDT_REFRESH] = { 0x50010000, 0x00001000 },
> > > > + [SBSA_GWDT_CONTROL] = { 0x50011000, 0x00001000 },
> > > > [SBSA_UART] = { 0x60000000, 0x00001000 },
> > > > [SBSA_RTC] = { 0x60010000, 0x00001000 },
> > > > [SBSA_GPIO] = { 0x60020000, 0x00001000 },
> > > > @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> > > > [SBSA_SECURE_UART_MM] = 9,
> > > > [SBSA_AHCI] = 10,
> > > > [SBSA_EHCI] = 11,
> > > > + [SBSA_SMMU] = 12, /* ... to 15 */
> > > > + [SBSA_GWDT] = 16,
> > > > };
> >
> > I guess your patch was not based on master here? You should make sure
> > you are rebased to the latest version before sending.
> >
> > > >
> > > > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> > > > @@ -141,6 +150,30 @@ static uint64_t sbsa_ref_cpu_mp_affinity
> > > (SBSAMachineState *sms, int idx)
> > > > return arm_cpu_mp_affinity(idx, clustersz);
> > > > }
> > > >
> > > > +static void create_wdt_fdt(SBSAMachineState *sms)
> > > > +{
> > > > + char *nodename;
> > > > + const char compat[] = "arm,sbsa-gwdt";
> > > > +
> > > > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > > > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > > > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > > > +
> > > > + nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
> > > > + qemu_fdt_add_subnode(sms->fdt, nodename);
> > > > +
> > > > + qemu_fdt_setprop(sms->fdt, nodename, "compatible",
> > > > + compat, sizeof(compat));
> > > > + qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
> > > > + 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
> > > > + 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
> > > > + qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
> > > > + GIC_FDT_IRQ_TYPE_PPI, irq,
> > > > + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> > > > + qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
> > > > + g_free(nodename);
> > > > +}
> > > > +
> > >
> > > Is this actually used anywhere? I ask because SBSA-ref is not a FDT
> > > booting machine and only uses FDT to transfer some dynamic info to
> > > arm-tf/edk2 and is not a full description tree. Your ACPI patch in
> > > edk2 certainly does not use this info.
> > >
> >
>
> Greame, if sbsa-ref does not support non edk2 boot, i.e.
> arm-tf/optee/uboot-uefi, then it's better to remove here and we can
> add later for the virt machine.
>
It is not that it does not support it, it is as real hardware you will
need to build the hardware knowledge into your firmware.
Graeme
> > From your reply above I would propose this hunk and the call below are
> > removed. If its needed to virt thats a seperate discussion which I think
> > was already happening in another thread.
> >
> Yep, agree.
>
> Maxim.
>
> > FOSS style is for inline replies not top posting FYI
> >
> > Graeme
> >
> > > Graeme
> > >
> > >
> > > > /*
> > > > * Firmware on this machine only uses ACPI table to load OS, these
> > > limited
> > > > * device tree nodes are just to let firmware know the info which varies
> > > from
> > > > @@ -219,6 +252,7 @@ static void create_fdt(SBSAMachineState *sms)
> > > >
> > > > g_free(nodename);
> > > > }
> > > > + create_wdt_fdt(sms);
> > > > }
> > > >
> > > > #define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
> > > > @@ -447,6 +481,20 @@ static void create_rtc(const SBSAMachineState *sms)
> > > > sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic,
> > > irq));
> > > > }
> > > >
> > > > +static void create_wdt(const SBSAMachineState *sms)
> > > > +{
> > > > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > > > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > > > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> > > > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > > > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > > > +
> > > > + 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(sms->gic, irq));
> > > > +}
> > > > +
> > > > static DeviceState *gpio_key_dev;
> > > > static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> > > > {
> > > > @@ -730,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
> > > >
> > > > create_rtc(sms);
> > > >
> > > > + create_wdt(sms);
> > > > +
> > > > create_gpio(sms);
> > > >
> > > > create_ahci(sms);
> > > > --
> > > > 2.18.4
> > > >
> > > >
> > >
> > >
> > > References:
> > >
> > > [1] mailto:graeme@nuviainc.com
> > > [2] mailto:shashi.mallela@linaro.org
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-16 8:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13 15:16 [PATCH v5 0/2] Add watchdog support for SbsaQemu Shashi Mallela
2020-10-13 15:16 ` [PATCH v5 1/2] hw/watchdog: Implement SBSA watchdog device Shashi Mallela
2020-10-13 15:16 ` [PATCH v5 2/2] hw/arm/sbsa-ref: add " Shashi Mallela
2020-10-14 9:31 ` Graeme Gregory
2020-10-14 17:04 ` Shashi Mallela
2020-10-15 14:10 ` Graeme Gregory
2020-10-15 15:21 ` Maxim Uvarov
2020-10-16 8:37 ` Graeme Gregory
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.