Re: [Qemu-devel] [PATCH 1/2 v5-test] add function DMA_set_return and DMA_set_channel_async in dma.c

[Paolo Bonzini]

Il 19/04/2012 16:07, Li Zhi Hui ha scritto:
> To avoid re-enter function fdctrl_transfer_handler, according paolo's 
> suggestion, the patch is used to test.
> 
> Signed-off-by: Li Zhi Hui <address@hidden>
> ---
>  hw/dma.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++--
>  hw/isa.h |    2 ++
>  2 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/dma.c b/hw/dma.c
> index 0a9322d..0a59164 100644
> --- a/hw/dma.c
> +++ b/hw/dma.c
> @@ -45,6 +45,8 @@ struct dma_regs {
>      uint8_t eop;
>      DMA_transfer_handler transfer_handler;
>      void *opaque;
> +    bool channel_running;
> +    bool channel_is_asynchronous;
>  };
>  
>  #define ADDR 0
> @@ -138,6 +140,8 @@ static inline void init_chan (struct dma_cont *d, int 
> ichan)
>      r = d->regs + ichan;
>      r->now[ADDR] = r->base[ADDR] << d->dshift;
>      r->now[COUNT] = 0;
> +    r->channel_running = false;
> +    r->channel_is_asynchronous = false;
>  }
>  
>  static inline int getff (struct dma_cont *d)
> @@ -375,9 +379,20 @@ static void DMA_run (void)
>  
>              mask = 1 << ichan;
>  
> -            if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) 
> {
> -                channel_run (icont, ichan);
> +            while ((0 == (d->mask & mask)) &&
> +                   (0 != (d->status & (mask << 4)))) {
> +                struct dma_regs *r = &dma_controllers[icont].regs[ichan];
> +                if (r->channel_running) {
> +                    assert(r->channel_is_asynchronous);
> +                    break;
> +                }
> +                r->channel_running = true;
> +                channel_run(icont, ichan);
>                  rearm = 1;
> +                if (r->channel_is_asynchronous) {
> +                    break;
> +                }
> +                r->channel_running = false;
>              }
>          }
>      }
> @@ -388,6 +403,35 @@ out:
>          qemu_bh_schedule_idle(dma_bh);
>  }
>  
> +void DMA_set_channel_async(int nchan, bool val)
> +{
> +    int icont, ichan;
> +    struct dma_regs *r;
> +
> +    icont = nchan > 3;
> +    ichan = nchan & 3;
> +    r = &dma_controllers[icont].regs[ichan];
> +    r->channel_is_asynchronous = val;
> +}
> +
> +void DMA_set_return(int nret, int nchan)
> +{
> +    struct dma_regs *r;
> +    struct dma_cont *d;
> +    int icont, ichan;
> +
> +    icont = nchan > 3;
> +    ichan = nchan & 3;
> +    d = dma_controllers;
> +    r = &d[icont].regs[ichan];
> +    r->now[COUNT] = nret;
> +    assert(r->channel_is_asynchronous);
> +    assert(r->channel_running);

Thanks, this is very much like what I had in mind, except that here I
would have called DMA_run.  Also you can then remove the bottom half
(and rearm logic) completely.

If calling DMA_run is not working, perhaps it is because you didn't
remove the bottom half.

I would hope that you can also change the "if (running) goto out;" to an
"assert(!running)", but I'm not so sure.

Paolo