diff mbox

[RFC] bcm2835_dma: add emulation of Raspberry Pi DMA controller

Message ID 1456787490-17112-1-git-send-email-Andrew.Baumann@microsoft.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Baumann Feb. 29, 2016, 11:11 p.m. UTC
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
This patch applies on top of the previous series for Windows and
framebuffer support:
  https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg06387.html

After preparing that, I was disappointed to discover that Raspbian
won't boot cleanly without the DMA controller. In the hope of beating
the freeze deadline (it's still February 29 here :-) I'm sending this
for review.

After applying this patch, it is possible to boot Raspbian to the GUI
using a command such as:

  qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
  2015-09-24-raspbian-jessie.img -append "rw earlyprintk loglevel=8
  console=ttyAMA0 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootwait"
  -dtb raspbian-boot/bcm2709-rpi-2-b.dtb -serial stdio

As before, this derives from the original (out of tree) work of
Gregory Estrade, Stefan Weil and others to support Raspberry Pi
1. This patch in particulary is Gregory's code, which I have cleaned
up for submission.

Thanks,
Andrew

 hw/arm/bcm2835_peripherals.c         |  26 +++
 hw/dma/Makefile.objs                 |   1 +
 hw/dma/bcm2835_dma.c                 | 397 +++++++++++++++++++++++++++++++++++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/dma/bcm2835_dma.h         |  47 +++++
 5 files changed, 473 insertions(+)
 create mode 100644 hw/dma/bcm2835_dma.c
 create mode 100644 include/hw/dma/bcm2835_dma.h

Comments

Peter Maydell March 3, 2016, 2:24 p.m. UTC | #1
On 29 February 2016 at 23:11, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> ---
> This patch applies on top of the previous series for Windows and
> framebuffer support:
>   https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg06387.html
>
> After preparing that, I was disappointed to discover that Raspbian
> won't boot cleanly without the DMA controller. In the hope of beating
> the freeze deadline (it's still February 29 here :-) I'm sending this
> for review.
>
> After applying this patch, it is possible to boot Raspbian to the GUI
> using a command such as:
>
>   qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
>   2015-09-24-raspbian-jessie.img -append "rw earlyprintk loglevel=8
>   console=ttyAMA0 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootwait"
>   -dtb raspbian-boot/bcm2709-rpi-2-b.dtb -serial stdio
>
> As before, this derives from the original (out of tree) work of
> Gregory Estrade, Stefan Weil and others to support Raspberry Pi
> 1. This patch in particulary is Gregory's code, which I have cleaned
> up for submission.

Should it have a Signed-off-by: line and/or authorship line
from Gregory, then?

(This is largely about giving credit where due, so it's Gregory's
choice really. I forget whether we've had this discussion before
but I couldn't find anything in a quick sweep through earlier mail
threads.)

> +static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
> +{
> +    BCM2835DMAChan *ch = &s->chan[c];
> +    uint32_t data, xlen, ylen;
> +    int16_t dst_stride, src_stride;
> +
> +    if (!(s->enable & (1 << c))) {
> +        return;
> +    }
> +
> +    while ((s->enable & (1 << c)) && (ch->conblk_ad != 0)) {
> +        /* CB fetch */
> +        ch->ti = ldl_phys(&s->dma_as, ch->conblk_ad);
> +        ch->source_ad = ldl_phys(&s->dma_as, ch->conblk_ad + 4);
> +        ch->dest_ad = ldl_phys(&s->dma_as, ch->conblk_ad + 8);
> +        ch->txfr_len = ldl_phys(&s->dma_as, ch->conblk_ad + 12);
> +        ch->stride = ldl_phys(&s->dma_as, ch->conblk_ad + 16);
> +        ch->nextconbk = ldl_phys(&s->dma_as, ch->conblk_ad + 20);

These should use the ldl_{le,be}_phys functions. (If you care
about modelling the response to "unreadable address" you can
use address_space_ldl_{le,be}.)

> +
> +        if (ch->ti & BCM2708_DMA_TDMODE) {
> +            /* 2D transfer mode */
> +            ylen = (ch->txfr_len >> 16) & 0x3fff;
> +            xlen = ch->txfr_len & 0xffff;
> +            dst_stride = ch->stride >> 16;
> +            src_stride = ch->stride & 0xffff;
> +        } else {
> +            ylen = 1;
> +            xlen = ch->txfr_len;
> +            dst_stride = 0;
> +            src_stride = 0;
> +        }
> +
> +        while (ylen != 0) {
> +            /* Normal transfer mode */
> +            while (xlen != 0) {
> +                if (ch->ti & BCM2708_DMA_S_IGNORE) {
> +                    /* Ignore reads */
> +                    data = 0;
> +                } else {
> +                    data = ldl_phys(&s->dma_as, ch->source_ad);
> +                }
> +                if (ch->ti & BCM2708_DMA_S_INC) {
> +                    ch->source_ad += 4;
> +                }
> +
> +                if (ch->ti & BCM2708_DMA_D_IGNORE) {
> +                    /* Ignore writes */
> +                } else {
> +                    stl_phys(&s->dma_as, ch->dest_ad, data);
> +                }
> +                if (ch->ti & BCM2708_DMA_D_INC) {
> +                    ch->dest_ad += 4;
> +                }
> +
> +                /* update remaining transfer length */
> +                xlen -= 4;
> +                if (ch->ti & BCM2708_DMA_TDMODE) {
> +                    ch->txfr_len = (ylen << 16) | xlen;
> +                } else {
> +                    ch->txfr_len = xlen;
> +                }
> +            }
> +
> +            if (--ylen != 0) {
> +                ch->source_ad += src_stride;
> +                ch->dest_ad += dst_stride;
> +            }
> +        }
> +        ch->cs |= BCM2708_DMA_END;
> +        if (ch->ti & BCM2708_DMA_INT_EN) {
> +            ch->cs |= BCM2708_DMA_INT;
> +            s->int_status |= (1 << c);
> +            qemu_set_irq(ch->irq, 1);
> +        }
> +
> +        /* Process next CB */
> +        ch->conblk_ad = ch->nextconbk;
> +    }

This loop allows a guest to make QEMU lock up (stop responding to monitor
commands, etc) if it feeds the device a circular loop of CBs. On the other
hand I don't think we have a good approach to avoiding this problem,
so never mind.

> +    ch->cs &= ~BCM2708_DMA_ACTIVE;
> +}
> +
> +static uint64_t bcm2835_dma_read(BCM2835DMAState *s, hwaddr offset,
> +                                 unsigned size, unsigned c)
> +{
> +    BCM2835DMAChan *ch = &s->chan[c];
> +    uint32_t res = 0;
> +
> +    assert(size == 4);
> +    assert(c < BCM2835_DMA_NCHANS);

It's a bit late to assert after you've already used c as
an array index... (the compiler is free to conclude that the
condition must always be true, for instance.)

> +
> +    switch (offset) {
> +    case BCM2708_DMA_CS:
> +        res = ch->cs;
> +        break;
> +    case BCM2708_DMA_ADDR:
> +        res = ch->conblk_ad;
> +        break;
> +    case BCM2708_DMA_INFO:
> +        res = ch->ti;
> +        break;
> +    case BCM2708_DMA_SOURCE_AD:
> +        res = ch->source_ad;
> +        break;
> +    case BCM2708_DMA_DEST_AD:
> +        res = ch->dest_ad;
> +        break;
> +    case BCM2708_DMA_TXFR_LEN:
> +        res = ch->txfr_len;
> +        break;
> +    case BCM2708_DMA_STRIDE:
> +        res = ch->stride;
> +        break;
> +    case BCM2708_DMA_NEXTCB:
> +        res = ch->nextconbk;
> +        break;
> +    case BCM2708_DMA_DEBUG:
> +        res = ch->debug;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
> +                      __func__, offset);
> +        break;
> +    }
> +    return res;
> +}
> +
> +static void bcm2835_dma_write(BCM2835DMAState *s, hwaddr offset,
> +                              uint64_t value, unsigned size, unsigned c)
> +{
> +    BCM2835DMAChan *ch = &s->chan[c];
> +    uint32_t oldcs;
> +
> +    assert(size == 4);
> +    assert(c < BCM2835_DMA_NCHANS);
> +
> +    switch (offset) {
> +    case BCM2708_DMA_CS:
> +        oldcs = ch->cs;
> +        if (value & BCM2708_DMA_RESET) {
> +            ch->cs |= BCM2708_DMA_RESET;

The comment about this bit earlier says it's self-clearing, but I
don't see where it gets cleared.

Otherwise looks OK.

thanks
-- PMM
Grégory ESTRADE March 3, 2016, 2:56 p.m. UTC | #2
I'd be glad if you add a sign-off by me, but it doesn't matter much. Having
this work finally getting into mainstream thanks to Andrew is what matters
there.
Best regards,
Gregory

On Thu, Mar 3, 2016 at 3:24 PM, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 29 February 2016 at 23:11, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> > Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> > ---
> > This patch applies on top of the previous series for Windows and
> > framebuffer support:
> >   https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg06387.html
> >
> > After preparing that, I was disappointed to discover that Raspbian
> > won't boot cleanly without the DMA controller. In the hope of beating
> > the freeze deadline (it's still February 29 here :-) I'm sending this
> > for review.
> >
> > After applying this patch, it is possible to boot Raspbian to the GUI
> > using a command such as:
> >
> >   qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
> >   2015-09-24-raspbian-jessie.img -append "rw earlyprintk loglevel=8
> >   console=ttyAMA0 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootwait"
> >   -dtb raspbian-boot/bcm2709-rpi-2-b.dtb -serial stdio
> >
> > As before, this derives from the original (out of tree) work of
> > Gregory Estrade, Stefan Weil and others to support Raspberry Pi
> > 1. This patch in particulary is Gregory's code, which I have cleaned
> > up for submission.
>
> Should it have a Signed-off-by: line and/or authorship line
> from Gregory, then?
>
> (This is largely about giving credit where due, so it's Gregory's
> choice really. I forget whether we've had this discussion before
> but I couldn't find anything in a quick sweep through earlier mail
> threads.)
>
> > +static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
> > +{
> > +    BCM2835DMAChan *ch = &s->chan[c];
> > +    uint32_t data, xlen, ylen;
> > +    int16_t dst_stride, src_stride;
> > +
> > +    if (!(s->enable & (1 << c))) {
> > +        return;
> > +    }
> > +
> > +    while ((s->enable & (1 << c)) && (ch->conblk_ad != 0)) {
> > +        /* CB fetch */
> > +        ch->ti = ldl_phys(&s->dma_as, ch->conblk_ad);
> > +        ch->source_ad = ldl_phys(&s->dma_as, ch->conblk_ad + 4);
> > +        ch->dest_ad = ldl_phys(&s->dma_as, ch->conblk_ad + 8);
> > +        ch->txfr_len = ldl_phys(&s->dma_as, ch->conblk_ad + 12);
> > +        ch->stride = ldl_phys(&s->dma_as, ch->conblk_ad + 16);
> > +        ch->nextconbk = ldl_phys(&s->dma_as, ch->conblk_ad + 20);
>
> These should use the ldl_{le,be}_phys functions. (If you care
> about modelling the response to "unreadable address" you can
> use address_space_ldl_{le,be}.)
>
> > +
> > +        if (ch->ti & BCM2708_DMA_TDMODE) {
> > +            /* 2D transfer mode */
> > +            ylen = (ch->txfr_len >> 16) & 0x3fff;
> > +            xlen = ch->txfr_len & 0xffff;
> > +            dst_stride = ch->stride >> 16;
> > +            src_stride = ch->stride & 0xffff;
> > +        } else {
> > +            ylen = 1;
> > +            xlen = ch->txfr_len;
> > +            dst_stride = 0;
> > +            src_stride = 0;
> > +        }
> > +
> > +        while (ylen != 0) {
> > +            /* Normal transfer mode */
> > +            while (xlen != 0) {
> > +                if (ch->ti & BCM2708_DMA_S_IGNORE) {
> > +                    /* Ignore reads */
> > +                    data = 0;
> > +                } else {
> > +                    data = ldl_phys(&s->dma_as, ch->source_ad);
> > +                }
> > +                if (ch->ti & BCM2708_DMA_S_INC) {
> > +                    ch->source_ad += 4;
> > +                }
> > +
> > +                if (ch->ti & BCM2708_DMA_D_IGNORE) {
> > +                    /* Ignore writes */
> > +                } else {
> > +                    stl_phys(&s->dma_as, ch->dest_ad, data);
> > +                }
> > +                if (ch->ti & BCM2708_DMA_D_INC) {
> > +                    ch->dest_ad += 4;
> > +                }
> > +
> > +                /* update remaining transfer length */
> > +                xlen -= 4;
> > +                if (ch->ti & BCM2708_DMA_TDMODE) {
> > +                    ch->txfr_len = (ylen << 16) | xlen;
> > +                } else {
> > +                    ch->txfr_len = xlen;
> > +                }
> > +            }
> > +
> > +            if (--ylen != 0) {
> > +                ch->source_ad += src_stride;
> > +                ch->dest_ad += dst_stride;
> > +            }
> > +        }
> > +        ch->cs |= BCM2708_DMA_END;
> > +        if (ch->ti & BCM2708_DMA_INT_EN) {
> > +            ch->cs |= BCM2708_DMA_INT;
> > +            s->int_status |= (1 << c);
> > +            qemu_set_irq(ch->irq, 1);
> > +        }
> > +
> > +        /* Process next CB */
> > +        ch->conblk_ad = ch->nextconbk;
> > +    }
>
> This loop allows a guest to make QEMU lock up (stop responding to monitor
> commands, etc) if it feeds the device a circular loop of CBs. On the other
> hand I don't think we have a good approach to avoiding this problem,
> so never mind.
>
> > +    ch->cs &= ~BCM2708_DMA_ACTIVE;
> > +}
> > +
> > +static uint64_t bcm2835_dma_read(BCM2835DMAState *s, hwaddr offset,
> > +                                 unsigned size, unsigned c)
> > +{
> > +    BCM2835DMAChan *ch = &s->chan[c];
> > +    uint32_t res = 0;
> > +
> > +    assert(size == 4);
> > +    assert(c < BCM2835_DMA_NCHANS);
>
> It's a bit late to assert after you've already used c as
> an array index... (the compiler is free to conclude that the
> condition must always be true, for instance.)
>
> > +
> > +    switch (offset) {
> > +    case BCM2708_DMA_CS:
> > +        res = ch->cs;
> > +        break;
> > +    case BCM2708_DMA_ADDR:
> > +        res = ch->conblk_ad;
> > +        break;
> > +    case BCM2708_DMA_INFO:
> > +        res = ch->ti;
> > +        break;
> > +    case BCM2708_DMA_SOURCE_AD:
> > +        res = ch->source_ad;
> > +        break;
> > +    case BCM2708_DMA_DEST_AD:
> > +        res = ch->dest_ad;
> > +        break;
> > +    case BCM2708_DMA_TXFR_LEN:
> > +        res = ch->txfr_len;
> > +        break;
> > +    case BCM2708_DMA_STRIDE:
> > +        res = ch->stride;
> > +        break;
> > +    case BCM2708_DMA_NEXTCB:
> > +        res = ch->nextconbk;
> > +        break;
> > +    case BCM2708_DMA_DEBUG:
> > +        res = ch->debug;
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset
> %"HWADDR_PRIx"\n",
> > +                      __func__, offset);
> > +        break;
> > +    }
> > +    return res;
> > +}
> > +
> > +static void bcm2835_dma_write(BCM2835DMAState *s, hwaddr offset,
> > +                              uint64_t value, unsigned size, unsigned c)
> > +{
> > +    BCM2835DMAChan *ch = &s->chan[c];
> > +    uint32_t oldcs;
> > +
> > +    assert(size == 4);
> > +    assert(c < BCM2835_DMA_NCHANS);
> > +
> > +    switch (offset) {
> > +    case BCM2708_DMA_CS:
> > +        oldcs = ch->cs;
> > +        if (value & BCM2708_DMA_RESET) {
> > +            ch->cs |= BCM2708_DMA_RESET;
>
> The comment about this bit earlier says it's self-clearing, but I
> don't see where it gets cleared.
>
> Otherwise looks OK.
>
> thanks
> -- PMM
>
Gerd Hoffmann March 3, 2016, 4:16 p.m. UTC | #3
Hi,

> > +        ch->cs |= BCM2708_DMA_END;
> > +        if (ch->ti & BCM2708_DMA_INT_EN) {
> > +            ch->cs |= BCM2708_DMA_INT;
> > +            s->int_status |= (1 << c);
> > +            qemu_set_irq(ch->irq, 1);
> > +        }
> > +
> > +        /* Process next CB */
> > +        ch->conblk_ad = ch->nextconbk;
> > +    }
> 
> This loop allows a guest to make QEMU lock up (stop responding to monitor
> commands, etc) if it feeds the device a circular loop of CBs. On the other
> hand I don't think we have a good approach to avoiding this problem,
> so never mind.

usb emulation has this problem too.

uhci queue heads can go in circles.  The emulation code keeps a linked
list of active queue heads, which is (among other bookkeeping things)
used to detect when we run in circles.  It's a legal thing to do for a
guest btw, so you can see that happening in practice.

until recently ehci could be tricked into running in loops too, by
creating a circular chain of IDTs.  Which is not legal according to
specs, so this went unnoticed for a while.  But a malicious guest can do
it nevertheless.  That one was fixed by stopping IDT processing in case
no data was transfered.  This is possible because the ehci controller
writes back the status to the IDT, so we can figure there is nothing to
do (because we already processed that IDT) without additional
bookkeeping, by simply checking the status.

From a brief look at the patch it seems you can not use the later for
the bcm2835 dma controller, I can't spot a place where the some status
is written back to the dma contol block ...

cheers,
  Gerd
Peter Maydell March 3, 2016, 4:21 p.m. UTC | #4
On 3 March 2016 at 16:16, Gerd Hoffmann <kraxel@redhat.com> wrote:
> usb emulation has this problem too.
>
> uhci queue heads can go in circles.  The emulation code keeps a linked
> list of active queue heads, which is (among other bookkeeping things)
> used to detect when we run in circles.  It's a legal thing to do for a
> guest btw, so you can see that happening in practice.
>
> until recently ehci could be tricked into running in loops too, by
> creating a circular chain of IDTs.  Which is not legal according to
> specs, so this went unnoticed for a while.  But a malicious guest can do
> it nevertheless.  That one was fixed by stopping IDT processing in case
> no data was transfered.  This is possible because the ehci controller
> writes back the status to the IDT, so we can figure there is nothing to
> do (because we already processed that IDT) without additional
> bookkeeping, by simply checking the status.
>
> From a brief look at the patch it seems you can not use the later for
> the bcm2835 dma controller, I can't spot a place where the some status
> is written back to the dma contol block ...

I guess a more general approach to the problem would be to have
a (hopefully easy) way to say "if this has been going on for too
long then arrange to defer continued processing of it til later,
and for now resume the guest". That's too big a can of worms for
this patch, though. (And for something that's only used in TCG
emulation we care much less about malicious guests than for devices
that can be used with KVM.)

thanks
-- PMM
Andrew Baumann March 3, 2016, 5:09 p.m. UTC | #5
> From: Peter Maydell [mailto:peter.maydell@linaro.org]

> Sent: Thursday, 3 March 2016 8:22 AM

> 

> On 3 March 2016 at 16:16, Gerd Hoffmann <kraxel@redhat.com> wrote:

> > usb emulation has this problem too.

> >

> > uhci queue heads can go in circles.  The emulation code keeps a linked

> > list of active queue heads, which is (among other bookkeeping things)

> > used to detect when we run in circles.  It's a legal thing to do for a

> > guest btw, so you can see that happening in practice.

> >

> > until recently ehci could be tricked into running in loops too, by

> > creating a circular chain of IDTs.  Which is not legal according to

> > specs, so this went unnoticed for a while.  But a malicious guest can do

> > it nevertheless.  That one was fixed by stopping IDT processing in case

> > no data was transfered.  This is possible because the ehci controller

> > writes back the status to the IDT, so we can figure there is nothing to

> > do (because we already processed that IDT) without additional

> > bookkeeping, by simply checking the status.

> >

> > From a brief look at the patch it seems you can not use the later for

> > the bcm2835 dma controller, I can't spot a place where the some status

> > is written back to the dma contol block ...

> 

> I guess a more general approach to the problem would be to have

> a (hopefully easy) way to say "if this has been going on for too

> long then arrange to defer continued processing of it til later,

> and for now resume the guest". That's too big a can of worms for

> this patch, though. (And for something that's only used in TCG

> emulation we care much less about malicious guests than for devices

> that can be used with KVM.)


It seems like the ultimate solution here is to model the concurrency of DMA. Run each DMA engine as its own thread, including modelling the ability to pause/abort transfers, and provide some suitable scheduling/resource arbitration mechanism, and then you don’t need to worry about whether a guest can tie up one of those threads in a loop, because it can't hurt anyone else. Of course, this is much more complex to implement.

But in this particular case, I _really_ hope that we don't have to worry about virtual raspberry pi's in multi-tenant hosting! :)

Thanks for the feedback,
Andrew
Gerd Hoffmann March 4, 2016, 8:02 a.m. UTC | #6
Hi,

> I guess a more general approach to the problem would be to have
> a (hopefully easy) way to say "if this has been going on for too
> long then arrange to defer continued processing of it til later,
> and for now resume the guest".

Doable today, by simply applying some limit and if we hit it schedule a
timer to continue later.

uhci emulation does this, but it's basically no extra work there as usb
emulation is timer based _anyway_.

> That's too big a can of worms for
> this patch, though. (And for something that's only used in TCG
> emulation we care much less about malicious guests than for devices
> that can be used with KVM.)

Agree, given the use case this isn't critical by any means.

cheers,
  Gerd
diff mbox

Patch

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index c2b812a..fdd346e 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -87,6 +87,14 @@  static void bcm2835_peripherals_init(Object *obj)
     object_initialize(&s->sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
     object_property_add_child(obj, "sdhci", OBJECT(&s->sdhci), NULL);
     qdev_set_parent_bus(DEVICE(&s->sdhci), sysbus_get_default());
+
+    /* DMA Channels */
+    object_initialize(&s->dma, sizeof(s->dma), TYPE_BCM2835_DMA);
+    object_property_add_child(obj, "dma", OBJECT(&s->dma), NULL);
+    qdev_set_parent_bus(DEVICE(&s->dma), sysbus_get_default());
+
+    object_property_add_const_link(OBJECT(&s->dma), "dma-mr",
+                                   OBJECT(&s->gpu_bus_mr), &error_abort);
 }
 
 static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
@@ -246,6 +254,24 @@  static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /* DMA Channels */
+    object_property_set_bool(OBJECT(&s->dma), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    memory_region_add_subregion(&s->peri_mr, DMA_OFFSET,
+                sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->dma), 0));
+    memory_region_add_subregion(&s->peri_mr, DMA15_OFFSET,
+                sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->dma), 1));
+
+    for (n = 0; n <= 12; n++) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->dma), n,
+                           qdev_get_gpio_in_named(DEVICE(&s->ic),
+                                                  BCM2835_IC_GPU_IRQ,
+                                                  INTERRUPT_DMA0 + n));
+    }
 }
 
 static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
