[PATCH v2] drm/xe/vsec: Allow for kernel context access

[Ruhl, Michael J]

>-----Original Message-----
>From: De Marchi, Lucas <lucas.demarchi at intel.com>
>Sent: Wednesday, May 7, 2025 3:46 PM
>To: Ruhl, Michael J <michael.j.ruhl at intel.com>
>Cc: intel-xe at lists.freedesktop.org; Johnstone, Nicholas
><nicholas.johnstone at intel.com>; Box, David E <david.e.box at intel.com>; Vivi,
>Rodrigo <rodrigo.vivi at intel.com>
>Subject: Re: [PATCH v2] drm/xe/vsec: Allow for kernel context access
>
>On Wed, May 07, 2025 at 03:37:40PM -0400, Michael J. Ruhl wrote:
>>The PMT driver API allows kernel level access to device
>
>ahn? What's that supposed to mean? what's the context that is calling
>this ?

Hi Lucas,

The PMT exports the pmt_telem_read_mmio() function which accesses HW registers
for telemetry information.

In this use case the SOCwatch kernel driver is accessing the API trying to read telemetry
information.

Since is happening from kernel level rather than user level, the mutex throws the
"might sleep" context error.

I tried to distill this the above comment...  Does adding the content above explain this
enough, or should I be more expansive (what details do you need?)?

(I unfortunately don't have the trace any more...)

Thanks,

Mike

>>data.  Because of this, the mutex lock use by the Xe copy
>>callback function turns out to be a poor choice.
>>
>>Change the mutex to a spinlock to avoid sleeping when it
>>isn't allowed.
>>
>>Fixes: 0c45e76fcc62 ("drm/xe/vsec: Support BMG devices")
>
>need to be explicit what this is fixing.
>
>Lucas De Marchi
>
>
>>Signed-off-by: Michael J. Ruhl <michael.j.ruhl at intel.com>
>>---
>>v2: checkpatch fix, update email address
>>
>> drivers/gpu/drm/xe/xe_device.c       | 5 ++---
>> drivers/gpu/drm/xe/xe_device_types.h | 2 +-
>> drivers/gpu/drm/xe/xe_vsec.c         | 4 ++--
>> 3 files changed, 5 insertions(+), 6 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>>index 75e753e0a682..72c81b8203a5 100644
>>--- a/drivers/gpu/drm/xe/xe_device.c
>>+++ b/drivers/gpu/drm/xe/xe_device.c
>>@@ -8,6 +8,7 @@
>> #include <linux/aperture.h>
>> #include <linux/delay.h>
>> #include <linux/fault-inject.h>
>>+#include <linux/spinlock.h>
>> #include <linux/units.h>
>>
>> #include <drm/drm_atomic_helper.h>
>>@@ -488,9 +489,7 @@ struct xe_device *xe_device_create(struct pci_dev
>*pdev,
>> 		goto err;
>> 	}
>>
>>-	err = drmm_mutex_init(&xe->drm, &xe->pmt.lock);
>>-	if (err)
>>-		goto err;
>>+	spin_lock_init(&xe->pmt.lock);
>>
>> 	err = xe_display_create(xe);
>> 	if (WARN_ON(err))
>>diff --git a/drivers/gpu/drm/xe/xe_device_types.h
>b/drivers/gpu/drm/xe/xe_device_types.h
>>index 06c65dace026..b57e1ba7cbdf 100644
>>--- a/drivers/gpu/drm/xe/xe_device_types.h
>>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>>@@ -527,7 +527,7 @@ struct xe_device {
>> 	/** @pmt: Support the PMT driver callback interface */
>> 	struct {
>> 		/** @pmt.lock: protect access for telemetry data */
>>-		struct mutex lock;
>>+		spinlock_t lock;
>> 	} pmt;
>>
>> 	/**
>>diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
>>index b378848d3b7b..df44e28abbdf 100644
>>--- a/drivers/gpu/drm/xe/xe_vsec.c
>>+++ b/drivers/gpu/drm/xe/xe_vsec.c
>>@@ -6,7 +6,7 @@
>> #include <linux/errno.h>
>> #include <linux/intel_vsec.h>
>> #include <linux/module.h>
>>-#include <linux/mutex.h>
>>+#include <linux/spinlock.h>
>> #include <linux/pci.h>
>> #include <linux/types.h>
>>
>>@@ -164,7 +164,7 @@ static int xe_pmt_telem_read(struct pci_dev *pdev,
>u32 guid, u64 *data, loff_t u
>>
>> 	telem_addr += offset + user_offset;
>>
>>-	guard(mutex)(&xe->pmt.lock);
>>+	guard(spinlock)(&xe->pmt.lock);
>>
>> 	/* indicate that we are not at an appropriate power level */
>> 	if (!xe_pm_runtime_get_if_active(xe))
>>--
>>2.49.0
>>