[PATCH] drm/i915/gvt: fix a bug of partially write ggtt enties

Zhao Yan yan.y.zhao at intel.com
Fri Jun 8 08:09:48 UTC 2018


when guest writes ggtt entries, it could write 8 bytes a time if
gtt_entry_size is 8. But, when vfio traps the writes, it could be split
into 2 consecutive 4-byte writes.

If each 4-byte partial write could trigger a host ggtt write, it is very
possible that a wrong combination is written to the host ggtt. E.g.
the higher 4 bytes is the old value, but the lower 4 bytes is the new
value, and this 8-byte combination is wrong but written to the ggtt, thus
causing bugs.
To handle this condition, we just record the first 4-byte write, then wait
until the second 4-byte write comes and write the combined 64-bit data to
host ggtt table.
To save memory space, we don't keep this information for every ggtt index,
we just record the last ggtt write position, and assume the two 4-byte
writes come in consecutively for each vgpu.
This assumption is right based on the fact that if gtt_entry_size is 8,
the guest memory physical address should be 64 bits, so when writing an
8-byte address into guest ggtt, 2 consecutive 4-byte writes at the same
ggtt index should be trapped.

Signed-off-by: Zhao Yan <yan.y.zhao at intel.com>
---
 drivers/gpu/drm/i915/gvt/gtt.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/gtt.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 78e55aa..8be8c38 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1591,6 +1591,7 @@ static struct intel_vgpu_mm *intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
 		vgpu_free_mm(mm);
 		return ERR_PTR(-ENOMEM);
 	}
+	mm->ggtt_mm.last_partial_off = -1UL;
 
 	return mm;
 }
@@ -1615,6 +1616,7 @@ void _intel_vgpu_mm_release(struct kref *mm_ref)
 		invalidate_ppgtt_mm(mm);
 	} else {
 		vfree(mm->ggtt_mm.virtual_ggtt);
+		mm->ggtt_mm.last_partial_off = -1UL;
 	}
 
 	vgpu_free_mm(mm);
@@ -1867,6 +1869,38 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
 	memcpy((void *)&e.val64 + (off & (info->gtt_entry_size - 1)), p_data,
 			bytes);
 
+	/* If ggtt entry size is 8 bytes, and it's split into two 4 bytes
+	 * write, we assume the two 4 bytes writes are consecutive.
+	 * Otherwise, we abort and report error
+	 */
+	if (bytes < info->gtt_entry_size) {
+		if (ggtt_mm->ggtt_mm.last_partial_off == -1UL) {
+			/* the first partial part*/
+			ggtt_mm->ggtt_mm.last_partial_off = off;
+			ggtt_mm->ggtt_mm.last_partial_data = e.val64;
+			return 0;
+		} else if ((g_gtt_index ==
+				(ggtt_mm->ggtt_mm.last_partial_off >>
+				info->gtt_entry_size_shift)) &&
+			(off !=	ggtt_mm->ggtt_mm.last_partial_off)) {
+			/* the second partial part */
+
+			int last_off = ggtt_mm->ggtt_mm.last_partial_off &
+				(info->gtt_entry_size - 1);
+
+			memcpy((void *)&e.val64 + last_off,
+				(void *)&ggtt_mm->ggtt_mm.last_partial_data +
+				last_off, bytes);
+
+			ggtt_mm->ggtt_mm.last_partial_off = -1UL;
+		} else {
+			gvt_vgpu_err("failed to populate guest ggtt entry: abnormal ggtt entry write sequence, last_partial_off=%lx, offset=%x, bytes=%d, ggtt entry size=%d\n",
+					ggtt_mm->ggtt_mm.last_partial_off, off,
+					bytes, info->gtt_entry_size);
+			return 0;
+		}
+	}
+
 	if (ops->test_present(&e)) {
 		gfn = ops->get_pfn(&e);
 		m = e;
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index 3792f2b..97e6264 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -150,6 +150,8 @@ struct intel_vgpu_mm {
 		} ppgtt_mm;
 		struct {
 			void *virtual_ggtt;
+			unsigned long last_partial_off;
+			u64 last_partial_data;
 		} ggtt_mm;
 	};
 };
-- 
1.9.1



More information about the intel-gvt-dev mailing list