Hi Peter,
> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Wednesday, 30 December 2015 16:13
> 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.
[...]
> > --- /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"?
Until we support more than one Broadcom SoC, probably not -- this is all based
on the Pi documentation, and I have no idea how much commonality there is to
other parts.
[...]
> > +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?).
These come from child devices (bcm2835_property in the present series). The
other case where this arises is a push in mbox 1, but that is already checked
in bcm2835_mbox_write, so I think an assert is appropriate.
[...]
> > +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 */
Huh? You mean newline? (There's already a space.)
Andrew
