[Qemu-devel] [RFC v5 0/5] hw/arm: add initial support for Canon DIGIC SoC

[Qemu-devel] [RFC v5 0/5] hw/arm: add initial support for Canon DIGIC SoC

[Antony Pavlov]

[RFC v5 1/5] hw/arm: add very initial support for Canon DIGIC SoC
[RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support
[RFC v5 3/5] hw/arm/digic: add timer support
[RFC v5 4/5] hw/arm/digic: add UART support
[RFC v5 5/5] hw/arm/digic: add NOR ROM support

Changes since v4:
 1. digic.h: parent_obj: change type Object -> DeviceState
 2. digic-uart: drop reg array
 3. digic_boards: fix K8P3215UQB comment
 4. Makefile: place digic stuff in own line
 5. drop cpu-qom.h inclusion
 6. digic.h: add private/public labels
 7. digic.h: fix guard macro
 8. move base address macros to digic.c
 9. fix header comments

Changes since v3:
 1. fix typos and formatting
 2. digic-timer: drop DPRINTF
 3. digic-timer: fix DIGIC4_TIMER_BASE() macro
 4. digic.c: fix max timer device string

Changes since v2:
 1. rebase over latest master;
   * pass available size to object_initialize().
 2. digic-uart: qemu_log: use LOG_UNIMP instead LOG_GUEST_ERROR;
 3. digic-boards: update rom image load code: introduce digic_load_rom().

Changes since v1:
 0. drop the "add ARM946E-S CPU" patch;
 1. convert to QOM, split DIGIC SoC code and board code
    (thanks to Andreas Fa:rber, Peter Maydell and Peter Crosthwaite);
 2. fix digic-uart (many thanks to Peter Crosthwaite
    for his comments);
 3. digic-boards: digic4_add_k8p3215uqb_rom(): update
    rom image load code: use the '-bios' option.

DIGIC is Canon Inc.'s name for a family of SoC
for digital cameras and camcorders.

See http://en.wikipedia.org/wiki/DIGIC for details.

There is no publicly available specification for
DIGIC chips. All information about DIGIC chip
internals is based on reverse engineering efforts
made by CHDK (http://chdk.wikia.com) and
Magic Lantern (http://www.magiclantern.fm) projects
contributors.

Also this patch series adds initial support for Canon
PowerShot A1100 IS compact camera (it is my only camera
with connected UART interface). As the DIGIC-based cameras
differences mostly are unsignificant (e.g. RAM-size,
ROM type and size, GPIO usage) the other compact
and DSLR cameras support can be easely added.

This DIGIC support patch series is inspired
by EOS QEMU from Magic Lantern project.
The main differences:
 * EOS QEMU uses home-brew all-in-one monolith design;
 this patch series uses conventional qemu object-centric design;
 * EOS QEMU tries provide simplest emulation for most
 controllers inside SoC to run Magic Lantern firmware;
 this patch series provide more complete support
 only for core devices to run barebox bootloader.
  ** EOS QEMU does not support timer counting
  (this patch series emulate 1 MHz counting);
  ** EOS QEMU support DIGIC UART only for output
  character to stderr; (this patch series emulate
  introduces full blown UART interface);
  ** EOS QEMU has incomplete ROM support;
  (this patch series uses conventional qemu pflash).

This initial DIGIC support can't be used to run
the original camera firmware, but it can successfully
run experimental version of barebox bootloader
(see http://www.barebox.org).

The last sources of barebox for PowerShot A1100 can be
obtained here:
  https://github.com/frantony/barebox/tree/next.digic.20130829

The precompiled ROM image usable with qemu can be
obtained here:
  https://github.com/frantony/barebox/blob/next.digic.20130829/canon-a1100-rom1.bin

This ROM image (after "dancing bit" encoding) can be run on
real Canon A1100 camera.

The short build instruction for __previous__ DIGIC barebox
version (it can be used with more recent sources too) can
be obtained here:
  http://lists.infradead.org/pipermail/barebox/2013-August/016007.html

[Qemu-devel] [RFC v5 1/5] hw/arm: add very initial support for Canon DIGIC SoC

[Antony Pavlov]

DIGIC is Canon Inc.'s name for a family of SoC
for digital cameras and camcorders.

There is no publicly available specification for
DIGIC chips. All information about DIGIC chip
internals is based on reverse engineering efforts
made by CHDK (http://chdk.wikia.com) and
Magic Lantern (http://www.magiclantern.fm) projects
contributors.

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 default-configs/arm-softmmu.mak |  1 +
 hw/arm/Makefile.objs            |  1 +
 hw/arm/digic.c                  | 65 +++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/digic.h          | 35 ++++++++++++++++++++++
 4 files changed, 102 insertions(+)
 create mode 100644 hw/arm/digic.c
 create mode 100644 include/hw/arm/digic.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index ac0815d..0d1d783 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -63,6 +63,7 @@ CONFIG_FRAMEBUFFER=y
 CONFIG_XILINX_SPIPS=y
 
 CONFIG_A9SCU=y
+CONFIG_DIGIC=y
 CONFIG_MARVELL_88W8618=y
 CONFIG_OMAP=y
 CONFIG_TSC210X=y
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 3671b42..eb548dd 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -4,4 +4,5 @@ obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
+obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
diff --git a/hw/arm/digic.c b/hw/arm/digic.c
new file mode 100644
index 0000000..0d38872
--- /dev/null
+++ b/hw/arm/digic.c
@@ -0,0 +1,65 @@
+/*
+ * QEMU model of the Canon DIGIC SoC.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include "hw/arm/digic.h"
+
+static void digic_init(Object *obj)
+{
+    DigicState *s = DIGIC(obj);
+
+    object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
+    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+}
+
+static void digic_realize(DeviceState *dev, Error **errp)
+{
+    DigicState *s = DIGIC(dev);
+    Error *err = NULL;
+
+    object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+}
+
+static void digic_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = digic_realize;
+}
+
+static const TypeInfo digic_type_info = {
+    .name = TYPE_DIGIC,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(DigicState),
+    .instance_init = digic_init,
+    .class_init = digic_class_init,
+};
+
+static void digic_register_types(void)
+{
+    type_register_static(&digic_type_info);
+}
+
+type_init(digic_register_types)
diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
new file mode 100644
index 0000000..b7d16fb
--- /dev/null
+++ b/include/hw/arm/digic.h
@@ -0,0 +1,35 @@
+/*
+ * Misc Canon DIGIC declarations.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef HW_ARM_DIGIC_H
+#define HW_ARM_DIGIC_H
+
+#include "cpu.h"
+
+#define TYPE_DIGIC "digic"
+
+#define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
+
+typedef struct DigicState {
+    /*< private >*/
+    DeviceState parent_obj;
+    /*< public >*/
+
+    ARMCPU cpu;
+} DigicState;
+
+#endif /* HW_ARM_DIGIC_H */
-- 
1.8.4.rc3

[Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Antony Pavlov]

Also this patch adds initial support for Canon
PowerShot A1100 IS compact camera.

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/Makefile.objs  |  1 +
 hw/arm/digic_boards.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)
 create mode 100644 hw/arm/digic_boards.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index eb548dd..69a8de5 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,4 +1,5 @@
 obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
+obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
new file mode 100644
index 0000000..77cfc81
--- /dev/null
+++ b/hw/arm/digic_boards.c
@@ -0,0 +1,88 @@
+/*
+ * QEMU model of the Canon DIGIC boards (cameras indeed :).
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * See docs here:
+ *   http://magiclantern.wikia.com/wiki/Register_Map
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include "hw/boards.h"
+#include "exec/address-spaces.h"
+#include "hw/arm/digic.h"
+
+typedef struct DigicBoardState {
+    DigicState *digic;
+    MemoryRegion ram;
+} DigicBoardState;
+
+typedef struct DigicBoard {
+    hwaddr ram_size;
+    hwaddr start_addr;
+} DigicBoard;
+
+static void digic4_board_setup_ram(DigicBoardState *s, hwaddr ram_size)
+{
+    memory_region_init_ram(&s->ram, NULL, "ram", ram_size);
+    memory_region_add_subregion(get_system_memory(), 0, &s->ram);
+    vmstate_register_ram_global(&s->ram);
+}
+
+static void digic4_board_init(DigicBoard *board)
+{
+    Error *err = NULL;
+
+    DigicBoardState *s = g_new(DigicBoardState, 1);
+
+    s->digic = DIGIC(object_new(TYPE_DIGIC));
+    object_property_set_bool(OBJECT(s->digic), true, "realized", &err);
+    if (err != NULL) {
+        fprintf(stderr, "Couldn't realize DIGIC SoC: %s\n",
+                error_get_pretty(err));
+        exit(1);
+    }
+
+    digic4_board_setup_ram(s, board->ram_size);
+
+    s->digic->cpu.env.regs[15] = board->start_addr;
+}
+
+static DigicBoard digic4_board_canon_a1100 = {
+    .ram_size = 64 * 1024 * 1024,
+    /* CHDK recommends this address for ROM disassembly */
+    .start_addr = 0xffc00000,
+};
+
+static void canon_a1100_init(QEMUMachineInitArgs *args)
+{
+    digic4_board_init(&digic4_board_canon_a1100);
+}
+
+static QEMUMachine canon_a1100 = {
+    .name = "canon-a1100",
+    .desc = "Canon PowerShot A1100 IS",
+    .init = &canon_a1100_init,
+};
+
+static void digic_register_machines(void)
+{
+    qemu_register_machine(&canon_a1100);
+}
+
+machine_init(digic_register_machines)
-- 
1.8.4.rc3

[Qemu-devel] [RFC v5 3/5] hw/arm/digic: add timer support

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/digic.c         |  28 ++++++++++++
 hw/timer/Makefile.objs |   1 +
 hw/timer/digic-timer.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/timer/digic-timer.h |  36 ++++++++++++++++
 include/hw/arm/digic.h |   6 +++
 5 files changed, 185 insertions(+)
 create mode 100644 hw/timer/digic-timer.c
 create mode 100644 hw/timer/digic-timer.h

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 0d38872..4a8e67a 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -22,24 +22,52 @@
 
 #include "hw/arm/digic.h"
 
+#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + (n) * 0x100)
+
 static void digic_init(Object *obj)
 {
     DigicState *s = DIGIC(obj);
+    DeviceState *dev;
+    int i;
 
     object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
     object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+
+    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
+#define DIGIC_TIMER_NAME_MLEN    11
+        char name[DIGIC_TIMER_NAME_MLEN];
+
+        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
+        dev = DEVICE(&s->timer[i]);
+        qdev_set_parent_bus(dev, sysbus_get_default());
+        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
+        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
+    }
 }
 
 static void digic_realize(DeviceState *dev, Error **errp)
 {
     DigicState *s = DIGIC(dev);
     Error *err = NULL;
+    SysBusDevice *sbd;
+    int i;
 
     object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
     if (err != NULL) {
         error_propagate(errp, err);
         return;
     }
+
+    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
+        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+
+        sbd = SYS_BUS_DEVICE(&s->timer[i]);
+        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
+    }
 }
 
 static void digic_class_init(ObjectClass *oc, void *data)
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index eca5905..5479aee 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -25,5 +25,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
 obj-$(CONFIG_SH4) += sh_timer.o
 obj-$(CONFIG_TUSB6010) += tusb6010.o
+obj-$(CONFIG_DIGIC) += digic-timer.o
 
 obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
new file mode 100644
index 0000000..8f5c62a
--- /dev/null
+++ b/hw/timer/digic-timer.c
@@ -0,0 +1,114 @@
+/*
+ * QEMU model of the Canon DIGIC timer block.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * See "Timer/Clock Module" docs here:
+ *   http://magiclantern.wikia.com/wiki/Register_Map
+ *
+ * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
+ * is used as a template.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "hw/ptimer.h"
+#include "qemu/main-loop.h"
+
+#include "hw/timer/digic-timer.h"
+
+# define DIGIC_TIMER_CONTROL 0x00
+# define DIGIC_TIMER_VALUE 0x0c
+
+static uint64_t digic_timer_read(void *opaque, hwaddr offset, unsigned size)
+{
+    DigicTimerState *s = opaque;
+    uint32_t ret = 0;
+
+    switch (offset) {
+    case DIGIC_TIMER_VALUE:
+        ret = (uint32_t)ptimer_get_count(s->ptimer);
+        ret &= 0xffff;
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                      "digic-timer: read access to unknown register 0x"
+                      TARGET_FMT_plx, offset);
+    }
+
+    return ret;
+}
+
+static void digic_timer_write(void *opaque, hwaddr offset,
+                              uint64_t value, unsigned size)
+{
+    DigicTimerState *s = opaque;
+
+    /* FIXME: without documentation every write just starts timer */
+    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
+    ptimer_run(s->ptimer, 1);
+}
+
+static const MemoryRegionOps digic_timer_ops = {
+    .read = digic_timer_read,
+    .write = digic_timer_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void digic_timer_tick(void *opaque)
+{
+    DigicTimerState *s = opaque;
+
+    ptimer_run(s->ptimer, 1);
+}
+
+static void digic_timer_init(Object *obj)
+{
+    DigicTimerState *s = DIGIC_TIMER(obj);
+
+    s->bh = qemu_bh_new(digic_timer_tick, s);
+    s->ptimer = ptimer_init(s->bh);
+
+    /*
+     * FIXME: there is no documentation on Digic timer
+     * frequency setup so let it always run at 1 MHz
+     */
+    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
+                          TYPE_DIGIC_TIMER, 0x100);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static const TypeInfo digic_timer_info = {
+    .name = TYPE_DIGIC_TIMER,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(DigicTimerState),
+    .instance_init = digic_timer_init,
+};
+
+static void digic_timer_register_type(void)
+{
+    type_register_static(&digic_timer_info);
+}
+
+type_init(digic_timer_register_type)
diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
new file mode 100644
index 0000000..daf271d
--- /dev/null
+++ b/hw/timer/digic-timer.h
@@ -0,0 +1,36 @@
+/*
+ * Canon DIGIC timer block declarations.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef HW_TIMER_DIGIC_TIMER_H
+#define HW_TIMER_DIGIC_TIMER_H
+
+#include "hw/sysbus.h"
+#include "qemu/typedefs.h"
+#include "hw/ptimer.h"
+
+#define TYPE_DIGIC_TIMER "digic-timer"
+#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
+
+typedef struct DigicTimerState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    QEMUBH *bh;
+    ptimer_state *ptimer;
+} DigicTimerState;
+
+#endif /* HW_TIMER_DIGIC_TIMER_H */
diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
index b7d16fb..177a06d 100644
--- a/include/hw/arm/digic.h
+++ b/include/hw/arm/digic.h
@@ -20,16 +20,22 @@
 
 #include "cpu.h"
 
+#include "hw/timer/digic-timer.h"
+
 #define TYPE_DIGIC "digic"
 
 #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
 
+#define DIGIC4_NB_TIMERS 3
+
 typedef struct DigicState {
     /*< private >*/
     DeviceState parent_obj;
     /*< public >*/
 
     ARMCPU cpu;
+
+    DigicTimerState timer[DIGIC4_NB_TIMERS];
 } DigicState;
 
 #endif /* HW_ARM_DIGIC_H */
-- 
1.8.4.rc3

[Qemu-devel] [RFC v5 4/5] hw/arm/digic: add UART support

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/digic.c         |  16 ++++
 hw/arm/digic_boards.c  |   7 ++
 hw/char/Makefile.objs  |   1 +
 hw/char/digic-uart.c   | 195 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/char/digic-uart.h   |  45 ++++++++++++
 include/hw/arm/digic.h |   2 +
 6 files changed, 266 insertions(+)
 create mode 100644 hw/char/digic-uart.c
 create mode 100644 hw/char/digic-uart.h

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 4a8e67a..f2c7a8f 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -24,6 +24,8 @@
 
 #define DIGIC4_TIMER_BASE(n)    (0xc0210000 + (n) * 0x100)
 
+#define DIGIC_UART_BASE          0xc0800000
+
 static void digic_init(Object *obj)
 {
     DigicState *s = DIGIC(obj);
@@ -43,6 +45,11 @@ static void digic_init(Object *obj)
         snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
         object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
     }
+
+    object_initialize(&s->uart, sizeof(s->uart), TYPE_DIGIC_UART);
+    dev = DEVICE(&s->uart);
+    qdev_set_parent_bus(dev, sysbus_get_default());
+    object_property_add_child(obj, "uart", OBJECT(&s->uart), NULL);
 }
 
 static void digic_realize(DeviceState *dev, Error **errp)
