Re: [Qemu-devel] [RFC 1/8] arm: NRF51/Microbit Memory container and SOC variants

[Stefan Hajnoczi <stefanha@gmail.com>]

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

On Wed, Jun 27, 2018 at 09:33:44AM +0200, Steffen Görtz wrote:

This looks like code that can be squashed into to Joel's 'microbit'
machine type series.  Please agree with Joel how to include this in his
upcoming v2 patch series.

There are several things going on here and the patch could be split into
several commits with commit descriptions that explain the rationale,
making it easier to review.  But if Joel is happy to work this into his
code as-is then there's no need to worry about structuring this patch
right now.

> Signed-off-by: Steffen Görtz <contrib@steffen-goertz.de>
> ---
>  hw/arm/microbit.c          |  8 ++--
>  hw/arm/nrf51_soc.c         | 98 +++++++++++++++++++++++++++-----------
>  include/hw/arm/nrf51_soc.h | 24 ++++++++--
>  3 files changed, 95 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
> index b61d0747fe..9deb1a36ca 100644
> --- a/hw/arm/microbit.c
> +++ b/hw/arm/microbit.c
> @@ -10,6 +10,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "hw/boards.h"
> +#include "hw/arm/arm.h"
>  
>  #include "hw/arm/nrf51_soc.h"
>  
> @@ -18,10 +19,11 @@ static void microbit_init(MachineState *machine)
>      DeviceState *dev;
>  
>      dev = qdev_create(NULL, TYPE_NRF51_SOC);
> -    if (machine->kernel_filename) {
> -        qdev_prop_set_string(dev, "kernel-filename", machine->kernel_filename);
> -    }
> +    qdev_prop_set_uint32(DEVICE(dev), "VARIANT", NRF51_VARIANT_AA);
>      object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
> +
> +    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> +            0x00000000);
>  }
>  
>  static void microbit_machine_init(MachineClass *mc)
> diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
> index b9f309aa6b..1f7c159edf 100644
> --- a/hw/arm/nrf51_soc.c
> +++ b/hw/arm/nrf51_soc.c
> @@ -22,23 +22,33 @@
>  #include "crypto/random.h"
>  
>  #include "hw/arm/nrf51_soc.h"
> -#include "hw/char/nrf51_uart.h"
>  
>  #define IOMEM_BASE      0x40000000
>  #define IOMEM_SIZE      0x20000000
>  
>  #define FLASH_BASE      0x00000000
> -#define FLASH_SIZE      (256 * 1024)
>  
>  #define FICR_BASE       0x10000000
>  #define FICR_SIZE       0x100
>  
>  #define SRAM_BASE       0x20000000
> -#define SRAM_SIZE       (16 * 1024)
>  
>  #define UART_BASE       0x40002000
>  #define UART_SIZE       0x1000
>  
> +#define PAGE_SIZE       0x0400
> +
> +
> +struct {
> +  hwaddr ram_size;
> +  hwaddr flash_size;
> +} NRF51VariantAttributes[] = {
> +        {.ram_size = 16, .flash_size = 256 },
> +        {.ram_size = 16, .flash_size = 128 },
> +        {.ram_size = 32, .flash_size = 256 },
> +};
> +
> +
>  static uint64_t clock_read(void *opaque, hwaddr addr, unsigned int size)
>  {
>      qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " [%u]\n", __func__, addr, size);
> @@ -101,62 +111,93 @@ static const MemoryRegionOps rng_ops = {
>      .write = rng_write
>  };
>  
> +static void nrf51_soc_init(Object *obj)
> +{
> +    NRF51State *s = NRF51_SOC(obj);
> +
> +    memory_region_init(&s->container, obj, "microbit-container",
> +            UINT64_MAX);
> +
> +    /* TODO: Change to armv6m when cortex-m0 core is available */
> +    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
> +    object_property_add_child(obj, "armv7m", OBJECT(&s->armv7m), &error_abort);
> +    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
> +    qdev_prop_set_string(DEVICE(&s->armv7m), "cpu-type",
> +                         ARM_CPU_TYPE_NAME("cortex-m3"));
> +
> +    object_initialize(&s->uart, sizeof(s->uart), TYPE_NRF51_UART);
> +    object_property_add_child(obj, "uart", OBJECT(&s->uart), &error_abort);
> +    qdev_set_parent_bus(DEVICE(&s->uart), sysbus_get_default());
> +}
> +
>  
>  static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
>  {
>      NRF51State *s = NRF51_SOC(dev_soc);
>      Error *err = NULL;
>  
> -    /* IO space */
> -    create_unimplemented_device("nrf51_soc.io", IOMEM_BASE, IOMEM_SIZE);
> -
> -    /* FICR */
> -    create_unimplemented_device("nrf51_soc.ficr", FICR_BASE, FICR_SIZE);
> +    if (!(s->part_variant > NRF51_VARIANT_INVALID
> +            && s->part_variant < NRF51_VARIANT_MAX)) {
> +        error_setg(errp, "VARIANT not set or invalid");
> +        return;
> +    }
>  
> -    MemoryRegion *system_memory = get_system_memory();
> -    MemoryRegion *sram = g_new(MemoryRegion, 1);
> -    MemoryRegion *flash = g_new(MemoryRegion, 1);
> +    memory_region_init_ram(&s->sram, NULL, "nrf51_soc.sram",
> +            NRF51VariantAttributes[s->part_variant].ram_size * PAGE_SIZE, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    memory_region_add_subregion(&s->container, SRAM_BASE, &s->sram);
>  
> -    memory_region_init_ram_nomigrate(flash, NULL, "nrf51.flash", FLASH_SIZE,
> +    memory_region_init_ram(&s->flash, NULL, "nrf51_soc.flash",
> +            NRF51VariantAttributes[s->part_variant].flash_size * PAGE_SIZE,
>              &err);
>      if (err) {
>          error_propagate(errp, err);
>          return;
>      }
> +    memory_region_add_subregion(&s->container, FLASH_BASE, &s->flash);
>  
> -    vmstate_register_ram_global(flash);
> -    memory_region_set_readonly(flash, true);
> +    qdev_prop_set_uint32(DEVICE(&s->armv7m), "num-irq", 60);
> +    object_property_set_link(OBJECT(&s->armv7m), OBJECT(&s->container),
> +                                         "memory", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
>  
> -    memory_region_add_subregion(system_memory, FLASH_BASE, flash);
>  
> -    memory_region_init_ram_nomigrate(sram, NULL, "nrf51.sram", SRAM_SIZE,
> -            &err);
> +    object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
>      if (err) {
>          error_propagate(errp, err);
>          return;
>      }
> -    vmstate_register_ram_global(sram);
> -    memory_region_add_subregion(system_memory, SRAM_BASE, sram);
>  
> -    /* TODO: implement a cortex m0 and update this */
> -    s->nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
> -               s->kernel_filename, ARM_CPU_TYPE_NAME("cortex-m3"));
> +    /* IO space */
> +    create_unimplemented_device("nrf51_soc.io", IOMEM_BASE, IOMEM_SIZE);
> +
> +    /* FICR */
> +    create_unimplemented_device("nrf51_soc.ficr", FICR_BASE, FICR_SIZE);
>  
> -    s->uart = nrf51_uart_create(UART_BASE, qdev_get_gpio_in(s->nvic, 2),
> -                                serial_hd(0));
> +    qdev_prop_set_chr(DEVICE(&s->uart), "chardev", serial_hd(0));
> +    qdev_init_nofail(DEVICE(&s->uart));
> +/*    sysbus_mmio_map(s, 0, UART_BASE);
> +    sysbus_connect_irq(s, 0, qdev_get_gpio_in(s->nvic, 2)); */
>  
>      memory_region_init_io(&s->clock, NULL, &clock_ops, NULL, "nrf51_soc.clock", 0x1000);
> -    memory_region_add_subregion_overlap(get_system_memory(), IOMEM_BASE, &s->clock, -1);
> +    memory_region_add_subregion_overlap(&s->container, IOMEM_BASE, &s->clock, -1);
>  
>      memory_region_init_io(&s->nvmc, NULL, &nvmc_ops, NULL, "nrf51_soc.nvmc", 0x1000);
> -    memory_region_add_subregion_overlap(get_system_memory(), 0x4001E000, &s->nvmc, -1);
> +    memory_region_add_subregion_overlap(&s->container, 0x4001E000, &s->nvmc, -1);
>  
>      memory_region_init_io(&s->rng, NULL, &rng_ops, NULL, "nrf51_soc.rng", 0x1000);
> -    memory_region_add_subregion_overlap(get_system_memory(), 0x4000D000, &s->rng, -1);
> +    memory_region_add_subregion_overlap(&s->container, 0x4000D000, &s->rng, -1);
>  }
>  
>  static Property nrf51_soc_properties[] = {
> -    DEFINE_PROP_STRING("kernel-filename", NRF51State, kernel_filename),
> +    DEFINE_PROP_INT32("VARIANT", NRF51State, part_variant,
> +            NRF51_VARIANT_INVALID),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -172,6 +213,7 @@ static const TypeInfo nrf51_soc_info = {
>      .name          = TYPE_NRF51_SOC,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(NRF51State),
> +    .instance_init = nrf51_soc_init,
>      .class_init    = nrf51_soc_class_init,
>  };
>  
> diff --git a/include/hw/arm/nrf51_soc.h b/include/hw/arm/nrf51_soc.h
> index f81cbbe3ce..c9af9659e9 100644
> --- a/include/hw/arm/nrf51_soc.h
> +++ b/include/hw/arm/nrf51_soc.h
> @@ -12,6 +12,8 @@
>  
>  #include "qemu/osdep.h"
>  #include "hw/sysbus.h"
> +#include "hw/arm/armv7m.h"
> +#include "hw/char/nrf51_uart.h"
>  
>  #define TYPE_NRF51_SOC "nrf51-soc"
>  #define NRF51_SOC(obj) \
> @@ -22,16 +24,30 @@ typedef struct NRF51State {
>      SysBusDevice parent_obj;
>  
>      /*< public >*/
> -    char *kernel_filename;
> -    DeviceState *nvic;
> -    DeviceState *uart;
> +    /* TODO: Change to armv6m when cortex-m0 core is available */
> +    ARMv7MState armv7m;
>  
> -    MemoryRegion iomem;
> +    Nrf51UART uart;
>  
> +    MemoryRegion container;
> +    MemoryRegion sram;
> +    MemoryRegion flash;
> +    MemoryRegion iomem;
>      MemoryRegion clock;
>      MemoryRegion nvmc;
>      MemoryRegion rng;
> +
> +    /* Properties */
> +    int32_t part_variant;
>  } NRF51State;
>  
> +typedef enum {
> +    NRF51_VARIANT_INVALID = -1,
> +    NRF51_VARIANT_AA = 0,
> +    NRF51_VARIANT_AB = 1,
> +    NRF51_VARIANT_AC = 2,
> +    NRF51_VARIANT_MAX = 3
> +} NRF51Variants;
> +
>  #endif
>  
> -- 
> 2.17.1
> 

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