[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 03/13] ide: Split out BMDMA code from ATA core
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] Re: [PATCH 03/13] ide: Split out BMDMA code from ATA core |
Date: |
Wed, 08 Dec 2010 15:46:50 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10 |
Am 08.12.2010 15:40, schrieb Stefan Hajnoczi:
> On Wed, Dec 8, 2010 at 2:35 PM, Kevin Wolf <address@hidden> wrote:
>> 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.
>
> 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
[Qemu-devel] [PATCH 09/13] ahci: add ahci emulation, Alexander Graf, 2010/12/08