@@ -68,6 +75,15 @@ static void digic_realize(DeviceState *dev, Error **errp)
         sbd = SYS_BUS_DEVICE(&s->timer[i]);
         sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
     }
+
+    object_property_set_bool(OBJECT(&s->uart), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    sbd = SYS_BUS_DEVICE(&s->uart);
+    sysbus_mmio_map(sbd, 0, DIGIC_UART_BASE);
 }
 
 static void digic_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 77cfc81..cced229 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -26,6 +26,13 @@
 #include "hw/boards.h"
 #include "exec/address-spaces.h"
 #include "hw/arm/digic.h"
+#include "hw/block/flash.h"
+#include "hw/loader.h"
+#include "sysemu/sysemu.h"
+
+#define DIGIC4_ROM0_BASE      0xf0000000
+#define DIGIC4_ROM1_BASE      0xf8000000
+# define DIGIC4_ROM_MAX_SIZE      0x08000000
 
 typedef struct DigicBoardState {
     DigicState *digic;
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index f8f3dbc..00d37ac 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -14,6 +14,7 @@ obj-$(CONFIG_COLDFIRE) += mcf_uart.o
 obj-$(CONFIG_OMAP) += omap_uart.o
 obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
+obj-$(CONFIG_DIGIC) += digic-uart.o
 
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
diff --git a/hw/char/digic-uart.c b/hw/char/digic-uart.c
new file mode 100644
index 0000000..fd8e077
--- /dev/null
+++ b/hw/char/digic-uart.c
@@ -0,0 +1,195 @@
+/*
+ * QEMU model of the Canon DIGIC UART block.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * See "Serial terminal" docs here:
+ *   http://magiclantern.wikia.com/wiki/Register_Map#Misc_Registers
+ *
+ * The QEMU model of the Milkymist UART block by Michael Walle
+ * is used as a template.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include "hw/hw.h"
+#include "hw/sysbus.h"
+#include "sysemu/char.h"
+
+#include "hw/char/digic-uart.h"
+
+enum {
+    ST_RX_RDY = (1 << 0),
+    ST_TX_RDY = (1 << 1),
+};
+
+static uint64_t digic_uart_read(void *opaque, hwaddr addr,
+                                unsigned size)
+{
+    DigicUartState *s = opaque;
+    uint64_t ret = 0;
+
+    addr >>= 2;
+
+    switch (addr) {
+    case R_RX:
+        s->reg_st &= ~(ST_RX_RDY);
+        ret = s->reg_rx;
+        break;
+
+    case R_ST:
+        ret = s->reg_st;
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                      "digic-uart: read access to unknown register 0x"
+                      TARGET_FMT_plx, addr << 2);
+    }
+
+    return ret;
+}
+
+static void digic_uart_write(void *opaque, hwaddr addr, uint64_t value,
+                             unsigned size)
+{
+    DigicUartState *s = opaque;
+    unsigned char ch = value;
+
+    addr >>= 2;
+
+    switch (addr) {
+    case R_TX:
+        if (s->chr) {
+            qemu_chr_fe_write_all(s->chr, &ch, 1);
+        }
+        break;
+
+    case R_ST:
+        /*
+         * Ignore write to R_ST.
+         *
+         * The point is that this register is actively used
+         * during receiving and transmitting symbols,
+         * but we don't know the function of most of bits.
+         *
+         * Ignoring writes to R_ST is only a simplification
+         * of the model. It has no perceptible side effects
+         * for existing guests.
+         */
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                      "digic-uart: write access to unknown register 0x"
+                      TARGET_FMT_plx, addr << 2);
+    }
+}
+
+static const MemoryRegionOps uart_mmio_ops = {
+    .read = digic_uart_read,
+    .write = digic_uart_write,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int uart_can_rx(void *opaque)
+{
+    DigicUartState *s = opaque;
+
+    return !(s->reg_st & ST_RX_RDY);
+}
+
+static void uart_rx(void *opaque, const uint8_t *buf, int size)
+{
+    DigicUartState *s = opaque;
+
+    assert(uart_can_rx(opaque));
+
+    s->reg_st |= ST_RX_RDY;
+    s->reg_rx = *buf;
+}
+
+static void uart_event(void *opaque, int event)
+{
+}
+
+static void digic_uart_reset(DeviceState *d)
+{
+    DigicUartState *s = DIGIC_UART(d);
+
+    s->reg_rx = 0;
+    s->reg_st = ST_TX_RDY;
+}
+
+static void digic_uart_realize(DeviceState *dev, Error **errp)
+{
+    DigicUartState *s = DIGIC_UART(dev);
+
+    s->chr = qemu_char_get_next_serial();
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
+    }
+}
+
+static void digic_uart_init(Object *obj)
+{
+    DigicUartState *s = DIGIC_UART(obj);
+
+    memory_region_init_io(&s->regs_region, OBJECT(s), &uart_mmio_ops, s,
+                          TYPE_DIGIC_UART, 0x18);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->regs_region);
+}
+
+static const VMStateDescription vmstate_digic_uart = {
+    .name = "digic-uart",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(reg_rx, DigicUartState),
+        VMSTATE_UINT32(reg_st, DigicUartState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void digic_uart_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = digic_uart_realize;
+    dc->reset = digic_uart_reset;
+    dc->vmsd = &vmstate_digic_uart;
+}
+
+static const TypeInfo digic_uart_info = {
+    .name = TYPE_DIGIC_UART,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(DigicUartState),
+    .instance_init = digic_uart_init,
+    .class_init = digic_uart_class_init,
+};
+
+static void digic_uart_register_types(void)
+{
+    type_register_static(&digic_uart_info);
+}
+
+type_init(digic_uart_register_types)
diff --git a/hw/char/digic-uart.h b/hw/char/digic-uart.h
new file mode 100644
index 0000000..2a333e7
--- /dev/null
+++ b/hw/char/digic-uart.h
@@ -0,0 +1,45 @@
+/*
+ * Canon DIGIC UART block declarations.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef HW_CHAR_DIGIC_UART_H
+#define HW_CHAR_DIGIC_UART_H
+
+#include "hw/sysbus.h"
+#include "qemu/typedefs.h"
+
+#define TYPE_DIGIC_UART "digic-uart"
+#define DIGIC_UART(obj) \
+    OBJECT_CHECK(DigicUartState, (obj), TYPE_DIGIC_UART)
+
+enum {
+    R_TX = 0x00,
+    R_RX,
+    R_ST = (0x14 >> 2),
+    R_MAX
+};
+
+typedef struct DigicUartState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion regs_region;
+    CharDriverState *chr;
+
+    uint32_t reg_rx;
+    uint32_t reg_st;
+} DigicUartState;
+
+#endif /* HW_CHAR_DIGIC_UART_H */
diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
index 177a06d..a739d6a 100644
--- a/include/hw/arm/digic.h
+++ b/include/hw/arm/digic.h
@@ -21,6 +21,7 @@
 #include "cpu.h"
 
 #include "hw/timer/digic-timer.h"
+#include "hw/char/digic-uart.h"
 
 #define TYPE_DIGIC "digic"
 
@@ -36,6 +37,7 @@ typedef struct DigicState {
     ARMCPU cpu;
 
     DigicTimerState timer[DIGIC4_NB_TIMERS];
+    DigicUartState uart;
 } DigicState;
 
 #endif /* HW_ARM_DIGIC_H */
-- 
1.8.4.rc3

[Qemu-devel] [RFC v5 5/5] hw/arm/digic: add NOR ROM support

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/digic_boards.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index cced229..a12b673 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -41,6 +41,10 @@ typedef struct DigicBoardState {
 
 typedef struct DigicBoard {
     hwaddr ram_size;
+    void (*add_rom0)(DigicBoardState *, hwaddr, const char *);
+    const char *rom0_def_filename;
+    void (*add_rom1)(DigicBoardState *, hwaddr, const char *);
+    const char *rom1_def_filename;
     hwaddr start_addr;
 } DigicBoard;
 
@@ -67,11 +71,71 @@ static void digic4_board_init(DigicBoard *board)
 
     digic4_board_setup_ram(s, board->ram_size);
 
+    if (board->add_rom0) {
+        board->add_rom0(s, DIGIC4_ROM0_BASE, board->rom0_def_filename);
+    }
+
+    if (board->add_rom1) {
+        board->add_rom1(s, DIGIC4_ROM1_BASE, board->rom1_def_filename);
+    }
+
     s->digic->cpu.env.regs[15] = board->start_addr;
 }
 
+static void digic_load_rom(DigicBoardState *s, hwaddr addr,
+                           hwaddr max_size, const char *def_filename)
+{
+
+    target_long rom_size;
+    const char *filename;
+
+    if (bios_name) {
+        filename = bios_name;
+    } else {
+        filename = def_filename;
+    }
+
+    if (filename) {
+        char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, filename);
+
+        if (!fn) {
+            fprintf(stderr, "Couldn't find rom image '%s'.\n", filename);
+            exit(1);
+        }
+
+        rom_size = load_image_targphys(fn, addr, max_size);
+        if (rom_size < 0 || rom_size > max_size) {
+            fprintf(stderr, "Couldn't load rom image '%s'\n", filename);
+            exit(1);
+        }
+    }
+}
+
+/*
+ * Samsung K8P3215UQB
+ * 64M Bit (4Mx16) Page Mode / Multi-Bank NOR Flash Memory
+ */
+static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, hwaddr addr,
+                                      const char *def_filename)
+{
+#define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024)
+#define FLASH_K8P3215UQB_SECTOR_SIZE (64 * 1024)
+
+    pflash_cfi02_register(addr, NULL, "pflash", FLASH_K8P3215UQB_SIZE,
+                          NULL, FLASH_K8P3215UQB_SECTOR_SIZE,
+                          FLASH_K8P3215UQB_SIZE / FLASH_K8P3215UQB_SECTOR_SIZE,
+                          DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE,
+                          4,
+                          0x00EC, 0x007E, 0x0003, 0x0001,
+                          0x0555, 0x2aa, 0);
+
+    digic_load_rom(s, addr, FLASH_K8P3215UQB_SIZE, def_filename);
+}
+
 static DigicBoard digic4_board_canon_a1100 = {
     .ram_size = 64 * 1024 * 1024,
+    .add_rom1 = digic4_add_k8p3215uqb_rom,
+    .rom1_def_filename = "canon-a1100-rom1.bin",
     /* CHDK recommends this address for ROM disassembly */
     .start_addr = 0xffc00000,
 };
-- 
1.8.4.rc3

Re: [Qemu-devel] [RFC v5 0/5] hw/arm: add initial support for Canon DIGIC SoC

[Antony Pavlov]

On Sat,  7 Sep 2013 11:04:22 +0400
Antony Pavlov <antonynpavlov@gmail.com> wrote:

ping
> [RFC v5 1/5] hw/arm: add very initial support for Canon DIGIC SoC
> [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support
> [RFC v5 3/5] hw/arm/digic: add timer support
> [RFC v5 4/5] hw/arm/digic: add UART support
> [RFC v5 5/5] hw/arm/digic: add NOR ROM support
> 
> Changes since v4:
>  1. digic.h: parent_obj: change type Object -> DeviceState
>  2. digic-uart: drop reg array
>  3. digic_boards: fix K8P3215UQB comment
>  4. Makefile: place digic stuff in own line
>  5. drop cpu-qom.h inclusion
>  6. digic.h: add private/public labels
>  7. digic.h: fix guard macro
>  8. move base address macros to digic.c
>  9. fix header comments
> 
> Changes since v3:
>  1. fix typos and formatting
>  2. digic-timer: drop DPRINTF
>  3. digic-timer: fix DIGIC4_TIMER_BASE() macro
>  4. digic.c: fix max timer device string
> 
> Changes since v2:
>  1. rebase over latest master;
>    * pass available size to object_initialize().
>  2. digic-uart: qemu_log: use LOG_UNIMP instead LOG_GUEST_ERROR;
>  3. digic-boards: update rom image load code: introduce digic_load_rom().
> 
> Changes since v1:
>  0. drop the "add ARM946E-S CPU" patch;
>  1. convert to QOM, split DIGIC SoC code and board code
>     (thanks to Andreas Fa:rber, Peter Maydell and Peter Crosthwaite);
>  2. fix digic-uart (many thanks to Peter Crosthwaite
>     for his comments);
>  3. digic-boards: digic4_add_k8p3215uqb_rom(): update
>     rom image load code: use the '-bios' option.
> 
> DIGIC is Canon Inc.'s name for a family of SoC
> for digital cameras and camcorders.
> 
> See http://en.wikipedia.org/wiki/DIGIC for details.
> 
> There is no publicly available specification for
> DIGIC chips. All information about DIGIC chip
> internals is based on reverse engineering efforts
> made by CHDK (http://chdk.wikia.com) and
> Magic Lantern (http://www.magiclantern.fm) projects
> contributors.
> 
> Also this patch series adds initial support for Canon
> PowerShot A1100 IS compact camera (it is my only camera
> with connected UART interface). As the DIGIC-based cameras
> differences mostly are unsignificant (e.g. RAM-size,
> ROM type and size, GPIO usage) the other compact
> and DSLR cameras support can be easely added.
> 
> This DIGIC support patch series is inspired
> by EOS QEMU from Magic Lantern project.
> The main differences:
>  * EOS QEMU uses home-brew all-in-one monolith design;
>  this patch series uses conventional qemu object-centric design;
>  * EOS QEMU tries provide simplest emulation for most
>  controllers inside SoC to run Magic Lantern firmware;
>  this patch series provide more complete support
>  only for core devices to run barebox bootloader.
>   ** EOS QEMU does not support timer counting
>   (this patch series emulate 1 MHz counting);
>   ** EOS QEMU support DIGIC UART only for output
>   character to stderr; (this patch series emulate
>   introduces full blown UART interface);
>   ** EOS QEMU has incomplete ROM support;
>   (this patch series uses conventional qemu pflash).
> 
> This initial DIGIC support can't be used to run
> the original camera firmware, but it can successfully
> run experimental version of barebox bootloader
> (see http://www.barebox.org).
> 
> The last sources of barebox for PowerShot A1100 can be
> obtained here:
>   https://github.com/frantony/barebox/tree/next.digic.20130829
> 
> The precompiled ROM image usable with qemu can be
> obtained here:
>   https://github.com/frantony/barebox/blob/next.digic.20130829/canon-a1100-rom1.bin
> 
> This ROM image (after "dancing bit" encoding) can be run on
> real Canon A1100 camera.
> 
> The short build instruction for __previous__ DIGIC barebox
> version (it can be used with more recent sources too) can
> be obtained here:
>   http://lists.infradead.org/pipermail/barebox/2013-August/016007.html


-- 
-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v5 1/5] hw/arm: add very initial support for Canon DIGIC SoC

[Andreas Färber]

Am 07.09.2013 09:04, schrieb Antony Pavlov:
> DIGIC is Canon Inc.'s name for a family of SoC
> for digital cameras and camcorders.
> 
> There is no publicly available specification for
> DIGIC chips. All information about DIGIC chip
> internals is based on reverse engineering efforts
> made by CHDK (http://chdk.wikia.com) and
> Magic Lantern (http://www.magiclantern.fm) projects
> contributors.
> 
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  default-configs/arm-softmmu.mak |  1 +
>  hw/arm/Makefile.objs            |  1 +
>  hw/arm/digic.c                  | 65 +++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/digic.h          | 35 ++++++++++++++++++++++
>  4 files changed, 102 insertions(+)
>  create mode 100644 hw/arm/digic.c
>  create mode 100644 include/hw/arm/digic.h

Perfect,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Thanks for your patience,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC v5 0/5] hw/arm: add initial support for Canon DIGIC SoC: ping-ping

[Antony Pavlov]

On Fri, 13 Sep 2013 18:37:27 +0400
Antony Pavlov <antonynpavlov@gmail.com> wrote:

ping-ping

> On Sat,  7 Sep 2013 11:04:22 +0400
> Antony Pavlov <antonynpavlov@gmail.com> wrote:
> 
> ping
> > [RFC v5 1/5] hw/arm: add very initial support for Canon DIGIC SoC
> > [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support
> > [RFC v5 3/5] hw/arm/digic: add timer support
> > [RFC v5 4/5] hw/arm/digic: add UART support
> > [RFC v5 5/5] hw/arm/digic: add NOR ROM support
> > 
> > Changes since v4:
> >  1. digic.h: parent_obj: change type Object -> DeviceState
> >  2. digic-uart: drop reg array
> >  3. digic_boards: fix K8P3215UQB comment
> >  4. Makefile: place digic stuff in own line
> >  5. drop cpu-qom.h inclusion
> >  6. digic.h: add private/public labels
> >  7. digic.h: fix guard macro
> >  8. move base address macros to digic.c
> >  9. fix header comments
> > 
> > Changes since v3:
> >  1. fix typos and formatting
> >  2. digic-timer: drop DPRINTF
> >  3. digic-timer: fix DIGIC4_TIMER_BASE() macro
> >  4. digic.c: fix max timer device string
> > 
> > Changes since v2:
> >  1. rebase over latest master;
> >    * pass available size to object_initialize().
> >  2. digic-uart: qemu_log: use LOG_UNIMP instead LOG_GUEST_ERROR;
> >  3. digic-boards: update rom image load code: introduce digic_load_rom().
> > 
> > Changes since v1:
> >  0. drop the "add ARM946E-S CPU" patch;
> >  1. convert to QOM, split DIGIC SoC code and board code
> >     (thanks to Andreas Fa:rber, Peter Maydell and Peter Crosthwaite);
> >  2. fix digic-uart (many thanks to Peter Crosthwaite
> >     for his comments);
> >  3. digic-boards: digic4_add_k8p3215uqb_rom(): update
> >     rom image load code: use the '-bios' option.
> > 
> > DIGIC is Canon Inc.'s name for a family of SoC
> > for digital cameras and camcorders.
> > 
> > See http://en.wikipedia.org/wiki/DIGIC for details.
> > 
> > There is no publicly available specification for
> > DIGIC chips. All information about DIGIC chip
> > internals is based on reverse engineering efforts
> > made by CHDK (http://chdk.wikia.com) and
> > Magic Lantern (http://www.magiclantern.fm) projects
> > contributors.
> > 
> > Also this patch series adds initial support for Canon
> > PowerShot A1100 IS compact camera (it is my only camera
> > with connected UART interface). As the DIGIC-based cameras
> > differences mostly are unsignificant (e.g. RAM-size,
> > ROM type and size, GPIO usage) the other compact
> > and DSLR cameras support can be easely added.
> > 
> > This DIGIC support patch series is inspired
> > by EOS QEMU from Magic Lantern project.
> > The main differences:
> >  * EOS QEMU uses home-brew all-in-one monolith design;
> >  this patch series uses conventional qemu object-centric design;
> >  * EOS QEMU tries provide simplest emulation for most
> >  controllers inside SoC to run Magic Lantern firmware;
> >  this patch series provide more complete support
> >  only for core devices to run barebox bootloader.
> >   ** EOS QEMU does not support timer counting
> >   (this patch series emulate 1 MHz counting);
> >   ** EOS QEMU support DIGIC UART only for output
> >   character to stderr; (this patch series emulate
> >   introduces full blown UART interface);
> >   ** EOS QEMU has incomplete ROM support;
> >   (this patch series uses conventional qemu pflash).
> > 
> > This initial DIGIC support can't be used to run
> > the original camera firmware, but it can successfully
> > run experimental version of barebox bootloader
> > (see http://www.barebox.org).
> > 
> > The last sources of barebox for PowerShot A1100 can be
> > obtained here:
> >   https://github.com/frantony/barebox/tree/next.digic.20130829
> > 
> > The precompiled ROM image usable with qemu can be
> > obtained here:
> >   https://github.com/frantony/barebox/blob/next.digic.20130829/canon-a1100-rom1.bin
> > 
> > This ROM image (after "dancing bit" encoding) can be run on
> > real Canon A1100 camera.
> > 
> > The short build instruction for __previous__ DIGIC barebox
> > version (it can be used with more recent sources too) can
> > be obtained here:
> >   http://lists.infradead.org/pipermail/barebox/2013-August/016007.html
> 
> 
> -- 
> Best regards,
>   Antony Pavlov

-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v5 0/5] hw/arm: add initial support for Canon DIGIC SoC: ping-ping-ping

[Antony Pavlov]

On Fri, 20 Sep 2013 13:01:14 +0400
Antony Pavlov <antonynpavlov@gmail.com> wrote:

ping-ping-ping

> On Fri, 13 Sep 2013 18:37:27 +0400
> Antony Pavlov <antonynpavlov@gmail.com> wrote:
> 
> ping-ping
> 
> > On Sat,  7 Sep 2013 11:04:22 +0400
> > Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > 
> > ping
> > > [RFC v5 1/5] hw/arm: add very initial support for Canon DIGIC SoC
> > > [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support
> > > [RFC v5 3/5] hw/arm/digic: add timer support
> > > [RFC v5 4/5] hw/arm/digic: add UART support
> > > [RFC v5 5/5] hw/arm/digic: add NOR ROM support
> > > 
> > > Changes since v4:
> > >  1. digic.h: parent_obj: change type Object -> DeviceState
> > >  2. digic-uart: drop reg array
> > >  3. digic_boards: fix K8P3215UQB comment
> > >  4. Makefile: place digic stuff in own line
> > >  5. drop cpu-qom.h inclusion
> > >  6. digic.h: add private/public labels
> > >  7. digic.h: fix guard macro
> > >  8. move base address macros to digic.c
> > >  9. fix header comments
> > > 
> > > Changes since v3:
> > >  1. fix typos and formatting
> > >  2. digic-timer: drop DPRINTF
> > >  3. digic-timer: fix DIGIC4_TIMER_BASE() macro
> > >  4. digic.c: fix max timer device string
> > > 
> > > Changes since v2:
> > >  1. rebase over latest master;
> > >    * pass available size to object_initialize().
> > >  2. digic-uart: qemu_log: use LOG_UNIMP instead LOG_GUEST_ERROR;
> > >  3. digic-boards: update rom image load code: introduce digic_load_rom().
> > > 
> > > Changes since v1:
> > >  0. drop the "add ARM946E-S CPU" patch;
> > >  1. convert to QOM, split DIGIC SoC code and board code
> > >     (thanks to Andreas Fa:rber, Peter Maydell and Peter Crosthwaite);
> > >  2. fix digic-uart (many thanks to Peter Crosthwaite
> > >     for his comments);
> > >  3. digic-boards: digic4_add_k8p3215uqb_rom(): update
> > >     rom image load code: use the '-bios' option.
> > > 
> > > DIGIC is Canon Inc.'s name for a family of SoC
> > > for digital cameras and camcorders.
> > > 
> > > See http://en.wikipedia.org/wiki/DIGIC for details.
> > > 
> > > There is no publicly available specification for
> > > DIGIC chips. All information about DIGIC chip
> > > internals is based on reverse engineering efforts
> > > made by CHDK (http://chdk.wikia.com) and
> > > Magic Lantern (http://www.magiclantern.fm) projects
> > > contributors.
> > > 
> > > Also this patch series adds initial support for Canon
> > > PowerShot A1100 IS compact camera (it is my only camera
> > > with connected UART interface). As the DIGIC-based cameras
> > > differences mostly are unsignificant (e.g. RAM-size,
> > > ROM type and size, GPIO usage) the other compact
> > > and DSLR cameras support can be easely added.
> > > 
> > > This DIGIC support patch series is inspired
> > > by EOS QEMU from Magic Lantern project.
> > > The main differences:
> > >  * EOS QEMU uses home-brew all-in-one monolith design;
> > >  this patch series uses conventional qemu object-centric design;
> > >  * EOS QEMU tries provide simplest emulation for most
> > >  controllers inside SoC to run Magic Lantern firmware;
> > >  this patch series provide more complete support
> > >  only for core devices to run barebox bootloader.
> > >   ** EOS QEMU does not support timer counting
> > >   (this patch series emulate 1 MHz counting);
> > >   ** EOS QEMU support DIGIC UART only for output
> > >   character to stderr; (this patch series emulate
> > >   introduces full blown UART interface);
> > >   ** EOS QEMU has incomplete ROM support;
> > >   (this patch series uses conventional qemu pflash).
> > > 
> > > This initial DIGIC support can't be used to run
> > > the original camera firmware, but it can successfully
> > > run experimental version of barebox bootloader
> > > (see http://www.barebox.org).
> > > 
> > > The last sources of barebox for PowerShot A1100 can be
> > > obtained here:
> > >   https://github.com/frantony/barebox/tree/next.digic.20130829
> > > 
> > > The precompiled ROM image usable with qemu can be
> > > obtained here:
> > >   https://github.com/frantony/barebox/blob/next.digic.20130829/canon-a1100-rom1.bin
> > > 
> > > This ROM image (after "dancing bit" encoding) can be run on
> > > real Canon A1100 camera.
> > > 
> > > The short build instruction for __previous__ DIGIC barebox
> > > version (it can be used with more recent sources too) can
> > > be obtained here:
> > >   http://lists.infradead.org/pipermail/barebox/2013-August/016007.html
> > 
> > 
> > -- 
> > Best regards,
> >   Antony Pavlov
> 
> -- 
> Best regards,
>   Antony Pavlov


-- 
-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v5 0/5] hw/arm: add initial support for Canon DIGIC SoC: ping-ping-ping

[Peter Maydell]

On 28 September 2013 19:41, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Fri, 20 Sep 2013 13:01:14 +0400
> Antony Pavlov <antonynpavlov@gmail.com> wrote:
>
> ping-ping-ping

Just FYI, I'm on holiday, but this is on the list for when I get back...

-- PMM

Re: [Qemu-devel] [RFC v5 3/5] hw/arm/digic: add timer support

[Peter Maydell]

On 7 September 2013 08:04, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> +static const TypeInfo digic_timer_info = {
> +    .name = TYPE_DIGIC_TIMER,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(DigicTimerState),
> +    .instance_init = digic_timer_init,
> +};

This needs a reset function (should reset the timer,
so we behave the same on reset as we do at startup).

It also needs a VMStateDescription, since the
ptimer is migratable state.

thanks
-- PMM

Re: [Qemu-devel] [RFC v5 4/5] hw/arm/digic: add UART support

[Peter Maydell]

On 7 September 2013 08:04, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> --- a/hw/arm/digic_boards.c
> +++ b/hw/arm/digic_boards.c
> @@ -26,6 +26,13 @@
>  #include "hw/boards.h"
>  #include "exec/address-spaces.h"
>  #include "hw/arm/digic.h"
> +#include "hw/block/flash.h"
> +#include "hw/loader.h"
> +#include "sysemu/sysemu.h"
> +
> +#define DIGIC4_ROM0_BASE      0xf0000000
> +#define DIGIC4_ROM1_BASE      0xf8000000
> +# define DIGIC4_ROM_MAX_SIZE      0x08000000

Stray extra spaces here. If you fix those then:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [RFC v5 5/5] hw/arm/digic: add NOR ROM support

[Peter Maydell]

On 7 September 2013 08:04, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  hw/arm/digic_boards.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
> index cced229..a12b673 100644
> --- a/hw/arm/digic_boards.c
> +++ b/hw/arm/digic_boards.c
> @@ -41,6 +41,10 @@ typedef struct DigicBoardState {
>
>  typedef struct DigicBoard {
>      hwaddr ram_size;
> +    void (*add_rom0)(DigicBoardState *, hwaddr, const char *);
> +    const char *rom0_def_filename;
> +    void (*add_rom1)(DigicBoardState *, hwaddr, const char *);
> +    const char *rom1_def_filename;

This is a bit odd but all our board models are a bit ad-hoc
so I don't object to it.

>      hwaddr start_addr;
>  } DigicBoard;
>
> @@ -67,11 +71,71 @@ static void digic4_board_init(DigicBoard *board)
>
>      digic4_board_setup_ram(s, board->ram_size);
>
> +    if (board->add_rom0) {
> +        board->add_rom0(s, DIGIC4_ROM0_BASE, board->rom0_def_filename);
> +    }
> +
> +    if (board->add_rom1) {
> +        board->add_rom1(s, DIGIC4_ROM1_BASE, board->rom1_def_filename);
> +    }
> +
>      s->digic->cpu.env.regs[15] = board->start_addr;
>  }
>
> +static void digic_load_rom(DigicBoardState *s, hwaddr addr,
> +                           hwaddr max_size, const char *def_filename)
> +{
> +
> +    target_long rom_size;
> +    const char *filename;
> +
> +    if (bios_name) {
> +        filename = bios_name;
> +    } else {
> +        filename = def_filename;
> +    }
> +
> +    if (filename) {
> +        char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, filename);
> +
> +        if (!fn) {
> +            fprintf(stderr, "Couldn't find rom image '%s'.\n", filename);
> +            exit(1);
> +        }
> +
> +        rom_size = load_image_targphys(fn, addr, max_size);
> +        if (rom_size < 0 || rom_size > max_size) {
> +            fprintf(stderr, "Couldn't load rom image '%s'\n", filename);
> +            exit(1);
> +        }

This is weird. Why not use drive_get(IF_PFLASH, ...) and pass
that to pflash_cfi02_register(), the same way the other ARM
boards arrange to load the contents of flash?

> +    }
> +}
> +
> +/*
> + * Samsung K8P3215UQB
> + * 64M Bit (4Mx16) Page Mode / Multi-Bank NOR Flash Memory
> + */
> +static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, hwaddr addr,
> +                                      const char *def_filename)
> +{
> +#define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024)
> +#define FLASH_K8P3215UQB_SECTOR_SIZE (64 * 1024)
> +
> +    pflash_cfi02_register(addr, NULL, "pflash", FLASH_K8P3215UQB_SIZE,
> +                          NULL, FLASH_K8P3215UQB_SECTOR_SIZE,
> +                          FLASH_K8P3215UQB_SIZE / FLASH_K8P3215UQB_SECTOR_SIZE,
> +                          DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE,
> +                          4,
> +                          0x00EC, 0x007E, 0x0003, 0x0001,
> +                          0x0555, 0x2aa, 0);
> +
> +    digic_load_rom(s, addr, FLASH_K8P3215UQB_SIZE, def_filename);
> +}
> +
>  static DigicBoard digic4_board_canon_a1100 = {
>      .ram_size = 64 * 1024 * 1024,
> +    .add_rom1 = digic4_add_k8p3215uqb_rom,
> +    .rom1_def_filename = "canon-a1100-rom1.bin",
>      /* CHDK recommends this address for ROM disassembly */
>      .start_addr = 0xffc00000,
>  };
> --
> 1.8.4.rc3
>

-- PMM

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Maydell]

On 7 September 2013 08:04, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> +
> +static void digic4_board_init(DigicBoard *board)
> +{
> +    Error *err = NULL;
> +
> +    DigicBoardState *s = g_new(DigicBoardState, 1);
> +
> +    s->digic = DIGIC(object_new(TYPE_DIGIC));
> +    object_property_set_bool(OBJECT(s->digic), true, "realized", &err);
> +    if (err != NULL) {
> +        fprintf(stderr, "Couldn't realize DIGIC SoC: %s\n",
> +                error_get_pretty(err));
> +        exit(1);
> +    }
> +
> +    digic4_board_setup_ram(s, board->ram_size);
> +
> +    s->digic->cpu.env.regs[15] = board->start_addr;

I still think this is wrong. Real hardware can't possibly
start at this address; we should boot the same way the
hardware does.

> +}
> +
> +static DigicBoard digic4_board_canon_a1100 = {
> +    .ram_size = 64 * 1024 * 1024,
> +    /* CHDK recommends this address for ROM disassembly */
> +    .start_addr = 0xffc00000,
> +};

thanks
-- PMM

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Georg Hofstetter]

Hello,

just for the record.

flash (ROM1) on these cameras starts at 0xF8000000 and is either
0x00800000, 0x01000000 ox 0x02000000 large. just like with every
chip-selected memory, where the CS/EN line is selected by address masks,
addressing beyond the size memory repeats the content over and over.

ROM0 (0xF0000000) is rarely used.

The ARM in DIGIC has the high vectors selected by hardware and so the
reset vector is 0xFFFF0000. There you will find a bootloader.
Due to the memories repeating over and over starting from 0xF8000000,
the CPU will read from 0xF87F0000, 0xF8FF0000 or 0xF9FF0000, depending
on flash size (see above).

This kind of addressing beyond real flash end and wrapping over is
intentionally used by canon in multiple places - even in the main
firmware and when reflashing.
Some blocks are reflashed on a regular basis. They are used for
properties, which are the configuration area.

If you want to make the emulator behave like the real hardware, then you
have to:

 - reset to 0xFFFF0000
 - place ROM0 at 0xF0000000
 - place ROM1 at 0xF8000000
 - make the memory subsystem address correctly: (pseudocode)
     if((virt_addr & 0xF8000000) == 0xF0000000)
     {
         real_addr = 0xF0000000 | (virt_addr & (rom0_size - 1));
     }
     if((virt_addr & 0xF8000000) == 0xF8000000)
     {
         real_addr = 0xF8000000 | (virt_addr & (rom1_size - 1));
     }
 - make sure the flash emulation supports reflashing (properties)
 - change qemu memory subsystem to support execution from a flash that
   can be reprogrammed (properties are rewritten during startup)
   (maybe this is already possible, but it wasn't so 6 months ago)

OR
 - make workarounds so the system gets close to that behavior ;)


BR,
Georg

Am 17.10.2013 20:01, schrieb Peter Maydell:
> On 7 September 2013 08:04, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> 
> I still think this is wrong. Real hardware can't possibly
> start at this address; we should boot the same way the
> hardware does.
> 
>> +}
>> +
>> +static DigicBoard digic4_board_canon_a1100 = {
>> +    .ram_size = 64 * 1024 * 1024,
>> +    /* CHDK recommends this address for ROM disassembly */
>> +    .start_addr = 0xffc00000,
>> +};
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Maydell]

On 17 October 2013 19:51, Georg Hofstetter <qemu@g3gg0.de> wrote:
> flash (ROM1) on these cameras starts at 0xF8000000 and is either
> 0x00800000, 0x01000000 ox 0x02000000 large. just like with every
> chip-selected memory, where the CS/EN line is selected by address masks,
> addressing beyond the size memory repeats the content over and over.
>
> ROM0 (0xF0000000) is rarely used.
>
> The ARM in DIGIC has the high vectors selected by hardware and so the
> reset vector is 0xFFFF0000. There you will find a bootloader.
> Due to the memories repeating over and over starting from 0xF8000000,
> the CPU will read from 0xF87F0000, 0xF8FF0000 or 0xF9FF0000, depending
> on flash size (see above).
>
> This kind of addressing beyond real flash end and wrapping over is
> intentionally used by canon in multiple places - even in the main
> firmware and when reflashing.
> Some blocks are reflashed on a regular basis. They are used for
> properties, which are the configuration area.

Thanks for this explanation of the hardware.

> If you want to make the emulator behave like the real hardware, then you
> have to:
>
>  - reset to 0xFFFF0000

Yep. This implies having a cpu property corresponding to "enable
hivecs from bootup" (matching the h/w config signal), and making
sure cpu reset honours it; that's fairly easy.

>  - place ROM0 at 0xF0000000
>  - place ROM1 at 0xF8000000
>  - make the memory subsystem address correctly: (pseudocode)
>      if((virt_addr & 0xF8000000) == 0xF0000000)
>      {
>          real_addr = 0xF0000000 | (virt_addr & (rom0_size - 1));
>      }
>      if((virt_addr & 0xF8000000) == 0xF8000000)
>      {
>          real_addr = 0xF8000000 | (virt_addr & (rom1_size - 1));
>      }

The easy way to do this is just to use memory region aliases
to repeat the ROM through the whole area; you can do that
in the board model without having to mess with the memory
subsystem itself.

>  - make sure the flash emulation supports reflashing (properties)
>  - change qemu memory subsystem to support execution from a flash that
>    can be reprogrammed (properties are rewritten during startup)
>    (maybe this is already possible, but it wasn't so 6 months ago)

I agree that these are probably missing features in our flash
emulation, but aren't they orthogonal to the question of how
we handle CPU reset and what the starting PC should be?

-- PMM

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Georg Hofstetter]

Am 17.10.2013 21:17, schrieb Peter Maydell:
> 
>>  - make sure the flash emulation supports reflashing (properties)
>>  - change qemu memory subsystem to support execution from a flash that
>>    can be reprogrammed (properties are rewritten during startup)
>>    (maybe this is already possible, but it wasn't so 6 months ago)
> 
> I agree that these are probably missing features in our flash
> emulation, but aren't they orthogonal to the question of how
> we handle CPU reset and what the starting PC should be?
> 

Hi Peter,

absolutely - this was just the whole list of behavior to be implemented
and/or emulated to get the emulator close to real hardware.
Its just something that would prevent a clean firmware boot and came to
my mind while writing about system startup.

So yeah, its a bit off topic :)

Regards,
Georg

Re: [Qemu-devel] [RFC v5 4/5] hw/arm/digic: add UART support

[Antony Pavlov]

On Thu, 17 Oct 2013 18:54:28 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 7 September 2013 08:04, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > --- a/hw/arm/digic_boards.c
> > +++ b/hw/arm/digic_boards.c
> > @@ -26,6 +26,13 @@
> >  #include "hw/boards.h"
> >  #include "exec/address-spaces.h"
> >  #include "hw/arm/digic.h"
> > +#include "hw/block/flash.h"
> > +#include "hw/loader.h"
> > +#include "sysemu/sysemu.h"
> > +
> > +#define DIGIC4_ROM0_BASE      0xf0000000
> > +#define DIGIC4_ROM1_BASE      0xf8000000
> > +# define DIGIC4_ROM_MAX_SIZE      0x08000000
> 
> Stray extra spaces here. If you fix those then:
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Yes, my bad!

But this fragment (defining DIGIC4_ROM* macros) is misplaced. It will go to the "add NOR ROM support" in the next patchseries version.


-- 
-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Antony Pavlov]

On Thu, 17 Oct 2013 20:17:15 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 17 October 2013 19:51, Georg Hofstetter <qemu@g3gg0.de> wrote:
> > flash (ROM1) on these cameras starts at 0xF8000000 and is either
> > 0x00800000, 0x01000000 ox 0x02000000 large. just like with every
> > chip-selected memory, where the CS/EN line is selected by address masks,
> > addressing beyond the size memory repeats the content over and over.
> >
> > ROM0 (0xF0000000) is rarely used.
> >
> > The ARM in DIGIC has the high vectors selected by hardware and so the
> > reset vector is 0xFFFF0000. There you will find a bootloader.
> > Due to the memories repeating over and over starting from 0xF8000000,
> > the CPU will read from 0xF87F0000, 0xF8FF0000 or 0xF9FF0000, depending
> > on flash size (see above).
> >
> > This kind of addressing beyond real flash end and wrapping over is
> > intentionally used by canon in multiple places - even in the main
> > firmware and when reflashing.
> > Some blocks are reflashed on a regular basis. They are used for
> > properties, which are the configuration area.
> 
> Thanks for this explanation of the hardware.
> 
> > If you want to make the emulator behave like the real hardware, then you
> > have to:
> >
> >  - reset to 0xFFFF0000
> 
> Yep. This implies having a cpu property corresponding to "enable
> hivecs from bootup" (matching the h/w config signal), and making
> sure cpu reset honours it; that's fairly easy.
> 
> >  - place ROM0 at 0xF0000000
> >  - place ROM1 at 0xF8000000
> >  - make the memory subsystem address correctly: (pseudocode)
> >      if((virt_addr & 0xF8000000) == 0xF0000000)
> >      {
> >          real_addr = 0xF0000000 | (virt_addr & (rom0_size - 1));
> >      }
> >      if((virt_addr & 0xF8000000) == 0xF8000000)
> >      {
> >          real_addr = 0xF8000000 | (virt_addr & (rom1_size - 1));
> >      }
> 
> The easy way to do this is just to use memory region aliases
> to repeat the ROM through the whole area; you can do that
> in the board model without having to mess with the memory
> subsystem itself.

Hmm. The current DIGIC patchseries already has the functionality like this!
Here is my console log:

$ xxd -g 1 -l 0x40 ./canon-a1100-rom1.bin 
0000000: 12 00 00 ea fe ff ff ea fe ff ff ea fe ff ff ea  ................
0000010: fe ff ff ea fe ff ff ea fe ff ff ea fe ff ff ea  ................
0000020: 62 61 72 65 62 6f 78 00 00 00 10 00 84 5e 01 00  barebox......^..
0000030: 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55  UUUUUUUUUUUUUUUU

$ ./arm-softmmu/qemu-system-arm -M canon-a1100 -bios ./canon-a1100-rom1.bin -serial stdio

Switch to console [cs0]


barebox 2013.08.0-00267-g85b83fb #125 Thu Aug 29 07:58:57 MSK 2013


Board: Canon PowerShot A1100 IS
digic-gpio c0220000.gpio: probed gpiochip-1 with base 0
cfi_flash f8000000.flash: found cfi flash at f8000000, size 4194304
malloc space: 0x00100000 -> 0x002fffff (size 2 MiB)
Open /dev/env0 No such file or directory
no valid environment found on /dev/env0. Using default environment
running /env/bin/init...
canon> /
canon> / md -b 0xf8000000+0x40
f8000000: 12 00 00 ea fe ff ff ea fe ff ff ea fe ff ff ea    ................
f8000010: fe ff ff ea fe ff ff ea fe ff ff ea fe ff ff ea    ................
f8000020: 62 61 72 65 62 6f 78 00 00 00 10 00 84 5e 01 00    barebox......^..
f8000030: 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55    UUUUUUUUUUUUUUUU
canon> / md -b 0xf8400000+0x40
f8400000: 12 00 00 ea fe ff ff ea fe ff ff ea fe ff ff ea    ................
f8400010: fe ff ff ea fe ff ff ea fe ff ff ea fe ff ff ea    ................
f8400020: 62 61 72 65 62 6f 78 00 00 00 10 00 84 5e 01 00    barebox......^..
f8400030: 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55    UUUUUUUUUUUUUUUU
canon> / md -b 0xf9000000+0x40
f9000000: 12 00 00 ea fe ff ff ea fe ff ff ea fe ff ff ea    ................
f9000010: fe ff ff ea fe ff ff ea fe ff ff ea fe ff ff ea    ................
f9000020: 62 61 72 65 62 6f 78 00 00 00 10 00 84 5e 01 00    barebox......^..
f9000030: 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55    UUUUUUUUUUUUUUUU
canon> / 

-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Antony Pavlov]

On Thu, 17 Oct 2013 20:17:15 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 17 October 2013 19:51, Georg Hofstetter <qemu@g3gg0.de> wrote:
> > flash (ROM1) on these cameras starts at 0xF8000000 and is either
> > 0x00800000, 0x01000000 ox 0x02000000 large. just like with every
> > chip-selected memory, where the CS/EN line is selected by address masks,
> > addressing beyond the size memory repeats the content over and over.
> >
> > ROM0 (0xF0000000) is rarely used.
> >
> > The ARM in DIGIC has the high vectors selected by hardware and so the
> > reset vector is 0xFFFF0000. There you will find a bootloader.
> > Due to the memories repeating over and over starting from 0xF8000000,
> > the CPU will read from 0xF87F0000, 0xF8FF0000 or 0xF9FF0000, depending
> > on flash size (see above).
> >
> > This kind of addressing beyond real flash end and wrapping over is
> > intentionally used by canon in multiple places - even in the main
> > firmware and when reflashing.
> > Some blocks are reflashed on a regular basis. They are used for
> > properties, which are the configuration area.
> 
> Thanks for this explanation of the hardware.
> 
> > If you want to make the emulator behave like the real hardware, then you
> > have to:
> >
> >  - reset to 0xFFFF0000
> 
> Yep. This implies having a cpu property corresponding to "enable
> hivecs from bootup" (matching the h/w config signal), and making
> sure cpu reset honours it; that's fairly easy.
> 
> >  - place ROM0 at 0xF0000000
> >  - place ROM1 at 0xF8000000
> >  - make the memory subsystem address correctly: (pseudocode)
> >      if((virt_addr & 0xF8000000) == 0xF0000000)
> >      {
> >          real_addr = 0xF0000000 | (virt_addr & (rom0_size - 1));
> >      }
> >      if((virt_addr & 0xF8000000) == 0xF8000000)
> >      {
> >          real_addr = 0xF8000000 | (virt_addr & (rom1_size - 1));
> >      }
> 
> The easy way to do this is just to use memory region aliases
> to repeat the ROM through the whole area; you can do that
> in the board model without having to mess with the memory
> subsystem itself.
> 
> >  - make sure the flash emulation supports reflashing (properties)
> >  - change qemu memory subsystem to support execution from a flash that
> >    can be reprogrammed (properties are rewritten during startup)
> >    (maybe this is already possible, but it wasn't so 6 months ago)
> 
> I agree that these are probably missing features in our flash
> emulation, but aren't they orthogonal to the question of how
> we handle CPU reset and what the starting PC should be?

Here is my proposition:

1. qemu board code setup CPU to start from 0xFFFF0000. (0xffff0000 is a ROM address
on DIGIC chips)

2. we need somehow put a 'jump-to-beginning-of-ROM' instruction to 0xffff0000.
(We can't put barebox to 0xffff0000 as barebox image is bigger that 64K.)

There is at least two possibilities to do so:
 * we can use specially prepared ROM image;
 * qemu board code can insert by itself a 'jump-to-beginning-of-ROM' instruction
after loading ROM image (as qemu MIPS Malta board code does).

3. CPU starts as usual. Branching to barebox code in ROM happends in a natural way!

Please comment my proposition.

-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Maydell]

On 4 December 2013 20:22, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Here is my proposition:
>
> 1. qemu board code setup CPU to start from 0xFFFF0000. (0xffff0000 is a ROM address
> on DIGIC chips)

Sort of. What we need is:
 1a. Add a "hivecs" property to the ARM CPU object (which
just sets env->cp15.c1_sys bit 13)

(this is about half a dozen lines of code max)

 1b DIGIC board init code creates the CPU and sets the hivecs property on it

(another handful of lines of code)

> 2. we need somehow put a 'jump-to-beginning-of-ROM' instruction to 0xffff0000.
> (We can't put barebox to 0xffff0000 as barebox image is bigger that 64K.)
>
> There is at least two possibilities to do so:
>  * we can use specially prepared ROM image;
>  * qemu board code can insert by itself a 'jump-to-beginning-of-ROM' instruction
> after loading ROM image (as qemu MIPS Malta board code does).

The board code should definitely not be writing random instructions into
memory. You need to provide a ROM image which will do the work.

> 3. CPU starts as usual. Branching to barebox code in ROM happends in a natural way!

This bit's OK :-)

thanks
-- PMM

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Antony Pavlov]

On Wed, 4 Dec 2013 20:29:05 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 4 December 2013 20:22, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Here is my proposition:
> >
> > 1. qemu board code setup CPU to start from 0xFFFF0000. (0xffff0000 is a ROM address
> > on DIGIC chips)
> 
> Sort of. What we need is:
>  1a. Add a "hivecs" property to the ARM CPU object (which
> just sets env->cp15.c1_sys bit 13)
> 
> (this is about half a dozen lines of code max)
> 
>  1b DIGIC board init code creates the CPU and sets the hivecs property on it
> 
> (another handful of lines of code)

Ok, I'll try to make the work at the weekend.

> > 2. we need somehow put a 'jump-to-beginning-of-ROM' instruction to 0xffff0000.
> > (We can't put barebox to 0xffff0000 as barebox image is bigger that 64K.)
> >
> > There is at least two possibilities to do so:
> >  * we can use specially prepared ROM image;
> >  * qemu board code can insert by itself a 'jump-to-beginning-of-ROM' instruction
> > after loading ROM image (as qemu MIPS Malta board code does).
> 
> The board code should definitely not be writing random instructions into
> memory. You need to provide a ROM image which will do the work.
> 
> > 3. CPU starts as usual. Branching to barebox code in ROM happends in a natural way!
> 
> This bit's OK :-)
> 
> thanks
> -- PMM


-- 
-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Maydell]

On 4 December 2013 21:20, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Wed, 4 Dec 2013 20:29:05 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
>> On 4 December 2013 20:22, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> > Here is my proposition:
>> >
>> > 1. qemu board code setup CPU to start from 0xFFFF0000. (0xffff0000 is a ROM address
>> > on DIGIC chips)
>>
>> Sort of. What we need is:
>>  1a. Add a "hivecs" property to the ARM CPU object (which
>> just sets env->cp15.c1_sys bit 13)
>>
>> (this is about half a dozen lines of code max)
>>
>>  1b DIGIC board init code creates the CPU and sets the hivecs property on it
>>
>> (another handful of lines of code)
>
> Ok, I'll try to make the work at the weekend.

The recent (not yet applied) patchset adding a property for the CBAR
register value is probably a good model to look at.

-- PMM

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Crosthwaite]

On Thu, Dec 5, 2013 at 7:34 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 4 December 2013 21:20, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> On Wed, 4 Dec 2013 20:29:05 +0000
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>>> On 4 December 2013 20:22, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>>> > Here is my proposition:
>>> >
>>> > 1. qemu board code setup CPU to start from 0xFFFF0000. (0xffff0000 is a ROM address
>>> > on DIGIC chips)
>>>
>>> Sort of. What we need is:
>>>  1a. Add a "hivecs" property to the ARM CPU object (which
>>> just sets env->cp15.c1_sys bit 13)
>>>
>>> (this is about half a dozen lines of code max)
>>>
>>>  1b DIGIC board init code creates the CPU and sets the hivecs property on it
>>>
>>> (another handful of lines of code)
>>
>> Ok, I'll try to make the work at the weekend.
>
> The recent (not yet applied) patchset adding a property for the CBAR
> register value is probably a good model to look at.
>

Is hivecs-on-reset ideally a new ARM_FEATURE or is there a simpler
conditional we can use as post_init time?

Regards,
Peter

> -- PMM
>

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Crosthwaite]

On Thu, Dec 5, 2013 at 7:20 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Wed, 4 Dec 2013 20:29:05 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
>> On 4 December 2013 20:22, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> > Here is my proposition:
>> >
>> > 1. qemu board code setup CPU to start from 0xFFFF0000. (0xffff0000 is a ROM address
>> > on DIGIC chips)
>>
>> Sort of. What we need is:
>>  1a. Add a "hivecs" property to the ARM CPU object (which
>> just sets env->cp15.c1_sys bit 13)
>>
>> (this is about half a dozen lines of code max)
>>
>>  1b DIGIC board init code creates the CPU and sets the hivecs property on it
>>
>> (another handful of lines of code)
>
> Ok, I'll try to make the work at the weekend.
>
>> > 2. we need somehow put a 'jump-to-beginning-of-ROM' instruction to 0xffff0000.
>> > (We can't put barebox to 0xffff0000 as barebox image is bigger that 64K.)
>> >
>> > There is at least two possibilities to do so:
>> >  * we can use specially prepared ROM image;
>> >  * qemu board code can insert by itself a 'jump-to-beginning-of-ROM' instruction
>> > after loading ROM image (as qemu MIPS Malta board code does).
>>
>> The board code should definitely not be writing random instructions into
>> memory. You need to provide a ROM image which will do the work.
>>

But the bootloader does this already. We have support for board
configurable secondary bootloops. Is this as simple as supporting
board configurable primary boot fragments?

arm_boot needs to be patched to do its bootstrap magic with no -kernel
arg I guess. I have something of that nature in my own tree, where I
have a command line argument that forces arm-boot to use the linux
bootstrap stuff (primary and secondary bootloops and all that)
regardless of image type.

Regards,
Peter

>> > 3. CPU starts as usual. Branching to barebox code in ROM happends in a natural way!
>>
>> This bit's OK :-)
>>
>> thanks
>> -- PMM
>
>
> --
> --
> Best regards,
>   Antony Pavlov
>

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Maydell]

On 5 December 2013 00:25, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> But the bootloader does this already. We have support for board
> configurable secondary bootloops. Is this as simple as supporting
> board configurable primary boot fragments?
>
> arm_boot needs to be patched to do its bootstrap magic with no -kernel
> arg I guess.

I'd really rather not extend the arm_boot code to more usage cases
if I can avoid it. It's really intended for loading kernels. In this case
the thing being loaded really is a ROM image, and the correct way
to handle this is to make the board model behave the same way
the hardware does and make the ROM image sit at the same place
in the memory map that the real ROM image does.

thanks
-- PMM

Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Peter Maydell]

