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: Fri, 19 Oct 2018 11:05:20 +0800 Message-ID: <20181019030520.GH4714@zhen-hp.sh.intel.com> References: <1539841231-3157-1-git-send-email-xiong.y.zhang@intel.com> Reply-To: Zhenyu Wang Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0399516703==" Return-path: In-Reply-To: <1539841231-3157-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 --===============0399516703== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="3+nIULlytNYGw3fk" Content-Disposition: inline --3+nIULlytNYGw3fk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2018.10.18 13:40:31 +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 > Reviewed-by: Zhenyu Wang Any more comment for this? We need it for current gvt broken on drm-tip, and it requires to change i915 for gvt ppgtt allocation, so I assume it's better to be merged by i915 directly, or do you like a gvt pull instead? Thanks. > --- > 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); > } > =20 > +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 --3+nIULlytNYGw3fk Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EARECAB0WIQTXuabgHDW6LPt9CICxBBozTXgYJwUCW8lJ8AAKCRCxBBozTXgY JyGMAKCHL8s1dD0mt0ZlRM3zcAdKtFOZ8gCgmBUd2HPmXsgApGsCcUNFIgv8xpE= =qwES -----END PGP SIGNATURE----- --3+nIULlytNYGw3fk-- --===============0399516703== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============0399516703==--