From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhenyu Wang Subject: Re: [PATCH] drm/i915: Add ppgtt to GVT GEM context Date: Mon, 15 Oct 2018 13:16:36 +0800 Message-ID: <20181015051636.GY4714@zhen-hp.sh.intel.com> References: <1539579050-2990-1-git-send-email-xiong.y.zhang@intel.com> Reply-To: Zhenyu Wang Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1594673940==" Return-path: In-Reply-To: <1539579050-2990-1-git-send-email-xiong.y.zhang@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Xiong Zhang Cc: intel-gfx@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1594673940== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="s6WJ5R58009U+S7n" Content-Disposition: inline --s6WJ5R58009U+S7n Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2018.10.15 12:50:50 +0800, Xiong Zhang wrote: > Currently the guest couldn't boot up under GVT-g environment as the > following call trace exists: > [ 272.504762] BUG: unable to handle kernel NULL pointer dereference at 0= 000000000000100 > [ 272.504834] Call Trace: > [ 272.504852] execlists_context_pin+0x2b2/0x520 [i915] > [ 272.504869] intel_gvt_scan_and_shadow_workload+0x50/0x4d0 [i915] > [ 272.504887] intel_vgpu_create_workload+0x3e2/0x570 [i915] > [ 272.504901] intel_vgpu_submit_execlist+0xc0/0x2a0 [i915] > [ 272.504916] elsp_mmio_write+0xc7/0x130 [i915] > [ 272.504930] intel_vgpu_mmio_reg_rw+0x24a/0x4c0 [i915] > [ 272.504944] intel_vgpu_emulate_mmio_write+0xac/0x240 [i915] > [ 272.504947] intel_vgpu_rw+0x22d/0x270 [kvmgt] > [ 272.504949] intel_vgpu_write+0x164/0x1f0 [kvmgt] >=20 > GVT GEM context is created by i915_gem_context_create_gvt() which > doesn't allocate ppgtt. So GVT GEM context structure doesn't have > a valid i915_hw_ppgtt. >=20 > This patch create ppgtt table at GVT GEM context creation, then assign > shadow ppgtt's root table address to this ppgtt when shadow ppgtt will > be used on GPU. So GVT GEM context has valid ppgtt address. But note > that this ppgtt only contain valid ppgtt root table address, the table > entry in this ppgtt structure are invalid. >=20 > Fixes:4a3d3f6785be("drm/i915: Match code to comment and enforce ppgtt for= execlists") >=20 > Signed-off-by: Xiong Zhang > --- > drivers/gpu/drm/i915/gvt/scheduler.c | 29 +++++++++++++++++++++++++++= ++ > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > 2 files changed, 30 insertions(+), 1 deletion(-) >=20 > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/= gvt/scheduler.c > index ea34003..b7e0529 100644 > --- a/drivers/gpu/drm/i915/gvt/scheduler.c > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c > @@ -334,6 +334,29 @@ static void release_shadow_wa_ctx(struct intel_shado= w_wa_ctx *wa_ctx) > i915_gem_object_put(wa_ctx->indirect_ctx.obj); > } > We may better add comment for this one as currently it might not be real root pointer for gvt context, so won't confuse people later. Others looks fine to me. Thanks! Reviewed-by: Zhenyu Wang > +static int set_context_ppgtt_from_shadow(struct intel_vgpu_workload *wor= kload, > + struct i915_gem_context *ctx) > +{ > + struct intel_vgpu_mm *mm =3D workload->shadow_mm; > + struct i915_hw_ppgtt *ppgtt =3D ctx->ppgtt; > + int i =3D 0; > + > + if (mm->type !=3D INTEL_GVT_MM_PPGTT || > + !mm->ppgtt_mm.shadowed) > + return -1; > + > + if (mm->ppgtt_mm.root_entry_type =3D=3D GTT_TYPE_PPGTT_ROOT_L4_ENTRY) > + px_dma(&ppgtt->pml4) =3D mm->ppgtt_mm.shadow_pdps[0]; > + else { > + for (i =3D 0; i < GVT_RING_CTX_NR_PDPS; i++) { > + px_dma(ppgtt->pdp.page_directory[i]) =3D > + mm->ppgtt_mm.shadow_pdps[i]; > + } > + } > + > + return 0; > +} > + > /** > * intel_gvt_scan_and_shadow_workload - audit the workload by scanning a= nd > * shadow it as well, include ringbuffer,wa_ctx and ctx. > @@ -358,6 +381,12 @@ int intel_gvt_scan_and_shadow_workload(struct intel_= vgpu_workload *workload) > if (workload->req) > return 0; > =20 > + ret =3D set_context_ppgtt_from_shadow(workload, shadow_ctx); > + if (ret < 0) { > + gvt_vgpu_err("workload shadow ppgtt isn't ready\n"); > + return ret; > + } > + > /* pin shadow context by gvt even the shadow context will be pinned > * when i915 alloc request. That is because gvt will update the guest > * context from shadow context when workload is completed, and at that > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i9= 15/i915_gem_context.c > index 8cbe580..b97963d 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -457,7 +457,7 @@ i915_gem_context_create_gvt(struct drm_device *dev) > if (ret) > return ERR_PTR(ret); > =20 > - ctx =3D __create_hw_context(to_i915(dev), NULL); > + ctx =3D i915_gem_create_context(to_i915(dev), NULL); > if (IS_ERR(ctx)) > goto out; > =20 > --=20 > 2.7.4 >=20 --=20 Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 --s6WJ5R58009U+S7n Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EARECAB0WIQTXuabgHDW6LPt9CICxBBozTXgYJwUCW8QitAAKCRCxBBozTXgY J/aIAJ9SVb07eWYKJqWRvMHy8I5W0FhxXACeOmJVpiHo4MrepTmO52lLX2AbClc= =6dru -----END PGP SIGNATURE----- --s6WJ5R58009U+S7n-- --===============1594673940== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============1594673940==--