On Wed, Dec 23, 2015 at 4:25 PM, Andrew Baumann <andrew.baum...@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.baum...@microsoft.com> > --- > > Notes: > v2: > * renamed bcm2835_sbm to bcm2835_mbox > * dropped bcm2835_arm_control.h (needed defs moved to bcm2835_mbox.c) > * documented use of private address space through defines in > bcm2835_mbox_defs.h > * remove unused fields from BCM2835Mbox > * s/int/bool/ where appropriate > * cleaned up logic in _update and _mbox_update_status for > clarity/simplicity > * added vmstate > * misc cleanup > > default-configs/arm-softmmu.mak | 1 + > hw/misc/Makefile.objs | 1 + > hw/misc/bcm2835_mbox.c | 323 > ++++++++++++++++++++++++++++++++++++ > include/hw/misc/bcm2835_mbox.h | 37 +++++ > include/hw/misc/bcm2835_mbox_defs.h | 26 +++ > 5 files changed, 388 insertions(+) > create mode 100644 hw/misc/bcm2835_mbox.c > create mode 100644 include/hw/misc/bcm2835_mbox.h > create mode 100644 include/hw/misc/bcm2835_mbox_defs.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..e36f2ee 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_mbox.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_mbox.c b/hw/misc/bcm2835_mbox.c > new file mode 100644 > index 0000000..69b8e2a > --- /dev/null > +++ b/hw/misc/bcm2835_mbox.c > @@ -0,0 +1,323 @@ > +/* > + * Raspberry Pi emulation (c) 2012 Gregory Estrade
Should we change this to "Broadcom SoC emulation"? > + * This code is licensed under the GNU GPLv2 and later. > + * > + * This file models the system mailboxes, which are used for > + * communication with low-bandwidth GPU peripherals. Refs: > + * https://github.com/raspberrypi/firmware/wiki/Mailboxes > + * https://github.com/raspberrypi/firmware/wiki/Accessing-mailboxes > + */ > + > +#include "hw/misc/bcm2835_mbox.h" > + > +/* Mailbox status register (...0x98) */ So the reg offsets could be MACRO defined here to avoid having to to comment 0x98 (and 0x9c) is two places. > +#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 */ > + > +static void mbox_update_status(BCM2835Mbox *mb) > +{ > + mb->status &= ~(ARM_MS_EMPTY | ARM_MS_FULL); > + if (mb->count == 0) { > + mb->status |= ARM_MS_EMPTY; > + } else if (mb->count == MBOX_SIZE) { > + mb->status |= ARM_MS_FULL; > + } > +} > + > +static void mbox_init(BCM2835Mbox *mb) mbox_reset > +{ > + int n; > + > + 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); mbox_update can call mbox_push with val == a DMA value, which usually suggests that this may be a guest controllable (and a guest error rather than an assert). Is the mbox AS guest accessible? (I had a look at the later patches, and the MBox AS seems private but can values be set via the guest indirectly?). > + mb->reg[mb->count++] = val; > + mbox_update_status(mb); > +} > + > +static void bcm2835_mbox_update(BCM2835MboxState *s) > +{ > + uint32_t value; > + bool set; > + int n; > + > + /* Avoid unwanted recursive calls */ You should really only need to comment this idea the once. The one below is more informative so you can drop this. > + s->mbox_irq_disabled = true; > + > + /* Get pending responses and put them in the vc->arm mbox, > + * as long as it's not full */ > + for (n = 0; n < MBOX_CHAN_COUNT; n++) { > + while (s->available[n] && !(s->mbox[0].status & ARM_MS_FULL)) { > + value = ldl_phys(&s->mbox_as, n << MBOX_AS_CHAN_SHIFT); > + if (value == MBOX_INVALID_DATA) { > + /* Interrupt pending, but there's no data. Hmmm... */ > + hw_error("%s: spurious interrupt on channel %d", __func__, > n); hw_error should be avoided, it should be an assert for something if this is a QEMU code error, or LOG_GUEST_ERROR or LOG_UNIMP. > + } > + mbox_push(&s->mbox[0], value); > + } > + } > + > + /* Try to push pending requests from the arm->vc mbox */ > + /* TODO (?) */ > + > + /* Re-enable calls from the IRQ routine */ > + s->mbox_irq_disabled = false; > + > + /* Update ARM IRQ status */ > + set = false; > + s->mbox[0].config &= ~ARM_MC_IHAVEDATAIRQPEND; > + if (!(s->mbox[0].status & ARM_MS_EMPTY)) { > + s->mbox[0].config |= ARM_MC_IHAVEDATAIRQPEND; > + if (s->mbox[0].config & ARM_MC_IHAVEDATAIRQEN) { > + set = true; > + } > + } > + qemu_set_irq(s->arm_irq, set); > +} > + > +static void bcm2835_mbox_set_irq(void *opaque, int irq, int level) > +{ > + BCM2835MboxState *s = opaque; > + > + s->available[irq] = level; > + > + /* avoid recursively calling bcm2835_mbox_update when the interrupt > + * status changes due to the ldl_phys call within that function */ Space before */ > + if (!s->mbox_irq_disabled) { > + bcm2835_mbox_update(s); > + } > +} > + > +static uint64_t bcm2835_mbox_read(void *opaque, hwaddr offset, unsigned size) > +{ > + BCM2835MboxState *s = opaque; > + uint32_t res = 0; > + > + offset &= 0xff; > + > + switch (offset) { > + case 0x80 ... 0x8c: /* MAIL0_READ */ > + 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; Blank line here (you seem to logically break the switches below in the write handler). > + case 0xb8: /* MAIL1_STATUS */ > + res = s->mbox[1].status; > + break; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n", > + __func__, offset); > + return 0; > + } > + > + bcm2835_mbox_update(s); > + > + return res; > +} > + > +static void bcm2835_mbox_write(void *opaque, hwaddr offset, > + uint64_t value, unsigned size) > +{ > + BCM2835MboxState *s = opaque; > + hwaddr childaddr; > + uint8_t ch; > + > + 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 ... 0xac: > + if (s->mbox[1].status & ARM_MS_FULL) { > + /* Mailbox full */ > + qemu_log_mask(LOG_GUEST_ERROR, "%s: mailbox full\n", __func__); > + } else { > + ch = value & 0xf; > + if (ch < MBOX_CHAN_COUNT) { > + childaddr = ch << MBOX_AS_CHAN_SHIFT; > + if (ldl_phys(&s->mbox_as, childaddr + MBOX_AS_PENDING)) { > + /* Child busy, push delayed. Push it in the arm->vc mbox > */ > + mbox_push(&s->mbox[1], value); > + } else { > + /* Push it directly to the child device */ > + stl_phys(&s->mbox_as, childaddr, value); > + } > + } else { > + /* Invalid channel number */ > + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid channel %u\n", > + __func__, ch); > + } > + } > + break; > + > + default: > + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n", > + __func__, offset); > + return; > + } > + > + bcm2835_mbox_update(s); > +} > + > +static const MemoryRegionOps bcm2835_mbox_ops = { > + .read = bcm2835_mbox_read, > + .write = bcm2835_mbox_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > + .valid.min_access_size = 4, > + .valid.max_access_size = 4, > +}; > + > +/* vmstate of a single mailbox */ > +static const VMStateDescription vmstate_bcm2835_mbox_box = { > + .name = TYPE_BCM2835_MBOX "_box", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32_ARRAY(reg, BCM2835Mbox, MBOX_SIZE), > + VMSTATE_UINT32(count, BCM2835Mbox), > + VMSTATE_UINT32(status, BCM2835Mbox), > + VMSTATE_UINT32(config, BCM2835Mbox), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +/* vmstate of the entire device */ > +static const VMStateDescription vmstate_bcm2835_mbox = { > + .name = TYPE_BCM2835_MBOX, > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .fields = (VMStateField[]) { > + VMSTATE_BOOL_ARRAY(available, BCM2835MboxState, MBOX_CHAN_COUNT), > + VMSTATE_STRUCT_ARRAY(mbox, BCM2835MboxState, 2, 1, > + vmstate_bcm2835_mbox_box, BCM2835Mbox), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void bcm2835_mbox_init(Object *obj) > +{ > + BCM2835MboxState *s = BCM2835_MBOX(obj); Blank line here. > + memory_region_init_io(&s->iomem, obj, &bcm2835_mbox_ops, s, > + TYPE_BCM2835_MBOX, 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_mbox_set_irq, MBOX_CHAN_COUNT); > +} > + > +static void bcm2835_mbox_reset(DeviceState *dev) > +{ > + BCM2835MboxState *s = BCM2835_MBOX(dev); > + int n; > + > + mbox_init(&s->mbox[0]); > + mbox_init(&s->mbox[1]); > + s->mbox_irq_disabled = false; > + for (n = 0; n < MBOX_CHAN_COUNT; n++) { > + s->available[n] = false; > + } > +} > + > +static void bcm2835_mbox_realize(DeviceState *dev, Error **errp) > +{ > + BCM2835MboxState *s = BCM2835_MBOX(dev); > + Object *obj; > + Error *err = NULL; > + > + obj = object_property_get_link(OBJECT(dev), "mbox_mr", &err); Link property should be - rather than _. > + if (obj == NULL) { > + error_setg(errp, "%s: required mbox_mr link not found: %s", > + __func__, error_get_pretty(err)); > + return; > + } > + > + s->mbox_mr = MEMORY_REGION(obj); > + address_space_init(&s->mbox_as, s->mbox_mr, NULL); > + bcm2835_mbox_reset(dev); > +} > + > +static void bcm2835_mbox_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->realize = bcm2835_mbox_realize; > + dc->reset = bcm2835_mbox_reset; > + dc->vmsd = &vmstate_bcm2835_mbox; > +} > + > +static TypeInfo bcm2835_mbox_info = { > + .name = TYPE_BCM2835_MBOX, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(BCM2835MboxState), > + .class_init = bcm2835_mbox_class_init, > + .instance_init = bcm2835_mbox_init, > +}; > + > +static void bcm2835_mbox_register_types(void) > +{ > + type_register_static(&bcm2835_mbox_info); > +} > + > +type_init(bcm2835_mbox_register_types) > diff --git a/include/hw/misc/bcm2835_mbox.h b/include/hw/misc/bcm2835_mbox.h > new file mode 100644 > index 0000000..f24560c > --- /dev/null > +++ b/include/hw/misc/bcm2835_mbox.h > @@ -0,0 +1,37 @@ > +/* > + * 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 > + > +#include "bcm2835_mbox_defs.h" > +#include "hw/sysbus.h" > +#include "exec/address-spaces.h" > + > +#define TYPE_BCM2835_MBOX "bcm2835_mbox" Type name should be - separated not _. With machine configs and device hotplug, typenames and properties have backwards compatibility issues so we want to get them straight from the start. > +#define BCM2835_MBOX(obj) \ > + OBJECT_CHECK(BCM2835MboxState, (obj), TYPE_BCM2835_MBOX) > + > +typedef struct { > + uint32_t reg[MBOX_SIZE]; > + uint32_t count; > + uint32_t status; > + uint32_t config; > +} BCM2835Mbox; > + > +typedef struct { > + /*< private >*/ > + SysBusDevice busdev; > + /*< public >*/ > + MemoryRegion *mbox_mr; > + AddressSpace mbox_as; > + MemoryRegion iomem; > + bool mbox_irq_disabled; > + qemu_irq arm_irq; I would move this one-up to before irq_disabled to avoid mixing io and internal state defs. > + bool available[MBOX_CHAN_COUNT]; > + BCM2835Mbox mbox[2]; > +} BCM2835MboxState; > + > +#endif > diff --git a/include/hw/misc/bcm2835_mbox_defs.h > b/include/hw/misc/bcm2835_mbox_defs.h > new file mode 100644 > index 0000000..48d8ee4 > --- /dev/null > +++ b/include/hw/misc/bcm2835_mbox_defs.h > @@ -0,0 +1,26 @@ > +/* > + * Raspberry Pi emulation (c) 2012 Gregory Estrade > + * This code is licensed under the GNU GPLv2 and later. > + */ > + > +#ifndef BCM2835_MBOX_DEFS_H > +#define BCM2835_MBOX_DEFS_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 > + > +/* Layout of the private address space used for communication between > + * the mbox device emulation, and child devices: each channel occupies > + * 16 bytes of address space, but only two registers are presently defined. > */ Space before */ Regards, Peter > +#define MBOX_AS_CHAN_SHIFT 4 > +#define MBOX_AS_DATA 0 /* request / response data (RW at offset 0) */ > +#define MBOX_AS_PENDING 4 /* pending response status (RO at offset 4) */ > + > +#endif /* BCM2835_MBOX_DEFS_H */ > -- > 2.5.3 >