On 5 December 2013 00:20, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Is hivecs-on-reset ideally a new ARM_FEATURE or is there a simpler
> conditional we can use as post_init time?

I think we want the property if (!arm_feature(ARM_FEATURE_M)).

-- PMM

[Qemu-devel] [RFC 0/2] ARM: make possible to use high vectors for reset exception

[Antony Pavlov]

[RFC 1/2] ARM: cpu: add "hivecs" property (high vectors on reset)
[RFC 2/2] ARM: arm_cpu_reset: make possible to use high vectors for

[Qemu-devel] [RFC 1/2] ARM: cpu: add "hivecs" property (high vectors on reset)

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 target-arm/cpu-qom.h |  1 +
 target-arm/cpu.c     | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index b55306a..116320c 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -128,6 +128,7 @@ typedef struct ARMCPU {
     uint32_t ccsidr[16];
     uint32_t reset_cbar;
     uint32_t reset_auxcr;
+    bool reset_hivecs;
 } ARMCPU;
 
 #define TYPE_AARCH64_CPU "aarch64-cpu"
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d40f2a7..f838499 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -20,6 +20,7 @@
 
 #include "cpu.h"
 #include "qemu-common.h"
+#include "hw/qdev-properties.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/loader.h"
 #endif
@@ -119,6 +120,12 @@ static void arm_cpu_reset(CPUState *s)
             env->regs[15] = pc & ~1;
         }
     }
