[PATCH v2 2/2] drm/i915/gvt: Support PPGTT table load command

[Zhao, Yan Y]

> -----Original Message-----
> From: intel-gvt-dev <intel-gvt-dev-bounces at lists.freedesktop.org> On Behalf
> Of Zhenyu Wang
> Sent: Monday, April 27, 2020 5:27 PM
> To: Zhao, Yan Y <yan.y.zhao at intel.com>
> Cc: intel-gvt-dev at lists.freedesktop.org; Zhang, Tina <tina.zhang at intel.com>
> Subject: Re: [PATCH v2 2/2] drm/i915/gvt: Support PPGTT table load
> command
> 
> On 2020.04.27 03:21:49 -0400, Yan Zhao wrote:
> > > >
> > > > I think the step is just to restore guest context and run through
> > > > loaded shadow mm, we only need to recover last guest pdp after
> > > > workload run.
> > > >
> > > sorry. sent too fast.
> > > I mean why not just update workload->shadow_mm and
> > > workload->shadow_mm->ppgtt_mm.guest_pdps, when lri pdp is found?
> > >
> > 1. if lri pdp1 (not the same as shadow_mm) and lri pdp2 (the same as
> > shadow_mm) are executed successively, we should not update the guest
> > pdp.
> 
> Actually we should, to replace with shadow ppgtt ptr also for same as
> shadow_mm case, otherwise wrong pdps would be loaded to HW. Next
> version will fix that.
> 
> > 2. if there's a save inhibit bit set in the workload, we cannot update
> > the guest context.
> 
> yep, looks currently we don't care about save inhibit but just update guest
> context. We might do this later.
> 
> > 3. there may be other cases to cause last entry in lri_shadow_mm no
> > aligning with the final one in workload.
> > so I think the safest way to update guest context is to find the
> > guest_pdps according to pdps in shadow context.
> 
> Hmm, I think it's safe now to mark last ppgtt LRI cmd as last context state
> because I can't think of other ways might affect that. And to find guest pdps,
> we have to walk through all shadow mm which seems not worth to me.
> 
No need to walk through all shadow mms.
Just shadow mms for current workload, and you can start from the most possible one.
Why not choose the most robust way ?  (like save inhibit. it's possible that we might miss something)

> Thanks
> 
> >
> > > > > > > >
> > > > > > > > and what's the reason for us to use a lri_shadow_mm list?
> > > > > > >
> > > > > > > As in theory there could be multiple ppgtt loads in one ring
> > > > > > > submission, and previous code always assumes one e.g put
> > > > > > > older scanned shadow mm if found another one, which was not
> > > > > > > right to me. So here just uses a link to track all possible loads.
> > > > > > >
> > > > > > > >
> > > > > > > >
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827