Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx> 于2022年12月21日周三 11:01写道：
>
> On 2022.12.20 17:40:14 +0800, Zheng Wang wrote:
> > If intel_gvt_dma_map_guest_page failed, it will call
> > ppgtt_invalidate_spt, which will finally free the spt. But the
> > caller function ppgtt_populate_spt_by_guest_entry does not notice
> > that, it will free spt again in its error path.
>
> indent
Yeap :)
> > + if (ret)
> > + goto err;
> > sub_se.val64 = se->val64;
> >
> > /* Copy the PAT field from PDE. */
> > @@ -1231,6 +1229,18 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> > ops->set_pfn(se, sub_spt->shadow_page.mfn);
> > ppgtt_set_shadow_entry(spt, se, index);
> > return 0;
> > +err:
> > + /* Undone the existing mappings of DMA addr. */
>
> We need a verb here for Undo.
Get it.
>
> > + for_each_present_shadow_entry(sub_spt, &sub_se, sub_index) {
> > + gvt_vdbg_mm("invalidate 4K entry\n");
> > + ppgtt_invalidate_pte(sub_spt, &sub_se);
> > + }
> > + /* Release the new allocated spt. */
> > + trace_spt_change(sub_spt->vgpu->id, "release", sub_spt,
> > + sub_spt->guest_page.gfn, sub_spt->shadow_page.type);
> > + ppgtt_free_spt(sub_spt);
> > + sub_spt = NULL;
>
> Not need to reset local variable that has no use then.
>
> I'll handle these trivial fixes during the merge.
>
Very thanks for that.
Best regards,
Zheng Wang
