Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Missing cancellation and synchronization of asynchronous GPU tasks 
after a signal or timeout leads to software race conditions, hardware state 
corruption, and GPU page faults.
--

commit 5c3d775056a5b2e3ebdba73f1d3b9858fe3716ba
Author: Adrián Larumbe <[email protected]>

drm/panfrost: Avoid cache flush after perfcnt sample in fully coherent systems

This commit avoids flushing the GPU caches after a perfcnt sample is made
available if the GPU and CPU are fully coherent.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c 
> b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index ffc77121070e2..b3f71d7fd82a9 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -52,7 +52,10 @@ void panfrost_perfcnt_clean_cache_done(struct 
> panfrost_device *pfdev)
>  
>  void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
>  {
> -     gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
> +     if (pfdev->features.selected_coherency != COHERENCY_ACE)
> +             gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
> +     else
> +             complete(&pfdev->perfcnt->dump_comp);
>  }
>  
>  static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)

[Severity: High]
This is a pre-existing issue, but does this interruptible wait in
panfrost_perfcnt_dump_locked() leave the GPU hardware in a busy state if it
aborts early?

When an unprivileged user issues the DRM_IOCTL_PANFROST_PERFCNT_DUMP ioctl,
the kernel waits for the sample to complete:

panfrost_perfcnt_dump_locked() {
    ...
    ret = wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp,
                                                    msecs_to_jiffies(1000));
    ...
}

If the user process is interrupted by a signal, this wait returns early,
leaving the GPU_CMD_PERFCNT_SAMPLE command executing asynchronously on the
hardware.

If the syscall is restarted by the kernel, the driver re-issues the
GPU_CMD_PERFCNT_SAMPLE command:

panfrost_perfcnt_dump_locked() {
    ...
    reinit_completion(&pfdev->perfcnt->dump_comp);
    ...
    gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
    ...
}

This issues a new command while the previous one is still in-flight, which
can corrupt the hardware state machine.

Alternatively, if the process exits instead,
panfrost_perfcnt_disable_locked() unmaps the GPU memory without waiting for
the in-flight sample to finish:

panfrost_perfcnt_disable_locked() {
    ...
    drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);
    perfcnt->buf = NULL;
    panfrost_gem_close(&perfcnt->mapping->obj->base.base, file_priv);
    ...
}

When the GPU finally writes the sample data, it hits unmapped memory,
causing a GPU page fault and forcing a GPU hard reset. Could proper
cancellation and synchronization of these asynchronous GPU tasks be added
after a signal or timeout?

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=13

Reply via email to