Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 00/26] Fix xe_force_wake_get() failure handling
@ 2024-10-14  7:55 Himal Prasad Ghimiray
  2024-10-14  7:44 ` ✓ CI.Patch_applied: success for Fix xe_force_wake_get() failure handling (rev10) Patchwork
                   ` (41 more replies)
  0 siblings, 42 replies; 82+ messages in thread
From: Himal Prasad Ghimiray @ 2024-10-14  7:55 UTC (permalink / raw)
  To: intel-xe
  Cc: Himal Prasad Ghimiray, Michal Wajdeczko, Badal Nilawar,
	Rodrigo Vivi, Lucas De Marchi, Nirmoy Das, Matthew Brost,
	Daniele Ceraolo Spurio, Ashutosh Dixit

In case of xe_force_wake_get() call the refcount for domains
are incremented irrespective of failure or success, which can lead to
undefined behaviour: 
[1] subsequent forcewake call by callee function assumes domains are 
already awake, which might not be true. This shows perfectly balanced 
xe_force_wake_get/_put can also cause problem.

[2] The functions are using xe_force_wake_get() and return on failures.
In this scenario only the first caller returns and other functions will
always assume domains to be awake.

[1] func_a() {
	XE_WARN(xe_force_wake_get()) <---> fails but increments refcount

	func_b();

	XE_WARN(xe_force_wake_put());<---> decrements refcounts 
 }

    func_b() {
	if(xe_force_wake_get()) <---> succeeds due to refcount of caller
		return;

	does mmio_operations(); <---> Domain might not be awake

	xe_force_wake_put(); <---> decrement refcount
 }
  	
[2] func_a() {
	int err = xe_force_wake_get(); <---> returns if fails, inc refcount
	if (err)
		return;

	mmio_operations();

	xe_force_wake_put(); 
 }

    func_b() {
        if (xe_force_wake_get()) <---> succeeds due to refcnt of func_a()
              return;

        mmio_operations(); <---> Invalid accesses

        xe_force_wake_put(); <---> decrements refcount for func_b
 }

This series ensures refcount is not incremented in case of
xe_force_wake_get() failure and return mask of successfully refcount 
incremented domains.

For single domains (except XE_FORCEWAKE_ALL), the return value will be
0 on failure and doesn't need call to xe_force_wake_put()(if called,
call will return with check since input mask will be 0).
For XE_FORCEWAKE_ALL caller need to compare the returned mask with
XE_FORCEWAKE_ALL to confirm the success of call and need to explicitly
call xe_force_wake_put() with returned mask to put awaken domains to
sleep.
 

-v2 (Rodrigo, Badal)
- Dont put successfully awaken domains to sleep. Instead rely on _put to
  do that.
- Modified xe_force_wake_get() to return the refcount-incremented domain
  mask and xe_force_wake_put() uses this returned mask.

-v3 (Michal, Badal)
- Use explicit type for domain masks.
- Dont use xe_assert in _get/_put error reporting.
- Restructure patches, so Patch 23 has only kernel-doc and return
  change.
- use xe_gt_WARN_ON instead of XE_WARN in _put error. 
- Improve kernel-doc for _get.

-v4
- Rebase fixes

-v5 (MattB, Rodrigo, Badal)
- Avoid explicit type unsigned int
- Use unsigned int as return for xe_force_wake_get()
- Add xe_gt_WARN_ON inside xe_force_wake_get() for ack failures

-v6 (Michal, Badal)
- Return actually refcounted domains by xe_force_wake_get(), to
  accommodate change XE_FORCEWAKE_ALL to single bit.
- Define a helper to check domain is in fw_ref or not.

-v7 (Michal, Badal)
- Fix kernel-doc and commit messages
- Add assert condition for valid input domains.

-v8
- Fix kernel-doc
- Rebase

-v9

