Re: [Qemu-devel] [PATCH v2 03/20] arm: add Faraday FTAPBBRG020 APB DMA support

[Paul Brook]

> The FTAPBBRG020 supports the DMA functions for the AHB-to-AHB,
> AHB-to-APB, APB-to-AHB, and APB-to-APB transactions.

All the timer code in this file looks suspect.  As a general rule everything 
should be event driven and complete immediately (or at least schedule a BH for 
immediate action if recursion is a concern), not relying on a periodic timer 
interrupts.

> +        qemu_mod_timer(s->qtimer,
> +            qemu_get_clock_ns(vm_clock) + 1);

For all practical purposes this is going to happen immediately, so you should 
not be using a timer.

> +        qemu_mod_timer(s->qtimer,
> +            qemu_get_clock_ns(vm_clock) + (get_ticks_per_sec() >> 2));

Why 0.25 seconds?  Usually this sort of try-again-soon behavior means you've 
missed a trigger event somewhere else.

> +    if (!cpu_physical_memory_is_io(c->src)) {
> +        src_map = src_ptr = cpu_physical_memory_map(c->src, &src_len, 0);
> +    }
> +    if (!cpu_physical_memory_is_io(c->dst)) {
> +        dst_map = dst_ptr = cpu_physical_memory_map(c->dst, &dst_len, 1);
> +    }

cpu_physical_memory_map might not map the whole region you requested.  This 
will cause badness in the subsequent code.


I suspect a log of this code anc and should be shared with your other DMA 
controller, and probably several of the existing DMA controllers.

Paul