linux-kernel - Re: [PATCH 05/10] gpu: nova-core: gsp: Add GSP command queue handling

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: <DCN6IC0DP3J4.3KR3OJEM0YCCF@nvidia.com>
Date: Mon, 08 Sep 2025 14:44:53 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Alexandre Courbot" <acourbot@...dia.com>, "Alistair Popple"
 <apopple@...dia.com>, <dri-devel@...ts.freedesktop.org>, <dakr@...nel.org>
Cc: "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>, "David Airlie" <airlied@...il.com>,
 "Simona Vetter" <simona@...ll.ch>, "Maarten Lankhorst"
 <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
 "Thomas Zimmermann" <tzimmermann@...e.de>, "John Hubbard"
 <jhubbard@...dia.com>, "Joel Fernandes" <joelagnelf@...dia.com>, "Timur
 Tabi" <ttabi@...dia.com>, <linux-kernel@...r.kernel.org>,
 <nouveau@...ts.freedesktop.org>, "Nouveau"
 <nouveau-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH 05/10] gpu: nova-core: gsp: Add GSP command queue
 handling

On Fri Sep 5, 2025 at 8:50 PM JST, Alexandre Courbot wrote:
>> +
>> +        Ok(GspCmdq {
>> +            dev: dev.into(),
>> +            msg_count: MSG_COUNT,
>> +            seq: 0,
>> +            gsp_mem,
>> +            _nr_ptes: nr_ptes as u32,
>> +        })
>> +    }
>> +
>> +    fn cpu_wptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].cpuq.tx.write_ptr).unwrap_unchecked() }
>> +    }
>> +
>> +    fn gsp_rptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].gspq.rx.read_ptr).unwrap_unchecked() }
>> +    }
>> +
>> +    fn cpu_rptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].cpuq.rx.read_ptr).unwrap_unchecked() }
>> +    }
>> +
>> +    fn gsp_wptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].gspq.tx.write_ptr).unwrap_unchecked() }
>> +    }
>
> Here is an easy trick to reduce the number of unsafe statements: have a
> method that returns a reference to the `gsp_mem` (which contains the
> unsafe part), and have these 4 methods call into it. And voilà, 3
> unsafes gone. :)

That sentence of mine is so wrong that I feel obligated to come back and
fix it before someone thinks this is an acceptable pattern. >_<

A method returning a reference to `gsp_mem` would still need to be
unsafe itself. Some areas of `gsp_mem` can be modified by the GSP at any
time, so it would be the responsibility of the caller to make sure that
it doesn't access any area that is currently under GSP ownership.

A better way to do this would be a have methods (safe, this time) that
provide references to the areas that are owned by the driver at the time
of calling.