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

[Lucas De Marchi]

On Mon, May 12, 2025 at 02:18:35PM -0400, Michael J. Ruhl wrote:
>The PMT driver API allows kernel level access to device
>data.  Because of this, the mutex lock use by the Xe copy

I already said twice this doesn't make any sense. Please stop sending
new versions if you can't justify it by showing the exact caller.
Here you'd actually introduce a bug with this patch.

>callback function turns out to be a poor choice.
>
>Since the context of the caller can not be known, change
>the mutex to a raw_spinlock to account for all use cases.

if you don't know the context you don't know what lock to use. There
isn't a magic lock that you should use for unknown contexts.

>
>Fixes: 0c45e76fcc62 ("drm/xe/vsec: Support BMG devices")
>Signed-off-by: Michael J. Ruhl <michael.j.ruhl at intel.com>
>---
>v2: checkpatch fix, update email address
>v3: use raw_spinlock
>
>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 399ae5f40321..0caa77d24f44 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>
>@@ -486,9 +487,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;
>+	raw_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..8f680a89ddb7 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;
>+		raw_spinlock_t lock;
> 	} pmt;
>
> 	/**
>diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
>index b378848d3b7b..3941e1760ea3 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(raw_spinlock)(&xe->pmt.lock);

you lock a raw_spinlock here,

>
> 	/* indicate that we are not at an appropriate power level */
> 	if (!xe_pm_runtime_get_if_active(xe))

then you check if it's active. That doesn't work. Down that code path it
does:

	spin_lock_irqsave(&dev->power.lock, flags);

If it ain't broken don't fix it. If it's broken show the breakage. The
fix may be in the caller or something in between.

Lucas De Marchi