| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <569a368a-b8ae-4c30-a9b8-038b7b97e0e6@arm.com>
Date: Mon, 21 Jul 2025 10:10:26 +0100
From: Lukas Zapolskas <lukas.zapolskas@....com>
To: Adrián Larumbe <adrian.larumbe@...labora.com>
Cc: Boris Brezillon <boris.brezillon@...labora.com>,
Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/7] drm/panthor: Add panthor perf initialization and
termination
On 18/07/2025 04:10, Adrián Larumbe wrote:
> On 16.05.2025 16:49, Lukas Zapolskas wrote:
>> Added the panthor_perf system initialization and unplug code to allow
>> for the handling of userspace sessions to be added in follow-up
>> patches.
>>
>> Signed-off-by: Lukas Zapolskas <lukas.zapolskas@....com>
>> ---
>> drivers/gpu/drm/panthor/panthor_device.c | 2 +
>> drivers/gpu/drm/panthor/panthor_device.h | 5 +-
>> drivers/gpu/drm/panthor/panthor_perf.c | 62 +++++++++++++++++++++++-
>> drivers/gpu/drm/panthor/panthor_perf.h | 1 +
>> 4 files changed, 68 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
>> index 76b4cf3dc391..7ac985d44655 100644
>> --- a/drivers/gpu/drm/panthor/panthor_device.c
>> +++ b/drivers/gpu/drm/panthor/panthor_device.c
>> @@ -98,6 +98,7 @@ void panthor_device_unplug(struct panthor_device *ptdev)
>> /* Now, try to cleanly shutdown the GPU before the device resources
>> * get reclaimed.
>> */
>> + panthor_perf_unplug(ptdev);
>> panthor_sched_unplug(ptdev);
>> panthor_fw_unplug(ptdev);
>> panthor_mmu_unplug(ptdev);
>> @@ -277,6 +278,7 @@ int panthor_device_init(struct panthor_device *ptdev)
>>
>> err_disable_autosuspend:
>> pm_runtime_dont_use_autosuspend(ptdev->base.dev);
>> + panthor_perf_unplug(ptdev);
>> panthor_sched_unplug(ptdev);
>>
>> err_unplug_fw:
>> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
>> index 657ccc39568c..818c4d96d448 100644
>> --- a/drivers/gpu/drm/panthor/panthor_device.h
>> +++ b/drivers/gpu/drm/panthor/panthor_device.h
>> @@ -27,7 +27,7 @@ struct panthor_heap_pool;
>> struct panthor_job;
>> struct panthor_mmu;
>> struct panthor_fw;
>> -struct panthor_perfcnt;
>> +struct panthor_perf;
>> struct panthor_vm;
>> struct panthor_vm_pool;
>>
>> @@ -138,6 +138,9 @@ struct panthor_device {
>> /** @devfreq: Device frequency scaling management data. */
>> struct panthor_devfreq *devfreq;
>>
>> + /** @perf: Performance counter management data. */
>> + struct panthor_perf *perf;
>> +
>> /** @unplug: Device unplug related fields. */
>> struct {
>> /** @lock: Lock used to serialize unplug operations. */
>> diff --git a/drivers/gpu/drm/panthor/panthor_perf.c b/drivers/gpu/drm/panthor/panthor_perf.c
>> index 66e9a197ac1f..9365ce9fed04 100644
>> --- a/drivers/gpu/drm/panthor/panthor_perf.c
>> +++ b/drivers/gpu/drm/panthor/panthor_perf.c
>> @@ -9,6 +9,19 @@
>> #include "panthor_fw.h"
>> #include "panthor_perf.h"
>
> You must include "panthor_regs.h" here or else GPU_MEM_FEATURES_L2_SLICES() won't be available.
> However, it seems this is something that should be done in the previous patch.
>
Will add that to the perf_info patch.
>>
>> +struct panthor_perf {
>> + /** @next_session: The ID of the next session. */
>> + u32 next_session;
>> +
>> + /** @session_range: The number of sessions supported at a time. */
>> + struct xa_limit session_range;
>> +
>> + /**
>> + * @sessions: Global map of sessions, accessed by their ID.
>> + */
>> + struct xarray sessions;
>> +};
>> +
>> struct panthor_perf_counter_block {
>> struct drm_panthor_perf_block_header header;
>> u64 counters[];
>> @@ -63,14 +76,61 @@ static void panthor_perf_info_init(struct panthor_device *ptdev)
>> * panthor_perf_init - Initialize the performance counter subsystem.
>> * @ptdev: Panthor device
>> *
>> + * The performance counters require the FW interface to be available to setup the
>> + * sampling ringbuffers, so this must be called only after FW is initialized.
>> + *
>> * Return: 0 on success, negative error code on failure.
>> */
>> int panthor_perf_init(struct panthor_device *ptdev)
>> {
>> + struct panthor_perf *perf __free(kfree) = NULL;
>> + int ret = 0;
>> +
>> if (!ptdev)
>> return -EINVAL;
>>
>> panthor_perf_info_init(ptdev);
>>
>> - return 0;
>> + perf = kzalloc(sizeof(*perf), GFP_KERNEL);
>> + if (ZERO_OR_NULL_PTR(perf))
>> + return -ENOMEM;
>> +
>> + xa_init_flags(&perf->sessions, XA_FLAGS_ALLOC);
>> +
>> + perf->session_range = (struct xa_limit) {
>> + .min = 0,
>> + .max = 1,
>> + };
>> +
>> + drm_info(&ptdev->base, "Performance counter subsystem initialized");
>> +
>> + ptdev->perf = no_free_ptr(perf);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * panthor_perf_unplug - Terminate the performance counter subsystem.
>> + * @ptdev: Panthor device.
>> + *
>> + * This function will terminate the performance counter control structures and any remaining
>> + * sessions, after waiting for any pending interrupts.
>> + */
>> +void panthor_perf_unplug(struct panthor_device *ptdev)
>> +{
>> + struct panthor_perf *perf = ptdev->perf;
>> +
>> + if (!perf)
>> + return;
>> +
>> + if (!xa_empty(&perf->sessions)) {
>> + drm_err(&ptdev->base,
>> + "Performance counter sessions active when unplugging the driver!");
>> + }
>
> I think this could only happen if someone forces module unload, even
> though there might still be processes which haven't yet closed the DRM
> file?
>
That sounds about right. The only time I have seen that warning was in development when
the session cleanup was not being done properly on process termination.
>> +
>> + xa_destroy(&perf->sessions);
>> +
>> + kfree(ptdev->perf);
>> +
>> + ptdev->perf = NULL;
>> }
>> diff --git a/drivers/gpu/drm/panthor/panthor_perf.h b/drivers/gpu/drm/panthor/panthor_perf.h
>> index 3c32c24c164c..e4805727b9e7 100644
>> --- a/drivers/gpu/drm/panthor/panthor_perf.h
>> +++ b/drivers/gpu/drm/panthor/panthor_perf.h
>> @@ -10,6 +10,7 @@
>> struct panthor_device;
>>
>> int panthor_perf_init(struct panthor_device *ptdev);
>> +void panthor_perf_unplug(struct panthor_device *ptdev);
>>
>> #endif /* __PANTHOR_PERF_H__ */
>>
>> --
>> 2.33.0.dirty
>
> Adrian Larumbe
Powered by blists - more mailing lists