+
+    if (cpu->reset_hivecs) {
+            uint32_t c1_sys = env->cp15.c1_sys;
+            env->cp15.c1_sys = c1_sys | (1 << 13);
+    }
+
     env->vfp.xregs[ARM_VFP_FPEXC] = 0;
 #endif
     set_flush_to_zero(1, &env->vfp.standard_fp_status);
@@ -854,6 +861,11 @@ typedef struct ARMCPUInfo {
     void (*class_init)(ObjectClass *oc, void *data);
 } ARMCPUInfo;
 
+static Property arm_cpu_properties[] = {
+    DEFINE_PROP_BOOL("hivecs", ARMCPU, reset_hivecs, false),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static const ARMCPUInfo arm_cpus[] = {
 #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
     { .name = "arm926",      .initfn = arm926_initfn },
@@ -902,6 +914,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
 
     acc->parent_realize = dc->realize;
     dc->realize = arm_cpu_realizefn;
+    dc->props = arm_cpu_properties;
 
     acc->parent_reset = cc->reset;
     cc->reset = arm_cpu_reset;
-- 
1.8.5

[Qemu-devel] [RFC 2/2] ARM: arm_cpu_reset: make possible to use high vectors for reset_exc

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 target-arm/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index f838499..6f548c7 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -126,6 +126,10 @@ static void arm_cpu_reset(CPUState *s)
             env->cp15.c1_sys = c1_sys | (1 << 13);
     }
 
+    if (!IS_M(env) && env->cp15.c1_sys & (1 << 13)) {
+            env->regs[15] = 0xFFFF0000;
+    }
+
     env->vfp.xregs[ARM_VFP_FPEXC] = 0;
 #endif
     set_flush_to_zero(1, &env->vfp.standard_fp_status);
-- 
1.8.5

Re: [Qemu-devel] [RFC 1/2] ARM: cpu: add "hivecs" property (high vectors on reset)

[Peter Crosthwaite]

On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  target-arm/cpu-qom.h |  1 +
>  target-arm/cpu.c     | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index b55306a..116320c 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -128,6 +128,7 @@ typedef struct ARMCPU {
>      uint32_t ccsidr[16];
>      uint32_t reset_cbar;
>      uint32_t reset_auxcr;
> +    bool reset_hivecs;
>  } ARMCPU;
>
>  #define TYPE_AARCH64_CPU "aarch64-cpu"
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index d40f2a7..f838499 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -20,6 +20,7 @@
>
>  #include "cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-properties.h"
>  #if !defined(CONFIG_USER_ONLY)
>  #include "hw/loader.h"
>  #endif
> @@ -119,6 +120,12 @@ static void arm_cpu_reset(CPUState *s)
>              env->regs[15] = pc & ~1;
>          }
>      }
> +
> +    if (cpu->reset_hivecs) {
> +            uint32_t c1_sys = env->cp15.c1_sys;
> +            env->cp15.c1_sys = c1_sys | (1 << 13);

Why the read modify write? Cant you do it as a one-liner |= ?

env->cp15.c1_sys |= (1 << 13);

> +    }
> +
>      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
>  #endif
>      set_flush_to_zero(1, &env->vfp.standard_fp_status);
> @@ -854,6 +861,11 @@ typedef struct ARMCPUInfo {
>      void (*class_init)(ObjectClass *oc, void *data);
>  } ARMCPUInfo;
>
> +static Property arm_cpu_properties[] = {
> +    DEFINE_PROP_BOOL("hivecs", ARMCPU, reset_hivecs, false),

As this is indicating a reset value, "reset" should feature in the
property name.

> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static const ARMCPUInfo arm_cpus[] = {
>  #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
>      { .name = "arm926",      .initfn = arm926_initfn },
> @@ -902,6 +914,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>
>      acc->parent_realize = dc->realize;
>      dc->realize = arm_cpu_realizefn;
> +    dc->props = arm_cpu_properties;

This unconditionally adds the property to all ARM CPUs, wheras its
only valid for a subset of ARM CPUs (!M). Check the earlier patches in
the CBAR/highbank/zynq series on list for the change pattern for
adding it as a conditional dynamic property instead. I'll take it into
that series if you want to rebase onto my patch set (will save you
having to conflict by adding post-init fn which is still absent from
mainline).

Regards,
Peter

>
>      acc->parent_reset = cc->reset;
>      cc->reset = arm_cpu_reset;
> --
> 1.8.5
>
>

Re: [Qemu-devel] [RFC 2/2] ARM: arm_cpu_reset: make possible to use high vectors for reset_exc

[Peter Crosthwaite]

On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  target-arm/cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index f838499..6f548c7 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -126,6 +126,10 @@ static void arm_cpu_reset(CPUState *s)
>              env->cp15.c1_sys = c1_sys | (1 << 13);
>      }
>
> +    if (!IS_M(env) && env->cp15.c1_sys & (1 << 13)) {
> +            env->regs[15] = 0xFFFF0000;

Dont see the need for the second if. This should only be reachable if
reset_hivecs == true. Can you just put this line after the above
"env->cp15.c1_sys = c1_sys | (1 << 13);" ?

With the change I suggested in P1, the reset_hivecs variable wont be a
property of M CPUs so it will be impossible for an M to have
rest_hivecs regardless of board code (mis)behaviour. So I think you
are safe to drop the IS_M here.

Regards,
Peter

> +    }
> +
>      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
>  #endif
>      set_flush_to_zero(1, &env->vfp.standard_fp_status);
> --
> 1.8.5
>
>

Re: [Qemu-devel] [RFC 1/2] ARM: cpu: add "hivecs" property (high vectors on reset)

[Peter Maydell]

On 7 December 2013 00:55, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  target-arm/cpu-qom.h |  1 +
>  target-arm/cpu.c     | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index b55306a..116320c 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -128,6 +128,7 @@ typedef struct ARMCPU {
>      uint32_t ccsidr[16];
>      uint32_t reset_cbar;
>      uint32_t reset_auxcr;
> +    bool reset_hivecs;
>  } ARMCPU;
>
>  #define TYPE_AARCH64_CPU "aarch64-cpu"
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index d40f2a7..f838499 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -20,6 +20,7 @@
>
>  #include "cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-properties.h"
>  #if !defined(CONFIG_USER_ONLY)
>  #include "hw/loader.h"
>  #endif
> @@ -119,6 +120,12 @@ static void arm_cpu_reset(CPUState *s)
>              env->regs[15] = pc & ~1;
>          }
>      }
> +
> +    if (cpu->reset_hivecs) {
> +            uint32_t c1_sys = env->cp15.c1_sys;
> +            env->cp15.c1_sys = c1_sys | (1 << 13);
> +    }

