From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=55218 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PQL6Z-0001uA-Rw
	for qemu-devel@nongnu.org; Wed, 08 Dec 2010 09:34:52 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1PQL6Y-000613-Ka
	for qemu-devel@nongnu.org; Wed, 08 Dec 2010 09:34:51 -0500
Received: from mx1.redhat.com ([209.132.183.28]:37710)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1PQL6Y-00060s-Cj
	for qemu-devel@nongnu.org; Wed, 08 Dec 2010 09:34:50 -0500
Message-ID: <4CFF97BF.4090901@redhat.com>
Date: Wed, 08 Dec 2010 15:35:43 +0100
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <1291810400-11309-1-git-send-email-agraf@suse.de>	<1291810400-11309-4-git-send-email-agraf@suse.de>
	<AANLkTinGLCbvcrF2dWNNzYDnJvgfiw58G8QCZEUM00uA@mail.gmail.com>
In-Reply-To: <AANLkTinGLCbvcrF2dWNNzYDnJvgfiw58G8QCZEUM00uA@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH 03/13] ide: Split out BMDMA code from ATA
	core
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Joerg Roedel <Joerg.Roedel@amd.com>, Paul Brook <paul@codesourcery.com>, Alexander Graf <agraf@suse.de>, QEMU-devel Developers <qemu-devel@nongnu.org>, Blue Swirl <blauwirbel@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, tj@kernel.org, Sebastian Herbszt <herbszt@gmx.de>, Roland Elek <elek.roland@gmail.com>

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

Kevin