On 2/13/24 18:10, Timur Tabi wrote:
On Tue, 2024-02-13 at 17:57 +0100, Danilo Krummrich wrote:
+ struct debugfs_blob_wrapper blob_init;
+ struct debugfs_blob_wrapper blob_intr;
+ struct debugfs_blob_wrapper blob_rm;
+ struct debugfs_blob_wrapper blob_pmu;
+ struct dentry *debugfs_logging_dir;
I think we should not create those from within the nvkm layer, but rather pass
them down through nvkm_device_pci_new().
Should they be created in nvkm_device_pci_new() also, even though we have no idea whether GSP is involved at that point?
We can pass some structure to nvkm_device_pci_new() where GSP sets the pointers to
the buffers and maybe a callback to clean them up. Once nvkm_device_pci_new() returns
we'd see if something has been set or not. If so, we can go ahead and create the
debugfs nodes.
Lifecycle wise I think we should ensure that removing the Nouveau kernel module
also cleans up those buffers. Even though keep-gsp-logging is considered unsafe,
we shouldn't leak memory.
I agree, but then there needs to be some way to keep these debugfs entries until the driver unloads. I don't know how to do that without creating some ugly global variables.
I fear that might be the only option. However, I still prefer a global list over a
memory leak.
For instance, can we allocate corresponding buffers in the driver layer, copy
things over and keep those buffers until nouveau_drm_exit()? This would also
avoid exposing those DMA buffers via debugfs.
The whole point behind this patch is to expose the buffers via debugfs. How else should they be exposed?
I think I only thought about the fail case where we could just copy over the data from
the DMA buffer to another buffer and expose that one instead. However, that doesn't
work if GSP loads successfully.
Hence, as mentioned above, we might just want to have some structure that we can add to
the global list that contains the pointers to the DMA buffers and maybe a callback
function to clean them up defined by the GSP code that we can simply call from
nouveau_drm_exit().
