Am 08.12.2010 15:40, schrieb Stefan Hajnoczi: > On Wed, Dec 8, 2010 at 2:35 PM, Kevin Wolf <kw...@redhat.com> wrote: >> Am 08.12.2010 15:26, schrieb Stefan Hajnoczi: >>> On Wed, Dec 8, 2010 at 12:13 PM, Alexander Graf <ag...@suse.de> wrote: >>>> @@ -486,8 +440,8 @@ void ide_dma_error(IDEState *s) >>>> ide_transfer_stop(s); >>>> s->error = ABRT_ERR; >>>> s->status = READY_STAT | ERR_STAT; >>>> - ide_dma_set_inactive(s->bus->bmdma); >>>> - s->bus->bmdma->status |= BM_STATUS_INT; >>>> + ide_set_inactive(s); >>>> + s->bus->dma.ops->set_status(s->bus->dma.opaque, BM_STATUS_INT); >>> >>> Is BM_STATUS_INT constant naming appropriate for a general DMA >>> abstraction? Perhaps DMA_STATUS_INT. >> >> BM_STATUS_INT is a bit in the status register of busmaster IDE. So in >> theory it shouldn't appear in generic ATA code, but I'm not sure how >> much of this we can fix at this point. >> >>> Instead of fleshing out these functions, how about initializing >>> dma.ops to NULL? The program crashes should anyone try to do DMA >>> before setting a real IDEDMAOps pointer. That's not as robust as >>> limping along with non-working IDE, but should be straightforward to >>> debug if it ever happens. It also requires less code. >> >> Allowing the guest to crash qemu is not an option. We'd have to check >> for NULL in all commands that initiate a DMA transfer. > > You're right, I wasn't aware that the ops gets a chance to execute > before we initialize them to BMDMA.
For example with ISA we never intialize it at all. Kevin
