linux-kernel - Re: [PATCH] Introduce Tyr

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250701111103.728ef3d7@fedora>
Date: Tue, 1 Jul 2025 11:11:03 +0200
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Daniel Almeida <daniel.almeida@...labora.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>, Miguel Ojeda <ojeda@...nel.org>, Alex
 Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, Andreas
 Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, Trevor
 Gross <tmgross@...ch.edu>, Daniel Stone <daniels@...labora.com>, Rob
 Herring <robh@...nel.org>, Alice Ryhl <alice.ryhl@...gle.com>, Beata
 Michalska <beata.michalska@....com>, Carsten Haitzler
 <carsten.haitzler@...s.arm.com>, Ashley Smith <ashley.smith@...labora.com>,
 linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 rust-for-linux@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH] Introduce Tyr

On Mon, 30 Jun 2025 18:12:02 +0200
Danilo Krummrich <dakr@...nel.org> wrote:

> On 6/30/25 6:06 PM, Boris Brezillon wrote:
> > On Sat, 28 Jun 2025 01:12:34 +0200
> > Danilo Krummrich <dakr@...nel.org> wrote:
> >   
> >>> +    pub(crate) fn log(&self, pdev: &platform::Device) {
> >>> +        let major = (self.gpu_id >> 16) & 0xff;
> >>> +        let minor = (self.gpu_id >> 8) & 0xff;
> >>> +        let status = self.gpu_id & 0xff;
> >>> +
> >>> +        let model_name = if let Some(model) = GPU_MODELS
> >>> +            .iter()
> >>> +            .find(|&f| f.major == major && f.minor == minor)
> >>> +        {
> >>> +            model.name
> >>> +        } else {
> >>> +            "unknown"
> >>> +        };
> >>> +
> >>> +        dev_info!(
> >>> +            pdev.as_ref(),
> >>> +            "mali-{} id 0x{:x} major 0x{:x} minor 0x{:x} status 0x{:x}",
> >>> +            model_name,
> >>> +            self.gpu_id >> 16,
> >>> +            major,
> >>> +            minor,
> >>> +            status
> >>> +        );
> >>> +
> >>> +        dev_info!(
> >>> +            pdev.as_ref(),
> >>> +            "Features: L2:{:#x} Tiler:{:#x} Mem:{:#x} MMU:{:#x} AS:{:#x}",
> >>> +            self.l2_features,
> >>> +            self.tiler_features,
> >>> +            self.mem_features,
> >>> +            self.mmu_features,
> >>> +            self.as_present
> >>> +        );
> >>> +
> >>> +        dev_info!(
> >>> +            pdev.as_ref(),
> >>> +            "shader_present=0x{:016x} l2_present=0x{:016x} tiler_present=0x{:016x}",
> >>> +            self.shader_present,
> >>> +            self.l2_present,
> >>> +            self.tiler_present
> >>> +        );
> >>> +
> >>> +        dev_info!(
> >>> +            pdev.as_ref(),
> >>> +            "PA bits: {}, VA bits: {}",
> >>> +            self.pa_bits(),
> >>> +            self.va_bits()
> >>> +        );
> >>> +    }  
> >>
> >> This is called from probe() and seems way too verbose for dev_info!(), please
> >> use dev_dbg!() instead.  
> > 
> > We do have the same level of verbosity in Panthor, and it's proven
> > useful when people are filling bug reports. Asking them to reload
> > the module with debug prints enabled is kinda annoying, and I don't
> > think I've heard anyone complaining that this was too verbose or slowing
> > down the boot, so I'd be tempted to keep it like that, and least for
> > the information printed in this function.  
> 
> Yeah, I think for the GPU revision bits that's reasonable, but do you really
> also need the other prints to be dev_info()? Don't you know this information
> from the combination of the GPU revision bits and the kernel version?

Sure, we could have a tool extracting most of that from the driver info
and DEV_QUERY ioctl(), but those info have been printed in Panfrost
since the early days. I picked those traces up in Panthor because devs
were used to it, and I honestly see no good reason to not print those as
dev_info() in Tyr too. What's your concern here? Is this about boot
time, not bloating the kernel logs or something else? I mean, we're
talking about less than 10 lines printed at boot/module-load-time.