Add enough code to emulate i.MX2 watchdog IP block so it would be
possible to reboot the machine running Linux Guest.
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Cc: yurovsky@gmail.com
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
hw/misc/Makefile.objs | 1 +
hw/misc/imx2_wdt.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
include/hw/misc/imx2_wdt.h | 36 ++++++++++++++
3 files changed, 154 insertions(+)
create mode 100644 hw/misc/imx2_wdt.c
create mode 100644 include/hw/misc/imx2_wdt.h
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index ac1be05a03..c393a93456 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -35,6 +35,7 @@ obj-$(CONFIG_IMX) += imx25_ccm.o
obj-$(CONFIG_IMX) += imx6_ccm.o
obj-$(CONFIG_IMX) += imx6_src.o
obj-$(CONFIG_IMX) += imx7_ccm.o
+obj-$(CONFIG_IMX) += imx2_wdt.o
obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
obj-$(CONFIG_MAINSTONE) += mst_fpga.o
diff --git a/hw/misc/imx2_wdt.c b/hw/misc/imx2_wdt.c
new file mode 100644
index 0000000000..9d97a19511
--- /dev/null
+++ b/hw/misc/imx2_wdt.c
@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) 2017, Impinj, Inc.
+ *
+ * i.MX2 Watchdog IP block
+ *
+ * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/hw.h"
+#include "exec/memory.h"
+#include "exec/address-spaces.h"
+#include "hw/sysbus.h"
+#include "qemu/log.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/watchdog.h"
+#include "qemu/error-report.h"
+#include "qemu/sizes.h"
+
+#include "hw/misc/imx2_wdt.h"
+
+
+#define IMX2_WDT_WCR_WDA BIT(5) /* -> External Reset WDOG_B */
+#define IMX2_WDT_WCR_SRS BIT(4) /* -> Software Reset Signal */
+
+static uint64_t imx2_wdt_read(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ IMX2WdtState *s = opaque;
+ const size_t index = addr / sizeof(s->reg[0]);
+
+ if (index < ARRAY_SIZE(s->reg))
+ return s->reg[index];
+ else
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+
+ return 0xDEADBEEF;
+}
+
+static void imx2_wdt_write(void *opaque, hwaddr addr,
+ uint64_t val64, unsigned int size)
+{
+ uint16_t value = val64;
+ IMX2WdtState *s = opaque;
+ const size_t index = addr / sizeof(s->reg[0]);
+
+ switch (index) {
+ case IMX2_WDT_WCR:
+ if (value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS))
+ watchdog_perform_action();
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+ }
+}
+
+static const MemoryRegionOps imx2_wdt_ops = {
+ .read = imx2_wdt_read,
+ .write = imx2_wdt_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .valid = {
+ /*
+ * Our device would not work correctly if the guest was doing
+ * unaligned access. This might not be a limitation on the real
+ * device but in practice there is no reason for a guest to access
+ * this device unaligned.
+ */
+ .min_access_size = 4,
+ .max_access_size = 4,
+ .unaligned = false,
+ },
+};
+
+static void imx2_wdt_realize(DeviceState *dev, Error **errp)
+{
+ IMX2WdtState *s = IMX2_WDT(dev);
+
+ memory_region_init_io(&s->mmio, OBJECT(dev),
+ &imx2_wdt_ops, s,
+ TYPE_IMX2_WDT".mmio", SZ_64K);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
+}
+
+static void imx2_wdt_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = imx2_wdt_realize;
+
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo imx2_wdt_info = {
+ .name = TYPE_IMX2_WDT,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(IMX2WdtState),
+ .class_init = imx2_wdt_class_init,
+};
+
+static WatchdogTimerModel model = {
+ .wdt_name = "imx2-watchdog",
+ .wdt_description = "i.MX2 Watchdog",
+};
+
+static void imx2_wdt_register_type(void)
+{
+ watchdog_add_model(&model);
+ type_register_static(&imx2_wdt_info);
+}
+type_init(imx2_wdt_register_type)
diff --git a/include/hw/misc/imx2_wdt.h b/include/hw/misc/imx2_wdt.h
new file mode 100644
index 0000000000..3a30ed1ef8
--- /dev/null
+++ b/include/hw/misc/imx2_wdt.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2017, Impinj, Inc.
+ *
+ * i.MX2 Watchdog IP block
+ *
+ * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef IMX2_WDT_H
+#define IMX2_WDT_H
+
+#include "qemu/bitops.h"
+#include "hw/sysbus.h"
+
+#define TYPE_IMX2_WDT "imx2.wdt"
+#define IMX2_WDT(obj) OBJECT_CHECK(IMX2WdtState, (obj), TYPE_IMX2_WDT)
+
+enum IMX2WdtRegisters {
+ IMX2_WDT_WCR,
+ IMX2_WDT_REG_NUM
+};
+
+
+typedef struct IMX2WdtState {
+ /* <private> */
+ SysBusDevice parent_obj;
+
+ MemoryRegion mmio;
+
+ uint16_t reg[IMX2_WDT_REG_NUM];
+} IMX2WdtState;
+
+#endif /* IMX7_SNVS_H */
--
2.13.5
On 18 September 2017 at 20:50, Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
> Add enough code to emulate i.MX2 watchdog IP block so it would be
> possible to reboot the machine running Linux Guest.
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org
> Cc: yurovsky@gmail.com
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> hw/misc/Makefile.objs | 1 +
> hw/misc/imx2_wdt.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
> include/hw/misc/imx2_wdt.h | 36 ++++++++++++++
> 3 files changed, 154 insertions(+)
> create mode 100644 hw/misc/imx2_wdt.c
> create mode 100644 include/hw/misc/imx2_wdt.h
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index ac1be05a03..c393a93456 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -35,6 +35,7 @@ obj-$(CONFIG_IMX) += imx25_ccm.o
> obj-$(CONFIG_IMX) += imx6_ccm.o
> obj-$(CONFIG_IMX) += imx6_src.o
> obj-$(CONFIG_IMX) += imx7_ccm.o
> +obj-$(CONFIG_IMX) += imx2_wdt.o
> obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
> obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
> obj-$(CONFIG_MAINSTONE) += mst_fpga.o
> diff --git a/hw/misc/imx2_wdt.c b/hw/misc/imx2_wdt.c
> new file mode 100644
> index 0000000000..9d97a19511
> --- /dev/null
> +++ b/hw/misc/imx2_wdt.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (c) 2017, Impinj, Inc.
> + *
> + * i.MX2 Watchdog IP block
> + *
> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "hw/hw.h"
> +#include "exec/memory.h"
> +#include "exec/address-spaces.h"
> +#include "hw/sysbus.h"
> +#include "qemu/log.h"
> +#include "qemu/timer.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/watchdog.h"
> +#include "qemu/error-report.h"
> +#include "qemu/sizes.h"
That's an awful lot of includes for a very simple device. Are
you sure they're all needed?
> +
> +#include "hw/misc/imx2_wdt.h"
> +
> +
> +#define IMX2_WDT_WCR_WDA BIT(5) /* -> External Reset WDOG_B */
> +#define IMX2_WDT_WCR_SRS BIT(4) /* -> Software Reset Signal */
> +
> +static uint64_t imx2_wdt_read(void *opaque, hwaddr addr,
> + unsigned int size)
> +{
> + IMX2WdtState *s = opaque;
> + const size_t index = addr / sizeof(s->reg[0]);
> +
> + if (index < ARRAY_SIZE(s->reg))
> + return s->reg[index];
> + else
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
> +
> + return 0xDEADBEEF;
> +}
> +
> +static void imx2_wdt_write(void *opaque, hwaddr addr,
> + uint64_t val64, unsigned int size)
> +{
> + uint16_t value = val64;
> + IMX2WdtState *s = opaque;
> + const size_t index = addr / sizeof(s->reg[0]);
> +
> + switch (index) {
> + case IMX2_WDT_WCR:
> + if (value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS))
> + watchdog_perform_action();
Missing "break"?
Also checkpatch should tell you you need more braces.
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
> + }
> +}
> +
> +static const MemoryRegionOps imx2_wdt_ops = {
> + .read = imx2_wdt_read,
> + .write = imx2_wdt_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> + .valid = {
> + /*
> + * Our device would not work correctly if the guest was doing
> + * unaligned access. This might not be a limitation on the real
> + * device but in practice there is no reason for a guest to access
> + * this device unaligned.
> + */
This sounds like it's perhaps a job for .impl.unaligned
rather than .valid.unaligned ?
> + .min_access_size = 4,
> + .max_access_size = 4,
> + .unaligned = false,
> + },
> +};
> +
> +static void imx2_wdt_realize(DeviceState *dev, Error **errp)
> +{
> + IMX2WdtState *s = IMX2_WDT(dev);
> +
> + memory_region_init_io(&s->mmio, OBJECT(dev),
> + &imx2_wdt_ops, s,
> + TYPE_IMX2_WDT".mmio", SZ_64K);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
> +}
> +
> +static void imx2_wdt_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = imx2_wdt_realize;
> +
> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo imx2_wdt_info = {
> + .name = TYPE_IMX2_WDT,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(IMX2WdtState),
> + .class_init = imx2_wdt_class_init,
> +};
This device needs a reset function and vmstate description.
> +
> +static WatchdogTimerModel model = {
> + .wdt_name = "imx2-watchdog",
> + .wdt_description = "i.MX2 Watchdog",
> +};
> +
> +static void imx2_wdt_register_type(void)
> +{
> + watchdog_add_model(&model);
> + type_register_static(&imx2_wdt_info);
> +}
thanks
-- PMM
On Fri, Oct 6, 2017 at 7:22 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 18 September 2017 at 20:50, Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
>> Add enough code to emulate i.MX2 watchdog IP block so it would be
>> possible to reboot the machine running Linux Guest.
>>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: qemu-devel@nongnu.org
>> Cc: qemu-arm@nongnu.org
>> Cc: yurovsky@gmail.com
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>> hw/misc/Makefile.objs | 1 +
>> hw/misc/imx2_wdt.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
>> include/hw/misc/imx2_wdt.h | 36 ++++++++++++++
>> 3 files changed, 154 insertions(+)
>> create mode 100644 hw/misc/imx2_wdt.c
>> create mode 100644 include/hw/misc/imx2_wdt.h
>>
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index ac1be05a03..c393a93456 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -35,6 +35,7 @@ obj-$(CONFIG_IMX) += imx25_ccm.o
>> obj-$(CONFIG_IMX) += imx6_ccm.o
>> obj-$(CONFIG_IMX) += imx6_src.o
>> obj-$(CONFIG_IMX) += imx7_ccm.o
>> +obj-$(CONFIG_IMX) += imx2_wdt.o
>> obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
>> obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
>> obj-$(CONFIG_MAINSTONE) += mst_fpga.o
>> diff --git a/hw/misc/imx2_wdt.c b/hw/misc/imx2_wdt.c
>> new file mode 100644
>> index 0000000000..9d97a19511
>> --- /dev/null
>> +++ b/hw/misc/imx2_wdt.c
>> @@ -0,0 +1,117 @@
>> +/*
>> + * Copyright (c) 2017, Impinj, Inc.
>> + *
>> + * i.MX2 Watchdog IP block
>> + *
>> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "qemu-common.h"
>> +#include "hw/hw.h"
>> +#include "exec/memory.h"
>> +#include "exec/address-spaces.h"
>> +#include "hw/sysbus.h"
>> +#include "qemu/log.h"
>> +#include "qemu/timer.h"
>> +#include "sysemu/sysemu.h"
>> +#include "sysemu/watchdog.h"
>> +#include "qemu/error-report.h"
>> +#include "qemu/sizes.h"
>
> That's an awful lot of includes for a very simple device. Are
> you sure they're all needed?
No, I am not sure. I'll double-check.
>
>> +
>> +#include "hw/misc/imx2_wdt.h"
>> +
>> +
>> +#define IMX2_WDT_WCR_WDA BIT(5) /* -> External Reset WDOG_B */
>> +#define IMX2_WDT_WCR_SRS BIT(4) /* -> Software Reset Signal */
>> +
>> +static uint64_t imx2_wdt_read(void *opaque, hwaddr addr,
>> + unsigned int size)
>> +{
>> + IMX2WdtState *s = opaque;
>> + const size_t index = addr / sizeof(s->reg[0]);
>> +
>> + if (index < ARRAY_SIZE(s->reg))
>> + return s->reg[index];
>> + else
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
>> +
>> + return 0xDEADBEEF;
>> +}
>> +
>> +static void imx2_wdt_write(void *opaque, hwaddr addr,
>> + uint64_t val64, unsigned int size)
>> +{
>> + uint16_t value = val64;
>> + IMX2WdtState *s = opaque;
>> + const size_t index = addr / sizeof(s->reg[0]);
>> +
>> + switch (index) {
>> + case IMX2_WDT_WCR:
>> + if (value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS))
>> + watchdog_perform_action();
>
> Missing "break"?
>
Oops, good catch! Will fix in v2.
> Also checkpatch should tell you you need more braces.
>
Yeah, I definitely forgot to run this through checkpatch, sorry.
>> + default:
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
>> + }
>> +}
>> +
>> +static const MemoryRegionOps imx2_wdt_ops = {
>> + .read = imx2_wdt_read,
>> + .write = imx2_wdt_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> + .valid = {
>> + /*
>> + * Our device would not work correctly if the guest was doing
>> + * unaligned access. This might not be a limitation on the real
>> + * device but in practice there is no reason for a guest to access
>> + * this device unaligned.
>> + */
>
> This sounds like it's perhaps a job for .impl.unaligned
> rather than .valid.unaligned ?
>
This was a snippet I copied from some already existing emulation
block, so I didn't pay too much attention. But yeah, now that you
suggested it, I think I should've used impl.valid as you suggest. Will
fix in v2.
>> + .min_access_size = 4,
>> + .max_access_size = 4,
>> + .unaligned = false,
>> + },
>> +};
>> +
>> +static void imx2_wdt_realize(DeviceState *dev, Error **errp)
>> +{
>> + IMX2WdtState *s = IMX2_WDT(dev);
>> +
>> + memory_region_init_io(&s->mmio, OBJECT(dev),
>> + &imx2_wdt_ops, s,
>> + TYPE_IMX2_WDT".mmio", SZ_64K);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
>> +}
>> +
>> +static void imx2_wdt_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->realize = imx2_wdt_realize;
>> +
>> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>> +}
>> +
>> +static const TypeInfo imx2_wdt_info = {
>> + .name = TYPE_IMX2_WDT,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(IMX2WdtState),
>> + .class_init = imx2_wdt_class_init,
>> +};
>
> This device needs a reset function and vmstate description.
>
OK, will fix in v2.
Thanks
Andrey Smirnov
© 2016 - 2025 Red Hat, Inc.