On Fri, Jul 28, 2023 at 06:35:12PM -0700, Sean Christopherson wrote: > Move the check that a vGPU is attacked from is_2MB_gtt_possible() all the typo: "attacked" --> "attached"
> way up to shadow_ppgtt_mm() to avoid unnecessary work, and to make it more This commit message does not match to what the patch does. The check in the patch is in ppgtt_populate_shadow_entry(). What you want is like below? @@ -1796,6 +1797,9 @@ static int shadow_ppgtt_mm(struct intel_vgpu_mm *mm) if (mm->ppgtt_mm.shadowed) return 0; + if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status)) + return -EINVAL; + mm->ppgtt_mm.shadowed = true; for (index = 0; index < ARRAY_SIZE(mm->ppgtt_mm.guest_pdps); index++) { > obvious that a future cleanup of is_2MB_gtt_possible() isn't introducing a > bug. > > is_2MB_gtt_possible() has only one caller, ppgtt_populate_shadow_entry(), > and all paths in ppgtt_populate_shadow_entry() eventually check for > attachment by way of intel_gvt_dma_map_guest_page(). > > And of the paths that lead to ppgtt_populate_shadow_entry(), > shadow_ppgtt_mm() is the only one that doesn't already check for > INTEL_VGPU_STATUS_ACTIVE or INTEL_VGPU_STATUS_ATTACHED. ... > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > index 5426a27c1b71..2aed31b497c9 100644 > --- a/drivers/gpu/drm/i915/gvt/gtt.c > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > @@ -1163,8 +1163,6 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu, > if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M)) > return 0; > > - if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status)) > - return -EINVAL; > pfn = gfn_to_pfn(vgpu->vfio_device.kvm, ops->get_pfn(entry)); > if (is_error_noslot_pfn(pfn)) > return -EINVAL; > @@ -1277,6 +1275,9 @@ static int ppgtt_populate_shadow_entry(struct > intel_vgpu *vgpu, > if (!pte_ops->test_present(ge)) > return 0; > > + if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status)) > + return -EINVAL; > + > gfn = pte_ops->get_pfn(ge); > > switch (ge->type) { > -- > 2.41.0.487.g6d72f3e995-goog >