| Message ID | 1334554162-18540-1-git-send-email-zhihuili@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 16, 2012 at 6:29 AM, Li Zhi Hui <zhihuili@linux.vnet.ibm.com> wrote: > add function DMA_set_return and delete bh_schedule in dma.c Please explain the reason for these code changes in the commit description. > > Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com> > --- > hw/dma.c | 21 ++++++++++++++------- > hw/isa.h | 1 + > 2 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/hw/dma.c b/hw/dma.c > index 0a9322d..48e153a 100644 > --- a/hw/dma.c > +++ b/hw/dma.c > @@ -357,12 +357,10 @@ static void DMA_run (void) > { > struct dma_cont *d; > int icont, ichan; > - int rearm = 0; Why are you removing the rearm behavior? I'm pretty sure other ISA devices rely on this otherwise the code wouldn't exist. Stefan
Il 16/04/2012 14:14, Stefan Hajnoczi ha scritto: > Why are you removing the rearm behavior? I'm pretty sure other ISA > devices rely on this otherwise the code wouldn't exist. Li is correct in that the code can be simplified a lot, but indeed his axe went a bit too far. :) With the exception of the floppy all other DMA devices do not reenter DMA_run, and do not need the idle bottom half hack because DMA is done once per interrupt. And now that the floppy is asynchronous it doesn't need the idle bottom half hack, either. So DMA_run can call channel_run from a while loop like this: while ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) { // pseudo-code, channel_running and channel_is_asynchronous // would be fields in dma_regs if (channel_running[ichan]) { assert(channel_is_asynchronous[ichan]); break; } channel_running[ichan]++; channel_run (icont, ichan); // channel_is_asynchronous would be set by a new function // DMA_register_channel_async. if (channel_is_asynchronous[ichan]) { break; } channel_running[ichan]--; } and DMA_set_return needs to do a tail call to "complete" the while loop: assert(channel_is_asynchronous[ichan]); assert(channel_running[ichan] == 1); channel_running[ichan]--; if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) { channel_run (icont, ichan); } Paolo
On 2012年04月17日 16:46, Paolo Bonzini wrote: > Il 16/04/2012 14:14, Stefan Hajnoczi ha scritto: >> Why are you removing the rearm behavior? I'm pretty sure other ISA >> devices rely on this otherwise the code wouldn't exist. > > Li is correct in that the code can be simplified a lot, but indeed his > axe went a bit too far. :) > > With the exception of the floppy all other DMA devices do not reenter > DMA_run, and do not need the idle bottom half hack because DMA is done > once per interrupt. And now that the floppy is asynchronous it doesn't > need the idle bottom half hack, either. > > So DMA_run can call channel_run from a while loop like this: > > while ((0 == (d->mask& mask))&& (0 != (d->status& (mask<< 4)))) { > // pseudo-code, channel_running and channel_is_asynchronous > // would be fields in dma_regs > if (channel_running[ichan]) { > assert(channel_is_asynchronous[ichan]); > break; > } > > channel_running[ichan]++; > channel_run (icont, ichan); > > // channel_is_asynchronous would be set by a new function > // DMA_register_channel_async. > if (channel_is_asynchronous[ichan]) { > break; > } > channel_running[ichan]--; > } > > and DMA_set_return needs to do a tail call to "complete" the while loop: > > assert(channel_is_asynchronous[ichan]); > assert(channel_running[ichan] == 1); > channel_running[ichan]--; > if ((0 == (d->mask& mask))&& (0 != (d->status& (mask<< 4)))) { > channel_run (icont, ichan); > } > > Paolo > Thank you very much for giving me suggestions, I have tried the method, it works well. But I also have another idea, I modify fdc.c by adding a flag to avoid re-enter, I have tested it. I will send a patch later. If you have any comments, please give me your opinion. thanks.
diff --git a/hw/dma.c b/hw/dma.c index 0a9322d..48e153a 100644 --- a/hw/dma.c +++ b/hw/dma.c @@ -357,12 +357,10 @@ static void DMA_run (void) { struct dma_cont *d; int icont, ichan; - int rearm = 0; static int running = 0; if (running) { - rearm = 1; - goto out; + return; } else { running = 1; } @@ -377,15 +375,11 @@ static void DMA_run (void) if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) { channel_run (icont, ichan); - rearm = 1; } } } running = 0; -out: - if (rearm) - qemu_bh_schedule_idle(dma_bh); } static void DMA_run_bh(void *unused) @@ -460,6 +454,19 @@ void DMA_schedule(int nchan) qemu_irq_pulse(*d->cpu_request_exit); } +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; +} + static void dma_reset(void *opaque) { struct dma_cont *d = opaque; diff --git a/hw/isa.h b/hw/isa.h index 40373fb..277f54c 100644 --- a/hw/isa.h +++ b/hw/isa.h @@ -92,4 +92,5 @@ void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit); void DMA_register_channel (int nchan, DMA_transfer_handler transfer_handler, void *opaque); +void DMA_set_return(int nret, int nchan); #endif
add function DMA_set_return and delete bh_schedule in dma.c Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com> --- hw/dma.c | 21 ++++++++++++++------- hw/isa.h | 1 + 2 files changed, 15 insertions(+), 7 deletions(-)