| 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
| ||
|
Message-ID: <btrbmoivzhslvirfj5ourlmtr5mah4brrzbyhibneb7j7bdqhk@qclukqco4nza>
Date: Mon, 29 Sep 2025 10:29:31 +1000
From: Alistair Popple <apopple@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Lyude Paul <lyude@...hat.com>, rust-for-linux@...r.kernel.org,
dri-devel@...ts.freedesktop.org, dakr@...nel.org, 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
Subject: Re: [PATCH v2 03/10] gpu: nova-core: gsp: Create wpr metadata
On 2025-09-26 at 11:24 +1000, Alexandre Courbot <acourbot@...dia.com> wrote...
> On Thu Sep 25, 2025 at 5:24 AM JST, Lyude Paul wrote:
> > On Mon, 2025-09-22 at 21:30 +1000, Alistair Popple wrote:
> >> The GSP requires some pieces of metadata to boot. These are passed in a
> >> struct which the GSP transfers via DMA. Create this struct and get a
> >> handle to it for future use when booting the GSP.
> >>
> >> Signed-off-by: Alistair Popple <apopple@...dia.com>
> >>
> >> ---
> >>
> >> Changes for v2:
> >> - Rebased on Alex's latest version
> >> ---
> >> drivers/gpu/nova-core/fb.rs | 1 -
> >> drivers/gpu/nova-core/firmware/gsp.rs | 3 +-
> >> drivers/gpu/nova-core/firmware/riscv.rs | 6 +-
> >> drivers/gpu/nova-core/gsp.rs | 1 +
> >> drivers/gpu/nova-core/gsp/boot.rs | 7 +++
> >> drivers/gpu/nova-core/gsp/fw.rs | 63 ++++++++++++++++++-
> >> .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 2 +
> >> 7 files changed, 75 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> >> index 4d6a1f452183..5580498ba2fb 100644
> >> --- a/drivers/gpu/nova-core/fb.rs
> >> +++ b/drivers/gpu/nova-core/fb.rs
> >> @@ -87,7 +87,6 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
> >> ///
> >> /// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
> >> #[derive(Debug)]
> >> -#[expect(dead_code)]
> >> pub(crate) struct FbLayout {
> >> /// Range of the framebuffer. Starts at `0`.
> >> pub(crate) fb: Range<u64>,
> >> diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
> >> index 9654810834d9..67b85e1db27d 100644
> >> --- a/drivers/gpu/nova-core/firmware/gsp.rs
> >> +++ b/drivers/gpu/nova-core/firmware/gsp.rs
> >> @@ -127,7 +127,7 @@ pub(crate) struct GspFirmware {
> >> /// Size in bytes of the firmware contained in [`Self::fw`].
> >> pub size: usize,
> >> /// Device-mapped GSP signatures matching the GPU's [`Chipset`].
> >> - signatures: DmaObject,
> >> + pub signatures: DmaObject,
> >> /// GSP bootloader, verifies the GSP firmware before loading and running it.
> >> pub bootloader: RiscvFirmware,
> >> }
> >> @@ -212,7 +212,6 @@ pub(crate) fn new<'a, 'b>(
> >> }))
> >> }
> >>
> >> - #[expect(unused)]
> >> /// Returns the DMA handle of the radix3 level 0 page table.
> >> pub(crate) fn radix3_dma_handle(&self) -> DmaAddress {
> >> self.level0.dma_handle()
> >> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
> >> index b90acfc81e78..dec33d2b631a 100644
> >> --- a/drivers/gpu/nova-core/firmware/riscv.rs
> >> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
> >> @@ -53,11 +53,11 @@ fn new(bin_fw: &BinFirmware<'_>) -> Result<Self> {
> >> #[expect(unused)]
> >> pub(crate) struct RiscvFirmware {
> >> /// Offset at which the code starts in the firmware image.
> >> - code_offset: u32,
> >> + pub code_offset: u32,
> >> /// Offset at which the data starts in the firmware image.
> >> - data_offset: u32,
> >> + pub data_offset: u32,
> >> /// Offset at which the manifest starts in the firmware image.
> >> - manifest_offset: u32,
> >> + pub manifest_offset: u32,
> >> /// Application version.
> >> app_version: u32,
> >> /// Device-mapped firmware image.
> >> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> >> index 0185f66971ff..2daa46f2a514 100644
> >> --- a/drivers/gpu/nova-core/gsp.rs
> >> +++ b/drivers/gpu/nova-core/gsp.rs
> >> @@ -13,6 +13,7 @@
> >> use kernel::ptr::Alignment;
> >> use kernel::transmute::{AsBytes, FromBytes};
> >>
> >> +use crate::fb::FbLayout;
> >> use fw::LibosMemoryRegionInitArgument;
> >>
> >> pub(crate) const GSP_PAGE_SHIFT: usize = 12;
> >> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> >> index fb22508128c4..1d2448331d7a 100644
> >> --- a/drivers/gpu/nova-core/gsp/boot.rs
> >> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> >> @@ -1,6 +1,8 @@
> >> // SPDX-License-Identifier: GPL-2.0
> >>
> >> use kernel::device;
> >> +use kernel::dma::CoherentAllocation;
> >> +use kernel::dma_write;
> >> use kernel::pci;
> >> use kernel::prelude::*;
> >>
> >> @@ -14,6 +16,7 @@
> >> FIRMWARE_VERSION,
> >> };
> >> use crate::gpu::Chipset;
> >> +use crate::gsp::GspFwWprMeta;
> >> use crate::regs;
> >> use crate::vbios::Vbios;
> >>
> >> @@ -132,6 +135,10 @@ pub(crate) fn boot(
> >> bar,
> >> )?;
> >>
> >> + let wpr_meta =
> >> + CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
> >> + dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
> >
> > Not something I think we need to block this series on, but this line does make
> > me wonder if we should have a variant of dma_write!() that uses
> > CoherentAllocation::write(), since I think that would actually be faster then
> > calling dma_write!() here.
>
> Can you elaborate a bit on this idea? Would it be faster because it uses
> a non-volatile write in this case?
>
> On a related note, I wish we could make all these accesses to
> single-instance coherent allocations non-fallible, as this is a pattern
> we use often in Nova and the only thing that can fail is
> `item_from_index`, which we know at build-time is valid as we are
> accessing the first element.
>
> So if we enforced a rule that `count` must be >= 0 in
> `CoherentAllocation::alloc_attrs` (which is not currently enforced but
> would make sense imho), we could maybe add a new variant to
> `dma_read/write` that matches a non-indexed expression, and makes a
> non-fallible access to the first element of the allocation? How does
> that sound?
Would this have to be limited to the first element though? I assume we could
make a CoherentAllocation variant where the number of elements is a compile time
constant and therefore dma_read/write on those would be infallible except at
build time.
> Or we could also introduce a new type for single-instance allocations if
> that makes more sense.
Powered by blists - more mailing lists