All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] arm: microbit Non-Volatile Memory Controller
@ 2019-01-20 14:34 Stefan Hajnoczi
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device() Stefan Hajnoczi
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2019-01-20 14:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, jim, jusual, Joel Stanley, Steffen Görtz,
	Peter Maydell, qemu-arm

This series adds the Non-Volatile Memory Controller, which controls access to
the User Information Control Registers (UICR), Factory Information Control
Registers (FICR), and flash memory.

This is the last piece of microbit work needed to make basic programs like
Micropython "Hello world" work under QEMU.

Originally sent as part of Steffen's longer microbit device emulation series, I
extracted this and deferred it until later because cleanups were necessary:

 * Use memory_region_flush_rom_device() to dirty/invalidate memory [Peter]
   ^--- Paolo: I CCed you on this new memory API
 * Fix device-introspect-test segfault due to missing owner when initializing
   FICR and UICR memory regions [Peter]
 * Fix off-by-one assertion checks [Peter]
 * Fix missing whitespace at end of comment [Peter]
 * Clear UICR on reset - we'd need a block device for true non-volatility
   [Peter]

Stefan Hajnoczi (1):
  memory: add memory_region_flush_rom_device()

Steffen Görtz (3):
  hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories
  arm: Instantiate NRF51 special NVM's and NVMC
  tests/microbit-test: Add tests for nRF51 NVMC

 hw/nvram/Makefile.objs       |   1 +
 include/exec/memory.h        |  18 ++
 include/hw/arm/nrf51_soc.h   |   2 +
 include/hw/nvram/nrf51_nvm.h |  64 ++++++
 exec.c                       |  12 ++
 hw/arm/nrf51_soc.c           |  41 ++--
 hw/nvram/nrf51_nvm.c         | 381 +++++++++++++++++++++++++++++++++++
 tests/microbit-test.c        |  97 +++++++++
 8 files changed, 604 insertions(+), 12 deletions(-)
 create mode 100644 include/hw/nvram/nrf51_nvm.h
 create mode 100644 hw/nvram/nrf51_nvm.c

-- 
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device()
  2019-01-20 14:34 [Qemu-devel] [PATCH 0/4] arm: microbit Non-Volatile Memory Controller Stefan Hajnoczi
@ 2019-01-20 14:34 ` Stefan Hajnoczi
  2019-01-22 16:36   ` Peter Maydell
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 2/4] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories Stefan Hajnoczi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2019-01-20 14:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, jim, jusual, Joel Stanley, Steffen Görtz,
	Peter Maydell, qemu-arm

ROM devices go via MemoryRegionOps->write() callbacks for write
operations and do not dirty/invalidate that memory.  Device emulation
must be able to mark memory ranges that have been modified internally
(e.g. using memory_region_get_ram_ptr()).

Introduce the memory_region_flush_rom_device() API for this purpose.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/exec/memory.h | 18 ++++++++++++++++++
 exec.c                | 12 ++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index cd2f209b64..abe9cc79c0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1344,6 +1344,24 @@ bool memory_region_snapshot_get_dirty(MemoryRegion *mr,
 void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
                                hwaddr size, unsigned client);
 
+/**
+ * memory_region_flush_rom_device: Mark a range of pages dirty and invalidate
+ *                                 TBs (for self-modifying code).
+ *
+ * The MemoryRegionOps->write() callback of a ROM device must use this function
+ * to mark byte ranges that have been modified internally, such as by directly
+ * accessing the memory returned by memory_region_get_ram_ptr().
+ *
+ * This function marks the range dirty and invalidates TBs so that TCG can
+ * detect self-modifying code.
+ *
+ * @mr: the region being flushed.
+ * @addr: the start, relative to the start of the region, of the range being
+ *        flushed.
+ * @size: the size, in bytes, of the range being flushed.
+ */
+void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size);
+
 /**
  * memory_region_set_readonly: Turn a memory region read-only (or read-write)
  *
diff --git a/exec.c b/exec.c
index 895449f926..105ff21e74 100644
--- a/exec.c
+++ b/exec.c
@@ -3162,6 +3162,18 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
     cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
 }
 
+void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size)
+{
+    /* In principle this function would work on other memory region types too,
+     * but the ROM device use case is the only one where this operation is
+     * necessary.  Other memory regions should use the
+     * address_space_read/write() APIs.
+     */
+    assert(memory_region_is_romd(mr));
+
+    invalidate_and_set_dirty(mr, addr, size);
+}
+
 static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
 {
     unsigned access_size_max = mr->ops->valid.max_access_size;
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/4] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories
  2019-01-20 14:34 [Qemu-devel] [PATCH 0/4] arm: microbit Non-Volatile Memory Controller Stefan Hajnoczi
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device() Stefan Hajnoczi
@ 2019-01-20 14:34 ` Stefan Hajnoczi
  2019-01-22 16:34   ` Peter Maydell
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 3/4] arm: Instantiate NRF51 special NVM's and NVMC Stefan Hajnoczi
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 4/4] tests/microbit-test: Add tests for nRF51 NVMC Stefan Hajnoczi
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2019-01-20 14:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, jim, jusual, Joel Stanley, Steffen Görtz,
	Peter Maydell, qemu-arm

From: Steffen Görtz <contrib@steffen-goertz.de>

The nRF51 contains three regions of non-volatile memory (NVM):
- CODE (R/W): contains code
- FICR (R): Factory information like code size, chip id etc.
- UICR (R/W): Changeable configuration data. Lock bits, Code
  protection configuration, Bootloader address, Nordic SoftRadio
  configuration, Firmware configuration.

