[PATCH] drm/i915/gvt: support inconsecutive partial PTE update
Zhenyu Wang
zhenyuw at linux.intel.com
Mon Sep 17 02:58:09 UTC 2018
On 2018.09.14 19:08:25 +0800, hang.yuan at linux.intel.com wrote:
> From: Hang Yuan <hang.yuan at linux.intel.com>
>
> Previously we assumed two 4-byte writes to the same PTE coming in sequence.
> But recently we observed inconsecutive partial write happening as well. So
> this patch enhances the previous solution. It now uses a list to save more
> partial writes. If one partial write can be combined with another one in
> the list to construct a full PTE, update its shadow entry. Otherwise, save
> the partial write in the list.
>
> This patch also update ggtt_mmio_write more to remove unnecessary virtual
> PTE read and add old shadow PTE invalidation on PTE update case.
>
> v2: invalidate old entry and flush ggtt (Zhenyu)
>
> Signed-off-by: Hang Yuan <hang.yuan at linux.intel.com>
> Cc: Yan Zhao <yan.y.zhao at intel.com>
> Cc: Xiaolin Zhang <xiaolin.zhang at intel.com>
> Cc: Zhenyu Wang <zhenyu.z.wang at intel.com>
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 119 +++++++++++++++++++++--------------------
> drivers/gpu/drm/i915/gvt/gtt.h | 9 +++-
> 2 files changed, 69 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 2402395..cf11f8f 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1905,7 +1905,6 @@ 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;
> }
> @@ -1930,7 +1929,6 @@ 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);
> @@ -2168,6 +2166,8 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
> struct intel_gvt_gtt_entry e, m;
> dma_addr_t dma_addr;
> int ret;
> + struct intel_gvt_partial_pte *partial_pte, *pos, *n;
> + bool partial_update = false;
>
> if (bytes != 4 && bytes != 8)
> return -EINVAL;
> @@ -2178,68 +2178,61 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
> if (!vgpu_gmadr_is_valid(vgpu, gma))
> return 0;
>
> - ggtt_get_guest_entry(ggtt_mm, &e, g_gtt_index);
> -
> + e.type = GTT_TYPE_GGTT_PTE;
> 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
> + * write, save the first 4 bytes in a list and update virtual
> + * PTE. Only update shadow PTE when the second 4 bytes comes.
> */
> 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;
> + bool found = false;
> +
> + list_for_each_entry_safe(pos, n,
> + &ggtt_mm->ggtt_mm.partial_pte_list, list) {
> + if (g_gtt_index == pos->offset >>
> + info->gtt_entry_size_shift) {
> + if (off != pos->offset) {
> + /* the second partial part*/
> +
> + int last_off = pos->offset &
> + (info->gtt_entry_size - 1);
> +
> + memcpy((void *)&e.val64 + last_off,
> + (void *)&pos->data + last_off,
> + bytes);
> +
> + list_del(&pos->list);
> + kfree(pos);
> + } else {
> + /* update of the first partial part */
> +
> + pos->data = e.val64;
> + ggtt_set_guest_entry(ggtt_mm, &e,
> + g_gtt_index);
> + return 0;
> + }
> + found = true;
> + break;
Move this two lines in above if (off != pos->offset) is easier to read.
> + }
> + }
>
> - return 0;
> + if (!found) {
> + /* the first partial part */
> +
> + partial_pte = kzalloc(sizeof(*partial_pte), GFP_KERNEL);
> + if (!partial_pte)
> + return -ENOMEM;
> + partial_pte->offset = off;
> + partial_pte->data = e.val64;
> + list_add_tail(&partial_pte->list,
> + &ggtt_mm->ggtt_mm.partial_pte_list);
> + partial_update = true;
> }
> }
>
> - if (ops->test_present(&e)) {
> + if (!partial_update && (ops->test_present(&e))) {
> gfn = ops->get_pfn(&e);
> m = e;
>
> @@ -2263,16 +2256,18 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
> } else
> ops->set_pfn(&m, dma_addr >> PAGE_SHIFT);
> } else {
> - ggtt_get_host_entry(ggtt_mm, &m, g_gtt_index);
> - ggtt_invalidate_pte(vgpu, &m);
> ops->set_pfn(&m, gvt->gtt.scratch_mfn);
> ops->clear_present(&m);
> }
>
> out:
> + ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index);
> +
> + ggtt_get_host_entry(ggtt_mm, &e, g_gtt_index);
> + ggtt_invalidate_pte(vgpu, &e);
For normal 8b size update of ggtt entry, does this invalidate of old pte page
always required? If yes, looks better split this as a separate fix instead.
> +
> ggtt_set_host_entry(ggtt_mm, &m, g_gtt_index);
> ggtt_invalidate(gvt->dev_priv);
> - ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index);
> return 0;
> }
>
> @@ -2430,6 +2425,8 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu)
>
> intel_vgpu_reset_ggtt(vgpu, false);
>
> + INIT_LIST_HEAD(>t->ggtt_mm->ggtt_mm.partial_pte_list);
> +
> return create_scratch_page_tree(vgpu);
> }
>
> @@ -2454,6 +2451,14 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu)
>
> static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu)
> {
> + struct intel_gvt_partial_pte *pos;
> +
> + list_for_each_entry(pos,
> + &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, list) {
> + gvt_dbg_mm("partial PTE update on hold 0x%lx : 0x%llx\n",
> + pos->offset, pos->data);
> + kfree(pos);
> + }
> intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
> vgpu->gtt.ggtt_mm = NULL;
> }
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
> index 7a9b361..a11bfee 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.h
> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> @@ -133,6 +133,12 @@ enum intel_gvt_mm_type {
>
> #define GVT_RING_CTX_NR_PDPS GEN8_3LVL_PDPES
>
> +struct intel_gvt_partial_pte {
> + unsigned long offset;
> + u64 data;
> + struct list_head list;
> +};
> +
> struct intel_vgpu_mm {
> enum intel_gvt_mm_type type;
> struct intel_vgpu *vgpu;
> @@ -157,8 +163,7 @@ struct intel_vgpu_mm {
> } ppgtt_mm;
> struct {
> void *virtual_ggtt;
> - unsigned long last_partial_off;
> - u64 last_partial_data;
> + struct list_head partial_pte_list;
> } ggtt_mm;
> };
> };
> --
> 2.7.4
>
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20180917/f7ee334a/attachment.sig>
More information about the intel-gvt-dev
mailing list