You don't need to do this on every reset. You can just
set the bit in cpu->reset_sctlr at realize if reset_hivecs
is true, and then reset will just reset to the correct
value.

thanks
-- PMM

Re: [Qemu-devel] [RFC 1/2] ARM: cpu: add "hivecs" property (high vectors on reset)

[Antony Pavlov]

On Sat, 7 Dec 2013 11:00:05 +1000
Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:

> On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  target-arm/cpu-qom.h |  1 +
> >  target-arm/cpu.c     | 13 +++++++++++++
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index b55306a..116320c 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -128,6 +128,7 @@ typedef struct ARMCPU {
> >      uint32_t ccsidr[16];
> >      uint32_t reset_cbar;
> >      uint32_t reset_auxcr;
> > +    bool reset_hivecs;
> >  } ARMCPU;
> >
> >  #define TYPE_AARCH64_CPU "aarch64-cpu"
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index d40f2a7..f838499 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -20,6 +20,7 @@
> >
> >  #include "cpu.h"
> >  #include "qemu-common.h"
> > +#include "hw/qdev-properties.h"
> >  #if !defined(CONFIG_USER_ONLY)
> >  #include "hw/loader.h"
> >  #endif
> > @@ -119,6 +120,12 @@ static void arm_cpu_reset(CPUState *s)
> >              env->regs[15] = pc & ~1;
> >          }
> >      }
> > +
> > +    if (cpu->reset_hivecs) {
> > +            uint32_t c1_sys = env->cp15.c1_sys;
> > +            env->cp15.c1_sys = c1_sys | (1 << 13);
> 
> Why the read modify write? Cant you do it as a one-liner |= ?
> 
> env->cp15.c1_sys |= (1 << 13);
> 
> > +    }
> > +
> >      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
> >  #endif
> >      set_flush_to_zero(1, &env->vfp.standard_fp_status);
> > @@ -854,6 +861,11 @@ typedef struct ARMCPUInfo {
> >      void (*class_init)(ObjectClass *oc, void *data);
> >  } ARMCPUInfo;
> >
> > +static Property arm_cpu_properties[] = {
> > +    DEFINE_PROP_BOOL("hivecs", ARMCPU, reset_hivecs, false),
> 
> As this is indicating a reset value, "reset" should feature in the
> property name.
> 
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> >  static const ARMCPUInfo arm_cpus[] = {
> >  #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
> >      { .name = "arm926",      .initfn = arm926_initfn },
> > @@ -902,6 +914,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
> >
> >      acc->parent_realize = dc->realize;
> >      dc->realize = arm_cpu_realizefn;
> > +    dc->props = arm_cpu_properties;
> 
> This unconditionally adds the property to all ARM CPUs, wheras its
> only valid for a subset of ARM CPUs (!M). Check the earlier patches in
> the CBAR/highbank/zynq series on list for the change pattern for
> adding it as a conditional dynamic property instead. I'll take it into

Is there any public repo with this changes?

> that series if you want to rebase onto my patch set (will save you
> having to conflict by adding post-init fn which is still absent from
> mainline).
> 
> Regards,
> Peter
> 
> >
> >      acc->parent_reset = cc->reset;
> >      cc->reset = arm_cpu_reset;
> > --
> > 1.8.5
> >
> >


-- 
-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC 2/2] ARM: arm_cpu_reset: make possible to use high vectors for reset_exc

[Antony Pavlov]

On Sat, 7 Dec 2013 11:08:40 +1000
Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:

> On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  target-arm/cpu.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index f838499..6f548c7 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -126,6 +126,10 @@ static void arm_cpu_reset(CPUState *s)
> >              env->cp15.c1_sys = c1_sys | (1 << 13);
> >      }
> >
> > +    if (!IS_M(env) && env->cp15.c1_sys & (1 << 13)) {
> > +            env->regs[15] = 0xFFFF0000;
> 
> Dont see the need for the second if. This should only be reachable if
> reset_hivecs == true. Can you just put this line after the above
> "env->cp15.c1_sys = c1_sys | (1 << 13);" ?

Here we have a small problem.
On the one hand we have 'reset_hivecs' property, on the other hand we have c1_sys[13].

What we have to do if during reset c1_sys[13] == 1 and 'reset_hivecs' is not set?

Also if c1_sys[13] == 0, but 'reset_hivecs' is set IMHO we have to set c1_sys[13] = 1.


> With the change I suggested in P1, the reset_hivecs variable wont be a
> property of M CPUs so it will be impossible for an M to have
> rest_hivecs regardless of board code (mis)behaviour. So I think you
> are safe to drop the IS_M here.
> 
> Regards,
> Peter
> 
> > +    }
> > +
> >      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
> >  #endif
> >      set_flush_to_zero(1, &env->vfp.standard_fp_status);
> > --
> > 1.8.5
> >
> >


