On 25/05/2016 00:59, Mark Cave-Ayland wrote:
> I eventually traced the corruption down to this section of code in
> dma_blk_cb() which was incorrectly truncating the unaligned iovecs:
>
> if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
> qemu_iovec_discard_back(&dbs->iov, dbs->iov.size &
> ~BDRV_SECTOR_MASK);
> }
>
> This was introduced in
> http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg02236.html to
> handle non-sector aligned SG lists, although given that this is a
> restriction of one particular implementation (PRDT) I'm not sure whether
> a plain revert is the correct thing to do or whether an alternative
> solution needs to be found.
Yeah, I have a plan for this bit. It's related to this code I'm adding
in patch 7:
+ /* This is not supported yet. It can only happen if the guest does
+ * reads and writes that are not aligned to one logical sectors
+ * _and_ cover multiple MemoryRegions.
+ */
+ assert(offset % s->qdev.blocksize == 0);
+ assert(iov->size % s->qdev.blocksize == 0);
The idea behind the "if" is that the I/O code cannot deal with a number
of bytes that is not a multiple of the logical sector size. These
assertions could be removed by generalizing the "if" to an arbitrary
mask, in this case s->qdev.blocksize - 1.
There are two things that are wrong however in the logic. First, the
"if" must be moved before the "if (dbs->iov.size & ~BDRV_SECTOR_MASK)",
because the call to qemu_iovec_discard_back can result in a zero-byte
QEMUIOVector. Second, the sg_cur_* variables must be rewound too.
Thanks,
Paolo
