[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups (rev9)
== Series Details == Series: drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups (rev9) URL : https://patchwork.freedesktop.org/series/112196/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/112196/revisions/9/mbox/ not found Build failed, no error log produced
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/bridge-connector: simplify handling of USB-C DP
== Series Details == Series: drm/bridge-connector: simplify handling of USB-C DP URL : https://patchwork.freedesktop.org/series/121560/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13439 -> Patchwork_121560v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_121560v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_121560v1, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/index.html Participating hosts (43 -> 41) -- Missing(2): fi-kbl-soraka fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_121560v1: ### IGT changes ### Possible regressions * igt@i915_selftest@live@mman: - bat-dg2-8: [PASS][1] -> [DMESG-WARN][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13439/bat-dg2-8/igt@i915_selftest@l...@mman.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-dg2-8/igt@i915_selftest@l...@mman.html Known issues Here are the changes found in Patchwork_121560v1 that come from known issues: ### IGT changes ### Issues hit * igt@core_auth@basic-auth: - bat-adlp-11:NOTRUN -> [ABORT][3] ([i915#8011]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-adlp-11/igt@core_a...@basic-auth.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-tgl-1115g4: [PASS][4] -> [FAIL][5] ([i915#7940]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13439/fi-tgl-1115g4/igt@i915_pm_...@basic-pci-d3-state.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/fi-tgl-1115g4/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_selftest@live@gt_lrc: - bat-dg2-11: [PASS][6] -> [INCOMPLETE][7] ([i915#7609] / [i915#7913]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13439/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html * igt@i915_selftest@live@gt_mocs: - bat-mtlp-6: [PASS][8] -> [DMESG-FAIL][9] ([i915#7059]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13439/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html * igt@i915_selftest@live@hangcheck: - fi-skl-guc: [PASS][10] -> [DMESG-FAIL][11] ([i915#8723]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13439/fi-skl-guc/igt@i915_selftest@l...@hangcheck.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/fi-skl-guc/igt@i915_selftest@l...@hangcheck.html * igt@i915_selftest@live@reset: - bat-rpls-2: NOTRUN -> [ABORT][12] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#7981] / [i915#8347]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-rpls-2/igt@i915_selftest@l...@reset.html * igt@i915_selftest@live@slpc: - bat-mtlp-6: NOTRUN -> [DMESG-WARN][13] ([i915#6367]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-mtlp-6/igt@i915_selftest@l...@slpc.html * igt@i915_suspend@basic-s3-without-i915: - bat-mtlp-6: NOTRUN -> [SKIP][14] ([i915#6645]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-mtlp-6/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_chamelium_hpd@common-hpd-after-suspend: - bat-mtlp-6: NOTRUN -> [SKIP][15] ([i915#7828]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-mtlp-6/igt@kms_chamelium_...@common-hpd-after-suspend.html * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence: - bat-dg2-11: NOTRUN -> [SKIP][16] ([i915#1845] / [i915#5354]) +3 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-dg2-11/igt@kms_pipe_crc_ba...@nonblocking-crc-frame-sequence.html * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1: - bat-rplp-1: [PASS][17] -> [ABORT][18] ([i915#8442] / [i915#8668]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13439/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html * igt@kms_pipe_crc_basic@suspend-read-crc: - bat-mtlp-6: NOTRUN -> [SKIP][19] ([i915#1845] / [i915#4078]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121560v1/bat-mtlp-6/igt@kms_pipe_crc_ba...@suspend-read-crc.html
[Intel-gfx] [PATCH v4 27/29] KVM: x86/mmu: Drop @slot param from exported/external page-track APIs
Refactor KVM's exported/external page-track, a.k.a. write-track, APIs
to take only the gfn and do the required memslot lookup in KVM proper.
Forcing users of the APIs to get the memslot unnecessarily bleeds
KVM internals into KVMGT and complicates usage of the APIs.
No functional change intended.
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 7 +--
arch/x86/kvm/mmu/mmu.c| 4 +-
arch/x86/kvm/mmu/page_track.c | 85 ---
arch/x86/kvm/mmu/page_track.h | 5 ++
drivers/gpu/drm/i915/gvt/kvmgt.c | 37 +++-
5 files changed, 80 insertions(+), 58 deletions(-)
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index f5c1db36cdb7..4afab697e21c 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -4,11 +4,6 @@
#include
-void kvm_write_track_add_gfn(struct kvm *kvm,
-struct kvm_memory_slot *slot, gfn_t gfn);
-void kvm_write_track_remove_gfn(struct kvm *kvm, struct kvm_memory_slot *slot,
- gfn_t gfn);
-
#ifdef CONFIG_KVM_EXTERNAL_WRITE_TRACKING
/*
* The notifier represented by @kvm_page_track_notifier_node is linked into
@@ -55,6 +50,8 @@ kvm_page_track_register_notifier(struct kvm *kvm,
void
kvm_page_track_unregister_notifier(struct kvm *kvm,
struct kvm_page_track_notifier_node *n);
+int kvm_write_track_add_gfn(struct kvm *kvm, gfn_t gfn);
+int kvm_write_track_remove_gfn(struct kvm *kvm, gfn_t gfn);
#else
/*
* Allow defining a node in a structure even if page tracking is disabled, e.g.
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index a0309fde3549..c6ae1885371c 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -840,7 +840,7 @@ static void account_shadowed(struct kvm *kvm, struct
kvm_mmu_page *sp)
/* the non-leaf shadow pages are keeping readonly. */
if (sp->role.level > PG_LEVEL_4K)
- return kvm_write_track_add_gfn(kvm, slot, gfn);
+ return __kvm_write_track_add_gfn(kvm, slot, gfn);
kvm_mmu_gfn_disallow_lpage(slot, gfn);
@@ -886,7 +886,7 @@ static void unaccount_shadowed(struct kvm *kvm, struct
kvm_mmu_page *sp)
slots = kvm_memslots_for_spte_role(kvm, sp->role);
slot = __gfn_to_memslot(slots, gfn);
if (sp->role.level > PG_LEVEL_4K)
- return kvm_write_track_remove_gfn(kvm, slot, gfn);
+ return __kvm_write_track_remove_gfn(kvm, slot, gfn);
kvm_mmu_gfn_allow_lpage(slot, gfn);
}
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index eedb5889d73e..2a64df38ccab 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -74,16 +74,8 @@ static void update_gfn_write_track(struct kvm_memory_slot
*slot, gfn_t gfn,
slot->arch.gfn_write_track[index] += count;
}
-/*
- * add guest page to the tracking pool so that corresponding access on that
- * page will be intercepted.
- *
- * @kvm: the guest instance we are interested in.
- * @slot: the @gfn belongs to.
- * @gfn: the guest page.
- */
-void kvm_write_track_add_gfn(struct kvm *kvm, struct kvm_memory_slot *slot,
-gfn_t gfn)
+void __kvm_write_track_add_gfn(struct kvm *kvm, struct kvm_memory_slot *slot,
+ gfn_t gfn)
{
lockdep_assert_held_write(>mmu_lock);
@@ -104,18 +96,9 @@ void kvm_write_track_add_gfn(struct kvm *kvm, struct
kvm_memory_slot *slot,
if (kvm_mmu_slot_gfn_write_protect(kvm, slot, gfn, PG_LEVEL_4K))
kvm_flush_remote_tlbs(kvm);
}
-EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
-/*
- * remove the guest page from the tracking pool which stops the interception
- * of corresponding access on that page.
- *
- * @kvm: the guest instance we are interested in.
- * @slot: the @gfn belongs to.
- * @gfn: the guest page.
- */
-void kvm_write_track_remove_gfn(struct kvm *kvm,
- struct kvm_memory_slot *slot, gfn_t gfn)
+void __kvm_write_track_remove_gfn(struct kvm *kvm,
+ struct kvm_memory_slot *slot, gfn_t gfn)
{
lockdep_assert_held_write(>mmu_lock);
@@ -133,7 +116,6 @@ void kvm_write_track_remove_gfn(struct kvm *kvm,
*/
kvm_mmu_gfn_allow_lpage(slot, gfn);
}
-EXPORT_SYMBOL_GPL(kvm_write_track_remove_gfn);
/*
* check if the corresponding access on the specified guest page is tracked.
@@ -257,4 +239,63 @@ void kvm_page_track_delete_slot(struct kvm *kvm, struct
kvm_memory_slot *slot)
srcu_read_unlock(>track_srcu, idx);
}
+/*
+ * add guest page to the tracking pool so that corresponding access on that
+ * page will be intercepted.
+ *
+ * @kvm: the guest instance we are interested in.
+ * @gfn: the guest page.
+ */
+int
[Intel-gfx] [PATCH v4 25/29] KVM: x86/mmu: Assert that correct locks are held for page write-tracking
When adding/removing gfns to/from write-tracking, assert that mmu_lock
is held for write, and that either slots_lock or kvm->srcu is held.
mmu_lock must be held for write to protect gfn_write_track's refcount,
and SRCU or slots_lock must be held to protect the memslot itself.
Tested-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/kvm/mmu/page_track.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index b835ba7f325c..29ae61f1e303 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -12,6 +12,7 @@
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include
#include
#include
@@ -77,9 +78,6 @@ static void update_gfn_write_track(struct kvm_memory_slot
*slot, gfn_t gfn,
* add guest page to the tracking pool so that corresponding access on that
* page will be intercepted.
*
- * It should be called under the protection both of mmu-lock and kvm->srcu
- * or kvm->slots_lock.
- *
* @kvm: the guest instance we are interested in.
* @slot: the @gfn belongs to.
* @gfn: the guest page.
@@ -87,6 +85,11 @@ static void update_gfn_write_track(struct kvm_memory_slot
*slot, gfn_t gfn,
void kvm_write_track_add_gfn(struct kvm *kvm, struct kvm_memory_slot *slot,
gfn_t gfn)
{
+ lockdep_assert_held_write(>mmu_lock);
+
+ lockdep_assert_once(lockdep_is_held(>slots_lock) ||
+ srcu_read_lock_held(>srcu));
+
if (WARN_ON(!kvm_page_track_write_tracking_enabled(kvm)))
return;
@@ -107,9 +110,6 @@ EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
* remove the guest page from the tracking pool which stops the interception
* of corresponding access on that page.
*
- * It should be called under the protection both of mmu-lock and kvm->srcu
- * or kvm->slots_lock.
- *
* @kvm: the guest instance we are interested in.
* @slot: the @gfn belongs to.
* @gfn: the guest page.
@@ -117,6 +117,11 @@ EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
void kvm_write_track_remove_gfn(struct kvm *kvm,
struct kvm_memory_slot *slot, gfn_t gfn)
{
+ lockdep_assert_held_write(>mmu_lock);
+
+ lockdep_assert_once(lockdep_is_held(>slots_lock) ||
+ srcu_read_lock_held(>srcu));
+
if (WARN_ON(!kvm_page_track_write_tracking_enabled(kvm)))
return;
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 26/29] KVM: x86/mmu: Bug the VM if write-tracking is used but not enabled
Bug the VM if something attempts to write-track a gfn, but write-tracking isn't enabled. The VM is doomed (and KVM has an egregious bug) if KVM or KVMGT wants to shadow guest page tables but can't because write-tracking isn't enabled. Tested-by: Yongwei Ma Signed-off-by: Sean Christopherson --- arch/x86/kvm/mmu/page_track.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c index 29ae61f1e303..eedb5889d73e 100644 --- a/arch/x86/kvm/mmu/page_track.c +++ b/arch/x86/kvm/mmu/page_track.c @@ -90,7 +90,7 @@ void kvm_write_track_add_gfn(struct kvm *kvm, struct kvm_memory_slot *slot, lockdep_assert_once(lockdep_is_held(>slots_lock) || srcu_read_lock_held(>srcu)); - if (WARN_ON(!kvm_page_track_write_tracking_enabled(kvm))) + if (KVM_BUG_ON(!kvm_page_track_write_tracking_enabled(kvm), kvm)) return; update_gfn_write_track(slot, gfn, 1); @@ -122,7 +122,7 @@ void kvm_write_track_remove_gfn(struct kvm *kvm, lockdep_assert_once(lockdep_is_held(>slots_lock) || srcu_read_lock_held(>srcu)); - if (WARN_ON(!kvm_page_track_write_tracking_enabled(kvm))) + if (KVM_BUG_ON(!kvm_page_track_write_tracking_enabled(kvm), kvm)) return; update_gfn_write_track(slot, gfn, -1); -- 2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 24/29] KVM: x86/mmu: Rename page-track APIs to reflect the new reality
Rename the page-track APIs to capture that they're all about tracking
writes, now that the facade of supporting multiple modes is gone.
Opportunstically replace "slot" with "gfn" in anticipation of removing
the @slot param from the external APIs.
No functional change intended.
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 8
arch/x86/kvm/mmu/mmu.c| 8
arch/x86/kvm/mmu/page_track.c | 21 +
arch/x86/kvm/mmu/page_track.h | 4 ++--
drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++--
5 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index 9e4ee26d1779..f5c1db36cdb7 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -4,10 +4,10 @@
#include
-void kvm_slot_page_track_add_page(struct kvm *kvm,
- struct kvm_memory_slot *slot, gfn_t gfn);
-void kvm_slot_page_track_remove_page(struct kvm *kvm,
-struct kvm_memory_slot *slot, gfn_t gfn);
+void kvm_write_track_add_gfn(struct kvm *kvm,
+struct kvm_memory_slot *slot, gfn_t gfn);
+void kvm_write_track_remove_gfn(struct kvm *kvm, struct kvm_memory_slot *slot,
+ gfn_t gfn);
#ifdef CONFIG_KVM_EXTERNAL_WRITE_TRACKING
/*
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index b8dce17bffdc..a0309fde3549 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -840,7 +840,7 @@ static void account_shadowed(struct kvm *kvm, struct
kvm_mmu_page *sp)
/* the non-leaf shadow pages are keeping readonly. */
if (sp->role.level > PG_LEVEL_4K)
- return kvm_slot_page_track_add_page(kvm, slot, gfn);
+ return kvm_write_track_add_gfn(kvm, slot, gfn);
kvm_mmu_gfn_disallow_lpage(slot, gfn);
@@ -886,7 +886,7 @@ static void unaccount_shadowed(struct kvm *kvm, struct
kvm_mmu_page *sp)
slots = kvm_memslots_for_spte_role(kvm, sp->role);
slot = __gfn_to_memslot(slots, gfn);
if (sp->role.level > PG_LEVEL_4K)
- return kvm_slot_page_track_remove_page(kvm, slot, gfn);
+ return kvm_write_track_remove_gfn(kvm, slot, gfn);
kvm_mmu_gfn_allow_lpage(slot, gfn);
}
@@ -2830,7 +2830,7 @@ int mmu_try_to_unsync_pages(struct kvm *kvm, const struct
kvm_memory_slot *slot,
* track machinery is used to write-protect upper-level shadow pages,
* i.e. this guards the role.level == 4K assertion below!
*/
- if (kvm_slot_page_track_is_active(kvm, slot, gfn))
+ if (kvm_gfn_is_write_tracked(kvm, slot, gfn))
return -EPERM;
/*
@@ -4231,7 +4231,7 @@ static bool page_fault_handle_page_track(struct kvm_vcpu
*vcpu,
* guest is writing the page which is write tracked which can
* not be fixed by page fault handler.
*/
- if (kvm_slot_page_track_is_active(vcpu->kvm, fault->slot, fault->gfn))
+ if (kvm_gfn_is_write_tracked(vcpu->kvm, fault->slot, fault->gfn))
return true;
return false;
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index cdc6069b8caf..b835ba7f325c 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -84,10 +84,9 @@ static void update_gfn_write_track(struct kvm_memory_slot
*slot, gfn_t gfn,
* @slot: the @gfn belongs to.
* @gfn: the guest page.
*/
-void kvm_slot_page_track_add_page(struct kvm *kvm,
- struct kvm_memory_slot *slot, gfn_t gfn)
+void kvm_write_track_add_gfn(struct kvm *kvm, struct kvm_memory_slot *slot,
+gfn_t gfn)
{
-
if (WARN_ON(!kvm_page_track_write_tracking_enabled(kvm)))
return;
@@ -102,12 +101,11 @@ void kvm_slot_page_track_add_page(struct kvm *kvm,
if (kvm_mmu_slot_gfn_write_protect(kvm, slot, gfn, PG_LEVEL_4K))
kvm_flush_remote_tlbs(kvm);
}
-EXPORT_SYMBOL_GPL(kvm_slot_page_track_add_page);
+EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
/*
* remove the guest page from the tracking pool which stops the interception
- * of corresponding access on that page. It is the opposed operation of
- * kvm_slot_page_track_add_page().
+ * of corresponding access on that page.
*
* It should be called under the protection both of mmu-lock and kvm->srcu
* or kvm->slots_lock.
@@ -116,8 +114,8 @@ EXPORT_SYMBOL_GPL(kvm_slot_page_track_add_page);
* @slot: the @gfn belongs to.
* @gfn: the guest page.
*/
-void kvm_slot_page_track_remove_page(struct kvm *kvm,
-struct kvm_memory_slot *slot, gfn_t gfn)
+void kvm_write_track_remove_gfn(struct kvm *kvm,
+ struct kvm_memory_slot *slot, gfn_t gfn)
{
if
[Intel-gfx] [PATCH v4 28/29] KVM: x86/mmu: Handle KVM bookkeeping in page-track APIs, not callers
Get/put references to KVM when a page-track notifier is (un)registered
instead of relying on the caller to do so. Forcing the caller to do the
bookkeeping is unnecessary and adds one more thing for users to get
wrong, e.g. see commit 9ed1fdee9ee3 ("drm/i915/gvt: Get reference to KVM
iff attachment to VM is successful").
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 11 +--
arch/x86/kvm/mmu/page_track.c | 18 --
drivers/gpu/drm/i915/gvt/kvmgt.c | 17 +++--
3 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index 4afab697e21c..3d040741044b 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -44,12 +44,11 @@ struct kvm_page_track_notifier_node {
struct kvm_page_track_notifier_node *node);
};
-void
-kvm_page_track_register_notifier(struct kvm *kvm,
-struct kvm_page_track_notifier_node *n);
-void
-kvm_page_track_unregister_notifier(struct kvm *kvm,
- struct kvm_page_track_notifier_node *n);
+int kvm_page_track_register_notifier(struct kvm *kvm,
+struct kvm_page_track_notifier_node *n);
+void kvm_page_track_unregister_notifier(struct kvm *kvm,
+ struct kvm_page_track_notifier_node *n);
+
int kvm_write_track_add_gfn(struct kvm *kvm, gfn_t gfn);
int kvm_write_track_remove_gfn(struct kvm *kvm, gfn_t gfn);
#else
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 2a64df38ccab..fd04e618ad2d 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -157,17 +157,22 @@ int kvm_page_track_init(struct kvm *kvm)
* register the notifier so that event interception for the tracked guest
* pages can be received.
*/
-void
-kvm_page_track_register_notifier(struct kvm *kvm,
-struct kvm_page_track_notifier_node *n)
+int kvm_page_track_register_notifier(struct kvm *kvm,
+struct kvm_page_track_notifier_node *n)
{
struct kvm_page_track_notifier_head *head;
+ if (!kvm || kvm->mm != current->mm)
+ return -ESRCH;
+
+ kvm_get_kvm(kvm);
+
head = >arch.track_notifier_head;
write_lock(>mmu_lock);
hlist_add_head_rcu(>node, >track_notifier_list);
write_unlock(>mmu_lock);
+ return 0;
}
EXPORT_SYMBOL_GPL(kvm_page_track_register_notifier);
@@ -175,9 +180,8 @@ EXPORT_SYMBOL_GPL(kvm_page_track_register_notifier);
* stop receiving the event interception. It is the opposed operation of
* kvm_page_track_register_notifier().
*/
-void
-kvm_page_track_unregister_notifier(struct kvm *kvm,
- struct kvm_page_track_notifier_node *n)
+void kvm_page_track_unregister_notifier(struct kvm *kvm,
+ struct kvm_page_track_notifier_node *n)
{
struct kvm_page_track_notifier_head *head;
@@ -187,6 +191,8 @@ kvm_page_track_unregister_notifier(struct kvm *kvm,
hlist_del_rcu(>node);
write_unlock(>mmu_lock);
synchronize_srcu(>track_srcu);
+
+ kvm_put_kvm(kvm);
}
EXPORT_SYMBOL_GPL(kvm_page_track_unregister_notifier);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 21342a93e418..eb50997dd369 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -654,21 +654,19 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
{
struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
-
- if (!vgpu->vfio_device.kvm ||
- vgpu->vfio_device.kvm->mm != current->mm) {
- gvt_vgpu_err("KVM is required to use Intel vGPU\n");
- return -ESRCH;
- }
+ int ret;
if (__kvmgt_vgpu_exist(vgpu))
return -EEXIST;
vgpu->track_node.track_write = kvmgt_page_track_write;
vgpu->track_node.track_remove_region = kvmgt_page_track_remove_region;
- kvm_get_kvm(vgpu->vfio_device.kvm);
- kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
->track_node);
+ ret = kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
+ >track_node);
+ if (ret) {
+ gvt_vgpu_err("KVM is required to use Intel vGPU\n");
+ return ret;
+ }
set_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status);
@@ -703,7 +701,6 @@ static void intel_vgpu_close_device(struct vfio_device
*vfio_dev)
kvm_page_track_unregister_notifier(vgpu->vfio_device.kvm,
[Intel-gfx] [PATCH v4 29/29] drm/i915/gvt: Drop final dependencies on KVM internal details
Open code gpa_to_gfn() in kvmgt_page_track_write() and drop KVMGT's dependency on kvm_host.h, i.e. include only kvm_page_track.h. KVMGT assumes "gfn == gpa >> PAGE_SHIFT" all over the place, including a few lines below in the same function with the same gpa, i.e. there's no reason to use KVM's helper for this one case. No functional change intended. Reviewed-by: Yan Zhao Tested-by: Yongwei Ma Signed-off-by: Sean Christopherson --- drivers/gpu/drm/i915/gvt/gvt.h | 3 ++- drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 2d65800d8e93..53a0a42a50db 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -34,10 +34,11 @@ #define _GVT_H_ #include -#include #include #include +#include + #include "i915_drv.h" #include "intel_gvt.h" diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index eb50997dd369..aaed3969f204 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -1585,7 +1585,7 @@ static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len, mutex_lock(>vgpu_lock); - if (kvmgt_gfn_is_write_protected(info, gpa_to_gfn(gpa))) + if (kvmgt_gfn_is_write_protected(info, gpa >> PAGE_SHIFT)) intel_vgpu_page_track_handler(info, gpa, (void *)val, len); -- 2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 21/29] KVM: x86/mmu: Move KVM-only page-track declarations to internal header
Bury the declaration of the page-track helpers that are intended only for
internal KVM use in a "private" header. In addition to guarding against
unwanted usage of the internal-only helpers, dropping their definitions
avoids exposing other structures that should be KVM-internal, e.g. for
memslots. This is a baby step toward making kvm_host.h a KVM-internal
header in the very distant future.
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 21 ++---
arch/x86/kvm/mmu/mmu.c| 3 ++-
arch/x86/kvm/mmu/page_track.c | 8 +--
arch/x86/kvm/mmu/page_track.h | 33 +++
arch/x86/kvm/x86.c| 1 +
5 files changed, 39 insertions(+), 27 deletions(-)
create mode 100644 arch/x86/kvm/mmu/page_track.h
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index 5c348ffdc194..76c0070dfe2a 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -2,6 +2,8 @@
#ifndef _ASM_X86_KVM_PAGE_TRACK_H
#define _ASM_X86_KVM_PAGE_TRACK_H
+#include
+
enum kvm_page_track_mode {
KVM_PAGE_TRACK_WRITE,
KVM_PAGE_TRACK_MAX,
@@ -46,26 +48,12 @@ struct kvm_page_track_notifier_node {
struct kvm_page_track_notifier_node *node);
};
-int kvm_page_track_init(struct kvm *kvm);
-void kvm_page_track_cleanup(struct kvm *kvm);
-
-bool kvm_page_track_write_tracking_enabled(struct kvm *kvm);
-int kvm_page_track_write_tracking_alloc(struct kvm_memory_slot *slot);
-
-void kvm_page_track_free_memslot(struct kvm_memory_slot *slot);
-int kvm_page_track_create_memslot(struct kvm *kvm,
- struct kvm_memory_slot *slot,
- unsigned long npages);
-
void kvm_slot_page_track_add_page(struct kvm *kvm,
struct kvm_memory_slot *slot, gfn_t gfn,
enum kvm_page_track_mode mode);
void kvm_slot_page_track_remove_page(struct kvm *kvm,
struct kvm_memory_slot *slot, gfn_t gfn,
enum kvm_page_track_mode mode);
-bool kvm_slot_page_track_is_active(struct kvm *kvm,
- const struct kvm_memory_slot *slot,
- gfn_t gfn, enum kvm_page_track_mode mode);
void
kvm_page_track_register_notifier(struct kvm *kvm,
@@ -73,10 +61,5 @@ kvm_page_track_register_notifier(struct kvm *kvm,
void
kvm_page_track_unregister_notifier(struct kvm *kvm,
struct kvm_page_track_notifier_node *n);
-void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
- int bytes);
-void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
-
-bool kvm_page_track_has_external_user(struct kvm *kvm);
#endif
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c1d3ac303964..88923b1eb510 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -25,6 +25,7 @@
#include "kvm_cache_regs.h"
#include "smm.h"
#include "kvm_emulate.h"
+#include "page_track.h"
#include "cpuid.h"
#include "spte.h"
@@ -53,7 +54,7 @@
#include
#include
#include
-#include
+
#include "trace.h"
extern bool itlb_multihit_kvm_mitigation;
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 2a6ab7c455c0..e15329d48f95 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -15,10 +15,9 @@
#include
#include
-#include
-
#include "mmu.h"
#include "mmu_internal.h"
+#include "page_track.h"
bool kvm_page_track_write_tracking_enabled(struct kvm *kvm)
{
@@ -300,8 +299,3 @@ void kvm_page_track_delete_slot(struct kvm *kvm, struct
kvm_memory_slot *slot)
n->track_remove_region(slot->base_gfn, slot->npages, n);
srcu_read_unlock(>track_srcu, idx);
}
-
-bool kvm_page_track_has_external_user(struct kvm *kvm)
-{
- return hlist_empty(>arch.track_notifier_head.track_notifier_list);
-}
diff --git a/arch/x86/kvm/mmu/page_track.h b/arch/x86/kvm/mmu/page_track.h
new file mode 100644
index ..89712f123ad3
--- /dev/null
+++ b/arch/x86/kvm/mmu/page_track.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __KVM_X86_PAGE_TRACK_H
+#define __KVM_X86_PAGE_TRACK_H
+
+#include
+
+#include
+
+int kvm_page_track_init(struct kvm *kvm);
+void kvm_page_track_cleanup(struct kvm *kvm);
+
+bool kvm_page_track_write_tracking_enabled(struct kvm *kvm);
+int kvm_page_track_write_tracking_alloc(struct kvm_memory_slot *slot);
+
+void kvm_page_track_free_memslot(struct kvm_memory_slot *slot);
+int kvm_page_track_create_memslot(struct kvm *kvm,
+ struct kvm_memory_slot *slot,
+ unsigned long npages);
+
+bool
[Intel-gfx] [PATCH v4 22/29] KVM: x86/mmu: Use page-track notifiers iff there are external users
Disable the page-track notifier code at compile time if there are no
external users, i.e. if CONFIG_KVM_EXTERNAL_WRITE_TRACKING=n. KVM itself
now hooks emulated writes directly instead of relying on the page-track
mechanism.
Provide a stub for "struct kvm_page_track_notifier_node" so that including
headers directly from the command line, e.g. for testing include guards,
doesn't fail due to a struct having an incomplete type.
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_host.h | 2 ++
arch/x86/include/asm/kvm_page_track.h | 22 +---
arch/x86/kvm/mmu/page_track.c | 10 -
arch/x86/kvm/mmu/page_track.h | 29 +++
4 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 85605f2497bb..33b1ceb30dd2 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1247,7 +1247,9 @@ struct kvm_arch {
* create an NX huge page (without hanging the guest).
*/
struct list_head possible_nx_huge_pages;
+#ifdef CONFIG_KVM_EXTERNAL_WRITE_TRACKING
struct kvm_page_track_notifier_head track_notifier_head;
+#endif
/*
* Protects marking pages unsync during page faults, as TDP MMU page
* faults only take mmu_lock for read. For simplicity, the unsync
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index 76c0070dfe2a..61adb07b5927 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -9,6 +9,14 @@ enum kvm_page_track_mode {
KVM_PAGE_TRACK_MAX,
};
+void kvm_slot_page_track_add_page(struct kvm *kvm,
+ struct kvm_memory_slot *slot, gfn_t gfn,
+ enum kvm_page_track_mode mode);
+void kvm_slot_page_track_remove_page(struct kvm *kvm,
+struct kvm_memory_slot *slot, gfn_t gfn,
+enum kvm_page_track_mode mode);
+
+#ifdef CONFIG_KVM_EXTERNAL_WRITE_TRACKING
/*
* The notifier represented by @kvm_page_track_notifier_node is linked into
* the head which will be notified when guest is triggering the track event.
@@ -48,18 +56,18 @@ struct kvm_page_track_notifier_node {
struct kvm_page_track_notifier_node *node);
};
-void kvm_slot_page_track_add_page(struct kvm *kvm,
- struct kvm_memory_slot *slot, gfn_t gfn,
- enum kvm_page_track_mode mode);
-void kvm_slot_page_track_remove_page(struct kvm *kvm,
-struct kvm_memory_slot *slot, gfn_t gfn,
-enum kvm_page_track_mode mode);
-
void
kvm_page_track_register_notifier(struct kvm *kvm,
struct kvm_page_track_notifier_node *n);
void
kvm_page_track_unregister_notifier(struct kvm *kvm,
struct kvm_page_track_notifier_node *n);
+#else
+/*
+ * Allow defining a node in a structure even if page tracking is disabled, e.g.
+ * to play nice with testing headers via direct inclusion from the command
line.
+ */
+struct kvm_page_track_notifier_node {};
+#endif /* CONFIG_KVM_EXTERNAL_WRITE_TRACKING */
#endif
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index e15329d48f95..b20aad7ac3fe 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -194,6 +194,7 @@ bool kvm_slot_page_track_is_active(struct kvm *kvm,
return !!READ_ONCE(slot->arch.gfn_track[mode][index]);
}
+#ifdef CONFIG_KVM_EXTERNAL_WRITE_TRACKING
void kvm_page_track_cleanup(struct kvm *kvm)
{
struct kvm_page_track_notifier_head *head;
@@ -255,14 +256,13 @@ EXPORT_SYMBOL_GPL(kvm_page_track_unregister_notifier);
* The node should figure out if the written page is the one that node is
* interested in by itself.
*/
-void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
- int bytes)
+void __kvm_page_track_write(struct kvm *kvm, gpa_t gpa, const u8 *new, int
bytes)
{
struct kvm_page_track_notifier_head *head;
struct kvm_page_track_notifier_node *n;
int idx;
- head = >kvm->arch.track_notifier_head;
+ head = >arch.track_notifier_head;
if (hlist_empty(>track_notifier_list))
return;
@@ -273,8 +273,6 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,
const u8 *new,
if (n->track_write)
n->track_write(gpa, new, bytes, n);
srcu_read_unlock(>track_srcu, idx);
-
- kvm_mmu_track_write(vcpu, gpa, new, bytes);
}
/*
@@ -299,3 +297,5 @@ void kvm_page_track_delete_slot(struct kvm *kvm, struct
kvm_memory_slot *slot)
[Intel-gfx] [PATCH v4 20/29] KVM: x86: Remove the unused page-track hook track_flush_slot()
From: Yan Zhao
Remove ->track_remove_slot(), there are no longer any users and it's
unlikely a "flush" hook will ever be the correct API to provide to an
external page-track user.
Cc: Zhenyu Wang
Suggested-by: Sean Christopherson
Signed-off-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 11 ---
arch/x86/kvm/mmu/mmu.c| 2 --
arch/x86/kvm/mmu/page_track.c | 26 --
3 files changed, 39 deletions(-)
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index cfd36c22b467..5c348ffdc194 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -33,16 +33,6 @@ struct kvm_page_track_notifier_node {
*/
void (*track_write)(gpa_t gpa, const u8 *new, int bytes,
struct kvm_page_track_notifier_node *node);
- /*
-* It is called when memory slot is being moved or removed
-* users can drop write-protection for the pages in that memory slot
-*
-* @kvm: the kvm where memory slot being moved or removed
-* @slot: the memory slot being moved or removed
-* @node: this node
-*/
- void (*track_flush_slot)(struct kvm *kvm, struct kvm_memory_slot *slot,
- struct kvm_page_track_notifier_node *node);
/*
* Invoked when a memory region is removed from the guest. Or in KVM
@@ -85,7 +75,6 @@ kvm_page_track_unregister_notifier(struct kvm *kvm,
struct kvm_page_track_notifier_node *n);
void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
int bytes);
-void kvm_page_track_flush_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
bool kvm_page_track_has_external_user(struct kvm *kvm);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c404264f8de5..c1d3ac303964 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6749,8 +6749,6 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
struct kvm_memory_slot *slot)
{
kvm_mmu_zap_all_fast(kvm);
-
- kvm_page_track_flush_slot(kvm, slot);
}
void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, u64 gen)
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index d971c28be99d..2a6ab7c455c0 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -278,32 +278,6 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t
gpa, const u8 *new,
kvm_mmu_track_write(vcpu, gpa, new, bytes);
}
-/*
- * Notify the node that memory slot is being removed or moved so that it can
- * drop write-protection for the pages in the memory slot.
- *
- * The node should figure out it has any write-protected pages in this slot
- * by itself.
- */
-void kvm_page_track_flush_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
-{
- struct kvm_page_track_notifier_head *head;
- struct kvm_page_track_notifier_node *n;
- int idx;
-
- head = >arch.track_notifier_head;
-
- if (hlist_empty(>track_notifier_list))
- return;
-
- idx = srcu_read_lock(>track_srcu);
- hlist_for_each_entry_srcu(n, >track_notifier_list, node,
- srcu_read_lock_held(>track_srcu))
- if (n->track_flush_slot)
- n->track_flush_slot(kvm, slot, n);
- srcu_read_unlock(>track_srcu, idx);
-}
-
/*
* Notify external page track nodes that a memory region is being removed from
* the VM, e.g. so that users can free any associated metadata.
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 19/29] drm/i915/gvt: switch from ->track_flush_slot() to ->track_remove_region()
From: Yan Zhao
Switch from the poorly named and flawed ->track_flush_slot() to the newly
introduced ->track_remove_region(). From KVMGT's perspective, the two
hooks are functionally equivalent, the only difference being that
->track_remove_region() is called only when KVM is 100% certain the
memory region will be removed, i.e. is invoked slightly later in KVM's
memslot modification flow.
Cc: Zhenyu Wang
Suggested-by: Sean Christopherson
Signed-off-by: Yan Zhao
[sean: handle name change, massage changelog, rebase]
Tested-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 3ea3cb9eb599..3f2327455d85 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -108,9 +108,8 @@ struct gvt_dma {
static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,
struct kvm_page_track_notifier_node *node);
-static void kvmgt_page_track_flush_slot(struct kvm *kvm,
- struct kvm_memory_slot *slot,
- struct kvm_page_track_notifier_node *node);
+static void kvmgt_page_track_remove_region(gfn_t gfn, unsigned long nr_pages,
+ struct kvm_page_track_notifier_node
*node);
static ssize_t intel_vgpu_show_description(struct mdev_type *mtype, char *buf)
{
@@ -666,7 +665,7 @@ static int intel_vgpu_open_device(struct vfio_device
*vfio_dev)
return -EEXIST;
vgpu->track_node.track_write = kvmgt_page_track_write;
- vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
+ vgpu->track_node.track_remove_region = kvmgt_page_track_remove_region;
kvm_get_kvm(vgpu->vfio_device.kvm);
kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
>track_node);
@@ -1617,22 +1616,20 @@ static void kvmgt_page_track_write(gpa_t gpa, const u8
*val, int len,
mutex_unlock(>vgpu_lock);
}
-static void kvmgt_page_track_flush_slot(struct kvm *kvm,
- struct kvm_memory_slot *slot,
- struct kvm_page_track_notifier_node *node)
+static void kvmgt_page_track_remove_region(gfn_t gfn, unsigned long nr_pages,
+ struct kvm_page_track_notifier_node
*node)
{
unsigned long i;
- gfn_t gfn;
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
mutex_lock(>vgpu_lock);
- for (i = 0; i < slot->npages; i++) {
- gfn = slot->base_gfn + i;
- if (kvmgt_gfn_is_write_protected(info, gfn))
- kvmgt_protect_table_del(info, gfn);
+ for (i = 0; i < nr_pages; i++) {
+ if (kvmgt_gfn_is_write_protected(info, gfn + i))
+ kvmgt_protect_table_del(info, gfn + i);
}
+
mutex_unlock(>vgpu_lock);
}
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 18/29] KVM: x86: Add a new page-track hook to handle memslot deletion
From: Yan Zhao
Add a new page-track hook, track_remove_region(), that is called when a
memslot DELETE operation is about to be committed. The "remove" hook
will be used by KVMGT and will effectively replace the existing
track_flush_slot() altogether now that KVM itself doesn't rely on the
"flush" hook either.
The "flush" hook is flawed as it's invoked before the memslot operation
is guaranteed to succeed, i.e. KVM might ultimately keep the existing
memslot without notifying external page track users, a.k.a. KVMGT. In
practice, this can't currently happen on x86, but there are no guarantees
that won't change in the future, not to mention that "flush" does a very
poor job of describing what is happening.
Pass in the gfn+nr_pages instead of the slot itself so external users,
i.e. KVMGT, don't need to exposed to KVM internals (memslots). This will
help set the stage for additional cleanups to the page-track APIs.
Opportunistically align the existing srcu_read_lock_held() usage so that
the new case doesn't stand out like a sore thumb (and not aligning the
new code makes bots unhappy).
Cc: Zhenyu Wang
Tested-by: Yongwei Ma
Signed-off-by: Yan Zhao
Co-developed-by: Sean Christopherson
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 12
arch/x86/kvm/mmu/page_track.c | 27 +--
arch/x86/kvm/x86.c| 3 +++
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index f744682648e7..cfd36c22b467 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -43,6 +43,17 @@ struct kvm_page_track_notifier_node {
*/
void (*track_flush_slot)(struct kvm *kvm, struct kvm_memory_slot *slot,
struct kvm_page_track_notifier_node *node);
+
+ /*
+* Invoked when a memory region is removed from the guest. Or in KVM
+* terms, when a memslot is deleted.
+*
+* @gfn: base gfn of the region being removed
+* @nr_pages: number of pages in the to-be-removed region
+* @node: this node
+*/
+ void (*track_remove_region)(gfn_t gfn, unsigned long nr_pages,
+ struct kvm_page_track_notifier_node *node);
};
int kvm_page_track_init(struct kvm *kvm);
@@ -75,6 +86,7 @@ kvm_page_track_unregister_notifier(struct kvm *kvm,
void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
int bytes);
void kvm_page_track_flush_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
+void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
bool kvm_page_track_has_external_user(struct kvm *kvm);
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index e6de9638e560..d971c28be99d 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -270,7 +270,7 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,
const u8 *new,
idx = srcu_read_lock(>track_srcu);
hlist_for_each_entry_srcu(n, >track_notifier_list, node,
- srcu_read_lock_held(>track_srcu))
+ srcu_read_lock_held(>track_srcu))
if (n->track_write)
n->track_write(gpa, new, bytes, n);
srcu_read_unlock(>track_srcu, idx);
@@ -298,12 +298,35 @@ void kvm_page_track_flush_slot(struct kvm *kvm, struct
kvm_memory_slot *slot)
idx = srcu_read_lock(>track_srcu);
hlist_for_each_entry_srcu(n, >track_notifier_list, node,
- srcu_read_lock_held(>track_srcu))
+ srcu_read_lock_held(>track_srcu))
if (n->track_flush_slot)
n->track_flush_slot(kvm, slot, n);
srcu_read_unlock(>track_srcu, idx);
}
+/*
+ * Notify external page track nodes that a memory region is being removed from
+ * the VM, e.g. so that users can free any associated metadata.
+ */
+void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
+{
+ struct kvm_page_track_notifier_head *head;
+ struct kvm_page_track_notifier_node *n;
+ int idx;
+
+ head = >arch.track_notifier_head;
+
+ if (hlist_empty(>track_notifier_list))
+ return;
+
+ idx = srcu_read_lock(>track_srcu);
+ hlist_for_each_entry_srcu(n, >track_notifier_list, node,
+ srcu_read_lock_held(>track_srcu))
+ if (n->track_remove_region)
+ n->track_remove_region(slot->base_gfn, slot->npages, n);
+ srcu_read_unlock(>track_srcu, idx);
+}
+
bool kvm_page_track_has_external_user(struct kvm *kvm)
{
return hlist_empty(>arch.track_notifier_head.track_notifier_list);
diff --git a/arch/x86/kvm/x86.c
[Intel-gfx] [PATCH v4 23/29] KVM: x86/mmu: Drop infrastructure for multiple page-track modes
Drop "support" for multiple page-track modes, as there is no evidence
that array-based and refcounted metadata is the optimal solution for
other modes, nor is there any evidence that other use cases, e.g. for
access-tracking, will be a good fit for the page-track machinery in
general.
E.g. one potential use case of access-tracking would be to prevent guest
access to poisoned memory (from the guest's perspective). In that case,
the number of poisoned pages is likely to be a very small percentage of
the guest memory, and there is no need to reference count the number of
access-tracking users, i.e. expanding gfn_track[] for a new mode would be
grossly inefficient. And for poisoned memory, host userspace would also
likely want to trap accesses, e.g. to inject #MC into the guest, and that
isn't currently supported by the page-track framework.
A better alternative for that poisoned page use case is likely a
variation of the proposed per-gfn attributes overlay (linked), which
would allow efficiently tracking the sparse set of poisoned pages, and by
default would exit to userspace on access.
Link: https://lore.kernel.org/all/y2wb48kd0j4vg...@google.com
Cc: Ben Gardon
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_host.h | 12 +--
arch/x86/include/asm/kvm_page_track.h | 11 +--
arch/x86/kvm/mmu/mmu.c| 14 ++--
arch/x86/kvm/mmu/page_track.c | 111 --
arch/x86/kvm/mmu/page_track.h | 3 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +-
6 files changed, 51 insertions(+), 104 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 33b1ceb30dd2..a915e23d61fa 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -288,13 +288,13 @@ struct kvm_kernel_irq_routing_entry;
* kvm_mmu_page_role tracks the properties of a shadow page (where shadow page
* also includes TDP pages) to determine whether or not a page can be used in
* the given MMU context. This is a subset of the overall kvm_cpu_role to
- * minimize the size of kvm_memory_slot.arch.gfn_track, i.e. allows allocating
- * 2 bytes per gfn instead of 4 bytes per gfn.
+ * minimize the size of kvm_memory_slot.arch.gfn_write_track, i.e. allows
+ * allocating 2 bytes per gfn instead of 4 bytes per gfn.
*
* Upper-level shadow pages having gptes are tracked for write-protection via
- * gfn_track. As above, gfn_track is a 16 bit counter, so KVM must not create
- * more than 2^16-1 upper-level shadow pages at a single gfn, otherwise
- * gfn_track will overflow and explosions will ensure.
+ * gfn_write_track. As above, gfn_write_track is a 16 bit counter, so KVM must
+ * not create more than 2^16-1 upper-level shadow pages at a single gfn,
+ * otherwise gfn_write_track will overflow and explosions will ensue.
*
* A unique shadow page (SP) for a gfn is created if and only if an existing SP
* cannot be reused. The ability to reuse a SP is tracked by its role, which
@@ -1005,7 +1005,7 @@ struct kvm_lpage_info {
struct kvm_arch_memory_slot {
struct kvm_rmap_head *rmap[KVM_NR_PAGE_SIZES];
struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
- unsigned short *gfn_track[KVM_PAGE_TRACK_MAX];
+ unsigned short *gfn_write_track;
};
/*
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index 61adb07b5927..9e4ee26d1779 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -4,17 +4,10 @@
#include
-enum kvm_page_track_mode {
- KVM_PAGE_TRACK_WRITE,
- KVM_PAGE_TRACK_MAX,
-};
-
void kvm_slot_page_track_add_page(struct kvm *kvm,
- struct kvm_memory_slot *slot, gfn_t gfn,
- enum kvm_page_track_mode mode);
+ struct kvm_memory_slot *slot, gfn_t gfn);
void kvm_slot_page_track_remove_page(struct kvm *kvm,
-struct kvm_memory_slot *slot, gfn_t gfn,
-enum kvm_page_track_mode mode);
+struct kvm_memory_slot *slot, gfn_t gfn);
#ifdef CONFIG_KVM_EXTERNAL_WRITE_TRACKING
/*
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 88923b1eb510..b8dce17bffdc 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -840,8 +840,7 @@ static void account_shadowed(struct kvm *kvm, struct
kvm_mmu_page *sp)
/* the non-leaf shadow pages are keeping readonly. */
if (sp->role.level > PG_LEVEL_4K)
- return kvm_slot_page_track_add_page(kvm, slot, gfn,
- KVM_PAGE_TRACK_WRITE);
+ return kvm_slot_page_track_add_page(kvm, slot, gfn);
kvm_mmu_gfn_disallow_lpage(slot, gfn);
@@ -887,8 +886,7 @@ static void unaccount_shadowed(struct kvm *kvm, struct
[Intel-gfx] [PATCH v4 16/29] KVM: x86: Reject memslot MOVE operations if KVMGT is attached
Disallow moving memslots if the VM has external page-track users, i.e. if
KVMGT is being used to expose a virtual GPU to the guest, as KVMGT doesn't
correctly handle moving memory regions.
Note, this is potential ABI breakage! E.g. userspace could move regions
that aren't shadowed by KVMGT without harming the guest. However, the
only known user of KVMGT is QEMU, and QEMU doesn't move generic memory
regions. KVM's own support for moving memory regions was also broken for
multiple years (albeit for an edge case, but arguably moving RAM is
itself an edge case), e.g. see commit edd4fa37baa6 ("KVM: x86: Allocate
new rmap and large page tracking when moving memslot").
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 3 +++
arch/x86/kvm/mmu/page_track.c | 5 +
arch/x86/kvm/x86.c| 7 +++
3 files changed, 15 insertions(+)
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index 8c4d216e3b2b..f744682648e7 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -75,4 +75,7 @@ kvm_page_track_unregister_notifier(struct kvm *kvm,
void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
int bytes);
void kvm_page_track_flush_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
+
+bool kvm_page_track_has_external_user(struct kvm *kvm);
+
#endif
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 891e5cc52b45..e6de9638e560 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -303,3 +303,8 @@ void kvm_page_track_flush_slot(struct kvm *kvm, struct
kvm_memory_slot *slot)
n->track_flush_slot(kvm, slot, n);
srcu_read_unlock(>track_srcu, idx);
}
+
+bool kvm_page_track_has_external_user(struct kvm *kvm)
+{
+ return hlist_empty(>arch.track_notifier_head.track_notifier_list);
+}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 059571d5abed..4394bb49051f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12606,6 +12606,13 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
struct kvm_memory_slot *new,
enum kvm_mr_change change)
{
+ /*
+* KVM doesn't support moving memslots when there are external page
+* trackers attached to the VM, i.e. if KVMGT is in use.
+*/
+ if (change == KVM_MR_MOVE && kvm_page_track_has_external_user(kvm))
+ return -EINVAL;
+
if (change == KVM_MR_CREATE || change == KVM_MR_MOVE) {
if ((new->base_gfn + new->npages - 1) > kvm_mmu_max_gfn())
return -EINVAL;
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 15/29] KVM: drm/i915/gvt: Drop @vcpu from KVM's ->track_write() hook
Drop @vcpu from KVM's ->track_write() hook provided for external users of
the page-track APIs now that KVM itself doesn't use the page-track
mechanism.
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_page_track.h | 5 ++---
arch/x86/kvm/mmu/page_track.c | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 10 --
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/kvm_page_track.h
b/arch/x86/include/asm/kvm_page_track.h
index eb186bc57f6a..8c4d216e3b2b 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -26,14 +26,13 @@ struct kvm_page_track_notifier_node {
* It is called when guest is writing the write-tracked page
* and write emulation is finished at that time.
*
-* @vcpu: the vcpu where the write access happened.
* @gpa: the physical address written by guest.
* @new: the data was written to the address.
* @bytes: the written length.
* @node: this node
*/
- void (*track_write)(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
- int bytes, struct kvm_page_track_notifier_node
*node);
+ void (*track_write)(gpa_t gpa, const u8 *new, int bytes,
+ struct kvm_page_track_notifier_node *node);
/*
* It is called when memory slot is being moved or removed
* users can drop write-protection for the pages in that memory slot
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 23088c90d2fd..891e5cc52b45 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -272,7 +272,7 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,
const u8 *new,
hlist_for_each_entry_srcu(n, >track_notifier_list, node,
srcu_read_lock_held(>track_srcu))
if (n->track_write)
- n->track_write(vcpu, gpa, new, bytes, n);
+ n->track_write(gpa, new, bytes, n);
srcu_read_unlock(>track_srcu, idx);
kvm_mmu_track_write(vcpu, gpa, new, bytes);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 034be0655daa..e9276500435d 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -106,9 +106,8 @@ struct gvt_dma {
#define vfio_dev_to_vgpu(vfio_dev) \
container_of((vfio_dev), struct intel_vgpu, vfio_device)
-static void kvmgt_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,
- const u8 *val, int len,
- struct kvm_page_track_notifier_node *node);
+static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,
+ struct kvm_page_track_notifier_node *node);
static void kvmgt_page_track_flush_slot(struct kvm *kvm,
struct kvm_memory_slot *slot,
struct kvm_page_track_notifier_node *node);
@@ -1603,9 +1602,8 @@ int intel_gvt_page_track_remove(struct intel_vgpu *info,
u64 gfn)
return 0;
}
-static void kvmgt_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,
- const u8 *val, int len,
- struct kvm_page_track_notifier_node *node)
+static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,
+ struct kvm_page_track_notifier_node *node)
{
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 17/29] drm/i915/gvt: Don't bother removing write-protection on to-be-deleted slot
When handling a slot "flush", don't call back into KVM to drop write
protection for gfns in the slot. Now that KVM rejects attempts to move
memory slots while KVMGT is attached, the only time a slot is "flushed"
is when it's being removed, i.e. the memslot and all its write-tracking
metadata is about to be deleted.
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e9276500435d..3ea3cb9eb599 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1630,14 +1630,8 @@ static void kvmgt_page_track_flush_slot(struct kvm *kvm,
for (i = 0; i < slot->npages; i++) {
gfn = slot->base_gfn + i;
- if (kvmgt_gfn_is_write_protected(info, gfn)) {
- write_lock(>mmu_lock);
- kvm_slot_page_track_remove_page(kvm, slot, gfn,
- KVM_PAGE_TRACK_WRITE);
- write_unlock(>mmu_lock);
-
+ if (kvmgt_gfn_is_write_protected(info, gfn))
kvmgt_protect_table_del(info, gfn);
- }
}
mutex_unlock(>vgpu_lock);
}
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 14/29] KVM: x86/mmu: Don't bounce through page-track mechanism for guest PTEs
Don't use the generic page-track mechanism to handle writes to guest PTEs
in KVM's MMU. KVM's MMU needs access to information that should not be
exposed to external page-track users, e.g. KVM needs (for some definitions
of "need") the vCPU to query the current paging mode, whereas external
users, i.e. KVMGT, have no ties to the current vCPU and so should never
need the vCPU.
Moving away from the page-track mechanism will allow dropping use of the
page-track mechanism for KVM's own MMU, and will also allow simplifying
and cleaning up the page-track APIs.
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_host.h | 1 -
arch/x86/kvm/mmu.h | 2 ++
arch/x86/kvm/mmu/mmu.c | 13 ++---
arch/x86/kvm/mmu/page_track.c | 2 ++
4 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 856ec22aceb6..85605f2497bb 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1247,7 +1247,6 @@ struct kvm_arch {
* create an NX huge page (without hanging the guest).
*/
struct list_head possible_nx_huge_pages;
- struct kvm_page_track_notifier_node mmu_sp_tracker;
struct kvm_page_track_notifier_head track_notifier_head;
/*
* Protects marking pages unsync during page faults, as TDP MMU page
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 92d5a1924fc1..253fb2093d5d 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -121,6 +121,8 @@ void kvm_mmu_unload(struct kvm_vcpu *vcpu);
void kvm_mmu_free_obsolete_roots(struct kvm_vcpu *vcpu);
void kvm_mmu_sync_roots(struct kvm_vcpu *vcpu);
void kvm_mmu_sync_prev_roots(struct kvm_vcpu *vcpu);
+void kvm_mmu_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
+int bytes);
static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
{
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 79ea57396d97..c404264f8de5 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5684,9 +5684,8 @@ static u64 *get_written_sptes(struct kvm_mmu_page *sp,
gpa_t gpa, int *nspte)
return spte;
}
-static void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
- const u8 *new, int bytes,
- struct kvm_page_track_notifier_node *node)
+void kvm_mmu_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
+int bytes)
{
gfn_t gfn = gpa >> PAGE_SHIFT;
struct kvm_mmu_page *sp;
@@ -6201,7 +6200,6 @@ static bool kvm_has_zapped_obsolete_pages(struct kvm *kvm)
int kvm_mmu_init_vm(struct kvm *kvm)
{
- struct kvm_page_track_notifier_node *node = >arch.mmu_sp_tracker;
int r;
INIT_LIST_HEAD(>arch.active_mmu_pages);
@@ -6215,9 +6213,6 @@ int kvm_mmu_init_vm(struct kvm *kvm)
return r;
}
- node->track_write = kvm_mmu_pte_write;
- kvm_page_track_register_notifier(kvm, node);
-
kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache;
kvm->arch.split_page_header_cache.gfp_zero = __GFP_ZERO;
@@ -6238,10 +6233,6 @@ static void mmu_free_vm_memory_caches(struct kvm *kvm)
void kvm_mmu_uninit_vm(struct kvm *kvm)
{
- struct kvm_page_track_notifier_node *node = >arch.mmu_sp_tracker;
-
- kvm_page_track_unregister_notifier(kvm, node);
-
if (tdp_mmu_enabled)
kvm_mmu_uninit_tdp_mmu(kvm);
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 0a2ac438d647..23088c90d2fd 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -274,6 +274,8 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa,
const u8 *new,
if (n->track_write)
n->track_write(vcpu, gpa, new, bytes, n);
srcu_read_unlock(>track_srcu, idx);
+
+ kvm_mmu_track_write(vcpu, gpa, new, bytes);
}
/*
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 12/29] KVM: x86/mmu: Move kvm_arch_flush_shadow_{all, memslot}() to mmu.c
Move x86's implementation of kvm_arch_flush_shadow_{all,memslot}() into
mmu.c, and make kvm_mmu_zap_all() static as it was globally visible only
for kvm_arch_flush_shadow_all(). This will allow refactoring
kvm_arch_flush_shadow_memslot() to call kvm_mmu_zap_all() directly without
having to expose kvm_mmu_zap_all_fast() outside of mmu.c. Keeping
everything in mmu.c will also likely simplify supporting TDX, which
intends to do zap only relevant SPTEs on memslot updates.
No functional change intended.
Suggested-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/include/asm/kvm_host.h | 1 -
arch/x86/kvm/mmu/mmu.c | 13 -
arch/x86/kvm/x86.c | 11 ---
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 28bd38303d70..856ec22aceb6 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1832,7 +1832,6 @@ void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
const struct kvm_memory_slot *memslot);
void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm,
const struct kvm_memory_slot *memslot);
-void kvm_mmu_zap_all(struct kvm *kvm);
void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, u64 gen);
void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned long kvm_nr_mmu_pages);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index ec169f5c7dce..c6dee659d592 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6732,7 +6732,7 @@ void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm,
*/
}
-void kvm_mmu_zap_all(struct kvm *kvm)
+static void kvm_mmu_zap_all(struct kvm *kvm)
{
struct kvm_mmu_page *sp, *node;
LIST_HEAD(invalid_list);
@@ -6757,6 +6757,17 @@ void kvm_mmu_zap_all(struct kvm *kvm)
write_unlock(>mmu_lock);
}
+void kvm_arch_flush_shadow_all(struct kvm *kvm)
+{
+ kvm_mmu_zap_all(kvm);
+}
+
+void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
+ struct kvm_memory_slot *slot)
+{
+ kvm_page_track_flush_slot(kvm, slot);
+}
+
void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, u64 gen)
{
WARN_ON(gen & KVM_MEMSLOT_GEN_UPDATE_IN_PROGRESS);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a6b9bea62fb8..059571d5abed 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12776,17 +12776,6 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
kvm_arch_free_memslot(kvm, old);
}
-void kvm_arch_flush_shadow_all(struct kvm *kvm)
-{
- kvm_mmu_zap_all(kvm);
-}
-
-void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
- struct kvm_memory_slot *slot)
-{
- kvm_page_track_flush_slot(kvm, slot);
-}
-
static inline bool kvm_guest_apic_has_interrupt(struct kvm_vcpu *vcpu)
{
return (is_guest_mode(vcpu) &&
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 11/29] drm/i915/gvt: Protect gfn hash table with vgpu_lock
Use vgpu_lock instead of KVM's mmu_lock to protect accesses to the hash
table used to track which gfns are write-protected when shadowing the
guest's GTT, and hoist the acquisition of vgpu_lock from
intel_vgpu_page_track_handler() out to its sole caller,
kvmgt_page_track_write().
This fixes a bug where kvmgt_page_track_write(), which doesn't hold
kvm->mmu_lock, could race with intel_gvt_page_track_remove() and trigger
a use-after-free.
Fixing kvmgt_page_track_write() by taking kvm->mmu_lock is not an option
as mmu_lock is a r/w spinlock, and intel_vgpu_page_track_handler() might
sleep when acquiring vgpu->cache_lock deep down the callstack:
intel_vgpu_page_track_handler()
|
|-> page_track->handler / ppgtt_write_protection_handler()
|
|-> ppgtt_handle_guest_write_page_table_bytes()
|
|-> ppgtt_handle_guest_write_page_table()
|
|-> ppgtt_handle_guest_entry_removal()
|
|-> ppgtt_invalidate_pte()
|
|-> intel_gvt_dma_unmap_guest_page()
|
|-> mutex_lock(>cache_lock);
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 55 +++
drivers/gpu/drm/i915/gvt/page_track.c | 10 +
2 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 6f52886c4051..034be0655daa 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -352,6 +352,8 @@ __kvmgt_protect_table_find(struct intel_vgpu *info, gfn_t
gfn)
{
struct kvmgt_pgfn *p, *res = NULL;
+ lockdep_assert_held(>vgpu_lock);
+
hash_for_each_possible(info->ptable, p, hnode, gfn) {
if (gfn == p->gfn) {
res = p;
@@ -1553,6 +1555,9 @@ int intel_gvt_page_track_add(struct intel_vgpu *info, u64
gfn)
if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, info->status))
return -ESRCH;
+ if (kvmgt_gfn_is_write_protected(info, gfn))
+ return 0;
+
idx = srcu_read_lock(>srcu);
slot = gfn_to_memslot(kvm, gfn);
if (!slot) {
@@ -1561,16 +1566,12 @@ int intel_gvt_page_track_add(struct intel_vgpu *info,
u64 gfn)
}
write_lock(>mmu_lock);
-
- if (kvmgt_gfn_is_write_protected(info, gfn))
- goto out;
-
kvm_slot_page_track_add_page(kvm, slot, gfn, KVM_PAGE_TRACK_WRITE);
+ write_unlock(>mmu_lock);
+
+ srcu_read_unlock(>srcu, idx);
+
kvmgt_protect_table_add(info, gfn);
-
-out:
- write_unlock(>mmu_lock);
- srcu_read_unlock(>srcu, idx);
return 0;
}
@@ -1583,24 +1584,22 @@ int intel_gvt_page_track_remove(struct intel_vgpu
*info, u64 gfn)
if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, info->status))
return -ESRCH;
- idx = srcu_read_lock(>srcu);
- slot = gfn_to_memslot(kvm, gfn);
- if (!slot) {
- srcu_read_unlock(>srcu, idx);
- return -EINVAL;
- }
-
- write_lock(>mmu_lock);
-
if (!kvmgt_gfn_is_write_protected(info, gfn))
- goto out;
+ return 0;
+ idx = srcu_read_lock(>srcu);
+ slot = gfn_to_memslot(kvm, gfn);
+ if (!slot) {
+ srcu_read_unlock(>srcu, idx);
+ return -EINVAL;
+ }
+
+ write_lock(>mmu_lock);
kvm_slot_page_track_remove_page(kvm, slot, gfn, KVM_PAGE_TRACK_WRITE);
+ write_unlock(>mmu_lock);
+ srcu_read_unlock(>srcu, idx);
+
kvmgt_protect_table_del(info, gfn);
-
-out:
- write_unlock(>mmu_lock);
- srcu_read_unlock(>srcu, idx);
return 0;
}
@@ -1611,9 +1610,13 @@ static void kvmgt_page_track_write(struct kvm_vcpu
*vcpu, gpa_t gpa,
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
+ mutex_lock(>vgpu_lock);
+
if (kvmgt_gfn_is_write_protected(info, gpa_to_gfn(gpa)))
intel_vgpu_page_track_handler(info, gpa,
(void *)val, len);
+
+ mutex_unlock(>vgpu_lock);
}
static void kvmgt_page_track_flush_slot(struct kvm *kvm,
@@ -1625,16 +1628,20 @@ static void kvmgt_page_track_flush_slot(struct kvm *kvm,
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
- write_lock(>mmu_lock);
+ mutex_lock(>vgpu_lock);
+
for (i = 0; i < slot->npages; i++) {
gfn = slot->base_gfn + i;
if (kvmgt_gfn_is_write_protected(info, gfn)) {
+ write_lock(>mmu_lock);
kvm_slot_page_track_remove_page(kvm, slot, gfn,
KVM_PAGE_TRACK_WRITE);
+
[Intel-gfx] [PATCH v4 10/29] drm/i915/gvt: Drop unused helper intel_vgpu_reset_gtt()
Drop intel_vgpu_reset_gtt() as it no longer has any callers. In addition
to eliminating dead code, this eliminates the last possible scenario where
__kvmgt_protect_table_find() can be reached without holding vgpu_lock.
Requiring vgpu_lock to be held when calling __kvmgt_protect_table_find()
will allow a protecting the gfn hash with vgpu_lock without too much fuss.
No functional change intended.
Fixes: ba25d977571e ("drm/i915/gvt: Do not destroy ppgtt_mm during vGPU
D3->D0.")
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/gtt.c | 18 --
drivers/gpu/drm/i915/gvt/gtt.h | 1 -
2 files changed, 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index f505be9e647a..c3c623b929ce 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2817,24 +2817,6 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool
invalidate_old)
ggtt_invalidate(gvt->gt);
}
-/**
- * intel_vgpu_reset_gtt - reset the all GTT related status
- * @vgpu: a vGPU
- *
- * This function is called from vfio core to reset reset all
- * GTT related status, including GGTT, PPGTT, scratch page.
- *
- */
-void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
-{
- /* Shadow pages are only created when there is no page
-* table tracking data, so remove page tracking data after
-* removing the shadow pages.
-*/
- intel_vgpu_destroy_all_ppgtt_mm(vgpu);
- intel_vgpu_reset_ggtt(vgpu, true);
-}
-
/**
* intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
* @gvt: intel gvt device
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index a3b0f59ec8bd..4cb183e06e95 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -224,7 +224,6 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool
invalidate_old);
void intel_vgpu_invalidate_ppgtt(struct intel_vgpu *vgpu);
int intel_gvt_init_gtt(struct intel_gvt *gvt);
-void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu);
void intel_gvt_clean_gtt(struct intel_gvt *gvt);
struct intel_vgpu_mm *intel_gvt_find_ppgtt_mm(struct intel_vgpu *vgpu,
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 13/29] KVM: x86/mmu: Don't rely on page-track mechanism to flush on memslot change
Call kvm_mmu_zap_all_fast() directly when flushing a memslot instead of
bouncing through the page-track mechanism. KVM (unfortunately) needs to
zap and flush all page tables on memslot DELETE/MOVE irrespective of
whether KVM is shadowing guest page tables.
This will allow changing KVM to register a page-track notifier on the
first shadow root allocation, and will also allow deleting the misguided
kvm_page_track_flush_slot() hook itself once KVM-GT also moves to a
different method for reacting to memslot changes.
No functional change intended.
Cc: Yan Zhao
Link: https://lore.kernel.org/r/20221110014821.1548347-2-sea...@google.com
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
arch/x86/kvm/mmu/mmu.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c6dee659d592..79ea57396d97 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6199,13 +6199,6 @@ static bool kvm_has_zapped_obsolete_pages(struct kvm
*kvm)
return unlikely(!list_empty_careful(>arch.zapped_obsolete_pages));
}
-static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm,
- struct kvm_memory_slot *slot,
- struct kvm_page_track_notifier_node *node)
-{
- kvm_mmu_zap_all_fast(kvm);
-}
-
int kvm_mmu_init_vm(struct kvm *kvm)
{
struct kvm_page_track_notifier_node *node = >arch.mmu_sp_tracker;
@@ -6223,7 +6216,6 @@ int kvm_mmu_init_vm(struct kvm *kvm)
}
node->track_write = kvm_mmu_pte_write;
- node->track_flush_slot = kvm_mmu_invalidate_zap_pages_in_memslot;
kvm_page_track_register_notifier(kvm, node);
kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache;
@@ -6765,6 +6757,8 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
struct kvm_memory_slot *slot)
{
+ kvm_mmu_zap_all_fast(kvm);
+
kvm_page_track_flush_slot(kvm, slot);
}
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 08/29] drm/i915/gvt: Don't rely on KVM's gfn_to_pfn() to query possible 2M GTT
Now that gvt_pin_guest_page() explicitly verifies the pinned PFN is a
transparent hugepage page, don't use KVM's gfn_to_pfn() to pre-check if a
2MiB GTT entry is possible and instead just try to map the GFN with a 2MiB
entry. Using KVM to query pfn that is ultimately managed through VFIO is
odd, and KVM's gfn_to_pfn() is not intended for non-KVM consumption; it's
exported only because of KVM vendor modules (x86 and PPC).
Open code the check on 2MiB support instead of keeping
is_2MB_gtt_possible() around for a single line of code.
Move the call to intel_gvt_dma_map_guest_page() for a 4KiB entry into its
case statement, i.e. fork the common path into the 4KiB and 2MiB "direct"
shadow paths. Keeping the call in the "common" path is arguably more in
the spirit of "one change per patch", but retaining the local "page_size"
variable is silly, i.e. the call site will be changed either way, and
jumping around the no-longer-common code is more subtle and rather odd,
i.e. would just need to be immediately cleaned up.
Drop the error message from gvt_pin_guest_page() when KVMGT attempts to
shadow a 2MiB guest page that isn't backed by a compatible hugepage in the
host. Dropping the pre-check on a THP makes it much more likely that the
"error" will be encountered in normal operation.
Reviewed-by: Yan Zhao
Tested-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/gtt.c | 49 ++--
drivers/gpu/drm/i915/gvt/kvmgt.c | 1 -
2 files changed, 8 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 61e38acee2d5..f505be9e647a 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1145,36 +1145,6 @@ static inline void ppgtt_generate_shadow_entry(struct
intel_gvt_gtt_entry *se,
ops->set_pfn(se, s->shadow_page.mfn);
}
-/*
- * Check if can do 2M page
- * @vgpu: target vgpu
- * @entry: target pfn's gtt entry
- *
- * Return 1 if 2MB huge gtt shadowing is possible, 0 if miscondition,
- * negative if found err.
- */
-static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
- struct intel_gvt_gtt_entry *entry)
-{
- const struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
- kvm_pfn_t pfn;
- int ret;
-
- if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M))
- return 0;
-
- pfn = gfn_to_pfn(vgpu->vfio_device.kvm, ops->get_pfn(entry));
- if (is_error_noslot_pfn(pfn))
- return -EINVAL;
-
- if (!pfn_valid(pfn))
- return -EINVAL;
-
- ret = PageTransHuge(pfn_to_page(pfn));
- kvm_release_pfn_clean(pfn);
- return ret;
-}
-
static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
struct intel_vgpu_ppgtt_spt *spt, unsigned long index,
struct intel_gvt_gtt_entry *se)
@@ -1268,7 +1238,7 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu
*vgpu,
{
const struct intel_gvt_gtt_pte_ops *pte_ops = vgpu->gvt->gtt.pte_ops;
struct intel_gvt_gtt_entry se = *ge;
- unsigned long gfn, page_size = PAGE_SIZE;
+ unsigned long gfn;
dma_addr_t dma_addr;
int ret;
@@ -1283,6 +1253,9 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu
*vgpu,
switch (ge->type) {
case GTT_TYPE_PPGTT_PTE_4K_ENTRY:
gvt_vdbg_mm("shadow 4K gtt entry\n");
+ ret = intel_gvt_dma_map_guest_page(vgpu, gfn, PAGE_SIZE,
_addr);
+ if (ret)
+ return -ENXIO;
break;
case GTT_TYPE_PPGTT_PTE_64K_ENTRY:
gvt_vdbg_mm("shadow 64K gtt entry\n");
@@ -1294,12 +1267,10 @@ static int ppgtt_populate_shadow_entry(struct
intel_vgpu *vgpu,
return split_64KB_gtt_entry(vgpu, spt, index, );
case GTT_TYPE_PPGTT_PTE_2M_ENTRY:
gvt_vdbg_mm("shadow 2M gtt entry\n");
- ret = is_2MB_gtt_possible(vgpu, ge);
- if (ret == 0)
+ if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M)
||
+ intel_gvt_dma_map_guest_page(vgpu, gfn,
+I915_GTT_PAGE_SIZE_2M,
_addr))
return split_2MB_gtt_entry(vgpu, spt, index, );
- else if (ret < 0)
- return ret;
- page_size = I915_GTT_PAGE_SIZE_2M;
break;
case GTT_TYPE_PPGTT_PTE_1G_ENTRY:
gvt_vgpu_err("GVT doesn't support 1GB entry\n");
@@ -1309,11 +1280,7 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu
*vgpu,
return -EINVAL;
}
- /* direct shadow */
- ret = intel_gvt_dma_map_guest_page(vgpu, gfn, page_size, _addr);
- if (ret)
- return -ENXIO;
-
+ /* Successfully shadowed a 4K or 2M page (without splitting). */
pte_ops->set_pfn(, dma_addr >>
[Intel-gfx] [PATCH v4 09/29] drm/i915/gvt: Use an "unsigned long" to iterate over memslot gfns
Use an "unsigned long" instead of an "int" when iterating over the gfns
in a memslot. The number of pages in the memslot is tracked as an
"unsigned long", e.g. KVMGT could theoretically break if a KVM memslot
larger than 16TiB were deleted (2^32 * 4KiB).
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 97c6d3c53710..6f52886c4051 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1620,7 +1620,7 @@ static void kvmgt_page_track_flush_slot(struct kvm *kvm,
struct kvm_memory_slot *slot,
struct kvm_page_track_notifier_node *node)
{
- int i;
+ unsigned long i;
gfn_t gfn;
struct intel_vgpu *info =
container_of(node, struct intel_vgpu, track_node);
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 07/29] drm/i915/gvt: Error out on an attempt to shadowing an unknown GTT entry type
Bail from ppgtt_populate_shadow_entry() if an unexpected GTT entry type is encountered instead of subtly falling through to the common "direct shadow" path. Eliminating the default/error path's reliance on the common handling will allow hoisting intel_gvt_dma_map_guest_page() into the case statements so that the 2MiB case can try intel_gvt_dma_map_guest_page() and fallback to splitting the entry on failure. Reviewed-by: Zhi Wang Tested-by: Yongwei Ma Signed-off-by: Sean Christopherson --- drivers/gpu/drm/i915/gvt/gtt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 2aed31b497c9..61e38acee2d5 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -1306,6 +1306,7 @@ static int ppgtt_populate_shadow_entry(struct intel_vgpu *vgpu, return -EINVAL; default: GEM_BUG_ON(1); + return -EINVAL; } /* direct shadow */ -- 2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 05/29] drm/i915/gvt: Put the page reference obtained by KVM's gfn_to_pfn()
Put the struct page reference acquired by gfn_to_pfn(), KVM's API is that
the caller is ultimately responsible for dropping any reference.
Note, kvm_release_pfn_clean() ensures the pfn is actually a refcounted
struct page before trying to put any references.
Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/gtt.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index f30922c55a0c..5426a27c1b71 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1158,6 +1158,7 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
{
const struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
kvm_pfn_t pfn;
+ int ret;
if (!HAS_PAGE_SIZES(vgpu->gvt->gt->i915, I915_GTT_PAGE_SIZE_2M))
return 0;
@@ -1171,7 +1172,9 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
if (!pfn_valid(pfn))
return -EINVAL;
- return PageTransHuge(pfn_to_page(pfn));
+ ret = PageTransHuge(pfn_to_page(pfn));
+ kvm_release_pfn_clean(pfn);
+ return ret;
}
static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 06/29] drm/i915/gvt: Explicitly check that vGPU is attached before shadowing
Move the check that a vGPU is attacked from is_2MB_gtt_possible() all the
way up to shadow_ppgtt_mm() to avoid unnecessary work, and to make it more
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.
workload_thread() <= pick_next_workload() => INTEL_VGPU_STATUS_ACTIVE
|
-> dispatch_workload()
|
|-> prepare_workload()
|
-> intel_vgpu_sync_oos_pages()
| |
| |-> ppgtt_set_guest_page_sync()
| |
| |-> sync_oos_page()
| |
| |-> ppgtt_populate_shadow_entry()
|
|-> intel_vgpu_flush_post_shadow()
|
1: |-> ppgtt_handle_guest_write_page_table()
|
|-> ppgtt_handle_guest_entry_add()
|
2: | -> ppgtt_populate_spt_by_guest_entry()
||
||-> ppgtt_populate_spt()
||
||-> ppgtt_populate_shadow_entry()
||
||-> ppgtt_populate_spt_by_guest_entry() [see
2]
|
|-> ppgtt_populate_shadow_entry()
kvmgt_page_track_write() <= KVM callback => INTEL_VGPU_STATUS_ATTACHED
|
|-> intel_vgpu_page_track_handler()
|
|-> ppgtt_write_protection_handler()
|
|-> ppgtt_handle_guest_write_page_table_bytes()
|
|-> ppgtt_handle_guest_write_page_table() [see 1]
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/gtt.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
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
[Intel-gfx] [PATCH v4 04/29] drm/i915/gvt: Don't try to unpin an empty page range
From: Yan Zhao Attempt to unpin pages in the error path of gvt_pin_guest_page() if and only if at least one page was successfully pinned. Unpinning doesn't cause functional problems, but vfio_device_container_unpin_pages() rightfully warns about being asked to unpin zero pages. Signed-off-by: Yan Zhao [sean: write changelog] Signed-off-by: Sean Christopherson --- drivers/gpu/drm/i915/gvt/kvmgt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 429f0f993a13..0366a699baf5 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -172,7 +172,8 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, *page = base_page; return 0; err: - gvt_unpin_guest_page(vgpu, gfn, npage * PAGE_SIZE); + if (npage) + gvt_unpin_guest_page(vgpu, gfn, npage * PAGE_SIZE); return ret; } -- 2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 03/29] drm/i915/gvt: Verify hugepages are contiguous in physical address space
When shadowing a GTT entry with a 2M page, verify that the pfns are
contiguous, not just that the struct page pointers are contiguous. The
memory map is virtual contiguous if "CONFIG_FLATMEM=y ||
CONFIG_SPARSEMEM_VMEMMAP=y", but not for "CONFIG_SPARSEMEM=y &&
CONFIG_SPARSEMEM_VMEMMAP=n", so theoretically KVMGT could encounter struct
pages that are virtually contiguous, but not physically contiguous.
In practice, this flaw is likely a non-issue as it would cause functional
problems iff a section isn't 2M aligned _and_ is directly adjacent to
another section with discontiguous pfns.
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index de675d799c7d..429f0f993a13 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -161,7 +161,7 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu,
unsigned long gfn,
if (npage == 0)
base_page = cur_page;
- else if (base_page + npage != cur_page) {
+ else if (page_to_pfn(base_page) + npage !=
page_to_pfn(cur_page)) {
gvt_vgpu_err("The pages are not continuous\n");
ret = -EINVAL;
npage++;
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 01/29] drm/i915/gvt: Verify pfn is "valid" before dereferencing "struct page"
Check that the pfn found by gfn_to_pfn() is actually backed by "struct
page" memory prior to retrieving and dereferencing the page. KVM
supports backing guest memory with VM_PFNMAP, VM_IO, etc., and so
there is no guarantee the pfn returned by gfn_to_pfn() has an associated
"struct page".
Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
Reviewed-by: Yan Zhao
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
drivers/gpu/drm/i915/gvt/gtt.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 4ec85308379a..58b9b316ae46 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1183,6 +1183,10 @@ static int is_2MB_gtt_possible(struct intel_vgpu *vgpu,
pfn = gfn_to_pfn(vgpu->vfio_device.kvm, ops->get_pfn(entry));
if (is_error_noslot_pfn(pfn))
return -EINVAL;
+
+ if (!pfn_valid(pfn))
+ return -EINVAL;
+
return PageTransHuge(pfn_to_page(pfn));
}
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 02/29] drm/i915/gvt: remove interface intel_gvt_is_valid_gfn
From: Yan Zhao
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
[sean: remove now-unused local variables]
Acked-by: Zhi Wang
Tested-by: Yongwei Ma
Signed-off-by: Sean Christopherson
---
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(>srcu);
- ret = kvm_is_visible_gfn(kvm, gfn);
- srcu_read_unlock(>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(, s, );
ppgtt_set_shadow_entry(spt, , i);
} else {
- gfn = ops->get_pfn();
- if (!intel_gvt_is_valid_gfn(vgpu, gfn)) {
- ops->set_pfn(, gvt->gtt.scratch_mfn);
- ppgtt_set_shadow_entry(spt, , i);
- continue;
- }
-
ret = ppgtt_populate_shadow_entry(vgpu, spt, i, );
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(, gvt->gtt.scratch_mfn);
- goto out;
- }
-
ret = intel_gvt_dma_map_guest_page(vgpu, gfn, PAGE_SIZE,
_addr);
if (ret) {
@@ -2359,7 +2326,6 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu
*vgpu, unsigned int off,
ops->clear_present();
}
-out:
ggtt_set_guest_entry(ggtt_mm, , g_gtt_index);
ggtt_get_host_entry(ggtt_mm, , g_gtt_index);
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] [PATCH v4 00/29] drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups
Fix a handful of minor bugs in KVMGT, and overhaul KVM's page-track APIs
to provide a leaner and cleaner interface. The motivation for this
series is to (significantly) reduce the number of KVM APIs that KVMGT
uses, with a long-term goal of making all kvm_host.h headers KVM-internal.
If there are no objections or issues, my plan is to take this through the
KVM tree for 6.6 (I had it ready early last week, and then forgot to actually
post v4, /facepalm).
Thanks much for all the help!
v4:
- Collect tags. [Yongwei, Zhi, Yan]
- Add a patch to fix a benign (other than a WARN) bug where KVMGT would
attempt to unpin an empty range. [Yan]
- Move the check for an attached vGPU all the way up to shadow_ppgtt_mm().
[Zhi]
v3:
- https://lore.kernel.org/all/20230513003600.818142-1-sea...@google.com
- Collect reviewed/tested tags (I apologize if I missed any, I manually
gathered them this time due to a goof in my workflow). [Yan]
- Drop check on max KVM paging size from KVMGT. [Yan]
- Drop the explicit change on THP pages, and instead validate that the
pfns (not struct page pointers) are contiguous. [Yan]
- Fix buggy intel_gvt_dma_map_guest_page() usage by eliminating a helper
for shadowing 2MiB GTT entries. [Yan]
- Move kvm_arch_flush_shadow_{all,memslot}() to mmu.c instead of exposing
kvm_mmu_zap_all_fast() outside of mmu.c. [Yan]
- Fix an alignment goof in hlist_for_each_entry_srcu() usage. [Yan]
- Wrap full definition of external page track structures with
CONFIG_KVM_EXTERNAL_WRITE_TRACKING. [Yan]
v2:
- https://lore.kernel.org/all/20230311002258.852397-1-sea...@google.com
- Reuse vgpu_lock to protect gfn hash instead of introducing a new (and
buggy) mutext. [Yan]
- Remove a spurious return from kvm_page_track_init(). [Yan]
- Take @kvm directly in the inner __kvm_page_track_write(). [Yan]
- Delete the gfn sanity check that relies on kvm_is_visible_gfn() instead
of providing a dedicated interface. [Yan]
v1: https://lore.kernel.org/lkml/20221223005739.1295925-1-sea...@google.com
Sean Christopherson (24):
drm/i915/gvt: Verify pfn is "valid" before dereferencing "struct page"
drm/i915/gvt: Verify hugepages are contiguous in physical address
space
drm/i915/gvt: Put the page reference obtained by KVM's gfn_to_pfn()
drm/i915/gvt: Explicitly check that vGPU is attached before shadowing
drm/i915/gvt: Error out on an attempt to shadowing an unknown GTT
entry type
drm/i915/gvt: Don't rely on KVM's gfn_to_pfn() to query possible 2M
GTT
drm/i915/gvt: Use an "unsigned long" to iterate over memslot gfns
drm/i915/gvt: Drop unused helper intel_vgpu_reset_gtt()
drm/i915/gvt: Protect gfn hash table with vgpu_lock
KVM: x86/mmu: Move kvm_arch_flush_shadow_{all,memslot}() to mmu.c
KVM: x86/mmu: Don't rely on page-track mechanism to flush on memslot
change
KVM: x86/mmu: Don't bounce through page-track mechanism for guest PTEs
KVM: drm/i915/gvt: Drop @vcpu from KVM's ->track_write() hook
KVM: x86: Reject memslot MOVE operations if KVMGT is attached
drm/i915/gvt: Don't bother removing write-protection on to-be-deleted
slot
KVM: x86/mmu: Move KVM-only page-track declarations to internal header
KVM: x86/mmu: Use page-track notifiers iff there are external users
KVM: x86/mmu: Drop infrastructure for multiple page-track modes
KVM: x86/mmu: Rename page-track APIs to reflect the new reality
KVM: x86/mmu: Assert that correct locks are held for page
write-tracking
KVM: x86/mmu: Bug the VM if write-tracking is used but not enabled
KVM: x86/mmu: Drop @slot param from exported/external page-track APIs
KVM: x86/mmu: Handle KVM bookkeeping in page-track APIs, not callers
drm/i915/gvt: Drop final dependencies on KVM internal details
Yan Zhao (5):
drm/i915/gvt: remove interface intel_gvt_is_valid_gfn
drm/i915/gvt: Don't try to unpin an empty page range
KVM: x86: Add a new page-track hook to handle memslot deletion
drm/i915/gvt: switch from ->track_flush_slot() to
->track_remove_region()
KVM: x86: Remove the unused page-track hook track_flush_slot()
arch/x86/include/asm/kvm_host.h | 16 +-
arch/x86/include/asm/kvm_page_track.h | 73 +++-
arch/x86/kvm/mmu.h| 2 +
arch/x86/kvm/mmu/mmu.c| 51 +++--
arch/x86/kvm/mmu/page_track.c | 256 +-
arch/x86/kvm/mmu/page_track.h | 58 ++
arch/x86/kvm/x86.c| 22 +--
drivers/gpu/drm/i915/gvt/gtt.c| 102 ++
drivers/gpu/drm/i915/gvt/gtt.h| 1 -
drivers/gpu/drm/i915/gvt/gvt.h| 3 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 120 +---
drivers/gpu/drm/i915/gvt/page_track.c | 10 +-
12 files changed, 322 insertions(+), 392 deletions(-)
create mode 100644 arch/x86/kvm/mmu/page_track.h
base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
--
2.41.0.487.g6d72f3e995-goog
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/bridge-connector: simplify handling of USB-C DP
== Series Details == Series: drm/bridge-connector: simplify handling of USB-C DP URL : https://patchwork.freedesktop.org/series/121560/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[Intel-gfx] [PATCH 4/4] soc: qcom: pmic_glink: properly describe the DP connector
During the discussion of the DP connectors, it was suggested that USB-C DisplayPort connectors should have DRM_MODE_CONNECTOR_DisplayPort connector type. This follows the example provided by other drivers (AMDGPU, Intel). To distinguish them from native DP ports, they should have the freshly defined USB as a subconnector type. Suggested-by: Simon Ser Cc: Neil Armstrong Signed-off-by: Dmitry Baryshkov --- drivers/soc/qcom/pmic_glink_altmode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c index 1dedacc52aea..9094944c6cc0 100644 --- a/drivers/soc/qcom/pmic_glink_altmode.c +++ b/drivers/soc/qcom/pmic_glink_altmode.c @@ -417,7 +417,8 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev, alt_port->bridge.funcs = _glink_altmode_bridge_funcs; alt_port->bridge.of_node = to_of_node(fwnode); alt_port->bridge.ops = DRM_BRIDGE_OP_HPD; - alt_port->bridge.type = DRM_MODE_CONNECTOR_USB; + alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; + alt_port->bridge.subtype = DRM_MODE_SUBCONNECTOR_USB; ret = devm_drm_bridge_add(dev, _port->bridge); if (ret) -- 2.39.2
[Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
To properly define the USB-C DP altmode connectors, add the USB
subconnector type.
Suggested-by: Simon Ser
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_connector.c | 1 +
include/uapi/drm/drm_mode.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a6066e4a5e9a..9e96b038f5d0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1050,6 +1050,7 @@ static const struct drm_prop_enum_list
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_DisplayPort, "DP"}, /* DP */
{ DRM_MODE_SUBCONNECTOR_Wireless,"Wireless" }, /* DP */
{ DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */
+ { DRM_MODE_SUBCONNECTOR_USB, "USB" }, /* DP */
};
DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 92d96a2b6763..0f74918b011c 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -398,6 +398,7 @@ enum drm_mode_subconnector {
DRM_MODE_SUBCONNECTOR_HDMIA = 11, /*DP */
DRM_MODE_SUBCONNECTOR_Native = 15, /*DP */
DRM_MODE_SUBCONNECTOR_Wireless= 18, /*DP */
+ DRM_MODE_SUBCONNECTOR_USB = 20, /*DP */
};
#define DRM_MODE_CONNECTOR_Unknown 0
--
2.39.2
[Intel-gfx] [PATCH 2/4] drm/bridge-connector: handle subconnector types
If the created connector type supports subconnector type property,
create and attach corresponding it. The default subtype value is 0,
which maps to the DRM_MODE_SUBCONNECTOR_Unknown type.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_bridge_connector.c | 33 +-
include/drm/drm_bridge.h | 4
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 07b5930b1282..a7b92f0d2430 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -329,7 +329,9 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
struct drm_connector *connector;
struct i2c_adapter *ddc = NULL;
struct drm_bridge *bridge, *panel_bridge = NULL;
+ enum drm_mode_subconnector subconnector;
int connector_type;
+ int ret;
bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL);
if (!bridge_connector)
@@ -365,8 +367,10 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
if (bridge->ops & DRM_BRIDGE_OP_MODES)
bridge_connector->bridge_modes = bridge;
- if (!drm_bridge_get_next_bridge(bridge))
+ if (!drm_bridge_get_next_bridge(bridge)) {
connector_type = bridge->type;
+ subconnector = bridge->subtype;
+ }
#ifdef CONFIG_OF
if (!drm_bridge_get_next_bridge(bridge) &&
@@ -399,6 +403,33 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
if (panel_bridge)
drm_panel_bridge_set_orientation(connector, panel_bridge);
+ if (connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+ drm_connector_attach_dp_subconnector_property(connector,
subconnector);
+ } else if (connector_type == DRM_MODE_CONNECTOR_DVII) {
+ ret = drm_mode_create_dvi_i_properties(drm);
+ if (ret)
+ return ERR_PTR(ret);
+
+ drm_object_attach_property(>base,
+
drm->mode_config.dvi_i_subconnector_property,
+ subconnector);
+ } else if (connector_type == DRM_MODE_CONNECTOR_TV) {
+ ret = drm_mode_create_tv_properties(drm,
+ BIT(DRM_MODE_TV_MODE_NTSC) |
+
BIT(DRM_MODE_TV_MODE_NTSC_443) |
+
BIT(DRM_MODE_TV_MODE_NTSC_J) |
+ BIT(DRM_MODE_TV_MODE_PAL) |
+ BIT(DRM_MODE_TV_MODE_PAL_M)
|
+ BIT(DRM_MODE_TV_MODE_PAL_N)
|
+
BIT(DRM_MODE_TV_MODE_SECAM));
+ if (ret)
+ return ERR_PTR(ret);
+
+ drm_object_attach_property(>base,
+
drm->mode_config.tv_subconnector_property,
+ subconnector);
+ }
+
return connector;
}
EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index bf964cdfb330..68b14ac5ac0d 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -739,6 +739,10 @@ struct drm_bridge {
* identifies the type of connected display.
*/
int type;
+ /**
+* @subtype: the subtype of the connector for the DP/TV/DVI-I cases.
+*/
+ enum drm_mode_subconnector subtype;
/**
* @interlace_allowed: Indicate that the bridge can handle interlaced
* modes.
--
2.39.2
[Intel-gfx] [PATCH 1/4] drm: allow specifying default subtype for the DP subconnector property
In the embedded usecases the default subtype depends on the bridge
chain, so it is easier to specify the subtype at the proprety attachment
type rather than specifying it later.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 3 ++-
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ++-
drivers/gpu/drm/drm_connector.c | 6 --
drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
include/drm/drm_connector.h | 3 ++-
5 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index d34037b85cf8..c18459ecd4be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -2022,7 +2022,8 @@ amdgpu_connector_add(struct amdgpu_device *adev,
if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
-
drm_connector_attach_dp_subconnector_property(_connector->base);
+
drm_connector_attach_dp_subconnector_property(_connector->base,
+
DRM_MODE_SUBCONNECTOR_Unknown);
}
return;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 943959012d04..297321f0199e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -759,7 +759,8 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
drm_dp_mst_topology_mgr_init(>mst_mgr,
adev_to_drm(dm->adev),
>dm_dp_aux.aux, 16, 4,
aconnector->connector_id);
- drm_connector_attach_dp_subconnector_property(>base);
+ drm_connector_attach_dp_subconnector_property(>base,
+
DRM_MODE_SUBCONNECTOR_Unknown);
}
int dm_mst_get_pbn_divider(struct dc_link *link)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a3d3e7dc08b2..a6066e4a5e9a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1577,10 +1577,12 @@ EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
/**
* drm_connector_attach_dp_subconnector_property - create subconnector
property for DP
* @connector: drm_connector to attach property
+ * @subtype: initial value for the subconnector type
*
* Called by a driver when DP connector is created.
*/
-void drm_connector_attach_dp_subconnector_property(struct drm_connector
*connector)
+void drm_connector_attach_dp_subconnector_property(struct drm_connector
*connector,
+ enum drm_mode_subconnector
subtype)
{
struct drm_mode_config *mode_config = >dev->mode_config;
@@ -1594,7 +1596,7 @@ void drm_connector_attach_dp_subconnector_property(struct
drm_connector *connect
drm_object_attach_property(>base,
mode_config->dp_subconnector_property,
- DRM_MODE_SUBCONNECTOR_Unknown);
+ subtype);
}
EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 474785110662..5819105187f6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5391,7 +5391,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
enum port port = dp_to_dig_port(intel_dp)->base.port;
if (!intel_dp_is_edp(intel_dp))
- drm_connector_attach_dp_subconnector_property(connector);
+ drm_connector_attach_dp_subconnector_property(connector,
+
DRM_MODE_SUBCONNECTOR_Unknown);
if (!IS_G4X(dev_priv) && port != PORT_A)
intel_attach_force_audio_property(connector);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5a8115dca359..a130a78f6e0f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1990,7 +1990,8 @@ const char *drm_get_hdcp_content_type_name(int val);
int drm_get_tv_mode_from_name(const char *name, size_t len);
int drm_mode_create_dvi_i_properties(struct drm_device *dev);
-void drm_connector_attach_dp_subconnector_property(struct drm_connector
*connector);
+void drm_connector_attach_dp_subconnector_property(struct drm_connector
*connector,
+ enum drm_mode_subconnector
subtype);
int drm_mode_create_tv_margin_properties(struct drm_device *dev);
int drm_mode_create_tv_properties_legacy(struct
[Intel-gfx] [PATCH 0/4] drm/bridge-connector: simplify handling of USB-C DP
During the discussion of DP connetors, it was pointed out that existing DP drivers supporting USB-C altmode (AMDGPU, Intel) use DRM_MODE_CONNECTOR_DisplayPort for such connectors rather than DRM_MODE_CONNECTOR_USB. This patchset attempts to solve this issue. It adds USB to the enum drm_dp_subconnector and then provides a simple yet efficient way to define DP-in-USB subconnector type for the drivers that use drm_bridge_connector. Dmitry Baryshkov (4): drm: allow specifying default subtype for the DP subconnector property drm/bridge-connector: handle subconnector types drm/uapi: document the USB subconnector type soc: qcom: pmic_glink: properly describe the DP connector .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 3 +- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 +- drivers/gpu/drm/drm_bridge_connector.c| 33 ++- drivers/gpu/drm/drm_connector.c | 7 ++-- drivers/gpu/drm/i915/display/intel_dp.c | 3 +- drivers/soc/qcom/pmic_glink_altmode.c | 3 +- include/drm/drm_bridge.h | 4 +++ include/drm/drm_connector.h | 3 +- include/uapi/drm/drm_mode.h | 1 + 9 files changed, 52 insertions(+), 8 deletions(-) -- 2.39.2
Re: [Intel-gfx] PR for ADLP DMC v2.20 and MTL DMC v2.13
On Wed, Jul 26, 2023 at 4:50 PM Gustavo Sousa wrote: > > The following changes since commit b6ea35ff6b9869470a0c68813f1668acb3d356a8: > > copy-firmware: Fix linking directories when using compression (2023-07-25 > 06:53:30 -0400) > > are available in the Git repository at: > > git://anongit.freedesktop.org/drm/drm-firmware dmc-adlp_2.20-mtl_2.13 Pulled and pushed out. josh > > for you to fetch changes up to fd6e13ca2a1141aeede3666f72d2a006a3903fc0: > > i915: Update MTL DMC to v2.13 (2023-07-26 13:59:54 -0300) > > > Gustavo Sousa (2): > i915: Update ADLP DMC to v2.20 > i915: Update MTL DMC to v2.13 > > WHENCE| 4 ++-- > i915/adlp_dmc.bin | Bin 79044 -> 79088 bytes > i915/mtl_dmc.bin | Bin 49104 -> 52164 bytes > 3 files changed, 2 insertions(+), 2 deletions(-)
[Intel-gfx] ✓ Fi.CI.IGT: success for Panel replay phase1 implementation (rev5)
== Series Details == Series: Panel replay phase1 implementation (rev5) URL : https://patchwork.freedesktop.org/series/94470/ State : success == Summary == CI Bug Log - changes from CI_DRM_13436_full -> Patchwork_94470v5_full Summary --- **SUCCESS** No regressions found. Participating hosts (10 -> 10) -- No changes in participating hosts Known issues Here are the changes found in Patchwork_94470v5_full that come from known issues: ### IGT changes ### Issues hit * igt@drm_fdinfo@busy-hang@bcs0: - shard-dg1: NOTRUN -> [SKIP][1] ([i915#8414]) +4 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-dg1-16/igt@drm_fdinfo@busy-h...@bcs0.html * igt@drm_fdinfo@busy-hang@rcs0: - shard-mtlp: NOTRUN -> [SKIP][2] ([i915#8414]) +6 similar issues [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-mtlp-3/igt@drm_fdinfo@busy-h...@rcs0.html * igt@gem_barrier_race@remote-request@rcs0: - shard-apl: [PASS][3] -> [ABORT][4] ([i915#7461] / [i915#8211] / [i915#8234]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-apl7/igt@gem_barrier_race@remote-requ...@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-apl4/igt@gem_barrier_race@remote-requ...@rcs0.html * igt@gem_basic@multigpu-create-close: - shard-mtlp: NOTRUN -> [SKIP][5] ([i915#7697]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-mtlp-8/igt@gem_ba...@multigpu-create-close.html * igt@gem_ctx_persistence@legacy-engines-mixed: - shard-snb: NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#1099]) +1 similar issue [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-snb6/igt@gem_ctx_persiste...@legacy-engines-mixed.html * igt@gem_eio@reset-stress: - shard-dg1: [PASS][7] -> [FAIL][8] ([i915#5784]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-dg1-18/igt@gem_...@reset-stress.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-dg1-18/igt@gem_...@reset-stress.html * igt@gem_exec_await@wide-contexts: - shard-dg2: [PASS][9] -> [TIMEOUT][10] ([i915#5892]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-dg2-1/igt@gem_exec_aw...@wide-contexts.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-dg2-1/igt@gem_exec_aw...@wide-contexts.html * igt@gem_exec_endless@dispatch@bcs0: - shard-dg2: [PASS][11] -> [TIMEOUT][12] ([i915#3778] / [i915#7016] / [i915#7921]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-dg2-1/igt@gem_exec_endless@dispa...@bcs0.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-dg2-5/igt@gem_exec_endless@dispa...@bcs0.html * igt@gem_exec_fair@basic-deadline: - shard-rkl: [PASS][13] -> [FAIL][14] ([i915#2846]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-rkl-2/igt@gem_exec_f...@basic-deadline.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-rkl-1/igt@gem_exec_f...@basic-deadline.html - shard-glk: [PASS][15] -> [FAIL][16] ([i915#2846]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-glk2/igt@gem_exec_f...@basic-deadline.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-glk7/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_fair@basic-none-solo@rcs0: - shard-apl: [PASS][17] -> [FAIL][18] ([i915#2842]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-apl2/igt@gem_exec_fair@basic-none-s...@rcs0.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-apl1/igt@gem_exec_fair@basic-none-s...@rcs0.html * igt@gem_exec_fair@basic-pace-share@rcs0: - shard-tglu: [PASS][19] -> [FAIL][20] ([i915#2842]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-tglu-7/igt@gem_exec_fair@basic-pace-sh...@rcs0.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-tglu-5/igt@gem_exec_fair@basic-pace-sh...@rcs0.html * igt@gem_exec_fair@basic-pace@bcs0: - shard-rkl: [PASS][21] -> [FAIL][22] ([i915#2842]) +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/shard-rkl-2/igt@gem_exec_fair@basic-p...@bcs0.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-rkl-7/igt@gem_exec_fair@basic-p...@bcs0.html * igt@gem_exec_fence@submit67: - shard-mtlp: NOTRUN -> [SKIP][23] ([i915#4812]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/shard-mtlp-3/igt@gem_exec_fe...@submit67.html * igt@gem_exec_reloc@basic-concurrent0: - shard-mtlp: NOTRUN -> [SKIP][24] ([i915#3281]) +1 similar
Re: [Intel-gfx] [PATCH v3 4/6] drm/i915/panelreplay: Initializaton and compute config for panel replay
Hi Animesh,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-tip/drm-tip]
url:
https://github.com/intel-lab-lkp/linux/commits/Animesh-Manna/drm-panelreplay-dpcd-register-definition-for-panelreplay/20230728-205902
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:
https://lore.kernel.org/r/20230728124609.2911830-5-animesh.manna%40intel.com
patch subject: [Intel-gfx] [PATCH v3 4/6] drm/i915/panelreplay: Initializaton
and compute config for panel replay
config: x86_64-randconfig-x001-20230728
(https://download.01.org/0day-ci/archive/20230728/202307282318.evel6esl-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce:
(https://download.01.org/0day-ci/archive/20230728/202307282318.evel6esl-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202307282318.evel6esl-...@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_dp.c:3386:27: warning: overlapping
>> comparisons always evaluate to true [-Wtautological-overlap-compare]
if (vsc->revision != 0x5 || vsc->revision != 0x7)
~^~~
1 warning generated.
vim +3386 drivers/gpu/drm/i915/display/intel_dp.c
3361
3362 static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
3363 struct dp_sdp *sdp, size_t size)
3364 {
3365 size_t length = sizeof(struct dp_sdp);
3366
3367 if (size < length)
3368 return -ENOSPC;
3369
3370 memset(sdp, 0, size);
3371
3372 /*
3373 * Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
3374 * VSC SDP Header Bytes
3375 */
3376 sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */
3377 sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet
Type */
3378 sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */
3379 sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data
Bytes */
3380
3381 /*
3382 * Other than revision 0x5 which supports Pixel
Encoding/Colorimetry
3383 * Format as per DP 1.4a spec, revision 0x7 also supports Pixel
3384 * Encoding/Colorimetry Format as per DP 2.0 spec.
3385 */
> 3386 if (vsc->revision != 0x5 || vsc->revision != 0x7)
3387 goto out;
3388
3389 /* VSC SDP Payload for DB16 through DB18 */
3390 /* Pixel Encoding and Colorimetry Formats */
3391 sdp->db[16] = (vsc->pixelformat & 0xf) << 4; /* DB16[7:4] */
3392 sdp->db[16] |= vsc->colorimetry & 0xf; /* DB16[3:0] */
3393
3394 switch (vsc->bpc) {
3395 case 6:
3396 /* 6bpc: 0x0 */
3397 break;
3398 case 8:
3399 sdp->db[17] = 0x1; /* DB17[3:0] */
3400 break;
3401 case 10:
3402 sdp->db[17] = 0x2;
3403 break;
3404 case 12:
3405 sdp->db[17] = 0x3;
3406 break;
3407 case 16:
3408 sdp->db[17] = 0x4;
3409 break;
3410 default:
3411 MISSING_CASE(vsc->bpc);
3412 break;
3413 }
3414 /* Dynamic Range and Component Bit Depth */
3415 if (vsc->dynamic_range == DP_DYNAMIC_RANGE_CTA)
3416 sdp->db[17] |= 0x80; /* DB17[7] */
3417
3418 /* Content Type */
3419 sdp->db[18] = vsc->content_type & 0x7;
3420
3421 out:
3422 return length;
3423 }
3424
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [Intel-gfx] [PATCH v2] drm/i915/guc/slpc: Restore efficient freq earlier
On Tue, Jul 25, 2023 at 06:00:44PM -0700, Vinay Belgaumkar wrote:
> This should be done before the soft min/max frequencies are restored.
> When we disable the "Ignore efficient frequency" flag, GuC does not
> actually bring the requested freq down to RPn.
>
> Specifically, this scenario-
>
> - ignore efficient freq set to true
> - reduce min to RPn (from efficient)
> - suspend
> - resume (includes GuC load, restore soft min/max, restore efficient freq)
> - validate min freq has been resored to RPn
>
> This will fail if we didn't first restore(disable, in this case) efficient
> freq flag before setting the soft min frequency.
>
> v2: Bring the min freq down to RPn when we disable efficient freq (Rodrigo)
> Also made the change to set the min softlimit to RPn at init. Otherwise, we
> were storing RPe there.
Thanks
Reviewed-by: Rodrigo Vivi
>
> Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8736
> Fixes: 55f9720dbf23 ("drm/i915/guc/slpc: Provide sysfs for efficient freq")
> Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient
> frequency")
> Signed-off-by: Vinay Belgaumkar
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 22 +
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index ee9f83af7cf6..477df260ae3a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -470,12 +470,19 @@ int intel_guc_slpc_set_ignore_eff_freq(struct
> intel_guc_slpc *slpc, bool val)
> ret = slpc_set_param(slpc,
>SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
>val);
> - if (ret)
> + if (ret) {
> guc_probe_error(slpc_to_guc(slpc), "Failed to set efficient
> freq(%d): %pe\n",
> val, ERR_PTR(ret));
> - else
> + } else {
> slpc->ignore_eff_freq = val;
>
> + /* Set min to RPn when we disable efficient freq */
> + if (val)
> + ret = slpc_set_param(slpc,
> +
> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> + slpc->min_freq);
> + }
> +
> intel_runtime_pm_put(>runtime_pm, wakeref);
> mutex_unlock(>lock);
> return ret;
> @@ -602,9 +609,8 @@ static int slpc_set_softlimits(struct intel_guc_slpc
> *slpc)
> return ret;
>
> if (!slpc->min_freq_softlimit) {
> - ret = intel_guc_slpc_get_min_freq(slpc,
> >min_freq_softlimit);
> - if (unlikely(ret))
> - return ret;
> + /* Min softlimit is initialized to RPn */
> + slpc->min_freq_softlimit = slpc->min_freq;
> slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit;
> } else {
> return intel_guc_slpc_set_min_freq(slpc,
> @@ -755,6 +761,9 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
> return ret;
> }
>
> + /* Set cached value of ignore efficient freq */
> + intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq);
> +
> /* Revert SLPC min/max to softlimits if necessary */
> ret = slpc_set_softlimits(slpc);
> if (unlikely(ret)) {
> @@ -765,9 +774,6 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
> /* Set cached media freq ratio mode */
> intel_guc_slpc_set_media_ratio_mode(slpc, slpc->media_ratio_mode);
>
> - /* Set cached value of ignore efficient freq */
> - intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq);
> -
> return 0;
> }
>
> --
> 2.38.1
>
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Hold reference to intel_context over life of i915_request
== Series Details == Series: drm/i915: Hold reference to intel_context over life of i915_request URL : https://patchwork.freedesktop.org/series/121510/ State : success == Summary == CI Bug Log - changes from CI_DRM_13435_full -> Patchwork_121510v1_full Summary --- **SUCCESS** No regressions found. Participating hosts (10 -> 10) -- No changes in participating hosts Known issues Here are the changes found in Patchwork_121510v1_full that come from known issues: ### IGT changes ### Issues hit * igt@api_intel_bb@blit-reloc-purge-cache: - shard-dg2: NOTRUN -> [SKIP][1] ([i915#8411]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-dg2-7/igt@api_intel...@blit-reloc-purge-cache.html - shard-rkl: NOTRUN -> [SKIP][2] ([i915#8411]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-rkl-7/igt@api_intel...@blit-reloc-purge-cache.html * igt@debugfs_test@basic-hwmon: - shard-rkl: NOTRUN -> [SKIP][3] ([i915#7456]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-rkl-7/igt@debugfs_t...@basic-hwmon.html * igt@device_reset@unbind-cold-reset-rebind: - shard-mtlp: NOTRUN -> [SKIP][4] ([i915#7701]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-mtlp-1/igt@device_re...@unbind-cold-reset-rebind.html * igt@drm_fdinfo@busy-hang@rcs0: - shard-mtlp: NOTRUN -> [SKIP][5] ([i915#8414]) +12 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-mtlp-3/igt@drm_fdinfo@busy-h...@rcs0.html * igt@drm_fdinfo@most-busy-idle-check-all@rcs0: - shard-rkl: [PASS][6] -> [FAIL][7] ([i915#7742]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-rkl-7/igt@drm_fdinfo@most-busy-idle-check-...@rcs0.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-rkl-6/igt@drm_fdinfo@most-busy-idle-check-...@rcs0.html * igt@gem_basic@multigpu-create-close: - shard-mtlp: NOTRUN -> [SKIP][8] ([i915#7697]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-mtlp-8/igt@gem_ba...@multigpu-create-close.html * igt@gem_ctx_persistence@legacy-engines-hostile-preempt: - shard-snb: NOTRUN -> [SKIP][9] ([fdo#109271] / [i915#1099]) +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-snb2/igt@gem_ctx_persiste...@legacy-engines-hostile-preempt.html * igt@gem_exec_balancer@invalid-bonds: - shard-dg2: NOTRUN -> [SKIP][10] ([i915#4036]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-dg2-7/igt@gem_exec_balan...@invalid-bonds.html * igt@gem_exec_capture@pi@ccs0: - shard-mtlp: [PASS][11] -> [FAIL][12] ([i915#7765]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-mtlp-6/igt@gem_exec_capture@p...@ccs0.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-mtlp-2/igt@gem_exec_capture@p...@ccs0.html * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][13] -> [FAIL][14] ([i915#2846]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-glk3/igt@gem_exec_f...@basic-deadline.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-glk8/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_fair@basic-none-solo@rcs0: - shard-apl: [PASS][15] -> [FAIL][16] ([i915#2842]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-apl1/igt@gem_exec_fair@basic-none-s...@rcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-apl7/igt@gem_exec_fair@basic-none-s...@rcs0.html * igt@gem_exec_fair@basic-pace-solo@rcs0: - shard-rkl: [PASS][17] -> [FAIL][18] ([i915#2842]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-rkl-1/igt@gem_exec_fair@basic-pace-s...@rcs0.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-rkl-7/igt@gem_exec_fair@basic-pace-s...@rcs0.html * igt@gem_exec_fence@submit67: - shard-mtlp: NOTRUN -> [SKIP][19] ([i915#4812]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-mtlp-3/igt@gem_exec_fe...@submit67.html * igt@gem_exec_flush@basic-wb-ro-default: - shard-dg2: NOTRUN -> [SKIP][20] ([i915#3539] / [i915#4852]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-dg2-7/igt@gem_exec_fl...@basic-wb-ro-default.html * igt@gem_exec_reloc@basic-concurrent0: - shard-mtlp: NOTRUN -> [SKIP][21] ([i915#3281]) +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/shard-mtlp-8/igt@gem_exec_re...@basic-concurrent0.html * igt@gem_exec_reloc@basic-write-gtt: - shard-rkl: NOTRUN ->
Re: [Intel-gfx] [RFC 4/8] drm/i915: Refactor PAT/object cache handling
On Fri, Jul 28, 2023 at 01:39:06PM +0100, Tvrtko Ursulin wrote:
>
> Forgot one part of your reply:
>
> On 28/07/2023 00:57, Matt Roper wrote:
> > On Thu, Jul 27, 2023 at 03:55:00PM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin
> > >
> > > Commit 9275277d5324 ("drm/i915: use pat_index instead of cache_level") has
> > > introduced PAT indices to i915 internal APIs, partially replacing the
> > > usage of driver internal cache_level, but has also added a few sub-
> > > optimal design decisions which this patch tries to improve upon.
> > >
> > > Principal change here is to invert the per platform cache level to PAT
> > > index table which was added by the referenced commit, and by doing so
> > > enable i915 to understand the cache mode between PAT indices, changing
> > > them from opaque to transparent.
> > >
> > > Once we have the inverted table we are able to remove the hidden false
> > > "return true" from i915_gem_object_has_cache_level and make the involved
> > > code path clearer.
> > >
> > > To achieve this we replace the enum i915_cache_level with i915_cache_t,
> > > composed of a more detailed representation of each cache mode (base mode
> > > plus flags).
> > >
> > > In this way we are able to express the differences between different
> > > write-back mode coherency settings on Meteorlake, which in turn enables us
> > > to map the i915 "cached" mode to the correct Meteorlake PAT index.
> > >
> > > We can also replace the platform dependent cache mode to string code in
> > > debugfs and elsewhere by the single implementation based on i915_cache_t.
> > >
> > > v2:
> > > * Fix PAT-to-cache-mode table for PVC. (Fei)
> > > * Cache display caching mode too. (Fei)
> > > * Improve and document criteria in i915_gem_object_can_bypass_llc()
> > > (Matt)
> > >
> > > v3:
> > > * Checkpath issues.
> > > * Cache mode flags check fixed.
> > >
> > > v4:
> > > * Fix intel_device_info->cache_modes array size. (Matt)
> > > * Boolean cache mode and flags query. (Matt)
> > > * Reduce number of cache macros with some macro magic.
> > > * One more checkpatch fix.
> > > * Tweak tables to show legacy and Gen12 WB is fully coherent.
> > >
> > > Signed-off-by: Tvrtko Ursulin
> > > References: 9275277d5324 ("drm/i915: use pat_index instead of
> > > cache_level")
> > > Cc: Chris Wilson
> > > Cc: Fei Yang
> > > Cc: Andi Shyti
> > > Cc: Matt Roper
> > > ---
> > > drivers/gpu/drm/i915/gem/i915_gem_domain.c| 60 +
> > > drivers/gpu/drm/i915/gem/i915_gem_domain.h| 5 +-
> > > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 3 +-
> > > drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +-
> > > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 4 +-
> > > drivers/gpu/drm/i915/gem/i915_gem_object.c| 117 ++
> > > drivers/gpu/drm/i915/gem/i915_gem_object.h| 11 +-
> > > .../gpu/drm/i915/gem/i915_gem_object_types.h | 116 +
> > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 8 +-
> > > drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 2 +-
> > > drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 20 +--
> > > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +-
> > > .../drm/i915/gem/selftests/huge_gem_object.c | 2 +-
> > > .../gpu/drm/i915/gem/selftests/huge_pages.c | 3 +-
> > > drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 10 +-
> > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
> > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 25 ++--
> > > drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 4 +-
> > > drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +-
> > > drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +-
> > > drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +-
> > > .../gpu/drm/i915/gt/intel_ring_submission.c | 4 +-
> > > drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +-
> > > drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
> > > .../gpu/drm/i915/gt/selftest_workarounds.c| 2 +-
> > > drivers/gpu/drm/i915/i915_cache.c | 89 +++--
> > > drivers/gpu/drm/i915/i915_cache.h | 70 ++-
> > > drivers/gpu/drm/i915/i915_debugfs.c | 53 ++--
> > > drivers/gpu/drm/i915/i915_driver.c| 4 +-
> > > drivers/gpu/drm/i915/i915_gem.c | 13 --
> > > drivers/gpu/drm/i915/i915_pci.c | 84 +++--
> > > drivers/gpu/drm/i915/i915_perf.c | 2 +-
> > > drivers/gpu/drm/i915/intel_device_info.h | 6 +-
> > > .../gpu/drm/i915/selftests/i915_gem_evict.c | 4 +-
> > > drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +-
> > > .../gpu/drm/i915/selftests/mock_gem_device.c | 14 +--
> > > 36 files changed, 391 insertions(+), 367 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > > index 57db9c581bf6..c15f83de33af 100644
Re: [Intel-gfx] [PATCH i-g-t 2/3] lib/igt_drm_clients: Store memory info in the client
On 27/07/2023 16:17, Tvrtko Ursulin wrote:
Hi,
On 27/07/2023 15:10, Kamil Konieczny wrote:
Hi Tvrtko,
On 2023-07-27 at 10:20:24 +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Define the storage structure and copy over memory data as parsed by the
fdinfo helpers.
v2:
* Fix empty region map entry skip condition. (Kamil, Chris)
Signed-off-by: Tvrtko Ursulin
Cc: Rob Clark
Cc: Chris Healy
Cc: Kamil Konieczny
---
lib/igt_drm_clients.c | 32
lib/igt_drm_clients.h | 11 +++
2 files changed, 43 insertions(+)
diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c
index fdea42752a81..47ad137d5f1f 100644
--- a/lib/igt_drm_clients.c
+++ b/lib/igt_drm_clients.c
@@ -103,6 +103,8 @@ igt_drm_client_update(struct igt_drm_client *c,
unsigned int pid, char *name,
c->clients->max_name_len = len;
}
+ /* Engines */
+
c->last_runtime = 0;
c->total_runtime = 0;
@@ -118,6 +120,13 @@ igt_drm_client_update(struct igt_drm_client *c,
unsigned int pid, char *name,
c->last[i] = info->busy[i];
}
+ /* Memory regions */
+ for (i = 0; i <= c->regions->max_region_id; i++) {
+ assert(i < ARRAY_SIZE(info->region_mem));
+
+ c->memory[i] = info->region_mem[i];
+ }
+
c->samples++;
c->status = IGT_DRM_CLIENT_ALIVE;
}
@@ -154,6 +163,8 @@ igt_drm_client_add(struct igt_drm_clients *clients,
c->id = info->id;
c->drm_minor = drm_minor;
c->clients = clients;
+
+ /* Engines */
c->engines = malloc(sizeof(*c->engines));
assert(c->engines);
memset(c->engines, 0, sizeof(*c->engines));
@@ -178,6 +189,27 @@ igt_drm_client_add(struct igt_drm_clients *clients,
c->last = calloc(c->engines->max_engine_id + 1, sizeof(c->last));
assert(c->val && c->last);
+ /* Memory regions */
+ c->regions = malloc(sizeof(*c->regions));
+ assert(c->regions);
+ memset(c->regions, 0, sizeof(*c->regions));
+ c->regions->names = calloc(info->last_region_index + 1,
+ sizeof(*c->regions->names));
+ assert(c->regions->names);
+
+ for (i = 0; i <= info->last_region_index; i++) {
+ /* Region map is allowed to be sparse. */
+ if (!info->region_names[i][0])
+ continue;
+
+ c->regions->names[i] = strdup(info->region_names[i]);
- ^
Should this be c->regions->num_regions?
No, it was supposed to carry over the same memory region index from the
region map provided to igt_parse_drm_fdinfo.
I copy pasted that concept from engine names (class names for i915) but,
given it is unused, maybe I should drop it.
Gputop does not need it, at least not yet, and I haven't thought much if
it will be useful for intel_gpu_top. Point is, it allows passing in
fixed region id to name mapping, which can simplify things and guarantee
order of memory regions in the arrays. (Otherwise the order would depend
on the order of keys in the fdinfo text.)
I think I'd like to keep this functionality for now. I do promise to rip
it out later, should I end up not needing it for intel_gpu_top after all.
Regards,
Tvrtko
Re: [Intel-gfx] [RFC 2/8] drm/i915: Split PTE encode between Gen12 and Meteorlake
On Fri, Jul 28, 2023 at 09:18:36AM +0100, Tvrtko Ursulin wrote:
>
> On 27/07/2023 23:25, Matt Roper wrote:
> > On Thu, Jul 27, 2023 at 03:54:58PM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin
> > >
> > > No need to run extra instructions which will never trigger on platforms
> > > before Meteorlake.
> > >
> > > Signed-off-by: Tvrtko Ursulin
> > > ---
> > > drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 26 ++
> > > 1 file changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> > > b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> > > index c8568e5d1147..862ac1d2de25 100644
> > > --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> > > +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> > > @@ -63,6 +63,30 @@ static u64 gen12_pte_encode(dma_addr_t addr,
> > > {
> > > gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW;
> > > + if (unlikely(flags & PTE_READ_ONLY))
> > > + pte &= ~GEN8_PAGE_RW;
> > > +
> > > + if (flags & PTE_LM)
> > > + pte |= GEN12_PPGTT_PTE_LM;
> > > +
> > > + if (pat_index & BIT(0))
> > > + pte |= GEN12_PPGTT_PTE_PAT0;
> > > +
> > > + if (pat_index & BIT(1))
> > > + pte |= GEN12_PPGTT_PTE_PAT1;
> > > +
> > > + if (pat_index & BIT(2))
> > > + pte |= GEN12_PPGTT_PTE_PAT2;
> > > +
> > > + return pte;
> > > +}
> > > +
> > > +static u64 mtl_pte_encode(dma_addr_t addr,
> > > + unsigned int pat_index,
> > > + u32 flags)
> > > +{
> > > + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW;
> > > +
> >
> > Would it be more readable to start with
> >
> > gen8_pte_t pte = gen12_pte_encode(addr, pat_index, flags);
> >
> > and then |-in only the MTL-specific bit(s) as appropriate?
> >
> > > if (unlikely(flags & PTE_READ_ONLY))
> > > pte &= ~GEN8_PAGE_RW;
> > > @@ -995,6 +1019,8 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt
> > > *gt,
> > >*/
> > > ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
> > > + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70))
> > > + ppgtt->vm.pte_encode = mtl_pte_encode;
> > > if (GRAPHICS_VER(gt->i915) >= 12)
> > > ppgtt->vm.pte_encode = gen12_pte_encode;
> >
> > I think you wanted 'else if' here. Otherwise you clobber the MTL
> > function pointer.
>
> Doh this was a strong fail.. Yes and yes.. I even had it like you suggest in
> that patch I mentioned to you earlier..
> https://patchwork.freedesktop.org/patch/546013/?series=120341=2.
>
> Do you have an opinion on that one perhaps?
Yeah, I overlooked that patch before, but it looks good to me.
Matt
>
> Thanks,
>
> Tvrtko
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
Re: [Intel-gfx] [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
On Thursday, July 27th, 2023 at 14:01, Christian König wrote: > > We do need patches to stop trying to infer the node type from the minor > > in libdrm, though. Emil has suggested using sysfs, which we already do > > in a few places in libdrm. > > That sounds like a really good idea to me as well. > > But what do we do with DRM_MAX_MINOR? Change it or keep it and say apps > should use drmGetDevices2() like Emil suggested? DRM_MAX_MINOR has been bumped to 64 now. With the new minor allocation scheme, DRM_MAX_MINOR is meaningless because there is no "max minor per type" concept anymore: the minor no longer contains the type. So I'd suggest leaving it as-is (so that old apps still continue to work on systems with < 64 devices like they do today) and mark it as deprecated.
Re: [Intel-gfx] [PATCH 16/17] cgroup/drm: Expose memory stats
One additional thought on one sub-topic: On 27/07/2023 18:08, Tvrtko Ursulin wrote: [snip] For something like this, you would probably want it to work inside the drm scheduler first. Presumably, this can be done by setting a weight on each runqueue, and perhaps adding a callback to update one for a running queue. Calculating the weights hierarchically might be fun.. It is not needed to work in drm scheduler first. In fact drm scheduler based drivers can plug into what I have since it already has the notion of scheduling priorities. They would only need to implement a hook which allow the cgroup controller to query client GPU utilisation and another to received the over budget signal. Amdgpu and msm AFAIK could be easy candidates because they both support per client utilisation and priorities. Looks like I need to put all this info back into the cover letter. Also, hierarchic weights and time budgets are all already there. What could be done later is make this all smarter and respect the time budget with more precision. That would however, in many cases including Intel, require co-operation with the firmware. In any case it is only work in the implementation, while the cgroup control interface remains the same. I have taken a look at how the rest of cgroup controllers change ownership when moved to a different cgroup, and the answer was: not at all. If we attempt to create the scheduler controls only on the first time the fd is used, you could probably get rid of all the tracking. Can you send a CPU file descriptor from process A to process B and have CPU usage belonging to process B show up in process' A cgroup, or vice-versa? Nope, I am not making any sense, am I? My point being it is not like-to-like, model is different. No ownership transfer would mean in wide deployments all GPU utilisation would be assigned to Xorg and so there is no point to any of this. No way to throttle a cgroup with un-important GPU clients for instance. If you just grab the current process' cgroup when a drm_sched_entity is created, you don't have everything charged to X.org. No need for complicated ownership tracking in drm_file. The same equivalent should be done in i915 as well when a context is created as it's not using the drm scheduler. Okay so essentially nuking the concept of DRM clients belongs to one cgroup and instead tracking at the context level. That is an interesting idea. I suspect implementation could require somewhat generalizing the concept of an "execution context", or at least expressing it via the DRM cgroup controller. I can give this a spin, or at least some more detailed thought, once we close on a few more details regarding charging in general. I didn't get much time to brainstorm this just yet, only one downside randomly came to mind later - with this approach for i915 we wouldn't correctly attribute any GPU activity done in the receiving process against our default contexts. Those would still be accounted to the sending process. How much problem in practice that would be remains to be investigated, including if it applies to other drivers too. If there is a good amount of deployed userspace which use the default context, then it would be a bit messy. Regards, Tvrtko *) For non DRM and non i915 people, default context is a GPU submission context implicitly created during the device node open. It always remains valid, including in the receiving process if SCM_RIGHTS is used.
[Intel-gfx] ✓ Fi.CI.BAT: success for Panel replay phase1 implementation (rev5)
== Series Details == Series: Panel replay phase1 implementation (rev5) URL : https://patchwork.freedesktop.org/series/94470/ State : success == Summary == CI Bug Log - changes from CI_DRM_13436 -> Patchwork_94470v5 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/index.html Participating hosts (42 -> 41) -- Missing(1): fi-snb-2520m Known issues Here are the changes found in Patchwork_94470v5 that come from known issues: ### IGT changes ### Issues hit * igt@i915_module_load@load: - bat-mtlp-8: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/bat-mtlp-8/igt@i915_module_l...@load.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/bat-mtlp-8/igt@i915_module_l...@load.html * igt@i915_selftest@live@execlists: - fi-bsw-nick:[PASS][3] -> [ABORT][4] ([i915#7911] / [i915#7913]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/fi-bsw-nick/igt@i915_selftest@l...@execlists.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/fi-bsw-nick/igt@i915_selftest@l...@execlists.html * igt@i915_selftest@live@gt_heartbeat: - fi-glk-j4005: [PASS][5] -> [DMESG-FAIL][6] ([i915#5334]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html * igt@i915_selftest@live@requests: - bat-mtlp-6: [PASS][7] -> [DMESG-FAIL][8] ([i915#8497]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/bat-mtlp-6/igt@i915_selftest@l...@requests.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/bat-mtlp-6/igt@i915_selftest@l...@requests.html * igt@i915_selftest@live@slpc: - bat-rpls-1: [PASS][9] -> [DMESG-WARN][10] ([i915#6367]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/bat-rpls-1/igt@i915_selftest@l...@slpc.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/bat-rpls-1/igt@i915_selftest@l...@slpc.html * igt@kms_chamelium_hpd@common-hpd-after-suspend: - fi-apl-guc: NOTRUN -> [SKIP][11] ([fdo#109271]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/fi-apl-guc/igt@kms_chamelium_...@common-hpd-after-suspend.html Possible fixes * igt@i915_pm_rpm@basic-pci-d3-state: - bat-adlp-9: [FAIL][12] ([i915#7940]) -> [PASS][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/bat-adlp-9/igt@i915_pm_...@basic-pci-d3-state.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/bat-adlp-9/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_pm_rpm@module-reload: - fi-apl-guc: [DMESG-FAIL][14] ([i915#8585]) -> [PASS][15] [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/fi-apl-guc/igt@i915_pm_...@module-reload.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/fi-apl-guc/igt@i915_pm_...@module-reload.html - fi-kbl-7567u: [FAIL][16] ([i915#7940]) -> [PASS][17] [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/fi-kbl-7567u/igt@i915_pm_...@module-reload.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/fi-kbl-7567u/igt@i915_pm_...@module-reload.html * igt@i915_selftest@live@gt_mocs: - bat-mtlp-8: [DMESG-FAIL][18] ([i915#7059]) -> [PASS][19] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html * igt@i915_selftest@live@requests: - bat-mtlp-8: [DMESG-FAIL][20] ([i915#8497]) -> [PASS][21] [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/bat-mtlp-8/igt@i915_selftest@l...@requests.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/bat-mtlp-8/igt@i915_selftest@l...@requests.html Warnings * igt@i915_pm_rpm@basic-pci-d3-state: - fi-cfl-guc: [FAIL][22] ([i915#7940]) -> [FAIL][23] ([i915#7691]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/fi-cfl-guc/igt@i915_pm_...@basic-pci-d3-state.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_94470v5/fi-cfl-guc/igt@i915_pm_...@basic-pci-d3-state.html * igt@kms_psr@sprite_plane_onoff: - bat-rplp-1: [ABORT][24] ([i915#8712]) -> [ABORT][25] ([i915#8442] / [i915#8668] / [i915#8712]) [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13436/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html [25]:
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Panel replay phase1 implementation (rev5)
== Series Details == Series: Panel replay phase1 implementation (rev5) URL : https://patchwork.freedesktop.org/series/94470/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Panel replay phase1 implementation (rev5)
== Series Details ==
Series: Panel replay phase1 implementation (rev5)
URL : https://patchwork.freedesktop.org/series/94470/
State : warning
== Summary ==
Error: dim checkpatch failed
445e87b0b6b4 drm/panelreplay: dpcd register definition for panelreplay
81963673fe7c drm/i915/panelreplay: Added HAS_PANEL_REPLAY() macro
d9b9bdca76bf drm/i915/psr: Move psr specific dpcd init into own function
-:55: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around
'intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED'
#55: FILE: drivers/gpu/drm/i915/display/intel_psr.c:499:
+ if (DISPLAY_VER(i915) >= 9 &&
(intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED)) {
total: 0 errors, 0 warnings, 1 checks, 70 lines checked
d7981359cc4e drm/i915/panelreplay: Initializaton and compute config for panel
replay
-:17: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#17:
- Set source_panel_replay_support flag under HAS_PNEL_REPLAY() check. [Jouni]
-:58: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'intel_dp' - possible
side-effects?
#58: FILE: drivers/gpu/drm/i915/display/intel_display_types.h:1989:
+#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support
&& \
+ (intel_dp)->psr.source_panel_replay_support)
total: 0 errors, 1 warnings, 1 checks, 240 lines checked
53b797e67556 drm/i915/panelreplay: Enable panel replay dpcd initialization for
DP
7c40fcb0136b drm/i915/panelreplay: enable/disable panel replay
Re: [Intel-gfx] [PATCH v3] drm/i915: Fix premature release of request's reusable memory
Hi Janusz,
On Thu, Jul 20, 2023 at 11:35:44AM +0200, Janusz Krzysztofik wrote:
> Infinite waits for completion of GPU activity have been observed in CI,
> mostly inside __i915_active_wait(), triggered by igt@gem_barrier_race or
> igt@perf@stress-open-close. Root cause analysis, based of ftrace dumps
> generated with a lot of extra trace_printk() calls added to the code,
> revealed loops of request dependencies being accidentally built,
> preventing the requests from being processed, each waiting for completion
> of another one's activity.
>
> After we substitute a new request for a last active one tracked on a
> timeline, we set up a dependency of our new request to wait on completion
> of current activity of that previous one. While doing that, we must take
> care of keeping the old request still in memory until we use its
> attributes for setting up that await dependency, or we can happen to set
> up the await dependency on an unrelated request that already reuses the
> memory previously allocated to the old one, already released. Combined
> with perf adding consecutive kernel context remote requests to different
> user context timelines, unresolvable loops of await dependencies can be
> built, leading do infinite waits.
>
> We obtain a pointer to the previous request to wait upon when we
> substitute it with a pointer to our new request in an active tracker,
> e.g. in intel_timeline.last_request. In some processing paths we protect
> that old request from being freed before we use it by getting a reference
> to it under RCU protection, but in others, e.g. __i915_request_commit()
> -> __i915_request_add_to_timeline() -> __i915_request_ensure_ordering(),
> we don't. But anyway, since the requests' memory is SLAB_FAILSAFE_BY_RCU,
> that RCU protection is not sufficient against reuse of memory.
>
> We could protect i915_request's memory from being prematurely reused by
> calling its release function via call_rcu() and using rcu_read_lock()
> consequently, as proposed in v1. However, that approach leads to
> significant (up to 10 times) increase of SLAB utilization by i915_request
> SLAB cache. Another potential approach is to take a reference to the
> previous active fence.
>
> When updating an active fence tracker, we first lock the new fence,
> substitute a pointer of the current active fence with the new one, then we
> lock the substituted fence. With this approach, there is a time window
> after the substitution and before the lock when the request can be
> concurrently released by an interrupt handler and its memory reused, then
> we may happen to lock and return a new, unrelated request.
>
> Always get a reference to the current active fence first, before
> replacing it with a new one. Having it protected from premature release
> and reuse, lock it and then replace with the new one but only if not
> yet signalled via a potential concurrent interrupt nor replaced with
> another one by a potential concurrent thread, otherwise retry, starting
> from getting a reference to the new current one. Adjust users to not
> get a reference to the previous active fence themselves and always put the
> reference got by __i915_active_fence_set() when no longer needed.
>
> v3: Fix lockdep splat reports and other issues caused by incorrect use of
> try_cmpxchg() (use (cmpxchg() != prev) instead)
> v2: Protect request's memory by getting a reference to it in favor of
> delegating its release to call_rcu() (Chris)
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8211
> Fixes: df9f85d8582e ("drm/i915: Serialise i915_active_fence_set() with
> itself")
> Suggested-by: Chris Wilson
> Signed-off-by: Janusz Krzysztofik
> Cc: # v5.6+
thanks for the offline clarification on this! It's another good
catch of yours :)
Reviewed-by: Andi Shyti
Thank you!
Andi
[Intel-gfx] [PATCH v3 3/6] drm/i915/psr: Move psr specific dpcd init into own function
From: Jouni Högander
This patch is preparing adding panel replay specific dpcd init.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_psr.c | 39 +---
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 04ab034a8d57..9fbcb4b93f11 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -472,27 +472,22 @@ static void intel_dp_get_su_granularity(struct intel_dp
*intel_dp)
intel_dp->psr.su_y_granularity = y;
}
-void intel_psr_init_dpcd(struct intel_dp *intel_dp)
+static void _psr_init_dpcd(struct intel_dp *intel_dp)
{
- struct drm_i915_private *dev_priv =
+ struct drm_i915_private *i915 =
to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
- drm_dp_dpcd_read(_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
-sizeof(intel_dp->psr_dpcd));
-
- if (!intel_dp->psr_dpcd[0])
- return;
- drm_dbg_kms(_priv->drm, "eDP panel supports PSR version %x\n",
+ drm_dbg_kms(>drm, "eDP panel supports PSR version %x\n",
intel_dp->psr_dpcd[0]);
if (drm_dp_has_quirk(_dp->desc, DP_DPCD_QUIRK_NO_PSR)) {
- drm_dbg_kms(_priv->drm,
+ drm_dbg_kms(>drm,
"PSR support not currently available for this
panel\n");
return;
}
if (!(intel_dp->edp_dpcd[1] & DP_EDP_SET_POWER_CAP)) {
- drm_dbg_kms(_priv->drm,
+ drm_dbg_kms(>drm,
"Panel lacks power state control, PSR cannot be
enabled\n");
return;
}
@@ -501,7 +496,7 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
intel_dp->psr.sink_sync_latency =
intel_dp_get_sink_sync_latency(intel_dp);
- if (DISPLAY_VER(dev_priv) >= 9 &&
+ if (DISPLAY_VER(i915) >= 9 &&
(intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED)) {
bool y_req = intel_dp->psr_dpcd[1] &
DP_PSR2_SU_Y_COORDINATE_REQUIRED;
@@ -519,14 +514,24 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
* GTC first.
*/
intel_dp->psr.sink_psr2_support = y_req && alpm;
- drm_dbg_kms(_priv->drm, "PSR2 %ssupported\n",
+ drm_dbg_kms(>drm, "PSR2 %ssupported\n",
intel_dp->psr.sink_psr2_support ? "" : "not ");
+ }
+}
- if (intel_dp->psr.sink_psr2_support) {
- intel_dp->psr.colorimetry_support =
- intel_dp_get_colorimetry_status(intel_dp);
- intel_dp_get_su_granularity(intel_dp);
- }
+void intel_psr_init_dpcd(struct intel_dp *intel_dp)
+{
+ drm_dp_dpcd_read(_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
+sizeof(intel_dp->psr_dpcd));
+
+ if (intel_dp->psr_dpcd[0])
+ _psr_init_dpcd(intel_dp);
+ /* TODO: Add PR case here */
+
+ if (intel_dp->psr.sink_psr2_support) {
+ intel_dp->psr.colorimetry_support =
+ intel_dp_get_colorimetry_status(intel_dp);
+ intel_dp_get_su_granularity(intel_dp);
}
}
--
2.29.0
[Intel-gfx] [PATCH v3 6/6] drm/i915/panelreplay: enable/disable panel replay
TRANS_DP2_CTL register is programmed to enable panel replay from source
and sink is enabled through panel replay dpcd configuration address.
Bspec: 1407940617
v1: Initial version.
v2:
- Use pr_* flags instead psr_* flags. [Jouni]
- Remove intel_dp_is_edp check as edp1.5 also has panel replay. [Jouni]
v3: cover letter updated and selective fetch condition check is added
before updating its bit in PSR2_MAN_TRK_CTL register. [Jouni]
Note: Initial plan is to enable panel replay in full-screen live active
frame update mode. In a incremental approach panel replay will be enabled
in selctive update mode if there is any gap in curent implementation.
Cc: Jouni Högander
Signed-off-by: Animesh Manna
---
.../drm/i915/display/intel_display_types.h| 1 +
drivers/gpu/drm/i915/display/intel_psr.c | 32 ---
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1ff7e6c03b44..41fbd49393f3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1696,6 +1696,7 @@ struct intel_psr {
u16 su_y_granularity;
bool source_panel_replay_support;
bool sink_panel_replay_support;
+ bool pr_enabled;
u32 dc3co_exitline;
u32 dc3co_exit_delay;
struct delayed_work dc3co_work;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index f6b00abe92d4..244fb336f6bc 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -599,8 +599,14 @@ static void intel_psr_enable_sink(struct intel_dp
*intel_dp)
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
u8 dpcd_val = DP_PSR_ENABLE;
- /* Enable ALPM at sink for psr2 */
+ if (intel_dp->psr.pr_enabled) {
+ drm_dp_dpcd_writeb(_dp->aux, PANEL_REPLAY_CONFIG,
+ DP_PANEL_REPLAY_ENABLE);
+ return;
+ }
+
if (intel_dp->psr.psr2_enabled) {
+ /* Enable ALPM at sink for psr2 */
drm_dp_dpcd_writeb(_dp->aux, DP_RECEIVER_ALPM_CONFIG,
DP_ALPM_ENABLE |
DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE);
@@ -750,6 +756,18 @@ static int psr2_block_count(struct intel_dp *intel_dp)
return psr2_block_count_lines(intel_dp) / 4;
}
+static void dg2_activate_panel_replay(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+ if (intel_dp->psr.psr2_sel_fetch_enabled)
+ intel_de_rmw(dev_priv,
PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
+0, ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE);
+
+ intel_de_rmw(dev_priv, TRANS_DP2_CTL(intel_dp->psr.transcoder), 0,
+TRANS_DP2_PANEL_REPLAY_ENABLE);
+}
+
static void hsw_activate_psr2(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -1361,8 +1379,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
lockdep_assert_held(_dp->psr.lock);
- /* psr1 and psr2 are mutually exclusive.*/
- if (intel_dp->psr.psr2_enabled)
+ /* psr1, psr2 and panel-replay are mutually exclusive.*/
+ if (intel_dp->psr.pr_enabled)
+ dg2_activate_panel_replay(intel_dp);
+ else if (intel_dp->psr.psr2_enabled)
hsw_activate_psr2(intel_dp);
else
hsw_activate_psr1(intel_dp);
@@ -1541,6 +1561,7 @@ static void intel_psr_enable_locked(struct intel_dp
*intel_dp,
drm_WARN_ON(_priv->drm, intel_dp->psr.enabled);
intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
+ intel_dp->psr.pr_enabled = crtc_state->has_pr;
intel_dp->psr.busy_frontbuffer_bits = 0;
intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
intel_dp->psr.transcoder = crtc_state->cpu_transcoder;
@@ -1586,7 +1607,10 @@ static void intel_psr_exit(struct intel_dp *intel_dp)
return;
}
- if (intel_dp->psr.psr2_enabled) {
+ if (intel_dp->psr.pr_enabled) {
+ intel_de_rmw(dev_priv, TRANS_DP2_CTL(intel_dp->psr.transcoder),
+TRANS_DP2_PANEL_REPLAY_ENABLE, 0);
+ } else if (intel_dp->psr.psr2_enabled) {
tgl_disallow_dc3co_on_psr2_exit(intel_dp);
val = intel_de_rmw(dev_priv, EDP_PSR2_CTL(cpu_transcoder),
--
2.29.0
[Intel-gfx] [PATCH v3 5/6] drm/i915/panelreplay: Enable panel replay dpcd initialization for DP
Due to similarity panel replay dpcd initialization got added in psr function which is specific for edp panel. This patch enables panel replay initialization for dp connector. Signed-off-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_psr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 7508e6c967e2..f6b00abe92d4 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -2735,6 +2735,9 @@ void intel_psr_init(struct intel_dp *intel_dp) if (!(HAS_PSR(dev_priv) || HAS_PANEL_REPLAY(dev_priv))) return; + if (!intel_dp_is_edp(intel_dp)) + intel_psr_init_dpcd(intel_dp); + /* * HSW spec explicitly says PSR is tied to port A. * BDW+ platforms have a instance of PSR registers per transcoder but -- 2.29.0
[Intel-gfx] [PATCH v3 4/6] drm/i915/panelreplay: Initializaton and compute config for panel replay
Modify existing PSR implementation to enable panel replay feature of DP 2.0
which is similar to PSR feature of EDP panel. There is different DPCD
address to check panel capability compare to PSR and vsc sdp header
is different.
v1: Initial version.
v2:
- Set source_panel_replay_support flag under HAS_PNEL_REPLAY() check. [Jouni]
- Code restructured around intel_panel_replay_init
and renamed to intel_panel_replay_init_dpcd. [Jouni]
- Remove the initial code modification around has_psr2 flag. [Jouni]
- Add CAN_PANEL_REPLAY() in intel_encoder_can_psr which is used to
enable in intel_psr_post_plane_update. [Jouni]
v3:
- Initialize both psr and panel-replay. [Jouni]
- Initialize both panel replay and psr if detected. [Jouni]
- Refactoring psr function by introducing _psr_compute_config(). [Jouni]
- Add check for !is_edp while deriving source_panel_replay_support. [Jouni]
- Enable panel replay dpcd initialization in a separate patch. [Jouni]
Cc: Jouni Högander
Signed-off-by: Animesh Manna
---
.../drm/i915/display/intel_display_types.h| 8 +-
drivers/gpu/drm/i915/display/intel_dp.c | 44 --
drivers/gpu/drm/i915/display/intel_psr.c | 88 +--
3 files changed, 104 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 731f2ec04d5c..1ff7e6c03b44 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1202,6 +1202,7 @@ struct intel_crtc_state {
bool has_psr2;
bool enable_psr2_sel_fetch;
bool req_psr2_sdp_prior_scanline;
+ bool has_pr;
bool wm_level_disabled;
u32 dc3co_exitline;
u16 su_y_granularity;
@@ -1693,6 +1694,8 @@ struct intel_psr {
bool irq_aux_error;
u16 su_w_granularity;
u16 su_y_granularity;
+ bool source_panel_replay_support;
+ bool sink_panel_replay_support;
u32 dc3co_exitline;
u32 dc3co_exit_delay;
struct delayed_work dc3co_work;
@@ -1983,12 +1986,15 @@ dp_to_lspcon(struct intel_dp *intel_dp)
#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
(intel_dp)->psr.source_support)
+#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support
&& \
+ (intel_dp)->psr.source_panel_replay_support)
+
static inline bool intel_encoder_can_psr(struct intel_encoder *encoder)
{
if (!intel_encoder_is_dp(encoder))
return false;
- return CAN_PSR(enc_to_intel_dp(encoder));
+ return CAN_PSR(enc_to_intel_dp(encoder)) ||
CAN_PANEL_REPLAY(enc_to_intel_dp(encoder));
}
static inline struct intel_digital_port *
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 03675620e3ea..0ba231ee6e34 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1946,12 +1946,22 @@ static void intel_dp_compute_vsc_colorimetry(const
struct intel_crtc_state *crtc
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- /*
-* Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
-* VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
-* Colorimetry Format indication.
-*/
- vsc->revision = 0x5;
+ if (crtc_state->has_pr) {
+ /*
+* Prepare VSC Header for SU as per DP 2.0 spec, Table 2-223
+* VSC SDP supporting 3D stereo, Panel Replay, and Pixel
+* Encoding/Colorimetry Format indication.
+*/
+ vsc->revision = 0x7;
+ } else {
+ /*
+* Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
+* VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
+* Colorimetry Format indication.
+*/
+ vsc->revision = 0x5;
+ }
+
vsc->length = 0x13;
/* DP 1.4a spec, Table 2-120 */
@@ -2060,6 +2070,21 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp
*intel_dp,
vsc->revision = 0x4;
vsc->length = 0xe;
}
+ } else if (crtc_state->has_pr) {
+ if (intel_dp->psr.colorimetry_support &&
+ intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
+ /* [Panel Replay with colorimetry info] */
+ intel_dp_compute_vsc_colorimetry(crtc_state, conn_state,
+vsc);
+ } else {
+ /*
+* [Panel Replay without colorimetry info]
+* Prepare VSC Header for SU as per DP 2.0 spec, Table
2-223
+* VSC SDP
[Intel-gfx] [PATCH v3 2/6] drm/i915/panelreplay: Added HAS_PANEL_REPLAY() macro
Platforms having Display 13 and above will support panel replay feature of DP 2.0 monitor. Added a HAS_PANEL_REPLAY() macro to check for panel replay capability. v1: Initial version. v2: DISPLAY_VER() removed as HAS_DP20() is having platform check. [Jouni] Cc: Jouni Högander Signed-off-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_display_device.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h index 3324bd453ca7..53bc8f972a26 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.h +++ b/drivers/gpu/drm/i915/display/intel_display_device.h @@ -60,6 +60,7 @@ struct drm_printer; #define HAS_MSO(i915) (DISPLAY_VER(i915) >= 12) #define HAS_OVERLAY(i915) (DISPLAY_INFO(i915)->has_overlay) #define HAS_PSR(i915) (DISPLAY_INFO(i915)->has_psr) +#define HAS_PANEL_REPLAY(dev_priv) (HAS_DP20(dev_priv)) #define HAS_PSR_HW_TRACKING(i915) (DISPLAY_INFO(i915)->has_psr_hw_tracking) #define HAS_PSR2_SEL_FETCH(i915) (DISPLAY_VER(i915) >= 12) #define HAS_SAGV(i915) (DISPLAY_VER(i915) >= 9 && !IS_LP(i915)) -- 2.29.0
[Intel-gfx] [PATCH v3 1/6] drm/panelreplay: dpcd register definition for panelreplay
DPCD register definition added to check and enable panel replay capability of the sink. Cc: Jouni Högander Signed-off-by: Animesh Manna --- include/drm/display/drm_dp.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 02f2ac4dd2df..c48696266d23 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -543,6 +543,10 @@ /* DFP Capability Extension */ #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT0x0a3 /* 2.0 */ +#define DP_PANEL_REPLAY_CAP 0x0b0 +# define DP_PANEL_REPLAY_SUPPORT(1 << 0) +# define DP_PR_SELECTIVE_UPDATE_SUPPORT (1 << 1) + /* Link Configuration */ #defineDP_LINK_BW_SET 0x100 # define DP_LINK_RATE_TABLE0x00/* eDP 1.4 */ @@ -716,6 +720,13 @@ #define DP_BRANCH_DEVICE_CTRL 0x1a1 # define DP_BRANCH_DEVICE_IRQ_HPD (1 << 0) +#define PANEL_REPLAY_CONFIG 0x1b0 +# define DP_PANEL_REPLAY_ENABLE (1 << 0) +# define DP_PR_UNRECOVERABLE_ERROR (1 << 3) +# define DP_PR_RFB_STORAGE_ERROR(1 << 4) +# define DP_PR_ACTIVE_FRAME_CRC_ERROR (1 << 5) +# define DP_PR_SELECTIVE_UPDATE_ENABLE (1 << 6) + #define DP_PAYLOAD_ALLOCATE_SET0x1c0 #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1 #define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2 -- 2.29.0
[Intel-gfx] [PATCH v3 0/6] Panel replay phase1 implementation
Panel Replay is a power saving feature for DP 2.0 monitor and similar to PSR on EDP. These patches are basic enablement patches added on top of existing psr framework to enable full-screen live active frame update mode of panel replay. Panel replay also can be enabled in selective update mode which will be enabled in a incremental approach. As per current design panel replay priority is higher than psr. intel_dp->psr.pr_enabled flag indicate panel replay is enabled. intel_dp->psr.pr_enabled + intel_dp->psr.psr2_enabled indicates panel replay is enabled in selective update mode. intel_dp->psr.pr_enabled + intel_dp->psr.psr2_enabled + intel_psr.selective_fetch enabled indicates panel replay is enabled in selective update mode with selective fetch. PSR replated flags remain same like before. Note: The patches are not tested due to unavailability of monitor. Cc: Jouni Högander Signed-off-by: Animesh Manna Animesh Manna (5): drm/panelreplay: dpcd register definition for panelreplay drm/i915/panelreplay: Added HAS_PANEL_REPLAY() macro drm/i915/panelreplay: Initializaton and compute config for panel replay drm/i915/panelreplay: Enable panel replay dpcd initialization for DP drm/i915/panelreplay: enable/disable panel replay Jouni Högander (1): drm/i915/psr: Move psr specific dpcd init into own function .../drm/i915/display/intel_display_device.h | 1 + .../drm/i915/display/intel_display_types.h| 9 +- drivers/gpu/drm/i915/display/intel_dp.c | 44 - drivers/gpu/drm/i915/display/intel_psr.c | 158 +- include/drm/display/drm_dp.h | 11 ++ 5 files changed, 168 insertions(+), 55 deletions(-) -- 2.29.0
Re: [Intel-gfx] [RFC 4/8] drm/i915: Refactor PAT/object cache handling
On 28/07/2023 08:14, Yang, Fei wrote: [snip] @@ -41,14 +42,17 @@ static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj) return false; /* - * For objects created by userspace through GEM_CREATE with pat_index - * set by set_pat extension, i915_gem_object_has_cache_level() will - * always return true, because the coherency of such object is managed i915_gem_object_has_cache_level() always return true means this function always return false. - * by userspace. Othereise the call here would fall back to checking - * whether the object is un-cached or write-through. + * Always flush cache for UMD objects with PAT index set. (obj->pat_set_by_user == true) indicates UMD knows how to handle the coherency, forcing clflush in KMD would be redundant. For Meteorlake I made gpu_write_needs_clflush() always return false anyway. Could you please submit a patch with kerneldoc for i915_drm.h explaining what the set domain ioctl is expected to do when set pat extension is used? With the focus on the use cases of how userspace is managing coherency using it, or it isn't, or what. */ - return !(i915_gem_object_has_cache_level(obj, I915_CACHE_NONE) || - i915_gem_object_has_cache_level(obj, I915_CACHE_WT)); + if (obj->pat_set_by_user) + return true; return false; Oops, thank you! I did warn in the cover letter I was getting confused by boolean logic conversions, cross-referencing three versions, and extracting the pat_set_by_user to call sites. :) + + /* + * Fully coherent cached access may end up with data in the CPU cache + * which hasn't hit memory yet. + */ + return i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_WB) && +i915_gem_object_has_cache_flag(obj, I915_CACHE_FLAG_COH2W); Why checking COH2W here? The logic was, if UC or WT return false, otherwise return true. So, as long as cache_mode is WB, it's sufficient to say true here, right? I was trying to penetrate the reason behind the check. Original code was: return !(obj->cache_level == I915_CACHE_NONE || obj->cache_level == I915_CACHE_WT); Which is equivalent to "is it WB", right? (Since it matches on both old LLC flavours.) Which I thought, in the context of this function, is supposed to answer the question of "can there be data in the shared cache written by the GPU but not committed to RAM yet". And then I thought that can only ever happen with 2-way coherency. Otherwise GPU writes never end up in the CPU cache. Did I get that wrong? Maybe I have.. Regards, Tvrtko
Re: [Intel-gfx] [RFC 7/8] drm/i915: Lift the user PAT restriction from use_cpu_reloc
On 28/07/2023 01:09, Matt Roper wrote: On Thu, Jul 27, 2023 at 03:55:03PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Now that i915 understands the caching modes behind PAT indices, we can refine the check in use_cpu_reloc() to not reject the uncached PAT if it was set by userspace. Instead it can decide based on the presence of full coherency which should be functionally equivalent on legacy platforms. We can ignore WT since it is only used by the display, and we can ignore Meteorlake since it will fail on the existing "has_llc" condition before the object cache mode check. Signed-off-by: Tvrtko Ursulin Cc: Fei Yang Cc: Matt Roper --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 9d6e49c8a4c6..f74b33670bad 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -640,16 +640,9 @@ static inline int use_cpu_reloc(const struct reloc_cache *cache, if (DBG_FORCE_RELOC == FORCE_GTT_RELOC) return false; - /* -* For objects created by userspace through GEM_CREATE with pat_index -* set by set_pat extension, i915_gem_object_has_cache_level() always -* return true, otherwise the call would fall back to checking whether -* the object is un-cached. -*/ return (cache->has_llc || obj->cache_dirty || - !(obj->pat_set_by_user || - i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC))); + i915_gem_object_has_cache_flag(obj, I915_CACHE_FLAG_COH2W)); My understanding of relocations is minimal, but does 2W actually matter here (CPU snooping GPU caches)? I would have expected only 1W coherency to be necessary (GPU snooping CPU caches)? I struggled with this one. Original code was: return (cache->has_llc || obj->cache_dirty || obj->cache_level != I915_CACHE_NONE); And I struggled to figure out the intent. It is not "don't do CPU relocations for uncached" because it will do them when LLC or dirty regardless. You could be right.. can we interpret it as any mode apart from uncached was viewed as coherent for CPU writes being seen by the GPU? In which case should/could it be based on I915_BO_CACHE_COHERENT_FOR_WRITE? Regards, Tvrtko Matt } static int eb_reserve_vma(struct i915_execbuffer *eb, -- 2.39.2
[Intel-gfx] ✓ Fi.CI.IGT: success for DSC misc fixes (rev5)
== Series Details == Series: DSC misc fixes (rev5) URL : https://patchwork.freedesktop.org/series/117662/ State : success == Summary == CI Bug Log - changes from CI_DRM_13435_full -> Patchwork_117662v5_full Summary --- **SUCCESS** No regressions found. Participating hosts (10 -> 9) -- Missing(1): pig-kbl-iris Known issues Here are the changes found in Patchwork_117662v5_full that come from known issues: ### IGT changes ### Issues hit * igt@api_intel_bb@blit-reloc-purge-cache: - shard-dg2: NOTRUN -> [SKIP][1] ([i915#8411]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-dg2-8/igt@api_intel...@blit-reloc-purge-cache.html - shard-rkl: NOTRUN -> [SKIP][2] ([i915#8411]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-rkl-1/igt@api_intel...@blit-reloc-purge-cache.html * igt@debugfs_test@basic-hwmon: - shard-rkl: NOTRUN -> [SKIP][3] ([i915#7456]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-rkl-1/igt@debugfs_t...@basic-hwmon.html * igt@device_reset@unbind-cold-reset-rebind: - shard-mtlp: NOTRUN -> [SKIP][4] ([i915#7701]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-mtlp-1/igt@device_re...@unbind-cold-reset-rebind.html * igt@drm_fdinfo@busy-hang@rcs0: - shard-mtlp: NOTRUN -> [SKIP][5] ([i915#8414]) +12 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-mtlp-8/igt@drm_fdinfo@busy-h...@rcs0.html * igt@gem_basic@multigpu-create-close: - shard-mtlp: NOTRUN -> [SKIP][6] ([i915#7697]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-mtlp-5/igt@gem_ba...@multigpu-create-close.html * igt@gem_ctx_persistence@legacy-engines-hostile-preempt: - shard-snb: NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#1099]) +1 similar issue [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-snb5/igt@gem_ctx_persiste...@legacy-engines-hostile-preempt.html * igt@gem_ctx_persistence@smoketest: - shard-rkl: [PASS][8] -> [FAIL][9] ([i915#5099]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-rkl-7/igt@gem_ctx_persiste...@smoketest.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-rkl-7/igt@gem_ctx_persiste...@smoketest.html * igt@gem_eio@kms: - shard-dg2: [PASS][10] -> [INCOMPLETE][11] ([i915#7892]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-dg2-10/igt@gem_...@kms.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-dg2-3/igt@gem_...@kms.html * igt@gem_exec_balancer@invalid-bonds: - shard-dg2: NOTRUN -> [SKIP][12] ([i915#4036]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-dg2-8/igt@gem_exec_balan...@invalid-bonds.html * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][13] -> [FAIL][14] ([i915#2846]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-glk3/igt@gem_exec_f...@basic-deadline.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-glk1/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_fair@basic-pace-solo@rcs0: - shard-rkl: [PASS][15] -> [FAIL][16] ([i915#2842]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-rkl-1/igt@gem_exec_fair@basic-pace-s...@rcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-rkl-1/igt@gem_exec_fair@basic-pace-s...@rcs0.html * igt@gem_exec_fence@submit67: - shard-mtlp: NOTRUN -> [SKIP][17] ([i915#4812]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-mtlp-8/igt@gem_exec_fe...@submit67.html * igt@gem_exec_flush@basic-wb-ro-default: - shard-dg2: NOTRUN -> [SKIP][18] ([i915#3539] / [i915#4852]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-dg2-8/igt@gem_exec_fl...@basic-wb-ro-default.html * igt@gem_exec_reloc@basic-concurrent0: - shard-mtlp: NOTRUN -> [SKIP][19] ([i915#3281]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-mtlp-5/igt@gem_exec_re...@basic-concurrent0.html * igt@gem_exec_reloc@basic-write-gtt: - shard-rkl: NOTRUN -> [SKIP][20] ([i915#3281]) +1 similar issue [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-rkl-1/igt@gem_exec_re...@basic-write-gtt.html * igt@gem_exec_reloc@basic-write-gtt-noreloc: - shard-dg2: NOTRUN -> [SKIP][21] ([i915#3281]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117662v5/shard-dg2-8/igt@gem_exec_re...@basic-write-gtt-noreloc.html * igt@gem_lmem_swapping@smem-oom@lmem0: - shard-dg2:
Re: [Intel-gfx] [RFC 4/8] drm/i915: Refactor PAT/object cache handling
Forgot one part of your reply:
On 28/07/2023 00:57, Matt Roper wrote:
On Thu, Jul 27, 2023 at 03:55:00PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Commit 9275277d5324 ("drm/i915: use pat_index instead of cache_level") has
introduced PAT indices to i915 internal APIs, partially replacing the
usage of driver internal cache_level, but has also added a few sub-
optimal design decisions which this patch tries to improve upon.
Principal change here is to invert the per platform cache level to PAT
index table which was added by the referenced commit, and by doing so
enable i915 to understand the cache mode between PAT indices, changing
them from opaque to transparent.
Once we have the inverted table we are able to remove the hidden false
"return true" from i915_gem_object_has_cache_level and make the involved
code path clearer.
To achieve this we replace the enum i915_cache_level with i915_cache_t,
composed of a more detailed representation of each cache mode (base mode
plus flags).
In this way we are able to express the differences between different
write-back mode coherency settings on Meteorlake, which in turn enables us
to map the i915 "cached" mode to the correct Meteorlake PAT index.
We can also replace the platform dependent cache mode to string code in
debugfs and elsewhere by the single implementation based on i915_cache_t.
v2:
* Fix PAT-to-cache-mode table for PVC. (Fei)
* Cache display caching mode too. (Fei)
* Improve and document criteria in i915_gem_object_can_bypass_llc() (Matt)
v3:
* Checkpath issues.
* Cache mode flags check fixed.
v4:
* Fix intel_device_info->cache_modes array size. (Matt)
* Boolean cache mode and flags query. (Matt)
* Reduce number of cache macros with some macro magic.
* One more checkpatch fix.
* Tweak tables to show legacy and Gen12 WB is fully coherent.
Signed-off-by: Tvrtko Ursulin
References: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
Cc: Chris Wilson
Cc: Fei Yang
Cc: Andi Shyti
Cc: Matt Roper
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 60 +
drivers/gpu/drm/i915/gem/i915_gem_domain.h| 5 +-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 3 +-
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_object.c| 117 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 11 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 116 +
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 8 +-
drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 20 +--
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +-
.../drm/i915/gem/selftests/huge_gem_object.c | 2 +-
.../gpu/drm/i915/gem/selftests/huge_pages.c | 3 +-
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 10 +-
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 25 ++--
drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 4 +-
drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +-
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +-
.../gpu/drm/i915/gt/intel_ring_submission.c | 4 +-
drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
.../gpu/drm/i915/gt/selftest_workarounds.c| 2 +-
drivers/gpu/drm/i915/i915_cache.c | 89 +++--
drivers/gpu/drm/i915/i915_cache.h | 70 ++-
drivers/gpu/drm/i915/i915_debugfs.c | 53 ++--
drivers/gpu/drm/i915/i915_driver.c| 4 +-
drivers/gpu/drm/i915/i915_gem.c | 13 --
drivers/gpu/drm/i915/i915_pci.c | 84 +++--
drivers/gpu/drm/i915/i915_perf.c | 2 +-
drivers/gpu/drm/i915/intel_device_info.h | 6 +-
.../gpu/drm/i915/selftests/i915_gem_evict.c | 4 +-
drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +-
.../gpu/drm/i915/selftests/mock_gem_device.c | 14 +--
36 files changed, 391 insertions(+), 367 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 57db9c581bf6..c15f83de33af 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -8,6 +8,7 @@
#include "display/intel_frontbuffer.h"
#include "gt/intel_gt.h"
+#include "i915_cache.h"
#include "i915_drv.h"
#include "i915_gem_clflush.h"
#include "i915_gem_domain.h"
@@ -41,14 +42,17 @@ static bool gpu_write_needs_clflush(struct
drm_i915_gem_object *obj)
return false;
/*
-* For objects created by userspace through GEM_CREATE with pat_index
-* set by set_pat extension, i915_gem_object_has_cache_level() will
-* always return true, because the
Re: [Intel-gfx] [RFC 4/8] drm/i915: Refactor PAT/object cache handling
On 28/07/2023 01:17, Matt Roper wrote:
On Thu, Jul 27, 2023 at 04:57:53PM -0700, Matt Roper wrote:
On Thu, Jul 27, 2023 at 03:55:00PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Commit 9275277d5324 ("drm/i915: use pat_index instead of cache_level") has
introduced PAT indices to i915 internal APIs, partially replacing the
usage of driver internal cache_level, but has also added a few sub-
optimal design decisions which this patch tries to improve upon.
Principal change here is to invert the per platform cache level to PAT
index table which was added by the referenced commit, and by doing so
enable i915 to understand the cache mode between PAT indices, changing
them from opaque to transparent.
Once we have the inverted table we are able to remove the hidden false
"return true" from i915_gem_object_has_cache_level and make the involved
code path clearer.
To achieve this we replace the enum i915_cache_level with i915_cache_t,
composed of a more detailed representation of each cache mode (base mode
plus flags).
In this way we are able to express the differences between different
write-back mode coherency settings on Meteorlake, which in turn enables us
to map the i915 "cached" mode to the correct Meteorlake PAT index.
We can also replace the platform dependent cache mode to string code in
debugfs and elsewhere by the single implementation based on i915_cache_t.
v2:
* Fix PAT-to-cache-mode table for PVC. (Fei)
* Cache display caching mode too. (Fei)
* Improve and document criteria in i915_gem_object_can_bypass_llc() (Matt)
v3:
* Checkpath issues.
* Cache mode flags check fixed.
v4:
* Fix intel_device_info->cache_modes array size. (Matt)
* Boolean cache mode and flags query. (Matt)
* Reduce number of cache macros with some macro magic.
* One more checkpatch fix.
* Tweak tables to show legacy and Gen12 WB is fully coherent.
Signed-off-by: Tvrtko Ursulin
References: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
Cc: Chris Wilson
Cc: Fei Yang
Cc: Andi Shyti
Cc: Matt Roper
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 60 +
drivers/gpu/drm/i915/gem/i915_gem_domain.h| 5 +-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 3 +-
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_object.c| 117 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 11 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 116 +
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 8 +-
drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 20 +--
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +-
.../drm/i915/gem/selftests/huge_gem_object.c | 2 +-
.../gpu/drm/i915/gem/selftests/huge_pages.c | 3 +-
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 10 +-
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 25 ++--
drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 4 +-
drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +-
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +-
.../gpu/drm/i915/gt/intel_ring_submission.c | 4 +-
drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
.../gpu/drm/i915/gt/selftest_workarounds.c| 2 +-
drivers/gpu/drm/i915/i915_cache.c | 89 +++--
drivers/gpu/drm/i915/i915_cache.h | 70 ++-
drivers/gpu/drm/i915/i915_debugfs.c | 53 ++--
drivers/gpu/drm/i915/i915_driver.c| 4 +-
drivers/gpu/drm/i915/i915_gem.c | 13 --
drivers/gpu/drm/i915/i915_pci.c | 84 +++--
drivers/gpu/drm/i915/i915_perf.c | 2 +-
drivers/gpu/drm/i915/intel_device_info.h | 6 +-
.../gpu/drm/i915/selftests/i915_gem_evict.c | 4 +-
drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +-
.../gpu/drm/i915/selftests/mock_gem_device.c | 14 +--
36 files changed, 391 insertions(+), 367 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 57db9c581bf6..c15f83de33af 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -8,6 +8,7 @@
#include "display/intel_frontbuffer.h"
#include "gt/intel_gt.h"
+#include "i915_cache.h"
#include "i915_drv.h"
#include "i915_gem_clflush.h"
#include "i915_gem_domain.h"
@@ -41,14 +42,17 @@ static bool gpu_write_needs_clflush(struct
drm_i915_gem_object *obj)
return false;
/*
-* For objects created by userspace through GEM_CREATE with pat_index
-* set by set_pat extension, i915_gem_object_has_cache_level() will
-* always
Re: [Intel-gfx] [RFC 5/8] drm/i915: Improve the vm_fault_gtt user PAT index restriction
On 28/07/2023 01:04, Matt Roper wrote:
On Thu, Jul 27, 2023 at 03:55:01PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Now that i915 understands the caching modes behind PAT indices, we can
refine the check in vm_fault_gtt() to not reject the uncached PAT if it
was set by userspace on a snoopable platform.
Signed-off-by: Tvrtko Ursulin
Cc: Fei Yang
Cc: Matt Roper
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index cd7f8ded0d6f..9aa6ecf68432 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -382,17 +382,9 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
goto err_reset;
}
- /*
-* For objects created by userspace through GEM_CREATE with pat_index
-* set by set_pat extension, coherency is managed by userspace, make
-* sure we don't fail handling the vm fault by calling
-* i915_gem_object_has_cache_level() which always return true for such
-* objects. Otherwise this helper function would fall back to checking
-* whether the object is un-cached.
-*/
- if (!((obj->pat_set_by_user ||
- i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC)) ||
- HAS_LLC(i915))) {
+ /* Access to snoopable pages through the GTT is incoherent. */
This comment was removed in the previous patch, but now it came back
here. Should we have just left it be in the previous patch?
Oops yes, fumble when splitting the single patch into this series.
I'm not really clear on what it means either. Are we using "GTT" as
shorthand to refer to the aperture here?
It is about CPU mmap access so I think so.
Original code was:
/* Access to snoopable pages through the GTT is incoherent. */
if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(i915)) {
ret = -EFAULT;
goto err_unpin;
}
Which was disallowing anything not uncached on snoopable platforms. So I
made it equivalent to that:
/* Access to snoopable pages through the GTT is incoherent. */
if (!i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) &&
!HAS_LLC(i915)) {
ret = -EFAULT;
goto err_unpin;
}
Should be like-for-like assuming PAT-to-cache-mode tables are all good.
On Meteorlake it is no change in behaviour either way due !HAS_LLC.
Regards,
Tvrtko
Matt
+ if (!i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) &&
+ !HAS_LLC(i915)) {
ret = -EFAULT;
goto err_unpin;
}
--
2.39.2
Re: [Intel-gfx] [RFC 4/8] drm/i915: Refactor PAT/object cache handling
On 28/07/2023 00:57, Matt Roper wrote:
On Thu, Jul 27, 2023 at 03:55:00PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Commit 9275277d5324 ("drm/i915: use pat_index instead of cache_level") has
introduced PAT indices to i915 internal APIs, partially replacing the
usage of driver internal cache_level, but has also added a few sub-
optimal design decisions which this patch tries to improve upon.
Principal change here is to invert the per platform cache level to PAT
index table which was added by the referenced commit, and by doing so
enable i915 to understand the cache mode between PAT indices, changing
them from opaque to transparent.
Once we have the inverted table we are able to remove the hidden false
"return true" from i915_gem_object_has_cache_level and make the involved
code path clearer.
To achieve this we replace the enum i915_cache_level with i915_cache_t,
composed of a more detailed representation of each cache mode (base mode
plus flags).
In this way we are able to express the differences between different
write-back mode coherency settings on Meteorlake, which in turn enables us
to map the i915 "cached" mode to the correct Meteorlake PAT index.
We can also replace the platform dependent cache mode to string code in
debugfs and elsewhere by the single implementation based on i915_cache_t.
v2:
* Fix PAT-to-cache-mode table for PVC. (Fei)
* Cache display caching mode too. (Fei)
* Improve and document criteria in i915_gem_object_can_bypass_llc() (Matt)
v3:
* Checkpath issues.
* Cache mode flags check fixed.
v4:
* Fix intel_device_info->cache_modes array size. (Matt)
* Boolean cache mode and flags query. (Matt)
* Reduce number of cache macros with some macro magic.
* One more checkpatch fix.
* Tweak tables to show legacy and Gen12 WB is fully coherent.
Signed-off-by: Tvrtko Ursulin
References: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
Cc: Chris Wilson
Cc: Fei Yang
Cc: Andi Shyti
Cc: Matt Roper
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 60 +
drivers/gpu/drm/i915/gem/i915_gem_domain.h| 5 +-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 3 +-
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_object.c| 117 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 11 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 116 +
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 8 +-
drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 20 +--
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +-
.../drm/i915/gem/selftests/huge_gem_object.c | 2 +-
.../gpu/drm/i915/gem/selftests/huge_pages.c | 3 +-
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 10 +-
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 25 ++--
drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 4 +-
drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +-
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +-
.../gpu/drm/i915/gt/intel_ring_submission.c | 4 +-
drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
.../gpu/drm/i915/gt/selftest_workarounds.c| 2 +-
drivers/gpu/drm/i915/i915_cache.c | 89 +++--
drivers/gpu/drm/i915/i915_cache.h | 70 ++-
drivers/gpu/drm/i915/i915_debugfs.c | 53 ++--
drivers/gpu/drm/i915/i915_driver.c| 4 +-
drivers/gpu/drm/i915/i915_gem.c | 13 --
drivers/gpu/drm/i915/i915_pci.c | 84 +++--
drivers/gpu/drm/i915/i915_perf.c | 2 +-
drivers/gpu/drm/i915/intel_device_info.h | 6 +-
.../gpu/drm/i915/selftests/i915_gem_evict.c | 4 +-
drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +-
.../gpu/drm/i915/selftests/mock_gem_device.c | 14 +--
36 files changed, 391 insertions(+), 367 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 57db9c581bf6..c15f83de33af 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -8,6 +8,7 @@
#include "display/intel_frontbuffer.h"
#include "gt/intel_gt.h"
+#include "i915_cache.h"
#include "i915_drv.h"
#include "i915_gem_clflush.h"
#include "i915_gem_domain.h"
@@ -41,14 +42,17 @@ static bool gpu_write_needs_clflush(struct
drm_i915_gem_object *obj)
return false;
/*
-* For objects created by userspace through GEM_CREATE with pat_index
-* set by set_pat extension, i915_gem_object_has_cache_level() will
-* always return true, because the coherency of such object is managed
-
Re: [Intel-gfx] [RFC 3/8] drm/i915: Cache PAT index used by the driver
On 27/07/2023 23:44, Matt Roper wrote:
On Thu, Jul 27, 2023 at 03:54:59PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Eliminate a bunch of runtime calls to i915_gem_get_pat_index() by caching
the interesting PAT indices in struct drm_i915_private. They are static
per platfrom so no need to consult a function every time.
Signed-off-by: Tvrtko Ursulin
Cc: Matt Roper
Cc: Fei Yang
---
drivers/gpu/drm/i915/Makefile | 1 +
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 3 +--
drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 7 ++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 26 ---
.../gpu/drm/i915/gem/selftests/huge_pages.c | 2 +-
drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 4 +--
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 4 +--
drivers/gpu/drm/i915/gt/intel_ggtt.c | 8 ++
drivers/gpu/drm/i915/gt/intel_migrate.c | 11 +++-
drivers/gpu/drm/i915/gt/selftest_migrate.c| 9 +++
drivers/gpu/drm/i915/gt/selftest_reset.c | 14 +++---
drivers/gpu/drm/i915/gt/selftest_tlb.c| 5 ++--
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 ++
drivers/gpu/drm/i915/i915_cache.c | 18 +
drivers/gpu/drm/i915/i915_cache.h | 13 ++
drivers/gpu/drm/i915/i915_driver.c| 3 +++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 8 ++
drivers/gpu/drm/i915/i915_gpu_error.c | 8 ++
drivers/gpu/drm/i915/selftests/i915_gem.c | 5 +---
.../gpu/drm/i915/selftests/i915_gem_evict.c | 4 +--
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 11 +++-
.../drm/i915/selftests/intel_memory_region.c | 4 +--
.../gpu/drm/i915/selftests/mock_gem_device.c | 2 ++
24 files changed, 89 insertions(+), 91 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_cache.c
create mode 100644 drivers/gpu/drm/i915/i915_cache.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c5fc91cd58e7..905a51a16588 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -35,6 +35,7 @@ subdir-ccflags-y += -I$(srctree)/$(src)
# core driver code
i915-y += i915_driver.o \
i915_drm_client.o \
+ i915_cache.o \
i915_config.o \
i915_getparam.o \
i915_ioctl.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 5a687a3686bd..0a1d40220020 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1330,8 +1330,7 @@ static void *reloc_iomap(struct i915_vma *batch,
ggtt->vm.insert_page(>vm,
i915_gem_object_get_dma_address(obj, page),
offset,
-i915_gem_get_pat_index(ggtt->vm.i915,
- I915_CACHE_NONE),
+eb->i915->pat_uc,
0);
} else {
offset += page << PAGE_SHIFT;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 5b0a5cf9a98a..1c8eb806b7d3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -563,11 +563,8 @@ static void dbg_poison(struct i915_ggtt *ggtt,
while (size) {
void __iomem *s;
- ggtt->vm.insert_page(>vm, addr,
-ggtt->error_capture.start,
-i915_gem_get_pat_index(ggtt->vm.i915,
- I915_CACHE_NONE),
-0);
+ ggtt->vm.insert_page(>vm, addr, ggtt->error_capture.start,
+ggtt->vm.i915->pat_uc, 0);
mb();
s = io_mapping_map_wc(>iomap,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 7078af2f8f79..6bd6c239f4ac 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -58,6 +58,16 @@ i915_ttm_cache_level(struct drm_i915_private *i915, struct
ttm_resource *res,
I915_CACHE_NONE;
}
+static unsigned int
+i915_ttm_cache_pat(struct drm_i915_private *i915, struct ttm_resource *res,
+ struct ttm_tt *ttm)
+{
+ return ((HAS_LLC(i915) || HAS_SNOOP(i915)) &&
+ !i915_ttm_gtt_binds_lmem(res) &&
This matches the existing logic of i915_ttm_cache_level(), but do you
know why LMEM buffers are always set to uncached? I don't understand
that part.
I am not sure - was thinking about that myself - like why not WC? WC PAT
exists on Gen12, but maybe using it wouldn't
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gem: Add check for bitmap_zalloc()
== Series Details == Series: drm/i915/gem: Add check for bitmap_zalloc() URL : https://patchwork.freedesktop.org/series/121491/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13435_full -> Patchwork_121491v1_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_121491v1_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_121491v1_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (10 -> 10) -- No changes in participating hosts Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_121491v1_full: ### IGT changes ### Possible regressions * igt@gem_ppgtt@shrink-vs-evict-any: - shard-rkl: [PASS][1] -> [ABORT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-rkl-2/igt@gem_pp...@shrink-vs-evict-any.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-rkl-7/igt@gem_pp...@shrink-vs-evict-any.html Known issues Here are the changes found in Patchwork_121491v1_full that come from known issues: ### IGT changes ### Issues hit * igt@api_intel_bb@blit-reloc-purge-cache: - shard-dg2: NOTRUN -> [SKIP][3] ([i915#8411]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-dg2-5/igt@api_intel...@blit-reloc-purge-cache.html - shard-rkl: NOTRUN -> [SKIP][4] ([i915#8411]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-rkl-4/igt@api_intel...@blit-reloc-purge-cache.html * igt@debugfs_test@basic-hwmon: - shard-rkl: NOTRUN -> [SKIP][5] ([i915#7456]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-rkl-4/igt@debugfs_t...@basic-hwmon.html * igt@device_reset@unbind-cold-reset-rebind: - shard-mtlp: NOTRUN -> [SKIP][6] ([i915#7701]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-mtlp-7/igt@device_re...@unbind-cold-reset-rebind.html * igt@drm_fdinfo@busy-hang@rcs0: - shard-mtlp: NOTRUN -> [SKIP][7] ([i915#8414]) +11 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-mtlp-3/igt@drm_fdinfo@busy-h...@rcs0.html * igt@gem_ctx_persistence@legacy-engines-hostile-preempt: - shard-snb: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#1099]) +1 similar issue [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-snb6/igt@gem_ctx_persiste...@legacy-engines-hostile-preempt.html * igt@gem_eio@in-flight-1us: - shard-mtlp: [PASS][9] -> [ABORT][10] ([i915#8503]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-mtlp-8/igt@gem_...@in-flight-1us.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-mtlp-1/igt@gem_...@in-flight-1us.html * igt@gem_exec_balancer@invalid-bonds: - shard-dg2: NOTRUN -> [SKIP][11] ([i915#4036]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-dg2-5/igt@gem_exec_balan...@invalid-bonds.html * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][12] -> [FAIL][13] ([i915#2846]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-glk3/igt@gem_exec_f...@basic-deadline.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-glk4/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_fair@basic-pace-solo@rcs0: - shard-rkl: [PASS][14] -> [FAIL][15] ([i915#2842]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/shard-rkl-1/igt@gem_exec_fair@basic-pace-s...@rcs0.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-rkl-4/igt@gem_exec_fair@basic-pace-s...@rcs0.html * igt@gem_exec_fence@submit67: - shard-mtlp: NOTRUN -> [SKIP][16] ([i915#4812]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-mtlp-3/igt@gem_exec_fe...@submit67.html * igt@gem_exec_flush@basic-wb-ro-default: - shard-dg2: NOTRUN -> [SKIP][17] ([i915#3539] / [i915#4852]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-dg2-5/igt@gem_exec_fl...@basic-wb-ro-default.html * igt@gem_exec_reloc@basic-gtt-active: - shard-mtlp: NOTRUN -> [SKIP][18] ([i915#3281]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-mtlp-3/igt@gem_exec_re...@basic-gtt-active.html * igt@gem_exec_reloc@basic-write-gtt: - shard-rkl: NOTRUN -> [SKIP][19] ([i915#3281]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121491v1/shard-rkl-4/igt@gem_exec_re...@basic-write-gtt.html *
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Avoid circular locking dependency when flush delayed work on gt reset (rev4)
== Series Details == Series: drm/i915: Avoid circular locking dependency when flush delayed work on gt reset (rev4) URL : https://patchwork.freedesktop.org/series/118898/ State : success == Summary == CI Bug Log - changes from CI_DRM_13434_full -> Patchwork_118898v4_full Summary --- **SUCCESS** No regressions found. Participating hosts (10 -> 10) -- No changes in participating hosts Known issues Here are the changes found in Patchwork_118898v4_full that come from known issues: ### IGT changes ### Issues hit * igt@api_intel_bb@blit-reloc-purge-cache: - shard-dg2: NOTRUN -> [SKIP][1] ([i915#8411]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-dg2-2/igt@api_intel...@blit-reloc-purge-cache.html - shard-dg1: NOTRUN -> [SKIP][2] ([i915#8411]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-dg1-14/igt@api_intel...@blit-reloc-purge-cache.html - shard-mtlp: NOTRUN -> [SKIP][3] ([i915#8411]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-mtlp-6/igt@api_intel...@blit-reloc-purge-cache.html * igt@debugfs_test@basic-hwmon: - shard-mtlp: NOTRUN -> [SKIP][4] ([i915#7456]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-mtlp-6/igt@debugfs_t...@basic-hwmon.html * igt@drm_fdinfo@busy-check-all@bcs0: - shard-dg1: NOTRUN -> [SKIP][5] ([i915#8414]) +4 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-dg1-15/igt@drm_fdinfo@busy-check-...@bcs0.html * igt@drm_fdinfo@busy-hang@rcs0: - shard-mtlp: NOTRUN -> [SKIP][6] ([i915#8414]) +6 similar issues [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-mtlp-8/igt@drm_fdinfo@busy-h...@rcs0.html * igt@drm_fdinfo@most-busy-check-all@rcs0: - shard-rkl: [PASS][7] -> [FAIL][8] ([i915#7742]) +1 similar issue [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13434/shard-rkl-6/igt@drm_fdinfo@most-busy-check-...@rcs0.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-rkl-6/igt@drm_fdinfo@most-busy-check-...@rcs0.html * igt@feature_discovery@chamelium: - shard-dg2: NOTRUN -> [SKIP][9] ([i915#4854]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-dg2-6/igt@feature_discov...@chamelium.html * igt@gem_busy@close-race: - shard-tglu: [PASS][10] -> [ABORT][11] ([i915#6016]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13434/shard-tglu-5/igt@gem_b...@close-race.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-tglu-4/igt@gem_b...@close-race.html * igt@gem_ccs@ctrl-surf-copy: - shard-dg1: NOTRUN -> [SKIP][12] ([i915#3555] / [i915#5325]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-dg1-15/igt@gem_...@ctrl-surf-copy.html * igt@gem_ccs@ctrl-surf-copy-new-ctx: - shard-mtlp: NOTRUN -> [SKIP][13] ([i915#5325]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-mtlp-6/igt@gem_...@ctrl-surf-copy-new-ctx.html * igt@gem_ctx_persistence@heartbeat-hostile: - shard-mtlp: NOTRUN -> [SKIP][14] ([i915#8555]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-mtlp-6/igt@gem_ctx_persiste...@heartbeat-hostile.html * igt@gem_ctx_persistence@legacy-engines-mixed: - shard-snb: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#1099]) +2 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-snb1/igt@gem_ctx_persiste...@legacy-engines-mixed.html * igt@gem_eio@in-flight-contexts-1us: - shard-mtlp: [PASS][16] -> [ABORT][17] ([i915#8503]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13434/shard-mtlp-1/igt@gem_...@in-flight-contexts-1us.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-mtlp-6/igt@gem_...@in-flight-contexts-1us.html * igt@gem_eio@unwedge-stress: - shard-snb: NOTRUN -> [FAIL][18] ([i915#8898]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-snb4/igt@gem_...@unwedge-stress.html * igt@gem_exec_await@wide-contexts: - shard-dg2: [PASS][19] -> [FAIL][20] ([i915#5892]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13434/shard-dg2-1/igt@gem_exec_aw...@wide-contexts.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-dg2-1/igt@gem_exec_aw...@wide-contexts.html * igt@gem_exec_balancer@invalid-bonds: - shard-dg2: NOTRUN -> [SKIP][21] ([i915#4036]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v4/shard-dg2-2/igt@gem_exec_balan...@invalid-bonds.html - shard-dg1: NOTRUN -> [SKIP][22] ([i915#4036]) [22]:
Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix LT debug print in SDP CRC enable
Hello Arun, > -Original Message- > From: Intel-gfx On Behalf Of Arun R > Murthy > Sent: Friday, July 14, 2023 11:08 AM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH] drm/i915/dp: Fix LT debug print in SDP CRC enable > > The debug print for enabling SDP CRC16 is applicable only for DP2.0, DP2.0 and UHBR? >but this > debug print was not within the uhbr check and was always printed. > Fis this by adding proper checks and returning. Typo. > > Signed-off-by: Arun R Murthy > --- > .../gpu/drm/i915/display/intel_dp_link_training.c| 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > index a263773f4d68..4485ef4f8ec6 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > @@ -1390,11 +1390,13 @@ void intel_dp_128b132b_sdp_crc16(struct > intel_dp *intel_dp, >* Default value of bit 31 is '0' hence discarding the write >* TODO: Corrective actions on SDP corruption yet to be defined >*/ > - if (intel_dp_is_uhbr(crtc_state)) > - /* DP v2.0 SCR on SDP CRC16 for 128b/132b Link Layer */ > - drm_dp_dpcd_writeb(_dp->aux, > - > DP_SDP_ERROR_DETECTION_CONFIGURATION, > -DP_SDP_CRC16_128B132B_EN); > + if (!intel_dp_is_uhbr(crtc_state)) > + return; I see that while calling this function in intel_ddi_pre_enable_dp(), we do have a check for for DP2.0 if (HAS_DP20(dev_priv)) intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder), crtc_state); Should this check be added within the function too for the sake of completion? Regards Chaitanya > + > + /* DP v2.0 SCR on SDP CRC16 for 128b/132b Link Layer */ > + drm_dp_dpcd_writeb(_dp->aux, > +DP_SDP_ERROR_DETECTION_CONFIGURATION, > +DP_SDP_CRC16_128B132B_EN); > > lt_dbg(intel_dp, DP_PHY_DPRX, "DP2.0 SDP CRC16 for 128b/132b > enabled\n"); } > -- > 2.25.1
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Hold reference to intel_context over life of i915_request
== Series Details == Series: drm/i915: Hold reference to intel_context over life of i915_request URL : https://patchwork.freedesktop.org/series/121510/ State : success == Summary == CI Bug Log - changes from CI_DRM_13435 -> Patchwork_121510v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/index.html Participating hosts (43 -> 41) -- Missing(2): fi-snb-2520m fi-pnv-d510 Known issues Here are the changes found in Patchwork_121510v1 that come from known issues: ### IGT changes ### Issues hit * igt@i915_pm_rpm@basic-pci-d3-state: - bat-adlp-9: [PASS][1] -> [FAIL][2] ([i915#7940]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-adlp-9/igt@i915_pm_...@basic-pci-d3-state.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-adlp-9/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_pm_rpm@module-reload: - fi-cfl-8700k: [PASS][3] -> [FAIL][4] ([i915#7940]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/fi-cfl-8700k/igt@i915_pm_...@module-reload.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/fi-cfl-8700k/igt@i915_pm_...@module-reload.html * igt@i915_selftest@live@gt_mocs: - bat-mtlp-8: [PASS][5] -> [DMESG-FAIL][6] ([i915#7059]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html * igt@i915_selftest@live@requests: - bat-mtlp-8: [PASS][7] -> [DMESG-FAIL][8] ([i915#8497]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-mtlp-8/igt@i915_selftest@l...@requests.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-mtlp-8/igt@i915_selftest@l...@requests.html - bat-mtlp-6: [PASS][9] -> [DMESG-FAIL][10] ([i915#8497]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-mtlp-6/igt@i915_selftest@l...@requests.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-mtlp-6/igt@i915_selftest@l...@requests.html * igt@i915_selftest@live@reset: - bat-rpls-2: [PASS][11] -> [ABORT][12] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#8347]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-rpls-2/igt@i915_selftest@l...@reset.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-rpls-2/igt@i915_selftest@l...@reset.html * igt@i915_selftest@live@slpc: - bat-rpls-1: [PASS][13] -> [DMESG-WARN][14] ([i915#6367]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-rpls-1/igt@i915_selftest@l...@slpc.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-rpls-1/igt@i915_selftest@l...@slpc.html Possible fixes * igt@i915_pm_rpm@basic-rte: - fi-kbl-7567u: [FAIL][15] ([i915#7940]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/fi-kbl-7567u/igt@i915_pm_...@basic-rte.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/fi-kbl-7567u/igt@i915_pm_...@basic-rte.html * igt@i915_pm_rpm@module-reload: - fi-tgl-1115g4: [FAIL][17] ([i915#7940]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/fi-tgl-1115g4/igt@i915_pm_...@module-reload.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/fi-tgl-1115g4/igt@i915_pm_...@module-reload.html * igt@i915_selftest@live@slpc: - bat-mtlp-6: [DMESG-WARN][19] ([i915#6367]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-mtlp-6/igt@i915_selftest@l...@slpc.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-mtlp-6/igt@i915_selftest@l...@slpc.html Warnings * igt@kms_psr@cursor_plane_move: - bat-rplp-1: [SKIP][21] ([i915#1072]) -> [ABORT][22] ([i915#8434] / [i915#8668]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13435/bat-rplp-1/igt@kms_psr@cursor_plane_move.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121510v1/bat-rplp-1/igt@kms_psr@cursor_plane_move.html [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367 [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059 [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461 [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913 [i915#7940]: https://gitlab.freedesktop.org/drm/intel/issues/7940 [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347 [i915#8434]:
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Hold reference to intel_context over life of i915_request
== Series Details == Series: drm/i915: Hold reference to intel_context over life of i915_request URL : https://patchwork.freedesktop.org/series/121510/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Hold reference to intel_context over life of i915_request
== Series Details == Series: drm/i915: Hold reference to intel_context over life of i915_request URL : https://patchwork.freedesktop.org/series/121510/ State : warning == Summary == Error: dim checkpatch failed 116e3202bc7d drm/i915: Hold reference to intel_context over life of i915_request -:28: CHECK:SPACING: No space is necessary after a cast #28: FILE: drivers/gpu/drm/i915/gt/intel_engine_types.h:61: +#define VIRTUAL_ENGINES BIT(BITS_PER_TYPE(intel_engine_mask_t) - 1) total: 0 errors, 0 warnings, 1 checks, 33 lines checked
Re: [Intel-gfx] [PATCH] drm/i915: Hold reference to intel_context over life of i915_request
On 28.07.2023 09:54, Andrzej Hajda wrote:
References to i915_requests may be trapped by userspace inside a
sync_file or dmabuf (dma-resv) and held indefinitely across different
proceses. To counter-act the memory leaks, we try to not to keep
references from the request past their completion.
On the other side on fence release we need to know if rq->engine
is valid and points to hw engine (true for non-virtual requests).
To make it possible extra bit has been added to rq->execution_mask,
for marking virtual engines.
Fixes: bcb9aa45d5a0 ("Revert "drm/i915: Hold reference to intel_context over life of
i915_request"")
Signed-off-by: Chris Wilson
Signed-off-by: Andrzej Hajda
Ups, I forgot to change the subject, should be:
drm/i915: fix request_pool assignment code on request release
Regards
Andrzej
---
Hi all,
This is squash of revert of fixed patch with Chris fix for internal
branch with expanded description.
Regards
Andrzej
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 1 +
drivers/gpu/drm/i915/i915_request.c | 7 ++-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index e99a6fa03d4539..a7e6775980043c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -58,6 +58,7 @@ struct i915_perf_group;
typedef u32 intel_engine_mask_t;
#define ALL_ENGINES ((intel_engine_mask_t)~0ul)
+#define VIRTUAL_ENGINES BIT(BITS_PER_TYPE(intel_engine_mask_t) - 1)
struct intel_hw_status_page {
struct list_head timelines;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0e3ef1c65d246..e7f748b2102263 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -5469,6 +5469,7 @@ guc_create_virtual(struct intel_engine_cs **siblings,
unsigned int count,
ve->base.submit_request = guc_submit_request;
ve->base.flags = I915_ENGINE_IS_VIRTUAL;
+ ve->base.mask = VIRTUAL_ENGINES;
intel_context_init(>context, >base);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 721e6aefec6b4d..0679863d10244f 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -134,9 +134,7 @@ static void i915_fence_release(struct dma_fence *fence)
i915_sw_fence_fini(>semaphore);
/*
-* Keep one request on each engine for reserved use under mempressure
-* do not use with virtual engines as this really is only needed for
-* kernel contexts.
+* Keep one request on each engine for reserved use under mempressure.
*
* We do not hold a reference to the engine here and so have to be
* very careful in what rq->engine we poke. The virtual engine is
@@ -166,8 +164,7 @@ static void i915_fence_release(struct dma_fence *fence)
* know that if the rq->execution_mask is a single bit, rq->engine
* can be a physical engine with the exact corresponding mask.
*/
- if (!intel_engine_is_virtual(rq->engine) &&
- is_power_of_2(rq->execution_mask) &&
+ if (is_power_of_2(rq->execution_mask) &&
!cmpxchg(>engine->request_pool, NULL, rq))
return;
Re: [Intel-gfx] [RFC 2/8] drm/i915: Split PTE encode between Gen12 and Meteorlake
On 27/07/2023 23:25, Matt Roper wrote:
On Thu, Jul 27, 2023 at 03:54:58PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
No need to run extra instructions which will never trigger on platforms
before Meteorlake.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index c8568e5d1147..862ac1d2de25 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -63,6 +63,30 @@ static u64 gen12_pte_encode(dma_addr_t addr,
{
gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW;
+ if (unlikely(flags & PTE_READ_ONLY))
+ pte &= ~GEN8_PAGE_RW;
+
+ if (flags & PTE_LM)
+ pte |= GEN12_PPGTT_PTE_LM;
+
+ if (pat_index & BIT(0))
+ pte |= GEN12_PPGTT_PTE_PAT0;
+
+ if (pat_index & BIT(1))
+ pte |= GEN12_PPGTT_PTE_PAT1;
+
+ if (pat_index & BIT(2))
+ pte |= GEN12_PPGTT_PTE_PAT2;
+
+ return pte;
+}
+
+static u64 mtl_pte_encode(dma_addr_t addr,
+ unsigned int pat_index,
+ u32 flags)
+{
+ gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW;
+
Would it be more readable to start with
gen8_pte_t pte = gen12_pte_encode(addr, pat_index, flags);
and then |-in only the MTL-specific bit(s) as appropriate?
if (unlikely(flags & PTE_READ_ONLY))
pte &= ~GEN8_PAGE_RW;
@@ -995,6 +1019,8 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
*/
ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
+ if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70))
+ ppgtt->vm.pte_encode = mtl_pte_encode;
if (GRAPHICS_VER(gt->i915) >= 12)
ppgtt->vm.pte_encode = gen12_pte_encode;
I think you wanted 'else if' here. Otherwise you clobber the MTL
function pointer.
Doh this was a strong fail.. Yes and yes.. I even had it like you
suggest in that patch I mentioned to you earlier..
https://patchwork.freedesktop.org/patch/546013/?series=120341=2.
Do you have an opinion on that one perhaps?
Thanks,
Tvrtko
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Make i915_coherent_map_type GT-centric
On Fri, Jul 28, 2023 at 09:07:14AM +0100, Tvrtko Ursulin wrote:
>
> On 28/07/2023 02:34, Andi Shyti wrote:
> > Hi Daniele and John,
> >
> > On Thu, Jul 27, 2023 at 12:35:02PM +0100, Tvrtko Ursulin wrote:
> > >
> > > On 26/07/2023 16:53, Jonathan Cavitt wrote:
> > > > Refactor i915_coherent_map_type to be GT-centric rather than
> > > > device-centric. Each GT may require different coherency
> > > > handling due to hardware workarounds.
> > > >
> > > > Since the function now takes a GT instead of the i915, the function is
> > > > renamed and moved to the gt folder.
> > > >
> > > > Suggested-by: Matt Roper
> > > > Signed-off-by: Jonathan Cavitt
> > >
> > > Works for me, thanks!
> > >
> > > I'd only suggest people familiar with the GuC/HuC/PXP side of things have
> > > a
> > > look on whether all the object in those path are shared access (GPU and
> > > CPU)
> > > at runtime. Maybe some are one off too. But that can be done later too.
> >
> > could you please take a look at this patch and if everything
> > looks all right from the GuC side, kindly ack it?
>
> Me? I can upgrade the "works for me" into an explicit ack if you want.
ehehe... I was asking GuC guys because you wanted a confirmation
from someone familiar with GuC/HuC/PXP :)
Andi
> Acked-by: Tvrtko Ursulin
>
> Regards,
>
> Tvrtko
>
> > Thanks,
> > Andi
> >
> > > Regards,
> > >
> > > Tvrtko
> > >
> > > > ---
> > > >drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++-
> > > >drivers/gpu/drm/i915/gem/i915_gem_object.h| 4
> > > >drivers/gpu/drm/i915/gem/i915_gem_pages.c | 15
> > > > ---
> > > >.../gpu/drm/i915/gem/selftests/i915_gem_migrate.c | 12 ++--
> > > >drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 +-
> > > >drivers/gpu/drm/i915/gt/intel_gt.c| 15
> > > > +++
> > > >drivers/gpu/drm/i915/gt/intel_gt.h| 3 +++
> > > >drivers/gpu/drm/i915/gt/intel_gtt.c | 4 ++--
> > > >drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> > > >drivers/gpu/drm/i915/gt/intel_ring.c | 3 ++-
> > > >drivers/gpu/drm/i915/gt/selftest_context.c| 2 +-
> > > >drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
> > > >drivers/gpu/drm/i915/gt/selftest_lrc.c| 2 +-
> > > >drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 3 +--
> > > >drivers/gpu/drm/i915/gt/uc/intel_guc.c| 2 +-
> > > >drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 3 +--
> > > >drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 ++-
> > > >drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c| 3 ++-
> > > >drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 3 ++-
> > > >drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +-
> > > >20 files changed, 46 insertions(+), 44 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > > > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > > > index ad0405375881..d753db3eef15 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > > > @@ -6,6 +6,7 @@
> > > >#include
> > > >#include "gem/i915_gem_region.h"
> > > > +#include "gt/intel_gt.h"
> > > >#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h"
> > > >#include "i915_drv.h"
> > > >#include "i915_utils.h"
> > > > @@ -632,7 +633,7 @@ static int intel_hdcp_gsc_initialize_message(struct
> > > > drm_i915_private *i915,
> > > > return PTR_ERR(obj);
> > > > }
> > > > - cmd_in = i915_gem_object_pin_map_unlocked(obj,
> > > > i915_coherent_map_type(i915, obj, true));
> > > > + cmd_in = i915_gem_object_pin_map_unlocked(obj,
> > > > intel_gt_coherent_map_type(gt, obj, true));
> > > > if (IS_ERR(cmd_in)) {
> > > > drm_err(>drm, "Failed to map gsc message
> > > > page!\n");
> > > > err = PTR_ERR(cmd_in);
> > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> > > > b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> > > > index 884a17275b3a..0c695b4c129f 100644
> > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> > > > @@ -716,10 +716,6 @@ void *__must_check i915_gem_object_pin_map(struct
> > > > drm_i915_gem_object *obj,
> > > >void *__must_check i915_gem_object_pin_map_unlocked(struct
> > > > drm_i915_gem_object *obj,
> > > > enum i915_map_type
> > > > type);
> > > > -enum i915_map_type i915_coherent_map_type(struct drm_i915_private
> > > > *i915,
> > > > - struct drm_i915_gem_object
> > > > *obj,
> > > > - bool always_coherent);
> > > > -
> > > >void __i915_gem_object_flush_map(struct drm_i915_gem_object *obj,
> > > >
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Make i915_coherent_map_type GT-centric
On 28/07/2023 02:34, Andi Shyti wrote:
Hi Daniele and John,
On Thu, Jul 27, 2023 at 12:35:02PM +0100, Tvrtko Ursulin wrote:
On 26/07/2023 16:53, Jonathan Cavitt wrote:
Refactor i915_coherent_map_type to be GT-centric rather than
device-centric. Each GT may require different coherency
handling due to hardware workarounds.
Since the function now takes a GT instead of the i915, the function is
renamed and moved to the gt folder.
Suggested-by: Matt Roper
Signed-off-by: Jonathan Cavitt
Works for me, thanks!
I'd only suggest people familiar with the GuC/HuC/PXP side of things have a
look on whether all the object in those path are shared access (GPU and CPU)
at runtime. Maybe some are one off too. But that can be done later too.
could you please take a look at this patch and if everything
looks all right from the GuC side, kindly ack it?
Me? I can upgrade the "works for me" into an explicit ack if you want.
Acked-by: Tvrtko Ursulin
Regards,
Tvrtko
Thanks,
Andi
Regards,
Tvrtko
---
drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++-
drivers/gpu/drm/i915/gem/i915_gem_object.h| 4
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 15 ---
.../gpu/drm/i915/gem/selftests/i915_gem_migrate.c | 12 ++--
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt.c| 15 +++
drivers/gpu/drm/i915/gt/intel_gt.h| 3 +++
drivers/gpu/drm/i915/gt/intel_gtt.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
drivers/gpu/drm/i915/gt/intel_ring.c | 3 ++-
drivers/gpu/drm/i915/gt/selftest_context.c| 2 +-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
drivers/gpu/drm/i915/gt/selftest_lrc.c| 2 +-
drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 3 +--
drivers/gpu/drm/i915/gt/uc/intel_guc.c| 2 +-
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 3 +--
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c| 3 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 3 ++-
drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +-
20 files changed, 46 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
index ad0405375881..d753db3eef15 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
@@ -6,6 +6,7 @@
#include
#include "gem/i915_gem_region.h"
+#include "gt/intel_gt.h"
#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h"
#include "i915_drv.h"
#include "i915_utils.h"
@@ -632,7 +633,7 @@ static int intel_hdcp_gsc_initialize_message(struct
drm_i915_private *i915,
return PTR_ERR(obj);
}
- cmd_in = i915_gem_object_pin_map_unlocked(obj,
i915_coherent_map_type(i915, obj, true));
+ cmd_in = i915_gem_object_pin_map_unlocked(obj,
intel_gt_coherent_map_type(gt, obj, true));
if (IS_ERR(cmd_in)) {
drm_err(>drm, "Failed to map gsc message page!\n");
err = PTR_ERR(cmd_in);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 884a17275b3a..0c695b4c129f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -716,10 +716,6 @@ void *__must_check i915_gem_object_pin_map(struct
drm_i915_gem_object *obj,
void *__must_check i915_gem_object_pin_map_unlocked(struct
drm_i915_gem_object *obj,
enum i915_map_type type);
-enum i915_map_type i915_coherent_map_type(struct drm_i915_private *i915,
- struct drm_i915_gem_object *obj,
- bool always_coherent);
-
void __i915_gem_object_flush_map(struct drm_i915_gem_object *obj,
unsigned long offset,
unsigned long size);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 89fc8ea6bcfc..6d262d269c71 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -465,21 +465,6 @@ void *i915_gem_object_pin_map_unlocked(struct
drm_i915_gem_object *obj,
return ret;
}
-enum i915_map_type i915_coherent_map_type(struct drm_i915_private *i915,
- struct drm_i915_gem_object *obj,
- bool always_coherent)
-{
- /*
-* Wa_22016122933: always return I915_MAP_WC for MTL
-*/
- if (i915_gem_object_is_lmem(obj) || IS_METEORLAKE(i915))
- return I915_MAP_WC;
- if (HAS_LLC(i915) || always_coherent)
- return
Re: [Intel-gfx] [PATCH] drm/i915/gem: Add check for bitmap_zalloc()
Hi,
On 28/07/2023 02:58, Jiasheng Jiang wrote:
Add the check for the return value of bitmap_zalloc() in order to
guarantee the success of the allocation.
Fixes: e9b73c67390a ("drm/i915: Reduce memory pressure during shrinker by
preallocating swizzle pages")
Signed-off-by: Jiasheng Jiang
---
drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index a049ca0b7980..e9cf99d95966 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -311,6 +311,11 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
if (!obj->bit_17) {
obj->bit_17 = bitmap_zalloc(obj->base.size >>
PAGE_SHIFT,
GFP_KERNEL);
+ if (!obj->bit_17) {
+ i915_gem_object_unlock(obj);
+ i915_gem_object_release_mmap_gtt(obj);
+ return -ENOMEM;
+ }
Hm the comment few lines above says:
/* Try to preallocate memory required to save swizzling on put-pages */
Lets emphasis the *try* for now. Then once the obj->bit_17 is attempted to be
used we have this:
i915_gem_object_save_bit_17_swizzle(..)
{
...
if (obj->bit_17 == NULL) {
obj->bit_17 = bitmap_zalloc(page_count, GFP_KERNEL);
if (obj->bit_17 == NULL) {
drm_err(obj->base.dev,
"Failed to allocate memory for bit 17
record\n");
return;
}
}
So despite this area of the driver being a bit before my time, I'd say it quite
possibly works as designed - only *tries* to preallocate but does not have to
and can cope with a later failure.
Good question might be why wouldn't it be better to do what you suggest. Trade
off would be between failing the ioctl and possibly crashing the application,
versus visual corruption if at use time allocation fails.
The whole swizzling thing also only applies to old GPUs, stuff before
Broadwell, which itself was released in 2014. So it is tempting to err on the
side of caution and leave it as is. I'll mull it over in the background, or
maybe someone else will have an opinion too.
Regards,
Tvrtko
}
} else {
bitmap_free(obj->bit_17);
[Intel-gfx] [PATCH] drm/i915: Hold reference to intel_context over life of i915_request
References to i915_requests may be trapped by userspace inside a
sync_file or dmabuf (dma-resv) and held indefinitely across different
proceses. To counter-act the memory leaks, we try to not to keep
references from the request past their completion.
On the other side on fence release we need to know if rq->engine
is valid and points to hw engine (true for non-virtual requests).
To make it possible extra bit has been added to rq->execution_mask,
for marking virtual engines.
Fixes: bcb9aa45d5a0 ("Revert "drm/i915: Hold reference to intel_context over
life of i915_request"")
Signed-off-by: Chris Wilson
Signed-off-by: Andrzej Hajda
---
Hi all,
This is squash of revert of fixed patch with Chris fix for internal
branch with expanded description.
Regards
Andrzej
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 1 +
drivers/gpu/drm/i915/i915_request.c | 7 ++-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index e99a6fa03d4539..a7e6775980043c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -58,6 +58,7 @@ struct i915_perf_group;
typedef u32 intel_engine_mask_t;
#define ALL_ENGINES ((intel_engine_mask_t)~0ul)
+#define VIRTUAL_ENGINES BIT(BITS_PER_TYPE(intel_engine_mask_t) - 1)
struct intel_hw_status_page {
struct list_head timelines;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0e3ef1c65d246..e7f748b2102263 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -5469,6 +5469,7 @@ guc_create_virtual(struct intel_engine_cs **siblings,
unsigned int count,
ve->base.submit_request = guc_submit_request;
ve->base.flags = I915_ENGINE_IS_VIRTUAL;
+ ve->base.mask = VIRTUAL_ENGINES;
intel_context_init(>context, >base);
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 721e6aefec6b4d..0679863d10244f 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -134,9 +134,7 @@ static void i915_fence_release(struct dma_fence *fence)
i915_sw_fence_fini(>semaphore);
/*
-* Keep one request on each engine for reserved use under mempressure
-* do not use with virtual engines as this really is only needed for
-* kernel contexts.
+* Keep one request on each engine for reserved use under mempressure.
*
* We do not hold a reference to the engine here and so have to be
* very careful in what rq->engine we poke. The virtual engine is
@@ -166,8 +164,7 @@ static void i915_fence_release(struct dma_fence *fence)
* know that if the rq->execution_mask is a single bit, rq->engine
* can be a physical engine with the exact corresponding mask.
*/
- if (!intel_engine_is_virtual(rq->engine) &&
- is_power_of_2(rq->execution_mask) &&
+ if (is_power_of_2(rq->execution_mask) &&
!cmpxchg(>engine->request_pool, NULL, rq))
return;
--
2.34.1
Re: [Intel-gfx] [RFC 4/8] drm/i915: Refactor PAT/object cache handling
[snip] > @@ -41,14 +42,17 @@ static bool gpu_write_needs_clflush(struct > drm_i915_gem_object *obj) > return false; > > /* > - * For objects created by userspace through GEM_CREATE with pat_index > - * set by set_pat extension, i915_gem_object_has_cache_level() will > - * always return true, because the coherency of such object is managed i915_gem_object_has_cache_level() always return true means this function always return false. > - * by userspace. Othereise the call here would fall back to checking > - * whether the object is un-cached or write-through. > + * Always flush cache for UMD objects with PAT index set. (obj->pat_set_by_user == true) indicates UMD knows how to handle the coherency, forcing clflush in KMD would be redundant. >*/ > - return !(i915_gem_object_has_cache_level(obj, I915_CACHE_NONE) || > - i915_gem_object_has_cache_level(obj, I915_CACHE_WT)); > + if (obj->pat_set_by_user) > + return true; return false; > + > + /* > + * Fully coherent cached access may end up with data in the CPU cache > + * which hasn't hit memory yet. > + */ > + return i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_WB) && > +i915_gem_object_has_cache_flag(obj, I915_CACHE_FLAG_COH2W); Why checking COH2W here? The logic was, if UC or WT return false, otherwise return true. So, as long as cache_mode is WB, it's sufficient to say true here, right? > }
