[PATCH 0/9] omap_dma: avoid non-bounds-checked memcopy

[PATCH 0/9] omap_dma: avoid non-bounds-checked memcopy

[Peter Maydell]

The omap_dma device is the only user of the soc_dma code.  This code
has a fastpath for when DMA transfers are from RAM to RAM.  The
current implementation of this has the caller of
soc_dma_port_add_mem() pass the underlying host address of the RAM
block that the DMA port is connected to (obtained via
memory_region_get_ram_ptr()).  Then the actual transfer function does
a simple memcpy().  This has several problems.
    
Most importantly, no bounds checking is done on the address and size
passed by the guest, so the memcpy source and destination might be
outside the backing host RAM entirely.  Secondly, because the DMA
access is done via this back door, there is no updating of the dirty
region when memory is written this way.

The better way to handle memory-to-memory DMA is to use
physical_memory_map(), because this will tell you if the address/size
you asked for would run off the edge of the RAM block, and it handles
updating dirty bits on write.

There are also various places where the omap_dma code is a bit
cavalier about possible integer overflows if the guest sets a
very high element and frame count for a transfer.

This series fixes both these problems, thus resolving
https://gitlab.com/qemu-project/qemu/-/work_items/3204
In the process it also simplifies the soc_dma code by removing
the unused "FIFO port" handling, so we don't have to check or
update that code.

thanks
-- PMM

Peter Maydell (9):
  hw/dma/soc_dma: Remove soc_dma_port_fifo support
  hw/dma/soc_dma: Simplify soc_dma_ch_update()
  hw/dma/soc_dma: Remove union from memmap_entry_s struct
  hw/dma/omap_dma: Be more careful about overflow in transfer setup
  hw/dma/soc_dma: dma bytes is uint64_t
  hw/dma/soc_dma: Use physical_memory_map() for mem2mem transfers
  hw/dma/soc_dma: Remove unused mem.base, paddr fields
  include/hw/arm/omap_dma.h: Move to include/hw/dma
  MAINTAINERS: Add soc_dma to OMAP section

 MAINTAINERS                       |   2 +
 hw/arm/omap1.c                    |   8 +-
 hw/dma/omap_dma.c                 |  37 +++---
 hw/dma/soc_dma.c                  | 196 ++++++++++--------------------
 include/hw/{arm => dma}/soc_dma.h |  33 ++---
 5 files changed, 97 insertions(+), 179 deletions(-)
 rename include/hw/{arm => dma}/soc_dma.h (70%)

-- 
2.43.0

[PATCH 1/9] hw/dma/soc_dma: Remove soc_dma_port_fifo support

[Peter Maydell]

Our current single OMAP SoC doesn't call the soc_dma_port_add_fifo(),
soc_dma_port_add_fifo_in() or soc_dma_port_add_fifo_out() functions.
Remove them, plus the soc_dma_port_fifo handling that only those
functions needed.

The motivation for this is that it removes a lot of code that is
careless about the fact that the largest possible DMA transfer is
more bits than will fit into an "int" variable, and which does direct
accesses to host memory pointers into guest backing RAM without doing
bounds checks.  Deleting this code means we don't have to audit and
update it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/dma/soc_dma.c         | 105 +--------------------------------------
 include/hw/arm/soc_dma.h |  24 ++-------
 2 files changed, 5 insertions(+), 124 deletions(-)

diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index d5c52b804f..8ba531feea 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -29,33 +29,6 @@ static void transfer_mem2mem(struct soc_dma_ch_s *ch)
     ch->paddr[1] += ch->bytes;
 }
 
