2013/1/26 Paul Brook <p...@codesourcery.com>

> > 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.
>

Agree,
I've cheated here, because I'm too lazy to checkout how to use mutex/thread
in QEMU,
I'll fix it later.


>
> > +        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.
>
>
Yes,
it's because I'm using timer instead of mutext/thread here,
so there might be some race conditions, and thus I added this stupid
work-around here.


> > +    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.
>
>
My bad, I forgot this, I'll fix it later.


>
> 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
>



-- 
Best wishes,
Kuo-Jung Su


2013/1/26 Paul Brook <p...@codesourcery.com>

> > 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
>



-- 
Best wishes,
Kuo-Jung Su

Reply via email to