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?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
