Re: [PATCH v8 07/20] drm/xe/vsec: Correct locking order

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Sep 14, 2026 at 09:33:42PM +0000, Ruhl, Michael J wrote:
> 
> >-----Original Message-----
> >From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> >Sent: Monday, September 14, 2026 12:08 PM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >Cc: platform-driver-x86@vger.kernel.org; intel-xe@lists.freedesktop.org;
> >hansg@kernel.org; ilpo.jarvinen@linux.intel.com; Brost, Matthew
> ><matthew.brost@intel.com>; thomas.hellstrom@linux.intel.com;
> >airlied@gmail.com; simona@ffwll.ch; david.e.box@linux.intel.com; Vijay,
> >Anoop C <anoop.c.vijay@intel.com>; Nilawar, Badal
> ><badal.nilawar@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>;
> >Ausmus, James <james.ausmus@intel.com>; Poosa, Karthik
> ><karthik.poosa@intel.com>
> >Subject: Re: [PATCH v8 07/20] drm/xe/vsec: Correct locking order
> >
> >On Fri, Sep 11, 2026 at 01:11:55PM -0700, Michael J. Ruhl wrote:
> >> Xe locking order is power state first, and then local locks.
> >>
> >> Reorder locking to correct usage.
> >> Use explicit functions because of multiple locks.
> >>
> >> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_vsec.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> >> index 71bc359b2ad1..dd7723ba6c1c 100644
> >> --- a/drivers/gpu/drm/xe/xe_vsec.c
> >> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> >> @@ -158,16 +158,19 @@ int xe_pmt_telem_read(struct device *dev, u32
> >guid, u64 *data, loff_t user_offse
> >>
> >>  	telem_addr += offset + user_offset;
> >>
> >> -	guard(mutex)(&xe->pmt.lock);
> >
> >please use the scope_guard like others have already suggested.
> 
> I missed the fact that you could add the {} to the scoped guard...
> 
> So you would like to see this:
> 
> >> -
> >>  	/* indicate that we are not at an appropriate power level */
> >>  	if (!xe_pm_runtime_get_if_active(xe))
> >>  		return -ENODATA;
> >>
> >> +	mutex_lock(&xe->pmt.lock);
> >> +
> >>  	/* set SoC re-mapper index register based on GUID memory region */
> >>  	xe->soc_remapper.set_telem_region(xe, mem_region);
> >>
> >>  	memcpy_fromio(data, telem_addr, count);
> >> +
> >> +	mutex_unlock(&xe->pmt.lock);
> 
> 	scoped_guard(mutex)(&xe->pmt.lock) {
> 
>   		/* set SoC re-mapper index register based on GUID memory region */
>   		xe->soc_remapper.set_telem_region(xe, mem_region);
> 
> 	  	memcpy_fromio(data, telem_addr, count)
> 	}
> 
> ?

yes

> 
> Thanks,
> 
> M
> 
> >> +
> >>  	xe_pm_runtime_put(xe);
> >>
> >>  	return count;
> >> --
> >> 2.43.0
> >>