[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v2] icount: make dma reads deterministic
From: |
Pavel Dovgalyuk |
Subject: |
RE: [PATCH v2] icount: make dma reads deterministic |
Date: |
Wed, 11 Mar 2020 15:42:48 +0300 |
Ping.
Pavel Dovgalyuk
> -----Original Message-----
> From: Pavel Dovgalyuk [mailto:address@hidden]
> Sent: Tuesday, March 03, 2020 3:27 PM
> To: address@hidden
> Cc: address@hidden; address@hidden; address@hidden; address@hidden;
> address@hidden
> Subject: [PATCH v2] icount: make dma reads deterministic
>
> Windows guest sometimes makes DMA requests with overlapping
> target addresses. This leads to the following structure of iov for
> the block driver:
>
> addr size1
> addr size2
> addr size3
>
> It means that three adjacent disk blocks should be read into the same
> memory buffer. Windows does not expects anything from these bytes
> (should it be data from the first block, or the last one, or some mix),
> but uses them somehow. It leads to non-determinism of the guest execution,
> because block driver does not preserve any order of reading.
>
> This situation was discusses in the mailing list at least twice:
> https://lists.gnu.org/archive/html/qemu-devel/2010-09/msg01996.html
> https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg05185.html
>
> This patch makes such disk reads deterministic in icount mode.
> It splits the whole request into several parts. Parts may overlap,
> but SGs inside one part do not overlap.
> Parts that are processed later overwrite the prior ones in case
> of overlapping.
>
> Examples for different SG part sequences:
>
> 1)
> A1 1000
> A2 1000
> A1 1000
> A3 1000
> ->
> One request is split into two.
> A1 1000
> A2 1000
> --
> A1 1000
> A3 1000
>
> 2)
> A1 800
> A2 1000
> A1 1000
> ->
> A1 800
> A2 1000
> --
> A1 1000
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
>
> --
>
> v2:
> - Rewritten the loop to split the request instead of skipping the parts
> (suggested by Kevin Wolf)
> ---
> dma-helpers.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/dma-helpers.c b/dma-helpers.c
> index e8a26e81e1..959e114595 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -13,6 +13,8 @@
> #include "trace-root.h"
> #include "qemu/thread.h"
> #include "qemu/main-loop.h"
> +#include "sysemu/cpus.h"
> +#include "qemu/range.h"
>
> /* #define DEBUG_IOMMU */
>
> @@ -142,6 +144,23 @@ static void dma_blk_cb(void *opaque, int ret)
> cur_addr = dbs->sg->sg[dbs->sg_cur_index].base + dbs->sg_cur_byte;
> cur_len = dbs->sg->sg[dbs->sg_cur_index].len - dbs->sg_cur_byte;
> mem = dma_memory_map(dbs->sg->as, cur_addr, &cur_len, dbs->dir);
> + /*
> + * Make reads deterministic in icount mode. Windows sometimes issues
> + * disk read requests with overlapping SGs. It leads
> + * to non-determinism, because resulting buffer contents may be mixed
> + * from several sectors. This code splits all SGs into several
> + * groups. SGs in every group do not overlap.
> + */
> + if (use_icount && dbs->dir == DMA_DIRECTION_FROM_DEVICE) {
> + int i;
> + for (i = 0 ; i < dbs->iov.niov ; ++i) {
> + if (ranges_overlap((intptr_t)dbs->iov.iov[i].iov_base, dbs-
> >iov.iov[i].iov_len,
> + (intptr_t)mem, cur_len)) {
> + mem = NULL;
> + break;
> + }
> + }
> + }
> if (!mem)
> break;
> qemu_iovec_add(&dbs->iov, mem, cur_len);