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