-static void transfer_mem2fifo(struct soc_dma_ch_s *ch)
-{
-    ch->io_fn[1](ch->io_opaque[1], ch->paddr[0], ch->bytes);
-    ch->paddr[0] += ch->bytes;
-}
-
-static void transfer_fifo2mem(struct soc_dma_ch_s *ch)
-{
-    ch->io_fn[0](ch->io_opaque[0], ch->paddr[1], ch->bytes);
-    ch->paddr[1] += ch->bytes;
-}
-
-/* This is further optimisable but isn't very important because often
- * DMA peripherals forbid this kind of transfers and even when they don't,
- * oprating systems may not need to use them.  */
-static void *fifo_buf;
-static int fifo_size;
-static void transfer_fifo2fifo(struct soc_dma_ch_s *ch)
-{
-    if (ch->bytes > fifo_size)
-        fifo_buf = g_realloc(fifo_buf, fifo_size = ch->bytes);
-
-    /* Implement as transfer_fifo2linear + transfer_linear2fifo.  */
-    ch->io_fn[0](ch->io_opaque[0], fifo_buf, ch->bytes);
-    ch->io_fn[1](ch->io_opaque[1], fifo_buf, ch->bytes);
-}
-
 struct dma_s {
     struct soc_dma_s soc;
     int chnum;
@@ -67,11 +40,6 @@ struct dma_s {
         enum soc_dma_port_type type;
         hwaddr addr;
         union {
-           struct {
-               void *opaque;
-               soc_dma_io_t fn;
-               int out;
-           } fifo;
            struct {
                void *base;
                size_t size;
@@ -129,20 +97,7 @@ static inline enum soc_dma_port_type soc_dma_ch_update_type(
     struct dma_s *dma = (struct dma_s *) ch->dma;
     struct memmap_entry_s *entry = soc_dma_lookup(dma, ch->vaddr[port]);
 
-    if (entry->type == soc_dma_port_fifo) {
-        while (entry < dma->memmap + dma->memmap_size &&
-                        entry->u.fifo.out != port)
-            entry ++;
-        if (entry->addr != ch->vaddr[port] || entry->u.fifo.out != port)
-            return soc_dma_port_other;
-
-        if (ch->type[port] != soc_dma_access_const)
-            return soc_dma_port_other;
-
-        ch->io_fn[port] = entry->u.fifo.fn;
-        ch->io_opaque[port] = entry->u.fifo.opaque;
-        return soc_dma_port_fifo;
-    } else if (entry->type == soc_dma_port_mem) {
+    if (entry->type == soc_dma_port_mem) {
         if (entry->addr > ch->vaddr[port] ||
                         entry->addr + entry->u.mem.size <= ch->vaddr[port])
             return soc_dma_port_other;
@@ -173,15 +128,8 @@ void soc_dma_ch_update(struct soc_dma_ch_s *ch)
     }
     dst = soc_dma_ch_update_type(ch, 1);
 
-    /* TODO: use src and dst as array indices.  */
     if (src == soc_dma_port_mem && dst == soc_dma_port_mem)
         ch->transfer_fn = transfer_mem2mem;
-    else if (src == soc_dma_port_mem && dst == soc_dma_port_fifo)
-        ch->transfer_fn = transfer_mem2fifo;
-    else if (src == soc_dma_port_fifo && dst == soc_dma_port_mem)
-        ch->transfer_fn = transfer_fifo2mem;
-    else if (src == soc_dma_port_fifo && dst == soc_dma_port_fifo)
-        ch->transfer_fn = transfer_fifo2fifo;
     else
         ch->transfer_fn = ch->dma->transfer_fn;
 
@@ -251,61 +199,10 @@ struct soc_dma_s *soc_dma_init(int n)
     }
 
     soc_dma_reset(&s->soc);
-    fifo_size = 0;
 
     return &s->soc;
 }
 
-void soc_dma_port_add_fifo(struct soc_dma_s *soc, hwaddr virt_base,
-                soc_dma_io_t fn, void *opaque, int out)
-{
-    struct memmap_entry_s *entry;
-    struct dma_s *dma = (struct dma_s *) soc;
-
-    dma->memmap = g_realloc(dma->memmap, sizeof(*entry) *
-                    (dma->memmap_size + 1));
-    entry = soc_dma_lookup(dma, virt_base);
-
-    if (dma->memmap_size) {
-        if (entry->type == soc_dma_port_mem) {
-            if (entry->addr <= virt_base &&
-                            entry->addr + entry->u.mem.size > virt_base) {
-                error_report("%s: FIFO at %"PRIx64
-                             " collides with RAM region at %"PRIx64
-                             "-%"PRIx64, __func__,
-                             virt_base, entry->addr,
-                             (entry->addr + entry->u.mem.size));
-                exit(-1);
-            }
-
-            if (entry->addr <= virt_base)
-                entry ++;
-        } else
-            while (entry < dma->memmap + dma->memmap_size &&
-                            entry->addr <= virt_base) {
-                if (entry->addr == virt_base && entry->u.fifo.out == out) {
-                    error_report("%s: FIFO at %"PRIx64
-                                 " collides FIFO at %"PRIx64,
-                                 __func__, virt_base, entry->addr);
-                    exit(-1);
-                }
-
-                entry ++;
-            }
-
-        memmove(entry + 1, entry,
-                        (uint8_t *) (dma->memmap + dma->memmap_size ++) -
-                        (uint8_t *) entry);
-    } else
-        dma->memmap_size ++;
-
-    entry->addr          = virt_base;
-    entry->type          = soc_dma_port_fifo;
-    entry->u.fifo.fn     = fn;
-    entry->u.fifo.opaque = opaque;
-    entry->u.fifo.out    = out;
-}
-
 void soc_dma_port_add_mem(struct soc_dma_s *soc, uint8_t *phys_base,
                 hwaddr virt_base, size_t size)
 {
diff --git a/include/hw/arm/soc_dma.h b/include/hw/arm/soc_dma.h
index bcdb91425a..e1d75bb3ba 100644
--- a/include/hw/arm/soc_dma.h
+++ b/include/hw/arm/soc_dma.h
@@ -25,12 +25,10 @@
 
 struct soc_dma_s;
 struct soc_dma_ch_s;
-typedef void (*soc_dma_io_t)(void *opaque, uint8_t *buf, int len);
 typedef void (*soc_dma_transfer_t)(struct soc_dma_ch_s *ch);
 
 enum soc_dma_port_type {
     soc_dma_port_mem,
-    soc_dma_port_fifo,
     soc_dma_port_other,
 };
 
@@ -57,8 +55,6 @@ struct soc_dma_ch_s {
     hwaddr vaddr[2];    /* Updated by .transfer_fn().  */
     /* Private */
     void *paddr[2];
-    soc_dma_io_t io_fn[2];
-    void *io_opaque[2];
 
     int running;
     soc_dma_transfer_t transfer_fn;
@@ -82,33 +78,21 @@ struct soc_dma_s {
 
 /* Call to activate or stop a DMA channel.  */
 void soc_dma_set_request(struct soc_dma_ch_s *ch, int level);
-/* Call after every write to one of the following fields and before
+/*
+ * Call after every write to one of the following fields and before
  * calling soc_dma_set_request(ch, 1):
  *   ch->type[0...1],
  *   ch->vaddr[0...1],
  *   ch->paddr[0...1],
- * or after a soc_dma_port_add_fifo() or soc_dma_port_add_mem().  */
+ * or after a soc_dma_port_add_mem().
+ */
 void soc_dma_ch_update(struct soc_dma_ch_s *ch);
 
 /* The SoC should call this when the DMA module is being reset.  */
 void soc_dma_reset(struct soc_dma_s *s);
 struct soc_dma_s *soc_dma_init(int n);
 
-void soc_dma_port_add_fifo(struct soc_dma_s *dma, hwaddr virt_base,
-                soc_dma_io_t fn, void *opaque, int out);
 void soc_dma_port_add_mem(struct soc_dma_s *dma, uint8_t *phys_base,
                 hwaddr virt_base, size_t size);
 
-static inline void soc_dma_port_add_fifo_in(struct soc_dma_s *dma,
-                hwaddr virt_base, soc_dma_io_t fn, void *opaque)
-{
-    return soc_dma_port_add_fifo(dma, virt_base, fn, opaque, 0);
-}
-
-static inline void soc_dma_port_add_fifo_out(struct soc_dma_s *dma,
-                hwaddr virt_base, soc_dma_io_t fn, void *opaque)
-{
-    return soc_dma_port_add_fifo(dma, virt_base, fn, opaque, 1);
-}
-
 #endif
-- 
2.43.0

[PATCH 2/9] hw/dma/soc_dma: Simplify soc_dma_ch_update()

[Peter Maydell]

Now we only have "mem" and "other" as soc_dma_port_type values, we
can simplify soc_dma_ch_update(): either both src and dst are mem, in
which case we use transfer_mem2mem and set update to 1 to tell
omap_dma_transfer_setup() to update all the guest-visible
src/dest/count information to indicate a completed transfer; or else
we use the omap_dma_transfer_generic() function, and we set update to
0 to tell omap_dma_transfer_setup() that the transfer function will
be updating the src/dest/count.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/dma/soc_dma.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index 8ba531feea..ff51338388 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -121,19 +121,14 @@ void soc_dma_ch_update(struct soc_dma_ch_s *ch)
     enum soc_dma_port_type src, dst;
 
     src = soc_dma_ch_update_type(ch, 0);
-    if (src == soc_dma_port_other) {
+    dst = soc_dma_ch_update_type(ch, 1);
+    if (src == soc_dma_port_other || dst == soc_dma_port_other) {
         ch->update = 0;
         ch->transfer_fn = ch->dma->transfer_fn;
-        return;
-    }
-    dst = soc_dma_ch_update_type(ch, 1);
-
-    if (src == soc_dma_port_mem && dst == soc_dma_port_mem)
+    } else {
+        ch->update = 1;
         ch->transfer_fn = transfer_mem2mem;
-    else
-        ch->transfer_fn = ch->dma->transfer_fn;
-
-    ch->update = (dst != soc_dma_port_other);
+    }
 }
 
 static void soc_dma_ch_freq_update(struct dma_s *s)
-- 
2.43.0

[PATCH 3/9] hw/dma/soc_dma: Remove union from memmap_entry_s struct

[Peter Maydell]

There's only one field in the union inside memmap_entry_s now that
we've removed the soc_dma_port_fifo handling.  Simplify by removing
the union.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/dma/soc_dma.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index ff51338388..bbcd2bdcd5 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -39,12 +39,10 @@ struct dma_s {
     struct memmap_entry_s {
         enum soc_dma_port_type type;
         hwaddr addr;
-        union {
-           struct {
-               void *base;
-               size_t size;
-           } mem;
-        } u;
+        struct {
+            void *base;
+            size_t size;
+        } mem;
     } *memmap;
     int memmap_size;
 
@@ -99,7 +97,7 @@ static inline enum soc_dma_port_type soc_dma_ch_update_type(
 
     if (entry->type == soc_dma_port_mem) {
         if (entry->addr > ch->vaddr[port] ||
-                        entry->addr + entry->u.mem.size <= ch->vaddr[port])
+                        entry->addr + entry->mem.size <= ch->vaddr[port])
             return soc_dma_port_other;
 
         /* TODO: support constant memory address for source port as used for
@@ -107,7 +105,7 @@ static inline enum soc_dma_port_type soc_dma_ch_update_type(
         if (ch->type[port] != soc_dma_access_const)
             return soc_dma_port_other;
 
-        ch->paddr[port] = (uint8_t *) entry->u.mem.base +
+        ch->paddr[port] = (uint8_t *) entry->mem.base +
                 (ch->vaddr[port] - entry->addr);
         /* TODO: save bytes left to the end of the mapping somewhere so we
          * can check we're not reading beyond it.  */
@@ -212,12 +210,12 @@ void soc_dma_port_add_mem(struct soc_dma_s *soc, uint8_t *phys_base,
         if (entry->type == soc_dma_port_mem) {
             if ((entry->addr >= virt_base && entry->addr < virt_base + size) ||
                             (entry->addr <= virt_base &&
-                             entry->addr + entry->u.mem.size > virt_base)) {
+                             entry->addr + entry->mem.size > virt_base)) {
                 error_report("%s: RAM at %"PRIx64 "-%"PRIx64
                              " collides with RAM region at %"PRIx64
                              "-%"PRIx64, __func__,
                              virt_base, virt_base + size,
-                             entry->addr, entry->addr + entry->u.mem.size);
+                             entry->addr, entry->addr + entry->mem.size);
                 exit(-1);
             }
 
@@ -246,8 +244,8 @@ void soc_dma_port_add_mem(struct soc_dma_s *soc, uint8_t *phys_base,
 
     entry->addr          = virt_base;
     entry->type          = soc_dma_port_mem;
-    entry->u.mem.base    = phys_base;
-    entry->u.mem.size    = size;
+    entry->mem.base    = phys_base;
+    entry->mem.size    = size;
 }
 
 /* TODO: port removal for ports like PCMCIA memory */
-- 
2.43.0

[PATCH 4/9] hw/dma/omap_dma: Be more careful about overflow in transfer setup

[Peter Maydell]

In omap_dma_transfer_setup(), the maximum number of elements we can
transfer is 0xffff * 0xffff == 0xfffe0001 (because the max frame
count and max elements per frame are both 65535).  However, we store
total element counts in 'int' variables, and use INT_MAX as a "bigger
than any valid value" sentinel, and when performing arithmetic with
the total count of transferred elements we are not careful about
avoiding overflows. Fix these:

 - use uint32_t rather than int for the local variables tracking
   various element and frame counts
 - use UINT_MAX as our sentinel
 - calculate new packet, element and frame counter values using
   arithmetic on a local uint32_t, rather than doing it in-place
   on local variables that are only 'int' because the actual
   counter registers are 16 bits
 - use 64-bit arithmetic when calculating how much to advance the
   source and dest pointers and the total dma->bytes transferred

Note that since soc_dma_ch_s::bytes is only 'int' this can still
overflow; we'll fix that in a subsequent patch.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/dma/omap_dma.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
index 77f1441498..14b310bf40 100644
--- a/hw/dma/omap_dma.c
+++ b/hw/dma/omap_dma.c
@@ -391,7 +391,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
     struct omap_dma_reg_set_s *a;
     struct omap_dma_channel_s *ch = dma->opaque;
     struct omap_dma_s *s = dma->dma->opaque;
-    int frames, min_elems, elements[__omap_dma_intr_last];
+    uint32_t frames, min_elems, elements[__omap_dma_intr_last];
 
     a = &ch->active_set;
 
@@ -403,7 +403,14 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
                         __func__, dma->num);
     }
 
-    min_elems = INT_MAX;
+    /*
+     * The maximum frame count and maximum element count are both 0xffff,
+     * so our worst case possible number of elements to transfer is
+     * 0xffff * 0xffff == 0xfffe0001. We can therefore keep element
+     * counts in a uint32_t and use UINT_MAX as a sentinel value for
+     * "not set" / "condition does not occur".
+     */
+    min_elems = UINT_MAX;
 
     /* Check all the conditions that terminate the transfer starting
      * with those that can occur the soonest.  */
@@ -413,7 +420,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
         if (elements[id] < min_elems)   \
             min_elems = elements[id];   \
     } else              \
-        elements[id] = INT_MAX;
+        elements[id] = UINT_MAX;
 
     /* Elements */
     INTR_CHECK(
@@ -465,7 +472,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
                     (a->frames - a->frame - 1) * a->elements +
                     (a->elements - a->element))
 
-    dma->bytes = min_elems * ch->data_type;
+    dma->bytes = (uint64_t)min_elems * ch->data_type;
 
     /* Set appropriate interrupts and/or deactivate channels */
 
@@ -528,8 +535,9 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
 
     /* Update packet number */
     if (ch->fs && ch->bs) {
-        a->pck_element += min_elems;
-        a->pck_element %= a->pck_elements;
+        /* Can't overflow: worst case min_elems 0xFFFE0001 + element 0xFFFF */
+        uint32_t new_pck_element = a->pck_element + min_elems;
+        a->pck_element = new_pck_element % a->pck_elements;
     }
 
     /*
@@ -537,13 +545,15 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
      * can skip part of this.
      */
     if (dma->update) {
+        /* Can't overflow: worst case min_elems 0xFFFE0001 + element 0xFFFF */
+        uint32_t new_element = a->element + min_elems;
         a->element += min_elems;
 
-        frames = a->element / a->elements;
-        a->element = a->element % a->elements;
+        frames = new_element / a->elements;
+        a->element = new_element % a->elements;
         a->frame += frames;
-        a->src += min_elems * a->elem_delta[0] + frames * a->frame_delta[0];
-        a->dest += min_elems * a->elem_delta[1] + frames * a->frame_delta[1];
+        a->src += (uint64_t)min_elems * a->elem_delta[0] + frames * a->frame_delta[0];
+        a->dest += (uint64_t)min_elems * a->elem_delta[1] + frames * a->frame_delta[1];
 
         /* If the channel is async, update cpc */
         if (!ch->sync && frames) {
-- 
2.43.0

[PATCH 5/9] hw/dma/soc_dma: dma bytes is uint64_t

[Peter Maydell]

The worst case number of DMA bytes that omap_dma will ask us to
transfer is 0xffff * 0xffff * 4 == 0x3fff80004, which is slightly
larger than fits into a uint32_t.  Move the byte count to uint64_t,
and adjust code that passes it around to also use uint64_t.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/dma/soc_dma.c         | 6 +++++-
 include/hw/arm/soc_dma.h | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index bbcd2bdcd5..1b93aaf565 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -49,11 +49,15 @@ struct dma_s {
     struct soc_dma_ch_s ch[];
 };
 
-static void soc_dma_ch_schedule(struct soc_dma_ch_s *ch, int delay_bytes)
+static void soc_dma_ch_schedule(struct soc_dma_ch_s *ch, uint64_t delay_bytes)
 {
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     struct dma_s *dma = (struct dma_s *) ch->dma;
 
+    /*
+     * Worst case delay bytes is only slightly larger than fits into
+     * a 32-bit integer, so this won't overflow.
+     */
     timer_mod(ch->timer, now + delay_bytes / dma->channel_freq);
 }
 
diff --git a/include/hw/arm/soc_dma.h b/include/hw/arm/soc_dma.h
index e1d75bb3ba..b5ad9743be 100644
--- a/include/hw/arm/soc_dma.h
+++ b/include/hw/arm/soc_dma.h
@@ -49,7 +49,7 @@ struct soc_dma_ch_s {
     int update;
 
     /* This should be set by dma->setup_fn().  */
-    int bytes;
+    uint64_t bytes;
     /* Initialised by the DMA module, call soc_dma_ch_update after writing.  */
     enum soc_dma_access_type type[2];
     hwaddr vaddr[2];    /* Updated by .transfer_fn().  */
-- 
2.43.0

[PATCH 6/9] hw/dma/soc_dma: Use physical_memory_map() for mem2mem transfers

[Peter Maydell]

The soc_dma code has a fastpath for when DMA transfers are from RAM
to RAM.  The current implementation of this has the caller of
soc_dma_port_add_mem() pass the underlying host address of the RAM
block that the DMA port is connected to (obtained via
memory_region_get_ram_ptr()).  Then the actual transfer function does
a simple memcpy(). This has several problems.

Most importantly, no bounds checking is done on the address and size
passed by the guest, so the memcpy source and destination might be
outside the backing host RAM entirely.  Secondly, because the DMA
access is done via this back door, there is no updating of the dirty
region when memory is written this way (there is a TODO comment
in omap_dma.c noting this).

Fix both of these by making the memory to memory transfer function
use physical_memory_map() to get the host addresses for the memory
copy.  That function will automatically give us the bounds check that
we want and return a short length if the transfer would run off the
end of the RAM MemoryRegion it starts in.  Since the OMAP DMA
documentation states that it's a guest error to misprogram the
addresses so that they fall outside the range that is valid for the
particular DMA port being addressed and that this can result in guest
memory corruption , we don't need to loop for short transfers, but
can simply log them and continue.

Note that we don't need to update addresses or bytecount here in the
transfer function, because when soc_dma_ch_update() selects
transfer_mem2mem it also sets ch->update to 1, which tells the
omap_dma_transfer_setup() code that it is responsible for updating
all the guest visible fields to match "transfer completed".

(We use physical_memory_map() here to match the use of
physical_memory_read() and physical_memory_write() in omap_dma.c;
making the DMA controller use an explicit AddressSpace would be
a separate cleanup task.)

Together with the preceding commits that fixed some integer overflow
problems, this fixes the "guest can provoke a bad memcpy() operation"
reported in issue #3204.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3204
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/dma/omap_dma.c |  5 -----
 hw/dma/soc_dma.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
index 14b310bf40..1a3ae74726 100644
--- a/hw/dma/omap_dma.c
+++ b/hw/dma/omap_dma.c
@@ -559,11 +559,6 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
         if (!ch->sync && frames) {
             ch->cpc = a->dest & 0xffff;
         }
-
-        /*
-         * TODO: if the destination port is IMIF or EMIFF, set the dirty
-         * bits on it.
-         */
     }
 
     omap_dma_interrupts_update(s);
diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index 1b93aaf565..6e325122d9 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -20,13 +20,58 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
+#include "qemu/log.h"
+#include "system/physmem.h"
 #include "hw/arm/soc_dma.h"
 
 static void transfer_mem2mem(struct soc_dma_ch_s *ch)
 {
-    memcpy(ch->paddr[0], ch->paddr[1], ch->bytes);
-    ch->paddr[0] += ch->bytes;
-    ch->paddr[1] += ch->bytes;
+    /*
+     * Memory-to-memory transfer: do the whole thing in one go.  The
+     * hardware spec says that it is invalid to program the OMAP DMA
+     * controller with addresses that don't match the port (i.e. to
+     * ask for a transfer to/from a memory port with a physaddr that
+     * isn't within that port range) and that if you do then the
+     * transfer continues and memory can be corrupted.  So we can map
+     * both source and destination, and treat short mappings and
+     * failed mappings as a guest error.
+     */
+    hwaddr srclen = ch->bytes;
+    hwaddr dstlen = ch->bytes;
+    hwaddr srcaddr = ch->vaddr[0];
+    hwaddr dstaddr = ch->vaddr[1];
+    void *srcmem, *dstmem;
+    hwaddr xferlen = 0;
+
+    srcmem = physical_memory_map(srcaddr, &srclen, false);
+    if (!srcmem) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "soc_dma mem2mem transfer: could not map source; "
+                      "guest error programming source port/address\n");
+        return;
+    }
+
+    dstmem = physical_memory_map(dstaddr, &dstlen, true);
+    if (!dstmem) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "soc_dma mem2mem transfer: could not map destination; "
+                      "guest error programming destination port/address\n");
+        goto unmap_src;
+    }
+
+    xferlen = MIN(srclen, dstlen);
+    if (xferlen < ch->bytes) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "soc_dma mem2mem transfer: could not transfer all data; "
+                      "guest error programming src or destination addresses\n");
+        /* Continue to transfer whatever did fit in the port window */
+    }
+
+    memmove(dstmem, srcmem, xferlen);
+
+    physical_memory_unmap(dstmem, dstlen, true, xferlen);
+unmap_src:
+    physical_memory_unmap(srcmem, srclen, false, xferlen);
 }
 
 struct dma_s {
-- 
2.43.0

[PATCH 7/9] hw/dma/soc_dma: Remove unused mem.base, paddr fields

[Peter Maydell]

Now that transfer_mem2mem() uses physical_memory_map(), the
soc_dma_ch_s::paddr field is unused; remove it, and the code that set
it, and the memmap_entry_s::mem.base and the soc_dma_port_add_mem()
phys_base argument that were passing around host pointers to use for
setting paddr.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/omap1.c           | 6 ++----
 hw/dma/soc_dma.c         | 9 +--------
 include/hw/arm/soc_dma.h | 7 ++-----
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index 44f9dd67c3..9e739a0712 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -3799,10 +3799,8 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *dram,
     s->port[tipb_mpui].addr_valid = omap_validate_tipb_mpui_addr;
 
     /* Register SDRAM and SRAM DMA ports for fast transfers.  */
-    soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(dram),
-                         OMAP_EMIFF_BASE, s->sdram_size);
-    soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(&s->imif_ram),
-                         OMAP_IMIF_BASE, s->sram_size);
+    soc_dma_port_add_mem(s->dma, OMAP_EMIFF_BASE, s->sdram_size);
+    soc_dma_port_add_mem(s->dma, OMAP_IMIF_BASE, s->sram_size);
 
     s->timer[0] = omap_mpu_timer_init(system_memory, 0xfffec500,
                     qdev_get_gpio_in(s->ih[0], OMAP_INT_TIMER1),
diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index 6e325122d9..ff890149ee 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -85,7 +85,6 @@ struct dma_s {
         enum soc_dma_port_type type;
         hwaddr addr;
         struct {
-            void *base;
             size_t size;
         } mem;
     } *memmap;
@@ -154,10 +153,6 @@ static inline enum soc_dma_port_type soc_dma_ch_update_type(
         if (ch->type[port] != soc_dma_access_const)
             return soc_dma_port_other;
 
-        ch->paddr[port] = (uint8_t *) entry->mem.base +
-                (ch->vaddr[port] - entry->addr);
-        /* TODO: save bytes left to the end of the mapping somewhere so we
-         * can check we're not reading beyond it.  */
         return soc_dma_port_mem;
     } else
         return soc_dma_port_other;
@@ -245,8 +240,7 @@ struct soc_dma_s *soc_dma_init(int n)
     return &s->soc;
 }
 