index 0e65ed0..a1abbcf 100644
--- a/hw/dma/Makefile.objs
+++ b/hw/dma/Makefile.objs
@@ -11,3 +11,4 @@  common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o
 
 obj-$(CONFIG_OMAP) += omap_dma.o soc_dma.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_dma.o
+obj-$(CONFIG_RASPI) += bcm2835_dma.o
diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
new file mode 100644
index 0000000..1b3cdcf
--- /dev/null
+++ b/hw/dma/bcm2835_dma.c
@@ -0,0 +1,397 @@ 
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/dma/bcm2835_dma.h"
+
+/* DMA CS Control and Status bits */
+#define BCM2708_DMA_ACTIVE      (1 << 0)
+#define BCM2708_DMA_END         (1 << 1) /* GE */
+#define BCM2708_DMA_INT         (1 << 2)
+#define BCM2708_DMA_ISPAUSED    (1 << 4)  /* Pause requested or not active */
+#define BCM2708_DMA_ISHELD      (1 << 5)  /* Is held by DREQ flow control */
+#define BCM2708_DMA_ERR         (1 << 8)
+#define BCM2708_DMA_ABORT       (1 << 30) /* stop current CB, go to next, WO */
+#define BCM2708_DMA_RESET       (1 << 31) /* WO, self clearing */
+
+/* DMA control block "info" field bits */
+#define BCM2708_DMA_INT_EN      (1 << 0)
+#define BCM2708_DMA_TDMODE      (1 << 1)
+#define BCM2708_DMA_WAIT_RESP   (1 << 3)
+#define BCM2708_DMA_D_INC       (1 << 4)
+#define BCM2708_DMA_D_WIDTH     (1 << 5)
+#define BCM2708_DMA_D_DREQ      (1 << 6)
+#define BCM2708_DMA_D_IGNORE    (1 << 7)
+#define BCM2708_DMA_S_INC       (1 << 8)
+#define BCM2708_DMA_S_WIDTH     (1 << 9)
+#define BCM2708_DMA_S_DREQ      (1 << 10)
+#define BCM2708_DMA_S_IGNORE    (1 << 11)
+
+/* Register offsets */
+#define BCM2708_DMA_CS          0x00 /* Control and Status */
+#define BCM2708_DMA_ADDR        0x04 /* Control block address */
+/* the current control block appears in the following registers - read only */
+#define BCM2708_DMA_INFO        0x08
+#define BCM2708_DMA_SOURCE_AD   0x0c
+#define BCM2708_DMA_DEST_AD     0x10
+#define BCM2708_DMA_TXFR_LEN    0x14
+#define BCM2708_DMA_STRIDE      0x18
+#define BCM2708_DMA_NEXTCB      0x1C
+#define BCM2708_DMA_DEBUG       0x20
+
+#define BCM2708_DMA_INT_STATUS  0xfe0 /* Interrupt status of each channel */
+#define BCM2708_DMA_ENABLE      0xff0 /* Global enable bits for each channel */
+
+#define BCM2708_DMA_CS_RW_MASK  0x30ff0001 /* All RW bits in DMA_CS */
+
+static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
+{
+    BCM2835DMAChan *ch = &s->chan[c];
+    uint32_t data, xlen, ylen;
+    int16_t dst_stride, src_stride;
+
+    if (!(s->enable & (1 << c))) {
+        return;
+    }
+
+    while ((s->enable & (1 << c)) && (ch->conblk_ad != 0)) {
+        /* CB fetch */
+        ch->ti = ldl_phys(&s->dma_as, ch->conblk_ad);
+        ch->source_ad = ldl_phys(&s->dma_as, ch->conblk_ad + 4);
+        ch->dest_ad = ldl_phys(&s->dma_as, ch->conblk_ad + 8);
+        ch->txfr_len = ldl_phys(&s->dma_as, ch->conblk_ad + 12);
+        ch->stride = ldl_phys(&s->dma_as, ch->conblk_ad + 16);
+        ch->nextconbk = ldl_phys(&s->dma_as, ch->conblk_ad + 20);
+
+        if (ch->ti & BCM2708_DMA_TDMODE) {
+            /* 2D transfer mode */
+            ylen = (ch->txfr_len >> 16) & 0x3fff;
+            xlen = ch->txfr_len & 0xffff;
+            dst_stride = ch->stride >> 16;
+            src_stride = ch->stride & 0xffff;
+        } else {
+            ylen = 1;
+            xlen = ch->txfr_len;
+            dst_stride = 0;
+            src_stride = 0;
+        }
+
+        while (ylen != 0) {
+            /* Normal transfer mode */
+            while (xlen != 0) {
+                if (ch->ti & BCM2708_DMA_S_IGNORE) {
+                    /* Ignore reads */
+                    data = 0;
+                } else {
+                    data = ldl_phys(&s->dma_as, ch->source_ad);
+                }
+                if (ch->ti & BCM2708_DMA_S_INC) {
+                    ch->source_ad += 4;
+                }
+
+                if (ch->ti & BCM2708_DMA_D_IGNORE) {
+                    /* Ignore writes */
+                } else {
+                    stl_phys(&s->dma_as, ch->dest_ad, data);
+                }
+                if (ch->ti & BCM2708_DMA_D_INC) {
+                    ch->dest_ad += 4;
+                }
+
+                /* update remaining transfer length */
+                xlen -= 4;
+                if (ch->ti & BCM2708_DMA_TDMODE) {
+                    ch->txfr_len = (ylen << 16) | xlen;
+                } else {
+                    ch->txfr_len = xlen;
+                }
+            }
+
+            if (--ylen != 0) {
+                ch->source_ad += src_stride;
+                ch->dest_ad += dst_stride;
+            }
+        }
+        ch->cs |= BCM2708_DMA_END;
+        if (ch->ti & BCM2708_DMA_INT_EN) {
+            ch->cs |= BCM2708_DMA_INT;
+            s->int_status |= (1 << c);
+            qemu_set_irq(ch->irq, 1);
+        }
+
+        /* Process next CB */
+        ch->conblk_ad = ch->nextconbk;
+    }
+    ch->cs &= ~BCM2708_DMA_ACTIVE;
+}
+
+static uint64_t bcm2835_dma_read(BCM2835DMAState *s, hwaddr offset,
+                                 unsigned size, unsigned c)
+{
+    BCM2835DMAChan *ch = &s->chan[c];
+    uint32_t res = 0;
+
+    assert(size == 4);
+    assert(c < BCM2835_DMA_NCHANS);
+
+    switch (offset) {
+    case BCM2708_DMA_CS:
+        res = ch->cs;
+        break;
+    case BCM2708_DMA_ADDR:
+        res = ch->conblk_ad;
+        break;
+    case BCM2708_DMA_INFO:
+        res = ch->ti;
+        break;
+    case BCM2708_DMA_SOURCE_AD:
+        res = ch->source_ad;
+        break;
+    case BCM2708_DMA_DEST_AD:
+        res = ch->dest_ad;
+        break;
+    case BCM2708_DMA_TXFR_LEN:
+        res = ch->txfr_len;
+        break;
+    case BCM2708_DMA_STRIDE:
+        res = ch->stride;
+        break;
+    case BCM2708_DMA_NEXTCB:
+        res = ch->nextconbk;
+        break;
+    case BCM2708_DMA_DEBUG:
+        res = ch->debug;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
+                      __func__, offset);
+        break;
+    }
+    return res;
+}
+
+static void bcm2835_dma_write(BCM2835DMAState *s, hwaddr offset,
+                              uint64_t value, unsigned size, unsigned c)
+{
+    BCM2835DMAChan *ch = &s->chan[c];
+    uint32_t oldcs;
+
+    assert(size == 4);
+    assert(c < BCM2835_DMA_NCHANS);
+
+    switch (offset) {
+    case BCM2708_DMA_CS:
+        oldcs = ch->cs;
+        if (value & BCM2708_DMA_RESET) {
+            ch->cs |= BCM2708_DMA_RESET;
+        }
+        if (value & BCM2708_DMA_ABORT) {
+            ch->cs |= BCM2708_DMA_ABORT;
+        }
+        if (value & BCM2708_DMA_END) {
+            ch->cs &= ~BCM2708_DMA_END;
+        }
+        if (value & BCM2708_DMA_INT) {
+            ch->cs &= ~BCM2708_DMA_INT;
+            s->int_status &= ~(1 << c);
+            qemu_set_irq(ch->irq, 0);
+        }
+        ch->cs &= ~BCM2708_DMA_CS_RW_MASK;
+        ch->cs |= (value & BCM2708_DMA_CS_RW_MASK);
+        if (!(oldcs & BCM2708_DMA_ACTIVE) && (ch->cs & BCM2708_DMA_ACTIVE)) {
+            bcm2835_dma_update(s, c);
+        }
+        break;
+    case BCM2708_DMA_ADDR:
+        ch->conblk_ad = value;
+        break;
+    case BCM2708_DMA_DEBUG:
+        ch->debug = value;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
+                      __func__, offset);
+        break;
+    }
+}
+
+static uint64_t bcm2835_dma0_read(void *opaque, hwaddr offset, unsigned size)
+{
+    BCM2835DMAState *s = opaque;
+
+    if (offset < 0xf00) {
+        return bcm2835_dma_read(s, (offset & 0xff), size, (offset >> 8) & 0xf);
+    } else {
+        switch (offset) {
+        case BCM2708_DMA_INT_STATUS:
+            return s->int_status;
+        case BCM2708_DMA_ENABLE:
+            return s->enable;
+        default:
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
+                          __func__, offset);
+            return 0;
+        }
+    }
+}
+
+static uint64_t bcm2835_dma15_read(void *opaque, hwaddr offset, unsigned size)
+{
+    return bcm2835_dma_read(opaque, (offset & 0xff), size, 15);
+}
+
+static void bcm2835_dma0_write(void *opaque, hwaddr offset, uint64_t value,
+                               unsigned size)
+{
+    BCM2835DMAState *s = opaque;
+
+    if (offset < 0xf00) {
+        bcm2835_dma_write(s, (offset & 0xff), value, size, (offset >> 8) & 0xf);
+    } else {
+        switch (offset) {
+        case BCM2708_DMA_INT_STATUS:
+            break;
+        case BCM2708_DMA_ENABLE:
+            s->enable = (value & 0xffff);
+            break;
+        default:
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
+                          __func__, offset);
+        }
+    }
+
+}
+
+static void bcm2835_dma15_write(void *opaque, hwaddr offset, uint64_t value,
+                                unsigned size)
+{
+    bcm2835_dma_write(opaque, (offset & 0xff), value, size, 15);
+}
+
+static const MemoryRegionOps bcm2835_dma0_ops = {
+    .read = bcm2835_dma0_read,
+    .write = bcm2835_dma0_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+};
+
+static const MemoryRegionOps bcm2835_dma15_ops = {
+    .read = bcm2835_dma15_read,
+    .write = bcm2835_dma15_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+};
+
+static const VMStateDescription vmstate_bcm2835_dma_chan = {
+    .name = TYPE_BCM2835_DMA "-chan",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(cs, BCM2835DMAChan),
+        VMSTATE_UINT32(conblk_ad, BCM2835DMAChan),
+        VMSTATE_UINT32(ti, BCM2835DMAChan),
+        VMSTATE_UINT32(source_ad, BCM2835DMAChan),
+        VMSTATE_UINT32(dest_ad, BCM2835DMAChan),
+        VMSTATE_UINT32(txfr_len, BCM2835DMAChan),
+        VMSTATE_UINT32(stride, BCM2835DMAChan),
+        VMSTATE_UINT32(nextconbk, BCM2835DMAChan),
+        VMSTATE_UINT32(debug, BCM2835DMAChan),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_bcm2835_dma = {
+    .name = TYPE_BCM2835_DMA,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT_ARRAY(chan, BCM2835DMAState, BCM2835_DMA_NCHANS, 1,
+                             vmstate_bcm2835_dma_chan, BCM2835DMAChan),
+        VMSTATE_UINT32(int_status, BCM2835DMAState),
+        VMSTATE_UINT32(enable, BCM2835DMAState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void bcm2835_dma_init(Object *obj)
+{
+    BCM2835DMAState *s = BCM2835_DMA(obj);
+    int n;
+
+    /* DMA channels 0-14 occupy a contiguous block of IO memory, along
+     * with the global enable and interrupt status bits. Channel 15
+     * has the same register map, but is mapped at a discontiguous
+     * address in a separate IO block.
+     */
+    memory_region_init_io(&s->iomem0, OBJECT(s), &bcm2835_dma0_ops, s,
+                          TYPE_BCM2835_DMA, 0x1000);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem0);
+
+    memory_region_init_io(&s->iomem15, OBJECT(s), &bcm2835_dma15_ops, s,
+                          TYPE_BCM2835_DMA "-chan15", 0x100);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem15);
+
+    for (n = 0; n < 16; n++) {
+        sysbus_init_irq(SYS_BUS_DEVICE(s), &s->chan[n].irq);
+    }
+}
+
+static void bcm2835_dma_reset(DeviceState *dev)
+{
+    BCM2835DMAState *s = BCM2835_DMA(dev);
+    int n;
+
+    s->enable = 0xffff;
+    s->int_status = 0;
+    for (n = 0; n < BCM2835_DMA_NCHANS; n++) {
+        s->chan[n].cs = 0;
+        s->chan[n].conblk_ad = 0;
+    }
+}
+
+static void bcm2835_dma_realize(DeviceState *dev, Error **errp)
+{
+    BCM2835DMAState *s = BCM2835_DMA(dev);
+    Error *err = NULL;
+    Object *obj;
+
+    obj = object_property_get_link(OBJECT(dev), "dma-mr", &err);
+    if (obj == NULL) {
+        error_setg(errp, "%s: required dma-mr link not found: %s",
+                   __func__, error_get_pretty(err));
+        return;
+    }
+
+    s->dma_mr = MEMORY_REGION(obj);
+    address_space_init(&s->dma_as, s->dma_mr, NULL);
+
+    bcm2835_dma_reset(dev);
+}
+
+static void bcm2835_dma_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = bcm2835_dma_realize;
+    dc->reset = bcm2835_dma_reset;
+    dc->vmsd = &vmstate_bcm2835_dma;
+}
+
+static TypeInfo bcm2835_dma_info = {
+    .name          = TYPE_BCM2835_DMA,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(BCM2835DMAState),
+    .class_init    = bcm2835_dma_class_init,
+    .instance_init = bcm2835_dma_init,
+};
+
+static void bcm2835_dma_register_types(void)
+{
+    type_register_static(&bcm2835_dma_info);
+}
+
+type_init(bcm2835_dma_register_types)
diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
index e19d360..e12ae37 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -16,6 +16,7 @@ 
 #include "hw/sysbus.h"
 #include "hw/char/bcm2835_aux.h"
 #include "hw/display/bcm2835_fb.h"
+#include "hw/dma/bcm2835_dma.h"
 #include "hw/intc/bcm2835_ic.h"
 #include "hw/misc/bcm2835_property.h"
 #include "hw/misc/bcm2835_mbox.h"
@@ -37,6 +38,7 @@  typedef struct BCM2835PeripheralState {
     SysBusDevice *uart0;
     BCM2835AuxState aux;
     BCM2835FBState fb;
+    BCM2835DMAState dma;
     BCM2835ICState ic;
     BCM2835PropertyState property;
     BCM2835MboxState mboxes;
diff --git a/include/hw/dma/bcm2835_dma.h b/include/hw/dma/bcm2835_dma.h
new file mode 100644
index 0000000..75312e2
--- /dev/null
+++ b/include/hw/dma/bcm2835_dma.h
@@ -0,0 +1,47 @@ 
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2835_DMA_H
+#define BCM2835_DMA_H
+
+#include "qemu-common.h"
+#include "exec/address-spaces.h"
+#include "hw/sysbus.h"
+
+typedef struct {
+    uint32_t cs;
+    uint32_t conblk_ad;
+    uint32_t ti;
+    uint32_t source_ad;
+    uint32_t dest_ad;
+    uint32_t txfr_len;
+    uint32_t stride;
+    uint32_t nextconbk;
+    uint32_t debug;
+
+    qemu_irq irq;
+} BCM2835DMAChan;
+
+#define TYPE_BCM2835_DMA "bcm2835-dma"
+#define BCM2835_DMA(obj) \
+        OBJECT_CHECK(BCM2835DMAState, (obj), TYPE_BCM2835_DMA)
+
+#define BCM2835_DMA_NCHANS 16
+
+typedef struct {
+    /*< private >*/
+    SysBusDevice busdev;
+    /*< public >*/
+
+    MemoryRegion iomem0, iomem15;
+    MemoryRegion *dma_mr;
+    AddressSpace dma_as;
+
+    BCM2835DMAChan chan[BCM2835_DMA_NCHANS];
+    uint32_t int_status;
+    uint32_t enable;
+} BCM2835DMAState;
+
+#endif