-- 
-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC 2/2] ARM: arm_cpu_reset: make possible to use high vectors for reset_exc

[Peter Maydell]

On 7 December 2013 20:49, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Sat, 7 Dec 2013 11:08:40 +1000
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>
>> On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
>> > ---
>> >  target-arm/cpu.c | 4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> > index f838499..6f548c7 100644
>> > --- a/target-arm/cpu.c
>> > +++ b/target-arm/cpu.c
>> > @@ -126,6 +126,10 @@ static void arm_cpu_reset(CPUState *s)
>> >              env->cp15.c1_sys = c1_sys | (1 << 13);
>> >      }
>> >
>> > +    if (!IS_M(env) && env->cp15.c1_sys & (1 << 13)) {
>> > +            env->regs[15] = 0xFFFF0000;
>>
>> Dont see the need for the second if. This should only be reachable if
>> reset_hivecs == true. Can you just put this line after the above
>> "env->cp15.c1_sys = c1_sys | (1 << 13);" ?
>
> Here we have a small problem.
> On the one hand we have 'reset_hivecs' property, on the other hand we have c1_sys[13].
>
> What we have to do if during reset c1_sys[13] == 1 and 'reset_hivecs' is not set?

See my other mail: on reset you should not care about the value
of reset_hivecs. At realize (ie in arm_cpu_realizefn(), at the top
(so it takes effect before that function calls
register_cp_regs_for_features()) you should do:

    if (cpu->reset_hivecs) {
        cpu->reset_sctlr |= (1 << 13);
    }

This will mean that c1_sys (the SCTLR) will always reset with
the hivecs bit set, which is what we want.

Then the only thing you need to do at reset is:
    if (env->cp15.c1_sys & (1 << 13)) {
        /* hivecs setting also affects the CPU reset address */
        env->regs[15] = 0xffff0000;
    }

(as Peter says we can rely on this never being true for M profile
so no specific check for that is required).

thanks
-- PMM

Re: [Qemu-devel] [RFC 1/2] ARM: cpu: add "hivecs" property (high vectors on reset)

[Peter Crosthwaite]

On Sun, Dec 8, 2013 at 6:44 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Sat, 7 Dec 2013 11:00:05 +1000
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>
>> On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
>> > ---
>> >  target-arm/cpu-qom.h |  1 +
>> >  target-arm/cpu.c     | 13 +++++++++++++
>> >  2 files changed, 14 insertions(+)
>> >
>> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> > index b55306a..116320c 100644
>> > --- a/target-arm/cpu-qom.h
>> > +++ b/target-arm/cpu-qom.h
>> > @@ -128,6 +128,7 @@ typedef struct ARMCPU {
>> >      uint32_t ccsidr[16];
>> >      uint32_t reset_cbar;
>> >      uint32_t reset_auxcr;
>> > +    bool reset_hivecs;
>> >  } ARMCPU;
>> >
>> >  #define TYPE_AARCH64_CPU "aarch64-cpu"
>> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> > index d40f2a7..f838499 100644
>> > --- a/target-arm/cpu.c
>> > +++ b/target-arm/cpu.c
>> > @@ -20,6 +20,7 @@
>> >
>> >  #include "cpu.h"
>> >  #include "qemu-common.h"
>> > +#include "hw/qdev-properties.h"
>> >  #if !defined(CONFIG_USER_ONLY)
>> >  #include "hw/loader.h"
>> >  #endif
>> > @@ -119,6 +120,12 @@ static void arm_cpu_reset(CPUState *s)
>> >              env->regs[15] = pc & ~1;
>> >          }
>> >      }
>> > +
>> > +    if (cpu->reset_hivecs) {
>> > +            uint32_t c1_sys = env->cp15.c1_sys;
>> > +            env->cp15.c1_sys = c1_sys | (1 << 13);
>>
>> Why the read modify write? Cant you do it as a one-liner |= ?
>>
>> env->cp15.c1_sys |= (1 << 13);
>>
>> > +    }
>> > +
>> >      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
>> >  #endif
>> >      set_flush_to_zero(1, &env->vfp.standard_fp_status);
>> > @@ -854,6 +861,11 @@ typedef struct ARMCPUInfo {
>> >      void (*class_init)(ObjectClass *oc, void *data);
>> >  } ARMCPUInfo;
>> >
>> > +static Property arm_cpu_properties[] = {
>> > +    DEFINE_PROP_BOOL("hivecs", ARMCPU, reset_hivecs, false),
>>
>> As this is indicating a reset value, "reset" should feature in the
>> property name.
>>
>> > +    DEFINE_PROP_END_OF_LIST(),
>> > +};
>> > +
>> >  static const ARMCPUInfo arm_cpus[] = {
>> >  #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
>> >      { .name = "arm926",      .initfn = arm926_initfn },
>> > @@ -902,6 +914,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>> >
>> >      acc->parent_realize = dc->realize;
>> >      dc->realize = arm_cpu_realizefn;
>> > +    dc->props = arm_cpu_properties;
>>
>> This unconditionally adds the property to all ARM CPUs, wheras its
>> only valid for a subset of ARM CPUs (!M). Check the earlier patches in
>> the CBAR/highbank/zynq series on list for the change pattern for
>> adding it as a conditional dynamic property instead. I'll take it into
>
> Is there any public repo with this changes?
>

Not as of this moment (and I will do something about that soon), but
generally speaking, this tool makes very light work of getting
someones series as a branch just using the list:

https://github.com/aliguori/patches

I highly recommend it over using personal trees and branches.

Regards,
Peter

>> that series if you want to rebase onto my patch set (will save you
>> having to conflict by adding post-init fn which is still absent from
>> mainline).
>>
>> Regards,
>> Peter
>>
>> >
>> >      acc->parent_reset = cc->reset;
>> >      cc->reset = arm_cpu_reset;
>> > --
>> > 1.8.5
>> >
>> >
>
>
> --
> --
> Best regards,
>   Antony Pavlov
>