Re: [PATCH v2 8/9] drm/tyr: add gpuvas debugfs file

> 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