- Fix kernel-doc
- Fix WARN_ON
- Rebase
 
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Badal Nilawar <badal.nilawar@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Himal Prasad Ghimiray (26):
  drm/xe: Add member initialized_domains to xe_force_wake()
  drm/xe/forcewake: Change awake_domain datatype
  drm/xe/forcewake: Add a helper xe_force_wake_ref_has_domain()
  drm/xe: Error handling in xe_force_wake_get()
  drm/xe: Modify xe_force_wake_put to handle _get returned mask
  drm/xe/device: Update handling of xe_force_wake_get return
  drm/xe/hdcp: Update handling of xe_force_wake_get return
  drm/xe/gsc: Update handling of xe_force_wake_get return
  drm/xe/gt: Update handling of xe_force_wake_get return
  drm/xe/xe_gt_idle: Update handling of xe_force_wake_get return
  drm/xe/devcoredump: Update handling of xe_force_wake_get return
  drm/xe/tests/mocs: Update xe_force_wake_get() return handling
  drm/xe/mocs: Update handling of xe_force_wake_get return
  drm/xe/xe_drm_client: Update handling of xe_force_wake_get return
  drm/xe/xe_gt_debugfs: Update handling of xe_force_wake_get return
  drm/xe/guc: Update handling of xe_force_wake_get return
  drm/xe/huc: Update handling of xe_force_wake_get return
  drm/xe/oa: Handle force_wake_get failure in xe_oa_stream_init()
  drm/xe/pat: Update handling of xe_force_wake_get return
  drm/xe/gt_tlb_invalidation_ggtt: Update handling of xe_force_wake_get
    return
  drm/xe/xe_reg_sr: Update handling of xe_force_wake_get return
  drm/xe/query: Update handling of xe_force_wake_get return
  drm/xe/vram: Update handling of xe_force_wake_get return
  drm/xe: forcewake debugfs open fails on xe_forcewake_get failure
  drm/xe: Ensure __must_check for xe_force_wake_get() return
  drm/xe: Change return type to void for xe_force_wake_put

 drivers/gpu/drm/xe/display/xe_hdcp_gsc.c    |   6 +-
 drivers/gpu/drm/xe/tests/xe_mocs.c          |  18 ++-
 drivers/gpu/drm/xe/xe_debugfs.c             |  27 ++++-
 drivers/gpu/drm/xe/xe_devcoredump.c         |  14 ++-
 drivers/gpu/drm/xe/xe_device.c              |  25 ++--
 drivers/gpu/drm/xe/xe_drm_client.c          |   8 +-
 drivers/gpu/drm/xe/xe_force_wake.c          | 122 +++++++++++++++-----
 drivers/gpu/drm/xe/xe_force_wake.h          |  23 +++-
 drivers/gpu/drm/xe/xe_force_wake_types.h    |   6 +-
 drivers/gpu/drm/xe/xe_gsc.c                 |  23 ++--
 drivers/gpu/drm/xe/xe_gsc_proxy.c           |   9 +-
 drivers/gpu/drm/xe/xe_gt.c                  | 105 +++++++++--------
 drivers/gpu/drm/xe/xe_gt_debugfs.c          |  13 +--
 drivers/gpu/drm/xe/xe_gt_idle.c             |  26 +++--
 drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c |   5 +-
 drivers/gpu/drm/xe/xe_guc.c                 |  13 ++-
 drivers/gpu/drm/xe/xe_guc_log.c             |   9 +-
 drivers/gpu/drm/xe/xe_guc_pc.c              |  50 +++++---
 drivers/gpu/drm/xe/xe_guc_submit.c          |   6 +-
 drivers/gpu/drm/xe/xe_huc.c                 |   8 +-
 drivers/gpu/drm/xe/xe_mocs.c                |  14 +--
 drivers/gpu/drm/xe/xe_oa.c                  |  11 +-
 drivers/gpu/drm/xe/xe_pat.c                 |  65 +++++------
 drivers/gpu/drm/xe/xe_query.c               |   8 +-
 drivers/gpu/drm/xe/xe_reg_sr.c              |  24 ++--
 drivers/gpu/drm/xe/xe_vram.c                |  12 +-
 26 files changed, 393 insertions(+), 257 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 82+ messages in thread

end of thread, other threads:[~2024-10-17 15:42 UTC | newest]

