[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 05/13] hw/nvram/nrf51_nvm: Add nRF51 non-vola
|
From: |
Peter Maydell |
|
Subject: |
Re: [Qemu-devel] [PATCH v4 05/13] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories |
|
Date: |
Mon, 5 Nov 2018 17:09:41 +0000 |
On 2 November 2018 at 17:07, Steffen Görtz <address@hidden> wrote:
> 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 <address@hidden>
Hi; this mostly looks good; I have a few comments below.
> ---
> hw/nvram/Makefile.objs | 1 +
> hw/nvram/nrf51_nvm.c | 333 +++++++++++++++++++++++++++++++++++
> include/hw/nvram/nrf51_nvm.h | 70 ++++++++
> 3 files changed, 404 insertions(+)
> create mode 100644 hw/nvram/nrf51_nvm.c
> create mode 100644 include/hw/nvram/nrf51_nvm.h
>
> diff --git a/hw/nvram/Makefile.objs b/hw/nvram/Makefile.objs
> index a912d25391..3f978e6212 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/hw/nvram/nrf51_nvm.c b/hw/nvram/nrf51_nvm.c
> new file mode 100644
> index 0000000000..094f7c6f7d
> --- /dev/null
> +++ b/hw/nvram/nrf51_nvm.c
> @@ -0,0 +1,333 @@
> +/*
> + * 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 <address@hidden>
> + *
> + * 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,
Non-standard indentation here -- should be 4-space.
> + 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,
> + .impl.unaligned = false,
unaligned = false is the default.
Should specify an endianness setting here.
(Same also below).
> +};
> +
> +/* 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);
> +
It's worth having the same assert as in uicr_read() here too.
(In fact off-end-of-array writes are worse than reads, so if you
were only going to assert in one place then write is where to do it.
But asserting in both places is best.)
> + 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,
> + .impl.unaligned = false,
> +};
> +
> +
> +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:
> + value &= ~(NRF51_PAGE_SIZE - 1);
> + if (value < (s->code_size * NRF51_PAGE_SIZE)) {
> + address_space_write(&s->as, value, MEMTXATTRS_UNSPECIFIED,
> + s->empty_page, NRF51_PAGE_SIZE);
Does the hardware really ignore failure to write here?
You can check the return of address_space_write() for != MEMTX_OK
to see whether the write attempt failed.
If the hardware doesn't provide any way to notify the guest about
a memory transaction failure, worth commenting to that effect,
so we know later that it isn't a bug that we're not checking.
> + }
> + break;
> + case NRF51_NVMC_ERASEALL:
> + if (value == NRF51_NVMC_ERASE) {
> + for (uint32_t i = 0; i < s->code_size; i++) {
Declaration inside for().
> + address_space_write(&s->as, i * NRF51_PAGE_SIZE,
> + MEMTXATTRS_UNSPECIFIED, s->empty_page,
> + NRF51_PAGE_SIZE);
> + }
> + memset(s->uicr_content, 0xFF, sizeof(s->uicr_content));
> + }
> + 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,
> + .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, NULL, &ficr_ops, s, "nrf51_soc.ficr",
> + sizeof(ficr_content));
> + sysbus_init_mmio(sbd, &s->ficr);
> +
> + memset(s->uicr_content, 0xFF, sizeof(s->uicr_content));
> + memory_region_init_io(&s->uicr, NULL, &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);
> +
> + if (!s->mr) {
> + error_setg(errp, "memory property was not set");
> + return;
> + }
> +
> + s->empty_page = g_malloc(NRF51_PAGE_SIZE);
> + memset(s->empty_page, 0xFF, NRF51_PAGE_SIZE);
> +
> + address_space_init(&s->as, s->mr, "system-memory");
> +}
> +
> +static Property nrf51_nvm_properties[] = {
> + DEFINE_PROP_UINT32("code-size", NRF51NVMState, code_size, 0x100),
> + DEFINE_PROP_LINK("memory", NRF51NVMState, mr, TYPE_MEMORY_REGION,
> + MemoryRegion *),
> + 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;
Missing reset function (at least s->config needs resetting, I think).
> +}
> +
> +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)
> diff --git a/include/hw/nvram/nrf51_nvm.h b/include/hw/nvram/nrf51_nvm.h
> new file mode 100644
> index 0000000000..df55e4027a
> --- /dev/null
> +++ b/include/hw/nvram/nrf51_nvm.h
> @@ -0,0 +1,70 @@
> +/*
> + * 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
> + * + code_size property to set the code size in number of pages.
> + *
> + * Accuracy of the peripheral model:
> + * + The NVMC is always ready, all requested erase operations succeed
> + * immediately.
> + * + CONFIG.WEN and CONFIG.EEN flags can be written and read back
> + * but are not evaluated to check whether a requested write/erase operation
> + * is legal.
> + * + Code regions (MPU configuration) are disregarded.
> + *
> + * Copyright 2018 Steffen Görtz <address@hidden>
> + *
> + * 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;
> +
> + uint32_t uicr_content[NRF51_UICR_FIXTURE_SIZE];
> + uint32_t code_size;
> + uint8_t *empty_page;
> + MemoryRegion *mr;
> + AddressSpace as;
> +
> + uint32_t config;
> +
> +} NRF51NVMState;
> +
> +
> +#endif
> --
> 2.19.1
>
thanks
-- PMM
- [Qemu-devel] [PATCH v4 00/13] arm: nRF51 Devices and Microbit Support, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 03/13] hw/misc/nrf51_rng: Add NRF51 random number generator peripheral, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 01/13] qtest: Add set_irq_in command to set IRQ/GPIO level, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 05/13] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 07/13] tests: Add bbc:microbit / nRF51 test suite, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 08/13] hw/gpio/nrf51_gpio: Add nRF51 GPIO peripheral, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 02/13] arm: Add header to host common definition for nRF51 SOC peripherals, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 04/13] arm: Instantiate NRF51 random number generator, Steffen Görtz, 2018/11/02
- [Qemu-devel] [PATCH v4 13/13] arm: Add Clock peripheral stub to NRF51 SOC, Steffen Görtz, 2018/11/02