linux-kernel - Re: [PATCH v4 13/20] gpu: nova-core: register sysmem flush page

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: <DAC09YUQ6NAN.3KL0WKZQ40DES@nvidia.com>
Date: Mon, 02 Jun 2025 20:20:34 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Danilo Krummrich" <dakr@...nel.org>, "Lyude Paul" <lyude@...hat.com>
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" <benno.lossin@...ton.me>,
 "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>, "Ben Skeggs" <bskeggs@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Alistair Popple"
 <apopple@...dia.com>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, <nouveau@...ts.freedesktop.org>,
 <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 13/20] gpu: nova-core: register sysmem flush page

On Mon Jun 2, 2025 at 8:09 PM JST, Danilo Krummrich wrote:
> On Fri, May 30, 2025 at 05:57:44PM -0400, Lyude Paul wrote:
>> On Wed, 2025-05-21 at 15:45 +0900, Alexandre Courbot wrote:
>> > Reserve a page of system memory so sysmembar can perform a read on it if
>> > a system write occurred since the last flush. Do this early as it can be
>> > required to e.g. reset the GPU falcons.
>> > 
>> > Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
>> > ---
>> >  drivers/gpu/nova-core/gpu.rs  | 45 +++++++++++++++++++++++++++++++++++++++++--
>> >  drivers/gpu/nova-core/regs.rs | 10 ++++++++++
>> >  2 files changed, 53 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> > index 50417f608dc7b445958ae43444a13c7593204fcf..a4e2cf1b529cc25fc168f68f9eaa6f4a7a9748eb 100644
>> > --- a/drivers/gpu/nova-core/gpu.rs
>> > +++ b/drivers/gpu/nova-core/gpu.rs
>> > @@ -2,6 +2,7 @@
>> >  
>> >  use kernel::{device, devres::Devres, error::code::*, pci, prelude::*};
>> >  
>> > +use crate::dma::DmaObject;
>> >  use crate::driver::Bar0;
>> >  use crate::firmware::{Firmware, FIRMWARE_VERSION};
>> >  use crate::gfw;
>> > @@ -158,12 +159,32 @@ fn new(bar: &Bar0) -> Result<Spec> {
>> >  }
>> >  
>> >  /// Structure holding the resources required to operate the GPU.
>> > -#[pin_data]
>> > +#[pin_data(PinnedDrop)]
>> >  pub(crate) struct Gpu {
>> >      spec: Spec,
>> >      /// MMIO mapping of PCI BAR 0
>> >      bar: Devres<Bar0>,
>> >      fw: Firmware,
>> > +    /// System memory page required for flushing all pending GPU-side memory writes done through
>> > +    /// PCIE into system memory.
>> > +    sysmem_flush: DmaObject,
>> > +}
>> > +
>> > +#[pinned_drop]
>> > +impl PinnedDrop for Gpu {
>> > +    fn drop(self: Pin<&mut Self>) {
>> > +        // Unregister the sysmem flush page before we release it.
>> > +        let _ = self.bar.try_access_with(|b| {
>> > +            regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::default()
>> > +                .set_adr_39_08(0)
>> > +                .write(b);
>> > +            if self.spec.chipset >= Chipset::GA102 {
>> > +                regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI::default()
>> > +                    .set_adr_63_40(0)
>> > +                    .write(b);
>> > +            }
>> > +        });
>> > +    }
>
> Sorry that I haven't noticed this before -- I think this should be self
> contained in a new type (e.g. SysmemFlush).

Agreed, a new type sounds like the best way to do and would also address
Lyude's comment. Note however that the destructor will need access to
the Bar, so somehow it will need to be called from the Gpu's own
`PinnedDrop` implementation (or the solution you come with to do it from
`Device::remove`).