1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v4 00/14] Initial i.MX7 support
  4. View patch

[Qemu-devel] [PATCH v4 05/14] i.MX: Add code to emulate i.MX7 SNVS IP-block

Andrey Smirnov posted 14 patches 7 years, 9 months ago
Only 12 patches received!
There is a newer version of this series
[Qemu-devel] [PATCH v4 05/14] i.MX: Add code to emulate i.MX7 SNVS IP-block
Posted by Andrey Smirnov 7 years, 9 months ago 
Add code to emulate SNVS IP-block. Currently only the bits needed to
be able to emulate machine shutdown are implemented.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Cc: yurovsky@gmail.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 hw/misc/Makefile.objs       |  1 +
 hw/misc/imx7_snvs.c         | 83 +++++++++++++++++++++++++++++++++++++++++++++
 include/hw/misc/imx7_snvs.h | 35 +++++++++++++++++++
 3 files changed, 119 insertions(+)
 create mode 100644 hw/misc/imx7_snvs.c
 create mode 100644 include/hw/misc/imx7_snvs.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 4b2b705a6c..019886912c 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -35,6 +35,7 @@ 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_IMX) += imx7_snvs.o
 obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
 obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
 obj-$(CONFIG_MAINSTONE) += mst_fpga.o
diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c
new file mode 100644
index 0000000000..670b9f4639
--- /dev/null
+++ b/hw/misc/imx7_snvs.c
@@ -0,0 +1,83 @@
+/*
+ * IMX7 Secure Non-Volatile Storage
+ *
+ * Copyright (c) 2017, Impinj, Inc.
+ *
+ * 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.
+ *
+ * Bare minimum emulation code needed to support being able to shut
+ * down linux guest gracefully.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/misc/imx7_snvs.h"
+#include "qemu/log.h"
+#include "sysemu/sysemu.h"
+
+static uint64_t imx7_snvs_read(void *opaque, hwaddr offset, unsigned size)
+{
+    return 0;
+}
+
+static void imx7_snvs_write(void *opaque, hwaddr offset,
+                            uint64_t v, unsigned size)
+{
+    const uint32_t value = v;
+    const uint32_t mask  = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN;
+
+    if (offset == SNVS_LPCR && ((value & mask) == mask)) {
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+    }
+}
+
+static const struct MemoryRegionOps imx7_snvs_ops = {
+    .read = imx7_snvs_read,
+    .write = imx7_snvs_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        /*
+         * 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 imx7_snvs_init(Object *obj)
+{
+    SysBusDevice *sd = SYS_BUS_DEVICE(obj);
+    IMX7SNVSState *s = IMX7_SNVS(obj);
+
+    memory_region_init_io(&s->mmio, obj, &imx7_snvs_ops, s,
+                          TYPE_IMX7_SNVS, 0x1000);
+
+    sysbus_init_mmio(sd, &s->mmio);
+}
+
+static void imx7_snvs_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc  = "i.MX7 Secure Non-Volatile Storage Module";
+}
+
+static const TypeInfo imx7_snvs_info = {
+    .name          = TYPE_IMX7_SNVS,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(IMX7SNVSState),
+    .instance_init = imx7_snvs_init,
+    .class_init    = imx7_snvs_class_init,
+};
+
+static void imx7_snvs_register_type(void)
+{
+    type_register_static(&imx7_snvs_info);
+}
+type_init(imx7_snvs_register_type)
diff --git a/include/hw/misc/imx7_snvs.h b/include/hw/misc/imx7_snvs.h
new file mode 100644
index 0000000000..255f8f26f9
--- /dev/null
+++ b/include/hw/misc/imx7_snvs.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2017, Impinj, Inc.
+ *
+ * i.MX7 SNVS block emulation code
+ *
+ * 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 IMX7_SNVS_H
+#define IMX7_SNVS_H
+
+#include "qemu/bitops.h"
+#include "hw/sysbus.h"
+
+
+enum IMX7SNVSRegisters {
+    SNVS_LPCR = 0x38,
+    SNVS_LPCR_TOP   = BIT(6),
+    SNVS_LPCR_DP_EN = BIT(5)
+};
+
+#define TYPE_IMX7_SNVS "imx7.snvs"
+#define IMX7_SNVS(obj) OBJECT_CHECK(IMX7SNVSState, (obj), TYPE_IMX7_SNVS)
+
+typedef struct IMX7SNVSState {
+    /* <private> */
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+} IMX7SNVSState;
+
+#endif /* IMX7_SNVS_H */
-- 
2.14.3
Re: [Qemu-devel] [PATCH v4 05/14] i.MX: Add code to emulate i.MX7 SNVS IP-block
Posted by Philippe Mathieu-Daudé 7 years, 8 months ago 
On 01/15/2018 10:37 PM, Andrey Smirnov wrote:
> Add code to emulate SNVS IP-block. Currently only the bits needed to
> be able to emulate machine shutdown are implemented.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org
> Cc: yurovsky@gmail.com
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  hw/misc/Makefile.objs       |  1 +
>  hw/misc/imx7_snvs.c         | 83 +++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/misc/imx7_snvs.h | 35 +++++++++++++++++++
>  3 files changed, 119 insertions(+)
>  create mode 100644 hw/misc/imx7_snvs.c
>  create mode 100644 include/hw/misc/imx7_snvs.h
> 
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 4b2b705a6c..019886912c 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -35,6 +35,7 @@ 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_IMX) += imx7_snvs.o
>  obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
>  obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
>  obj-$(CONFIG_MAINSTONE) += mst_fpga.o
> diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c
> new file mode 100644
> index 0000000000..670b9f4639
> --- /dev/null
> +++ b/hw/misc/imx7_snvs.c
> @@ -0,0 +1,83 @@
> +/*
> + * IMX7 Secure Non-Volatile Storage
> + *
> + * Copyright (c) 2017, Impinj, Inc.
> + *
> + * 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.
> + *
> + * Bare minimum emulation code needed to support being able to shut
> + * down linux guest gracefully.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/misc/imx7_snvs.h"
> +#include "qemu/log.h"
> +#include "sysemu/sysemu.h"
> +
> +static uint64_t imx7_snvs_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    return 0;
> +}
> +
> +static void imx7_snvs_write(void *opaque, hwaddr offset,
> +                            uint64_t v, unsigned size)
> +{
> +    const uint32_t value = v;
> +    const uint32_t mask  = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN;
> +
> +    if (offset == SNVS_LPCR && ((value & mask) == mask)) {
> +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +    }
> +}
> +
> +static const struct MemoryRegionOps imx7_snvs_ops = {
> +    .read = imx7_snvs_read,

Same here, you can remove the imx7_snvs_read() function since
memory_region_dispatch_read() takes care of this and return 0.

> +    .write = imx7_snvs_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        /*
> +         * 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 imx7_snvs_init(Object *obj)
> +{
> +    SysBusDevice *sd = SYS_BUS_DEVICE(obj);
> +    IMX7SNVSState *s = IMX7_SNVS(obj);
> +
> +    memory_region_init_io(&s->mmio, obj, &imx7_snvs_ops, s,
> +                          TYPE_IMX7_SNVS, 0x1000);
> +
> +    sysbus_init_mmio(sd, &s->mmio);
> +}
> +
> +static void imx7_snvs_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->desc  = "i.MX7 Secure Non-Volatile Storage Module";
> +}
> +
> +static const TypeInfo imx7_snvs_info = {
> +    .name          = TYPE_IMX7_SNVS,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(IMX7SNVSState),
> +    .instance_init = imx7_snvs_init,
> +    .class_init    = imx7_snvs_class_init,
> +};
> +
> +static void imx7_snvs_register_type(void)
> +{
> +    type_register_static(&imx7_snvs_info);
> +}
> +type_init(imx7_snvs_register_type)
> diff --git a/include/hw/misc/imx7_snvs.h b/include/hw/misc/imx7_snvs.h
> new file mode 100644
> index 0000000000..255f8f26f9
> --- /dev/null
> +++ b/include/hw/misc/imx7_snvs.h
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright (c) 2017, Impinj, Inc.
> + *
> + * i.MX7 SNVS block emulation code
> + *
> + * 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 IMX7_SNVS_H
> +#define IMX7_SNVS_H
> +
> +#include "qemu/bitops.h"
> +#include "hw/sysbus.h"
> +
> +
> +enum IMX7SNVSRegisters {
> +    SNVS_LPCR = 0x38,
> +    SNVS_LPCR_TOP   = BIT(6),
> +    SNVS_LPCR_DP_EN = BIT(5)
> +};
> +
> +#define TYPE_IMX7_SNVS "imx7.snvs"
> +#define IMX7_SNVS(obj) OBJECT_CHECK(IMX7SNVSState, (obj), TYPE_IMX7_SNVS)
> +
> +typedef struct IMX7SNVSState {
> +    /* <private> */
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion mmio;
> +} IMX7SNVSState;
> +
> +#endif /* IMX7_SNVS_H */
>
Re: [Qemu-devel] [PATCH v4 05/14] i.MX: Add code to emulate i.MX7 SNVS IP-block
Posted by Andrey Smirnov 7 years, 8 months ago 
On Wed, Jan 31, 2018 at 9:10 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 01/15/2018 10:37 PM, Andrey Smirnov wrote:
>> Add code to emulate SNVS IP-block. Currently only the bits needed to
>> be able to emulate machine shutdown are implemented.
>>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Cc: qemu-devel@nongnu.org
>> Cc: qemu-arm@nongnu.org
>> Cc: yurovsky@gmail.com
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  hw/misc/Makefile.objs       |  1 +
>>  hw/misc/imx7_snvs.c         | 83 +++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/misc/imx7_snvs.h | 35 +++++++++++++++++++
>>  3 files changed, 119 insertions(+)
>>  create mode 100644 hw/misc/imx7_snvs.c
>>  create mode 100644 include/hw/misc/imx7_snvs.h
>>
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 4b2b705a6c..019886912c 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -35,6 +35,7 @@ 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_IMX) += imx7_snvs.o
>>  obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
>>  obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
>>  obj-$(CONFIG_MAINSTONE) += mst_fpga.o
>> diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c
>> new file mode 100644
>> index 0000000000..670b9f4639
>> --- /dev/null
>> +++ b/hw/misc/imx7_snvs.c
>> @@ -0,0 +1,83 @@
>> +/*
>> + * IMX7 Secure Non-Volatile Storage
>> + *
>> + * Copyright (c) 2017, Impinj, Inc.
>> + *
>> + * 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.
>> + *
>> + * Bare minimum emulation code needed to support being able to shut
>> + * down linux guest gracefully.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/misc/imx7_snvs.h"
>> +#include "qemu/log.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +static uint64_t imx7_snvs_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +    return 0;
>> +}
>> +
>> +static void imx7_snvs_write(void *opaque, hwaddr offset,
>> +                            uint64_t v, unsigned size)
>> +{
>> +    const uint32_t value = v;
>> +    const uint32_t mask  = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN;
>> +
>> +    if (offset == SNVS_LPCR && ((value & mask) == mask)) {
>> +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>> +    }
>> +}
>> +
>> +static const struct MemoryRegionOps imx7_snvs_ops = {
>> +    .read = imx7_snvs_read,
>
> Same here, you can remove the imx7_snvs_read() function since
> memory_region_dispatch_read() takes care of this and return 0.
>

Hmm, I don't think I agree both from reading code and trying this out.
Without .read callback the call chain ends up being the following
memory_region_dispatch_read() -> memory_region_dispatch_read1() ->
access_with_adjusted_size(..., memory_region_oldmmio_read_accessor,
...) -> memory_region_oldmmio_read_accessor() ->
mr->ops->old_mmio.read[ctz32(size)]() -> SEGFAULT

As much as I'd love to get rid of dummy .read callback, I don't see a
way to do that, unfortunately.

Thanks,
Andrey Smirnov

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.