Read and write access to the memories is managed by the
Non-volatile memory controller.

Memory schema:
 [ CPU ] -+- [ NVM, either FICR, UICR or CODE ]
          |      |
          \- [ NVMC ]

Signed-off-by: Steffen Görtz <contrib@steffen-goertz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 * Fix device-introspect-test segfault due to missing owner when
   initializing FICR and UICR memory regions [Peter]
 * Fix off-by-one assertion checks [Peter]
 * Fix missing whitespace at end of comment [Peter]
 * Clear UICR on reset - we'd need a block device for true
   non-volatility [Peter]
 * Use memory_region_flush_rom_device() to dirty/invalidate memory
   [Peter]
---
 hw/nvram/Makefile.objs       |   1 +
 include/hw/nvram/nrf51_nvm.h |  64 ++++++
 hw/nvram/nrf51_nvm.c         | 381 +++++++++++++++++++++++++++++++++++
 3 files changed, 446 insertions(+)
 create mode 100644 include/hw/nvram/nrf51_nvm.h
 create mode 100644 hw/nvram/nrf51_nvm.c

diff --git a/hw/nvram/Makefile.objs b/hw/nvram/Makefile.objs
index b318e53a43..26f7b4ca35 100644
--- a/hw/nvram/Makefile.objs
+++ b/hw/nvram/Makefile.objs
@@ -5,3 +5,4 @@ common-obj-y += fw_cfg.o
 common-obj-y += chrp_nvram.o
 common-obj-$(CONFIG_MAC_NVRAM) += mac_nvram.o
 obj-$(CONFIG_PSERIES) += spapr_nvram.o
