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

[Zhenyu Wang]

On 2020.04.28 00:41:48 +0000, Zhao, Yan Y wrote:
> 
> > 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.

Current shadow mm list is for per vgpu instead of per workload, so in this patch
which adds new list for all possible shadow mm per workload.

> Why not choose the most robust way ?  (like save inhibit. it's possible that we might miss something)
>

I think it's robust because we know exactly what the result should be
for hw behavior emulation. How about to add a debug func to double
check if context pdps matches? So in normal operation it won't bother.

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

-- 
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/20200428/e94bc02a/attachment.sig>