Thread overview: 82+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-14  7:55 [PATCH v9 00/26] Fix xe_force_wake_get() failure handling Himal Prasad Ghimiray
2024-10-14  7:44 ` ✓ CI.Patch_applied: success for Fix xe_force_wake_get() failure handling (rev10) Patchwork
2024-10-14  7:44 ` ✓ CI.checkpatch: " Patchwork
2024-10-14  7:46 ` ✓ CI.KUnit: " Patchwork
2024-10-14  7:55 ` [PATCH v9 01/26] drm/xe: Add member initialized_domains to xe_force_wake() Himal Prasad Ghimiray
2024-10-14  7:55 ` [PATCH v9 02/26] drm/xe/forcewake: Change awake_domain datatype Himal Prasad Ghimiray
2024-10-14  7:55 ` [PATCH v9 03/26] drm/xe/forcewake: Add a helper xe_force_wake_ref_has_domain() Himal Prasad Ghimiray
2024-10-14  7:55 ` [PATCH v9 04/26] drm/xe: Error handling in xe_force_wake_get() Himal Prasad Ghimiray
2024-10-14  8:32   ` Nirmoy Das
2024-10-14  7:55 ` [PATCH v9 05/26] drm/xe: Modify xe_force_wake_put to handle _get returned mask Himal Prasad Ghimiray
2024-10-14  8:52   ` Nirmoy Das
2024-10-14  7:55 ` [PATCH v9 06/26] drm/xe/device: Update handling of xe_force_wake_get return Himal Prasad Ghimiray
2024-10-15 13:28   ` Nirmoy Das
2024-10-15 14:35   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 07/26] drm/xe/hdcp: " Himal Prasad Ghimiray
2024-10-15 13:29   ` Nirmoy Das
2024-10-14  7:55 ` [PATCH v9 08/26] drm/xe/gsc: " Himal Prasad Ghimiray
2024-10-15 13:57   ` Nirmoy Das
2024-10-14  7:55 ` [PATCH v9 09/26] drm/xe/gt: " Himal Prasad Ghimiray
2024-10-15 14:24   ` Nirmoy Das
2024-10-15 14:44   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 10/26] drm/xe/xe_gt_idle: " Himal Prasad Ghimiray
2024-10-15 14:25   ` Nirmoy Das
2024-10-14  7:55 ` [PATCH v9 11/26] drm/xe/devcoredump: " Himal Prasad Ghimiray
2024-10-15 14:26   ` Nirmoy Das
2024-10-15 16:06   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 12/26] drm/xe/tests/mocs: Update xe_force_wake_get() return handling Himal Prasad Ghimiray
2024-10-15 14:47   ` Nirmoy Das
2024-10-15 16:08   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 13/26] drm/xe/mocs: Update handling of xe_force_wake_get return Himal Prasad Ghimiray
2024-10-15 15:09   ` Nirmoy Das
2024-10-15 17:59   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 14/26] drm/xe/xe_drm_client: " Himal Prasad Ghimiray
2024-10-15 15:17   ` Nirmoy Das
2024-10-15 18:00   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 15/26] drm/xe/xe_gt_debugfs: " Himal Prasad Ghimiray
2024-10-15 15:18   ` Nirmoy Das
2024-10-15 18:09   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 16/26] drm/xe/guc: " Himal Prasad Ghimiray
2024-10-15 15:20   ` Nirmoy Das
2024-10-15 18:32   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 17/26] drm/xe/huc: " Himal Prasad Ghimiray
2024-10-15 15:21   ` Nirmoy Das
2024-10-15 18:20   ` Nilawar, Badal
2024-10-15 18:42   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 18/26] drm/xe/oa: Handle force_wake_get failure in xe_oa_stream_init() Himal Prasad Ghimiray
2024-10-15 15:21   ` Nirmoy Das
2024-10-16 12:34   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 19/26] drm/xe/pat: Update handling of xe_force_wake_get return Himal Prasad Ghimiray
2024-10-15 15:28   ` Nirmoy Das
2024-10-16 12:35   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 20/26] drm/xe/gt_tlb_invalidation_ggtt: " Himal Prasad Ghimiray
2024-10-15 15:29   ` Nirmoy Das
2024-10-16 12:36   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 21/26] drm/xe/xe_reg_sr: " Himal Prasad Ghimiray
2024-10-15 15:30   ` Nirmoy Das
2024-10-16 12:38   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 22/26] drm/xe/query: " Himal Prasad Ghimiray
2024-10-15 15:31   ` Nirmoy Das
2024-10-16 12:40   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 23/26] drm/xe/vram: " Himal Prasad Ghimiray
2024-10-15 15:34   ` Nirmoy Das
2024-10-16 12:41   ` Nilawar, Badal
2024-10-14  7:55 ` [PATCH v9 24/26] drm/xe: forcewake debugfs open fails on xe_forcewake_get failure Himal Prasad Ghimiray
2024-10-15 16:02   ` Nilawar, Badal
2024-10-14  7:56 ` [PATCH v9 25/26] drm/xe: Ensure __must_check for xe_force_wake_get() return Himal Prasad Ghimiray
2024-10-14  8:57   ` Nirmoy Das
2024-10-14  7:56 ` [PATCH v9 26/26] drm/xe: Change return type to void for xe_force_wake_put Himal Prasad Ghimiray
2024-10-14  9:00   ` Nirmoy Das
2024-10-14  7:57 ` ✓ CI.Build: success for Fix xe_force_wake_get() failure handling (rev10) Patchwork
2024-10-14  7:59 ` ✓ CI.Hooks: " Patchwork
2024-10-14  8:01 ` ✓ CI.checksparse: " Patchwork
2024-10-14  8:27 ` ✓ CI.BAT: " Patchwork
2024-10-14  9:25 ` ✗ CI.FULL: failure " Patchwork
2024-10-17  5:40 ` ✓ CI.Patch_applied: success for Fix xe_force_wake_get() failure handling (rev11) Patchwork
2024-10-17  5:40 ` ✗ CI.checkpatch: warning " Patchwork
2024-10-17  5:42 ` ✓ CI.KUnit: success " Patchwork
2024-10-17  5:53 ` ✓ CI.Build: " Patchwork
2024-10-17  5:55 ` ✓ CI.Hooks: " Patchwork
2024-10-17  5:57 ` ✓ CI.checksparse: " Patchwork
2024-10-17  6:20 ` ✓ CI.BAT: " Patchwork
2024-10-17 15:42 ` ✗ CI.FULL: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox