[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Possibility of unaligned DMA accesses via the QEMU DMA
From: |
Mark Cave-Ayland |
Subject: |
Re: [Qemu-devel] Possibility of unaligned DMA accesses via the QEMU DMA API? |
Date: |
Mon, 22 Jul 2013 09:04:46 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130116 Icedove/10.0.12 |
On 18/07/13 14:44, Alexander Graf wrote:
What I would do, however, is to complete even the INPUT/OUTPUT_MORE
commands only at the end of the whole request. This is definitely
allowed behaviour, and it ensures that a memory region isn't already
reused by the OS while e.g. a write request is still running and taking
data from this memory. We should only complete the DMA command as
soon as we don't touch the memory any more.
Yes, that's the version that I described as "throw away almost all of today's code
and rewrite it" :).
Keep in mind that the same DMA controller can be used for Ethernet, so coupling
it very tightly with
> IDE doesn't sound overly appealing to me either.
I had a go at implementing this over yesterday afternoon and I managed
to come up with something that boots both Linux and Darwin. I've
uploaded it to github in order for you to take a look at here:
https://github.com/mcayland/qemu/tree/mca-macio.
On the plus side, the code seems a lot more readable; but on the
downside my heisenbug is *still* present, even in this version of the
code (see more below). Does the revised version look good enough to you
both? If so, I'll tidy it up a bit further and post it to the qemu-devel
list.
In order to ensure that things aren't too tightly coupled to IDE, I've
introduced a new ready() function pointer which is called from the
modified DBDMA code. Even though it's currently not used for anything
other than IDE in QEMU, I think this would enable the same controller to
be used with Ethernet if required.
A couple of stylistic points for the patch:
1) Can I swap m->aiocb for s->bus->dma->aiocb? This seems to be the
field used by the internal IDE core.
2) Should the bdrv_acct_start() calls be removed from
pmac_ide_transfer()? It looks as if they are already handled in
ide_sector_start_dma() and ide_atapi_cmd_read_dma().
Now back to the heisenbug: for Kevin's benefit, the reason I started
looking at this is because I have a bug with Alex's existing patches in
that Aurenlien's test Linux PPC images fails to boot with DMA timeout
errors (see http://www.ilande.co.uk/tmp/bad-ppc-linux.png). Sadly if I
try and debug it (by compiling with -O0 -g or adding debug printf()
statements) then everything works as normal :/
On the plus side, with my revised version I can trigger the bug fairly
easily by commenting out DBDMA_kick in ide_dbdma_start() and enabling
DEBUG_IDE_ATAPI in hw/ide/internal.h. Note that while the timeout occurs
on the CDROM for the majority of time, I've still seen it very
occasionally on the HD device too.
With DEBUG_IDE_ATAPI enabled the tail of the debug output I can get
without the bug disappearing looks like this:
reply: tx_size=36 elem_tx_size=0 index=0
byte_count_limit=36
status=0x58
reply: tx_size=0 elem_tx_size=0 index=36
status=0x50
ATAPI limit=0x8 packet: 46 00 00 00 00 00 00 00 08 00 00 00
reply: tx_size=8 elem_tx_size=0 index=0
byte_count_limit=8
status=0x58
reply: tx_size=0 elem_tx_size=0 index=8
status=0x50
ATAPI limit=0x14 packet: 46 01 00 00 00 00 00 00 14 00 00 00
reply: tx_size=20 elem_tx_size=0 index=0
byte_count_limit=20
status=0x58
reply: tx_size=0 elem_tx_size=0 index=20
status=0x50
ATAPI limit=0xc packet: 43 00 00 00 00 00 01 00 0c 00 00 00
reply: tx_size=12 elem_tx_size=0 index=0
byte_count_limit=12
status=0x58
reply: tx_size=0 elem_tx_size=0 index=12
status=0x50
ATAPI limit=0x14 packet: 43 00 00 00 00 00 01 00 14 00 00 00
reply: tx_size=20 elem_tx_size=0 index=0
byte_count_limit=20
status=0x58
reply: tx_size=0 elem_tx_size=0 index=20
status=0x50
ATAPI limit=0xc packet: 43 00 01 00 00 00 00 00 0c 00 00 00
reply: tx_size=12 elem_tx_size=0 index=0
byte_count_limit=12
status=0x58
reply: tx_size=0 elem_tx_size=0 index=12
status=0x50
ATAPI limit=0x20 packet: 51 00 00 00 00 00 00 00 20 00 00 00
Data ready for CD device: 20 bytes
When the bug appears, the guest doesn't bother to program the DMA
controller to finish the transfer which makes me wonder if one of the
IDE status flags is not being cleared correctly? Sadly if I enable
DEBUG_IDE then of the course the delay introduced makes everything work
again :/
Interestingly enough, the final 0x51 command packet above has s->lba set
to -1. Given that the code tries to use s->lba as the DMA sector number,
this is definitely a bug - what should we actually be doing in this case?
ATB,
Mark.