+obj-$(CONFIG_NRF51_SOC) += nrf51_nvm.o
diff --git a/include/hw/nvram/nrf51_nvm.h b/include/hw/nvram/nrf51_nvm.h
new file mode 100644
index 0000000000..0a8b41b358
--- /dev/null
+++ b/include/hw/nvram/nrf51_nvm.h
@@ -0,0 +1,64 @@
+/*
+ * Nordic Semiconductor nRF51 non-volatile memory
+ *
+ * It provides an interface to erase regions in flash memory.
+ * Furthermore it provides the user and factory information registers.
+ *
+ * QEMU interface:
+ * + sysbus MMIO regions 0: NVMC peripheral registers
+ * + sysbus MMIO regions 1: FICR peripheral registers
+ * + sysbus MMIO regions 2: UICR peripheral registers
+ * + flash-size property: flash size in bytes.
+ *
+ * Accuracy of the peripheral model:
+ * + Code regions (MPU configuration) are disregarded.
+ *
+ * Copyright 2018 Steffen Görtz <contrib@steffen-goertz.de>
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#ifndef NRF51_NVM_H
+#define NRF51_NVM_H
+
+#include "hw/sysbus.h"
+#define TYPE_NRF51_NVM "nrf51_soc.nvm"
+#define NRF51_NVM(obj) OBJECT_CHECK(NRF51NVMState, (obj), TYPE_NRF51_NVM)
+
+#define NRF51_UICR_FIXTURE_SIZE 64
+
+#define NRF51_NVMC_SIZE         0x1000
+
+#define NRF51_NVMC_READY        0x400
+#define NRF51_NVMC_READY_READY  0x01
+#define NRF51_NVMC_CONFIG       0x504
+#define NRF51_NVMC_CONFIG_MASK  0x03
+#define NRF51_NVMC_CONFIG_WEN   0x01
+#define NRF51_NVMC_CONFIG_EEN   0x02
+#define NRF51_NVMC_ERASEPCR1    0x508
+#define NRF51_NVMC_ERASEPCR0    0x510
+#define NRF51_NVMC_ERASEALL     0x50C
+#define NRF51_NVMC_ERASEUICR    0x514
+#define NRF51_NVMC_ERASE        0x01
+
+#define NRF51_UICR_SIZE         0x100
+
+typedef struct NRF51NVMState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+    MemoryRegion ficr;
+    MemoryRegion uicr;
+    MemoryRegion flash;
+
+    uint32_t uicr_content[NRF51_UICR_FIXTURE_SIZE];
+    uint32_t flash_size;
+    uint32_t *storage;
+
+    uint32_t config;
+
+} NRF51NVMState;
+
+
+#endif
diff --git a/hw/nvram/nrf51_nvm.c b/hw/nvram/nrf51_nvm.c
new file mode 100644
index 0000000000..e51228d669
--- /dev/null
+++ b/hw/nvram/nrf51_nvm.c
@@ -0,0 +1,381 @@
+/*
+ * Nordic Semiconductor nRF51 non-volatile memory
+ *
+ * It provides an interface to erase regions in flash memory.
+ * Furthermore it provides the user and factory information registers.
+ *
+ * Reference Manual: http://infocenter.nordicsemi.com/pdf/nRF51_RM_v3.0.pdf
+ *
+ * See nRF51 reference manual and product sheet sections:
+ * + Non-Volatile Memory Controller (NVMC)
+ * + Factory Information Configuration Registers (FICR)
+ * + User Information Configuration Registers (UICR)
+ *
+ * Copyright 2018 Steffen Görtz <contrib@steffen-goertz.de>
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "exec/address-spaces.h"
+#include "hw/arm/nrf51.h"
+#include "hw/nvram/nrf51_nvm.h"
+
+/*
+ * FICR Registers Assignments
+ * CODEPAGESIZE      0x010
+ * CODESIZE          0x014
+ * CLENR0            0x028
+ * PPFC              0x02C
+ * NUMRAMBLOCK       0x034
+ * SIZERAMBLOCKS     0x038
+ * SIZERAMBLOCK[0]   0x038
+ * SIZERAMBLOCK[1]   0x03C
+ * SIZERAMBLOCK[2]   0x040
+ * SIZERAMBLOCK[3]   0x044
+ * CONFIGID          0x05C
+ * DEVICEID[0]       0x060
+ * DEVICEID[1]       0x064
+ * ER[0]             0x080
+ * ER[1]             0x084
+ * ER[2]             0x088
+ * ER[3]             0x08C
+ * IR[0]             0x090
+ * IR[1]             0x094
+ * IR[2]             0x098
+ * IR[3]             0x09C
+ * DEVICEADDRTYPE    0x0A0
+ * DEVICEADDR[0]     0x0A4
+ * DEVICEADDR[1]     0x0A8
+ * OVERRIDEEN        0x0AC
+ * NRF_1MBIT[0]      0x0B0
+ * NRF_1MBIT[1]      0x0B4
+ * NRF_1MBIT[2]      0x0B8
+ * NRF_1MBIT[3]      0x0BC
+ * NRF_1MBIT[4]      0x0C0
+ * BLE_1MBIT[0]      0x0EC
+ * BLE_1MBIT[1]      0x0F0
+ * BLE_1MBIT[2]      0x0F4
+ * BLE_1MBIT[3]      0x0F8
+ * BLE_1MBIT[4]      0x0FC
+ */
+static const uint32_t ficr_content[64] = {
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0x00000400,
+    0x00000100, 0xFFFFFFFF, 0xFFFFFFFF, 0x00000002, 0x00002000,
+    0x00002000, 0x00002000, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0x00000003,
+    0x12345678, 0x9ABCDEF1, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
+    0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF
+};
+
+static uint64_t ficr_read(void *opaque, hwaddr offset, unsigned int size)
+{
+    assert(offset < sizeof(ficr_content));
+    return ficr_content[offset / 4];
+}
+
+static void ficr_write(void *opaque, hwaddr offset, uint64_t value,
+        unsigned int size)
+{
+    /* Intentionally do nothing */
+}
+
+static const MemoryRegionOps ficr_ops = {
+    .read = ficr_read,
+    .write = ficr_write,
+    .impl.min_access_size = 4,
+    .impl.max_access_size = 4,
+    .endianness = DEVICE_LITTLE_ENDIAN
+};
+
+/*
+ * UICR Registers Assignments
+ * CLENR0           0x000
+ * RBPCONF          0x004
+ * XTALFREQ         0x008
+ * FWID             0x010
+ * BOOTLOADERADDR   0x014
+ * NRFFW[0]         0x014
+ * NRFFW[1]         0x018
+ * NRFFW[2]         0x01C
+ * NRFFW[3]         0x020
+ * NRFFW[4]         0x024
+ * NRFFW[5]         0x028
+ * NRFFW[6]         0x02C
+ * NRFFW[7]         0x030
+ * NRFFW[8]         0x034
+ * NRFFW[9]         0x038
+ * NRFFW[10]        0x03C
+ * NRFFW[11]        0x040
+ * NRFFW[12]        0x044
+ * NRFFW[13]        0x048
+ * NRFFW[14]        0x04C
+ * NRFHW[0]         0x050
+ * NRFHW[1]         0x054
+ * NRFHW[2]         0x058
+ * NRFHW[3]         0x05C
+ * NRFHW[4]         0x060
+ * NRFHW[5]         0x064
+ * NRFHW[6]         0x068
+ * NRFHW[7]         0x06C
+ * NRFHW[8]         0x070
+ * NRFHW[9]         0x074
+ * NRFHW[10]        0x078
+ * NRFHW[11]        0x07C
+ * CUSTOMER[0]      0x080
+ * CUSTOMER[1]      0x084
+ * CUSTOMER[2]      0x088
+ * CUSTOMER[3]      0x08C
+ * CUSTOMER[4]      0x090
+ * CUSTOMER[5]      0x094
+ * CUSTOMER[6]      0x098
+ * CUSTOMER[7]      0x09C
+ * CUSTOMER[8]      0x0A0
+ * CUSTOMER[9]      0x0A4
+ * CUSTOMER[10]     0x0A8
+ * CUSTOMER[11]     0x0AC
+ * CUSTOMER[12]     0x0B0
+ * CUSTOMER[13]     0x0B4
+ * CUSTOMER[14]     0x0B8
+ * CUSTOMER[15]     0x0BC
+ * CUSTOMER[16]     0x0C0
+ * CUSTOMER[17]     0x0C4
+ * CUSTOMER[18]     0x0C8
+ * CUSTOMER[19]     0x0CC
+ * CUSTOMER[20]     0x0D0
+ * CUSTOMER[21]     0x0D4
+ * CUSTOMER[22]     0x0D8
+ * CUSTOMER[23]     0x0DC
+ * CUSTOMER[24]     0x0E0
+ * CUSTOMER[25]     0x0E4
+ * CUSTOMER[26]     0x0E8
+ * CUSTOMER[27]     0x0EC
+ * CUSTOMER[28]     0x0F0
+ * CUSTOMER[29]     0x0F4
+ * CUSTOMER[30]     0x0F8
+ * CUSTOMER[31]     0x0FC
+ */
+
+static uint64_t uicr_read(void *opaque, hwaddr offset, unsigned int size)
+{
+    NRF51NVMState *s = NRF51_NVM(opaque);
+
+    assert(offset < sizeof(s->uicr_content));
+    return s->uicr_content[offset / 4];
+}
+
+static void uicr_write(void *opaque, hwaddr offset, uint64_t value,
+        unsigned int size)
+{
+    NRF51NVMState *s = NRF51_NVM(opaque);
+
+    assert(offset < sizeof(s->uicr_content));
+    s->uicr_content[offset / 4] = value;
+}
+
+static const MemoryRegionOps uicr_ops = {
+    .read = uicr_read,
+    .write = uicr_write,
+    .impl.min_access_size = 4,
+    .impl.max_access_size = 4,
+    .endianness = DEVICE_LITTLE_ENDIAN
+};
+
+
+static uint64_t io_read(void *opaque, hwaddr offset, unsigned int size)
+{
+    NRF51NVMState *s = NRF51_NVM(opaque);
+    uint64_t r = 0;
+
+    switch (offset) {
+    case NRF51_NVMC_READY:
+        r = NRF51_NVMC_READY_READY;
+        break;
+    case NRF51_NVMC_CONFIG:
+        r = s->config;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: bad read offset 0x%" HWADDR_PRIx "\n", __func__, offset);
+        break;
+    }
+
+    return r;
+}
+
+static void io_write(void *opaque, hwaddr offset, uint64_t value,
+        unsigned int size)
+{
+    NRF51NVMState *s = NRF51_NVM(opaque);
+
+    switch (offset) {
+    case NRF51_NVMC_CONFIG:
+        s->config = value & NRF51_NVMC_CONFIG_MASK;
+        break;
+    case NRF51_NVMC_ERASEPCR0:
+    case NRF51_NVMC_ERASEPCR1:
+        if (s->config & NRF51_NVMC_CONFIG_EEN) {
+            /* Mask in-page sub address */
+            value &= ~(NRF51_PAGE_SIZE - 1);
+            if (value < (s->flash_size - NRF51_PAGE_SIZE)) {
+                memset(s->storage + value / 4, 0xFF, NRF51_PAGE_SIZE);
+                memory_region_flush_rom_device(&s->flash, value,
+                                               NRF51_PAGE_SIZE);
+            }
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+            "%s: Flash erase at 0x%" HWADDR_PRIx" while flash not erasable.\n",
+            __func__, offset);
+        }
+        break;
+    case NRF51_NVMC_ERASEALL:
+        if (value == NRF51_NVMC_ERASE) {
+            if (s->config & NRF51_NVMC_CONFIG_EEN) {
+                memset(s->storage, 0xFF, s->flash_size);
+                memory_region_flush_rom_device(&s->flash, 0, s->flash_size);
+                memset(s->uicr_content, 0xFF, sizeof(s->uicr_content));
+            } else {
+                qemu_log_mask(LOG_GUEST_ERROR, "%s: Flash not erasable.\n",
+                              __func__);
+            }
+        }
+        break;
+    case NRF51_NVMC_ERASEUICR:
+        if (value == NRF51_NVMC_ERASE) {
+            memset(s->uicr_content, 0xFF, sizeof(s->uicr_content));
+        }
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: bad write offset 0x%" HWADDR_PRIx "\n", __func__, offset);
+    }
+}
+
+static const MemoryRegionOps io_ops = {
+        .read = io_read,
+        .write = io_write,
+        .impl.min_access_size = 4,
+        .impl.max_access_size = 4,
+        .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+
+static void flash_write(void *opaque, hwaddr offset, uint64_t value,
+        unsigned int size)
+{
+    NRF51NVMState *s = NRF51_NVM(opaque);
+
+    if (s->config & NRF51_NVMC_CONFIG_WEN) {
+        assert(offset < s->flash_size);
+        /* NOR Flash only allows bits to be flipped from 1's to 0's on write */
+        s->storage[offset / 4] &= value;
+    } else {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: Flash write 0x%" HWADDR_PRIx" while flash not writable.\n",
+                __func__, offset);
+    }
+}
+
+
+
+static const MemoryRegionOps flash_ops = {
+    .write = flash_write,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void nrf51_nvm_init(Object *obj)
+{
+    NRF51NVMState *s = NRF51_NVM(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+    memory_region_init_io(&s->mmio, obj, &io_ops, s, "nrf51_soc.nvmc",
+                          NRF51_NVMC_SIZE);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    memory_region_init_io(&s->ficr, obj, &ficr_ops, s, "nrf51_soc.ficr",
+                          sizeof(ficr_content));
+    sysbus_init_mmio(sbd, &s->ficr);
+
+    memory_region_init_io(&s->uicr, obj, &uicr_ops, s, "nrf51_soc.uicr",
+                          sizeof(s->uicr_content));
+    sysbus_init_mmio(sbd, &s->uicr);
+}
+
+static void nrf51_nvm_realize(DeviceState *dev, Error **errp)
+{
+    NRF51NVMState *s = NRF51_NVM(dev);
+    Error *err = NULL;
+
+    memory_region_init_rom_device(&s->flash, OBJECT(dev), &flash_ops, s,
+        "nrf51_soc.flash", s->flash_size, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    s->storage = memory_region_get_ram_ptr(&s->flash);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->flash);
+}
+
+static void nrf51_nvm_reset(DeviceState *dev)
+{
+    NRF51NVMState *s = NRF51_NVM(dev);
+
+    s->config = 0x00;
+    memset(s->uicr_content, 0xFF, sizeof(s->uicr_content));
+}
+
+static Property nrf51_nvm_properties[] = {
+    DEFINE_PROP_UINT32("flash-size", NRF51NVMState, flash_size, 0x40000),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static const VMStateDescription vmstate_nvm = {
+    .name = "nrf51_soc.nvm",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(uicr_content, NRF51NVMState,
+                NRF51_UICR_FIXTURE_SIZE),
+        VMSTATE_UINT32(config, NRF51NVMState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void nrf51_nvm_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->props = nrf51_nvm_properties;
+    dc->vmsd = &vmstate_nvm;
+    dc->realize = nrf51_nvm_realize;
+    dc->reset = nrf51_nvm_reset;
+}
+
+static const TypeInfo nrf51_nvm_info = {
+    .name = TYPE_NRF51_NVM,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(NRF51NVMState),
+    .instance_init = nrf51_nvm_init,
+    .class_init = nrf51_nvm_class_init
+};
+
+static void nrf51_nvm_register_types(void)
+{
+    type_register_static(&nrf51_nvm_info);
+}
+
+type_init(nrf51_nvm_register_types)
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/4] arm: Instantiate NRF51 special NVM's and NVMC
  2019-01-20 14:34 [Qemu-devel] [PATCH 0/4] arm: microbit Non-Volatile Memory Controller Stefan Hajnoczi
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device() Stefan Hajnoczi
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 2/4] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories Stefan Hajnoczi
@ 2019-01-20 14:34 ` Stefan Hajnoczi
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 4/4] tests/microbit-test: Add tests for nRF51 NVMC Stefan Hajnoczi
  3 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2019-01-20 14:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, jim, jusual, Joel Stanley, Steffen Görtz,
	Peter Maydell, qemu-arm

From: Steffen Görtz <contrib@steffen-goertz.de>

Instantiates UICR, FICR, FLASH and NVMC in nRF51 SOC.

Signed-off-by: Steffen Görtz <contrib@steffen-goertz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/arm/nrf51_soc.h |  2 ++
 hw/arm/nrf51_soc.c         | 41 +++++++++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/include/hw/arm/nrf51_soc.h b/include/hw/arm/nrf51_soc.h
index e06f0304b4..c8c91cf9ff 100644
--- a/include/hw/arm/nrf51_soc.h
+++ b/include/hw/arm/nrf51_soc.h
@@ -15,6 +15,7 @@
 #include "hw/char/nrf51_uart.h"
 #include "hw/misc/nrf51_rng.h"
 #include "hw/gpio/nrf51_gpio.h"
+#include "hw/nvram/nrf51_nvm.h"
 #include "hw/timer/nrf51_timer.h"
 
 #define TYPE_NRF51_SOC "nrf51-soc"
@@ -32,6 +33,7 @@ typedef struct NRF51State {
 
     NRF51UARTState uart;
     NRF51RNGState rng;
+    NRF51NVMState nvm;
     NRF51GPIOState gpio;
     NRF51TimerState timer[NRF51_NUM_TIMERS];
 
diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index 1630c27594..b839daea8b 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -29,8 +29,10 @@
  * are supported in the future, add a sub-class of NRF51SoC for
  * the specific variants
  */
-#define NRF51822_FLASH_SIZE     (256 * NRF51_PAGE_SIZE)
-#define NRF51822_SRAM_SIZE      (16 * NRF51_PAGE_SIZE)
+#define NRF51822_FLASH_PAGES    256
+#define NRF51822_SRAM_PAGES     16
+#define NRF51822_FLASH_SIZE     (NRF51822_FLASH_PAGES * NRF51_PAGE_SIZE)
+#define NRF51822_SRAM_SIZE      (NRF51822_SRAM_PAGES * NRF51_PAGE_SIZE)
 
 #define BASE_TO_IRQ(base) ((base >> 12) & 0x1F)
 
@@ -81,14 +83,6 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
 
     memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
 
-    memory_region_init_rom(&s->flash, OBJECT(s), "nrf51.flash", s->flash_size,
-            &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-    memory_region_add_subregion(&s->container, NRF51_FLASH_BASE, &s->flash);
-
     memory_region_init_ram(&s->sram, NULL, "nrf51.sram", s->sram_size, &err);
     if (err) {
         error_propagate(errp, err);
@@ -121,6 +115,29 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
                        qdev_get_gpio_in(DEVICE(&s->cpu),
                        BASE_TO_IRQ(NRF51_RNG_BASE)));
 
+    /* UICR, FICR, NVMC, FLASH */
+    object_property_set_uint(OBJECT(&s->nvm), s->flash_size, "flash-size",
+                             &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    object_property_set_bool(OBJECT(&s->nvm), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->nvm), 0);
+    memory_region_add_subregion_overlap(&s->container, NRF51_NVMC_BASE, mr, 0);
+    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->nvm), 1);
+    memory_region_add_subregion_overlap(&s->container, NRF51_FICR_BASE, mr, 0);
+    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->nvm), 2);
+    memory_region_add_subregion_overlap(&s->container, NRF51_UICR_BASE, mr, 0);
+    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->nvm), 3);
+    memory_region_add_subregion_overlap(&s->container, NRF51_FLASH_BASE, mr, 0);
+
     /* GPIO */
     object_property_set_bool(OBJECT(&s->gpio), true, "realized", &err);
     if (err) {
@@ -158,8 +175,6 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
 
     create_unimplemented_device("nrf51_soc.io", NRF51_IOMEM_BASE,
                                 NRF51_IOMEM_SIZE);
-    create_unimplemented_device("nrf51_soc.ficr", NRF51_FICR_BASE,
-                                NRF51_FICR_SIZE);
     create_unimplemented_device("nrf51_soc.private",
                                 NRF51_PRIVATE_BASE, NRF51_PRIVATE_SIZE);
 }
@@ -186,6 +201,8 @@ static void nrf51_soc_init(Object *obj)
     sysbus_init_child_obj(obj, "rng", &s->rng, sizeof(s->rng),
                            TYPE_NRF51_RNG);
 
+    sysbus_init_child_obj(obj, "nvm", &s->nvm, sizeof(s->nvm), TYPE_NRF51_NVM);
+
     sysbus_init_child_obj(obj, "gpio", &s->gpio, sizeof(s->gpio),
                           TYPE_NRF51_GPIO);
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/4] tests/microbit-test: Add tests for nRF51 NVMC
  2019-01-20 14:34 [Qemu-devel] [PATCH 0/4] arm: microbit Non-Volatile Memory Controller Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 3/4] arm: Instantiate NRF51 special NVM's and NVMC Stefan Hajnoczi
@ 2019-01-20 14:34 ` Stefan Hajnoczi
  2019-01-22 16:35   ` Peter Maydell
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2019-01-20 14:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, jim, jusual, Joel Stanley, Steffen Görtz,
	Peter Maydell, qemu-arm

From: Steffen Görtz <contrib@steffen-goertz.de>

Signed-off-by: Steffen Görtz <contrib@steffen-goertz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/microbit-test.c | 97 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/tests/microbit-test.c b/tests/microbit-test.c
index 0c125535f6..d3edd38643 100644
--- a/tests/microbit-test.c
+++ b/tests/microbit-test.c
@@ -20,8 +20,104 @@
 
 #include "hw/arm/nrf51.h"
 #include "hw/gpio/nrf51_gpio.h"
+#include "hw/nvram/nrf51_nvm.h"
 #include "hw/timer/nrf51_timer.h"
 
+#define FLASH_SIZE          (256 * NRF51_PAGE_SIZE)
+
+static void fill_and_erase(hwaddr base, hwaddr size, uint32_t address_reg)
+{
+    hwaddr i;
+
+    /* Erase Page */
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x02);
+    writel(NRF51_NVMC_BASE + address_reg, base);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+
+    /* Check memory */
+    for (i = 0; i < size / 4; i++) {
+        g_assert_cmpuint(readl(base + i * 4), ==, 0xFFFFFFFF);
+    }
+
+    /* Fill memory */
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x01);
+    for (i = 0; i < size / 4; i++) {
+        writel(base + i * 4, i);
+        g_assert_cmpuint(readl(base + i * 4), ==, i);
+    }
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+}
+
+static void test_nrf51_nvmc(void)
+{
+    uint32_t value;
+    hwaddr i;
+
+    /* Test always ready */
+    value = readl(NRF51_NVMC_BASE + NRF51_NVMC_READY);
+    g_assert_cmpuint(value & 0x01, ==, 0x01);
+
+    /* Test write-read config register */
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x03);
+    g_assert_cmpuint(readl(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG), ==, 0x03);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+    g_assert_cmpuint(readl(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG), ==, 0x00);
+
+    /* Test PCR0 */
+    fill_and_erase(NRF51_FLASH_BASE, NRF51_PAGE_SIZE, NRF51_NVMC_ERASEPCR0);
+    fill_and_erase(NRF51_FLASH_BASE + NRF51_PAGE_SIZE,
+                   NRF51_PAGE_SIZE, NRF51_NVMC_ERASEPCR0);
+
+    /* Test PCR1 */
+    fill_and_erase(NRF51_FLASH_BASE, NRF51_PAGE_SIZE, NRF51_NVMC_ERASEPCR1);
+    fill_and_erase(NRF51_FLASH_BASE + NRF51_PAGE_SIZE,
+                   NRF51_PAGE_SIZE, NRF51_NVMC_ERASEPCR1);
+
+    /* Erase all */
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x02);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_ERASEALL, 0x01);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x01);
+    for (i = 0; i < FLASH_SIZE / 4; i++) {
+        writel(NRF51_FLASH_BASE + i * 4, i);
+        g_assert_cmpuint(readl(NRF51_FLASH_BASE + i * 4), ==, i);
+    }
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x02);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_ERASEALL, 0x01);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+
+    for (i = 0; i < FLASH_SIZE / 4; i++) {
+        g_assert_cmpuint(readl(NRF51_FLASH_BASE + i * 4), ==, 0xFFFFFFFF);
+    }
+
+    /* Erase UICR */
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x02);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_ERASEUICR, 0x01);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+
+    for (i = 0; i < NRF51_UICR_SIZE / 4; i++) {
+        g_assert_cmpuint(readl(NRF51_UICR_BASE + i * 4), ==, 0xFFFFFFFF);
+    }
+
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x01);
+    for (i = 0; i < NRF51_UICR_SIZE / 4; i++) {
+        writel(NRF51_UICR_BASE + i * 4, i);
+        g_assert_cmpuint(readl(NRF51_UICR_BASE + i * 4), ==, i);
+    }
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x02);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_ERASEUICR, 0x01);
+    writel(NRF51_NVMC_BASE + NRF51_NVMC_CONFIG, 0x00);
+
+    for (i = 0; i < NRF51_UICR_SIZE / 4; i++) {
+        g_assert_cmpuint(readl(NRF51_UICR_BASE + i * 4), ==, 0xFFFFFFFF);
+    }
+}
+
 static void test_nrf51_gpio(void)
 {
     size_t i;
@@ -246,6 +342,7 @@ int main(int argc, char **argv)
     global_qtest = qtest_initf("-machine microbit");
 
     qtest_add_func("/microbit/nrf51/gpio", test_nrf51_gpio);
+    qtest_add_func("/microbit/nrf51/nvmc", test_nrf51_nvmc);
     qtest_add_func("/microbit/nrf51/timer", test_nrf51_timer);
 
     ret = g_test_run();
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 2/4] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories Stefan Hajnoczi
@ 2019-01-22 16:34   ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2019-01-22 16:34 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: QEMU Developers, Paolo Bonzini, Jim Mussared, Julia Suvorova,
	Joel Stanley, Steffen Görtz, qemu-arm

On Sun, 20 Jan 2019 at 14:35, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> From: Steffen Görtz <contrib@steffen-goertz.de>
>
> The nRF51 contains three regions of non-volatile memory (NVM):
> - CODE (R/W): contains code
> - FICR (R): Factory information like code size, chip id etc.
> - UICR (R/W): Changeable configuration data. Lock bits, Code
>   protection configuration, Bootloader address, Nordic SoftRadio
>   configuration, Firmware configuration.
>
> Read and write access to the memories is managed by the
> Non-volatile memory controller.
>
> Memory schema:
>  [ CPU ] -+- [ NVM, either FICR, UICR or CODE ]
>           |      |
>           \- [ NVMC ]
>
> Signed-off-by: Steffen Görtz <contrib@steffen-goertz.de>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  * Fix device-introspect-test segfault due to missing owner when
>    initializing FICR and UICR memory regions [Peter]
>  * Fix off-by-one assertion checks [Peter]
>  * Fix missing whitespace at end of comment [Peter]
>  * Clear UICR on reset - we'd need a block device for true
>    non-volatility [Peter]
>  * Use memory_region_flush_rom_device() to dirty/invalidate memory
>    [Peter]
> ---


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] tests/microbit-test: Add tests for nRF51 NVMC
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 4/4] tests/microbit-test: Add tests for nRF51 NVMC Stefan Hajnoczi
@ 2019-01-22 16:35   ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2019-01-22 16:35 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: QEMU Developers, Paolo Bonzini, Jim Mussared, Julia Suvorova,
	Joel Stanley, Steffen Görtz, qemu-arm

On Sun, 20 Jan 2019 at 14:35, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> From: Steffen Görtz <contrib@steffen-goertz.de>
>
> Signed-off-by: Steffen Görtz <contrib@steffen-goertz.de>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device()
  2019-01-20 14:34 ` [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device() Stefan Hajnoczi
@ 2019-01-22 16:36   ` Peter Maydell
  2019-01-23  0:34     ` Paolo Bonzini
  2019-01-23 21:07     ` Stefan Hajnoczi
  0 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2019-01-22 16:36 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: QEMU Developers, Paolo Bonzini, Jim Mussared, Julia Suvorova,
	Joel Stanley, Steffen Görtz, qemu-arm

On Sun, 20 Jan 2019 at 14:35, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> ROM devices go via MemoryRegionOps->write() callbacks for write
> operations and do not dirty/invalidate that memory.  Device emulation
> must be able to mark memory ranges that have been modified internally
> (e.g. using memory_region_get_ram_ptr()).
>
> Introduce the memory_region_flush_rom_device() API for this purpose.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  include/exec/memory.h | 18 ++++++++++++++++++
>  exec.c                | 12 ++++++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index cd2f209b64..abe9cc79c0 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1344,6 +1344,24 @@ bool memory_region_snapshot_get_dirty(MemoryRegion *mr,
>  void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
>                                 hwaddr size, unsigned client);
>
> +/**
> + * memory_region_flush_rom_device: Mark a range of pages dirty and invalidate
> + *                                 TBs (for self-modifying code).
> + *
> + * The MemoryRegionOps->write() callback of a ROM device must use this function
> + * to mark byte ranges that have been modified internally, such as by directly
> + * accessing the memory returned by memory_region_get_ram_ptr().
> + *
> + * This function marks the range dirty and invalidates TBs so that TCG can
> + * detect self-modifying code.
> + *
> + * @mr: the region being flushed.
> + * @addr: the start, relative to the start of the region, of the range being
> + *        flushed.
> + * @size: the size, in bytes, of the range being flushed.
> + */
> +void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size);
> +
>  /**
>   * memory_region_set_readonly: Turn a memory region read-only (or read-write)
>   *
> diff --git a/exec.c b/exec.c
> index 895449f926..105ff21e74 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3162,6 +3162,18 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
>      cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
>  }
>
> +void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size)
> +{
> +    /* In principle this function would work on other memory region types too,
> +     * but the ROM device use case is the only one where this operation is
> +     * necessary.  Other memory regions should use the
> +     * address_space_read/write() APIs.
> +     */
> +    assert(memory_region_is_romd(mr));
> +
> +    invalidate_and_set_dirty(mr, addr, size);
> +}
> +
>  static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
>  {
>      unsigned access_size_max = mr->ops->valid.max_access_size;

API and implementation make sense to me, but better that Paolo reviews
this I think. I guess we should add calls to this to the pflash device
models too...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device()
  2019-01-22 16:36   ` Peter Maydell
@ 2019-01-23  0:34     ` Paolo Bonzini
  2019-01-23 21:07     ` Stefan Hajnoczi
  1 sibling, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2019-01-23  0:34 UTC (permalink / raw)
  To: Peter Maydell, Stefan Hajnoczi
  Cc: QEMU Developers, Jim Mussared, Julia Suvorova, Joel Stanley,
	Steffen Görtz, qemu-arm

On 22/01/19 17:36, Peter Maydell wrote:
>> +void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size)
>> +{
>> +    /* In principle this function would work on other memory region types too,
>> +     * but the ROM device use case is the only one where this operation is
>> +     * necessary.  Other memory regions should use the
>> +     * address_space_read/write() APIs.
>> +     */
>> +    assert(memory_region_is_romd(mr));
>> +
>> +    invalidate_and_set_dirty(mr, addr, size);
>> +}
>> +
>>  static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
>>  {
>>      unsigned access_size_max = mr->ops->valid.max_access_size;
> API and implementation make sense to me, but better that Paolo reviews
> this I think. I guess we should add calls to this to the pflash device
> models too...

Yes, I agree.   The implementation makes sense, though maybe we could
just rename invalidate_and_set_dirty to
memory_region_invalidate_and_set_dirty.  Whatever you guys prefer.

Paolo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device()
  2019-01-22 16:36   ` Peter Maydell
  2019-01-23  0:34     ` Paolo Bonzini
@ 2019-01-23 21:07     ` Stefan Hajnoczi
  2019-01-23 21:13       ` Peter Maydell
  1 sibling, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2019-01-23 21:07 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Stefan Hajnoczi, Jim Mussared, Steffen Görtz,
	QEMU Developers, qemu-arm, Joel Stanley, Paolo Bonzini,
	Julia Suvorova

[-- Attachment #1: Type: text/plain, Size: 3256 bytes --]

On Tue, Jan 22, 2019 at 04:36:36PM +0000, Peter Maydell wrote:
> On Sun, 20 Jan 2019 at 14:35, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > ROM devices go via MemoryRegionOps->write() callbacks for write
> > operations and do not dirty/invalidate that memory.  Device emulation
> > must be able to mark memory ranges that have been modified internally
> > (e.g. using memory_region_get_ram_ptr()).
> >
> > Introduce the memory_region_flush_rom_device() API for this purpose.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  include/exec/memory.h | 18 ++++++++++++++++++
> >  exec.c                | 12 ++++++++++++
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index cd2f209b64..abe9cc79c0 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -1344,6 +1344,24 @@ bool memory_region_snapshot_get_dirty(MemoryRegion *mr,
> >  void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
> >                                 hwaddr size, unsigned client);
> >
> > +/**
> > + * memory_region_flush_rom_device: Mark a range of pages dirty and invalidate
> > + *                                 TBs (for self-modifying code).
> > + *
> > + * The MemoryRegionOps->write() callback of a ROM device must use this function
> > + * to mark byte ranges that have been modified internally, such as by directly
> > + * accessing the memory returned by memory_region_get_ram_ptr().
> > + *
> > + * This function marks the range dirty and invalidates TBs so that TCG can
> > + * detect self-modifying code.
> > + *
> > + * @mr: the region being flushed.
> > + * @addr: the start, relative to the start of the region, of the range being
> > + *        flushed.
> > + * @size: the size, in bytes, of the range being flushed.
> > + */
> > +void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size);
> > +
> >  /**
> >   * memory_region_set_readonly: Turn a memory region read-only (or read-write)
> >   *
> > diff --git a/exec.c b/exec.c
> > index 895449f926..105ff21e74 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -3162,6 +3162,18 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
> >      cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
> >  }
> >
> > +void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size)
> > +{
> > +    /* In principle this function would work on other memory region types too,
> > +     * but the ROM device use case is the only one where this operation is
> > +     * necessary.  Other memory regions should use the
> > +     * address_space_read/write() APIs.
> > +     */
> > +    assert(memory_region_is_romd(mr));
> > +
> > +    invalidate_and_set_dirty(mr, addr, size);
> > +}
> > +
> >  static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
> >  {
> >      unsigned access_size_max = mr->ops->valid.max_access_size;
> 
> API and implementation make sense to me, but better that Paolo reviews
> this I think. I guess we should add calls to this to the pflash device
> models too...

Okay, will cover pflash in the next revision.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device()
  2019-01-23 21:07     ` Stefan Hajnoczi
@ 2019-01-23 21:13       ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2019-01-23 21:13 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Stefan Hajnoczi, Jim Mussared, Steffen Görtz,
	QEMU Developers, qemu-arm, Joel Stanley, Paolo Bonzini,
	Julia Suvorova

On Wed, 23 Jan 2019 at 21:07, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> On Tue, Jan 22, 2019 at 04:36:36PM +0000, Peter Maydell wrote:
> > API and implementation make sense to me, but better that Paolo reviews
> > this I think. I guess we should add calls to this to the pflash device
> > models too...
>
> Okay, will cover pflash in the next revision.

We can do that as a separate patch, since it's not related to
the microbit work. For this lot we just need to make a decision
about whether to do this this way or have as Paolo suggested
"memory_region_invalidate_and_set_dirty()". I don't have a
strong opinion, and it sounded like Paolo didn't either, so
let's go with the code you have here.

I can take this patchset via target-arm.next.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-01-23 21:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-20 14:34 [Qemu-devel] [PATCH 0/4] arm: microbit Non-Volatile Memory Controller Stefan Hajnoczi
2019-01-20 14:34 ` [Qemu-devel] [PATCH 1/4] memory: add memory_region_flush_rom_device() Stefan Hajnoczi
2019-01-22 16:36   ` Peter Maydell
2019-01-23  0:34     ` Paolo Bonzini
2019-01-23 21:07     ` Stefan Hajnoczi
2019-01-23 21:13       ` Peter Maydell
2019-01-20 14:34 ` [Qemu-devel] [PATCH 2/4] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories Stefan Hajnoczi
2019-01-22 16:34   ` Peter Maydell
2019-01-20 14:34 ` [Qemu-devel] [PATCH 3/4] arm: Instantiate NRF51 special NVM's and NVMC Stefan Hajnoczi
2019-01-20 14:34 ` [Qemu-devel] [PATCH 4/4] tests/microbit-test: Add tests for nRF51 NVMC Stefan Hajnoczi
2019-01-22 16:35   ` Peter Maydell

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.