| 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: <lgdrzylx2pf3t3ive7tcmuqlqu2vpjuvf5ztnyuoyrb6onecgh@6vieyyz5jzoj>
Date: Mon, 27 Jan 2025 12:46:27 +0000
From: Adrián Larumbe <adrian.larumbe@...labora.com>
To: Lukas Zapolskas <lukas.zapolskas@....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,
Mihail Atanassov <mihail.atanassov@....com>, nd@....com
Subject: Re: [RFC v2 3/8] drm/panthor: Add panthor_perf_init and
panthor_perf_unplug
On 11.12.2024 16:50, 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 | 7 +++
> drivers/gpu/drm/panthor/panthor_device.h | 5 +-
> drivers/gpu/drm/panthor/panthor_perf.c | 77 ++++++++++++++++++++++++
> drivers/gpu/drm/panthor/panthor_perf.h | 3 +
> 4 files changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 00f7b8ce935a..1a81a436143b 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -19,6 +19,7 @@
> #include "panthor_fw.h"
> #include "panthor_gpu.h"
> #include "panthor_mmu.h"
> +#include "panthor_perf.h"
> #include "panthor_regs.h"
> #include "panthor_sched.h"
>
> @@ -97,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);
> @@ -262,6 +264,10 @@ int panthor_device_init(struct panthor_device *ptdev)
> if (ret)
> goto err_unplug_fw;
>
> + ret = panthor_perf_init(ptdev);
> + if (ret)
> + goto err_unplug_fw;
> +
> /* ~3 frames */
> pm_runtime_set_autosuspend_delay(ptdev->base.dev, 50);
> pm_runtime_use_autosuspend(ptdev->base.dev);
> @@ -275,6 +281,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 636542c1dcbd..aca33d03036c 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -26,7 +26,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;
>
> @@ -137,6 +137,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 0e3d769c1805..e0dc6c4b0cf1 100644
> --- a/drivers/gpu/drm/panthor/panthor_perf.c
> +++ b/drivers/gpu/drm/panthor/panthor_perf.c
> @@ -13,6 +13,24 @@
> #include "panthor_perf.h"
> #include "panthor_regs.h"
>
> +struct panthor_perf {
> + /**
> + * @block_set: The global counter set configured onto the HW.
> + */
> + u8 block_set;
I think this field is not used in any further patches. Only in the sampler
struct definition later on you include the same field and assign it from
the ioctl setup arguments.
> + /** @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;
> +};
> +
> /**
> * PANTHOR_PERF_COUNTERS_PER_BLOCK - On CSF architectures pre-11.x, the number of counters
> * per block was hardcoded to be 64. Arch 11.0 onwards supports the PRFCNT_FEATURES GPU register,
> @@ -45,3 +63,62 @@ void panthor_perf_info_init(struct panthor_device *ptdev)
> perf_info->shader_blocks = hweight64(ptdev->gpu_info.shader_present);
> }
>
> +/**
> + * 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;
> +
> + if (!ptdev)
> + return -EINVAL;
> +
> + perf = devm_kzalloc(ptdev->base.dev, sizeof(*perf), GFP_KERNEL);
> + if (ZERO_OR_NULL_PTR(perf))
> + return -ENOMEM;
> +
> + xa_init_flags(&perf->sessions, XA_FLAGS_ALLOC);
> +
> + /* Currently, we only support a single session at a time. */
> + perf->session_range = (struct xa_limit) {
> + .min = 0,
> + .max = 1,
> + };
I guess at the moment we only allow a single session because periodic sampling
isn't yet implemented. Does that mean multisession support will not be made
available for manual samplers in the future?
> +
> + drm_info(&ptdev->base, "Performance counter subsystem initialized");
> +
> + ptdev->perf = perf;
> +
> + return 0;
> +}
> +
> +/**
> + * 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!");
> +
> + xa_destroy(&perf->sessions);
> +
> + devm_kfree(ptdev->base.dev, ptdev->perf);
If we always call devm_kfree, then what is the point of allocating ptdev->perf
with devm_kzalloc?
> + ptdev->perf = NULL;
> +}
> diff --git a/drivers/gpu/drm/panthor/panthor_perf.h b/drivers/gpu/drm/panthor/panthor_perf.h
> index cff537a370c9..90af8b18358c 100644
> --- a/drivers/gpu/drm/panthor/panthor_perf.h
> +++ b/drivers/gpu/drm/panthor/panthor_perf.h
> @@ -9,4 +9,7 @@ struct panthor_device;
>
> void panthor_perf_info_init(struct panthor_device *ptdev);
>
> +int panthor_perf_init(struct panthor_device *ptdev);
> +void panthor_perf_unplug(struct panthor_device *ptdev);
> +
> #endif /* __PANTHOR_PERF_H__ */
> --
> 2.25.1
Adrian Larumbe
Powered by blists - more mailing lists