On 2019.04.04 09:11:55 +0100, Chris Wilson wrote:
> Quoting Zhenyu Wang (2019-04-04 08:48:34)
> > On 2019.04.04 08:30:56 +0100, Chris Wilson wrote:
> > > ppgtt_free_all_spt() iterates the radixtree as it is deleting it,
> > > forgoing all protection against the leaves being freed in the process
> > > (leaving the iter pointing into the void).
> > >
> > > A minimal fix seems to be to use the available post_shadow_list to
> > > decompose the tree into a list prior to destroying the radixtree.
> > >
> > > Alerted by the sparse warnings:
> > >
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignment (different address spaces)
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: expected void **slot
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: got void [noderef] <asn:4> **
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignment (different address spaces)
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: expected void **slot
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: got void [noderef] <asn:4> **
> > > drivers/gpu/drm/i915/gvt/gtt.c:758:45: warning: incorrect type in argument 1 (different address spaces)
> > > drivers/gpu/drm/i915/gvt/gtt.c:758:45: expected void [noderef] <asn:4> **slot
> > > drivers/gpu/drm/i915/gvt/gtt.c:758:45: got void **slot
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in argument 1 (different address spaces)
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: expected void [noderef] <asn:4> **slot
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: got void **slot
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignment (different address spaces)
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: expected void **slot
> > > drivers/gpu/drm/i915/gvt/gtt.c:757:9: got void [noderef] <asn:4> **
> > >
> > > This would also have been loudly warning if run through CI for the
> > > invalid RCU dereferences.
> > >
> > > Fixes: b6c126a39345 ("drm/i915/gvt: Manage shadow pages with radix tree")
> > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > Cc: Changbin Du <changbin.du@xxxxxxxxx>
> > > Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> > > Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/gvt/gtt.c | 12 +++++++++---
> > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> > > index cf133ef03873..9814773882ec 100644
> > > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > > @@ -750,14 +750,20 @@ static void ppgtt_free_spt(struct intel_vgpu_ppgtt_spt *spt)
> > >
> > > static void ppgtt_free_all_spt(struct intel_vgpu *vgpu)
> > > {
> > > - struct intel_vgpu_ppgtt_spt *spt;
> > > + struct intel_vgpu_ppgtt_spt *spt, *spn;
> > > struct radix_tree_iter iter;
> > > - void **slot;
> > > + LIST_HEAD(all_spt);
> > > + void __rcu **slot;
> > >
> > > + rcu_read_lock();
> > > radix_tree_for_each_slot(slot, &vgpu->gtt.spt_tree, &iter, 0) {
> > > spt = radix_tree_deref_slot(slot);
> > > - ppgtt_free_spt(spt);
> > > + list_move(&spt->post_shadow_list, &all_spt);
> > > }
> > > + rcu_read_unlock();
> > > +
> > > + list_for_each_entry_safe(spt, spn, &all_spt, post_shadow_list)
> > > + ppgtt_free_spt(spt);
> > > }
> > >
> >
> > As we ensure to flush post shadow list, so this is safe to reuse.
>
> Phew! I looked, couldn't see that it would be used at this point, so
> hoped for the best.
>
> > Reviewed-by: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
>
> Will you take both of these patches through your tree?
Yes.
Thanks!
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
