[Intel-gfx] [PATCH v3] drm/i915/pxp: don't start pxp without mei_pxp bind

[Intel-gfx] [PATCH v3] drm/i915/pxp: don't start pxp without mei_pxp bind

[Juston Li]

pxp will not start correctly until after mei_pxp bind completes and
intel_pxp_init_hw() is called.
Wait for the bind to complete before proceeding with startup.

This fixes a race condition during bootup where we observed a small
window for pxp commands to be sent, starting pxp before mei_pxp bind
completed.

Changes since v2:
- wait for pxp_component to bind instead of returning -EAGAIN (Daniele)

Changes since v1:
- check pxp_component instead of pxp_component_added (Daniele)
- pxp_component needs tee_mutex (Daniele)
- return -EAGAIN so caller knows to retry (Daniele)

Signed-off-by: Juston Li <justonli@chromium.org>
---
 drivers/gpu/drm/i915/pxp/intel_pxp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index 15311eaed848..17109c513259 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -176,6 +176,18 @@ static void pxp_queue_termination(struct intel_pxp *pxp)
 	spin_unlock_irq(&gt->irq_lock);
 }
 
+static bool pxp_component_bound(struct intel_pxp *pxp)
+{
+	bool bound = false;
+
+	mutex_lock(&pxp->tee_mutex);
+	if (pxp->pxp_component)
+		bound = true;
+	mutex_unlock(&pxp->tee_mutex);
+
+	return bound;
+}
+
 /*
  * the arb session is restarted from the irq work when we receive the
  * termination completion interrupt
@@ -187,6 +199,9 @@ int intel_pxp_start(struct intel_pxp *pxp)
 	if (!intel_pxp_is_enabled(pxp))
 		return -ENODEV;
 
+	if (wait_for(pxp_component_bound(pxp), 250))
+		return -ENXIO;
+
 	mutex_lock(&pxp->arb_mutex);
 
 	if (pxp->arb_is_valid)
-- 
2.37.1.595.g718a3a8f04-goog

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pxp: don't start pxp without mei_pxp bind (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3543 bytes --]

== Series Details ==

Series: drm/i915/pxp: don't start pxp without mei_pxp bind (rev3)
URL   : https://patchwork.freedesktop.org/series/107099/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11998 -> Patchwork_107099v3
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/index.html

Participating hosts (32 -> 31)
------------------------------

  Additional (1): fi-hsw-4770 
  Missing    (2): bat-dg2-10 bat-adls-5 

Known issues
------------

  Here are the changes found in Patchwork_107099v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-4770:        NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#3012])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-hsw-4770:        NOTRUN -> [INCOMPLETE][2] ([i915#4817] / [i915#6598])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/fi-hsw-4770/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - fi-hsw-4770:        NOTRUN -> [SKIP][3] ([fdo#109271]) +9 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/fi-hsw-4770/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-hsw-4770:        NOTRUN -> [SKIP][4] ([fdo#109271] / [fdo#111827]) +7 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/fi-hsw-4770/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_psr@sprite_plane_onoff:
    - fi-hsw-4770:        NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#1072]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html

  
#### Possible fixes ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
    - fi-bsw-kefka:       [FAIL][6] ([i915#6298]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6598]: https://gitlab.freedesktop.org/drm/intel/issues/6598


Build changes
-------------

  * Linux: CI_DRM_11998 -> Patchwork_107099v3

  CI-20190529: 20190529
  CI_DRM_11998: fa98df3eddac93dac4377cb41a1cc58beb314c91 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6632: a0ac4d449e551fd5c78b56f85cd534330ea60507 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_107099v3: fa98df3eddac93dac4377cb41a1cc58beb314c91 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

8e5edc9a226f drm/i915/pxp: don't start pxp without mei_pxp bind

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/index.html

[-- Attachment #2: Type: text/html, Size: 4481 bytes --]

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/pxp: don't start pxp without mei_pxp bind (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 12528 bytes --]

== Series Details ==

Series: drm/i915/pxp: don't start pxp without mei_pxp bind (rev3)
URL   : https://patchwork.freedesktop.org/series/107099/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11998_full -> Patchwork_107099v3_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_107099v3_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_107099v3_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (13 -> 11)
------------------------------

  Missing    (2): pig-glk-j5005 shard-dg1 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_107099v3_full:

### IGT changes ###

#### Warnings ####

  * igt@gem_eio@in-flight-suspend:
    - shard-apl:          [INCOMPLETE][1] ([i915#6598]) -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl7/igt@gem_eio@in-flight-suspend.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl6/igt@gem_eio@in-flight-suspend.html

  
Known issues
------------

  Here are the changes found in Patchwork_107099v3_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@wait-wedge-1us:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([i915#5904] / [i915#62]) +10 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl3/igt@gem_eio@wait-wedge-1us.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl8/igt@gem_eio@wait-wedge-1us.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([i915#2842])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl3/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - shard-kbl:          [PASS][7] -> [FAIL][8] ([i915#2842]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-kbl7/igt@gem_exec_fair@basic-none@vecs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-kbl7/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@i915_selftest@live@hangcheck:
    - shard-tglb:         [PASS][9] -> [DMESG-WARN][10] ([i915#5591])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-tglb7/igt@i915_selftest@live@hangcheck.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-tglb3/igt@i915_selftest@live@hangcheck.html

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#3886]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium@dp-hpd-storm-disable:
    - shard-apl:          NOTRUN -> [SKIP][12] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@kms_chamelium@dp-hpd-storm-disable.html

  * igt@kms_color@legacy-gamma@pipe-b-dp-1:
    - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([i915#180] / [i915#5904] / [i915#62]) +19 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl3/igt@kms_color@legacy-gamma@pipe-b-dp-1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl8/igt@kms_color@legacy-gamma@pipe-b-dp-1.html

  * igt@kms_cursor_crc@cursor-rapid-movement@pipe-a-dp-1-128x128:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([i915#62]) +75 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl7/igt@kms_cursor_crc@cursor-rapid-movement@pipe-a-dp-1-128x128.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl8/igt@kms_cursor_crc@cursor-rapid-movement@pipe-a-dp-1-128x128.html

  * igt@kms_draw_crc@draw-method-rgb565-mmap-wc-4tiled:
    - shard-apl:          NOTRUN -> [SKIP][17] ([fdo#109271]) +32 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@kms_draw_crc@draw-method-rgb565-mmap-wc-4tiled.html

  * igt@kms_flip@flip-vs-wf_vblank-interruptible@a-dp1:
    - shard-apl:          [PASS][18] -> [DMESG-WARN][19] ([i915#180] / [i915#1982] / [i915#5904] / [i915#62])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl3/igt@kms_flip@flip-vs-wf_vblank-interruptible@a-dp1.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl8/igt@kms_flip@flip-vs-wf_vblank-interruptible@a-dp1.html

  * igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1:
    - shard-kbl:          [PASS][20] -> [FAIL][21] ([i915#1188]) +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-kbl1/igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-kbl1/igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> [FAIL][22] ([fdo#108145] / [i915#265])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][23] ([i915#265])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb.html

  * igt@kms_psr2_sf@overlay-plane-move-continuous-sf:
    - shard-apl:          NOTRUN -> [SKIP][24] ([fdo#109271] / [i915#658]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@kms_psr2_sf@overlay-plane-move-continuous-sf.html

  * igt@kms_psr_stress_test@flip-primary-invalidate-overlay:
    - shard-tglb:         [PASS][25] -> [SKIP][26] ([i915#5519])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-tglb7/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-tglb3/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-deadline:
    - shard-kbl:          [FAIL][27] ([i915#2846]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-kbl1/igt@gem_exec_fair@basic-deadline.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-kbl1/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none@vcs1:
    - shard-kbl:          [FAIL][29] ([i915#2842]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-kbl7/igt@gem_exec_fair@basic-none@vcs1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-kbl7/igt@gem_exec_fair@basic-none@vcs1.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-apl:          [DMESG-WARN][31] ([i915#5566] / [i915#716]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl8/igt@gen9_exec_parse@allowed-single.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl2/igt@gen9_exec_parse@allowed-single.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
    - shard-apl:          [DMESG-WARN][33] ([i915#1982] / [i915#62]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl8/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl8/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html

  * igt@kms_cursor_crc@cursor-offscreen@pipe-c-dp-1-128x42:
    - shard-apl:          [DMESG-WARN][35] ([i915#62]) -> [PASS][36] +30 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl8/igt@kms_cursor_crc@cursor-offscreen@pipe-c-dp-1-128x42.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl8/igt@kms_cursor_crc@cursor-offscreen@pipe-c-dp-1-128x42.html

  * igt@kms_hdr@bpc-switch@pipe-a-dp-1:
    - shard-kbl:          [FAIL][37] ([i915#1188]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-kbl7/igt@kms_hdr@bpc-switch@pipe-a-dp-1.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-kbl4/igt@kms_hdr@bpc-switch@pipe-a-dp-1.html

  
#### Warnings ####

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-kbl:          [FAIL][39] ([i915#2842]) -> [SKIP][40] ([fdo#109271])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-kbl7/igt@gem_exec_fair@basic-pace@vecs0.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-kbl1/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@i915_suspend@forcewake:
    - shard-apl:          [INCOMPLETE][41] ([i915#6598]) -> [INCOMPLETE][42] ([i915#180] / [i915#6598])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl4/igt@i915_suspend@forcewake.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl4/igt@i915_suspend@forcewake.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-apl:          [FAIL][43] ([fdo#108145] / [i915#265]) -> [DMESG-FAIL][44] ([fdo#108145] / [i915#180] / [i915#62])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl3/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl8/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html

  * igt@runner@aborted:
    - shard-apl:          ([FAIL][45], [FAIL][46]) ([fdo#109271] / [i915#3002] / [i915#4312] / [i915#5257] / [i915#6599]) -> [FAIL][47] ([i915#3002] / [i915#4312] / [i915#5257] / [i915#6599])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl6/igt@runner@aborted.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11998/shard-apl8/igt@runner@aborted.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/shard-apl1/igt@runner@aborted.html

  
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5519]: https://gitlab.freedesktop.org/drm/intel/issues/5519
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
  [i915#5904]: https://gitlab.freedesktop.org/drm/intel/issues/5904
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6598]: https://gitlab.freedesktop.org/drm/intel/issues/6598
  [i915#6599]: https://gitlab.freedesktop.org/drm/intel/issues/6599
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716


Build changes
-------------

  * Linux: CI_DRM_11998 -> Patchwork_107099v3

  CI-20190529: 20190529
  CI_DRM_11998: fa98df3eddac93dac4377cb41a1cc58beb314c91 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6632: a0ac4d449e551fd5c78b56f85cd534330ea60507 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_107099v3: fa98df3eddac93dac4377cb41a1cc58beb314c91 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107099v3/index.html

[-- Attachment #2: Type: text/html, Size: 16014 bytes --]

Re: [Intel-gfx] [PATCH v3] drm/i915/pxp: don't start pxp without mei_pxp bind

[Andrzej Hajda]

On 18.08.2022 19:42, Juston Li wrote:
> pxp will not start correctly until after mei_pxp bind completes and
> intel_pxp_init_hw() is called.
> Wait for the bind to complete before proceeding with startup.
> 
> This fixes a race condition during bootup where we observed a small
> window for pxp commands to be sent, starting pxp before mei_pxp bind
> completed.
> 
> Changes since v2:
> - wait for pxp_component to bind instead of returning -EAGAIN (Daniele)
> 
> Changes since v1:
> - check pxp_component instead of pxp_component_added (Daniele)
> - pxp_component needs tee_mutex (Daniele)
> - return -EAGAIN so caller knows to retry (Daniele)
> 
> Signed-off-by: Juston Li <justonli@chromium.org>

In typical usage of component framework driver postpones initialization 
till component is bound. In such case checking/waiting for component as 
in this patch is not necessary and the code is more straightforward.
I wonder how it behaves on component unbind.

Anyway:
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej


> ---
>   drivers/gpu/drm/i915/pxp/intel_pxp.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index 15311eaed848..17109c513259 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -176,6 +176,18 @@ static void pxp_queue_termination(struct intel_pxp *pxp)
>   	spin_unlock_irq(&gt->irq_lock);
>   }
>   
> +static bool pxp_component_bound(struct intel_pxp *pxp)
> +{
> +	bool bound = false;
> +
> +	mutex_lock(&pxp->tee_mutex);
> +	if (pxp->pxp_component)
> +		bound = true;
> +	mutex_unlock(&pxp->tee_mutex);
> +
> +	return bound;
> +}
> +
>   /*
>    * the arb session is restarted from the irq work when we receive the
>    * termination completion interrupt
> @@ -187,6 +199,9 @@ int intel_pxp_start(struct intel_pxp *pxp)
>   	if (!intel_pxp_is_enabled(pxp))
>   		return -ENODEV;
>   
> +	if (wait_for(pxp_component_bound(pxp), 250))
> +		return -ENXIO;
> +
>   	mutex_lock(&pxp->arb_mutex);
>   
>   	if (pxp->arb_is_valid)

Re: [Intel-gfx] [PATCH v3] drm/i915/pxp: don't start pxp without mei_pxp bind

[Juston Li]

On Fri, Aug 19, 2022 at 4:53 AM Andrzej Hajda <andrzej.hajda@intel.com> wrote:
>
> On 18.08.2022 19:42, Juston Li wrote:
> > pxp will not start correctly until after mei_pxp bind completes and
> > intel_pxp_init_hw() is called.
> > Wait for the bind to complete before proceeding with startup.
> >
> > This fixes a race condition during bootup where we observed a small
> > window for pxp commands to be sent, starting pxp before mei_pxp bind
> > completed.
> >
> > Changes since v2:
> > - wait for pxp_component to bind instead of returning -EAGAIN (Daniele)
> >
> > Changes since v1:
> > - check pxp_component instead of pxp_component_added (Daniele)
> > - pxp_component needs tee_mutex (Daniele)
> > - return -EAGAIN so caller knows to retry (Daniele)
> >
> > Signed-off-by: Juston Li <justonli@chromium.org>
>
> In typical usage of component framework driver postpones initialization
> till component is bound. In such case checking/waiting for component as
> in this patch is not necessary and the code is more straightforward.
> I wonder how it behaves on component unbind.
>
> Anyway:
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Thanks Andrzej!

Any other comments Daniele?
Otherwise, need some help from someone to merge this :)

Thanks
Juston

> Regards
> Andrzej
>
>
> > ---
> >   drivers/gpu/drm/i915/pxp/intel_pxp.c | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > index 15311eaed848..17109c513259 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > @@ -176,6 +176,18 @@ static void pxp_queue_termination(struct intel_pxp *pxp)
> >       spin_unlock_irq(&gt->irq_lock);
> >   }
> >
> > +static bool pxp_component_bound(struct intel_pxp *pxp)
> > +{
> > +     bool bound = false;
> > +
> > +     mutex_lock(&pxp->tee_mutex);
> > +     if (pxp->pxp_component)
> > +             bound = true;
> > +     mutex_unlock(&pxp->tee_mutex);
> > +
> > +     return bound;
> > +}
> > +
> >   /*
> >    * the arb session is restarted from the irq work when we receive the
> >    * termination completion interrupt
> > @@ -187,6 +199,9 @@ int intel_pxp_start(struct intel_pxp *pxp)
> >       if (!intel_pxp_is_enabled(pxp))
> >               return -ENODEV;
> >
> > +     if (wait_for(pxp_component_bound(pxp), 250))
> > +             return -ENXIO;
> > +
> >       mutex_lock(&pxp->arb_mutex);
> >
> >       if (pxp->arb_is_valid)
>

Re: [Intel-gfx] [PATCH v3] drm/i915/pxp: don't start pxp without mei_pxp bind

[Ceraolo Spurio, Daniele]

On 8/23/2022 2:15 PM, Juston Li wrote:
> On Fri, Aug 19, 2022 at 4:53 AM Andrzej Hajda <andrzej.hajda@intel.com> wrote:
>> On 18.08.2022 19:42, Juston Li wrote:
>>> pxp will not start correctly until after mei_pxp bind completes and
>>> intel_pxp_init_hw() is called.
>>> Wait for the bind to complete before proceeding with startup.
>>>
>>> This fixes a race condition during bootup where we observed a small
>>> window for pxp commands to be sent, starting pxp before mei_pxp bind
>>> completed.
>>>
>>> Changes since v2:
>>> - wait for pxp_component to bind instead of returning -EAGAIN (Daniele)
>>>
>>> Changes since v1:
>>> - check pxp_component instead of pxp_component_added (Daniele)
>>> - pxp_component needs tee_mutex (Daniele)
>>> - return -EAGAIN so caller knows to retry (Daniele)
>>>
>>> Signed-off-by: Juston Li <justonli@chromium.org>
>> In typical usage of component framework driver postpones initialization
>> till component is bound. In such case checking/waiting for component as
>> in this patch is not necessary and the code is more straightforward.
>> I wonder how it behaves on component unbind.

This component is only used for a specific use-case (content 
protection), so we don't want to hold back the whole graphics driver 
initialization for that. Unbind can only happen on suspend or driver 
removal and in both cases we're not accepting userspace submission at 
that point.

>> Anyway:
>> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Thanks Andrzej!
>
> Any other comments Daniele?
> Otherwise, need some help from someone to merge this :)

No other comments from me. I've pushed the patch to gt-next.

Thanks,
Daniele

>
> Thanks
> Juston
>
>> Regards
>> Andrzej
>>
>>
>>> ---
>>>    drivers/gpu/drm/i915/pxp/intel_pxp.c | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
>>> index 15311eaed848..17109c513259 100644
>>> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
>>> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
>>> @@ -176,6 +176,18 @@ static void pxp_queue_termination(struct intel_pxp *pxp)
>>>        spin_unlock_irq(&gt->irq_lock);
>>>    }
>>>
>>> +static bool pxp_component_bound(struct intel_pxp *pxp)
>>> +{
>>> +     bool bound = false;
>>> +
>>> +     mutex_lock(&pxp->tee_mutex);
>>> +     if (pxp->pxp_component)
>>> +             bound = true;
>>> +     mutex_unlock(&pxp->tee_mutex);
>>> +
>>> +     return bound;
>>> +}
>>> +
>>>    /*
>>>     * the arb session is restarted from the irq work when we receive the
>>>     * termination completion interrupt
>>> @@ -187,6 +199,9 @@ int intel_pxp_start(struct intel_pxp *pxp)
>>>        if (!intel_pxp_is_enabled(pxp))
>>>                return -ENODEV;
>>>
>>> +     if (wait_for(pxp_component_bound(pxp), 250))
>>> +             return -ENXIO;
>>> +
>>>        mutex_lock(&pxp->arb_mutex);
>>>
>>>        if (pxp->arb_is_valid)