| 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: <2ff114ba-46b5-4a5c-80b8-3bd840dd9f3f@nvidia.com>
Date: Sun, 7 Dec 2025 22:00:55 -0800
From: John Hubbard <jhubbard@...dia.com>
To: Joel Fernandes <joelagnelf@...dia.com>, Danilo Krummrich <dakr@...nel.org>
Cc: Alexandre Courbot <acourbot@...dia.com>, Timur Tabi <ttabi@...dia.com>,
Alistair Popple <apopple@...dia.com>, Edwin Peer <epeer@...dia.com>,
Zhi Wang <zhiw@...dia.com>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Bjorn Helgaas <bhelgaas@...gle.com>,
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>,
nouveau@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 23/31] gpu: nova-core: Hopper/Blackwell: add FSP Chain of
Trust boot
On 12/5/25 9:15 AM, Joel Fernandes wrote:
> Hi John,
>
> On 12/3/2025 12:59 AM, John Hubbard wrote:
>> + ///
>> + /// This version takes pre-extracted signatures and FMC image data.
>> + /// Used when signatures are extracted separately from the full ELF file.
>> + #[allow(clippy::too_many_arguments)]
>> + pub(crate) fn boot_gsp_fmc_with_signatures(
>> + dev: &device::Device<device::Bound>,
>> + bar: &crate::driver::Bar0,
>> + chipset: crate::gpu::Chipset,
>> + fmc_image_fw: &crate::dma::DmaObject, // Contains only the image section
>> + fmc_boot_params: &kernel::dma::CoherentAllocation<GspFmcBootParams>,
>> + total_reserved_size: u64,
>> + resume: bool,
>> + fsp_falcon: &crate::falcon::Falcon<crate::falcon::fsp::Fsp>,
>> + signatures: &FmcSignatures,
>> + ) -> Result<()> {
>> + dev_dbg!(dev, "Starting FSP boot sequence for {}\n", chipset);
>
> I see lots of dev_dbg left as you also pointed. I guess you will remove these as
> we discussed in the other thread.
Yes. I am thinking of leaving in a very few items (like 1 or 2), as sort of
progress indicators, but even those are up for veto of course.
>
>> +
>> + // Build FSP Chain of Trust message
>> + let fmc_addr = fmc_image_fw.dma_handle(); // Now points to image data only
>> + let fmc_boot_params_addr = fmc_boot_params.dma_handle();
>> +
>> + // frts_offset is relative to FB end: FRTS_location = FB_END - frts_offset
>> + let frts_offset = if !resume {
>> + let mut frts_reserved_size = if chipset.needs_large_reserved_mem() {
>> + 0x220000 // heap_size_non_wpr for Hopper/Blackwell+
>
> Can we please use constants for the magic numbers? Also with comment headers
> clearly documenting the constants where they are defined.
>
>> + } else {
>> + total_reserved_size
>> + };
>> +
>> + // Add PMU reserved size
>> + frts_reserved_size += u64::from(crate::fb::PMU_RESERVED_SIZE);
>> +
>> + frts_reserved_size
>> + .align_up(Alignment::new::<0x200000>())
>> + .unwrap_or(frts_reserved_size)
>> + } else {
>> + 0
>> + };
>> + let frts_size = if !resume { 0x100000 } else { 0 }; // 1MB FRTS size
>
>
> Should use SZ_ constants, here and everywhere. Like SZ_1M etc [1].
Yes, I'm still glitching and not always doing that, good catch.
>
> [1] https://rust.docs.kernel.org/src/kernel/sizes.rs.html
>
>> +
>> + // Build the FSP message
>> + let msg = KBox::new(
>> + FspMessage {
>> + mctp_header: (mctp::HEADER_SOM << 31)
>> + | (mctp::HEADER_EOM << 30)
>> + | (mctp::HEADER_SEID << 16)
>> + | (mctp::HEADER_SEQ << 28),
>> +
>> + nvdm_header: (mctp::MSG_TYPE_VENDOR_PCI)
>> + | (mctp::VENDOR_ID_NV << 8)
>> + | (mctp::NVDM_TYPE_COT << 24),
>> +
>> + cot: NvdmPayloadCot {
>> + version: FSP_COT_VERSION,
>> + size: core::mem::size_of::<NvdmPayloadCot>() as u16,
>> + gsp_fmc_sysmem_offset: fmc_addr,
>> + frts_sysmem_offset: 0,
>> + frts_sysmem_size: 0,
>> + frts_vidmem_offset: frts_offset,
>> + frts_vidmem_size: frts_size,
>> + hash384: signatures.hash384,
>> + public_key: signatures.public_key,
>> + signature: signatures.signature,
>> + gsp_boot_args_sysmem_offset: fmc_boot_params_addr,
>> + },
>> + },
>> + GFP_KERNEL,
>> + )?;
>> +
>> + // Convert message to bytes for sending
>> + let msg_bytes = msg.as_bytes();
>> +
>> + dev_dbg!(
>> + dev,
>> + "FSP COT Message:\n size={} bytes\n fmc_addr={:#x}\n boot_params={:#x}\n \
>> + frts_offset={:#x}\n frts_size={:#x}\n",
>> + msg_bytes.len(),
>> + fmc_addr,
>> + fmc_boot_params_addr,
>> + frts_offset,
>> + frts_size
>> + );
>> +
>> + // Send COT message to FSP and wait for response
>> + Self::send_sync_fsp(dev, bar, fsp_falcon, mctp::NVDM_TYPE_COT, msg_bytes)?;
>
> This seems to diverge from the pattern we use in GSP `cmdq`. Can we keep it
> consistent, like `send_sync_fsp<M>` ?
>
> pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
> where
> M: CommandToGsp,
>
> Alistair/Alex may have more design suggestions since they came up with this
> pattern. Especially around safety (guarantees that the part of buffer being read
> will not be modified by HW etc).
>
> Over all, I feel we have 3 different mechanisms now (1 upstream, 2 in the
> works). The GSP cmdq, RM control and then this one. It would be good to get some
> consistency in the API designs for all these different mechanisms (and possibly
> share any common code).
>
I'll try to catch up with the GSP cmdq approach and apply it.
thanks,
--
John Hubbard
Powered by blists - more mailing lists