[PATCH 2/2] drm/i915/gvt: support inconsecutive partial gtt entry write
Zhenyu Wang
zhenyuw at linux.intel.com
Mon Oct 8 09:58:11 UTC 2018
On 2018.09.19 06:54:41 +0000, Zhang, Xiaolin wrote:
> It looks fine for me.
>
> Reviewed-by: Xiaolin Zhang <xiaolin.zhang at intel.com>
>
Applied, thanks!
>
> -----Original Message-----
> From: hang.yuan at linux.intel.com [mailto:hang.yuan at linux.intel.com]
> Sent: Wednesday, September 19, 2018 2:42 PM
> To: intel-gvt-dev at lists.freedesktop.org
> Cc: Hang Yuan <hang.yuan at linux.intel.com>; Zhao, Yan Y <yan.y.zhao at intel.com>; Zhang, Xiaolin <xiaolin.zhang at intel.com>; Wang, Zhenyu Z <zhenyu.z.wang at intel.com>
> Subject: [PATCH 2/2] drm/i915/gvt: support inconsecutive partial gtt entry write
>
> 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.
>
> v2: invalidate old entry and flush ggtt (Zhenyu)
> v3: split old ggtt page unmap to another patch (Zhenyu)
> v4: refine codes (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 | 107 ++++++++++++++++++++---------------------
> drivers/gpu/drm/i915/gvt/gtt.h | 9 +++-
> 2 files changed, 60 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 3a725ac..58e166e 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,57 @@ 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);
> + found = true;
> + break;
> + }
> +
> + /* update of the first partial part */
> + pos->data = e.val64;
> + ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index);
> + return 0;
> + }
> + }
>
> - 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;
>
> @@ -2432,6 +2421,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); }
>
> @@ -2456,6 +2447,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
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
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/20181008/5b67e896/attachment.sig>
More information about the intel-gvt-dev
mailing list