mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Zhenyu Wang <zhenyuw@linux.intel.com>,
	Zhi Wang <zhi.a.wang@intel.com>
Cc: kvm@vger.kernel.org, intel-gvt-dev@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Yan Zhao <yan.y.zhao@intel.com>, Ben Gardon <bgardon@google.com>
Subject: [PATCH v2 03/27] drm/i915/gvt: remove interface intel_gvt_is_valid_gfn
Date: Fri, 10 Mar 2023 16:22:34 -0800	[thread overview]
Message-ID: <20230311002258.852397-4-seanjc@google.com> (raw)
In-Reply-To: <20230311002258.852397-1-seanjc@google.com>

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


  parent reply	other threads:[~2023-03-11  0:23 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-11  0:22 [PATCH v2 00/27] drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 01/27] drm/i915/gvt: Verify pfn is "valid" before dereferencing "struct page" Sean Christopherson
2023-03-13 15:37   ` Wang, Wei W
2023-03-15 18:13     ` [Intel-gfx] " Andrzej Hajda
2023-03-15 19:23       ` Sean Christopherson
2023-03-17  4:20   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 02/27] KVM: x86/mmu: Factor out helper to get max mapping size of a memslot Sean Christopherson
2023-03-13 15:37   ` Wang, Wei W
2023-03-11  0:22 ` Sean Christopherson [this message]
2023-03-17  4:26   ` [PATCH v2 03/27] drm/i915/gvt: remove interface intel_gvt_is_valid_gfn Yan Zhao
2023-03-11  0:22 ` [PATCH v2 04/27] drm/i915/gvt: Incorporate KVM memslot info into check for 2MiB GTT entry Sean Christopherson
2023-03-14  3:09   ` Yan Zhao
2023-03-14 17:13     ` Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 05/27] drm/i915/gvt: Verify VFIO-pinned page is THP when shadowing 2M gtt entry Sean Christopherson
2023-03-17  5:33   ` Yan Zhao
2023-05-04 20:41     ` Sean Christopherson
2023-05-06  6:35       ` Yan Zhao
2023-05-06 10:57         ` Yan Zhao
2023-05-08 14:05           ` Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 06/27] drm/i915/gvt: Put the page reference obtained by KVM's gfn_to_pfn() Sean Christopherson
2023-03-17  6:18   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 07/27] drm/i915/gvt: Don't rely on KVM's gfn_to_pfn() to query possible 2M GTT Sean Christopherson
2023-03-17  5:37   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 08/27] drm/i915/gvt: Use an "unsigned long" to iterate over memslot gfns Sean Christopherson
2023-03-17  6:19   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 09/27] drm/i915/gvt: Drop unused helper intel_vgpu_reset_gtt() Sean Christopherson
2023-03-17  6:20   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 10/27] drm/i915/gvt: Protect gfn hash table with vgpu_lock Sean Christopherson
2023-03-17  6:21   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 11/27] KVM: x86/mmu: Don't rely on page-track mechanism to flush on memslot change Sean Christopherson
2023-03-15  1:08   ` Yan Zhao
2023-03-15 15:32     ` Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 12/27] KVM: x86/mmu: Don't bounce through page-track mechanism for guest PTEs Sean Christopherson
2023-03-17  6:37   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 13/27] KVM: drm/i915/gvt: Drop @vcpu from KVM's ->track_write() hook Sean Christopherson
2023-03-17  7:28   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 14/27] KVM: x86: Reject memslot MOVE operations if KVMGT is attached Sean Christopherson
2023-03-15  8:03   ` Yan Zhao
2023-03-15 15:43     ` Sean Christopherson
2023-03-16  9:27       ` Yan Zhao
2023-03-17  7:29   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 15/27] drm/i915/gvt: Don't bother removing write-protection on to-be-deleted slot Sean Christopherson
2023-03-17  7:30   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 16/27] KVM: x86: Add a new page-track hook to handle memslot deletion Sean Christopherson
2023-03-17  7:43   ` Yan Zhao
2023-03-17 16:20     ` Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 17/27] drm/i915/gvt: switch from ->track_flush_slot() to ->track_remove_region() Sean Christopherson
2023-03-17  7:45   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 18/27] KVM: x86: Remove the unused page-track hook track_flush_slot() Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 19/27] KVM: x86/mmu: Move KVM-only page-track declarations to internal header Sean Christopherson
2023-03-15  8:44   ` Yan Zhao
2023-03-15 15:13     ` Sean Christopherson
2023-03-16  9:19       ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 20/27] KVM: x86/mmu: Use page-track notifiers iff there are external users Sean Christopherson
2023-03-15  9:34   ` Yan Zhao
2023-03-15 16:21     ` Sean Christopherson
2023-03-16  9:29       ` Yan Zhao
2023-03-15 10:36   ` Yan Zhao
2023-03-15 16:54     ` Sean Christopherson
2023-05-04 19:54       ` Sean Christopherson
2023-05-06  1:08         ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 21/27] KVM: x86/mmu: Drop infrastructure for multiple page-track modes Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 22/27] KVM: x86/mmu: Rename page-track APIs to reflect the new reality Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 23/27] KVM: x86/mmu: Assert that correct locks are held for page write-tracking Sean Christopherson
2023-03-17  7:55   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 24/27] KVM: x86/mmu: Bug the VM if write-tracking is used but not enabled Sean Christopherson
2023-03-11  0:22 ` [PATCH v2 25/27] KVM: x86/mmu: Drop @slot param from exported/external page-track APIs Sean Christopherson
2023-03-17  8:28   ` Yan Zhao
2023-03-23  8:50     ` Yan Zhao
2023-05-03 23:16       ` Sean Christopherson
2023-05-04  2:17         ` Yan Zhao
2023-05-08  1:15           ` Yan Zhao
2023-05-11 22:39             ` Sean Christopherson
2023-05-12  2:58               ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 26/27] KVM: x86/mmu: Handle KVM bookkeeping in page-track APIs, not callers Sean Christopherson
2023-03-17  8:52   ` Yan Zhao
2023-03-11  0:22 ` [PATCH v2 27/27] drm/i915/gvt: Drop final dependencies on KVM internal details Sean Christopherson
2023-03-17  8:58   ` Yan Zhao
2023-03-13  9:58 ` [PATCH v2 00/27] drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups Yan Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230311002258.852397-4-seanjc@google.com \
    --to=seanjc@google.com \
    --cc=bgardon@google.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=yan.y.zhao@intel.com \
    --cc=zhenyuw@linux.intel.com \
    --cc=zhi.a.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®