diff mbox series

[1/6] accel/ivpu: Add missing locks around mmu queues

Message ID 20250204084622.2422544-2-jacek.lawrynowicz@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series accel/ivpu: Changes for 6.15 2025-02-04 | expand

Commit Message

Jacek Lawrynowicz Feb. 4, 2025, 8:46 a.m. UTC 
From: Andrzej Kacprowski <Andrzej.Kacprowski@intel.com>

Multiple threads were accessing mmu cmd queue simultaneously
causing sporadic failures in ivpu_mmu_cmdq_sync() function.
Protect critical code with mmu mutex.

Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Signed-off-by: Andrzej Kacprowski <Andrzej.Kacprowski@intel.com>
Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_mmu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Jeffrey Hugo Feb. 14, 2025, 4:41 p.m. UTC | #1 
On 2/4/2025 1:46 AM, Jacek Lawrynowicz wrote:
> From: Andrzej Kacprowski <Andrzej.Kacprowski@intel.com>
> 
> Multiple threads were accessing mmu cmd queue simultaneously
> causing sporadic failures in ivpu_mmu_cmdq_sync() function.
> Protect critical code with mmu mutex.

Describe a scenario in which this can occur? The two functions this 
patch modify cannot run concurrently from what I can tell.

-Jeff
Jacek Lawrynowicz Feb. 17, 2025, 3:26 p.m. UTC | #2 
Hi,

On 2/14/2025 5:41 PM, Jeffrey Hugo wrote:
> On 2/4/2025 1:46 AM, Jacek Lawrynowicz wrote:
>> From: Andrzej Kacprowski <Andrzej.Kacprowski@intel.com>
>>
>> Multiple threads were accessing mmu cmd queue simultaneously
>> causing sporadic failures in ivpu_mmu_cmdq_sync() function.
>> Protect critical code with mmu mutex.
> 
> Describe a scenario in which this can occur? The two functions this patch modify cannot run concurrently from what I can tell.

Functions from this diff are called in bottom IRQ handler when memory faults are detected.
The CMDQ is also accessed from IOCTLs when mapping/unmapping buffers in NPU MMU (ivpu_mmu_invalidate_tlb()).

Jacek
Jeffrey Hugo Feb. 18, 2025, 3:52 p.m. UTC | #3 
On 2/17/2025 8:26 AM, Jacek Lawrynowicz wrote:
> Hi,
> 
> On 2/14/2025 5:41 PM, Jeffrey Hugo wrote:
>> On 2/4/2025 1:46 AM, Jacek Lawrynowicz wrote:
>>> From: Andrzej Kacprowski <Andrzej.Kacprowski@intel.com>
>>>
>>> Multiple threads were accessing mmu cmd queue simultaneously
>>> causing sporadic failures in ivpu_mmu_cmdq_sync() function.
>>> Protect critical code with mmu mutex.
>>
>> Describe a scenario in which this can occur? The two functions this patch modify cannot run concurrently from what I can tell.
> 
> Functions from this diff are called in bottom IRQ handler when memory faults are detected.
> The CMDQ is also accessed from IOCTLs when mapping/unmapping buffers in NPU MMU (ivpu_mmu_invalidate_tlb()).

Ah.  Ok.  I think pointing that out in the commit text would be very 
helpful.

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
diff mbox series

Patch

diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
index b80bdded9fd79..5ea010568faa4 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -895,6 +895,9 @@  static int ivpu_mmu_evtq_disable(struct ivpu_device *vdev)
 
 void ivpu_mmu_discard_events(struct ivpu_device *vdev)
 {
+	struct ivpu_mmu_info *mmu = vdev->mmu;
+
+	mutex_lock(&mmu->lock);
 	/*
 	 * Disable event queue (stop MMU from updating the producer)
 	 * to allow synchronization of consumer and producer indexes
@@ -908,6 +911,8 @@  void ivpu_mmu_discard_events(struct ivpu_device *vdev)
 	ivpu_mmu_evtq_enable(vdev);
 
 	drm_WARN_ON_ONCE(&vdev->drm, vdev->mmu->evtq.cons != vdev->mmu->evtq.prod);
+
+	mutex_unlock(&mmu->lock);
 }
 
 int ivpu_mmu_disable_ssid_events(struct ivpu_device *vdev, u32 ssid)
@@ -920,6 +925,8 @@  int ivpu_mmu_disable_ssid_events(struct ivpu_device *vdev, u32 ssid)
 	if (ssid > IVPU_MMU_CDTAB_ENT_COUNT)
 		return -EINVAL;
 
+	mutex_lock(&mmu->lock);
+
 	entry = cdtab->base + (ssid * IVPU_MMU_CDTAB_ENT_SIZE);
 
 	val = READ_ONCE(entry[0]);
@@ -932,6 +939,8 @@  int ivpu_mmu_disable_ssid_events(struct ivpu_device *vdev, u32 ssid)
 	ivpu_mmu_cmdq_write_cfgi_all(vdev);
 	ivpu_mmu_cmdq_sync(vdev);
 
+	mutex_unlock(&mmu->lock);
+
 	return 0;
 }