| Message ID | 1494581878-2258-2-git-send-email-tina.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote: > Guest i915 driver may update the ppgtt table just before this workload > is going to be submitted to the hardware by device model. This case > wasn't handled well by device model before, due to the small time > window between removing old ppgtt entry and adding the new one. Errors > occur when the workload is executed by hardware during that small time > window. This patch is to remove this time window by adding the new ppgtt > entry first and then remove the old one. > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> This should be reviewed on the GVT mailing list, and this should include the squashed hunk which exports the newly added capability. Regards, Joonas
> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > Behalf Of Joonas Lahtinen > Sent: Monday, May 15, 2017 6:50 PM > To: Zhang, Tina <tina.zhang@intel.com>; intel-gvt-dev@lists.freedesktop.org > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt > update process by adding entry first > > On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote: > > Guest i915 driver may update the ppgtt table just before this workload > > is going to be submitted to the hardware by device model. This case > > wasn't handled well by device model before, due to the small time > > window between removing old ppgtt entry and adding the new one. Errors > > occur when the workload is executed by hardware during that small time > > window. This patch is to remove this time window by adding the new > > ppgtt entry first and then remove the old one. > > > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> > > This should be reviewed on the GVT mailing list, and this should include the > squashed hunk which exports the newly added capability. Thanks for the comments. I'm sorry I didn’t get it, here. Do you want me to remove this patch from this patch set? Well, the reason this patch is here is because we met a GPU hang issue when guest i915 using full ppgtt. This is a blocking issue for enabling guest i915 full ppgtt functionality. This patch is to fix that issue. So, with this patch, the device model can have the capability to support guest i915 full ppgtt functionality. And the other patches in this patch set are used for guest to communicate with host whether the full ppgtt capability is supported. > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation > _______________________________________________ > intel-gvt-dev mailing list > intel-gvt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
On to, 2017-05-18 at 02:23 +0000, Zhang, Tina wrote: > > > > > -----Original Message----- > > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > > Behalf Of Joonas Lahtinen > > Sent: Monday, May 15, 2017 6:50 PM > > > > To: Zhang, Tina <tina.zhang@intel.com>; intel-gvt-dev@lists.freedesktop.org > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt > > update process by adding entry first > > > > On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote: > > > > > > Guest i915 driver may update the ppgtt table just before this workload > > > is going to be submitted to the hardware by device model. This case > > > wasn't handled well by device model before, due to the small time > > > window between removing old ppgtt entry and adding the new one. Errors > > > occur when the workload is executed by hardware during that small time > > > window. This patch is to remove this time window by adding the new > > > ppgtt entry first and then remove the old one. > > > > > > > > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> > > > > This should be reviewed on the GVT mailing list, and this should include the > > squashed hunk which exports the newly added capability. > Thanks for the comments. I'm sorry I didn’t get it, here. Do you want > me to remove this patch from this patch set? It might be easier to merge if this is sent as two separate patch series, because other gets merged through gvt tree and the other directly to tip. > Well, the reason this patch is here is because we met a GPU hang > issue when guest i915 using full ppgtt. This is a blocking issue for > enabling guest i915 full ppgtt functionality. This patch is to fix > that issue. So, with this patch, the device model can have the > capability to support guest i915 full ppgtt functionality. And the > other patches in this patch set are used for guest to communicate > with host whether the full ppgtt capability is supported. Yes, but to help bisecting, the capability should be exported in the same patch that implements the capability. Regards, Joonas
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index c6f0077..8caad86 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -975,29 +975,26 @@ static int ppgtt_populate_shadow_page(struct intel_vgpu_ppgtt_spt *spt) } static int ppgtt_handle_guest_entry_removal(struct intel_vgpu_guest_page *gpt, - unsigned long index) + struct intel_gvt_gtt_entry *se, unsigned long index) { struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt); struct intel_vgpu_shadow_page *sp = &spt->shadow_page; struct intel_vgpu *vgpu = spt->vgpu; struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops; - struct intel_gvt_gtt_entry e; int ret; - ppgtt_get_shadow_entry(spt, &e, index); - - trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, e.val64, + trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, se->val64, index); - if (!ops->test_present(&e)) + if (!ops->test_present(se)) return 0; - if (ops->get_pfn(&e) == vgpu->gtt.scratch_pt[sp->type].page_mfn) + if (ops->get_pfn(se) == vgpu->gtt.scratch_pt[sp->type].page_mfn) return 0; - if (gtt_type_is_pt(get_next_pt_type(e.type))) { + if (gtt_type_is_pt(get_next_pt_type(se->type))) { struct intel_vgpu_ppgtt_spt *s = - ppgtt_find_shadow_page(vgpu, ops->get_pfn(&e)); + ppgtt_find_shadow_page(vgpu, ops->get_pfn(se)); if (!s) { gvt_vgpu_err("fail to find guest page\n"); ret = -ENXIO; @@ -1007,12 +1004,10 @@ static int ppgtt_handle_guest_entry_removal(struct intel_vgpu_guest_page *gpt, if (ret) goto fail; } - ops->set_pfn(&e, vgpu->gtt.scratch_pt[sp->type].page_mfn); - ppgtt_set_shadow_entry(spt, &e, index); return 0; fail: gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d\n", - spt, e.val64, e.type); + spt, se->val64, se->type); return ret; } @@ -1232,22 +1227,37 @@ static int ppgtt_handle_guest_write_page_table( { struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt); struct intel_vgpu *vgpu = spt->vgpu; + int type = spt->shadow_page.type; struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops; + struct intel_gvt_gtt_entry se; int ret; int new_present; new_present = ops->test_present(we); - ret = ppgtt_handle_guest_entry_removal(gpt, index); - if (ret) - goto fail; + /* + * Adding the new entry first and then removing the old one, that can + * guarantee the ppgtt table is validated during the window between + * adding and removal. + */ + ppgtt_get_shadow_entry(spt, &se, index); if (new_present) { ret = ppgtt_handle_guest_entry_add(gpt, we, index); if (ret) goto fail; } + + ret = ppgtt_handle_guest_entry_removal(gpt, &se, index); + if (ret) + goto fail; + + if (!new_present) { + ops->set_pfn(&se, vgpu->gtt.scratch_pt[type].page_mfn); + ppgtt_set_shadow_entry(spt, &se, index); + } + return 0; fail: gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d.\n", @@ -1319,7 +1329,7 @@ static int ppgtt_handle_guest_write_page_table_bytes(void *gp, struct intel_vgpu *vgpu = spt->vgpu; struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops; const struct intel_gvt_device_info *info = &vgpu->gvt->device_info; - struct intel_gvt_gtt_entry we; + struct intel_gvt_gtt_entry we, se; unsigned long index; int ret; @@ -1335,7 +1345,8 @@ static int ppgtt_handle_guest_write_page_table_bytes(void *gp, return ret; } else { if (!test_bit(index, spt->post_shadow_bitmap)) { - ret = ppgtt_handle_guest_entry_removal(gpt, index); + ppgtt_get_shadow_entry(spt, &se, index); + ret = ppgtt_handle_guest_entry_removal(gpt, &se, index); if (ret) return ret; }
Guest i915 driver may update the ppgtt table just before this workload is going to be submitted to the hardware by device model. This case wasn't handled well by device model before, due to the small time window between removing old ppgtt entry and adding the new one. Errors occur when the workload is executed by hardware during that small time window. This patch is to remove this time window by adding the new ppgtt entry first and then remove the old one. Signed-off-by: Tina Zhang <tina.zhang@intel.com> --- drivers/gpu/drm/i915/gvt/gtt.c | 45 ++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 17 deletions(-)