* [Qemu-devel] [PATCH 2/8] bcm2835_property: add bcm2835 property channel
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
@ 2015-12-04 6:01 ` Andrew Baumann
2015-12-04 6:01 ` [Qemu-devel] [PATCH 3/8] bcm2835_ic: add bcm2835 interrupt controller Andrew Baumann
` (7 subsequent siblings)
8 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-04 6:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, Andrew Baumann, qemu-arm, Paolo Bonzini
This sits behind the mailbox interface, and implements
request/response queries for system properties. The
framebuffer-related properties will be added in a later patch.
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
hw/misc/Makefile.objs | 1 +
hw/misc/bcm2835_property.c | 262 +++++++++++++++++++++++++++++++++++++
include/hw/misc/bcm2835_property.h | 27 ++++
3 files changed, 290 insertions(+)
create mode 100644 hw/misc/bcm2835_property.c
create mode 100644 include/hw/misc/bcm2835_property.h
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index b9379f2..f4286f2 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -34,6 +34,7 @@ obj-$(CONFIG_OMAP) += omap_gpmc.o
obj-$(CONFIG_OMAP) += omap_l4.o
obj-$(CONFIG_OMAP) += omap_sdrc.o
obj-$(CONFIG_OMAP) += omap_tap.o
+obj-$(CONFIG_RASPI) += bcm2835_property.o
obj-$(CONFIG_RASPI) += bcm2835_sbm.o
obj-$(CONFIG_SLAVIO) += slavio_misc.o
obj-$(CONFIG_ZYNQ) += zynq_slcr.o
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
new file mode 100644
index 0000000..d25c056
--- /dev/null
+++ b/hw/misc/bcm2835_property.c
@@ -0,0 +1,262 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "hw/misc/bcm2835_property.h"
+#include "hw/arm/bcm2835_mbox.h"
+
+/* https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface */
+
+static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
+{
+ uint32_t tag;
+ uint32_t bufsize;
+ uint32_t tot_len;
+ size_t resplen;
+ uint32_t tmp;
+
+ value &= ~0xf;
+
+ s->addr = value;
+
+ tot_len = ldl_phys(&s->dma_as, value);
+
+ /* @(addr + 4) : Buffer response code */
+ value = s->addr + 8;
+ while (value + 8 <= s->addr + tot_len) {
+ tag = ldl_phys(&s->dma_as, value);
+ bufsize = ldl_phys(&s->dma_as, value + 4);
+ /* @(value + 8) : Request/response indicator */
+ resplen = 0;
+ switch (tag) {
+ case 0x00000000: /* End tag */
+ break;
+ case 0x00000001: /* Get firmware revision */
+ stl_phys(&s->dma_as, value + 12, 346337);
+ resplen = 4;
+ break;
+
+ case 0x00010001: /* Get board model */
+ resplen = 4;
+ break;
+ case 0x00010002: /* Get board revision */
+ resplen = 4;
+ break;
+ case 0x00010003: /* Get board MAC address */
+ /* write the first four bytes of the 6-byte MAC */
+ stl_phys(&s->dma_as, value + 12, 0xB827EBD0);
+ /* write the last two bytes, avoid any write past the buffer end */
+ stb_phys(&s->dma_as, value + 16, 0xEE);
+ stb_phys(&s->dma_as, value + 17, 0xDF);
+ resplen = 6;
+ break;
+ case 0x00010004: /* Get board serial */
+ resplen = 8;
+ break;
+ case 0x00010005: /* Get ARM memory */
+ /* base */
+ stl_phys(&s->dma_as, value + 12, 0);
+ /* size */
+ stl_phys(&s->dma_as, value + 16, s->ram_size);
+ resplen = 8;
+ break;
+ case 0x00028001: /* Set power state */
+ /* Assume that whatever device they asked for exists,
+ * and we'll just claim we set it to the desired state */
+ tmp = ldl_phys(&s->dma_as, value + 16);
+ stl_phys(&s->dma_as, value + 16, (tmp & 1));
+ resplen = 8;
+ break;
+
+ /* Clocks */
+
+ case 0x00030001: /* Get clock state */
+ stl_phys(&s->dma_as, value + 16, 0x1);
+ resplen = 8;
+ break;
+
+ case 0x00038001: /* Set clock state */
+ resplen = 8;
+ break;
+
+ case 0x00030002: /* Get clock rate */
+ case 0x00030004: /* Get max clock rate */
+ case 0x00030007: /* Get min clock rate */
+ switch (ldl_phys(&s->dma_as, value + 12)) {
+ case 1: /* EMMC */
+ stl_phys(&s->dma_as, value + 16, 50000000);
+ break;
+ case 2: /* UART */
+ stl_phys(&s->dma_as, value + 16, 3000000);
+ break;
+ default:
+ stl_phys(&s->dma_as, value + 16, 700000000);
+ break;
+ }
+ resplen = 8;
+ break;
+
+ case 0x00038002: /* Set clock rate */
+ case 0x00038004: /* Set max clock rate */
+ case 0x00038007: /* Set min clock rate */
+ resplen = 8;
+ break;
+
+ /* Temperature */
+
+ case 0x00030006: /* Get temperature */
+ stl_phys(&s->dma_as, value + 16, 25000);
+ resplen = 8;
+ break;
+
+ case 0x0003000A: /* Get max temperature */
+ stl_phys(&s->dma_as, value + 16, 99000);
+ resplen = 8;
+ break;
+
+
+ case 0x00060001: /* Get DMA channels */
+ /* channels 2-5 */
+ stl_phys(&s->dma_as, value + 12, 0x003C);
+ resplen = 4;
+ break;
+
+ case 0x00050001: /* Get command line */
+ resplen = 0;
+ break;
+
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2835_property: unhandled tag %08x\n", tag);
+ break;
+ }
+
+ if (tag == 0) {
+ break;
+ }
+
+ stl_phys(&s->dma_as, value + 8, (1 << 31) | resplen);
+ value += bufsize + 12;
+ }
+
+ /* Buffer response code */
+ stl_phys(&s->dma_as, s->addr + 4, (1 << 31));
+}
+
+static uint64_t bcm2835_property_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+ BCM2835PropertyState *s = (BCM2835PropertyState *)opaque;
+ uint32_t res = 0;
+
+ switch (offset) {
+ case 0:
+ res = MBOX_CHAN_PROPERTY | s->addr;
+ s->pending = 0;
+ qemu_set_irq(s->mbox_irq, 0);
+ break;
+ case 4:
+ res = s->pending;
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2835_property_read: Bad offset %x\n", (int)offset);
+ return 0;
+ }
+ return res;
+}
+
+static void bcm2835_property_write(void *opaque, hwaddr offset,
+ uint64_t value, unsigned size)
+{
+ BCM2835PropertyState *s = (BCM2835PropertyState *)opaque;
+ switch (offset) {
+ case 0:
+ if (!s->pending) {
+ s->pending = 1;
+ bcm2835_property_mbox_push(s, value);
+ qemu_set_irq(s->mbox_irq, 1);
+ }
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2835_property_write: Bad offset %x\n", (int)offset);
+ return;
+ }
+
+}
+
+
+static const MemoryRegionOps bcm2835_property_ops = {
+ .read = bcm2835_property_read,
+ .write = bcm2835_property_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+
+static const VMStateDescription vmstate_bcm2835_property = {
+ .name = TYPE_BCM2835_PROPERTY,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void bcm2835_property_init(Object *obj)
+{
+ BCM2835PropertyState *s = BCM2835_PROPERTY(obj);
+ memory_region_init_io(&s->iomem, OBJECT(s), &bcm2835_property_ops, s,
+ TYPE_BCM2835_PROPERTY, 0x10);
+ sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
+ sysbus_init_irq(SYS_BUS_DEVICE(s), &s->mbox_irq);
+}
+
+static void bcm2835_property_realize(DeviceState *dev, Error **errp)
+{
+ BCM2835PropertyState *s = BCM2835_PROPERTY(dev);
+ Object *obj;
+ Error *err = NULL;
+
+ obj = object_property_get_link(OBJECT(dev), "dma_mr", &err);
+ if (err || obj == NULL) {
+ error_setg(errp, "bcm2835_property: required dma_mr link not found");
+ return;
+ }
+
+ s->dma_mr = MEMORY_REGION(obj);
+ address_space_init(&s->dma_as, s->dma_mr, NULL);
+
+ s->pending = 0;
+}
+
+static Property bcm2835_property_props[] = {
+ DEFINE_PROP_UINT32("ram-size", BCM2835PropertyState, ram_size, 0),
+ DEFINE_PROP_END_OF_LIST()
+};
+
+static void bcm2835_property_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->props = bcm2835_property_props;
+ dc->realize = bcm2835_property_realize;
+ dc->vmsd = &vmstate_bcm2835_property;
+}
+
+static TypeInfo bcm2835_property_info = {
+ .name = TYPE_BCM2835_PROPERTY,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(BCM2835PropertyState),
+ .class_init = bcm2835_property_class_init,
+ .instance_init = bcm2835_property_init,
+};
+
+static void bcm2835_property_register_types(void)
+{
+ type_register_static(&bcm2835_property_info);
+}
+
+type_init(bcm2835_property_register_types)
diff --git a/include/hw/misc/bcm2835_property.h b/include/hw/misc/bcm2835_property.h
new file mode 100644
index 0000000..c952165
--- /dev/null
+++ b/include/hw/misc/bcm2835_property.h
@@ -0,0 +1,27 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2835_PROPERTY_H
+#define BCM2835_PROPERTY_H
+
+#include "hw/sysbus.h"
+#include "exec/address-spaces.h"
+
+#define TYPE_BCM2835_PROPERTY "bcm2835_property"
+#define BCM2835_PROPERTY(obj) \
+ OBJECT_CHECK(BCM2835PropertyState, (obj), TYPE_BCM2835_PROPERTY)
+
+typedef struct {
+ SysBusDevice busdev;
+ MemoryRegion *dma_mr;
+ AddressSpace dma_as;
+ MemoryRegion iomem;
+ uint32_t addr;
+ int pending;
+ qemu_irq mbox_irq;
+ uint32_t ram_size;
+} BCM2835PropertyState;
+
+#endif
--
2.5.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 3/8] bcm2835_ic: add bcm2835 interrupt controller
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
2015-12-04 6:01 ` [Qemu-devel] [PATCH 2/8] bcm2835_property: add bcm2835 property channel Andrew Baumann
@ 2015-12-04 6:01 ` Andrew Baumann
2015-12-06 5:19 ` Peter Crosthwaite
2015-12-04 6:01 ` [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller Andrew Baumann
` (6 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Andrew Baumann @ 2015-12-04 6:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, Andrew Baumann, qemu-arm, Paolo Bonzini
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
hw/intc/Makefile.objs | 1 +
hw/intc/bcm2835_ic.c | 234 +++++++++++++++++++++++++++++++++++++++++++
include/hw/intc/bcm2835_ic.h | 26 +++++
3 files changed, 261 insertions(+)
create mode 100644 hw/intc/bcm2835_ic.c
create mode 100644 include/hw/intc/bcm2835_ic.h
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 004b0c2..2ad1204 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -24,6 +24,7 @@ obj-$(CONFIG_GRLIB) += grlib_irqmp.o
obj-$(CONFIG_IOAPIC) += ioapic.o
obj-$(CONFIG_OMAP) += omap_intc.o
obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
+obj-$(CONFIG_RASPI) += bcm2835_ic.o
obj-$(CONFIG_SH4) += sh_intc.o
obj-$(CONFIG_XICS) += xics.o
obj-$(CONFIG_XICS_KVM) += xics_kvm.o
diff --git a/hw/intc/bcm2835_ic.c b/hw/intc/bcm2835_ic.c
new file mode 100644
index 0000000..2419575
--- /dev/null
+++ b/hw/intc/bcm2835_ic.c
@@ -0,0 +1,234 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+/* Heavily based on pl190.c, copyright terms below. */
+
+/*
+ * Arm PrimeCell PL190 Vector Interrupt Controller
+ *
+ * Copyright (c) 2006 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ */
+
+#include "hw/intc/bcm2835_ic.h"
+
+#define IR_B 2
+#define IR_1 0
+#define IR_2 1
+
+/* Update interrupts. */
+static void bcm2835_ic_update(BCM2835IcState *s)
+{
+ int set;
+ int i;
+
+ set = 0;
+ if (s->fiq_enable) {
+ set = s->level[s->fiq_select >> 5] & (1u << (s->fiq_select & 0x1f));
+ }
+ qemu_set_irq(s->fiq, set);
+
+ set = 0;
+ for (i = 0; i < 3; i++) {
+ set |= (s->level[i] & s->irq_enable[i]);
+ }
+ qemu_set_irq(s->irq, set);
+
+}
+
+static void bcm2835_ic_set_irq(void *opaque, int irq, int level)
+{
+ BCM2835IcState *s = (BCM2835IcState *)opaque;
+
+ if (irq >= 0 && irq <= 71) {
+ if (level) {
+ s->level[irq >> 5] |= 1u << (irq & 0x1f);
+ } else {
+ s->level[irq >> 5] &= ~(1u << (irq & 0x1f));
+ }
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2835_ic_set_irq: Bad irq %d\n", irq);
+ }
+
+ bcm2835_ic_update(s);
+}
+
+static const int irq_dups[] = { 7, 9, 10, 18, 19, 53, 54, 55, 56, 57, 62, -1 };
+
+static uint64_t bcm2835_ic_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+ BCM2835IcState *s = (BCM2835IcState *)opaque;
+ int i;
+ int p = 0;
+ uint32_t res = 0;
+
+ switch (offset) {
+ case 0x00: /* IRQ basic pending */
+ /* bits 0-7 - ARM irqs */
+ res = (s->level[IR_B] & s->irq_enable[IR_B]) & 0xff;
+ for (i = 0; i < 64; i++) {
+ if (i == irq_dups[p]) {
+ /* bits 10-20 - selected GPU irqs */
+ if (s->level[i >> 5] & s->irq_enable[i >> 5]
+ & (1u << (i & 0x1f))) {
+ res |= (1u << (10 + p));
+ }
+ p++;
+ } else {
+ /* bits 8-9 - one or more bits set in pending registers 1-2 */
+ if (s->level[i >> 5] & s->irq_enable[i >> 5]
+ & (1u << (i & 0x1f))) {
+ res |= (1u << (8 + (i >> 5)));
+ }
+ }
+ }
+ break;
+ case 0x04: /* IRQ pending 1 */
+ res = s->level[IR_1] & s->irq_enable[IR_1];
+ break;
+ case 0x08: /* IRQ pending 2 */
+ res = s->level[IR_2] & s->irq_enable[IR_2];
+ break;
+ case 0x0C: /* FIQ register */
+ res = (s->fiq_enable << 7) | s->fiq_select;
+ break;
+ case 0x10: /* Interrupt enable register 1 */
+ res = s->irq_enable[IR_1];
+ break;
+ case 0x14: /* Interrupt enable register 2 */
+ res = s->irq_enable[IR_2];
+ break;
+ case 0x18: /* Base interrupt enable register */
+ res = s->irq_enable[IR_B];
+ break;
+ case 0x1C: /* Interrupt disable register 1 */
+ res = ~s->irq_enable[IR_1];
+ break;
+ case 0x20: /* Interrupt disable register 2 */
+ res = ~s->irq_enable[IR_2];
+ break;
+ case 0x24: /* Base interrupt disable register */
+ res = ~s->irq_enable[IR_B];
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2835_ic_read: Bad offset %x\n", (int)offset);
+ return 0;
+ }
+
+ return res;
+}
+
+static void bcm2835_ic_write(void *opaque, hwaddr offset,
+ uint64_t val, unsigned size)
+{
+ BCM2835IcState *s = (BCM2835IcState *)opaque;
+
+ switch (offset) {
+ case 0x0C: /* FIQ register */
+ s->fiq_select = (val & 0x7f);
+ s->fiq_enable = (val >> 7) & 0x1;
+ break;
+ case 0x10: /* Interrupt enable register 1 */
+ s->irq_enable[IR_1] |= val;
+ break;
+ case 0x14: /* Interrupt enable register 2 */
+ s->irq_enable[IR_2] |= val;
+ break;
+ case 0x18: /* Base interrupt enable register */
+ s->irq_enable[IR_B] |= (val & 0xff);
+ break;
+ case 0x1C: /* Interrupt disable register 1 */
+ s->irq_enable[IR_1] &= ~val;
+ break;
+ case 0x20: /* Interrupt disable register 2 */
+ s->irq_enable[IR_2] &= ~val;
+ break;
+ case 0x24: /* Base interrupt disable register */
+ s->irq_enable[IR_B] &= (~val & 0xff);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2835_ic_write: Bad offset %x\n", (int)offset);
+ return;
+ }
+ bcm2835_ic_update(s);
+}
+
+static const MemoryRegionOps bcm2835_ic_ops = {
+ .read = bcm2835_ic_read,
+ .write = bcm2835_ic_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void bcm2835_ic_reset(DeviceState *d)
+{
+ BCM2835IcState *s = BCM2835_IC(d);
+ int i;
+
+ for (i = 0; i < 3; i++) {
+ s->irq_enable[i] = 0;
+ }
+ s->fiq_enable = 0;
+ s->fiq_select = 0;
+}
+
+static void bcm2835_ic_init(Object *obj)
+{
+ BCM2835IcState *s = BCM2835_IC(obj);
+
+ memory_region_init_io(&s->iomem, obj, &bcm2835_ic_ops, s, TYPE_BCM2835_IC,
+ 0x200);
+ sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
+
+ qdev_init_gpio_in(DEVICE(s), bcm2835_ic_set_irq, 72);
+ sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq);
+ sysbus_init_irq(SYS_BUS_DEVICE(s), &s->fiq);
+}
+
+static void bcm2835_ic_realize(DeviceState *dev, Error **errp)
+{
+}
+
+static const VMStateDescription vmstate_bcm2835_ic = {
+ .name = TYPE_BCM2835_IC,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(level, BCM2835IcState, 3),
+ VMSTATE_UINT32_ARRAY(irq_enable, BCM2835IcState, 3),
+ VMSTATE_INT32(fiq_enable, BCM2835IcState),
+ VMSTATE_INT32(fiq_select, BCM2835IcState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void bcm2835_ic_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = bcm2835_ic_realize;
+ dc->reset = bcm2835_ic_reset;
+ dc->vmsd = &vmstate_bcm2835_ic;
+}
+
+static TypeInfo bcm2835_ic_info = {
+ .name = TYPE_BCM2835_IC,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(BCM2835IcState),
+ .class_init = bcm2835_ic_class_init,
+ .instance_init = bcm2835_ic_init,
+};
+
+static void bcm2835_ic_register_types(void)
+{
+ type_register_static(&bcm2835_ic_info);
+}
+
+type_init(bcm2835_ic_register_types)
diff --git a/include/hw/intc/bcm2835_ic.h b/include/hw/intc/bcm2835_ic.h
new file mode 100644
index 0000000..428139d
--- /dev/null
+++ b/include/hw/intc/bcm2835_ic.h
@@ -0,0 +1,26 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2835_IC_H
+#define BCM2835_IC_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_BCM2835_IC "bcm2835_ic"
+#define BCM2835_IC(obj) OBJECT_CHECK(BCM2835IcState, (obj), TYPE_BCM2835_IC)
+
+typedef struct BCM2835IcState {
+ SysBusDevice busdev;
+ MemoryRegion iomem;
+
+ uint32_t level[3];
+ uint32_t irq_enable[3];
+ int fiq_enable;
+ int fiq_select;
+ qemu_irq irq;
+ qemu_irq fiq;
+} BCM2835IcState;
+
+#endif
--
2.5.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 3/8] bcm2835_ic: add bcm2835 interrupt controller
2015-12-04 6:01 ` [Qemu-devel] [PATCH 3/8] bcm2835_ic: add bcm2835 interrupt controller Andrew Baumann
@ 2015-12-06 5:19 ` Peter Crosthwaite
2015-12-09 6:25 ` Andrew Baumann
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-06 5:19 UTC (permalink / raw)
To: Andrew Baumann
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> ---
> hw/intc/Makefile.objs | 1 +
> hw/intc/bcm2835_ic.c | 234 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/intc/bcm2835_ic.h | 26 +++++
> 3 files changed, 261 insertions(+)
> create mode 100644 hw/intc/bcm2835_ic.c
> create mode 100644 include/hw/intc/bcm2835_ic.h
>
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 004b0c2..2ad1204 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -24,6 +24,7 @@ obj-$(CONFIG_GRLIB) += grlib_irqmp.o
> obj-$(CONFIG_IOAPIC) += ioapic.o
> obj-$(CONFIG_OMAP) += omap_intc.o
> obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
> +obj-$(CONFIG_RASPI) += bcm2835_ic.o
We are trying to encourage -'s for the separators in new file names,
although admittedly is does seem that intc/ is lagging here.
> obj-$(CONFIG_SH4) += sh_intc.o
> obj-$(CONFIG_XICS) += xics.o
> obj-$(CONFIG_XICS_KVM) += xics_kvm.o
> diff --git a/hw/intc/bcm2835_ic.c b/hw/intc/bcm2835_ic.c
> new file mode 100644
> index 0000000..2419575
> --- /dev/null
> +++ b/hw/intc/bcm2835_ic.c
> @@ -0,0 +1,234 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +/* Heavily based on pl190.c, copyright terms below. */
> +
> +/*
You can just make this one header comment rather than stop and start
block comments again.
> + * Arm PrimeCell PL190 Vector Interrupt Controller
> + *
> + * Copyright (c) 2006 CodeSourcery.
> + * Written by Paul Brook
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +#include "hw/intc/bcm2835_ic.h"
> +
> +#define IR_B 2
IR_B should move down. Although some of my following comment may
obsolete this def anyway.
> +#define IR_1 0
> +#define IR_2 1
> +
> +/* Update interrupts. */
> +static void bcm2835_ic_update(BCM2835IcState *s)
> +{
> + int set;
> + int i;
> +
> + set = 0;
> + if (s->fiq_enable) {
> + set = s->level[s->fiq_select >> 5] & (1u << (s->fiq_select & 0x1f));
> + }
> + qemu_set_irq(s->fiq, set);
> +
> + set = 0;
> + for (i = 0; i < 3; i++) {
> + set |= (s->level[i] & s->irq_enable[i]);
> + }
> + qemu_set_irq(s->irq, set);
> +
> +}
> +
> +static void bcm2835_ic_set_irq(void *opaque, int irq, int level)
> +{
> + BCM2835IcState *s = (BCM2835IcState *)opaque;
> +
> + if (irq >= 0 && irq <= 71) {
I assume the 72 comes from 64 GPU interrupts +8 ARM interrupts.
Looking at the TRM, these seem to be different enumeration schemes and
not considered one interrupt list. You should use named GPIOs to split
it into the two named sets "GPU" and "ARM", I notice in patch 5 you
have:
+#define ARM_IRQ0_BASE 64
To convert the ARM IRQ indexing scheme to the GPU one. This would go
away with the separate name sets.
> + if (level) {
> + s->level[irq >> 5] |= 1u << (irq & 0x1f);
It also means you can have a single uint64_t (perhaps named
"gpu_level") in the state to hold the full GPU interrupt list,
avoiding the need for the >> 5 ... & 0x1f logic that you need a few
times. The ARM interrupts then become their own separate uint8_t
(arm_level?).
> + } else {
> + s->level[irq >> 5] &= ~(1u << (irq & 0x1f));
> + }
So these 5 lines will reduce to:
s->gpu_level = deposit64(s->gpu_level, index, 1, !!level);
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "bcm2835_ic_set_irq: Bad irq %d\n", irq);
As you explicitly define the length of the interrupts list at 72, this
would be fatal and should g_assert_not_reached(). But I would drip the
if-else (and the check) altogether as the GPIO API should be doing its
own checking.
> + }
> +
> + bcm2835_ic_update(s);
> +}
> +
> +static const int irq_dups[] = { 7, 9, 10, 18, 19, 53, 54, 55, 56, 57, 62, -1 };
> +
> +static uint64_t bcm2835_ic_read(void *opaque, hwaddr offset,
> + unsigned size)
> +{
> + BCM2835IcState *s = (BCM2835IcState *)opaque;
Cast unneeded.
> + int i;
> + int p = 0;
> + uint32_t res = 0;
> +
> + switch (offset) {
> + case 0x00: /* IRQ basic pending */
Rather than commenting the hex, define macros for the register map
that are then used for both read and write switches.
> + /* bits 0-7 - ARM irqs */
> + res = (s->level[IR_B] & s->irq_enable[IR_B]) & 0xff;
> + for (i = 0; i < 64; i++) {
> + if (i == irq_dups[p]) {
> + /* bits 10-20 - selected GPU irqs */
> + if (s->level[i >> 5] & s->irq_enable[i >> 5]
> + & (1u << (i & 0x1f))) {
> + res |= (1u << (10 + p));
This will get simpler with the unit64_t GPU interrupt list.
> + }
> + p++;
> + } else {
> + /* bits 8-9 - one or more bits set in pending registers 1-2 */
> + if (s->level[i >> 5] & s->irq_enable[i >> 5]
> + & (1u << (i & 0x1f))) {
The common subexpression
s->level[i >> 5] & s->irq_enable[i >> 5] & (1u << (i & 0x1f))
can be avoided with a local variable (bool irq_pending).
> + res |= (1u << (8 + (i >> 5)));
A nit, but i /32 better documents the intent of the shift.
> + }
> + }
> + }
> + break;
> + case 0x04: /* IRQ pending 1 */
> + res = s->level[IR_1] & s->irq_enable[IR_1];
extract64 with a case-fallthrough would be usable for getting the two
halves from a uint64_t:
something like:
case 0x08:
shift = 32;
/* fallthrough */
case 0x04:
res = extract64(s->gpu_level, shift, 32) &
extract64(s->gpu_irq_enable, shift, 32);
break;
> + break;
> + case 0x08: /* IRQ pending 2 */
> + res = s->level[IR_2] & s->irq_enable[IR_2];
> + break;
> + case 0x0C: /* FIQ register */
> + res = (s->fiq_enable << 7) | s->fiq_select;
> + break;
> + case 0x10: /* Interrupt enable register 1 */
> + res = s->irq_enable[IR_1];
irq_enable would also be split to gpu_irq_enable and arm_irq_enable
separate variables under split GPIO namespace implementation.
> + break;
> + case 0x14: /* Interrupt enable register 2 */
> + res = s->irq_enable[IR_2];
> + break;
> + case 0x18: /* Base interrupt enable register */
> + res = s->irq_enable[IR_B];
> + break;
> + case 0x1C: /* Interrupt disable register 1 */
> + res = ~s->irq_enable[IR_1];
> + break;
> + case 0x20: /* Interrupt disable register 2 */
> + res = ~s->irq_enable[IR_2];
> + break;
> + case 0x24: /* Base interrupt disable register */
> + res = ~s->irq_enable[IR_B];
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "bcm2835_ic_read: Bad offset %x\n", (int)offset);
%s __func__ for the function name component. This guards against the
message going stale if the function is renamed.
Use HWADDR_PRIx rather than cast the offset.
> + return 0;
> + }
> +
> + return res;
> +}
> +
> +static void bcm2835_ic_write(void *opaque, hwaddr offset,
> + uint64_t val, unsigned size)
Indentation of continued argument lists should indent to one past the
"(" which started the list:
static void bcm2835_ic_write(void *opaque, hwaddr offset,
uint64_t val, unsigned size)
> +{
> + BCM2835IcState *s = (BCM2835IcState *)opaque;
Cast unneeded.
> +
> + switch (offset) {
> + case 0x0C: /* FIQ register */
> + s->fiq_select = (val & 0x7f);
> + s->fiq_enable = (val >> 7) & 0x1;
> + break;
> + case 0x10: /* Interrupt enable register 1 */
> + s->irq_enable[IR_1] |= val;
> + break;
> + case 0x14: /* Interrupt enable register 2 */
> + s->irq_enable[IR_2] |= val;
> + break;
> + case 0x18: /* Base interrupt enable register */
> + s->irq_enable[IR_B] |= (val & 0xff);
> + break;
> + case 0x1C: /* Interrupt disable register 1 */
> + s->irq_enable[IR_1] &= ~val;
> + break;
> + case 0x20: /* Interrupt disable register 2 */
> + s->irq_enable[IR_2] &= ~val;
> + break;
> + case 0x24: /* Base interrupt disable register */
> + s->irq_enable[IR_B] &= (~val & 0xff);
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "bcm2835_ic_write: Bad offset %x\n", (int)offset);
> + return;
> + }
> + bcm2835_ic_update(s);
> +}
> +
> +static const MemoryRegionOps bcm2835_ic_ops = {
> + .read = bcm2835_ic_read,
> + .write = bcm2835_ic_write,
You do not handle anything that is not a word access so you should add
the min_access_size and max_access_size definitions.
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void bcm2835_ic_reset(DeviceState *d)
> +{
> + BCM2835IcState *s = BCM2835_IC(d);
> + int i;
> +
> + for (i = 0; i < 3; i++) {
> + s->irq_enable[i] = 0;
> + }
> + s->fiq_enable = 0;
> + s->fiq_select = 0;
> +}
> +
> +static void bcm2835_ic_init(Object *obj)
> +{
> + BCM2835IcState *s = BCM2835_IC(obj);
> +
> + memory_region_init_io(&s->iomem, obj, &bcm2835_ic_ops, s, TYPE_BCM2835_IC,
> + 0x200);
> + sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
> +
> + qdev_init_gpio_in(DEVICE(s), bcm2835_ic_set_irq, 72);
This is where the split would happen:
qdev_init_gpio_in_named(DEVICE(s), bcm2835_ic_set_irq, BCM2835_IC_GPU_IRQ, 64);
qdev_init_gpio_in_named(DEVICE(s), bcm2835_ic_set_irq, BCM2835_IC_ARM_IRQ, 8);
Where the two macros are something like "gpu-irq" and "arm-irq".
> + sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq);
> + sysbus_init_irq(SYS_BUS_DEVICE(s), &s->fiq);
> +}
> +
> +static void bcm2835_ic_realize(DeviceState *dev, Error **errp)
> +{
> +}
Nop realize is not needed. Just leave the callback unset in class_init.
> +
> +static const VMStateDescription vmstate_bcm2835_ic = {
> + .name = TYPE_BCM2835_IC,
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(level, BCM2835IcState, 3),
> + VMSTATE_UINT32_ARRAY(irq_enable, BCM2835IcState, 3),
> + VMSTATE_INT32(fiq_enable, BCM2835IcState),
> + VMSTATE_INT32(fiq_select, BCM2835IcState),
fiq_select should be a bool.
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void bcm2835_ic_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = bcm2835_ic_realize;
So this line can be dropped.
> + dc->reset = bcm2835_ic_reset;
> + dc->vmsd = &vmstate_bcm2835_ic;
> +}
> +
> +static TypeInfo bcm2835_ic_info = {
> + .name = TYPE_BCM2835_IC,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(BCM2835IcState),
> + .class_init = bcm2835_ic_class_init,
> + .instance_init = bcm2835_ic_init,
> +};
> +
> +static void bcm2835_ic_register_types(void)
> +{
> + type_register_static(&bcm2835_ic_info);
> +}
> +
> +type_init(bcm2835_ic_register_types)
> diff --git a/include/hw/intc/bcm2835_ic.h b/include/hw/intc/bcm2835_ic.h
> new file mode 100644
> index 0000000..428139d
> --- /dev/null
> +++ b/include/hw/intc/bcm2835_ic.h
> @@ -0,0 +1,26 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +#ifndef BCM2835_IC_H
> +#define BCM2835_IC_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_BCM2835_IC "bcm2835_ic"
> +#define BCM2835_IC(obj) OBJECT_CHECK(BCM2835IcState, (obj), TYPE_BCM2835_IC)
> +
> +typedef struct BCM2835IcState {
Add the comments:
/*< private >*/
> + SysBusDevice busdev;
and:
/*< public >*/
As they make some auto-generated documentation nicer.
> + MemoryRegion iomem;
> +
> + uint32_t level[3];
> + uint32_t irq_enable[3];
> + int fiq_enable;
You migrate this as a uint32_t so it must be a uint32_t here as well.
> + int fiq_select;
> + qemu_irq irq;
> + qemu_irq fiq;
> +} BCM2835IcState;
BCM2835ICState. Acronyms in camel case should be all caps.
Regards,
Peter
> +
> +#endif
> --
> 2.5.3
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 3/8] bcm2835_ic: add bcm2835 interrupt controller
2015-12-06 5:19 ` Peter Crosthwaite
@ 2015-12-09 6:25 ` Andrew Baumann
0 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-09 6:25 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
Peter,
Thanks for the feedback on this patch. I agree with all of it, but I do have one minor quibble...
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Saturday, 5 December 2015 21:20
> On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> > --- a/hw/intc/Makefile.objs
> > +++ b/hw/intc/Makefile.objs
> > @@ -24,6 +24,7 @@ obj-$(CONFIG_GRLIB) += grlib_irqmp.o
> > obj-$(CONFIG_IOAPIC) += ioapic.o
> > obj-$(CONFIG_OMAP) += omap_intc.o
> > obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
> > +obj-$(CONFIG_RASPI) += bcm2835_ic.o
>
> We are trying to encourage -'s for the separators in new file names,
> although admittedly is does seem that intc/ is lagging here.
Sigh. I can rename all the new files if you insist, but what's the rationale for this?
Just for the record:
% find qemu -iname *.[ch] | grep -c -
1728
% find qemu -iname *.[ch] | grep -c _
1166
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
2015-12-04 6:01 ` [Qemu-devel] [PATCH 2/8] bcm2835_property: add bcm2835 property channel Andrew Baumann
2015-12-04 6:01 ` [Qemu-devel] [PATCH 3/8] bcm2835_ic: add bcm2835 interrupt controller Andrew Baumann
@ 2015-12-04 6:01 ` Andrew Baumann
2015-12-06 5:25 ` Peter Crosthwaite
2015-12-04 6:01 ` [Qemu-devel] [PATCH 5/8] bcm2835_peripherals: add rollup device for bcm2835 peripherals Andrew Baumann
` (5 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Andrew Baumann @ 2015-12-04 6:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, Andrew Baumann, qemu-arm, Paolo Bonzini
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
hw/sd/Makefile.objs | 1 +
hw/sd/bcm2835_emmc.c | 800 +++++++++++++++++++++++++++++++++++++++++++
include/hw/sd/bcm2835_emmc.h | 56 +++
3 files changed, 857 insertions(+)
create mode 100644 hw/sd/bcm2835_emmc.c
create mode 100644 include/hw/sd/bcm2835_emmc.h
diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
index f1aed83..b9cb9b8 100644
--- a/hw/sd/Makefile.objs
+++ b/hw/sd/Makefile.objs
@@ -6,3 +6,4 @@ common-obj-$(CONFIG_SDHCI) += sdhci.o
obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
obj-$(CONFIG_OMAP) += omap_mmc.o
obj-$(CONFIG_PXA2XX) += pxa2xx_mmci.o
+obj-$(CONFIG_RASPI) += bcm2835_emmc.o
diff --git a/hw/sd/bcm2835_emmc.c b/hw/sd/bcm2835_emmc.c
new file mode 100644
index 0000000..46628b5
--- /dev/null
+++ b/hw/sd/bcm2835_emmc.c
@@ -0,0 +1,800 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "sysemu/blockdev.h"
+#include "hw/sd/bcm2835_emmc.h"
+
+/*
+ * Controller registers
+ */
+
+#define SDHCI_DMA_ADDRESS 0x00
+#define SDHCI_ARGUMENT2 SDHCI_DMA_ADDRESS
+
+#define SDHCI_BLOCK_SIZE 0x04
+#define SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz & 0xFFF))
+
+#define SDHCI_BLOCK_COUNT 0x06
+
+#define SDHCI_ARGUMENT 0x08
+
+#define SDHCI_TRANSFER_MODE 0x0C
+#define SDHCI_TRNS_DMA 0x01
+#define SDHCI_TRNS_BLK_CNT_EN 0x02
+#define SDHCI_TRNS_AUTO_CMD12 0x04
+#define SDHCI_TRNS_AUTO_CMD23 0x08
+#define SDHCI_TRNS_READ 0x10
+#define SDHCI_TRNS_MULTI 0x20
+
+#define SDHCI_COMMAND 0x0E
+#define SDHCI_CMD_RESP_MASK 0x03
+#define SDHCI_CMD_CRC 0x08
+#define SDHCI_CMD_INDEX 0x10
+#define SDHCI_CMD_DATA 0x20
+#define SDHCI_CMD_ABORTCMD 0xC0
+
+#define SDHCI_CMD_RESP_NONE 0x00
+#define SDHCI_CMD_RESP_LONG 0x01
+#define SDHCI_CMD_RESP_SHORT 0x02
+#define SDHCI_CMD_RESP_SHORT_BUSY 0x03
+
+#define SDHCI_MAKE_CMD(c, f) (((c & 0xff) << 8) | (f & 0xff))
+#define SDHCI_GET_CMD(c) ((c>>8) & 0x3f)
+
+#define SDHCI_RESPONSE 0x10
+
+#define SDHCI_BUFFER 0x20
+
+#define SDHCI_PRESENT_STATE 0x24
+#define SDHCI_CMD_INHIBIT 0x00000001
+#define SDHCI_DATA_INHIBIT 0x00000002
+#define SDHCI_DOING_WRITE 0x00000100
+#define SDHCI_DOING_READ 0x00000200
+#define SDHCI_SPACE_AVAILABLE 0x00000400
+#define SDHCI_DATA_AVAILABLE 0x00000800
+#define SDHCI_CARD_PRESENT 0x00010000
+#define SDHCI_WRITE_PROTECT 0x00080000
+#define SDHCI_DATA_LVL_MASK 0x00F00000
+#define SDHCI_DATA_LVL_SHIFT 20
+
+#define SDHCI_HOST_CONTROL 0x28
+#define SDHCI_CTRL_LED 0x01
+#define SDHCI_CTRL_4BITBUS 0x02
+#define SDHCI_CTRL_HISPD 0x04
+#define SDHCI_CTRL_DMA_MASK 0x18
+#define SDHCI_CTRL_SDMA 0x00
+#define SDHCI_CTRL_ADMA1 0x08
+#define SDHCI_CTRL_ADMA32 0x10
+#define SDHCI_CTRL_ADMA64 0x18
+#define SDHCI_CTRL_8BITBUS 0x20
+
+#define SDHCI_POWER_CONTROL 0x29
+#define SDHCI_POWER_ON 0x01
+#define SDHCI_POWER_180 0x0A
+#define SDHCI_POWER_300 0x0C
+#define SDHCI_POWER_330 0x0E
+
+#define SDHCI_BLOCK_GAP_CONTROL 0x2A
+
+#define SDHCI_WAKE_UP_CONTROL 0x2B
+#define SDHCI_WAKE_ON_INT 0x01
+#define SDHCI_WAKE_ON_INSERT 0x02
+#define SDHCI_WAKE_ON_REMOVE 0x04
+
+#define SDHCI_CLOCK_CONTROL 0x2C
+#define SDHCI_DIVIDER_SHIFT 8
+#define SDHCI_DIVIDER_HI_SHIFT 6
+#define SDHCI_DIV_MASK 0xFF
+#define SDHCI_DIV_MASK_LEN 8
+#define SDHCI_DIV_HI_MASK 0x300
+#define SDHCI_PROG_CLOCK_MODE 0x0020
+#define SDHCI_CLOCK_CARD_EN 0x0004
+#define SDHCI_CLOCK_INT_STABLE 0x0002
+#define SDHCI_CLOCK_INT_EN 0x0001
+
+#define SDHCI_TIMEOUT_CONTROL 0x2E
+
+#define SDHCI_SOFTWARE_RESET 0x2F
+#define SDHCI_RESET_ALL 0x01
+#define SDHCI_RESET_CMD 0x02
+#define SDHCI_RESET_DATA 0x04
+
+#define SDHCI_INT_STATUS 0x30
+#define SDHCI_INT_ENABLE 0x34
+#define SDHCI_SIGNAL_ENABLE 0x38
+#define SDHCI_INT_RESPONSE 0x00000001
+#define SDHCI_INT_DATA_END 0x00000002
+#define SDHCI_INT_DMA_END 0x00000008
+#define SDHCI_INT_SPACE_AVAIL 0x00000010
+#define SDHCI_INT_DATA_AVAIL 0x00000020
+#define SDHCI_INT_CARD_INSERT 0x00000040
+#define SDHCI_INT_CARD_REMOVE 0x00000080
+#define SDHCI_INT_CARD_INT 0x00000100
+#define SDHCI_INT_ERROR 0x00008000
+#define SDHCI_INT_TIMEOUT 0x00010000
+#define SDHCI_INT_CRC 0x00020000
+#define SDHCI_INT_END_BIT 0x00040000
+#define SDHCI_INT_INDEX 0x00080000
+#define SDHCI_INT_DATA_TIMEOUT 0x00100000
+#define SDHCI_INT_DATA_CRC 0x00200000
+#define SDHCI_INT_DATA_END_BIT 0x00400000
+#define SDHCI_INT_BUS_POWER 0x00800000
+#define SDHCI_INT_ACMD12ERR 0x01000000
+#define SDHCI_INT_ADMA_ERROR 0x02000000
+
+#define SDHCI_INT_NORMAL_MASK 0x00007FFF
+#define SDHCI_INT_ERROR_MASK 0xFFFF8000
+
+#define SDHCI_INT_CMD_MASK (SDHCI_INT_RESPONSE | SDHCI_INT_TIMEOUT | \
+ SDHCI_INT_CRC | SDHCI_INT_END_BIT | SDHCI_INT_INDEX)
+#define SDHCI_INT_DATA_MASK (SDHCI_INT_DATA_END | SDHCI_INT_DMA_END | \
+ SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL | \
+ SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
+ SDHCI_INT_DATA_END_BIT | SDHCI_INT_ADMA_ERROR)
+#define SDHCI_INT_ALL_MASK ((unsigned int)-1)
+
+/* Per the BCM2835 ARM peripherals document (p76), some interrupts are
+ * not implemented (reserved) */
+#define BCM2835_DISABLED_INTS (SDHCI_INT_DMA_END | SDHCI_INT_CARD_INSERT \
+ | SDHCI_INT_CARD_REMOVE)
+
+#define SDHCI_ACMD12_ERR 0x3C
+
+#define SDHCI_HOST_CONTROL2 0x3E
+#define SDHCI_CTRL_UHS_MASK 0x0007
+#define SDHCI_CTRL_UHS_SDR12 0x0000
+#define SDHCI_CTRL_UHS_SDR25 0x0001
+#define SDHCI_CTRL_UHS_SDR50 0x0002
+#define SDHCI_CTRL_UHS_SDR104 0x0003
+#define SDHCI_CTRL_UHS_DDR50 0x0004
+#define SDHCI_CTRL_VDD_180 0x0008
+#define SDHCI_CTRL_DRV_TYPE_MASK 0x0030
+#define SDHCI_CTRL_DRV_TYPE_B 0x0000
+#define SDHCI_CTRL_DRV_TYPE_A 0x0010
+#define SDHCI_CTRL_DRV_TYPE_C 0x0020
+#define SDHCI_CTRL_DRV_TYPE_D 0x0030
+#define SDHCI_CTRL_EXEC_TUNING 0x0040
+#define SDHCI_CTRL_TUNED_CLK 0x0080
+#define SDHCI_CTRL_PRESET_VAL_ENABLE 0x8000
+
+#define SDHCI_CAPABILITIES 0x40
+#define SDHCI_TIMEOUT_CLK_MASK 0x0000003F
+#define SDHCI_TIMEOUT_CLK_SHIFT 0
+#define SDHCI_TIMEOUT_CLK_UNIT 0x00000080
+#define SDHCI_CLOCK_BASE_MASK 0x00003F00
+#define SDHCI_CLOCK_V3_BASE_MASK 0x0000FF00
+#define SDHCI_CLOCK_BASE_SHIFT 8
+#define SDHCI_MAX_BLOCK_MASK 0x00030000
+#define SDHCI_MAX_BLOCK_SHIFT 16
+#define SDHCI_CAN_DO_8BIT 0x00040000
+#define SDHCI_CAN_DO_ADMA2 0x00080000
+#define SDHCI_CAN_DO_ADMA1 0x00100000
+#define SDHCI_CAN_DO_HISPD 0x00200000
+#define SDHCI_CAN_DO_SDMA 0x00400000
+#define SDHCI_CAN_VDD_330 0x01000000
+#define SDHCI_CAN_VDD_300 0x02000000
+#define SDHCI_CAN_VDD_180 0x04000000
+#define SDHCI_CAN_64BIT 0x10000000
+
+#define SDHCI_SUPPORT_SDR50 0x00000001
+#define SDHCI_SUPPORT_SDR104 0x00000002
+#define SDHCI_SUPPORT_DDR50 0x00000004
+#define SDHCI_DRIVER_TYPE_A 0x00000010
+#define SDHCI_DRIVER_TYPE_C 0x00000020
+#define SDHCI_DRIVER_TYPE_D 0x00000040
+#define SDHCI_RETUNING_TIMER_COUNT_MASK 0x00000F00
+#define SDHCI_RETUNING_TIMER_COUNT_SHIFT 8
+#define SDHCI_USE_SDR50_TUNING 0x00002000
+#define SDHCI_RETUNING_MODE_MASK 0x0000C000
+#define SDHCI_RETUNING_MODE_SHIFT 14
+#define SDHCI_CLOCK_MUL_MASK 0x00FF0000
+#define SDHCI_CLOCK_MUL_SHIFT 16
+
+#define SDHCI_CAPABILITIES_1 0x44
+
+#define SDHCI_MAX_CURRENT 0x48
+#define SDHCI_MAX_CURRENT_330_MASK 0x0000FF
+#define SDHCI_MAX_CURRENT_330_SHIFT 0
+#define SDHCI_MAX_CURRENT_300_MASK 0x00FF00
+#define SDHCI_MAX_CURRENT_300_SHIFT 8
+#define SDHCI_MAX_CURRENT_180_MASK 0xFF0000
+#define SDHCI_MAX_CURRENT_180_SHIFT 16
+#define SDHCI_MAX_CURRENT_MULTIPLIER 4
+
+/* 4C-4F reserved for more max current */
+
+#define SDHCI_SET_ACMD12_ERROR 0x50
+#define SDHCI_SET_INT_ERROR 0x52
+
+#define SDHCI_ADMA_ERROR 0x54
+
+/* 55-57 reserved */
+
+#define SDHCI_ADMA_ADDRESS 0x58
+
+/* 60-FB reserved */
+
+#define SDHCI_SLOT_INT_STATUS 0xFC
+
+#define SDHCI_HOST_VERSION 0xFE
+#define SDHCI_VENDOR_VER_MASK 0xFF00
+#define SDHCI_VENDOR_VER_SHIFT 8
+#define SDHCI_SPEC_VER_MASK 0x00FF
+#define SDHCI_SPEC_VER_SHIFT 0
+#define SDHCI_SPEC_100 0
+#define SDHCI_SPEC_200 1
+#define SDHCI_SPEC_300 2
+
+/*
+ * End of controller registers.
+ */
+#define MMC_VDD_165_195 0x00000080 /* VDD voltage 1.65 - 1.95 */
+#define MMC_VDD_20_21 0x00000100 /* VDD voltage 2.0 ~ 2.1 */
+#define MMC_VDD_21_22 0x00000200 /* VDD voltage 2.1 ~ 2.2 */
+#define MMC_VDD_22_23 0x00000400 /* VDD voltage 2.2 ~ 2.3 */
+#define MMC_VDD_23_24 0x00000800 /* VDD voltage 2.3 ~ 2.4 */
+#define MMC_VDD_24_25 0x00001000 /* VDD voltage 2.4 ~ 2.5 */
+#define MMC_VDD_25_26 0x00002000 /* VDD voltage 2.5 ~ 2.6 */
+#define MMC_VDD_26_27 0x00004000 /* VDD voltage 2.6 ~ 2.7 */
+#define MMC_VDD_27_28 0x00008000 /* VDD voltage 2.7 ~ 2.8 */
+#define MMC_VDD_28_29 0x00010000 /* VDD voltage 2.8 ~ 2.9 */
+#define MMC_VDD_29_30 0x00020000 /* VDD voltage 2.9 ~ 3.0 */
+#define MMC_VDD_30_31 0x00040000 /* VDD voltage 3.0 ~ 3.1 */
+#define MMC_VDD_31_32 0x00080000 /* VDD voltage 3.1 ~ 3.2 */
+#define MMC_VDD_32_33 0x00100000 /* VDD voltage 3.2 ~ 3.3 */
+#define MMC_VDD_33_34 0x00200000 /* VDD voltage 3.3 ~ 3.4 */
+#define MMC_VDD_34_35 0x00400000 /* VDD voltage 3.4 ~ 3.5 */
+#define MMC_VDD_35_36 0x00800000 /* VDD voltage 3.5 ~ 3.6 */
+
+#define MMC_CAP_4_BIT_DATA (1 << 0) /* Can the host do 4 bit transfers */
+#define MMC_CAP_MMC_HIGHSPEED (1 << 1) /* Can do MMC high-speed timing */
+#define MMC_CAP_SD_HIGHSPEED (1 << 2) /* Can do SD high-speed timing */
+#define MMC_CAP_SDIO_IRQ (1 << 3) /* Can signal pending SDIO IRQs */
+#define MMC_CAP_SPI (1 << 4) /* Talks only SPI protocols */
+#define MMC_CAP_NEEDS_POLL (1 << 5) /* Needs polling for card-detection */
+#define MMC_CAP_8_BIT_DATA (1 << 6) /* Can the host do 8 bit transfers */
+#define MMC_CAP_DISABLE (1 << 7) /* Can the host be disabled */
+#define MMC_CAP_NONREMOVABLE (1 << 8) /* Nonremovable e.g. eMMC */
+#define MMC_CAP_WAIT_WHILE_BUSY (1 << 9) /* Waits while card is busy */
+#define MMC_CAP_ERASE (1 << 10) /* Allow erase/trim commands */
+#define MMC_CAP_1_8V_DDR (1 << 11) /* can support */
+ /* DDR mode at 1.8V */
+#define MMC_CAP_1_2V_DDR (1 << 12) /* can support */
+ /* DDR mode at 1.2V */
+#define MMC_CAP_POWER_OFF_CARD (1 << 13) /* Can power off after boot */
+#define MMC_CAP_BUS_WIDTH_TEST (1 << 14) /* CMD14/CMD19 bus width ok */
+#define MMC_CAP_UHS_SDR12 (1 << 15) /* Host supports UHS SDR12 mode */
+#define MMC_CAP_UHS_SDR25 (1 << 16) /* Host supports UHS SDR25 mode */
+#define MMC_CAP_UHS_SDR50 (1 << 17) /* Host supports UHS SDR50 mode */
+#define MMC_CAP_UHS_SDR104 (1 << 18) /* Host supports UHS SDR104 mode */
+#define MMC_CAP_UHS_DDR50 (1 << 19) /* Host supports UHS DDR50 mode */
+#define MMC_CAP_SET_XPC_330 (1 << 20) /* Host supports >150mA current at 3.3V */
+#define MMC_CAP_SET_XPC_300 (1 << 21) /* Host supports >150mA current at 3.0V */
+#define MMC_CAP_SET_XPC_180 (1 << 22) /* Host supports >150mA current at 1.8V */
+#define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */
+#define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */
+#define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */
+#define MMC_CAP_MAX_CURRENT_200 (1 << 26) /* Host max current limit is 200mA */
+#define MMC_CAP_MAX_CURRENT_400 (1 << 27) /* Host max current limit is 400mA */
+#define MMC_CAP_MAX_CURRENT_600 (1 << 28) /* Host max current limit is 600mA */
+#define MMC_CAP_MAX_CURRENT_800 (1 << 29) /* Host max current limit is 800mA */
+#define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */
+#define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */
+
+
+#define MMC_CAP2_BOOTPART_NOACC (1 << 0) /* Boot partition no access */
+#define MMC_CAP2_CACHE_CTRL (1 << 1) /* Allow cache control */
+#define MMC_CAP2_POWEROFF_NOTIFY (1 << 2) /* Notify poweroff supported */
+#define MMC_CAP2_NO_MULTI_READ (1 << 3) /* Multiblock reads don't work */
+#define MMC_CAP2_FORCE_MULTIBLOCK (1 << 4) /* Always use multiblock transfers */
+
+#define COMPLETION_DELAY (100000)
+
+static void bcm2835_emmc_set_irq(BCM2835EmmcState *s)
+{
+ /* the error bit must be set iff there are any other errors */
+ assert(((s->interrupt & SDHCI_INT_ERROR) == 0)
+ == ((s->interrupt & SDHCI_INT_ERROR_MASK & ~SDHCI_INT_ERROR) == 0));
+
+ if (s->irpt_en & s->irpt_mask & s->interrupt & ~BCM2835_DISABLED_INTS) {
+ qemu_set_irq(s->irq, 1);
+ } else {
+ qemu_set_irq(s->irq, 0);
+ }
+}
+
+static void autocmd12(BCM2835EmmcState *s)
+{
+ SDRequest request;
+ uint8_t response[16];
+
+ if (!(s->cmdtm & SDHCI_TRNS_AUTO_CMD12)) {
+ return;
+ }
+
+ request.cmd = 12;
+ request.arg = 0;
+ request.crc = 0;
+ sd_do_command(s->card, &request, response);
+}
+
+static void autocmd23(BCM2835EmmcState *s)
+{
+ SDRequest request;
+ uint8_t response[16];
+
+ if (!(s->cmdtm & SDHCI_TRNS_AUTO_CMD23)) {
+ return;
+ }
+
+ request.cmd = 23;
+ request.arg = (s->blksizecnt >> 16) & 0xffff;
+ request.crc = 0;
+ sd_do_command(s->card, &request, response);
+}
+
+static void delayed_completion(void *opaque)
+{
+ BCM2835EmmcState *s = (BCM2835EmmcState *)opaque;
+
+ s->interrupt |= SDHCI_INT_DATA_END;
+ autocmd12(s);
+
+ bcm2835_emmc_set_irq(s);
+}
+
+
+static uint64_t bcm2835_emmc_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+ BCM2835EmmcState *s = (BCM2835EmmcState *)opaque;
+ uint32_t res = 0;
+ uint8_t tmp = 0;
+ int set_irq = 0;
+ uint32_t blkcnt;
+ uint8_t cmd;
+ int64_t now;
+
+ assert(size == 4);
+
+ switch (offset) {
+ case SDHCI_ARGUMENT2: /* ARG2 */
+ res = s->arg2;
+ break;
+ case SDHCI_BLOCK_SIZE: /* BLKSIZECNT */
+ res = s->blksizecnt;
+ break;
+ case SDHCI_ARGUMENT: /* ARG1 */
+ res = s->arg1;
+ break;
+ case SDHCI_TRANSFER_MODE: /* CMDTM */
+ res = s->cmdtm;
+ break;
+ case SDHCI_RESPONSE+0: /* RESP0 */
+ res = s->resp0;
+ break;
+ case SDHCI_RESPONSE+4: /* RESP1 */
+ res = s->resp1;
+ break;
+ case SDHCI_RESPONSE+8: /* RESP2 */
+ res = s->resp2;
+ break;
+ case SDHCI_RESPONSE+12: /* RESP3 */
+ res = s->resp3;
+ break;
+ case SDHCI_BUFFER: /* DATA */
+ cmd = ((s->cmdtm >> (16 + 8)) & 0x3f);
+
+ s->data = 0;
+ tmp = sd_read_data(s->card);
+ s->data |= (tmp << 0);
+ tmp = sd_read_data(s->card);
+ s->data |= (tmp << 8);
+ tmp = sd_read_data(s->card);
+ s->data |= (tmp << 16);
+ tmp = sd_read_data(s->card);
+ s->data |= (tmp << 24);
+
+ s->status |= SDHCI_DATA_AVAILABLE;
+
+ s->bytecnt += 4;
+
+ if (s->bytecnt == 512) {
+ s->bytecnt = 0;
+ if (s->cmdtm & SDHCI_TRNS_BLK_CNT_EN) {
+ blkcnt = (s->blksizecnt >> 16) & 0xffff;
+ blkcnt--;
+ s->blksizecnt = (blkcnt << 16) | (s->blksizecnt & 0xffff);
+ if (blkcnt == 0) {
+ s->status &= ~SDHCI_DATA_AVAILABLE;
+
+ if (COMPLETION_DELAY > 0) {
+ now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
+ timer_mod(s->delay_timer,
+ now + COMPLETION_DELAY);
+ } else {
+ s->interrupt |= SDHCI_INT_DATA_END;
+ autocmd12(s);
+ }
+ }
+ }
+ if (!s->acmd && (cmd == 17)) {
+ /* Single read */
+ s->status &= ~SDHCI_DATA_AVAILABLE;
+
+ s->interrupt |= SDHCI_INT_DATA_END;
+ }
+ }
+ if (!sd_data_ready(s->card)) {
+ s->status &= ~SDHCI_DATA_AVAILABLE;
+
+ s->interrupt |= SDHCI_INT_DATA_END;
+ }
+
+ if (s->status & SDHCI_DATA_AVAILABLE) {
+ s->interrupt |= SDHCI_INT_DATA_AVAIL;
+ }
+
+ set_irq = 1;
+ res = s->data;
+ break;
+ case SDHCI_PRESENT_STATE: /* STATUS */
+ res = s->status;
+ break;
+ case SDHCI_HOST_CONTROL: /* CONTROL0 */
+ res = s->control0;
+ break;
+ case SDHCI_CLOCK_CONTROL: /* CONTROL1 */
+ res = s->control1;
+ break;
+ case SDHCI_INT_STATUS: /* INTERRUPT */
+ res = s->interrupt & s->irpt_mask;
+ break;
+ case SDHCI_INT_ENABLE: /* IRPT_MASK */
+ res = s->irpt_mask;
+ break;
+ case SDHCI_SIGNAL_ENABLE: /* IRPT_EN */
+ res = s->irpt_en;
+ break;
+ case SDHCI_CAPABILITIES:
+ res = s->caps;
+ break;
+ case SDHCI_CAPABILITIES_1:
+ res = s->caps;
+ break;
+ case SDHCI_ACMD12_ERR: /* CONTROL2 */
+ res = s->control2;
+ break;
+ case SDHCI_SET_ACMD12_ERROR: /* FORCE_IRPT */
+ res = s->force_irpt;
+ break;
+ case SDHCI_SLOT_INT_STATUS: /* SLOTISR_VERSION */
+ res = s->slotisr_ver;
+ break;
+ case SDHCI_MAX_CURRENT:
+ res = s->maxcurr;
+ break;
+ case SDHCI_MAX_CURRENT+4:
+ res = s->maxcurr2;
+ break;
+ default:
+ break;
+ }
+
+ if (set_irq) {
+ bcm2835_emmc_set_irq(s);
+ }
+
+ return res;
+}
+
+static void bcm2835_emmc_write(void *opaque, hwaddr offset,
+ uint64_t value, unsigned size)
+{
+ BCM2835EmmcState *s = (BCM2835EmmcState *)opaque;
+ uint8_t cmd;
+ SDRequest request;
+ uint8_t response[16];
+ int resplen;
+ uint32_t blkcnt;
+ int64_t now;
+
+ assert(size == 4);
+
+ switch (offset) {
+ case SDHCI_ARGUMENT2: /* ARG2 */
+ s->arg2 = value;
+ break;
+ case SDHCI_BLOCK_SIZE: /* BLKSIZECNT */
+ s->blksizecnt = value;
+ break;
+ case SDHCI_ARGUMENT: /* ARG1 */
+ s->arg1 = value;
+ break;
+ case SDHCI_TRANSFER_MODE: /* CMDTM */
+ s->cmdtm = value;
+ cmd = ((value >> (16 + 8)) & 0x3f);
+
+ if (!s->acmd && (cmd == 18 || cmd == 25)) {
+ autocmd23(s);
+ }
+
+ request.cmd = cmd;
+ request.arg = s->arg1;
+ request.crc = 0;
+
+ s->bytecnt = 0;
+
+ s->status &= ~SDHCI_DATA_AVAILABLE;
+ s->status &= ~SDHCI_SPACE_AVAILABLE;
+
+ resplen = sd_do_command(s->card, &request, response);
+
+ if (resplen > 0) {
+ if (resplen == 4) {
+ s->resp0 = (response[0] << 24)
+ | (response[1] << 16)
+ | (response[2] << 8)
+ | (response[3] << 0);
+ if (!s->acmd && ((cmd == 24) || (cmd == 25))) {
+ s->status |= SDHCI_SPACE_AVAILABLE;
+ s->interrupt |= SDHCI_INT_SPACE_AVAIL;
+ }
+ } else if (resplen == 16) {
+ s->resp3 = 0
+ | (response[1-1] << 16)
+ | (response[2-1] << 8)
+ | (response[3-1] << 0);
+ s->resp2 = 0
+ | (response[0+4-1] << 24)
+ | (response[1+4-1] << 16)
+ | (response[2+4-1] << 8)
+ | (response[3+4-1] << 0);
+ s->resp1 = 0
+ | (response[0+8-1] << 24)
+ | (response[1+8-1] << 16)
+ | (response[2+8-1] << 8)
+ | (response[3+8-1] << 0);
+ s->resp0 = 0
+ | (response[0+12-1] << 24)
+ | (response[1+12-1] << 16)
+ | (response[2+12-1] << 8)
+ | (response[3+12-1] << 0);
+ }
+
+ s->interrupt |= SDHCI_INT_RESPONSE;
+
+ if (!s->acmd && (cmd == 12)) {
+ /* Stop transmission */
+ s->status &= ~SDHCI_SPACE_AVAILABLE;
+ s->interrupt |= SDHCI_INT_DATA_END;
+ } else {
+ if (sd_data_ready(s->card)) {
+ s->status |= SDHCI_DATA_AVAILABLE;
+ s->interrupt |= SDHCI_INT_DATA_AVAIL;
+ } else {
+ s->interrupt |= SDHCI_INT_DATA_END;
+ }
+ }
+ bcm2835_emmc_set_irq(s);
+ } else {
+ /* Unrecognized commands */
+ if ((!s->acmd && (cmd == 52))
+ || (!s->acmd && (cmd == 5))
+ ) {
+ s->interrupt |= SDHCI_INT_TIMEOUT;
+ s->interrupt |= SDHCI_INT_ERROR;
+ }
+ if (!s->acmd && (cmd == 0)) {
+ s->interrupt |= SDHCI_INT_RESPONSE;
+ }
+ if (!s->acmd && (cmd == 7)) {
+ s->interrupt |= SDHCI_INT_RESPONSE;
+ }
+ bcm2835_emmc_set_irq(s);
+ }
+ if (cmd == 55) {
+ s->acmd = 1;
+ } else {
+ s->acmd = 0;
+ }
+ break;
+ case SDHCI_BUFFER: /* DATA */
+ cmd = ((s->cmdtm >> (16 + 8)) & 0x3f);
+
+ s->data = value;
+
+ sd_write_data(s->card, (value >> 0) & 0xff);
+ sd_write_data(s->card, (value >> 8) & 0xff);
+ sd_write_data(s->card, (value >> 16) & 0xff);
+ sd_write_data(s->card, (value >> 24) & 0xff);
+
+ s->status |= SDHCI_SPACE_AVAILABLE;
+
+ s->bytecnt += 4;
+
+ if (s->bytecnt == 512) {
+ s->bytecnt = 0;
+ if (s->cmdtm & SDHCI_TRNS_BLK_CNT_EN) {
+ blkcnt = (s->blksizecnt >> 16) & 0xffff;
+ blkcnt--;
+ s->blksizecnt = (blkcnt << 16) | (s->blksizecnt & 0xffff);
+ if (blkcnt == 0) {
+ if (COMPLETION_DELAY > 0) {
+ now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
+ timer_mod(s->delay_timer,
+ now + COMPLETION_DELAY);
+ } else {
+ s->interrupt |= SDHCI_INT_DATA_END;
+ autocmd12(s);
+ }
+ }
+ }
+ if (!s->acmd && (cmd == 24)) {
+ /* Single write */
+ s->status &= ~SDHCI_SPACE_AVAILABLE;
+
+ s->interrupt |= SDHCI_INT_DATA_END;
+ }
+ }
+
+ if (s->status & SDHCI_SPACE_AVAILABLE) {
+ s->interrupt |= SDHCI_INT_SPACE_AVAIL;
+ }
+
+ bcm2835_emmc_set_irq(s);
+ break;
+ case SDHCI_HOST_CONTROL: /* CONTROL0 */
+ s->control0 &= ~0x007f0026;
+ value &= 0x007f0026;
+ s->control0 |= value;
+ break;
+ case SDHCI_CLOCK_CONTROL: /* CONTROL1 */
+ s->control0 &= ~0x070fffe7;
+ value &= 0x070fffe7;
+ if (value & ((SDHCI_RESET_ALL
+ | SDHCI_RESET_CMD
+ | SDHCI_RESET_DATA) << 24)) {
+ /* Reset */
+ /* TODO: implement proper reset logic. In the meantime,
+ * one observed side-effect of reset is clearing the card
+ * inserted bit. */
+ if (value & (SDHCI_RESET_ALL << 24)) {
+ s->interrupt &= ~SDHCI_INT_CARD_INSERT;
+ }
+ value &= ~((SDHCI_RESET_ALL
+ | SDHCI_RESET_CMD
+ | SDHCI_RESET_DATA) << 24);
+ }
+ s->control1 |= value;
+ break;
+ case SDHCI_INT_STATUS: /* INTERRUPT */
+ s->interrupt &= ~value;
+ /* clearing all error sources also clears the overall error status */
+ if ((s->interrupt & SDHCI_INT_ERROR_MASK) == SDHCI_INT_ERROR) {
+ s->interrupt &= ~SDHCI_INT_ERROR_MASK;
+ }
+ bcm2835_emmc_set_irq(s);
+ break;
+
+ case SDHCI_INT_ENABLE: /* IRPT_MASK */
+ s->irpt_mask = value;
+ bcm2835_emmc_set_irq(s);
+ break;
+ case SDHCI_SIGNAL_ENABLE: /* IRPT_EN */
+ s->irpt_en = value;
+ bcm2835_emmc_set_irq(s);
+ break;
+ case SDHCI_ACMD12_ERR: /* CONTROL2 */
+ s->control2 &= ~0x00e7009f;
+ value &= 0x00e7009f;
+ s->control2 |= value;
+ break;
+ case SDHCI_SET_ACMD12_ERROR: /* FORCE_IRPT */
+ s->force_irpt = value;
+ break;
+
+ default:
+ break;
+ }
+}
+
+static const MemoryRegionOps bcm2835_emmc_ops = {
+ .read = bcm2835_emmc_read,
+ .write = bcm2835_emmc_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const VMStateDescription vmstate_bcm2835_emmc = {
+ .name = TYPE_BCM2835_EMMC,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void bcm2835_emmc_init(Object *obj)
+{
+ BCM2835EmmcState *s = BCM2835_EMMC(obj);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &bcm2835_emmc_ops, s,
+ TYPE_BCM2835_EMMC, 0x100000);
+ sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
+ sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq);
+}
+
+static void bcm2835_emmc_realize(DeviceState *dev, Error **errp)
+{
+ BCM2835EmmcState *s = BCM2835_EMMC(dev);
+ DriveInfo *dinfo;
+
+ dinfo = drive_get(IF_SD, 0, 0);
+ if (!dinfo) {
+ error_setg(errp, "bcm2835_emmc: missing SD card");
+ return;
+ }
+ s->card = sd_init(blk_by_legacy_dinfo(dinfo), 0);
+
+ s->arg2 = 0;
+ s->blksizecnt = 0;
+ s->arg1 = 0;
+ s->cmdtm = 0;
+ s->resp0 = 0;
+ s->resp1 = 0;
+ s->resp2 = 0;
+ s->resp3 = 0;
+ s->data = 0;
+ s->status = (0x1ff << 16);
+ s->control0 = 0;
+ s->control1 = SDHCI_CLOCK_INT_STABLE;
+
+ /* Although the Broadcom doc says it is unimplemented/reserved, on
+ * real hardware the card inserted interrupt is actually set at
+ * boot, and is later cleared by a reset command (as observed on a
+ * Raspberry Pi 2). Moreover, EDK2/UEFI depend on seeing this bit
+ * set, so we set it here and later clear it in the reset. */
+ s->interrupt = SDHCI_INT_CARD_INSERT;
+
+ s->irpt_mask = 0;
+ s->irpt_en = 0;
+ s->control2 = 0;
+ s->force_irpt = 0;
+ s->spi_int_spt = 0;
+ s->slotisr_ver = (0x9900 | SDHCI_SPEC_300) << 16;
+ s->caps = 0;
+ s->caps2 = 0;
+ s->maxcurr = 1;
+ s->maxcurr2 = 0;
+
+ s->acmd = 0;
+ s->write_op = 0;
+
+ s->delay_timer = timer_new_us(QEMU_CLOCK_VIRTUAL, delayed_completion, s);
+}
+
+static void bcm2835_emmc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = bcm2835_emmc_realize;
+ dc->vmsd = &vmstate_bcm2835_emmc;
+}
+
+static TypeInfo bcm2835_emmc_info = {
+ .name = TYPE_BCM2835_EMMC,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(BCM2835EmmcState),
+ .class_init = bcm2835_emmc_class_init,
+ .instance_init = bcm2835_emmc_init,
+};
+
+static void bcm2835_emmc_register_types(void)
+{
+ type_register_static(&bcm2835_emmc_info);
+}
+
+type_init(bcm2835_emmc_register_types)
diff --git a/include/hw/sd/bcm2835_emmc.h b/include/hw/sd/bcm2835_emmc.h
new file mode 100644
index 0000000..b756c67
--- /dev/null
+++ b/include/hw/sd/bcm2835_emmc.h
@@ -0,0 +1,56 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2835_EMMC_H
+#define BCM2835_EMMC_H
+
+#include "hw/sysbus.h"
+#include "hw/sd/sd.h"
+#include "qemu/timer.h"
+
+#define TYPE_BCM2835_EMMC "bcm2835_emmc"
+#define BCM2835_EMMC(obj) \
+ OBJECT_CHECK(BCM2835EmmcState, (obj), TYPE_BCM2835_EMMC)
+
+typedef struct {
+ SysBusDevice busdev;
+ MemoryRegion iomem;
+
+ SDState *card;
+
+ uint32_t arg2;
+ uint32_t blksizecnt;
+ uint32_t arg1;
+ uint32_t cmdtm;
+ uint32_t resp0;
+ uint32_t resp1;
+ uint32_t resp2;
+ uint32_t resp3;
+ uint32_t data;
+ uint32_t status;
+ uint32_t control0;
+ uint32_t control1;
+ uint32_t interrupt;
+ uint32_t irpt_mask;
+ uint32_t irpt_en;
+ uint32_t control2;
+ uint32_t force_irpt;
+ uint32_t spi_int_spt;
+ uint32_t slotisr_ver;
+ uint32_t caps;
+ uint32_t caps2;
+ uint32_t maxcurr;
+ uint32_t maxcurr2;
+
+ int acmd;
+ int write_op;
+
+ uint32_t bytecnt;
+
+ QEMUTimer *delay_timer;
+ qemu_irq irq;
+} BCM2835EmmcState;
+
+#endif
--
2.5.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-04 6:01 ` [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller Andrew Baumann
@ 2015-12-06 5:25 ` Peter Crosthwaite
2015-12-09 6:19 ` Andrew Baumann
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-06 5:25 UTC (permalink / raw)
To: Andrew Baumann
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> ---
> hw/sd/Makefile.objs | 1 +
> hw/sd/bcm2835_emmc.c | 800 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/sd/bcm2835_emmc.h | 56 +++
> 3 files changed, 857 insertions(+)
> create mode 100644 hw/sd/bcm2835_emmc.c
> create mode 100644 include/hw/sd/bcm2835_emmc.h
>
> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
> index f1aed83..b9cb9b8 100644
> --- a/hw/sd/Makefile.objs
> +++ b/hw/sd/Makefile.objs
> @@ -6,3 +6,4 @@ common-obj-$(CONFIG_SDHCI) += sdhci.o
> obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
> obj-$(CONFIG_OMAP) += omap_mmc.o
> obj-$(CONFIG_PXA2XX) += pxa2xx_mmci.o
> +obj-$(CONFIG_RASPI) += bcm2835_emmc.o
> diff --git a/hw/sd/bcm2835_emmc.c b/hw/sd/bcm2835_emmc.c
> new file mode 100644
> index 0000000..46628b5
> --- /dev/null
> +++ b/hw/sd/bcm2835_emmc.c
> @@ -0,0 +1,800 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +#include "sysemu/blockdev.h"
> +#include "hw/sd/bcm2835_emmc.h"
> +
> +/*
> + * Controller registers
> + */
> +
> +#define SDHCI_DMA_ADDRESS 0x00
> +#define SDHCI_ARGUMENT2 SDHCI_DMA_ADDRESS
> +
Is this IP just SDHCI? We already model SDHCI in QEMU, see
hw/sd/sdhci.c. If there are RPi specific features to the SDHCI
implementation they should be added as optional extensions (probabably
via subclassing) to the existing SDHCI model.
Regards,
Peter
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-06 5:25 ` Peter Crosthwaite
@ 2015-12-09 6:19 ` Andrew Baumann
2015-12-09 7:40 ` Peter Crosthwaite
0 siblings, 1 reply; 33+ messages in thread
From: Andrew Baumann @ 2015-12-09 6:19 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Saturday, 5 December 2015 21:26
> Is this IP just SDHCI? We already model SDHCI in QEMU, see
> hw/sd/sdhci.c. If there are RPi specific features to the SDHCI
> implementation they should be added as optional extensions (probabably
> via subclassing) to the existing SDHCI model.
So yes, it turns out this is fairly similar to SDHCI (-> lots of wasted work by Gregory and me, sigh), and indeed Linux boots with the existing sdhci emulation. However, there are some quirks, and UEFI/Windows depend on them. Namely:
* The host control registers (offset 0x28 and above) seem to differ significantly. Maybe this is due to the SDHC version -- according to the BCM2835 peripherals spec, the controller implements "Version 3.0 Draft 1.0" of the SDHC spec, but of course I can't find that spec online anywhere. Luckily nothing seems to depend on this, besides a few spurious warnings about invalid writes.
* Power is assumed to be always on -- the sdhci model requires the guest to turn it on by a write at offset 0x29 before issuing any commands, but on pi this bit is marked reserved, and commands are issued immediately after reset.
* The card inserted interrupt is rather broken on pi: it is set at the start of day, but a reset command clears it and it stays clear thereafter (and never generates interrupts).
There's an inconsistency with response handling, too, although I'm not sure if it's a quirk of the Pi or a general bug in sdhci. Pi UEFI sends a CMD23 without setting any of the response bits, but this command does in fact generate a 4-byte R1 response. The question is whether this should be treated as an error, or whether it simply means that the host wants to ignore the response. In sdhci, the following code path (around line 246) raises a "command index" error in the case that a non-zero response is returned but no response bits were set in the command register:
} else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
s->errintsts |= SDHC_EIS_CMDIDX;
s->norintsts |= SDHC_NIS_ERR;
}
I do not observe this behaviour on the real Pi2 (and it breaks UEFI). The hardware semantics appear to be "if the command generates a response, but you didn't want to see it, we'll successfully complete the command and ignore the response", whereas the sdhci implementation raises an error for this as well as signalling completion. I have read the "SD Specifications Part A2 SD Host Controller Simplified Specification Version 2.00", but did not find anything describing this case, so it could be that this is open to interpretation. (It could also be specified in SDHC v3.) The specific error also seems odd -- my understanding is that a "command index" error means that the index in the response didn't match the index of the issued command, but that's hardly what is happening here.
Assuming this latter bug can be fixed generically, how do you propose handling the Pi quirks? I could add a bool property for "bcm2835-quirks" or similar and just special-case the relevant code (my preferred approach). I'm also open to subclassing, but no idea how that would work in practice, so would need some pointers.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 6:19 ` Andrew Baumann
@ 2015-12-09 7:40 ` Peter Crosthwaite
2015-12-09 18:17 ` Andrew Baumann
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-09 7:40 UTC (permalink / raw)
To: Andrew Baumann
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
On Tue, Dec 8, 2015 at 10:19 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
>> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
>> Sent: Saturday, 5 December 2015 21:26
>> Is this IP just SDHCI? We already model SDHCI in QEMU, see
>> hw/sd/sdhci.c. If there are RPi specific features to the SDHCI
>> implementation they should be added as optional extensions (probabably
>> via subclassing) to the existing SDHCI model.
>
> So yes, it turns out this is fairly similar to SDHCI (-> lots of wasted work by Gregory and me, sigh), and indeed Linux boots with the existing sdhci emulation. However, there are some quirks, and UEFI/Windows depend on them. Namely:
> * The host control registers (offset 0x28 and above) seem to differ significantly. Maybe this is due to the SDHC version -- according to the BCM2835 peripherals spec, the controller implements "Version 3.0 Draft 1.0" of the SDHC spec, but of course I can't find that spec online anywhere. Luckily nothing seems to depend on this, besides a few spurious warnings about invalid writes.
Looks reasonably consistent from a quick scan? 0x28 in shdci.c is only
doing the ADMA stuff while there are other fields on the BCM model.
> * Power is assumed to be always on -- the sdhci model requires the guest to turn it on by a write at offset 0x29 before issuing any commands, but on pi this bit is marked reserved, and commands are issued immediately after reset.
Does this help?:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06271.html
> * The card inserted interrupt is rather broken on pi: it is set at the start of day, but a reset command clears it and it stays clear thereafter (and never generates interrupts).
>
Is that more likely to be an IP connectivity problem (wierd input to
the card-detect pin in the SoC)?
> There's an inconsistency with response handling, too, although I'm not sure if it's a quirk of the Pi or a general bug in sdhci. Pi UEFI sends a CMD23 without setting any of the response bits, but this command does in fact generate a 4-byte R1 response. The question is whether this should be treated as an error, or whether it simply means that the host wants to ignore the response. In sdhci, the following code path (around line 246) raises a "command index" error in the case that a non-zero response is returned but no response bits were set in the command register:
>
> } else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
> s->errintsts |= SDHC_EIS_CMDIDX;
> s->norintsts |= SDHC_NIS_ERR;
> }
>
> I do not observe this behaviour on the real Pi2 (and it breaks UEFI). The hardware semantics appear to be "if the command generates a response, but you didn't want to see it, we'll successfully complete the command and ignore the response", whereas the sdhci implementation raises an error for this as well as signalling completion. I have read the "SD Specifications Part A2 SD Host Controller Simplified Specification Version 2.00", but did not find anything describing this case, so it could be that this is open to interpretation. (It could also be specified in SDHC v3.) The specific error also seems odd -- my understanding is that a "command index" error means that the index in the response didn't match the index of the issued command, but that's hardly what is happening here.
>
Starting to sound like a bug.
> Assuming this latter bug can be fixed generically, how do you propose handling the Pi quirks? I could add a bool property for "bcm2835-quirks" or similar and just special-case the relevant code (my preferred approach). I'm also open to subclassing, but no idea how that would work in practice, so would need some pointers.
>
I think we need a more definitive list of the register level features
that are different or added, I am not seeing what is BCM specific just
yet.
Regards,
Peter
> Thanks,
> Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 7:40 ` Peter Crosthwaite
@ 2015-12-09 18:17 ` Andrew Baumann
2015-12-09 18:54 ` Peter Crosthwaite
0 siblings, 1 reply; 33+ messages in thread
From: Andrew Baumann @ 2015-12-09 18:17 UTC (permalink / raw)
To: Peter Crosthwaite, Igor Mitsyanko
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Tuesday, 8 December 2015 23:40
> On Tue, Dec 8, 2015 at 10:19 PM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> >> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> >> Sent: Saturday, 5 December 2015 21:26
> >> Is this IP just SDHCI? We already model SDHCI in QEMU, see
> >> hw/sd/sdhci.c. If there are RPi specific features to the SDHCI
> >> implementation they should be added as optional extensions (probabably
> >> via subclassing) to the existing SDHCI model.
> >
> > So yes, it turns out this is fairly similar to SDHCI (-> lots of wasted work by
> Gregory and me, sigh), and indeed Linux boots with the existing sdhci
> emulation. However, there are some quirks, and UEFI/Windows depend on
> them. Namely:
> > * The host control registers (offset 0x28 and above) seem to differ
> significantly. Maybe this is due to the SDHC version -- according to the
> BCM2835 peripherals spec, the controller implements "Version 3.0 Draft 1.0"
> of the SDHC spec, but of course I can't find that spec online anywhere. Luckily
> nothing seems to depend on this, besides a few spurious warnings about
> invalid writes.
>
> Looks reasonably consistent from a quick scan? 0x28 in shdci.c is only
> doing the ADMA stuff while there are other fields on the BCM model.
You're right, upon closer examination, it's not as bad as I thought (just reserved / unimplemented bits). The one significant difference that seems likely to cause a problem is the first register (offset 0x0-0x3) which is ARG2 on Pi (used for auto CMD 23 support) but the SDMA system address on SDHCI (Pi doesn't support DMA in the controller). This will break if a guest wants to use auto cmd 23 -- I've yet to see one that does, but Gregory's model implements it, so perhaps he did.
> > * Power is assumed to be always on -- the sdhci model requires the guest
> to turn it on by a write at offset 0x29 before issuing any commands, but on pi
> this bit is marked reserved, and commands are issued immediately after
> reset.
>
> Does this help?:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06271.html
Yes, that's the same change I made. Is it going to be applied?
> > * The card inserted interrupt is rather broken on pi: it is set at the start of
> day, but a reset command clears it and it stays clear thereafter (and never
> generates interrupts).
> >
>
> Is that more likely to be an IP connectivity problem (wierd input to
> the card-detect pin in the SoC)?
That might be it, but in any case I still need to model it somehow.
> > There's an inconsistency with response handling, too, although I'm not sure
> if it's a quirk of the Pi or a general bug in sdhci. Pi UEFI sends a CMD23
> without setting any of the response bits, but this command does in fact
> generate a 4-byte R1 response. The question is whether this should be
> treated as an error, or whether it simply means that the host wants to ignore
> the response. In sdhci, the following code path (around line 246) raises a
> "command index" error in the case that a non-zero response is returned but
> no response bits were set in the command register:
> >
> > } else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
> > s->errintsts |= SDHC_EIS_CMDIDX;
> > s->norintsts |= SDHC_NIS_ERR;
> > }
> >
> > I do not observe this behaviour on the real Pi2 (and it breaks UEFI). The
> hardware semantics appear to be "if the command generates a response,
> but you didn't want to see it, we'll successfully complete the command and
> ignore the response", whereas the sdhci implementation raises an error for
> this as well as signalling completion. I have read the "SD Specifications Part A2
> SD Host Controller Simplified Specification Version 2.00", but did not find
> anything describing this case, so it could be that this is open to interpretation.
> (It could also be specified in SDHC v3.) The specific error also seems odd -- my
> understanding is that a "command index" error means that the index in the
> response didn't match the index of the issued command, but that's hardly
> what is happening here.
> >
>
> Starting to sound like a bug.
I think so, yes. It was written this way in the original commit -- I'm adding the original author (Igor Mitsyanko) to the thread, in case he has any insight.
> > Assuming this latter bug can be fixed generically, how do you propose
> handling the Pi quirks? I could add a bool property for "bcm2835-quirks" or
> similar and just special-case the relevant code (my preferred approach). I'm
> also open to subclassing, but no idea how that would work in practice, so
> would need some pointers.
> >
>
> I think we need a more definitive list of the register level features
> that are different or added, I am not seeing what is BCM specific just
> yet.
The complete diff needed to boot Windows appears below. The first hunk avoids re-triggering the insert interrupt on card reset, the second fixes the bug described above, and the last hunk is equivalent to your patch linked above. I propose that we make the first conditional under a suitably-named bool property to enable the quirk, and apply the second two fixes directly.
Thoughts?
Andrew
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index d70d1a6..877dd51 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -193,7 +193,7 @@ static void sdhci_reset(SDHCIState *s)
* initialization */
memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
- sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+ //sd_set_cb(s->card, s->ro_cb, s->eject_cb);
s->data_count = 0;
s->stopped_state = sdhc_not_stopped;
}
@@ -243,9 +243,11 @@ static void sdhci_send_command(SDHCIState *s)
(s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) {
s->norintsts |= SDHC_NIS_TRSCMP;
}
+#if 0
} else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
s->errintsts |= SDHC_EIS_CMDIDX;
s->norintsts |= SDHC_NIS_ERR;
+#endif
}
if (s->norintstsen & SDHC_NISEN_CMDCMP) {
@@ -831,7 +833,7 @@ static void sdhci_data_transfer(void *opaque)
static bool sdhci_can_issue_command(SDHCIState *s)
{
- if (!SDHC_CLOCK_IS_ON(s->clkcon) || !(s->pwrcon & SDHC_POWER_ON) ||
+ if (!SDHC_CLOCK_IS_ON(s->clkcon) || /* !(s->pwrcon & SDHC_POWER_ON) || */
(((s->prnsts & SDHC_DATA_INHIBIT) || s->stopped_state) &&
((s->cmdreg & SDHC_CMD_DATA_PRESENT) ||
((s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY &&
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 18:17 ` Andrew Baumann
@ 2015-12-09 18:54 ` Peter Crosthwaite
2015-12-09 19:01 ` Andrew Baumann
` (2 more replies)
0 siblings, 3 replies; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-09 18:54 UTC (permalink / raw)
To: Andrew Baumann, Peter Maydell, Kevin OConnor
Cc: Grégory ESTRADE, Igor Mitsyanko, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
On Wed, Dec 9, 2015 at 10:17 AM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
>> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
>> Sent: Tuesday, 8 December 2015 23:40
>> On Tue, Dec 8, 2015 at 10:19 PM, Andrew Baumann
>> <Andrew.Baumann@microsoft.com> wrote:
>> >> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
>> >> Sent: Saturday, 5 December 2015 21:26
>> >> Is this IP just SDHCI? We already model SDHCI in QEMU, see
>> >> hw/sd/sdhci.c. If there are RPi specific features to the SDHCI
>> >> implementation they should be added as optional extensions (probabably
>> >> via subclassing) to the existing SDHCI model.
>> >
>> > So yes, it turns out this is fairly similar to SDHCI (-> lots of wasted work by
>> Gregory and me, sigh), and indeed Linux boots with the existing sdhci
>> emulation. However, there are some quirks, and UEFI/Windows depend on
>> them. Namely:
>> > * The host control registers (offset 0x28 and above) seem to differ
>> significantly. Maybe this is due to the SDHC version -- according to the
>> BCM2835 peripherals spec, the controller implements "Version 3.0 Draft 1.0"
>> of the SDHC spec, but of course I can't find that spec online anywhere. Luckily
>> nothing seems to depend on this, besides a few spurious warnings about
>> invalid writes.
>>
>> Looks reasonably consistent from a quick scan? 0x28 in shdci.c is only
>> doing the ADMA stuff while there are other fields on the BCM model.
>
> You're right, upon closer examination, it's not as bad as I thought (just reserved / unimplemented bits). The one significant difference that seems likely to cause a problem is the first register (offset 0x0-0x3) which is ARG2 on Pi (used for auto CMD 23 support) but the SDMA system address on SDHCI (Pi doesn't support DMA in the controller). This will break if a guest wants to use auto cmd 23 -- I've yet to see one that does, but Gregory's model implements it, so perhaps he did.
>
>> > * Power is assumed to be always on -- the sdhci model requires the guest
>> to turn it on by a write at offset 0x29 before issuing any commands, but on pi
>> this bit is marked reserved, and commands are issued immediately after
>> reset.
>>
>> Does this help?:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06271.html
>
> Yes, that's the same change I made. Is it going to be applied?
>
It missed the boat for 2.5, but you could help by putting a tested-by
or reviewed-by to the patch.
>> > * The card inserted interrupt is rather broken on pi: it is set at the start of
>> day, but a reset command clears it and it stays clear thereafter (and never
>> generates interrupts).
>> >
>>
>> Is that more likely to be an IP connectivity problem (wierd input to
>> the card-detect pin in the SoC)?
>
> That might be it, but in any case I still need to model it somehow.
>
Ideally, this would be managed on the board level but it is kinda hard
the way to code is organised. Currently SDHCI is managing the SD card
instantiation due to a lack of QOMification. So for the moment, this
would be valid as a boolean property which disables the card detect
logic completely. If we get the SD QOMification though, the power,
card detect and sd interface proper can then be more finely managed on
the board level for all these varying connectivity configurations.
>> > There's an inconsistency with response handling, too, although I'm not sure
>> if it's a quirk of the Pi or a general bug in sdhci. Pi UEFI sends a CMD23
>> without setting any of the response bits, but this command does in fact
>> generate a 4-byte R1 response. The question is whether this should be
>> treated as an error, or whether it simply means that the host wants to ignore
>> the response. In sdhci, the following code path (around line 246) raises a
>> "command index" error in the case that a non-zero response is returned but
>> no response bits were set in the command register:
>> >
>> > } else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
>> > s->errintsts |= SDHC_EIS_CMDIDX;
>> > s->norintsts |= SDHC_NIS_ERR;
>> > }
>> >
>> > I do not observe this behaviour on the real Pi2 (and it breaks UEFI). The
>> hardware semantics appear to be "if the command generates a response,
>> but you didn't want to see it, we'll successfully complete the command and
>> ignore the response", whereas the sdhci implementation raises an error for
>> this as well as signalling completion. I have read the "SD Specifications Part A2
>> SD Host Controller Simplified Specification Version 2.00", but did not find
>> anything describing this case, so it could be that this is open to interpretation.
>> (It could also be specified in SDHC v3.) The specific error also seems odd -- my
>> understanding is that a "command index" error means that the index in the
>> response didn't match the index of the issued command, but that's hardly
>> what is happening here.
>> >
>>
>> Starting to sound like a bug.
>
> I think so, yes. It was written this way in the original commit -- I'm adding the original author (Igor Mitsyanko) to the thread, in case he has any insight.
>
>> > Assuming this latter bug can be fixed generically, how do you propose
>> handling the Pi quirks? I could add a bool property for "bcm2835-quirks" or
>> similar and just special-case the relevant code (my preferred approach). I'm
>> also open to subclassing, but no idea how that would work in practice, so
>> would need some pointers.
>> >
>>
>> I think we need a more definitive list of the register level features
>> that are different or added, I am not seeing what is BCM specific just
>> yet.
>
> The complete diff needed to boot Windows appears below. The first hunk avoids re-triggering the insert interrupt on card reset,
Is this a runtime reset or an initial reset that is causing you grief?
You patch is also patching the controller reset so is it more a case
of the interrupt trigger on controller reset that is causing you
issues?
Regards,
Peter
>the second fixes the bug described above, and the last hunk is equivalent to your patch linked above. I propose that we make the first conditional under a suitably-named bool property to enable the quirk, and apply the second two fixes directly.
>
> Thoughts?
>
> Andrew
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index d70d1a6..877dd51 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -193,7 +193,7 @@ static void sdhci_reset(SDHCIState *s)
> * initialization */
> memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
>
> - sd_set_cb(s->card, s->ro_cb, s->eject_cb);
> + //sd_set_cb(s->card, s->ro_cb, s->eject_cb);
> s->data_count = 0;
> s->stopped_state = sdhc_not_stopped;
> }
> @@ -243,9 +243,11 @@ static void sdhci_send_command(SDHCIState *s)
> (s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) {
> s->norintsts |= SDHC_NIS_TRSCMP;
> }
> +#if 0
> } else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
> s->errintsts |= SDHC_EIS_CMDIDX;
> s->norintsts |= SDHC_NIS_ERR;
> +#endif
> }
>
> if (s->norintstsen & SDHC_NISEN_CMDCMP) {
> @@ -831,7 +833,7 @@ static void sdhci_data_transfer(void *opaque)
>
> static bool sdhci_can_issue_command(SDHCIState *s)
> {
> - if (!SDHC_CLOCK_IS_ON(s->clkcon) || !(s->pwrcon & SDHC_POWER_ON) ||
> + if (!SDHC_CLOCK_IS_ON(s->clkcon) || /* !(s->pwrcon & SDHC_POWER_ON) || */
> (((s->prnsts & SDHC_DATA_INHIBIT) || s->stopped_state) &&
> ((s->cmdreg & SDHC_CMD_DATA_PRESENT) ||
> ((s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY &&
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 18:54 ` Peter Crosthwaite
@ 2015-12-09 19:01 ` Andrew Baumann
2015-12-09 21:01 ` Peter Maydell
2015-12-09 23:09 ` Kevin O'Connor
2 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-09 19:01 UTC (permalink / raw)
To: Peter Crosthwaite, Peter Maydell, Kevin OConnor
Cc: Grégory ESTRADE, Igor Mitsyanko, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Wednesday, 9 December 2015 10:55
> > Yes, that's the same change I made. Is it going to be applied?
> >
>
> It missed the boat for 2.5, but you could help by putting a tested-by
> or reviewed-by to the patch.
Ok.
> > The complete diff needed to boot Windows appears below. The first hunk
> avoids re-triggering the insert interrupt on card reset,
>
> Is this a runtime reset or an initial reset that is causing you grief?
> You patch is also patching the controller reset so is it more a case
> of the interrupt trigger on controller reset that is causing you
> issues?
It's the software-issued controller reset... at board power on, the insert interrupt bit is set (and UEFI depends on seeing it set), but when the Windows driver starts, it issues a reset command which clears the interrupt status. It also has a bug which livelocks the system if an insert interrupt is seen after that reset (which never happens on hardware, obviously).
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 18:54 ` Peter Crosthwaite
2015-12-09 19:01 ` Andrew Baumann
@ 2015-12-09 21:01 ` Peter Maydell
2015-12-09 21:37 ` Andrew Baumann
2015-12-09 21:38 ` Peter Crosthwaite
2015-12-09 23:09 ` Kevin O'Connor
2 siblings, 2 replies; 33+ messages in thread
From: Peter Maydell @ 2015-12-09 21:01 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Crosthwaite, Igor Mitsyanko, Stefan Weil,
Grégory ESTRADE, qemu-devel@nongnu.org Developers,
Andrew Baumann, Kevin OConnor, qemu-arm, Paolo Bonzini
On 9 December 2015 at 18:54, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Wed, Dec 9, 2015 at 10:17 AM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
>>> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
>>> Is that more likely to be an IP connectivity problem (wierd input to
>>> the card-detect pin in the SoC)?
>>
>> That might be it, but in any case I still need to model it somehow.
>>
>
> Ideally, this would be managed on the board level but it is kinda hard
> the way to code is organised. Currently SDHCI is managing the SD card
> instantiation due to a lack of QOMification. So for the moment, this
> would be valid as a boolean property which disables the card detect
> logic completely. If we get the SD QOMification though, the power,
> card detect and sd interface proper can then be more finely managed on
> the board level for all these varying connectivity configurations.
Card detect you should be able to handle on the board level without
waiting for QOMification -- it's just modelled as a QEMU irq line
(the vexpress board connects it up to a random register, or you could
not connect it to anything, etc).
I'm working on the QOMification of sd.c this week, though I'm very
rusty on how to properly model buses.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 21:01 ` Peter Maydell
@ 2015-12-09 21:37 ` Andrew Baumann
2015-12-09 21:38 ` Peter Crosthwaite
1 sibling, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-09 21:37 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: Grégory ESTRADE, Igor Mitsyanko, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers,
Kevin OConnor, qemu-arm, Paolo Bonzini
> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> Sent: Wednesday, 9 December 2015 13:01
> On 9 December 2015 at 18:54, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
> > On Wed, Dec 9, 2015 at 10:17 AM, Andrew Baumann
> > <Andrew.Baumann@microsoft.com> wrote:
> >>> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> >>> Is that more likely to be an IP connectivity problem (wierd input to
> >>> the card-detect pin in the SoC)?
> >>
> >> That might be it, but in any case I still need to model it somehow.
> >>
> >
> > Ideally, this would be managed on the board level but it is kinda hard
> > the way to code is organised. Currently SDHCI is managing the SD card
> > instantiation due to a lack of QOMification. So for the moment, this
> > would be valid as a boolean property which disables the card detect
> > logic completely. If we get the SD QOMification though, the power,
> > card detect and sd interface proper can then be more finely managed on
> > the board level for all these varying connectivity configurations.
>
> Card detect you should be able to handle on the board level without
> waiting for QOMification -- it's just modelled as a QEMU irq line
> (the vexpress board connects it up to a random register, or you could
> not connect it to anything, etc).
Forgive me if I'm slow, but I don’t see how that could work for sdhci.
Vexpress uses pl181, which has GPIO _outputs_ for card inserted etc. sdhci multiplexes the card inserted interrupt (which originates in sd.c) along with other SD events like command completion into a single interrupt line.
If there was a way to interpose on the irq between sd and sdhci, I think I could do what I want, but right now they are allocated and wired up in sdhci's init routine.
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 21:01 ` Peter Maydell
2015-12-09 21:37 ` Andrew Baumann
@ 2015-12-09 21:38 ` Peter Crosthwaite
1 sibling, 0 replies; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-09 21:38 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, Igor Mitsyanko, Stefan Weil,
Grégory ESTRADE, qemu-devel@nongnu.org Developers,
Andrew Baumann, Kevin OConnor, qemu-arm, Paolo Bonzini
On Wed, Dec 9, 2015 at 1:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 December 2015 at 18:54, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Wed, Dec 9, 2015 at 10:17 AM, Andrew Baumann
>> <Andrew.Baumann@microsoft.com> wrote:
>>>> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
>>>> Is that more likely to be an IP connectivity problem (wierd input to
>>>> the card-detect pin in the SoC)?
>>>
>>> That might be it, but in any case I still need to model it somehow.
>>>
>>
>> Ideally, this would be managed on the board level but it is kinda hard
>> the way to code is organised. Currently SDHCI is managing the SD card
>> instantiation due to a lack of QOMification. So for the moment, this
>> would be valid as a boolean property which disables the card detect
>> logic completely. If we get the SD QOMification though, the power,
>> card detect and sd interface proper can then be more finely managed on
>> the board level for all these varying connectivity configurations.
>
> Card detect you should be able to handle on the board level without
> waiting for QOMification -- it's just modelled as a QEMU irq line
> (the vexpress board connects it up to a random register, or you could
> not connect it to anything, etc).
>
It's probably worth waiting for the QOMification though, so the GPIO
out has a correct formal owner.
> I'm working on the QOMification of sd.c this week, though I'm very
> rusty on how to properly model buses.
>
The last bus I did was SSI (SPI) which I am reasonably confident that
it is up to date unless we are trying to achieve the "busless" goal or
pure QOM links. When I was looking at NAND QOMification (unfortunately
I didn't make it to conclusion) I noticed that many of the features
were common to SSI, mainly WRT GPIO based individual chip select
lines. Since SD is has SPI compatibility mode an SD card should
actually be a subclass of SSI_DEVICE. I'm thinking the same might
apply on the bus side. So you could subclass SSI_BUS as SD_BUS which
picks up SSI's CS stuff for free while quite naturally having the
consequence of SD cards being SPI devices.
Regards,
Peter
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller
2015-12-09 18:54 ` Peter Crosthwaite
2015-12-09 19:01 ` Andrew Baumann
2015-12-09 21:01 ` Peter Maydell
@ 2015-12-09 23:09 ` Kevin O'Connor
2 siblings, 0 replies; 33+ messages in thread
From: Kevin O'Connor @ 2015-12-09 23:09 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Peter Crosthwaite, Igor Mitsyanko, Stefan Weil,
Grégory ESTRADE, qemu-devel@nongnu.org Developers,
Andrew Baumann, qemu-arm, Paolo Bonzini
On Wed, Dec 09, 2015 at 10:54:38AM -0800, Peter Crosthwaite wrote:
> On Wed, Dec 9, 2015 at 10:17 AM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> >> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> >> Sent: Tuesday, 8 December 2015 23:40
> >> On Tue, Dec 8, 2015 at 10:19 PM, Andrew Baumann
> >> <Andrew.Baumann@microsoft.com> wrote:
> >> >> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> >> > I do not observe this behaviour on the real Pi2 (and it breaks UEFI). The
> >> hardware semantics appear to be "if the command generates a response,
> >> but you didn't want to see it, we'll successfully complete the command and
> >> ignore the response", whereas the sdhci implementation raises an error for
> >> this as well as signalling completion. I have read the "SD Specifications Part A2
> >> SD Host Controller Simplified Specification Version 2.00", but did not find
> >> anything describing this case, so it could be that this is open to interpretation.
> >> (It could also be specified in SDHC v3.) The specific error also seems odd -- my
> >> understanding is that a "command index" error means that the index in the
> >> response didn't match the index of the issued command, but that's hardly
> >> what is happening here.
FYI the SDHCI v3 spec is available online. Confusingly, it's listed
as an addendum instead of explicitly as the sdhci spec. The file is
partA2_300.pdf .
-Kevin
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 5/8] bcm2835_peripherals: add rollup device for bcm2835 peripherals
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
` (2 preceding siblings ...)
2015-12-04 6:01 ` [Qemu-devel] [PATCH 4/8] bcm2835_emmc: add bcm2835 MMC/SD controller Andrew Baumann
@ 2015-12-04 6:01 ` Andrew Baumann
2015-12-04 6:01 ` [Qemu-devel] [PATCH 6/8] bcm2836_control: add bcm2836 ARM control logic Andrew Baumann
` (4 subsequent siblings)
8 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-04 6:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, Andrew Baumann, qemu-arm, Paolo Bonzini
This device maintains all the non-CPU peripherals on bcm2835 (Pi1)
which are also present on bcm2836 (Pi2). It also implements the
private address space used for DMA.
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
hw/arm/Makefile.objs | 1 +
hw/arm/bcm2835_peripherals.c | 185 +++++++++++++++++++++++++++++++++++
include/hw/arm/bcm2835_peripherals.h | 42 ++++++++
include/hw/arm/raspi_platform.h | 161 ++++++++++++++++++++++++++++++
4 files changed, 389 insertions(+)
create mode 100644 hw/arm/bcm2835_peripherals.c
create mode 100644 include/hw/arm/bcm2835_peripherals.h
create mode 100644 include/hw/arm/raspi_platform.h
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 2195b60..82cc142 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -11,6 +11,7 @@ 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
obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
+obj-$(CONFIG_RASPI) += bcm2835_peripherals.o
obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
new file mode 100644
index 0000000..3a3a1aa
--- /dev/null
+++ b/hw/arm/bcm2835_peripherals.c
@@ -0,0 +1,185 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "hw/arm/bcm2835_peripherals.h"
+#include "hw/arm/bcm2835_mbox.h"
+#include "hw/arm/raspi_platform.h"
+
+static void bcm2835_peripherals_init(Object *obj)
+{
+ BCM2835PeripheralState *s = BCM2835_PERIPHERALS(obj);
+
+ /* Memory region for peripheral devices, which we export to our parent */
+ memory_region_init_io(&s->peri_mr, OBJECT(s), NULL, s,
+ "bcm2835_peripherals", 0x1000000);
+ object_property_add_child(obj, "peripheral_io", OBJECT(&s->peri_mr), NULL);
+ sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->peri_mr);
+
+ /* Internal memory region for peripheral bus addresses (not exported) */
+ memory_region_init_io(&s->gpu_bus_mr, OBJECT(s), NULL, s, "bcm2835_gpu_bus",
+ (uint64_t)1 << 32);
+ object_property_add_child(obj, "gpu_bus", OBJECT(&s->gpu_bus_mr), NULL);
+
+ /* Internal memory region for communication of mailbox channel data */
+ memory_region_init_io(&s->mbox_mr, OBJECT(s), NULL, s, "bcm2835_mbox",
+ MBOX_CHAN_COUNT << 4);
+
+ /* Interrupt Controller */
+ object_initialize(&s->ic, sizeof(s->ic), TYPE_BCM2835_IC);
+ object_property_add_child(obj, "ic", OBJECT(&s->ic), NULL);
+ qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
+
+ /* UART0 */
+ s->uart0 = SYS_BUS_DEVICE(object_new("pl011"));
+ object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
+ qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
+
+ /* Semaphores / Doorbells / Mailboxes */
+ object_initialize(&s->sbm, sizeof(s->sbm), TYPE_BCM2835_SBM);
+ object_property_add_child(obj, "sbm", OBJECT(&s->sbm), NULL);
+ qdev_set_parent_bus(DEVICE(&s->sbm), sysbus_get_default());
+
+ object_property_add_const_link(OBJECT(&s->sbm), "mbox_mr",
+ OBJECT(&s->mbox_mr), &error_abort);
+
+ /* Property channel */
+ object_initialize(&s->property, sizeof(s->property), TYPE_BCM2835_PROPERTY);
+ object_property_add_child(obj, "property", OBJECT(&s->property), NULL);
+ qdev_set_parent_bus(DEVICE(&s->property), sysbus_get_default());
+
+ object_property_add_const_link(OBJECT(&s->property), "dma_mr",
+ OBJECT(&s->gpu_bus_mr), &error_abort);
+
+ /* Extended Mass Media Controller */
+ object_initialize(&s->emmc, sizeof(s->emmc), TYPE_BCM2835_EMMC);
+ object_property_add_child(obj, "emmc", OBJECT(&s->emmc), NULL);
+ qdev_set_parent_bus(DEVICE(&s->emmc), sysbus_get_default());
+}
+
+static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
+{
+ BCM2835PeripheralState *s = BCM2835_PERIPHERALS(dev);
+ MemoryRegion *ram;
+ Error *err = NULL;
+ uint32_t ram_size;
+ int n;
+
+ /* Map peripherals and RAM into the GPU address space. */
+ memory_region_init_alias(&s->peri_mr_alias, OBJECT(s),
+ "bcm2835_peripherals", &s->peri_mr, 0,
+ memory_region_size(&s->peri_mr));
+
+ memory_region_add_subregion_overlap(&s->gpu_bus_mr, BCM2835_VC_PERI_BASE,
+ &s->peri_mr_alias, 1);
+
+ /* XXX: assume that RAM is contiguous and mapped at system address zero */
+ ram = memory_region_find(get_system_memory(), 0, 1).mr;
+ assert(ram != NULL && memory_region_size(ram) >= 128 * 1024 * 1024);
+ ram_size = memory_region_size(ram);
+
+ /* RAM is aliased four times (different cache configurations) on the GPU */
+ for (n = 0; n < 4; n++) {
+ memory_region_init_alias(&s->ram_alias[n], OBJECT(s),
+ "bcm2835_gpu_ram_alias[*]", ram, 0, ram_size);
+ memory_region_add_subregion_overlap(&s->gpu_bus_mr, (hwaddr)n << 30,
+ &s->ram_alias[n], 0);
+ }
+
+ /* Interrupt Controller */
+ object_property_set_bool(OBJECT(&s->ic), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&s->peri_mr, ARMCTRL_IC_OFFSET,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->ic), 0));
+ sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
+
+ /* UART0 */
+ object_property_set_bool(OBJECT(s->uart0), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&s->peri_mr, UART0_OFFSET,
+ sysbus_mmio_get_region(s->uart0, 0));
+ sysbus_connect_irq(s->uart0, 0,
+ qdev_get_gpio_in(DEVICE(&s->ic), INTERRUPT_VC_UART));
+
+ /* Semaphores / Doorbells / Mailboxes */
+ object_property_set_bool(OBJECT(&s->sbm), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&s->peri_mr, ARMCTRL_0_SBM_OFFSET,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->sbm), 0));
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->sbm), 0,
+ qdev_get_gpio_in(DEVICE(&s->ic), INTERRUPT_ARM_MAILBOX));
+
+ /* Mailbox-addressable peripherals use the private mbox_mr address space
+ * and pseudo-irqs to dispatch requests and responses. */
+
+ /* Property channel */
+ object_property_set_int(OBJECT(&s->property), ram_size, "ram-size", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ object_property_set_bool(OBJECT(&s->property), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&s->mbox_mr, MBOX_CHAN_PROPERTY<<4,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->property), 0));
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->property), 0,
+ qdev_get_gpio_in(DEVICE(&s->sbm), MBOX_CHAN_PROPERTY));
+
+ /* Extended Mass Media Controller */
+ object_property_set_bool(OBJECT(&s->emmc), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&s->peri_mr, EMMC_OFFSET,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->emmc), 0));
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
+ qdev_get_gpio_in(DEVICE(&s->ic),
+ INTERRUPT_VC_ARASANSDIO));
+}
+
+static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ dc->realize = bcm2835_peripherals_realize;
+}
+
+static const TypeInfo bcm2835_peripherals_type_info = {
+ .name = TYPE_BCM2835_PERIPHERALS,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(BCM2835PeripheralState),
+ .instance_init = bcm2835_peripherals_init,
+ .class_init = bcm2835_peripherals_class_init,
+};
+
+static void bcm2835_peripherals_register_types(void)
+{
+ type_register_static(&bcm2835_peripherals_type_info);
+}
+
+type_init(bcm2835_peripherals_register_types)
diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
new file mode 100644
index 0000000..e760a7b
--- /dev/null
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -0,0 +1,42 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2835_PERIPHERALS_H
+#define BCM2835_PERIPHERALS_H
+
+#include "qemu-common.h"
+#include "exec/address-spaces.h"
+#include "hw/sysbus.h"
+#include "hw/intc/bcm2835_ic.h"
+#include "hw/misc/bcm2835_property.h"
+#include "hw/misc/bcm2835_sbm.h"
+#include "hw/sd/bcm2835_emmc.h"
+
+#define TYPE_BCM2835_PERIPHERALS "bcm2835_peripherals"
+#define BCM2835_PERIPHERALS(obj) \
+ OBJECT_CHECK(BCM2835PeripheralState, (obj), TYPE_BCM2835_PERIPHERALS)
+
+typedef struct BCM2835PeripheralState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+ /*< public >*/
+
+ MemoryRegion peri_mr, peri_mr_alias, gpu_bus_mr, mbox_mr;
+ MemoryRegion ram_alias[4];
+ qemu_irq irq, fiq;
+
+ SysBusDevice *uart0;
+ BCM2835IcState ic;
+ BCM2835PropertyState property;
+ BCM2835SbmState sbm;
+ BCM2835EmmcState emmc;
+} BCM2835PeripheralState;
+
+#endif /* BCM2835_PERIPHERALS_H */
diff --git a/include/hw/arm/raspi_platform.h b/include/hw/arm/raspi_platform.h
new file mode 100644
index 0000000..8386949
--- /dev/null
+++ b/include/hw/arm/raspi_platform.h
@@ -0,0 +1,161 @@
+/*
+ * bcm2708 aka bcm2835/2836 aka Raspberry Pi/Pi2 SoC platform defines
+ *
+ * These definitions are derived from those in Linux at
+ * arch/arm/mach-{bcm2708,bcm2709}/include/mach/platform.h
+ * where they carry the following notice:
+ */
+
+/*
+ * arch/arm/mach-bcm2708/include/mach/platform.h
+ *
+ * Copyright (C) 2010 Broadcom
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+/* Peripheral base address on the VC (GPU) system bus */
+#define BCM2835_VC_PERI_BASE 0x7e000000
+
+/* Peripheral base addresses seen by the CPU: Pi1 and Pi2 differ */
+#define BCM2835_PERI_BASE 0x20000000
+#define BCM2836_PERI_BASE 0x3F000000
+
+/* "QA7" (Pi2) interrupt controller and mailboxes etc. */
+#define BCM2836_CONTROL_BASE 0x40000000
+
+#define MCORE_OFFSET 0x0000 /* Fake frame buffer device
+ * (the multicore sync block) */
+#define IC0_OFFSET 0x2000
+#define ST_OFFSET 0x3000 /* System Timer */
+#define MPHI_OFFSET 0x6000 /* Message-based Parallel Host Intf. */
+#define DMA_OFFSET 0x7000 /* DMA controller, channels 0-14 */
+#define ARM_OFFSET 0xB000 /* BCM2708 ARM control block */
+#define ARMCTRL_OFFSET (ARM_OFFSET + 0x000)
+#define ARMCTRL_IC_OFFSET (ARM_OFFSET + 0x200) /* Interrupt controller */
+#define ARMCTRL_TIMER0_1_OFFSET (ARM_OFFSET + 0x400) /* Timer 0 and 1 */
+#define ARMCTRL_0_SBM_OFFSET (ARM_OFFSET + 0x800) /* User 0 (ARM) Semaphores
+ * Doorbells & Mailboxes */
+#define PM_OFFSET 0x100000 /* Power Management, Reset controller
+ * and Watchdog registers */
+#define PCM_CLOCK_OFFSET 0x101098 /* PCM Clock */
+#define RNG_OFFSET 0x104000 /* Hardware RNG */
+#define GPIO_OFFSET 0x200000 /* GPIO */
+#define UART0_OFFSET 0x201000 /* Uart 0 */
+#define MMCI0_OFFSET 0x202000 /* MMC interface */
+#define I2S_OFFSET 0x203000 /* I2S */
+#define SPI0_OFFSET 0x204000 /* SPI0 */
+#define BSC0_OFFSET 0x205000 /* BSC0 I2C/TWI */
+#define UART1_OFFSET 0x215000 /* Uart 1 */
+#define EMMC_OFFSET 0x300000 /* eMMC interface */
+#define SMI_OFFSET 0x600000 /* SMI */
+#define BSC1_OFFSET 0x804000 /* BSC1 I2C/TWI */
+#define USB_OFFSET 0x980000 /* DTC_OTG USB controller */
+#define DMA15_OFFSET 0xE05000 /* DMA controller, channel 15 */
+
+/*
+ * Interrupt assignments
+ */
+
+#define ARM_IRQ1_BASE 0
+#define INTERRUPT_TIMER0 (ARM_IRQ1_BASE + 0)
+#define INTERRUPT_TIMER1 (ARM_IRQ1_BASE + 1)
+#define INTERRUPT_TIMER2 (ARM_IRQ1_BASE + 2)
+#define INTERRUPT_TIMER3 (ARM_IRQ1_BASE + 3)
+#define INTERRUPT_CODEC0 (ARM_IRQ1_BASE + 4)
+#define INTERRUPT_CODEC1 (ARM_IRQ1_BASE + 5)
+#define INTERRUPT_CODEC2 (ARM_IRQ1_BASE + 6)
+#define INTERRUPT_VC_JPEG (ARM_IRQ1_BASE + 7)
+#define INTERRUPT_ISP (ARM_IRQ1_BASE + 8)
+#define INTERRUPT_VC_USB (ARM_IRQ1_BASE + 9)
+#define INTERRUPT_VC_3D (ARM_IRQ1_BASE + 10)
+#define INTERRUPT_TRANSPOSER (ARM_IRQ1_BASE + 11)
+#define INTERRUPT_MULTICORESYNC0 (ARM_IRQ1_BASE + 12)
+#define INTERRUPT_MULTICORESYNC1 (ARM_IRQ1_BASE + 13)
+#define INTERRUPT_MULTICORESYNC2 (ARM_IRQ1_BASE + 14)
+#define INTERRUPT_MULTICORESYNC3 (ARM_IRQ1_BASE + 15)
+#define INTERRUPT_DMA0 (ARM_IRQ1_BASE + 16)
+#define INTERRUPT_DMA1 (ARM_IRQ1_BASE + 17)
+#define INTERRUPT_VC_DMA2 (ARM_IRQ1_BASE + 18)
+#define INTERRUPT_VC_DMA3 (ARM_IRQ1_BASE + 19)
+#define INTERRUPT_DMA4 (ARM_IRQ1_BASE + 20)
+#define INTERRUPT_DMA5 (ARM_IRQ1_BASE + 21)
+#define INTERRUPT_DMA6 (ARM_IRQ1_BASE + 22)
+#define INTERRUPT_DMA7 (ARM_IRQ1_BASE + 23)
+#define INTERRUPT_DMA8 (ARM_IRQ1_BASE + 24)
+#define INTERRUPT_DMA9 (ARM_IRQ1_BASE + 25)
+#define INTERRUPT_DMA10 (ARM_IRQ1_BASE + 26)
+#define INTERRUPT_DMA11 (ARM_IRQ1_BASE + 27)
+#define INTERRUPT_DMA12 (ARM_IRQ1_BASE + 28)
+#define INTERRUPT_AUX (ARM_IRQ1_BASE + 29)
+#define INTERRUPT_ARM (ARM_IRQ1_BASE + 30)
+#define INTERRUPT_VPUDMA (ARM_IRQ1_BASE + 31)
+
+#define ARM_IRQ2_BASE 32
+#define INTERRUPT_HOSTPORT (ARM_IRQ2_BASE + 0)
+#define INTERRUPT_VIDEOSCALER (ARM_IRQ2_BASE + 1)
+#define INTERRUPT_CCP2TX (ARM_IRQ2_BASE + 2)
+#define INTERRUPT_SDC (ARM_IRQ2_BASE + 3)
+#define INTERRUPT_DSI0 (ARM_IRQ2_BASE + 4)
+#define INTERRUPT_AVE (ARM_IRQ2_BASE + 5)
+#define INTERRUPT_CAM0 (ARM_IRQ2_BASE + 6)
+#define INTERRUPT_CAM1 (ARM_IRQ2_BASE + 7)
+#define INTERRUPT_HDMI0 (ARM_IRQ2_BASE + 8)
+#define INTERRUPT_HDMI1 (ARM_IRQ2_BASE + 9)
+#define INTERRUPT_PIXELVALVE1 (ARM_IRQ2_BASE + 10)
+#define INTERRUPT_I2CSPISLV (ARM_IRQ2_BASE + 11)
+#define INTERRUPT_DSI1 (ARM_IRQ2_BASE + 12)
+#define INTERRUPT_PWA0 (ARM_IRQ2_BASE + 13)
+#define INTERRUPT_PWA1 (ARM_IRQ2_BASE + 14)
+#define INTERRUPT_CPR (ARM_IRQ2_BASE + 15)
+#define INTERRUPT_SMI (ARM_IRQ2_BASE + 16)
+#define INTERRUPT_GPIO0 (ARM_IRQ2_BASE + 17)
+#define INTERRUPT_GPIO1 (ARM_IRQ2_BASE + 18)
+#define INTERRUPT_GPIO2 (ARM_IRQ2_BASE + 19)
+#define INTERRUPT_GPIO3 (ARM_IRQ2_BASE + 20)
+#define INTERRUPT_VC_I2C (ARM_IRQ2_BASE + 21)
+#define INTERRUPT_VC_SPI (ARM_IRQ2_BASE + 22)
+#define INTERRUPT_VC_I2SPCM (ARM_IRQ2_BASE + 23)
+#define INTERRUPT_VC_SDIO (ARM_IRQ2_BASE + 24)
+#define INTERRUPT_VC_UART (ARM_IRQ2_BASE + 25)
+#define INTERRUPT_SLIMBUS (ARM_IRQ2_BASE + 26)
+#define INTERRUPT_VEC (ARM_IRQ2_BASE + 27)
+#define INTERRUPT_CPG (ARM_IRQ2_BASE + 28)
+#define INTERRUPT_RNG (ARM_IRQ2_BASE + 29)
+#define INTERRUPT_VC_ARASANSDIO (ARM_IRQ2_BASE + 30)
+#define INTERRUPT_AVSPMON (ARM_IRQ2_BASE + 31)
+
+#define ARM_IRQ0_BASE 64
+#define INTERRUPT_ARM_TIMER (ARM_IRQ0_BASE + 0)
+#define INTERRUPT_ARM_MAILBOX (ARM_IRQ0_BASE + 1)
+#define INTERRUPT_ARM_DOORBELL_0 (ARM_IRQ0_BASE + 2)
+#define INTERRUPT_ARM_DOORBELL_1 (ARM_IRQ0_BASE + 3)
+#define INTERRUPT_VPU0_HALTED (ARM_IRQ0_BASE + 4)
+#define INTERRUPT_VPU1_HALTED (ARM_IRQ0_BASE + 5)
+#define INTERRUPT_ILLEGAL_TYPE0 (ARM_IRQ0_BASE + 6)
+#define INTERRUPT_ILLEGAL_TYPE1 (ARM_IRQ0_BASE + 7)
+#define INTERRUPT_PENDING1 (ARM_IRQ0_BASE + 8)
+#define INTERRUPT_PENDING2 (ARM_IRQ0_BASE + 9)
+#define INTERRUPT_JPEG (ARM_IRQ0_BASE + 10)
+#define INTERRUPT_USB (ARM_IRQ0_BASE + 11)
+#define INTERRUPT_3D (ARM_IRQ0_BASE + 12)
+#define INTERRUPT_DMA2 (ARM_IRQ0_BASE + 13)
+#define INTERRUPT_DMA3 (ARM_IRQ0_BASE + 14)
+#define INTERRUPT_I2C (ARM_IRQ0_BASE + 15)
+#define INTERRUPT_SPI (ARM_IRQ0_BASE + 16)
+#define INTERRUPT_I2SPCM (ARM_IRQ0_BASE + 17)
+#define INTERRUPT_SDIO (ARM_IRQ0_BASE + 18)
+#define INTERRUPT_UART (ARM_IRQ0_BASE + 19)
+#define INTERRUPT_ARASANSDIO (ARM_IRQ0_BASE + 20)
--
2.5.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 6/8] bcm2836_control: add bcm2836 ARM control logic
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
` (3 preceding siblings ...)
2015-12-04 6:01 ` [Qemu-devel] [PATCH 5/8] bcm2835_peripherals: add rollup device for bcm2835 peripherals Andrew Baumann
@ 2015-12-04 6:01 ` Andrew Baumann
2015-12-04 6:01 ` [Qemu-devel] [PATCH 7/8] bcm2836: add bcm2836 soc device Andrew Baumann
` (3 subsequent siblings)
8 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-04 6:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, Andrew Baumann, qemu-arm, Paolo Bonzini
This module is specific to the bcm2836 (Pi2). It implements the top
level interrupt controller, and mailboxes used for inter-processor
synchronisation.
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
hw/intc/Makefile.objs | 2 +-
hw/intc/bcm2836_control.c | 344 ++++++++++++++++++++++++++++++++++++++
include/hw/intc/bcm2836_control.h | 49 ++++++
3 files changed, 394 insertions(+), 1 deletion(-)
create mode 100644 hw/intc/bcm2836_control.c
create mode 100644 include/hw/intc/bcm2836_control.h
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 2ad1204..6a13a39 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -24,7 +24,7 @@ obj-$(CONFIG_GRLIB) += grlib_irqmp.o
obj-$(CONFIG_IOAPIC) += ioapic.o
obj-$(CONFIG_OMAP) += omap_intc.o
obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
-obj-$(CONFIG_RASPI) += bcm2835_ic.o
+obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
obj-$(CONFIG_SH4) += sh_intc.o
obj-$(CONFIG_XICS) += xics.o
obj-$(CONFIG_XICS_KVM) += xics_kvm.o
diff --git a/hw/intc/bcm2836_control.c b/hw/intc/bcm2836_control.c
new file mode 100644
index 0000000..c47b36f
--- /dev/null
+++ b/hw/intc/bcm2836_control.c
@@ -0,0 +1,344 @@
+/*
+ * Rasperry Pi 2 emulation ARM control logic module.
+ * Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * At present, only implements interrupt routing, and mailboxes (i.e.,
+ * not local timer, PMU interrupt, or AXI counters).
+ *
+ * Ref:
+ * https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2836/QA7_rev3.4.pdf
+ *
+ * Based on bcm2835_ic.c, terms below...
+ */
+
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+/* Heavily based on pl190.c, copyright terms below. */
+
+/*
+ * Arm PrimeCell PL190 Vector Interrupt Controller
+ *
+ * Copyright (c) 2006 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ */
+
+#include "hw/intc/bcm2836_control.h"
+
+#define ROUTE_CORE(x) ((x) & 0x3)
+#define ROUTE_FIQ(x) (((x) & 0x4) != 0)
+
+#define IRQ_BIT(cntrl, num) (((cntrl) & (1 << (num))) != 0)
+#define FIQ_BIT(cntrl, num) (((cntrl) & (1 << ((num) + 4))) != 0)
+
+#define IRQ_CNTPSIRQ 0
+#define IRQ_CNTPNSIRQ 1
+#define IRQ_CNTHPIRQ 2
+#define IRQ_CNTVIRQ 3
+#define IRQ_MAILBOX0 4
+#define IRQ_MAILBOX1 5
+#define IRQ_MAILBOX2 6
+#define IRQ_MAILBOX3 7
+#define IRQ_GPU 8
+#define IRQ_PMU 9
+#define IRQ_AXI 10
+#define IRQ_TIMER 11
+#define IRQ_MAX IRQ_TIMER
+
+/* Update interrupts. */
+static void bcm2836_control_update(BCM2836ControlState *s)
+{
+ int i, j;
+
+ /*
+ * reset pending IRQs/FIQs
+ */
+
+ for (i = 0; i < BCM2836_NCORES; i++) {
+ s->irqsrc[i] = s->fiqsrc[i] = 0;
+ }
+
+ /*
+ * apply routing logic, update status regs
+ */
+
+ if (s->gpu_irq) {
+ assert(s->route_gpu_irq < BCM2836_NCORES);
+ s->irqsrc[s->route_gpu_irq] |= (uint32_t)1 << IRQ_GPU;
+ }
+
+ if (s->gpu_fiq) {
+ assert(s->route_gpu_fiq < BCM2836_NCORES);
+ s->fiqsrc[s->route_gpu_fiq] |= (uint32_t)1 << IRQ_GPU;
+ }
+
+ for (i = 0; i < BCM2836_NCORES; i++) {
+ /* handle local interrupts for this core */
+ if (s->localirqs[i]) {
+ assert(s->localirqs[i] < (1 << IRQ_MAILBOX0));
+ for (j = 0; j < IRQ_MAILBOX0; j++) {
+ if ((s->localirqs[i] & (1 << j)) != 0) {
+ /* local interrupt j is set */
+ if (FIQ_BIT(s->timercontrol[i], j)) {
+ /* deliver a FIQ */
+ s->fiqsrc[i] |= (uint32_t)1 << j;
+ } else if (IRQ_BIT(s->timercontrol[i], j)) {
+ /* deliver an IRQ */
+ s->irqsrc[i] |= (uint32_t)1 << j;
+ } else {
+ /* the interrupt is masked */
+ }
+ }
+ }
+ }
+
+ /* handle mailboxes for this core */
+ for (j = 0; j < BCM2836_MBPERCORE; j++) {
+ if (s->mailboxes[i * BCM2836_MBPERCORE + j] != 0) {
+ /* mailbox j is set */
+ if (FIQ_BIT(s->mailboxcontrol[i], j)) {
+ /* deliver a FIQ */
+ s->fiqsrc[i] |= (uint32_t)1 << (j + IRQ_MAILBOX0);
+ } else if (IRQ_BIT(s->mailboxcontrol[i], j)) {
+ /* deliver an IRQ */
+ s->irqsrc[i] |= (uint32_t)1 << (j + IRQ_MAILBOX0);
+ } else {
+ /* the interrupt is masked */
+ }
+ }
+ }
+ }
+
+ /*
+ * call set_irq appropriately for each output
+ */
+
+ for (i = 0; i < BCM2836_NCORES; i++) {
+ qemu_set_irq(s->irq[i], s->irqsrc[i] != 0);
+ qemu_set_irq(s->fiq[i], s->fiqsrc[i] != 0);
+ }
+}
+
+static void bcm2836_control_set_local_irq(void *opaque, int core, int local_irq,
+ int level)
+{
+ BCM2836ControlState *s = (BCM2836ControlState *)opaque;
+
+ assert(core >= 0 && core < BCM2836_NCORES);
+ assert(local_irq >= 0 && local_irq <= IRQ_CNTVIRQ);
+
+ if (level) {
+ s->localirqs[core] |= 1 << local_irq;
+ } else {
+ s->localirqs[core] &= ~((uint32_t)1 << local_irq);
+ }
+
+ bcm2836_control_update(s);
+}
+
+/* XXX: the following wrapper functions are a kludgy workaround,
+ * needed because I can't seem to pass useful information in the "irq"
+ * parameter when using named interrupts. Feel free to clean this up!
+ */
+
+static void bcm2836_control_set_local_irq0(void *opaque, int core, int level)
+{
+ bcm2836_control_set_local_irq(opaque, core, 0, level);
+}
+
+static void bcm2836_control_set_local_irq1(void *opaque, int core, int level)
+{
+ bcm2836_control_set_local_irq(opaque, core, 1, level);
+}
+
+static void bcm2836_control_set_local_irq2(void *opaque, int core, int level)
+{
+ bcm2836_control_set_local_irq(opaque, core, 2, level);
+}
+
+static void bcm2836_control_set_local_irq3(void *opaque, int core, int level)
+{
+ bcm2836_control_set_local_irq(opaque, core, 3, level);
+}
+
+static void bcm2836_control_set_gpu_irq(void *opaque, int irq, int level)
+{
+ BCM2836ControlState *s = (BCM2836ControlState *)opaque;
+
+ s->gpu_irq = level;
+
+ bcm2836_control_update(s);
+}
+
+static void bcm2836_control_set_gpu_fiq(void *opaque, int irq, int level)
+{
+ BCM2836ControlState *s = (BCM2836ControlState *)opaque;
+
+ s->gpu_fiq = level;
+
+ bcm2836_control_update(s);
+}
+
+static uint64_t bcm2836_control_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+ BCM2836ControlState *s = (BCM2836ControlState *)opaque;
+
+ if (offset == 0xc) {
+ /* GPU interrupt routing */
+ assert(s->route_gpu_fiq < BCM2836_NCORES
+ && s->route_gpu_irq < BCM2836_NCORES);
+ return ((uint32_t)s->route_gpu_fiq << 2) | s->route_gpu_irq;
+ } else if (offset >= 0x40 && offset < 0x50) {
+ /* Timer interrupt control registers */
+ return s->timercontrol[(offset - 0x40) >> 2];
+ } else if (offset >= 0x50 && offset < 0x60) {
+ /* Mailbox interrupt control registers */
+ return s->mailboxcontrol[(offset - 0x50) >> 2];
+ } else if (offset >= 0x60 && offset < 0x70) {
+ /* IRQ source registers */
+ return s->irqsrc[(offset - 0x60) >> 2];
+ } else if (offset >= 0x70 && offset < 0x80) {
+ /* FIQ source registers */
+ return s->fiqsrc[(offset - 0x70) >> 2];
+ } else if (offset >= 0xc0 && offset < 0x100) {
+ /* Mailboxes */
+ return s->mailboxes[(offset - 0xc0) >> 2];
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2836_control_read: Bad offset %x\n", (int)offset);
+ return 0;
+ }
+}
+
+static void bcm2836_control_write(void *opaque, hwaddr offset,
+ uint64_t val, unsigned size)
+{
+ BCM2836ControlState *s = (BCM2836ControlState *)opaque;
+
+ if (offset == 0xc) {
+ /* GPU interrupt routing */
+ s->route_gpu_irq = val & 0x3;
+ s->route_gpu_fiq = (val >> 2) & 0x3;
+ } else if (offset >= 0x40 && offset < 0x50) {
+ /* Timer interrupt control registers */
+ s->timercontrol[(offset - 0x40) >> 2] = val & 0xff;
+ } else if (offset >= 0x50 && offset < 0x60) {
+ /* Mailbox interrupt control registers */
+ s->mailboxcontrol[(offset - 0x50) >> 2] = val & 0xff;
+ } else if (offset >= 0x80 && offset < 0xc0) {
+ /* Mailbox set registers */
+ s->mailboxes[(offset - 0x80) >> 2] |= val;
+ } else if (offset >= 0xc0 && offset < 0x100) {
+ /* Mailbox clear registers */
+ s->mailboxes[(offset - 0xc0) >> 2] &= ~val;
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "bcm2836_control_write: Bad offset %x\n", (int)offset);
+ return;
+ }
+
+ bcm2836_control_update(s);
+}
+
+static const MemoryRegionOps bcm2836_control_ops = {
+ .read = bcm2836_control_read,
+ .write = bcm2836_control_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void bcm2836_control_reset(DeviceState *d)
+{
+ BCM2836ControlState *s = BCM2836_CONTROL(d);
+ int i;
+
+ s->route_gpu_irq = s->route_gpu_fiq = 0;
+
+ for (i = 0; i < BCM2836_NCORES; i++) {
+ s->timercontrol[i] = 0;
+ s->mailboxcontrol[i] = 0;
+ }
+
+ for (i = 0; i < BCM2836_NCORES * BCM2836_MBPERCORE; i++) {
+ s->mailboxes[i] = 0;
+ }
+}
+
+static void bcm2836_control_init(Object *obj)
+{
+ BCM2836ControlState *s = BCM2836_CONTROL(obj);
+ DeviceState *dev = DEVICE(obj);
+
+ memory_region_init_io(&s->iomem, obj, &bcm2836_control_ops, s,
+ TYPE_BCM2836_CONTROL, 0x100);
+ sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
+
+ /* inputs from each CPU core */
+ qdev_init_gpio_in_named(dev, bcm2836_control_set_local_irq0, "cntpsirq",
+ BCM2836_NCORES);
+ qdev_init_gpio_in_named(dev, bcm2836_control_set_local_irq1, "cntpnsirq",
+ BCM2836_NCORES);
+ qdev_init_gpio_in_named(dev, bcm2836_control_set_local_irq2, "cnthpirq",
+ BCM2836_NCORES);
+ qdev_init_gpio_in_named(dev, bcm2836_control_set_local_irq3, "cntvirq",
+ BCM2836_NCORES);
+ /* qdev_init_gpio_in_named(dev, bcm2836_control_set_pmu_irq, "pmuirq",
+ BCM2836_NCORES); */
+
+ /* IRQ and FIQ inputs from upstream bcm2835 controller */
+ qdev_init_gpio_in_named(dev, bcm2836_control_set_gpu_irq, "gpu_irq", 1);
+ qdev_init_gpio_in_named(dev, bcm2836_control_set_gpu_fiq, "gpu_fiq", 1);
+
+ /* outputs to CPU cores */
+ qdev_init_gpio_out_named(dev, s->irq, "irq", BCM2836_NCORES);
+ qdev_init_gpio_out_named(dev, s->fiq, "fiq", BCM2836_NCORES);
+}
+
+static void bcm2836_control_realize(DeviceState *dev, Error **errp)
+{
+}
+
+static const VMStateDescription vmstate_bcm2836_control = {
+ .name = TYPE_BCM2836_CONTROL,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(mailboxes, BCM2836ControlState,
+ BCM2836_NCORES * BCM2836_MBPERCORE),
+ VMSTATE_UINT8(route_gpu_irq, BCM2836ControlState),
+ VMSTATE_UINT8(route_gpu_fiq, BCM2836ControlState),
+ VMSTATE_UINT32_ARRAY(timercontrol, BCM2836ControlState, BCM2836_NCORES),
+ VMSTATE_UINT32_ARRAY(mailboxcontrol, BCM2836ControlState,
+ BCM2836_NCORES),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void bcm2836_control_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = bcm2836_control_realize;
+ dc->reset = bcm2836_control_reset;
+ dc->vmsd = &vmstate_bcm2836_control;
+}
+
+static TypeInfo bcm2836_control_info = {
+ .name = TYPE_BCM2836_CONTROL,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(BCM2836ControlState),
+ .class_init = bcm2836_control_class_init,
+ .instance_init = bcm2836_control_init,
+};
+
+static void bcm2836_control_register_types(void)
+{
+ type_register_static(&bcm2836_control_info);
+}
+
+type_init(bcm2836_control_register_types)
diff --git a/include/hw/intc/bcm2836_control.h b/include/hw/intc/bcm2836_control.h
new file mode 100644
index 0000000..b6345cc
--- /dev/null
+++ b/include/hw/intc/bcm2836_control.h
@@ -0,0 +1,49 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2836_CONTROL_H
+#define BCM2836_CONTROL_H
+
+#include "hw/sysbus.h"
+
+/* 4 mailboxes per core, for 16 total */
+#define BCM2836_NCORES 4
+#define BCM2836_MBPERCORE 4
+
+#define TYPE_BCM2836_CONTROL "bcm2836_control"
+#define BCM2836_CONTROL(obj) \
+ OBJECT_CHECK(BCM2836ControlState, (obj), TYPE_BCM2836_CONTROL)
+
+typedef struct BCM2836ControlState {
+ SysBusDevice busdev;
+ MemoryRegion iomem;
+
+ /* interrupt status registers (not directly visible to user) */
+ bool gpu_irq, gpu_fiq;
+ uint32_t localirqs[BCM2836_NCORES];
+
+ /* mailboxes */
+ uint32_t mailboxes[BCM2836_NCORES * BCM2836_MBPERCORE];
+
+ /* interrupt routing/control registers */
+ uint8_t route_gpu_irq, route_gpu_fiq;
+ uint32_t timercontrol[BCM2836_NCORES];
+ uint32_t mailboxcontrol[BCM2836_NCORES];
+
+ /* interrupt source registers, post-routing (visible) */
+ uint32_t irqsrc[BCM2836_NCORES];
+ uint32_t fiqsrc[BCM2836_NCORES];
+
+ /* outputs to CPU cores */
+ qemu_irq irq[BCM2836_NCORES];
+ qemu_irq fiq[BCM2836_NCORES];
+} BCM2836ControlState;
+
+#endif
--
2.5.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 7/8] bcm2836: add bcm2836 soc device
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
` (4 preceding siblings ...)
2015-12-04 6:01 ` [Qemu-devel] [PATCH 6/8] bcm2836_control: add bcm2836 ARM control logic Andrew Baumann
@ 2015-12-04 6:01 ` Andrew Baumann
2015-12-04 6:01 ` [Qemu-devel] [PATCH 8/8] raspi: add raspberry pi 2 machine Andrew Baumann
` (2 subsequent siblings)
8 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-04 6:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, Andrew Baumann, qemu-arm, Paolo Bonzini
This is the SoC for Raspberry Pi 2.
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
hw/arm/Makefile.objs | 2 +-
hw/arm/bcm2836.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++
include/hw/arm/bcm2836.h | 33 ++++++++++++
3 files changed, 169 insertions(+), 1 deletion(-)
create mode 100644 hw/arm/bcm2836.c
create mode 100644 include/hw/arm/bcm2836.h
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 82cc142..f55f8d2 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -11,7 +11,7 @@ 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
obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
-obj-$(CONFIG_RASPI) += bcm2835_peripherals.o
+obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o
obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
new file mode 100644
index 0000000..4b0544e
--- /dev/null
+++ b/hw/arm/bcm2836.c
@@ -0,0 +1,135 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "hw/arm/bcm2836.h"
+#include "hw/arm/raspi_platform.h"
+#include "hw/sysbus.h"
+#include "sysemu/sysemu.h" /* for smp_cpus */
+#include "exec/address-spaces.h"
+
+static void bcm2836_init(Object *obj)
+{
+ BCM2836State *s = BCM2836(obj);
+ int n;
+
+ /* TODO: probably shouldn't be using smp_cpus here */
+ assert(smp_cpus <= BCM2836_NCPUS);
+ for (n = 0; n < smp_cpus; n++) {
+ object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
+ "cortex-a15-" TYPE_ARM_CPU);
+ object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
+ &error_abort);
+ }
+
+ object_initialize(&s->ic, sizeof(s->ic), TYPE_BCM2836_CONTROL);
+ object_property_add_child(obj, "ic", OBJECT(&s->ic), NULL);
+ qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
+
+ object_initialize(&s->peripherals, sizeof(s->peripherals),
+ TYPE_BCM2835_PERIPHERALS);
+ object_property_add_child(obj, "peripherals", OBJECT(&s->peripherals),
+ &error_abort);
+ qdev_set_parent_bus(DEVICE(&s->peripherals), sysbus_get_default());
+}
+
+static void bcm2836_realize(DeviceState *dev, Error **errp)
+{
+ BCM2836State *s = BCM2836(dev);
+ Error *err = NULL;
+ int n;
+
+ /* common peripherals from bcm2835 */
+ object_property_set_bool(OBJECT(&s->peripherals), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 0,
+ BCM2836_PERI_BASE, 1);
+
+ /* bcm2836 interrupt controller (and mailboxes, etc.) */
+ object_property_set_bool(OBJECT(&s->ic), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->ic), 0, BCM2836_CONTROL_BASE);
+
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0,
+ qdev_get_gpio_in_named(DEVICE(&s->ic), "gpu_irq", 0));
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
+ qdev_get_gpio_in_named(DEVICE(&s->ic), "gpu_fiq", 0));
+
+ /* TODO: probably shouldn't be using smp_cpus here */
+ assert(smp_cpus <= BCM2836_NCPUS);
+ for (n = 0; n < smp_cpus; n++) {
+ /* Mirror bcm2836, which has clusterid set to 0xf */
+ s->cpus[n].mp_affinity = 0xF00 | n;
+
+ /* set periphbase/CBAR value for CPU-local registers */
+ object_property_set_int(OBJECT(&s->cpus[n]),
+ BCM2836_PERI_BASE + MCORE_OFFSET,
+ "reset-cbar", &err);
+ if (err) {
+ error_report_err(err);
+ exit(1);
+ }
+
+ object_property_set_bool(OBJECT(&s->cpus[n]), true, "realized", &err);
+ if (err) {
+ error_report_err(err);
+ exit(1);
+ }
+
+ /* Connect irq/fiq outputs from the interrupt controller. */
+ qdev_connect_gpio_out_named(DEVICE(&s->ic), "irq", n,
+ qdev_get_gpio_in(DEVICE(&s->cpus[n]),
+ ARM_CPU_IRQ));
+ qdev_connect_gpio_out_named(DEVICE(&s->ic), "fiq", n,
+ qdev_get_gpio_in(DEVICE(&s->cpus[n]),
+ ARM_CPU_FIQ));
+
+ /* Connect timers from the CPU to the interrupt controller */
+ s->cpus[n].gt_timer_outputs[GTIMER_PHYS]
+ = qdev_get_gpio_in_named(DEVICE(&s->ic), "cntpsirq", 0);
+ s->cpus[n].gt_timer_outputs[GTIMER_VIRT]
+ = qdev_get_gpio_in_named(DEVICE(&s->ic), "cntvirq", 0);
+ }
+}
+
+static void bcm2836_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ dc->realize = bcm2836_realize;
+
+ /*
+ * Reason: creates an ARM CPU, thus use after free(), see
+ * arm_cpu_class_init()
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
+}
+
+static const TypeInfo bcm2836_type_info = {
+ .name = TYPE_BCM2836,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(BCM2836State),
+ .instance_init = bcm2836_init,
+ .class_init = bcm2836_class_init,
+};
+
+static void bcm2836_register_types(void)
+{
+ type_register_static(&bcm2836_type_info);
+}
+
+type_init(bcm2836_register_types)
diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
new file mode 100644
index 0000000..a78e919
--- /dev/null
+++ b/include/hw/arm/bcm2836.h
@@ -0,0 +1,33 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2836_H
+#define BCM2836_H
+
+#include "hw/arm/arm.h"
+#include "hw/arm/bcm2835_peripherals.h"
+#include "hw/intc/bcm2836_control.h"
+
+#define TYPE_BCM2836 "bcm2836"
+#define BCM2836(obj) OBJECT_CHECK(BCM2836State, (obj), TYPE_BCM2836)
+
+#define BCM2836_NCPUS 4
+
+typedef struct BCM2836State {
+ /*< private >*/
+ DeviceState parent_obj;
+ /*< public >*/
+
+ ARMCPU cpus[BCM2836_NCPUS];
+ BCM2836ControlState ic;
+ BCM2835PeripheralState peripherals;
+} BCM2836State;
+
+#endif /* BCM2836_H */
--
2.5.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 8/8] raspi: add raspberry pi 2 machine
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
` (5 preceding siblings ...)
2015-12-04 6:01 ` [Qemu-devel] [PATCH 7/8] bcm2836: add bcm2836 soc device Andrew Baumann
@ 2015-12-04 6:01 ` Andrew Baumann
2015-12-07 6:36 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Peter Crosthwaite
2015-12-21 22:49 ` Peter Crosthwaite
8 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-04 6:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, Andrew Baumann, qemu-arm, Paolo Bonzini
bcm2835/Pi1 requires more peripherals, and will be added in a later
patch series.
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
hw/arm/Makefile.objs | 2 +-
hw/arm/raspi.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 180 insertions(+), 1 deletion(-)
create mode 100644 hw/arm/raspi.c
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index f55f8d2..a711e4d 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -11,7 +11,7 @@ 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
obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
-obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o
+obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
new file mode 100644
index 0000000..cc80c56
--- /dev/null
+++ b/hw/arm/raspi.c
@@ -0,0 +1,179 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+/* Based on versatilepb.c, copyright terms below. */
+
+/*
+ * ARM Versatile Platform/Application Baseboard System emulation.
+ *
+ * Copyright (c) 2005-2007 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ */
+
+#include "hw/arm/bcm2836.h"
+#include "qemu/error-report.h"
+#include "hw/boards.h"
+#include "hw/loader.h"
+#include "hw/arm/arm.h"
+#include "sysemu/sysemu.h"
+#include "hw/arm/raspi_platform.h"
+
+#define SMPBOOT_ADDR 0x300 /* this should leave enough space for ATAGS */
+#define MVBAR_ADDR 0x400 /* secure vectors */
+#define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */
+#define FIRMWARE_ADDR 0x8000 /* Pi loads kernel.img here by default */
+
+/* Table of Linux board IDs for different Pi versions */
+static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43};
+
+typedef struct RaspiMachineState {
+ union {
+ Object obj;
+ BCM2836State pi2;
+ } soc;
+ MemoryRegion ram;
+} RaspiMachineState;
+
+static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
+{
+ static const uint32_t smpboot[] = {
+ 0xE1A0E00F, /* mov lr, pc */
+ 0xE3A0FE42, /* mov pc, #0x420 ;call BOARDSETUP_ADDR */
+ 0xEE100FB0, /* mrc p15, 0, r0, c0, c0, 5;get core ID */
+ 0xE7E10050, /* ubfx r0, r0, #0, #2 ;extract LSB */
+ 0xE59F5014, /* ldr r5, =0x400000CC ;load mbox base */
+ 0xE320F001, /* 1: yield */
+ 0xE7953200, /* ldr r3, [r5, r0, lsl #4] ;read mbox for our core*/
+ 0xE3530000, /* cmp r3, #0 ;spin while zero */
+ 0x0AFFFFFB, /* beq 1b */
+ 0xE7853200, /* str r3, [r5, r0, lsl #4] ;clear mbox */
+ 0xE12FFF13, /* bx r3 ;jump to target */
+ 0x400000CC, /* (constant: mailbox 3 read/clear base) */
+ };
+
+ assert(SMPBOOT_ADDR + sizeof(smpboot) <= MVBAR_ADDR);
+ rom_add_blob_fixed("raspi_smpboot", smpboot, sizeof(smpboot),
+ info->smp_loader_start);
+}
+
+static void write_board_setup(ARMCPU *cpu, const struct arm_boot_info *info)
+{
+ static const uint32_t board_setup[] = {
+ /* MVBAR_ADDR: secure monitor vectors */
+ 0xEAFFFFFE, /* (spin) */
+ 0xEAFFFFFE, /* (spin) */
+ 0xE1B0F00E, /* movs pc, lr ;SMC exception return */
+ 0xEAFFFFFE, /* (spin) */
+ 0xEAFFFFFE, /* (spin) */
+ 0xEAFFFFFE, /* (spin) */
+ 0xEAFFFFFE, /* (spin) */
+ 0xEAFFFFFE, /* (spin) */
+ /* BOARDSETUP_ADDR */
+ 0xE3A00B01, /* mov r0, #0x400 ;MVBAR_ADDR */
+ 0xEE0C0F30, /* mcr p15, 0, r0, c12, c0, 1 ;set MVBAR */
+ 0xE3000131, /* movw r0, #0x131 ;enable HVC, AW, FW, NS */
+ 0xEE010F11, /* mcr p15, 0, r0, c1, c1, 0 ;write SCR */
+ 0xE1A0100E, /* mov r1, lr ;save LR across SMC */
+ 0xE1600070, /* smc #0 ;monitor call */
+ 0xE1A0F001, /* mov pc, r1 ;return */
+ };
+
+ rom_add_blob_fixed("raspi_boardsetup", board_setup, sizeof(board_setup),
+ MVBAR_ADDR);
+}
+
+static void reset_secondary(ARMCPU *cpu, const struct arm_boot_info *info)
+{
+ CPUState *cs = CPU(cpu);
+ cpu_set_pc(cs, info->smp_loader_start);
+}
+
+static void setup_boot(MachineState *machine, int version, size_t ram_size)
+{
+ static struct arm_boot_info binfo;
+ int r;
+
+ binfo.board_id = raspi_boardid[version];
+ binfo.ram_size = ram_size;
+ binfo.nb_cpus = smp_cpus;
+
+ /* Pi2 supports security extensions, which require special setup code */
+ if (version == 2) {
+ binfo.smp_loader_start = SMPBOOT_ADDR,
+ binfo.write_secondary_boot = write_smpboot,
+ binfo.secondary_cpu_reset_hook = reset_secondary,
+ binfo.board_setup_addr = BOARDSETUP_ADDR;
+ binfo.write_board_setup = write_board_setup;
+ binfo.secure_board_setup = true;
+ binfo.secure_boot = true;
+ }
+
+ /* If the user specified a "firmware" image (e.g. UEFI), we bypass
+ the normal Linux boot process */
+ if (machine->firmware) {
+ /* load the firmware image (typically kernel.img) */
+ r = load_image_targphys(machine->firmware, FIRMWARE_ADDR,
+ ram_size - FIRMWARE_ADDR);
+ if (r < 0) {
+ error_report("Failed to load firmware from %s", machine->firmware);
+ exit(1);
+ }
+
+ /* set variables so arm_load_kernel does the right thing */
+ binfo.entry = FIRMWARE_ADDR;
+ binfo.firmware_loaded = true;
+ } else {
+ /* Just let arm_load_kernel do everything for us... */
+ binfo.kernel_filename = machine->kernel_filename;
+ binfo.kernel_cmdline = machine->kernel_cmdline;
+ binfo.initrd_filename = machine->initrd_filename;
+ }
+
+ arm_load_kernel(ARM_CPU(first_cpu), &binfo);
+}
+
+static void raspi2_init(MachineState *machine)
+{
+ RaspiMachineState *s = g_new0(RaspiMachineState, 1);
+
+ /* Initialise the SOC */
+ object_initialize(&s->soc.pi2, sizeof(s->soc.pi2), TYPE_BCM2836);
+ object_property_add_child(OBJECT(machine), "soc", &s->soc.obj,
+ &error_abort);
+
+ /* Allocate and map RAM */
+ memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
+ machine->ram_size);
+ memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram, 0);
+
+ /* Setup the SOC */
+ object_property_set_bool(&s->soc.obj, true, "realized", &error_abort);
+
+ /* Boot! */
+ setup_boot(machine, 2, machine->ram_size);
+}
+
+static void raspi2_machine_init(MachineClass *mc)
+{
+ mc->desc = "Raspberry Pi 2";
+ mc->init = raspi2_init;
+ mc->block_default_type = IF_SD;
+ mc->no_parallel = 1;
+ mc->no_floppy = 1;
+ mc->no_cdrom = 1;
+ mc->max_cpus = BCM2836_NCPUS;
+ /* XXX: Temporary restriction in RAM size from the full 1GB. Since
+ * we do not yet support the framebuffer / GPU, we need to limit
+ * RAM usable by the OS to sit below the peripherals. */
+ mc->default_ram_size = BCM2836_PERI_BASE;
+};
+DEFINE_MACHINE("raspi2", raspi2_machine_init)
--
2.5.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
` (6 preceding siblings ...)
2015-12-04 6:01 ` [Qemu-devel] [PATCH 8/8] raspi: add raspberry pi 2 machine Andrew Baumann
@ 2015-12-07 6:36 ` Peter Crosthwaite
2015-12-07 17:24 ` Andrew Baumann
2015-12-21 22:49 ` Peter Crosthwaite
8 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-07 6:36 UTC (permalink / raw)
To: Andrew Baumann
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> This adds the system mailboxes which are used to communicate with a
> number of GPU peripherals on Pi/Pi2.
>
Are there any publically available specs for this? I found the
peripheral manual which documents the interrupts controller but I
couldnt easily find a doc to cover this. I can review without docs,
but it is a little harder. A few top level sentances about how it
works and what it does would help in commit message if we can't get a
doc.
Regards,
Peter
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> ---
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-07 6:36 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Peter Crosthwaite
@ 2015-12-07 17:24 ` Andrew Baumann
0 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-07 17:24 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Sunday, 6 December 2015 22:37
> On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> > This adds the system mailboxes which are used to communicate with a
> > number of GPU peripherals on Pi/Pi2.
> >
>
> Are there any publically available specs for this? I found the
> peripheral manual which documents the interrupts controller but I
> couldnt easily find a doc to cover this. I can review without docs,
> but it is a little harder. A few top level sentances about how it
> works and what it does would help in commit message if we can't get a
> doc.
The only docs I could find are:
https://github.com/raspberrypi/firmware/wiki/Mailboxes
https://github.com/raspberrypi/firmware/wiki/Accessing-mailboxes
It probably doesn't hurt to add these links to either a header comment or commit message. I'll do that in v2.
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-04 6:01 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Andrew Baumann
` (7 preceding siblings ...)
2015-12-07 6:36 ` [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes Peter Crosthwaite
@ 2015-12-21 22:49 ` Peter Crosthwaite
2015-12-21 23:15 ` Andrew Baumann
8 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-21 22:49 UTC (permalink / raw)
To: Andrew Baumann
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> This adds the system mailboxes which are used to communicate with a
> number of GPU peripherals on Pi/Pi2.
>
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> ---
> default-configs/arm-softmmu.mak | 1 +
> hw/misc/Makefile.objs | 1 +
> hw/misc/bcm2835_sbm.c | 280 ++++++++++++++++++++
> include/hw/arm/bcm2835_arm_control.h | 481 +++++++++++++++++++++++++++++++++++
Do we need this file as of this patch?
> include/hw/arm/bcm2835_mbox.h | 19 ++
> include/hw/misc/bcm2835_sbm.h | 37 +++
> 6 files changed, 819 insertions(+)
> create mode 100644 hw/misc/bcm2835_sbm.c
> create mode 100644 include/hw/arm/bcm2835_arm_control.h
> create mode 100644 include/hw/arm/bcm2835_mbox.h
> create mode 100644 include/hw/misc/bcm2835_sbm.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index d9b90a5..a9f82a1 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -79,6 +79,7 @@ CONFIG_TUSB6010=y
> CONFIG_IMX=y
> CONFIG_MAINSTONE=y
> CONFIG_NSERIES=y
> +CONFIG_RASPI=y
> CONFIG_REALVIEW=y
> CONFIG_ZAURUS=y
> CONFIG_ZYNQ=y
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index aeb6b7d..b9379f2 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -34,6 +34,7 @@ obj-$(CONFIG_OMAP) += omap_gpmc.o
> obj-$(CONFIG_OMAP) += omap_l4.o
> obj-$(CONFIG_OMAP) += omap_sdrc.o
> obj-$(CONFIG_OMAP) += omap_tap.o
> +obj-$(CONFIG_RASPI) += bcm2835_sbm.o
> obj-$(CONFIG_SLAVIO) += slavio_misc.o
> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> obj-$(CONFIG_ZYNQ) += zynq-xadc.o
> diff --git a/hw/misc/bcm2835_sbm.c b/hw/misc/bcm2835_sbm.c
> new file mode 100644
> index 0000000..a5c9324
> --- /dev/null
> +++ b/hw/misc/bcm2835_sbm.c
> @@ -0,0 +1,280 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +#include "hw/misc/bcm2835_sbm.h"
> +#include "hw/arm/bcm2835_arm_control.h"
> +
> +static void mbox_update_status(BCM2835Mbox *mb)
> +{
> + if (mb->count == 0) {
> + mb->status |= ARM_MS_EMPTY;
> + } else {
> + mb->status &= ~ARM_MS_EMPTY;
> + }
> + if (mb->count == MBOX_SIZE) {
> + mb->status |= ARM_MS_FULL;
> + } else {
> + mb->status &= ~ARM_MS_FULL;
> + }
The start-from-scratch approach makes this shorter:
mb->status &= ~(ARM_MS_EMPTY | ARM_MS_FULL);
if (mb->count == 0) {
mb->status |= ARM_MS_EMPTY;
}
if (mb->count == MBOX_SIZE) {
mb->status |= ARM_MS_FULL;
}
> +}
> +
> +static void mbox_init(BCM2835Mbox *mb)
This looks like a reset function.
> +{
> + int n;
Blank line here.
> + mb->count = 0;
> + mb->config = 0;
> + for (n = 0; n < MBOX_SIZE; n++) {
> + mb->reg[n] = MBOX_INVALID_DATA;
> + }
> + mbox_update_status(mb);
> +}
> +
> +static uint32_t mbox_pull(BCM2835Mbox *mb, int index)
> +{
> + int n;
> + uint32_t val;
> +
> + assert(mb->count > 0);
> + assert(index < mb->count);
> +
> + val = mb->reg[index];
> + for (n = index + 1; n < mb->count; n++) {
> + mb->reg[n - 1] = mb->reg[n];
> + }
> + mb->count--;
> + mb->reg[mb->count] = MBOX_INVALID_DATA;
> +
> + mbox_update_status(mb);
> +
> + return val;
> +}
> +
> +static void mbox_push(BCM2835Mbox *mb, uint32_t val)
> +{
> + assert(mb->count < MBOX_SIZE);
> + mb->reg[mb->count++] = val;
> + mbox_update_status(mb);
> +}
> +
> +static void bcm2835_sbm_update(BCM2835SbmState *s)
What does Sbm stand for? If it is acronym is should be all caps in camel case.
> +{
> + int set;
bool.
> + int done, n;
> + uint32_t value;
> +
> + /* Avoid unwanted recursive calls */
> + s->mbox_irq_disabled = 1;
> +
> + /* Get pending responses and put them in the vc->arm mbox */
> + done = 0;
> + while (!done) {
> + done = 1;
> + if (s->mbox[0].status & ARM_MS_FULL) {
> + /* vc->arm mbox full, exit */
break here.
> + } else {
so you can drop the else and get back a level of indent.
> + for (n = 0; n < MBOX_CHAN_COUNT; n++) {
> + if (s->available[n]) {
> + value = ldl_phys(&s->mbox_as, n<<4);
> + if (value != MBOX_INVALID_DATA) {
> + mbox_push(&s->mbox[0], value);
> + } else {
> + /* Hmmm... */
Needs more comment :)
> + }
> + done = 0;
> + break;
> + }
> + }
> + }
> + }
> +
> + /* Try to push pending requests from the arm->vc mbox */
> + /* TODO (?) */
> +
> + /* Re-enable calls from the IRQ routine */
> + s->mbox_irq_disabled = 0;
> +
> + /* Update ARM IRQ status */
> + set = 0;
> + if (s->mbox[0].status & ARM_MS_EMPTY) {
> + s->mbox[0].config &= ~ARM_MC_IHAVEDATAIRQPEND;
This can also be reduced to just if (no else), by always clearing
ARM_MC_IHAVEDATAIRQPEND. This makes it consistent with the way you
handle set.
> + } else {
> + s->mbox[0].config |= ARM_MC_IHAVEDATAIRQPEND;
> + if (s->mbox[0].config & ARM_MC_IHAVEDATAIRQEN) {
> + set = 1;
> + }
> + }
> + qemu_set_irq(s->arm_irq, set);
> +}
> +
> +static void bcm2835_sbm_set_irq(void *opaque, int irq, int level)
> +{
> + BCM2835SbmState *s = (BCM2835SbmState *)opaque;
Blank line. No cast needed.
> + s->available[irq] = level;
> + if (!s->mbox_irq_disabled) {
I don't think you need this. IO devices are not thread safe and
core-code knows it. So you don't need to worry about interruption and
re-entry of your IRQ handler.
> + bcm2835_sbm_update(s);
> + }
> +}
> +
> +static uint64_t bcm2835_sbm_read(void *opaque, hwaddr offset,
> + unsigned size)
> +{
> + BCM2835SbmState *s = (BCM2835SbmState *)opaque;
cast.
> + uint32_t res = 0;
> +
> + offset &= 0xff;
> +
> + switch (offset) {
> + case 0x80: /* MAIL0_READ */
> + case 0x84:
> + case 0x88:
> + case 0x8c:
case 0x80..0x8c
> + if (s->mbox[0].status & ARM_MS_EMPTY) {
> + res = MBOX_INVALID_DATA;
> + } else {
> + res = mbox_pull(&s->mbox[0], 0);
> + }
> + break;
> + case 0x90: /* MAIL0_PEEK */
> + res = s->mbox[0].reg[0];
> + break;
> + case 0x94: /* MAIL0_SENDER */
> + break;
> + case 0x98: /* MAIL0_STATUS */
> + res = s->mbox[0].status;
> + break;
> + case 0x9c: /* MAIL0_CONFIG */
> + res = s->mbox[0].config;
> + break;
> + case 0xb8: /* MAIL1_STATUS */
> + res = s->mbox[1].status;
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "bcm2835_sbm_read: Bad offset %x\n", (int)offset);
> + return 0;
> + }
> +
> + bcm2835_sbm_update(s);
> +
> + return res;
> +}
> +
> +static void bcm2835_sbm_write(void *opaque, hwaddr offset,
> + uint64_t value, unsigned size)
> +{
> + int ch;
> +
> + BCM2835SbmState *s = (BCM2835SbmState *)opaque;
> +
> + offset &= 0xff;
> +
> + switch (offset) {
> + case 0x94: /* MAIL0_SENDER */
> + break;
> + case 0x9c: /* MAIL0_CONFIG */
> + s->mbox[0].config &= ~ARM_MC_IHAVEDATAIRQEN;
> + s->mbox[0].config |= value & ARM_MC_IHAVEDATAIRQEN;
> + break;
> + case 0xa0:
> + case 0xa4:
> + case 0xa8:
> + case 0xac:
case ...
> + if (s->mbox[1].status & ARM_MS_FULL) {
> + /* Guest error */
LOG_GUEST_ERROR.
> + } else {
> + ch = value & 0xf;
> + if (ch < MBOX_CHAN_COUNT) {
> + if (ldl_phys(&s->mbox_as, (ch<<4) + 4)) {
What is significance of memory offset +4? Can you have some macros for
this for some hints as to what the functionality is?
> + /* Push delayed, push it in the arm->vc mbox */
> + mbox_push(&s->mbox[1], value);
> + } else {
> + stl_phys(&s->mbox_as, ch<<4, value);
> + }
> + } else {
> + /* Invalid channel number */
LOG_GUEST_ERROR.
> + }
> + }
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "bcm2835_sbm_write: Bad offset %x\n", (int)offset);
> + return;
> + }
> +
> + bcm2835_sbm_update(s);
> +}
> +
> +static const MemoryRegionOps bcm2835_sbm_ops = {
> + .read = bcm2835_sbm_read,
> + .write = bcm2835_sbm_write,
Add access restrictions.
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const VMStateDescription vmstate_bcm2835_sbm = {
> + .name = TYPE_BCM2835_SBM,
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
Is the state missing?
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void bcm2835_sbm_init(Object *obj)
> +{
> + BCM2835SbmState *s = BCM2835_SBM(obj);
> + memory_region_init_io(&s->iomem, obj, &bcm2835_sbm_ops, s,
> + TYPE_BCM2835_SBM, 0x400);
> + sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
> + sysbus_init_irq(SYS_BUS_DEVICE(s), &s->arm_irq);
> + qdev_init_gpio_in(DEVICE(s), bcm2835_sbm_set_irq, MBOX_CHAN_COUNT);
> +}
> +
> +static void bcm2835_sbm_realize(DeviceState *dev, Error **errp)
> +{
> + int n;
> + BCM2835SbmState *s = BCM2835_SBM(dev);
> + Object *obj;
> + Error *err = NULL;
> +
> + obj = object_property_get_link(OBJECT(dev), "mbox_mr", &err);
> + if (err || obj == NULL) {
> + error_setg(errp, "bcm2835_sbm: required mbox_mr link not found");
Propagate the original error along with this new mesage. %s with
error_get_pretty(err) can do this.
> + return;
> + }
> +
> + s->mbox_mr = MEMORY_REGION(obj);
> + address_space_init(&s->mbox_as, s->mbox_mr, NULL);
> +
> + mbox_init(&s->mbox[0]);
> + mbox_init(&s->mbox[1]);
> + s->mbox_irq_disabled = 0;
> + for (n = 0; n < MBOX_CHAN_COUNT; n++) {
> + s->available[n] = 0;
> + }
This looks like reset behaviour, and should be moved into dc->reset callback.
> +}
> +
> +static void bcm2835_sbm_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = bcm2835_sbm_realize;
> + dc->vmsd = &vmstate_bcm2835_sbm;
> +}
> +
> +static TypeInfo bcm2835_sbm_info = {
> + .name = TYPE_BCM2835_SBM,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(BCM2835SbmState),
> + .class_init = bcm2835_sbm_class_init,
> + .instance_init = bcm2835_sbm_init,
> +};
> +
> +static void bcm2835_sbm_register_types(void)
> +{
> + type_register_static(&bcm2835_sbm_info);
> +}
> +
> +type_init(bcm2835_sbm_register_types)
> diff --git a/include/hw/arm/bcm2835_arm_control.h b/include/hw/arm/bcm2835_arm_control.h
> new file mode 100644
> index 0000000..28acba8
> --- /dev/null
> +++ b/include/hw/arm/bcm2835_arm_control.h
> @@ -0,0 +1,481 @@
> +/*
> + * linux/arch/arm/mach-bcm2708/arm_control.h
> + *
> + * Copyright (C) 2010 Broadcom
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#ifndef BCM2708_ARM_CONTROL_H_
> +#define BCM2708_ARM_CONTROL_H_
> +
> +/*
> + * Definitions and addresses for the ARM CONTROL logic
> + * This file is manually generated.
> + */
> +
> +#define ARM_BASE 0x7E00B000
> +
> +/* Basic configuration */
> +#define ARM_CONTROL0 HW_REGISTER_RW(ARM_BASE+0x000)
> +#define ARM_C0_SIZ128M 0x00000000
> +#define ARM_C0_SIZ256M 0x00000001
> +#define ARM_C0_SIZ512M 0x00000002
> +#define ARM_C0_SIZ1G 0x00000003
> +#define ARM_C0_BRESP0 0x00000000
> +#define ARM_C0_BRESP1 0x00000004
> +#define ARM_C0_BRESP2 0x00000008
> +#define ARM_C0_BOOTHI 0x00000010
> +#define ARM_C0_UNUSED05 0x00000020 /* free */
> +#define ARM_C0_FULLPERI 0x00000040
> +#define ARM_C0_UNUSED78 0x00000180 /* free */
> +#define ARM_C0_JTAGMASK 0x00000E00
> +#define ARM_C0_JTAGOFF 0x00000000
> +#define ARM_C0_JTAGBASH 0x00000800 /* Debug on GPIO off */
> +#define ARM_C0_JTAGGPIO 0x00000C00 /* Debug on GPIO on */
> +#define ARM_C0_APROTMSK 0x0000F000
> +#define ARM_C0_DBG0SYNC 0x00010000 /* VPU0 halt sync */
> +#define ARM_C0_DBG1SYNC 0x00020000 /* VPU1 halt sync */
> +#define ARM_C0_SWDBGREQ 0x00040000 /* HW debug request */
> +#define ARM_C0_PASSHALT 0x00080000 /* ARM halt passed to debugger */
> +#define ARM_C0_PRIO_PER 0x00F00000 /* per priority mask */
> +#define ARM_C0_PRIO_L2 0x0F000000
> +#define ARM_C0_PRIO_UC 0xF0000000
> +
> +#define ARM_C0_APROTPASS 0x0000A000 /* Translate 1:1 */
> +#define ARM_C0_APROTUSER 0x00000000 /* Only user mode */
> +#define ARM_C0_APROTSYST 0x0000F000 /* Only system mode */
> +
> +
> +#define ARM_CONTROL1 HW_REGISTER_RW(ARM_BASE+0x440)
> +#define ARM_C1_TIMER 0x00000001 /* re-route timer IRQ to VC */
> +#define ARM_C1_MAIL 0x00000002 /* re-route Mail IRQ to VC */
> +#define ARM_C1_BELL0 0x00000004 /* re-route Doorbell 0 to VC */
> +#define ARM_C1_BELL1 0x00000008 /* re-route Doorbell 1 to VC */
> +#define ARM_C1_PERSON 0x00000100 /* peripherals on */
> +#define ARM_C1_REQSTOP 0x00000200 /* ASYNC bridge request stop */
> +
> +#define ARM_STATUS HW_REGISTER_RW(ARM_BASE+0x444)
> +#define ARM_S_ACKSTOP 0x80000000 /* Bridge stopped */
> +#define ARM_S_READPEND 0x000003FF /* pending reads counter */
> +#define ARM_S_WRITPEND 0x000FFC00 /* pending writes counter */
> +
> +#define ARM_ERRHALT HW_REGISTER_RW(ARM_BASE+0x448)
> +#define ARM_EH_PERIBURST 0x00000001 /* Burst write seen on peri bus */
> +#define ARM_EH_ILLADDRS1 0x00000002 /* Address bits 25-27 error */
> +#define ARM_EH_ILLADDRS2 0x00000004 /* Address bits 31-28 error */
> +#define ARM_EH_VPU0HALT 0x00000008 /* VPU0 halted & in debug mode */
> +#define ARM_EH_VPU1HALT 0x00000010 /* VPU1 halted & in debug mode */
> +#define ARM_EH_ARMHALT 0x00000020 /* ARM in halted debug mode */
> +
> +#define ARM_ID_SECURE HW_REGISTER_RW(ARM_BASE+0x00C)
> +#define ARM_ID HW_REGISTER_RW(ARM_BASE+0x44C)
> +#define ARM_IDVAL 0x364D5241
> +
> +/* Translation memory */
> +#define ARM_TRANSLATE HW_REGISTER_RW(ARM_BASE+0x100)
> +/* 32 locations: 0x100.. 0x17F */
> +/* 32 spare means we CAN go to 64 pages.... */
> +
> +
> +/* Interrupts */
> +#define ARM_IRQ_PEND0 HW_REGISTER_RW(ARM_BASE+0x200) /* Top IRQ bits */
> +#define ARM_I0_TIMER 0x00000001 /* timer IRQ */
> +#define ARM_I0_MAIL 0x00000002 /* Mail IRQ */
> +#define ARM_I0_BELL0 0x00000004 /* Doorbell 0 */
> +#define ARM_I0_BELL1 0x00000008 /* Doorbell 1 */
> +#define ARM_I0_BANK1 0x00000100 /* Bank1 IRQ */
> +#define ARM_I0_BANK2 0x00000200 /* Bank2 IRQ */
> +
> +#define ARM_IRQ_PEND1 HW_REGISTER_RW(ARM_BASE+0x204) /* All bank1 IRQ bits */
> +/* todo: all I1_interrupt sources */
> +#define ARM_IRQ_PEND2 HW_REGISTER_RW(ARM_BASE+0x208) /* All bank2 IRQ bits */
> +/* todo: all I2_interrupt sources */
> +
> +#define ARM_IRQ_FAST HW_REGISTER_RW(ARM_BASE+0x20C) /* FIQ control */
> +#define ARM_IF_INDEX 0x0000007F /* FIQ select */
> +#define ARM_IF_ENABLE 0x00000080 /* FIQ enable */
> +#define ARM_IF_VCMASK 0x0000003F /* FIQ = (index from VC source) */
> +#define ARM_IF_TIMER 0x00000040 /* FIQ = ARM timer */
> +#define ARM_IF_MAIL 0x00000041 /* FIQ = ARM Mail */
> +#define ARM_IF_BELL0 0x00000042 /* FIQ = ARM Doorbell 0 */
> +#define ARM_IF_BELL1 0x00000043 /* FIQ = ARM Doorbell 1 */
> +#define ARM_IF_VP0HALT 0x00000044 /* FIQ = VPU0 Halt seen */
> +#define ARM_IF_VP1HALT 0x00000045 /* FIQ = VPU1 Halt seen */
> +#define ARM_IF_ILLEGAL 0x00000046 /* FIQ = Illegal access seen */
> +
> +#define ARM_IRQ_ENBL1 HW_REGISTER_RW(ARM_BASE+0x210) /* Bank1 enable bits */
> +#define ARM_IRQ_ENBL2 HW_REGISTER_RW(ARM_BASE+0x214) /* Bank2 enable bits */
> +#define ARM_IRQ_ENBL3 HW_REGISTER_RW(ARM_BASE+0x218) /* ARM irqs enable bits */
> +#define ARM_IRQ_DIBL1 HW_REGISTER_RW(ARM_BASE+0x21C) /* Bank1 disable bits */
> +#define ARM_IRQ_DIBL2 HW_REGISTER_RW(ARM_BASE+0x220) /* Bank2 disable bits */
> +#define ARM_IRQ_DIBL3 HW_REGISTER_RW(ARM_BASE+0x224) /* ARM irqs disable bits */
> +#define ARM_IE_TIMER 0x00000001 /* Timer IRQ */
> +#define ARM_IE_MAIL 0x00000002 /* Mail IRQ */
> +#define ARM_IE_BELL0 0x00000004 /* Doorbell 0 */
> +#define ARM_IE_BELL1 0x00000008 /* Doorbell 1 */
> +#define ARM_IE_VP0HALT 0x00000010 /* VPU0 Halt */
> +#define ARM_IE_VP1HALT 0x00000020 /* VPU1 Halt */
> +#define ARM_IE_ILLEGAL 0x00000040 /* Illegal access seen */
> +
> +/* Timer */
> +/* For reg. fields see sp804 spec. */
> +#define ARM_T_LOAD HW_REGISTER_RW(ARM_BASE+0x400)
> +#define ARM_T_VALUE HW_REGISTER_RW(ARM_BASE+0x404)
> +#define ARM_T_CONTROL HW_REGISTER_RW(ARM_BASE+0x408)
> +#define ARM_T_IRQCNTL HW_REGISTER_RW(ARM_BASE+0x40C)
> +#define ARM_T_RAWIRQ HW_REGISTER_RW(ARM_BASE+0x410)
> +#define ARM_T_MSKIRQ HW_REGISTER_RW(ARM_BASE+0x414)
> +#define ARM_T_RELOAD HW_REGISTER_RW(ARM_BASE+0x418)
> +#define ARM_T_PREDIV HW_REGISTER_RW(ARM_BASE+0x41c)
> +#define ARM_T_FREECNT HW_REGISTER_RW(ARM_BASE+0x420)
> +
> +#define TIMER_CTRL_ONESHOT (1 << 0)
> +#define TIMER_CTRL_32BIT (1 << 1)
> +#define TIMER_CTRL_DIV1 (0 << 2)
> +#define TIMER_CTRL_DIV16 (1 << 2)
> +#define TIMER_CTRL_DIV256 (2 << 2)
> +#define TIMER_CTRL_IE (1 << 5)
> +#define TIMER_CTRL_PERIODIC (1 << 6)
> +#define TIMER_CTRL_ENABLE (1 << 7)
> +#define TIMER_CTRL_DBGHALT (1 << 8)
> +#define TIMER_CTRL_ENAFREE (1 << 9)
> +#define TIMER_CTRL_FREEDIV_SHIFT 16)
> +#define TIMER_CTRL_FREEDIV_MASK 0xff
> +
> +/* Semaphores, Doorbells, Mailboxes */
> +#define ARM_SBM_OWN0 (ARM_BASE+0x800)
> +#define ARM_SBM_OWN1 (ARM_BASE+0x900)
> +#define ARM_SBM_OWN2 (ARM_BASE+0xA00)
> +#define ARM_SBM_OWN3 (ARM_BASE+0xB00)
> +
> +/* MAILBOXES
> + * Register flags are common across all
> + * owner registers. See end of this section
> + *
> + * Semaphores, Doorbells, Mailboxes Owner 0
> + *
> + */
> +
> +#define ARM_0_SEMS HW_REGISTER_RW(ARM_SBM_OWN0+0x00)
> +#define ARM_0_SEM0 HW_REGISTER_RW(ARM_SBM_OWN0+0x00)
> +#define ARM_0_SEM1 HW_REGISTER_RW(ARM_SBM_OWN0+0x04)
> +#define ARM_0_SEM2 HW_REGISTER_RW(ARM_SBM_OWN0+0x08)
> +#define ARM_0_SEM3 HW_REGISTER_RW(ARM_SBM_OWN0+0x0C)
> +#define ARM_0_SEM4 HW_REGISTER_RW(ARM_SBM_OWN0+0x10)
> +#define ARM_0_SEM5 HW_REGISTER_RW(ARM_SBM_OWN0+0x14)
> +#define ARM_0_SEM6 HW_REGISTER_RW(ARM_SBM_OWN0+0x18)
> +#define ARM_0_SEM7 HW_REGISTER_RW(ARM_SBM_OWN0+0x1C)
When you have a regular structure like this, you should collapse it
down using some arithmatic:
#define ARM_0_SEM(x) HW_REGISTER_RW(ARM_SBM_OWN0 + 0x4 * (x))
> +#define ARM_0_BELL0 HW_REGISTER_RW(ARM_SBM_OWN0+0x40)
> +#define ARM_0_BELL1 HW_REGISTER_RW(ARM_SBM_OWN0+0x44)
> +#define ARM_0_BELL2 HW_REGISTER_RW(ARM_SBM_OWN0+0x48)
> +#define ARM_0_BELL3 HW_REGISTER_RW(ARM_SBM_OWN0+0x4C)
Here as well.
> +/* MAILBOX 0 access in Owner 0 area */
> +/* Some addresses should ONLY be used by owner 0 */
> +#define ARM_0_MAIL0_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0x80) /* .. 0x8C (4 locations) */
> +#define ARM_0_MAIL0_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0x80) /* .. 0x8C (4 locs) Normal read */
> +#define ARM_0_MAIL0_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0x90) /* none-pop read */
> +#define ARM_0_MAIL0_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0x94) /* Sender read (only LS 2 bits) */
> +#define ARM_0_MAIL0_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0x98) /* Status read */
> +#define ARM_0_MAIL0_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0x9C) /* Config read/write */
> +/* MAILBOX 1 access in Owner 0 area */
> +/* Owner 0 should only WRITE to this mailbox */
> +#define ARM_0_MAIL1_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xA0) /* .. 0xAC (4 locations) */
> +/*#define ARM_0_MAIL1_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xA0) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_0_MAIL1_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xB0) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_0_MAIL1_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xB4) */ /* DO NOT USE THIS !!!!! */
> +#define ARM_0_MAIL1_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xB8) /* Status read */
> +/*#define ARM_0_MAIL1_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xBC) */ /* DO NOT USE THIS !!!!! */
> +/* General SEM, BELL, MAIL config/status */
> +#define ARM_0_SEMCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xE0) /* semaphore clear/debug register */
> +#define ARM_0_BELLCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xE4) /* Doorbells clear/debug register */
> +#define ARM_0_ALL_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xF8) /* ALL interrupts */
> +#define ARM_0_MY_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN0+0xFC) /* IRQS pending for owner 0 */
> +
> +/* Semaphores, Doorbells, Mailboxes Owner 1 */
> +#define ARM_1_SEMS HW_REGISTER_RW(ARM_SBM_OWN1+0x00)
> +#define ARM_1_SEM0 HW_REGISTER_RW(ARM_SBM_OWN1+0x00)
> +#define ARM_1_SEM1 HW_REGISTER_RW(ARM_SBM_OWN1+0x04)
> +#define ARM_1_SEM2 HW_REGISTER_RW(ARM_SBM_OWN1+0x08)
> +#define ARM_1_SEM3 HW_REGISTER_RW(ARM_SBM_OWN1+0x0C)
> +#define ARM_1_SEM4 HW_REGISTER_RW(ARM_SBM_OWN1+0x10)
> +#define ARM_1_SEM5 HW_REGISTER_RW(ARM_SBM_OWN1+0x14)
> +#define ARM_1_SEM6 HW_REGISTER_RW(ARM_SBM_OWN1+0x18)
> +#define ARM_1_SEM7 HW_REGISTER_RW(ARM_SBM_OWN1+0x1C)
> +#define ARM_1_BELL0 HW_REGISTER_RW(ARM_SBM_OWN1+0x40)
> +#define ARM_1_BELL1 HW_REGISTER_RW(ARM_SBM_OWN1+0x44)
> +#define ARM_1_BELL2 HW_REGISTER_RW(ARM_SBM_OWN1+0x48)
> +#define ARM_1_BELL3 HW_REGISTER_RW(ARM_SBM_OWN1+0x4C)
You have large repeitition between ARM_0 and ARM_1, should also be
collapased using Macros args. It is however generally rare to need to
have the individual defs for each window of a regular structure, as
the switch case logic that implements the functionality, is also
usually regular and just uses math on addresses to figure out what to
index, rather than fully defining the address map.
> +/* MAILBOX 0 access in Owner 0 area */
> +/* Owner 1 should only WRITE to this mailbox */
> +#define ARM_1_MAIL0_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0x80) /* .. 0x8C (4 locations) */
> +/*#define ARM_1_MAIL0_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0x80) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_1_MAIL0_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0x90) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_1_MAIL0_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0x94) */ /* DO NOT USE THIS !!!!! */
> +#define ARM_1_MAIL0_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0x98) /* Status read */
> +/*#define ARM_1_MAIL0_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0x9C) */ /* DO NOT USE THIS !!!!! */
> +/* MAILBOX 1 access in Owner 0 area */
> +#define ARM_1_MAIL1_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xA0) /* .. 0xAC (4 locations) */
> +#define ARM_1_MAIL1_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xA0) /* .. 0xAC (4 locs) Normal read */
> +#define ARM_1_MAIL1_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xB0) /* none-pop read */
> +#define ARM_1_MAIL1_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xB4) /* Sender read (only LS 2 bits) */
> +#define ARM_1_MAIL1_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xB8) /* Status read */
> +#define ARM_1_MAIL1_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xBC)
> +/* General SEM, BELL, MAIL config/status */
> +#define ARM_1_SEMCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xE0) /* semaphore clear/debug register */
> +#define ARM_1_BELLCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xE4) /* Doorbells clear/debug register */
> +#define ARM_1_MY_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xFC) /* IRQS pending for owner 1 */
> +#define ARM_1_ALL_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN1+0xF8) /* ALL interrupts */
> +
> +/* Semaphores, Doorbells, Mailboxes Owner 2 */
> +#define ARM_2_SEMS HW_REGISTER_RW(ARM_SBM_OWN2+0x00)
> +#define ARM_2_SEM0 HW_REGISTER_RW(ARM_SBM_OWN2+0x00)
> +#define ARM_2_SEM1 HW_REGISTER_RW(ARM_SBM_OWN2+0x04)
> +#define ARM_2_SEM2 HW_REGISTER_RW(ARM_SBM_OWN2+0x08)
> +#define ARM_2_SEM3 HW_REGISTER_RW(ARM_SBM_OWN2+0x0C)
> +#define ARM_2_SEM4 HW_REGISTER_RW(ARM_SBM_OWN2+0x10)
> +#define ARM_2_SEM5 HW_REGISTER_RW(ARM_SBM_OWN2+0x14)
> +#define ARM_2_SEM6 HW_REGISTER_RW(ARM_SBM_OWN2+0x18)
> +#define ARM_2_SEM7 HW_REGISTER_RW(ARM_SBM_OWN2+0x1C)
> +#define ARM_2_BELL0 HW_REGISTER_RW(ARM_SBM_OWN2+0x40)
> +#define ARM_2_BELL1 HW_REGISTER_RW(ARM_SBM_OWN2+0x44)
> +#define ARM_2_BELL2 HW_REGISTER_RW(ARM_SBM_OWN2+0x48)
> +#define ARM_2_BELL3 HW_REGISTER_RW(ARM_SBM_OWN2+0x4C)
> +/* MAILBOX 0 access in Owner 2 area */
> +/* Owner 2 should only WRITE to this mailbox */
> +#define ARM_2_MAIL0_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0x80) /* .. 0x8C (4 locations) */
> +/*#define ARM_2_MAIL0_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0x80) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_2_MAIL0_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0x90) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_2_MAIL0_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0x94) */ /* DO NOT USE THIS !!!!! */
> +#define ARM_2_MAIL0_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0x98) /* Status read */
> +/*#define ARM_2_MAIL0_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0x9C) */ /* DO NOT USE THIS !!!!! */
> +/* MAILBOX 1 access in Owner 2 area */
> +/* Owner 2 should only WRITE to this mailbox */
> +#define ARM_2_MAIL1_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xA0) /* .. 0xAC (4 locations) */
> +/*#define ARM_2_MAIL1_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xA0) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_2_MAIL1_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xB0) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_2_MAIL1_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xB4) */ /* DO NOT USE THIS !!!!! */
> +#define ARM_2_MAIL1_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xB8) /* Status read */
> +/*#define ARM_2_MAIL1_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xBC) */ /* DO NOT USE THIS !!!!! */
> +/* General SEM, BELL, MAIL config/status */
> +#define ARM_2_SEMCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xE0) /* semaphore clear/debug register */
> +#define ARM_2_BELLCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xE4) /* Doorbells clear/debug register */
> +#define ARM_2_MY_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xFC) /* IRQS pending for owner 2 */
> +#define ARM_2_ALL_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN2+0xF8) /* ALL interrupts */
> +
> +/* Semaphores, Doorbells, Mailboxes Owner 3 */
> +#define ARM_3_SEMS HW_REGISTER_RW(ARM_SBM_OWN3+0x00)
> +#define ARM_3_SEM0 HW_REGISTER_RW(ARM_SBM_OWN3+0x00)
> +#define ARM_3_SEM1 HW_REGISTER_RW(ARM_SBM_OWN3+0x04)
> +#define ARM_3_SEM2 HW_REGISTER_RW(ARM_SBM_OWN3+0x08)
> +#define ARM_3_SEM3 HW_REGISTER_RW(ARM_SBM_OWN3+0x0C)
> +#define ARM_3_SEM4 HW_REGISTER_RW(ARM_SBM_OWN3+0x10)
> +#define ARM_3_SEM5 HW_REGISTER_RW(ARM_SBM_OWN3+0x14)
> +#define ARM_3_SEM6 HW_REGISTER_RW(ARM_SBM_OWN3+0x18)
> +#define ARM_3_SEM7 HW_REGISTER_RW(ARM_SBM_OWN3+0x1C)
> +#define ARM_3_BELL0 HW_REGISTER_RW(ARM_SBM_OWN3+0x40)
> +#define ARM_3_BELL1 HW_REGISTER_RW(ARM_SBM_OWN3+0x44)
> +#define ARM_3_BELL2 HW_REGISTER_RW(ARM_SBM_OWN3+0x48)
> +#define ARM_3_BELL3 HW_REGISTER_RW(ARM_SBM_OWN3+0x4C)
> +/* MAILBOX 0 access in Owner 3 area */
> +/* Owner 3 should only WRITE to this mailbox */
> +#define ARM_3_MAIL0_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0x80) /* .. 0x8C (4 locations) */
> +/*#define ARM_3_MAIL0_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0x80) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_3_MAIL0_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0x90) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_3_MAIL0_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0x94) */ /* DO NOT USE THIS !!!!! */
> +#define ARM_3_MAIL0_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0x98) /* Status read */
> +/*#define ARM_3_MAIL0_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0x9C) */ /* DO NOT USE THIS !!!!! */
> +/* MAILBOX 1 access in Owner 3 area */
> +/* Owner 3 should only WRITE to this mailbox */
> +#define ARM_3_MAIL1_WRT \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xA0) /* .. 0xAC (4 locations) */
> +/*#define ARM_3_MAIL1_RD \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xA0) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_3_MAIL1_POL \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xB0) */ /* DO NOT USE THIS !!!!! */
> +/*#define ARM_3_MAIL1_SND \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xB4) */ /* DO NOT USE THIS !!!!! */
> +#define ARM_3_MAIL1_STA \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xB8) /* Status read */
> +/*#define ARM_3_MAIL1_CNF \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xBC) */ /* DO NOT USE THIS !!!!! */
> +/* General SEM, BELL, MAIL config/status */
> +#define ARM_3_SEMCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xE0) /* semaphore clear/debug register */
> +#define ARM_3_BELLCLRDBG \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xE4) /* Doorbells clear/debug register */
> +#define ARM_3_MY_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xFC) /* IRQS pending for owner 3 */
> +#define ARM_3_ALL_IRQS \
> + HW_REGISTER_RW(ARM_SBM_OWN3+0xF8) /* ALL interrupts */
> +
> +/* Mailbox flags. Valid for all owners */
> +
> +/* Mailbox status register (...0x98) */
> +#define ARM_MS_FULL 0x80000000
> +#define ARM_MS_EMPTY 0x40000000
> +#define ARM_MS_LEVEL 0x400000FF /* Max. value depends on mailbox depth */
> +
> +/* MAILBOX config/status register (...0x9C) */
> +/* ANY write to this register clears the error bits! */
> +#define ARM_MC_IHAVEDATAIRQEN 0x00000001 /* mbox irq enable: has data */
> +#define ARM_MC_IHAVESPACEIRQEN 0x00000002 /* mbox irq enable: has space */
> +#define ARM_MC_OPPISEMPTYIRQEN 0x00000004 /* mbox irq enable: Opp is empty */
> +#define ARM_MC_MAIL_CLEAR 0x00000008 /* mbox clear write 1, then 0 */
> +#define ARM_MC_IHAVEDATAIRQPEND 0x00000010 /* mbox irq pending: has space */
> +#define ARM_MC_IHAVESPACEIRQPEND 0x00000020 /* mbox irq pending: Opp is empty */
> +#define ARM_MC_OPPISEMPTYIRQPEND 0x00000040 /* mbox irq pending */
> +/* Bit 7 is unused */
> +#define ARM_MC_ERRNOOWN 0x00000100 /* error : none owner read from mailbox */
> +#define ARM_MC_ERROVERFLW 0x00000200 /* error : write to fill mailbox */
> +#define ARM_MC_ERRUNDRFLW 0x00000400 /* error : read from empty mailbox */
> +
> +/* Semaphore clear/debug register (...0xE0) */
> +#define ARM_SD_OWN0 0x00000003 /* Owner of sem 0 */
> +#define ARM_SD_OWN1 0x0000000C /* Owner of sem 1 */
> +#define ARM_SD_OWN2 0x00000030 /* Owner of sem 2 */
> +#define ARM_SD_OWN3 0x000000C0 /* Owner of sem 3 */
> +#define ARM_SD_OWN4 0x00000300 /* Owner of sem 4 */
> +#define ARM_SD_OWN5 0x00000C00 /* Owner of sem 5 */
> +#define ARM_SD_OWN6 0x00003000 /* Owner of sem 6 */
> +#define ARM_SD_OWN7 0x0000C000 /* Owner of sem 7 */
Regular structure.
> +#define ARM_SD_SEM0 0x00010000 /* Status of sem 0 */
> +#define ARM_SD_SEM1 0x00020000 /* Status of sem 1 */
> +#define ARM_SD_SEM2 0x00040000 /* Status of sem 2 */
> +#define ARM_SD_SEM3 0x00080000 /* Status of sem 3 */
> +#define ARM_SD_SEM4 0x00100000 /* Status of sem 4 */
> +#define ARM_SD_SEM5 0x00200000 /* Status of sem 5 */
> +#define ARM_SD_SEM6 0x00400000 /* Status of sem 6 */
> +#define ARM_SD_SEM7 0x00800000 /* Status of sem 7 */
> +
Regular structure. More below.
> +/* Doorbells clear/debug register (...0xE4) */
> +#define ARM_BD_OWN0 0x00000003 /* Owner of doorbell 0 */
> +#define ARM_BD_OWN1 0x0000000C /* Owner of doorbell 1 */
> +#define ARM_BD_OWN2 0x00000030 /* Owner of doorbell 2 */
> +#define ARM_BD_OWN3 0x000000C0 /* Owner of doorbell 3 */
> +#define ARM_BD_BELL0 0x00000100 /* Status of doorbell 0 */
> +#define ARM_BD_BELL1 0x00000200 /* Status of doorbell 1 */
> +#define ARM_BD_BELL2 0x00000400 /* Status of doorbell 2 */
> +#define ARM_BD_BELL3 0x00000800 /* Status of doorbell 3 */
> +
> +/* MY IRQS register (...0xF8) */
> +#define ARM_MYIRQ_BELL 0x00000001 /* This owner has a doorbell IRQ */
> +#define ARM_MYIRQ_MAIL 0x00000002 /* This owner has a mailbox IRQ */
> +
> +/* ALL IRQS register (...0xF8) */
> +#define ARM_AIS_BELL0 0x00000001 /* Doorbell 0 IRQ pending */
> +#define ARM_AIS_BELL1 0x00000002 /* Doorbell 1 IRQ pending */
> +#define ARM_AIS_BELL2 0x00000004 /* Doorbell 2 IRQ pending */
> +#define ARM_AIS_BELL3 0x00000008 /* Doorbell 3 IRQ pending */
> +#define ARM_AIS0_HAVEDATA 0x00000010 /* MAIL 0 has data IRQ pending */
> +#define ARM_AIS0_HAVESPAC 0x00000020 /* MAIL 0 has space IRQ pending */
> +#define ARM_AIS0_OPPEMPTY 0x00000040 /* MAIL 0 opposite is empty IRQ */
> +#define ARM_AIS1_HAVEDATA 0x00000080 /* MAIL 1 has data IRQ pending */
> +#define ARM_AIS1_HAVESPAC 0x00000100 /* MAIL 1 has space IRQ pending */
> +#define ARM_AIS1_OPPEMPTY 0x00000200 /* MAIL 1 opposite is empty IRQ */
> +/* Note that bell-0, bell-1 and MAIL0 IRQ go only to the ARM */
> +/* Whilst that bell-2, bell-3 and MAIL1 IRQ go only to the VC */
> +/* */
> +/* ARM JTAG BASH */
> +/* */
> +#define AJB_BASE 0x7e2000c0
> +
> +#define AJBCONF HW_REGISTER_RW(AJB_BASE+0x00)
> +#define AJB_BITS0 0x000000
> +#define AJB_BITS4 0x000004
> +#define AJB_BITS8 0x000008
> +#define AJB_BITS12 0x00000C
> +#define AJB_BITS16 0x000010
> +#define AJB_BITS20 0x000014
> +#define AJB_BITS24 0x000018
> +#define AJB_BITS28 0x00001C
> +#define AJB_BITS32 0x000020
> +#define AJB_BITS34 0x000022
> +#define AJB_OUT_MS 0x000040
> +#define AJB_OUT_LS 0x000000
> +#define AJB_INV_CLK 0x000080
> +#define AJB_D0_RISE 0x000100
> +#define AJB_D0_FALL 0x000000
> +#define AJB_D1_RISE 0x000200
> +#define AJB_D1_FALL 0x000000
> +#define AJB_IN_RISE 0x000400
> +#define AJB_IN_FALL 0x000000
> +#define AJB_ENABLE 0x000800
> +#define AJB_HOLD0 0x000000
> +#define AJB_HOLD1 0x001000
> +#define AJB_HOLD2 0x002000
> +#define AJB_HOLD3 0x003000
> +#define AJB_RESETN 0x004000
> +#define AJB_CLKSHFT 16
> +#define AJB_BUSY 0x80000000
> +#define AJBTMS HW_REGISTER_RW(AJB_BASE+0x04)
> +#define AJBTDI HW_REGISTER_RW(AJB_BASE+0x08)
> +#define AJBTDO HW_REGISTER_RW(AJB_BASE+0x0c)
> +
> +#endif
> diff --git a/include/hw/arm/bcm2835_mbox.h b/include/hw/arm/bcm2835_mbox.h
> new file mode 100644
> index 0000000..78ab814
> --- /dev/null
> +++ b/include/hw/arm/bcm2835_mbox.h
> @@ -0,0 +1,19 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +#ifndef BCM2835_MBOX_H
> +#define BCM2835_MBOX_H
> +
> +/* Constants shared with the ARM identifying separate mailbox channels */
> +#define MBOX_CHAN_POWER 0 /* for use by the power management interface */
> +#define MBOX_CHAN_FB 1 /* for use by the frame buffer */
> +#define MBOX_CHAN_VCHIQ 3 /* for use by the VCHIQ interface */
> +#define MBOX_CHAN_PROPERTY 8 /* for use by the property channel */
> +#define MBOX_CHAN_COUNT 9
> +
> +#define MBOX_SIZE 32
> +#define MBOX_INVALID_DATA 0x0f
> +
> +#endif /* BCM2835_MBOX_H */
> diff --git a/include/hw/misc/bcm2835_sbm.h b/include/hw/misc/bcm2835_sbm.h
> new file mode 100644
> index 0000000..da36fbe
> --- /dev/null
> +++ b/include/hw/misc/bcm2835_sbm.h
> @@ -0,0 +1,37 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +#ifndef BCM2835_SBM_H
> +#define BCM2835_SBM_H
> +
> +#include "hw/sysbus.h"
> +#include "exec/address-spaces.h"
> +#include "hw/arm/bcm2835_mbox.h"
> +
> +#define TYPE_BCM2835_SBM "bcm2835_sbm"
> +#define BCM2835_SBM(obj) \
> + OBJECT_CHECK(BCM2835SbmState, (obj), TYPE_BCM2835_SBM)
> +
> +typedef struct {
> + MemoryRegion *mbox_mr;
> + AddressSpace mbox_as;
I can't see where these two fields are used. A quick scan shows that
the SbmState's copy is uses for all DMA ops. If these are needed,
mbox_init should pass a pointer to the the SbmState then this saves
the pointer and navigates as needed back to the container structure to
get the AS.
> + uint32_t reg[MBOX_SIZE];
> + int count;
> + uint32_t status;
> + uint32_t config;
> +} BCM2835Mbox;
> +
> +typedef struct {
/*< private >*/
> + SysBusDevice busdev;
/*< public >*/
Regards,
Peter
> + MemoryRegion *mbox_mr;
> + AddressSpace mbox_as;
> + MemoryRegion iomem;
> + int mbox_irq_disabled;
> + qemu_irq arm_irq;
> + int available[MBOX_CHAN_COUNT];
> + BCM2835Mbox mbox[2];
> +} BCM2835SbmState;
> +
> +#endif
> --
> 2.5.3
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-21 22:49 ` Peter Crosthwaite
@ 2015-12-21 23:15 ` Andrew Baumann
2015-12-21 23:33 ` Peter Crosthwaite
2015-12-21 23:34 ` Grégory ESTRADE
0 siblings, 2 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-21 23:15 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
Hi Peter,
Thanks for the review!
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Monday, 21 December 2015 14:49
> On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> > This adds the system mailboxes which are used to communicate with a
> > number of GPU peripherals on Pi/Pi2.
> >
> > Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> > ---
> > default-configs/arm-softmmu.mak | 1 +
> > hw/misc/Makefile.objs | 1 +
> > hw/misc/bcm2835_sbm.c | 280 ++++++++++++++++++++
>
> > include/hw/arm/bcm2835_arm_control.h | 481
> +++++++++++++++++++++++++++++++++++
>
> Do we need this file as of this patch?
Yes, for ARM_MC_IHAVEDATAIRQEN and other related defines.
[...]
> > +static void bcm2835_sbm_update(BCM2835SbmState *s)
>
> What does Sbm stand for? If it is acronym is should be all caps in camel case.
I'm not sure -- it comes from Gregory's code; maybe he can comment? Where this device gets instantiated, there is a comment of
/* Semaphores / Doorbells / Mailboxes */
... but only mailboxes are implemented here. I'm guessing maybe it's intended as "system mailboxes". I can rename it.
> > +{
> > + int set;
>
> bool.
>
> > + int done, n;
> > + uint32_t value;
> > +
> > + /* Avoid unwanted recursive calls */
> > + s->mbox_irq_disabled = 1;
> > +
> > + /* Get pending responses and put them in the vc->arm mbox */
> > + done = 0;
> > + while (!done) {
> > + done = 1;
> > + if (s->mbox[0].status & ARM_MS_FULL) {
> > + /* vc->arm mbox full, exit */
>
> break here.
>
> > + } else {
>
> so you can drop the else and get back a level of indent.
>
> > + for (n = 0; n < MBOX_CHAN_COUNT; n++) {
> > + if (s->available[n]) {
> > + value = ldl_phys(&s->mbox_as, n<<4);
> > + if (value != MBOX_INVALID_DATA) {
> > + mbox_push(&s->mbox[0], value);
> > + } else {
> > + /* Hmmm... */
>
> Needs more comment :)
Gregory -- help! :)
[...]
> > + s->available[irq] = level;
> > + if (!s->mbox_irq_disabled) {
>
> I don't think you need this. IO devices are not thread safe and
> core-code knows it. So you don't need to worry about interruption and
> re-entry of your IRQ handler.
I will have to put a debugger on this to confirm, but I think the issue is that we are using a private memory region and GPIOs to route mailbox data and interrupts to the relevant sub peripherals. The ldl_phys invokes another device emulation (such as bcm2835_property.c in this series), which may cause it to signal an interrupt back to us via qemu_set_irq which will recursively run our handler. We want that IRQ to be recorded but "squashed".
[...]
> > +
> > + switch (offset) {
> > + case 0x80: /* MAIL0_READ */
> > + case 0x84:
> > + case 0x88:
> > + case 0x8c:
>
> case 0x80..0x8c
Woah! Is that standard C?
[...]
> > --- /dev/null
> > +++ b/include/hw/arm/bcm2835_arm_control.h
> > @@ -0,0 +1,481 @@
> > +/*
> > + * linux/arch/arm/mach-bcm2708/arm_control.h
[...]
> When you have a regular structure like this, you should collapse it
> down using some arithmatic:
Notice that this file comes from Linux. I know it's not pretty, but can we please keep it as-is, for comparison purposes? I'm not sure there's much value in cleaning it up locally...
> > --- /dev/null
> > +++ b/include/hw/misc/bcm2835_sbm.h
> > @@ -0,0 +1,37 @@
> > +/*
> > + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> > + * This code is licensed under the GNU GPLv2 and later.
> > + */
> > +
> > +#ifndef BCM2835_SBM_H
> > +#define BCM2835_SBM_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "exec/address-spaces.h"
> > +#include "hw/arm/bcm2835_mbox.h"
> > +
> > +#define TYPE_BCM2835_SBM "bcm2835_sbm"
> > +#define BCM2835_SBM(obj) \
> > + OBJECT_CHECK(BCM2835SbmState, (obj), TYPE_BCM2835_SBM)
> > +
> > +typedef struct {
> > + MemoryRegion *mbox_mr;
> > + AddressSpace mbox_as;
>
> I can't see where these two fields are used. A quick scan shows that
> the SbmState's copy is uses for all DMA ops. If these are needed,
> mbox_init should pass a pointer to the the SbmState then this saves
> the pointer and navigates as needed back to the container structure to
> get the AS.
You're right. They're uninitialised/unused detritus from a previous refactor. I'll remove them.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-21 23:15 ` Andrew Baumann
@ 2015-12-21 23:33 ` Peter Crosthwaite
2015-12-21 23:36 ` Grégory ESTRADE
` (2 more replies)
2015-12-21 23:34 ` Grégory ESTRADE
1 sibling, 3 replies; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-21 23:33 UTC (permalink / raw)
To: Andrew Baumann
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
On Mon, Dec 21, 2015 at 3:15 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> Hi Peter,
>
> Thanks for the review!
>
>> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
>> Sent: Monday, 21 December 2015 14:49
>> On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
>> <Andrew.Baumann@microsoft.com> wrote:
>> > This adds the system mailboxes which are used to communicate with a
>> > number of GPU peripherals on Pi/Pi2.
>> >
>> > Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
>> > ---
>> > default-configs/arm-softmmu.mak | 1 +
>> > hw/misc/Makefile.objs | 1 +
>> > hw/misc/bcm2835_sbm.c | 280 ++++++++++++++++++++
>>
>> > include/hw/arm/bcm2835_arm_control.h | 481
>> +++++++++++++++++++++++++++++++++++
>>
>> Do we need this file as of this patch?
>
> Yes, for ARM_MC_IHAVEDATAIRQEN and other related defines.
>
> [...]
>> > +static void bcm2835_sbm_update(BCM2835SbmState *s)
>>
>> What does Sbm stand for? If it is acronym is should be all caps in camel case.
>
> I'm not sure -- it comes from Gregory's code; maybe he can comment? Where this device gets instantiated, there is a comment of
>
> /* Semaphores / Doorbells / Mailboxes */
>
> ... but only mailboxes are implemented here. I'm guessing maybe it's intended as "system mailboxes". I can rename it.
>
>
>> > +{
>> > + int set;
>>
>> bool.
>>
>> > + int done, n;
>> > + uint32_t value;
>> > +
>> > + /* Avoid unwanted recursive calls */
>> > + s->mbox_irq_disabled = 1;
>> > +
>> > + /* Get pending responses and put them in the vc->arm mbox */
>> > + done = 0;
>> > + while (!done) {
>> > + done = 1;
>> > + if (s->mbox[0].status & ARM_MS_FULL) {
>> > + /* vc->arm mbox full, exit */
>>
>> break here.
>>
>> > + } else {
>>
>> so you can drop the else and get back a level of indent.
>>
>> > + for (n = 0; n < MBOX_CHAN_COUNT; n++) {
>> > + if (s->available[n]) {
>> > + value = ldl_phys(&s->mbox_as, n<<4);
>> > + if (value != MBOX_INVALID_DATA) {
>> > + mbox_push(&s->mbox[0], value);
>> > + } else {
>> > + /* Hmmm... */
>>
>> Needs more comment :)
>
> Gregory -- help! :)
>
> [...]
>> > + s->available[irq] = level;
>> > + if (!s->mbox_irq_disabled) {
>>
>> I don't think you need this. IO devices are not thread safe and
>> core-code knows it. So you don't need to worry about interruption and
>> re-entry of your IRQ handler.
>
> I will have to put a debugger on this to confirm, but I think the issue is that we are using a private memory region and GPIOs to route mailbox data and interrupts to the relevant sub peripherals. The ldl_phys invokes another device emulation (such as bcm2835_property.c in this series), which may cause it to signal an interrupt back to us via qemu_set_irq which will recursively run our handler. We want that IRQ to be recorded but "squashed".
>
> [...]
>> > +
>> > + switch (offset) {
>> > + case 0x80: /* MAIL0_READ */
>> > + case 0x84:
>> > + case 0x88:
>> > + case 0x8c:
>>
>> case 0x80..0x8c
>
> Woah! Is that standard C?
>
Yes, its probably one of the more recent language standards though.
QEMU does use to more modern features liberally.
> [...]
>> > --- /dev/null
>> > +++ b/include/hw/arm/bcm2835_arm_control.h
>> > @@ -0,0 +1,481 @@
>> > +/*
>> > + * linux/arch/arm/mach-bcm2708/arm_control.h
> [...]
>> When you have a regular structure like this, you should collapse it
>> down using some arithmatic:
>
> Notice that this file comes from Linux. I know it's not pretty, but can we please keep it as-is, for comparison purposes? I'm not sure there's much value in cleaning it up locally...
>
It looks very autogenerated and seems pretty nasty on the repetition.
As implementers of the hardware, it is much rarer to need these
repetitious defs than the software users on the other side. "Do
something specific with CPU#3's Mbox#5" is going to appear in
software, but hardware implementers generally don't have a choice to
implement things specifically and it usually ends up being looped and
the exploded defs are never used. If there are only a handful of
genuinely single defs needed, can they be fished out?
Regards,
Peter
>> > --- /dev/null
>> > +++ b/include/hw/misc/bcm2835_sbm.h
>> > @@ -0,0 +1,37 @@
>> > +/*
>> > + * Raspberry Pi emulation (c) 2012 Gregory Estrade
>> > + * This code is licensed under the GNU GPLv2 and later.
>> > + */
>> > +
>> > +#ifndef BCM2835_SBM_H
>> > +#define BCM2835_SBM_H
>> > +
>> > +#include "hw/sysbus.h"
>> > +#include "exec/address-spaces.h"
>> > +#include "hw/arm/bcm2835_mbox.h"
>> > +
>> > +#define TYPE_BCM2835_SBM "bcm2835_sbm"
>> > +#define BCM2835_SBM(obj) \
>> > + OBJECT_CHECK(BCM2835SbmState, (obj), TYPE_BCM2835_SBM)
>> > +
>> > +typedef struct {
>> > + MemoryRegion *mbox_mr;
>> > + AddressSpace mbox_as;
>>
>> I can't see where these two fields are used. A quick scan shows that
>> the SbmState's copy is uses for all DMA ops. If these are needed,
>> mbox_init should pass a pointer to the the SbmState then this saves
>> the pointer and navigates as needed back to the container structure to
>> get the AS.
>
> You're right. They're uninitialised/unused detritus from a previous refactor. I'll remove them.
>
> Thanks,
> Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-21 23:33 ` Peter Crosthwaite
@ 2015-12-21 23:36 ` Grégory ESTRADE
2015-12-21 23:59 ` Andrew Baumann
2015-12-23 20:32 ` Paolo Bonzini
2 siblings, 0 replies; 33+ messages in thread
From: Grégory ESTRADE @ 2015-12-21 23:36 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Peter Crosthwaite, Stefan Weil,
qemu-devel@nongnu.org Developers, Andrew Baumann, qemu-arm,
Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 5920 bytes --]
Indeed, this header is generated from the Broadcom's Linux drivers headers,
with a Perl script.
On Tue, Dec 22, 2015 at 12:33 AM, Peter Crosthwaite <
crosthwaitepeter@gmail.com> wrote:
> On Mon, Dec 21, 2015 at 3:15 PM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> > Hi Peter,
> >
> > Thanks for the review!
> >
> >> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> >> Sent: Monday, 21 December 2015 14:49
> >> On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
> >> <Andrew.Baumann@microsoft.com> wrote:
> >> > This adds the system mailboxes which are used to communicate with a
> >> > number of GPU peripherals on Pi/Pi2.
> >> >
> >> > Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> >> > ---
> >> > default-configs/arm-softmmu.mak | 1 +
> >> > hw/misc/Makefile.objs | 1 +
> >> > hw/misc/bcm2835_sbm.c | 280 ++++++++++++++++++++
> >>
> >> > include/hw/arm/bcm2835_arm_control.h | 481
> >> +++++++++++++++++++++++++++++++++++
> >>
> >> Do we need this file as of this patch?
> >
> > Yes, for ARM_MC_IHAVEDATAIRQEN and other related defines.
> >
> > [...]
> >> > +static void bcm2835_sbm_update(BCM2835SbmState *s)
> >>
> >> What does Sbm stand for? If it is acronym is should be all caps in
> camel case.
> >
> > I'm not sure -- it comes from Gregory's code; maybe he can comment?
> Where this device gets instantiated, there is a comment of
> >
> > /* Semaphores / Doorbells / Mailboxes */
> >
> > ... but only mailboxes are implemented here. I'm guessing maybe it's
> intended as "system mailboxes". I can rename it.
> >
> >
> >> > +{
> >> > + int set;
> >>
> >> bool.
> >>
> >> > + int done, n;
> >> > + uint32_t value;
> >> > +
> >> > + /* Avoid unwanted recursive calls */
> >> > + s->mbox_irq_disabled = 1;
> >> > +
> >> > + /* Get pending responses and put them in the vc->arm mbox */
> >> > + done = 0;
> >> > + while (!done) {
> >> > + done = 1;
> >> > + if (s->mbox[0].status & ARM_MS_FULL) {
> >> > + /* vc->arm mbox full, exit */
> >>
> >> break here.
> >>
> >> > + } else {
> >>
> >> so you can drop the else and get back a level of indent.
> >>
> >> > + for (n = 0; n < MBOX_CHAN_COUNT; n++) {
> >> > + if (s->available[n]) {
> >> > + value = ldl_phys(&s->mbox_as, n<<4);
> >> > + if (value != MBOX_INVALID_DATA) {
> >> > + mbox_push(&s->mbox[0], value);
> >> > + } else {
> >> > + /* Hmmm... */
> >>
> >> Needs more comment :)
> >
> > Gregory -- help! :)
> >
> > [...]
> >> > + s->available[irq] = level;
> >> > + if (!s->mbox_irq_disabled) {
> >>
> >> I don't think you need this. IO devices are not thread safe and
> >> core-code knows it. So you don't need to worry about interruption and
> >> re-entry of your IRQ handler.
> >
> > I will have to put a debugger on this to confirm, but I think the issue
> is that we are using a private memory region and GPIOs to route mailbox
> data and interrupts to the relevant sub peripherals. The ldl_phys invokes
> another device emulation (such as bcm2835_property.c in this series), which
> may cause it to signal an interrupt back to us via qemu_set_irq which will
> recursively run our handler. We want that IRQ to be recorded but "squashed".
> >
> > [...]
> >> > +
> >> > + switch (offset) {
> >> > + case 0x80: /* MAIL0_READ */
> >> > + case 0x84:
> >> > + case 0x88:
> >> > + case 0x8c:
> >>
> >> case 0x80..0x8c
> >
> > Woah! Is that standard C?
> >
>
> Yes, its probably one of the more recent language standards though.
> QEMU does use to more modern features liberally.
>
> > [...]
> >> > --- /dev/null
> >> > +++ b/include/hw/arm/bcm2835_arm_control.h
> >> > @@ -0,0 +1,481 @@
> >> > +/*
> >> > + * linux/arch/arm/mach-bcm2708/arm_control.h
> > [...]
> >> When you have a regular structure like this, you should collapse it
> >> down using some arithmatic:
> >
> > Notice that this file comes from Linux. I know it's not pretty, but can
> we please keep it as-is, for comparison purposes? I'm not sure there's much
> value in cleaning it up locally...
> >
>
> It looks very autogenerated and seems pretty nasty on the repetition.
>
> As implementers of the hardware, it is much rarer to need these
> repetitious defs than the software users on the other side. "Do
> something specific with CPU#3's Mbox#5" is going to appear in
> software, but hardware implementers generally don't have a choice to
> implement things specifically and it usually ends up being looped and
> the exploded defs are never used. If there are only a handful of
> genuinely single defs needed, can they be fished out?
>
> Regards,
> Peter
>
> >> > --- /dev/null
> >> > +++ b/include/hw/misc/bcm2835_sbm.h
> >> > @@ -0,0 +1,37 @@
> >> > +/*
> >> > + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> >> > + * This code is licensed under the GNU GPLv2 and later.
> >> > + */
> >> > +
> >> > +#ifndef BCM2835_SBM_H
> >> > +#define BCM2835_SBM_H
> >> > +
> >> > +#include "hw/sysbus.h"
> >> > +#include "exec/address-spaces.h"
> >> > +#include "hw/arm/bcm2835_mbox.h"
> >> > +
> >> > +#define TYPE_BCM2835_SBM "bcm2835_sbm"
> >> > +#define BCM2835_SBM(obj) \
> >> > + OBJECT_CHECK(BCM2835SbmState, (obj), TYPE_BCM2835_SBM)
> >> > +
> >> > +typedef struct {
> >> > + MemoryRegion *mbox_mr;
> >> > + AddressSpace mbox_as;
> >>
> >> I can't see where these two fields are used. A quick scan shows that
> >> the SbmState's copy is uses for all DMA ops. If these are needed,
> >> mbox_init should pass a pointer to the the SbmState then this saves
> >> the pointer and navigates as needed back to the container structure to
> >> get the AS.
> >
> > You're right. They're uninitialised/unused detritus from a previous
> refactor. I'll remove them.
> >
> > Thanks,
> > Andrew
>
[-- Attachment #2: Type: text/html, Size: 8263 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-21 23:33 ` Peter Crosthwaite
2015-12-21 23:36 ` Grégory ESTRADE
@ 2015-12-21 23:59 ` Andrew Baumann
2015-12-23 20:32 ` Paolo Bonzini
2 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-21 23:59 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Grégory ESTRADE, Stefan Weil,
Peter Crosthwaite, qemu-devel@nongnu.org Developers, qemu-arm,
Paolo Bonzini
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Monday, 21 December 2015 15:33
> On Mon, Dec 21, 2015 at 3:15 PM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> >> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> >> Sent: Monday, 21 December 2015 14:49
> >> On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
> >> <Andrew.Baumann@microsoft.com> wrote:
> >> > +
> >> > + switch (offset) {
> >> > + case 0x80: /* MAIL0_READ */
> >> > + case 0x84:
> >> > + case 0x88:
> >> > + case 0x8c:
> >>
> >> case 0x80..0x8c
> >
> > Woah! Is that standard C?
> >
>
> Yes, its probably one of the more recent language standards though.
> QEMU does use to more modern features liberally.
Notwithstanding that it's widely used in QEMU (and a nice feature too), and not that I have a problem with adopting it, but just because I'm a pedant: this is definitely not in C99 (per qemu/HACKING). It doesn't appears in C11 either (at least not the final draft). It's a gcc extension:
http://gcc.gnu.org/onlinedocs/gcc/Case-Ranges.html
> > [...]
> >> > --- /dev/null
> >> > +++ b/include/hw/arm/bcm2835_arm_control.h
> >> > @@ -0,0 +1,481 @@
> >> > +/*
> >> > + * linux/arch/arm/mach-bcm2708/arm_control.h
> > [...]
> >> When you have a regular structure like this, you should collapse it
> >> down using some arithmatic:
> >
> > Notice that this file comes from Linux. I know it's not pretty, but can we
> please keep it as-is, for comparison purposes? I'm not sure there's much
> value in cleaning it up locally...
> >
>
> It looks very autogenerated and seems pretty nasty on the repetition.
>
> As implementers of the hardware, it is much rarer to need these
> repetitious defs than the software users on the other side. "Do
> something specific with CPU#3's Mbox#5" is going to appear in
> software, but hardware implementers generally don't have a choice to
> implement things specifically and it usually ends up being looped and
> the exploded defs are never used. If there are only a handful of
> genuinely single defs needed, can they be fished out?
I think so. I'll do that then.
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-21 23:33 ` Peter Crosthwaite
2015-12-21 23:36 ` Grégory ESTRADE
2015-12-21 23:59 ` Andrew Baumann
@ 2015-12-23 20:32 ` Paolo Bonzini
2015-12-23 23:59 ` Peter Crosthwaite
2 siblings, 1 reply; 33+ messages in thread
From: Paolo Bonzini @ 2015-12-23 20:32 UTC (permalink / raw)
To: Peter Crosthwaite, Andrew Baumann
Cc: Peter Maydell, Peter Crosthwaite, Stefan Weil,
Grégory ESTRADE, qemu-devel@nongnu.org Developers, qemu-arm
On 22/12/2015 00:33, Peter Crosthwaite wrote:
>>> >>
>>> >> case 0x80..0x8c
>> >
>> > Woah! Is that standard C?
>> >
> Yes, its probably one of the more recent language standards though.
> QEMU does use to more modern features liberally.
It's actually "case 0x80 ... 0x8c:".
>> Notice that this file comes from Linux. I know it's not pretty, but
>> can we please keep it as-is, for comparison purposes? I'm not sure
>> there's much value in cleaning it up locally...
>
> It looks very autogenerated and seems pretty nasty on the repetition.
>
> As implementers of the hardware, it is much rarer to need these
> repetitious defs than the software users on the other side. "Do
> something specific with CPU#3's Mbox#5" is going to appear in
> software, but hardware implementers generally don't have a choice to
> implement things specifically and it usually ends up being looped and
> the exploded defs are never used. If there are only a handful of
> genuinely single defs needed, can they be fished out?
I see your point and I'm definitely in favor of rewriting headers from
scratch when practical, but any cleanup made is a recipe for unwanted
changes and bugs, especially if the source is full of repetitions.
Paolo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-23 20:32 ` Paolo Bonzini
@ 2015-12-23 23:59 ` Peter Crosthwaite
2015-12-24 0:13 ` Andrew Baumann
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2015-12-23 23:59 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Peter Crosthwaite, Stefan Weil,
Grégory ESTRADE, qemu-devel@nongnu.org Developers,
Andrew Baumann, qemu-arm
On Wed, Dec 23, 2015 at 12:32 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 22/12/2015 00:33, Peter Crosthwaite wrote:
>>>> >>
>>>> >> case 0x80..0x8c
>>> >
>>> > Woah! Is that standard C?
>>> >
>> Yes, its probably one of the more recent language standards though.
>> QEMU does use to more modern features liberally.
>
> It's actually "case 0x80 ... 0x8c:".
>
>>> Notice that this file comes from Linux. I know it's not pretty, but
Where in Linux exactly? I am actually having trouble finding the
original source. Grepping around linux-next, there is no mach
bcm-2708. I can see this mach in an rPI specific fork, but I don't see
it in upstream Linux.
Is it the case that Gregory did this work some time ago now, and that
original source has been refactored away to something different
anyway?
In the grepping I also notice that you are defining the regspecs in
header for the SP804 timer, which we already model and manages its own
defs. Those should be in an sp804 specific header, and Linux does the
same:
$ git grep TIMER_CTRL_DIV1
drivers/clocksource/timer-integrator-ap.c: ctrl |= TIMER_CTRL_DIV16
drivers/clocksource/timer-integrator-ap.c: ctrl |= TIMER_CTRL_DIV16
drivers/clocksource/timer-sp.h:#define TIMER_CTRL_DIV1 (0 << 2)
drivers/clocksource/timer-sp.h:#define TIMER_CTRL_DIV16 (1 << 2) /* ACVR
>>> can we please keep it as-is, for comparison purposes? I'm not sure
>>> there's much value in cleaning it up locally...
>>
>> It looks very autogenerated and seems pretty nasty on the repetition.
>>
>> As implementers of the hardware, it is much rarer to need these
>> repetitious defs than the software users on the other side. "Do
>> something specific with CPU#3's Mbox#5" is going to appear in
>> software, but hardware implementers generally don't have a choice to
>> implement things specifically and it usually ends up being looped and
>> the exploded defs are never used. If there are only a handful of
>> genuinely single defs needed, can they be fished out?
>
> I see your point and I'm definitely in favor of rewriting headers from
> scratch when practical, but any cleanup made is a recipe for unwanted
> changes and bugs, especially if the source is full of repetitions.
>
If this is from Linux, and we want this to be immutable, then
shouldn't we be managing it via our existing linux-headers import
scripts?
Regards,
Peter
> Paolo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-23 23:59 ` Peter Crosthwaite
@ 2015-12-24 0:13 ` Andrew Baumann
0 siblings, 0 replies; 33+ messages in thread
From: Andrew Baumann @ 2015-12-24 0:13 UTC (permalink / raw)
To: Peter Crosthwaite, Paolo Bonzini
Cc: Peter Maydell, Peter Crosthwaite, Stefan Weil,
Grégory ESTRADE, qemu-devel@nongnu.org Developers, qemu-arm
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Wednesday, 23 December 2015 15:59
> >>> Notice that this file comes from Linux. I know it's not pretty, but
>
> Where in Linux exactly? I am actually having trouble finding the
> original source. Grepping around linux-next, there is no mach
> bcm-2708. I can see this mach in an rPI specific fork, but I don't see
> it in upstream Linux.
After some grovelling, I think it originates from here:
https://github.com/raspberrypi/linux/blob/rpi-4.1.y/arch/arm/mach-bcm2708/include/mach/arm_control.h
However, there are some diffs between that and my version, which look like they originated from someone hacking it to get each line under 80 characteres (to satisfy checkpatch.pl?). This rather defeats the purpose of keeping it unmodified.
As far as I can tell, we only need the mailbox-related defs. I'll just pull those out into the relevant device emulation, and drop the file.
Andrew
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] bcm2835_sbm: add BCM2835 mailboxes
2015-12-21 23:15 ` Andrew Baumann
2015-12-21 23:33 ` Peter Crosthwaite
@ 2015-12-21 23:34 ` Grégory ESTRADE
1 sibling, 0 replies; 33+ messages in thread
From: Grégory ESTRADE @ 2015-12-21 23:34 UTC (permalink / raw)
To: Andrew Baumann
Cc: Peter Maydell, Peter Crosthwaite, Stefan Weil,
qemu-devel@nongnu.org Developers, Peter Crosthwaite, qemu-arm,
Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 4981 bytes --]
Hi,
The "sbm" is a typo. It was meant to be "smb": Semaphore MailBoxes...
Sorry about that.
Regards,
Gregory
On Tue, Dec 22, 2015 at 12:15 AM, Andrew Baumann <
Andrew.Baumann@microsoft.com> wrote:
> Hi Peter,
>
> Thanks for the review!
>
> > From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> > Sent: Monday, 21 December 2015 14:49
> > On Thu, Dec 3, 2015 at 10:01 PM, Andrew Baumann
> > <Andrew.Baumann@microsoft.com> wrote:
> > > This adds the system mailboxes which are used to communicate with a
> > > number of GPU peripherals on Pi/Pi2.
> > >
> > > Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> > > ---
> > > default-configs/arm-softmmu.mak | 1 +
> > > hw/misc/Makefile.objs | 1 +
> > > hw/misc/bcm2835_sbm.c | 280 ++++++++++++++++++++
> >
> > > include/hw/arm/bcm2835_arm_control.h | 481
> > +++++++++++++++++++++++++++++++++++
> >
> > Do we need this file as of this patch?
>
> Yes, for ARM_MC_IHAVEDATAIRQEN and other related defines.
>
> [...]
> > > +static void bcm2835_sbm_update(BCM2835SbmState *s)
> >
> > What does Sbm stand for? If it is acronym is should be all caps in camel
> case.
>
> I'm not sure -- it comes from Gregory's code; maybe he can comment? Where
> this device gets instantiated, there is a comment of
>
> /* Semaphores / Doorbells / Mailboxes */
>
> ... but only mailboxes are implemented here. I'm guessing maybe it's
> intended as "system mailboxes". I can rename it.
>
>
> > > +{
> > > + int set;
> >
> > bool.
> >
> > > + int done, n;
> > > + uint32_t value;
> > > +
> > > + /* Avoid unwanted recursive calls */
> > > + s->mbox_irq_disabled = 1;
> > > +
> > > + /* Get pending responses and put them in the vc->arm mbox */
> > > + done = 0;
> > > + while (!done) {
> > > + done = 1;
> > > + if (s->mbox[0].status & ARM_MS_FULL) {
> > > + /* vc->arm mbox full, exit */
> >
> > break here.
> >
> > > + } else {
> >
> > so you can drop the else and get back a level of indent.
> >
> > > + for (n = 0; n < MBOX_CHAN_COUNT; n++) {
> > > + if (s->available[n]) {
> > > + value = ldl_phys(&s->mbox_as, n<<4);
> > > + if (value != MBOX_INVALID_DATA) {
> > > + mbox_push(&s->mbox[0], value);
> > > + } else {
> > > + /* Hmmm... */
> >
> > Needs more comment :)
>
> Gregory -- help! :)
>
> [...]
> > > + s->available[irq] = level;
> > > + if (!s->mbox_irq_disabled) {
> >
> > I don't think you need this. IO devices are not thread safe and
> > core-code knows it. So you don't need to worry about interruption and
> > re-entry of your IRQ handler.
>
> I will have to put a debugger on this to confirm, but I think the issue is
> that we are using a private memory region and GPIOs to route mailbox data
> and interrupts to the relevant sub peripherals. The ldl_phys invokes
> another device emulation (such as bcm2835_property.c in this series), which
> may cause it to signal an interrupt back to us via qemu_set_irq which will
> recursively run our handler. We want that IRQ to be recorded but "squashed".
>
> [...]
> > > +
> > > + switch (offset) {
> > > + case 0x80: /* MAIL0_READ */
> > > + case 0x84:
> > > + case 0x88:
> > > + case 0x8c:
> >
> > case 0x80..0x8c
>
> Woah! Is that standard C?
>
> [...]
> > > --- /dev/null
> > > +++ b/include/hw/arm/bcm2835_arm_control.h
> > > @@ -0,0 +1,481 @@
> > > +/*
> > > + * linux/arch/arm/mach-bcm2708/arm_control.h
> [...]
> > When you have a regular structure like this, you should collapse it
> > down using some arithmatic:
>
> Notice that this file comes from Linux. I know it's not pretty, but can we
> please keep it as-is, for comparison purposes? I'm not sure there's much
> value in cleaning it up locally...
>
> > > --- /dev/null
> > > +++ b/include/hw/misc/bcm2835_sbm.h
> > > @@ -0,0 +1,37 @@
> > > +/*
> > > + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> > > + * This code is licensed under the GNU GPLv2 and later.
> > > + */
> > > +
> > > +#ifndef BCM2835_SBM_H
> > > +#define BCM2835_SBM_H
> > > +
> > > +#include "hw/sysbus.h"
> > > +#include "exec/address-spaces.h"
> > > +#include "hw/arm/bcm2835_mbox.h"
> > > +
> > > +#define TYPE_BCM2835_SBM "bcm2835_sbm"
> > > +#define BCM2835_SBM(obj) \
> > > + OBJECT_CHECK(BCM2835SbmState, (obj), TYPE_BCM2835_SBM)
> > > +
> > > +typedef struct {
> > > + MemoryRegion *mbox_mr;
> > > + AddressSpace mbox_as;
> >
> > I can't see where these two fields are used. A quick scan shows that
> > the SbmState's copy is uses for all DMA ops. If these are needed,
> > mbox_init should pass a pointer to the the SbmState then this saves
> > the pointer and navigates as needed back to the container structure to
> > get the AS.
>
> You're right. They're uninitialised/unused detritus from a previous
> refactor. I'll remove them.
>
> Thanks,
> Andrew
>
[-- Attachment #2: Type: text/html, Size: 6975 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread