| 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: <npxngncbzvf6js4c3buteqedwmxyptffb2l6dvk6a2qwyr7zq7@oamzi7eatbh5>
Date: Fri, 17 Oct 2025 11:49:39 +1100
From: Alistair Popple <apopple@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: 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 v5 08/14] gpu: nova-core: gsp: Create rmargs
On 2025-10-16 at 17:24 +1100, Alexandre Courbot <acourbot@...dia.com> wrote...
> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
> > Initialise the GSP resource manager arguments (rmargs) which provide
> > initialisation parameters to the GSP firmware during boot. The rmargs
> > structure contains arguments to configure the GSP message/command queue
> > location.
> >
> > These are mapped for coherent DMA and added to the libos data structure
> > for access when booting GSP.
> >
> > Signed-off-by: Alistair Popple <apopple@...dia.com>
> >
> > ---
> >
> > Changes for v5:
> > - Derive Zeroable trait
> >
> > Changes for v2:
> > - Rebased on Alex's latest series
> > ---
> > drivers/gpu/nova-core/gsp.rs | 16 +++
> > drivers/gpu/nova-core/gsp/cmdq.rs | 24 +++-
> > drivers/gpu/nova-core/gsp/fw.rs | 60 ++++++++
> > .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 132 ------------------
>
> Mmm, looks like we are removing bindings. Can we not add them in the
> first place? :)
Bah. That's just bad rebasing - updating the bindings to add Zeroable to them
was as much of a pain as I thought it would be :-)
> > 4 files changed, 97 insertions(+), 135 deletions(-)
> >
> > diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> > index 1d472c5fad7a..58b595b8badd 100644
> > --- a/drivers/gpu/nova-core/gsp.rs
> > +++ b/drivers/gpu/nova-core/gsp.rs
> > @@ -19,6 +19,7 @@
> > mod fw;
> >
> > use fw::LibosMemoryRegionInitArgument;
> > +use fw::GspArgumentsCached;
> >
> > pub(crate) mod cmdq;
> >
> > @@ -36,6 +37,7 @@ pub(crate) struct Gsp {
> > logintr: LogBuffer,
> > logrm: LogBuffer,
> > pub(crate) cmdq: Cmdq,
> > + rmargs: CoherentAllocation<GspArgumentsCached>,
> > }
> >
> > #[repr(C)]
> > @@ -117,12 +119,26 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
> >
> > // Creates its own PTE array.
> > let cmdq = Cmdq::new(dev)?;
> > + let rmargs = CoherentAllocation::<GspArgumentsCached>::alloc_coherent(
>
> Let's add a space between the declaration of `cmdq` and `rmargs`.
Ok.
>
> > + dev,
> > + 1,
> > + GFP_KERNEL | __GFP_ZERO,
> > + )?;
> > + dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs)?)?;
> > +
> > + dma_write!(
> > + rmargs[0] = fw::GspArgumentsCached::new(
> > + fw::MessageQueueInitArguments::new(&cmdq),
> > + fw::GspSrInitArguments::new()
> > + )
> > + )?;
> >
> > Ok(try_pin_init!(Self {
> > libos,
> > loginit,
> > logintr,
> > logrm,
> > + rmargs,
> > cmdq,
> > }))
> > }
> > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> > index 3f8cb7a35922..da074a2ed0d9 100644
> > --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> > @@ -6,7 +6,7 @@
> >
> > use kernel::alloc::flags::GFP_KERNEL;
> > use kernel::device;
> > -use kernel::dma::CoherentAllocation;
> > +use kernel::dma::{CoherentAllocation, DmaAddress};
> > use kernel::dma_write;
> > use kernel::io::poll::read_poll_timeout;
> > use kernel::prelude::*;
> > @@ -247,10 +247,25 @@ pub(crate) struct Cmdq {
> > dev: ARef<device::Device>,
> > seq: u32,
> > gsp_mem: DmaGspMem,
> > - pub _nr_ptes: u32,
>
> We probably shouldn't have introduced this unused member in the first place.
Good point, it's a hangover from the implementations in previous versions.
>
> > }
> >
> > impl Cmdq {
> > + /// Offset of the data after the PTEs.
> > + const POST_PTE_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq);
> > +
> > + /// Offset of command queue ring buffer.
> > + pub(crate) const CMDQ_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq)
> > + + core::mem::offset_of!(Msgq, msgq)
> > + - Self::POST_PTE_OFFSET;
> > +
> > + /// Offset of message queue ring buffer.
> > + pub(crate) const STATQ_OFFSET: usize = core::mem::offset_of!(GspMem, gspq)
> > + + core::mem::offset_of!(Msgq, msgq)
> > + - Self::POST_PTE_OFFSET;
> > +
> > + /// Number of page table entries for the GSP shared region.
> > + pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
> > +
> > pub(crate) fn new(dev: &device::Device<device::Bound>) -> Result<Cmdq> {
> > let gsp_mem = DmaGspMem::new(dev)?;
> > let nr_ptes = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
> > @@ -260,7 +275,6 @@ pub(crate) fn new(dev: &device::Device<device::Bound>) -> Result<Cmdq> {
> > dev: dev.into(),
> > seq: 0,
> > gsp_mem,
> > - _nr_ptes: nr_ptes as u32,
> > })
> > }
> >
> > @@ -490,4 +504,8 @@ pub(crate) fn receive_msg_from_gsp<M: MessageFromGsp, R>(
> > .advance_cpu_read_ptr(msg_header.length().div_ceil(GSP_PAGE_SIZE as u32));
> > result
> > }
> > +
> > + pub(crate) fn dma_handle(&self) -> DmaAddress {
> > + self.gsp_mem.0.dma_handle()
> > + }
> > }
> > diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> > index a2ce570ddfaf..70abda1c2af8 100644
> > --- a/drivers/gpu/nova-core/gsp/fw.rs
> > +++ b/drivers/gpu/nova-core/gsp/fw.rs
> > @@ -16,6 +16,7 @@
> >
> > use crate::firmware::gsp::GspFirmware;
> > use crate::gpu::Chipset;
> > +use crate::gsp::cmdq::Cmdq;
> > use crate::gsp::FbLayout;
> > use crate::gsp::GSP_PAGE_SIZE;
> >
> > @@ -483,3 +484,62 @@ unsafe impl AsBytes for GspMsgElement {}
> > // SAFETY: This struct only contains integer types for which all bit patterns
> > // are valid.
> > unsafe impl FromBytes for GspMsgElement {}
> > +
> > +#[repr(transparent)]
> > +pub(crate) struct GspArgumentsCached(bindings::GSP_ARGUMENTS_CACHED);
> > +
> > +impl GspArgumentsCached {
> > + pub(crate) fn new(
> > + queue_arguments: MessageQueueInitArguments,
> > + sr_arguments: GspSrInitArguments,
> > + ) -> Self {
> > + Self(bindings::GSP_ARGUMENTS_CACHED {
> > + messageQueueInitArguments: queue_arguments.0,
> > + srInitArguments: sr_arguments.0,
> > + bDmemStack: 1,
> > + ..Default::default()
> > + })
> > + }
> > +}
> > +
> > +impl From<GspArgumentsCached> for bindings::GSP_ARGUMENTS_CACHED {
> > + fn from(value: GspArgumentsCached) -> Self {
> > + value.0
> > + }
> > +}
>
> This `From` impl seems unneeded?
Indeed. I don't remember why I added it, must have been needed in an earlier
version. I'm suprised clippy doesn't complain.
Powered by blists - more mailing lists