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

when guest writes ggtt entries, it could write 8 bytes a time if
gtt_entry_size is 8. But, qemu could split the 8 bytes 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 and to spot partial write as early as possible, we
don't keep this information for every ggtt index. Instread, 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 characteristic of ggtt entry which
stores memory address. When gtt_entry_size is 8, the guest memory physical
address should be 64 bits, so any sane guest driver should write 8-byte
long data at a time, so 2 consecutive 4-byte writes at the same ggtt index
should be trapped in gvt.

v2:
when incomplete ggtt entry write is located, e.g.
    1. guest only writes 4 bytes at a ggtt offset and no long writes the
       rest 4 bytes.
    2. guest writes 4 bytes of a ggtt offset, then write at other ggtt
       offsets, then return back to write the left 4 bytes of the first
       ggtt offset.
add error handling logic to remap host entry to scratch page, and mark
guest virtual ggtt entry as not present.  (zhenyu wang)

Signed-off-by: Zhao Yan <yan.y.zhao@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 2329654..4efec8f 100644 (file)
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1592,6 +1592,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;
 }
@@ -1616,6 +1617,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);
@@ -1868,6 +1870,62 @@ 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 {
+                       int last_offset;
+
+                       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);
+
+                       /* set host ggtt entry to scratch page and clear
+                        * virtual ggtt entry as not present for last
+                        * partially write offset
+                        */
+                       last_offset = ggtt_mm->ggtt_mm.last_partial_off &
+                                       (~(info->gtt_entry_size - 1));
+
+                       ggtt_get_host_entry(ggtt_mm, &m, last_offset);
+                       ggtt_invalidate_pte(vgpu, &m);
+                       ops->set_pfn(&m, gvt->gtt.scratch_mfn);
+                       ops->clear_present(&m);
+                       ggtt_set_host_entry(ggtt_mm, &m, last_offset);
+                       ggtt_invalidate(gvt->dev_priv);
+
+                       ggtt_get_guest_entry(ggtt_mm, &e, last_offset);
+                       ops->clear_present(&e);
+                       ggtt_set_guest_entry(ggtt_mm, &e, last_offset);
+
+                       ggtt_mm->ggtt_mm.last_partial_off = off;
+                       ggtt_mm->ggtt_mm.last_partial_data = e.val64;
+
+                       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 (file)
--- 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;
        };
 };