[Qemu-devel] Re: [PATCH 03/13] ide: Split out BMDMA code from ATA core

[Kevin Wolf]

Am 08.12.2010 15:26, schrieb Stefan Hajnoczi:
> On Wed, Dec 8, 2010 at 12:13 PM, Alexander Graf <address@hidden> 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.

Kevin