-void soc_dma_port_add_mem(struct soc_dma_s *soc, uint8_t *phys_base,
-                hwaddr virt_base, size_t size)
+void soc_dma_port_add_mem(struct soc_dma_s *soc, hwaddr virt_base, size_t size)
 {
     struct memmap_entry_s *entry;
     struct dma_s *dma = (struct dma_s *) soc;
@@ -293,7 +287,6 @@ void soc_dma_port_add_mem(struct soc_dma_s *soc, uint8_t *phys_base,
 
     entry->addr          = virt_base;
     entry->type          = soc_dma_port_mem;
-    entry->mem.base    = phys_base;
     entry->mem.size    = size;
 }
 
diff --git a/include/hw/arm/soc_dma.h b/include/hw/arm/soc_dma.h
index b5ad9743be..fdae7a29c2 100644
--- a/include/hw/arm/soc_dma.h
+++ b/include/hw/arm/soc_dma.h
@@ -53,8 +53,6 @@ struct soc_dma_ch_s {
     /* Initialised by the DMA module, call soc_dma_ch_update after writing.  */
     enum soc_dma_access_type type[2];
     hwaddr vaddr[2];    /* Updated by .transfer_fn().  */
-    /* Private */
-    void *paddr[2];
 
     int running;
     soc_dma_transfer_t transfer_fn;
@@ -83,7 +81,6 @@ void soc_dma_set_request(struct soc_dma_ch_s *ch, int level);
  * calling soc_dma_set_request(ch, 1):
  *   ch->type[0...1],
  *   ch->vaddr[0...1],
- *   ch->paddr[0...1],
  * or after a soc_dma_port_add_mem().
  */
 void soc_dma_ch_update(struct soc_dma_ch_s *ch);
@@ -92,7 +89,7 @@ void soc_dma_ch_update(struct soc_dma_ch_s *ch);
 void soc_dma_reset(struct soc_dma_s *s);
 struct soc_dma_s *soc_dma_init(int n);
 
-void soc_dma_port_add_mem(struct soc_dma_s *dma, uint8_t *phys_base,
-                hwaddr virt_base, size_t size);
+void soc_dma_port_add_mem(struct soc_dma_s *dma,
+                          hwaddr virt_base, size_t size);
 
 #endif
-- 
2.43.0

[PATCH 8/9] include/hw/arm/omap_dma.h: Move to include/hw/dma

[Peter Maydell]

omap_dma.h is the header file for hw/dma/omap_dma.c; it fits better
to put it in include/hw/dma/ to match where we have the .c file.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/omap1.c                    | 2 +-
 hw/dma/omap_dma.c                 | 2 +-
 hw/dma/soc_dma.c                  | 2 +-
 include/hw/{arm => dma}/soc_dma.h | 0
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename include/hw/{arm => dma}/soc_dma.h (100%)

diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index 9e739a0712..0f78cbd410 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -32,7 +32,7 @@
 #include "hw/sd/sd.h"
 #include "system/blockdev.h"
 #include "system/system.h"
-#include "hw/arm/soc_dma.h"
+#include "hw/dma/soc_dma.h"
 #include "system/qtest.h"
 #include "system/reset.h"
 #include "system/runstate.h"
diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
index 1a3ae74726..16575e73b7 100644
--- a/hw/dma/omap_dma.c
+++ b/hw/dma/omap_dma.c
@@ -22,7 +22,7 @@
 #include "qemu/timer.h"
 #include "hw/arm/omap.h"
 #include "hw/core/irq.h"
-#include "hw/arm/soc_dma.h"
+#include "hw/dma/soc_dma.h"
 #include "system/physmem.h"
 
 struct omap_dma_channel_s {
diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index ff890149ee..4feb22e6ea 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -22,7 +22,7 @@
 #include "qemu/timer.h"
 #include "qemu/log.h"
 #include "system/physmem.h"
-#include "hw/arm/soc_dma.h"
+#include "hw/dma/soc_dma.h"
 
 static void transfer_mem2mem(struct soc_dma_ch_s *ch)
 {
diff --git a/include/hw/arm/soc_dma.h b/include/hw/dma/soc_dma.h
similarity index 100%
rename from include/hw/arm/soc_dma.h
rename to include/hw/dma/soc_dma.h
-- 
2.43.0

[PATCH 9/9] MAINTAINERS: Add soc_dma to OMAP section

[Peter Maydell]

The hw/dma/soc_dma.c code appears to have been written with the idea
that it abstracts out DMA transfer operations from the details of a
particular DMA controller device.  In practice, it's used only by the
omap_dma code and I would not today recommend trying to use it in any
new DMA device.  Add the files to the OMAP section of MAINTAINERS so
that patches can be cc'd to the appropriate places.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6171cc7494..bf1ef71f24 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2201,7 +2201,9 @@ M: Peter Maydell <peter.maydell@linaro.org>
 L: qemu-arm@nongnu.org
 S: Odd Fixes
 F: hw/*/omap*
+F: hw/dma/soc_dma.c
 F: include/hw/arm/omap.h
+F: include/hw/dma/soc_dma.h
 F: docs/system/arm/sx1.rst
 F: tests/functional/arm/test_sx1.py
 
-- 
2.43.0

Re: [PATCH 8/9] include/hw/arm/omap_dma.h: Move to include/hw/dma

[Philippe Mathieu-Daudé]

On 10/7/26 12:59, Peter Maydell wrote:
> omap_dma.h is the header file for hw/dma/omap_dma.c; it fits better
> to put it in include/hw/dma/ to match where we have the .c file.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/omap1.c                    | 2 +-
>   hw/dma/omap_dma.c                 | 2 +-
>   hw/dma/soc_dma.c                  | 2 +-
>   include/hw/{arm => dma}/soc_dma.h | 0
>   4 files changed, 3 insertions(+), 3 deletions(-)
>   rename include/hw/{arm => dma}/soc_dma.h (100%)

Per the cover letter, "The omap_dma device is the only user of
the soc_dma code", then maybe take the rename opportunity to
name as omap_dma.h? Anyhow:

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 8/9] include/hw/arm/omap_dma.h: Move to include/hw/dma

[Peter Maydell]

On Fri, 10 Jul 2026 at 14:37, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> On 10/7/26 12:59, Peter Maydell wrote:
> > omap_dma.h is the header file for hw/dma/omap_dma.c; it fits better
> > to put it in include/hw/dma/ to match where we have the .c file.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/arm/omap1.c                    | 2 +-
> >   hw/dma/omap_dma.c                 | 2 +-
> >   hw/dma/soc_dma.c                  | 2 +-
> >   include/hw/{arm => dma}/soc_dma.h | 0
> >   4 files changed, 3 insertions(+), 3 deletions(-)
> >   rename include/hw/{arm => dma}/soc_dma.h (100%)
>
> Per the cover letter, "The omap_dma device is the only user of
> the soc_dma code", then maybe take the rename opportunity to
> name as omap_dma.h?

It's the only user but it is still a separate C file, so
I think we should keep soc_dma.h to match the soc_dma.c filename.

-- PMM

Re: [PATCH 3/9] hw/dma/soc_dma: Remove union from memmap_entry_s struct

[Philippe Mathieu-Daudé]

On 10/7/26 12:59, Peter Maydell wrote:
> There's only one field in the union inside memmap_entry_s now that
> we've removed the soc_dma_port_fifo handling.  Simplify by removing
> the union.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/dma/soc_dma.c | 22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 1/9] hw/dma/soc_dma: Remove soc_dma_port_fifo support

[Philippe Mathieu-Daudé]

On 10/7/26 12:58, Peter Maydell wrote:
> Our current single OMAP SoC doesn't call the soc_dma_port_add_fifo(),
> soc_dma_port_add_fifo_in() or soc_dma_port_add_fifo_out() functions.
> Remove them, plus the soc_dma_port_fifo handling that only those
> functions needed.
> 
> The motivation for this is that it removes a lot of code that is
> careless about the fact that the largest possible DMA transfer is
> more bits than will fit into an "int" variable, and which does direct
> accesses to host memory pointers into guest backing RAM without doing
> bounds checks.  Deleting this code means we don't have to audit and
> update it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/dma/soc_dma.c         | 105 +--------------------------------------
>   include/hw/arm/soc_dma.h |  24 ++-------
>   2 files changed, 5 insertions(+), 124 deletions(-)

Nice.

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 6/9] hw/dma/soc_dma: Use physical_memory_map() for mem2mem transfers

[Philippe Mathieu-Daudé]

On 10/7/26 12:59, Peter Maydell wrote:
> The soc_dma code has a fastpath for when DMA transfers are from RAM
> to RAM.  The current implementation of this has the caller of
> soc_dma_port_add_mem() pass the underlying host address of the RAM
> block that the DMA port is connected to (obtained via
> memory_region_get_ram_ptr()).  Then the actual transfer function does
> a simple memcpy(). This has several problems.
> 
> Most importantly, no bounds checking is done on the address and size
> passed by the guest, so the memcpy source and destination might be
> outside the backing host RAM entirely.  Secondly, because the DMA
> access is done via this back door, there is no updating of the dirty
> region when memory is written this way (there is a TODO comment
> in omap_dma.c noting this).
> 
> Fix both of these by making the memory to memory transfer function
> use physical_memory_map() to get the host addresses for the memory
> copy.  That function will automatically give us the bounds check that
> we want and return a short length if the transfer would run off the
> end of the RAM MemoryRegion it starts in.  Since the OMAP DMA
> documentation states that it's a guest error to misprogram the
> addresses so that they fall outside the range that is valid for the
> particular DMA port being addressed and that this can result in guest
> memory corruption , we don't need to loop for short transfers, but
> can simply log them and continue.
> 
> Note that we don't need to update addresses or bytecount here in the
> transfer function, because when soc_dma_ch_update() selects
> transfer_mem2mem it also sets ch->update to 1, which tells the
> omap_dma_transfer_setup() code that it is responsible for updating
> all the guest visible fields to match "transfer completed".
> 
> (We use physical_memory_map() here to match the use of
> physical_memory_read() and physical_memory_write() in omap_dma.c;
> making the DMA controller use an explicit AddressSpace would be
> a separate cleanup task.)
> 
> Together with the preceding commits that fixed some integer overflow
> problems, this fixes the "guest can provoke a bad memcpy() operation"
> reported in issue #3204.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3204
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/dma/omap_dma.c |  5 -----
>   hw/dma/soc_dma.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++---
>   2 files changed, 48 insertions(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 7/9] hw/dma/soc_dma: Remove unused mem.base, paddr fields

[Philippe Mathieu-Daudé]

On 10/7/26 12:59, Peter Maydell wrote:
> Now that transfer_mem2mem() uses physical_memory_map(), the
> soc_dma_ch_s::paddr field is unused; remove it, and the code that set
> it, and the memmap_entry_s::mem.base and the soc_dma_port_add_mem()
> phys_base argument that were passing around host pointers to use for
> setting paddr.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/omap1.c           | 6 ++----
>   hw/dma/soc_dma.c         | 9 +--------
>   include/hw/arm/soc_dma.h | 7 ++-----
>   3 files changed, 5 insertions(+), 17 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 9/9] MAINTAINERS: Add soc_dma to OMAP section

[Philippe Mathieu-Daudé]

On 10/7/26 12:59, Peter Maydell wrote:
> The hw/dma/soc_dma.c code appears to have been written with the idea
> that it abstracts out DMA transfer operations from the details of a
> particular DMA controller device.  In practice, it's used only by the
> omap_dma code and I would not today recommend trying to use it in any
> new DMA device.  Add the files to the OMAP section of MAINTAINERS so
> that patches can be cc'd to the appropriate places.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   MAINTAINERS | 2 ++
>   1 file changed, 2 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 4/9] hw/dma/omap_dma: Be more careful about overflow in transfer setup

[Jim MacArthur]

On Fri, Jul 10, 2026 at 11:59:02AM +0100, Peter Maydell wrote:
> In omap_dma_transfer_setup(), the maximum number of elements we can
> transfer is 0xffff * 0xffff == 0xfffe0001 (because the max frame
> count and max elements per frame are both 65535).  However, we store
> total element counts in 'int' variables, and use INT_MAX as a "bigger
> than any valid value" sentinel, and when performing arithmetic with
> the total count of transferred elements we are not careful about
> avoiding overflows. Fix these:
> 
>  - use uint32_t rather than int for the local variables tracking
>    various element and frame counts
>  - use UINT_MAX as our sentinel
>  - calculate new packet, element and frame counter values using
>    arithmetic on a local uint32_t, rather than doing it in-place
>    on local variables that are only 'int' because the actual
>    counter registers are 16 bits
>  - use 64-bit arithmetic when calculating how much to advance the
>    source and dest pointers and the total dma->bytes transferred
> 
> Note that since soc_dma_ch_s::bytes is only 'int' this can still
> overflow; we'll fix that in a subsequent patch.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

LGTM, thanks Peter.

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

Re: [PATCH 5/9] hw/dma/soc_dma: dma bytes is uint64_t

[Jim MacArthur]

On Fri, Jul 10, 2026 at 11:59:03AM +0100, Peter Maydell wrote:
> The worst case number of DMA bytes that omap_dma will ask us to
> transfer is 0xffff * 0xffff * 4 == 0x3fff80004, which is slightly
> larger than fits into a uint32_t.  Move the byte count to uint64_t,
> and adjust code that passes it around to also use uint64_t.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/dma/soc_dma.c         | 6 +++++-
>  include/hw/arm/soc_dma.h | 2 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

Re: [PATCH 2/9] hw/dma/soc_dma: Simplify soc_dma_ch_update()

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Now we only have "mem" and "other" as soc_dma_port_type values, we
> can simplify soc_dma_ch_update(): either both src and dst are mem, in
> which case we use transfer_mem2mem and set update to 1 to tell
> omap_dma_transfer_setup() to update all the guest-visible
> src/dest/count information to indicate a completed transfer; or else
> we use the omap_dma_transfer_generic() function, and we set update to
> 0 to tell omap_dma_transfer_setup() that the transfer function will
> be updating the src/dest/count.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/dma/soc_dma.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
> index 8ba531feea..ff51338388 100644
> --- a/hw/dma/soc_dma.c
> +++ b/hw/dma/soc_dma.c
> @@ -121,19 +121,14 @@ void soc_dma_ch_update(struct soc_dma_ch_s *ch)
>      enum soc_dma_port_type src, dst;
>  
>      src = soc_dma_ch_update_type(ch, 0);
> -    if (src == soc_dma_port_other) {
> +    dst = soc_dma_ch_update_type(ch, 1);
> +    if (src == soc_dma_port_other || dst == soc_dma_port_other) {
>          ch->update = 0;
>          ch->transfer_fn = ch->dma->transfer_fn;
> -        return;
> -    }
> -    dst = soc_dma_ch_update_type(ch, 1);
> -
> -    if (src == soc_dma_port_mem && dst == soc_dma_port_mem)
> +    } else {
> +        ch->update = 1;
>          ch->transfer_fn = transfer_mem2mem;
> -    else
> -        ch->transfer_fn = ch->dma->transfer_fn;
> -
> -    ch->update = (dst != soc_dma_port_other);
> +    }
>  }

heh the diff is messy and the soc_dma_ch_update_type could do with some
clean-up but for this patch:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>  
>  static void soc_dma_ch_freq_update(struct dma_s *s)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro