lkml.org 
[lkml]   [2023]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 03/27] drm/i915/gvt: remove interface intel_gvt_is_valid_gfn
    Tested-by: Yan Zhao <yan.y.zhao@intel.com>

    On Fri, Mar 10, 2023 at 04:22:34PM -0800, Sean Christopherson wrote:
    > From: Yan Zhao <yan.y.zhao@intel.com>
    >
    > Currently intel_gvt_is_valid_gfn() is called in two places:
    > (1) shadowing guest GGTT entry
    > (2) shadowing guest PPGTT leaf entry,
    > which was introduced in commit cc753fbe1ac4
    > ("drm/i915/gvt: validate gfn before set shadow page entry").
    >
    > However, now it's not necessary to call this interface any more, because
    > a. GGTT partial write issue has been fixed by
    > commit bc0686ff5fad
    > ("drm/i915/gvt: support inconsecutive partial gtt entry write")
    > commit 510fe10b6180
    > ("drm/i915/gvt: fix a bug of partially write ggtt enties")
    > b. PPGTT resides in normal guest RAM and we only treat 8-byte writes
    > as valid page table writes. Any invalid GPA found is regarded as
    > an error, either due to guest misbehavior/attack or bug in host
    > shadow code.
    > So,rather than do GFN pre-checking and replace invalid GFNs with
    > scratch GFN and continue silently, just remove the pre-checking and
    > abort PPGTT shadowing on error detected.
    > c. GFN validity check is still performed in
    > intel_gvt_dma_map_guest_page() --> gvt_pin_guest_page().
    > It's more desirable to call VFIO interface to do both validity check
    > and mapping.
    > Calling intel_gvt_is_valid_gfn() to do GFN validity check from KVM side
    > while later mapping the GFN through VFIO interface is unnecessarily
    > fragile and confusing for unaware readers.
    >
    > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
    > [sean: remove now-unused local variables]
    > Signed-off-by: Sean Christopherson <seanjc@google.com>
    > ---
    > drivers/gpu/drm/i915/gvt/gtt.c | 36 +---------------------------------
    > 1 file changed, 1 insertion(+), 35 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
    > index 58b9b316ae46..f30922c55a0c 100644
    > --- a/drivers/gpu/drm/i915/gvt/gtt.c
    > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
    > @@ -49,22 +49,6 @@
    > static bool enable_out_of_sync = false;
    > static int preallocated_oos_pages = 8192;
    >
    > -static bool intel_gvt_is_valid_gfn(struct intel_vgpu *vgpu, unsigned long gfn)
    > -{
    > - struct kvm *kvm = vgpu->vfio_device.kvm;
    > - int idx;
    > - bool ret;
    > -
    > - if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
    > - return false;
    > -
    > - idx = srcu_read_lock(&kvm->srcu);
    > - ret = kvm_is_visible_gfn(kvm, gfn);
    > - srcu_read_unlock(&kvm->srcu, idx);
    > -
    > - return ret;
    > -}
    > -
    > /*
    > * validate a gm address and related range size,
    > * translate it to host gm address
    > @@ -1333,11 +1317,9 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu *vgpu,
    > static int ppgtt_populate_spt(struct intel_vgpu_ppgtt_spt *spt)
    > {
    > struct intel_vgpu *vgpu = spt->vgpu;
    > - struct intel_gvt *gvt = vgpu->gvt;
    > - const struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
    > struct intel_vgpu_ppgtt_spt *s;
    > struct intel_gvt_gtt_entry se, ge;
    > - unsigned long gfn, i;
    > + unsigned long i;
    > int ret;
    >
    > trace_spt_change(spt->vgpu->id, "born", spt,
    > @@ -1354,13 +1336,6 @@ static int ppgtt_populate_spt(struct intel_vgpu_ppgtt_spt *spt)
    > ppgtt_generate_shadow_entry(&se, s, &ge);
    > ppgtt_set_shadow_entry(spt, &se, i);
    > } else {
    > - gfn = ops->get_pfn(&ge);
    > - if (!intel_gvt_is_valid_gfn(vgpu, gfn)) {
    > - ops->set_pfn(&se, gvt->gtt.scratch_mfn);
    > - ppgtt_set_shadow_entry(spt, &se, i);
    > - continue;
    > - }
    > -
    > ret = ppgtt_populate_shadow_entry(vgpu, spt, i, &ge);
    > if (ret)
    > goto fail;
    > @@ -2335,14 +2310,6 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
    > m.val64 = e.val64;
    > m.type = e.type;
    >
    > - /* one PTE update may be issued in multiple writes and the
    > - * first write may not construct a valid gfn
    > - */
    > - if (!intel_gvt_is_valid_gfn(vgpu, gfn)) {
    > - ops->set_pfn(&m, gvt->gtt.scratch_mfn);
    > - goto out;
    > - }
    > -
    > ret = intel_gvt_dma_map_guest_page(vgpu, gfn, PAGE_SIZE,
    > &dma_addr);
    > if (ret) {
    > @@ -2359,7 +2326,6 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
    > ops->clear_present(&m);
    > }
    >
    > -out:
    > ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index);
    >
    > ggtt_get_host_entry(ggtt_mm, &e, g_gtt_index);
    > --
    > 2.40.0.rc1.284.g88254d51c5-goog
    >

    \
     
     \ /
      Last update: 2023-03-27 01:04    [W:3.205 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site