linux-kernel - Re: [PATCH v2 1/2] gpu: nova-core: add initial driver stub

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] [day] [month] [year] [list]

Message-Id: <D7LTY1FIUOTR.3E6MZ8BD0HW7E@nvidia.com>
Date: Fri, 07 Feb 2025 10:41:37 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Danilo Krummrich" <dakr@...nel.org>
Cc: "airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch"
 <simona@...ll.ch>, "corbet@....net" <corbet@....net>,
 "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
 "mripard@...nel.org" <mripard@...nel.org>, "tzimmermann@...e.de"
 <tzimmermann@...e.de>, "ajanulgu@...hat.com" <ajanulgu@...hat.com>,
 "lyude@...hat.com" <lyude@...hat.com>, "pstanner@...hat.com"
 <pstanner@...hat.com>, "Zhi Wang" <zhiw@...dia.com>, "Neo Jia"
 <cjia@...dia.com>, "John Hubbard" <jhubbard@...dia.com>, "Ben Skeggs"
 <bskeggs@...dia.com>, "Andy Currid" <ACurrid@...dia.com>,
 "ojeda@...nel.org" <ojeda@...nel.org>, "alex.gaynor@...il.com"
 <alex.gaynor@...il.com>, "boqun.feng@...il.com" <boqun.feng@...il.com>,
 "gary@...yguo.net" <gary@...yguo.net>, "bjorn3_gh@...tonmail.com"
 <bjorn3_gh@...tonmail.com>, "benno.lossin@...ton.me"
 <benno.lossin@...ton.me>, "a.hindborg@...nel.org" <a.hindborg@...nel.org>,
 "aliceryhl@...gle.com" <aliceryhl@...gle.com>, "tmgross@...ch.edu"
 <tmgross@...ch.edu>, "dri-devel@...ts.freedesktop.org"
 <dri-devel@...ts.freedesktop.org>, "linux-doc@...r.kernel.org"
 <linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "nouveau@...ts.freedesktop.org"
 <nouveau@...ts.freedesktop.org>, "rust-for-linux@...r.kernel.org"
 <rust-for-linux@...r.kernel.org>, "Nouveau"
 <nouveau-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH v2 1/2] gpu: nova-core: add initial driver stub

Hi Danilo,

On Thu Feb 6, 2025 at 11:49 PM JST, Danilo Krummrich wrote:
>> > +impl Spec {
>> > +    fn new(bar: &Devres<Bar0>) -> Result<Spec> {
>> > +        let bar = bar.try_access().ok_or(ENXIO)?;
>> > +        let boot0 = regs::Boot0::read(&bar);
>> > +
>> > +        let Some(chipset) = Chipset::from_u32(boot0.chipset()) else {
>> > +            return Err(ENODEV);
>> > +        };
>> > +
>> > +        let Some(arch) = Architecture::from_u32(boot0.arch()) else {
>> > +            return Err(ENODEV);
>> > +        };
>>
>> Technically the Architecture is already known if the Chipset has been
>> built successfully, so there should be no need to build it again (and
>> test for a failure that cannot happen at this point).
>>
>> Since the architecture information is already embedded in Chipset, maybe
>> we can have an arch() method there?
>>
>> Something like:
>>
>>     impl Chipset {
>>         pub(crate) fn arch(self) -> Architecture {
>>             match self as u32 & !0xf {
>>                 0x160 => Architecture::Turing,
>>                 0x170 => Architecture::Ampere,
>>                 0x190 => Architecture::Ada,
>>                 _ => unreachable!(),
>>             }
>>         }
>>     }
>
> I thought about this, which is also why the comment above says: "consider to
> store within Chipset, if arbitrary_enum_discriminant becomes stable".
>
> I did not go with what you suggest because it leaves us with either
> Chipset::arch() returning a Result, which is annoying, or with Chipset::arch()
> being able to panic the kernel, which I'd dislike even more.
>
> There's also a third option, which would be to have some kind of unknown
> architecture, which we could catch later on, but that's just a worse variation
> of returning a Result.
>
> Another reason was that I did not want to encode register specific masks into
> the Chipset type.

Agreed, none of these solutions are completely satisfying. From the
point that we have successfully built a Chipset, we should be able to
get its architecture without any runtime error or failure path. So I
guess this leaves matching all the variants. It's a bit verbose, but
hopefully later we can gather all the static information about chipsets
in a single place, and generate these implementations (including a
Display implementation - ideally the debug one would also print the hex
representation of the chipset to be more useful for troubleshooting)
using macros.

Cheers,
Alex.