> On 31 Jul 2026, at 09:59, Alice Ryhl <[email protected]> wrote:
>
> On Fri, Jul 31, 2026 at 01:05:46AM +0800, Alvin Sun wrote:
>> Add a gpuvas debugfs file listing all GPU VAs for the Tyr DRM driver.
>> Collects VMs into a shared list during firmware init and renders them
>> via dump_gpuva_info on read.
>>
>> Signed-off-by: Alvin Sun <[email protected]>
>> ---
>> drivers/gpu/drm/tyr/debugfs.rs | 65
>> ++++++++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/tyr/driver.rs | 16 +++++++++++
>> drivers/gpu/drm/tyr/fw.rs | 8 ++++++
>> drivers/gpu/drm/tyr/tyr.rs | 1 +
>> drivers/gpu/drm/tyr/vm.rs | 5 ++++
>> 5 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tyr/debugfs.rs b/drivers/gpu/drm/tyr/debugfs.rs
>> new file mode 100644
>> index 0000000000000..d381b1901bd08
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tyr/debugfs.rs
>> @@ -0,0 +1,65 @@
>> +// SPDX-License-Identifier: GPL-2.0 or MIT
>> +
>> +//! Debugfs support for the Tyr DRM driver.
>> +
>> +use kernel::{
>> + alloc::KVec,
>> + drm,
>> + new_mutex,
>> + prelude::*,
>> + seq_file,
>> + sync::{
>> + Arc,
>> + Mutex, //
>> + }, //
>> +};
>> +
>> +use crate::{
>> + driver::TyrDrmDriver,
>> + vm::Vm, //
>> +};
>> +
>> +/// Registry of VMs for debugfs access.
>> +#[pin_data]
>> +pub(crate) struct VmRegistry<'drm> {
>> + #[pin]
>> + vms: Mutex<KVec<Arc<Vm<'drm>>>>,
>> +}
>> +
>> +impl<'drm> VmRegistry<'drm> {
>> + pub(crate) fn new() -> impl PinInit<Self> {
>> + pin_init!(Self { vms <- new_mutex!(KVec::new()) })
>> + }
>> +
>> + pub(crate) fn register(&self, vm: Arc<Vm<'drm>>) -> Result {
>> + Ok(self.vms.lock().push(vm, GFP_KERNEL)?)
>> + }
>> +
>> + fn for_each(&self, mut f: impl FnMut(&Vm<'drm>) -> Result) -> Result {
>> + for vm in self.vms.lock().iter() {
>> + f(vm)?;
>> + }
>> + Ok(())
>> + }
>> +}
>
> This code maintains a separate list of all of the vms for access from
> debugfs, but I would have expected that this is not necessary. If we
> already store the vms inside the driver's private data, then can't we
> just access them from the normal storage location?
Panthor also has a similar concept. I agree that we can reach the VMs
from other places, but they’re scattered, and this type buys the convenience
to do so, which we will need for other, non-debugfs-related features.
I wonder if we should move the registry to the top level in the private data,
though:
i.e.: {
fw: ..
mmu: …
registry: VmRegistry
}
>
> By having multiple copies of the same information, you risk that they
> get out of sync. In this case, you never remove vms from the list even
> if the vm stops being used, which seems wrong.
>
> Alice
I think this can be solved by having vm::new() take a &VmRegistry argument,
same for vm::destroy(). This forces callers to provide the list so it can be
kept
in sync.
This is just one approach though, perhaps someone has a better idea?
— Daniel
