On Thu May 15, 2025 at 1:38 AM JST, Danilo Krummrich wrote:
> On Wed, May 07, 2025 at 10:52:45PM +0900, Alexandre Courbot wrote:
>> The FWSEC firmware needs to be extracted from the VBIOS and patched with
>> the desired command, as well as the right signature. Do this so we are
>> ready to load and run this firmware into the GSP falcon and create the
>> FRTS region.
>>
>> [joelagn...@nvidia.com: give better names to FalconAppifHdrV1's fields]
>> Signed-off-by: Alexandre Courbot <acour...@nvidia.com>
>> ---
>> drivers/gpu/nova-core/dma.rs | 3 -
>> drivers/gpu/nova-core/firmware.rs | 18 ++
>> drivers/gpu/nova-core/firmware/fwsec.rs | 359
>> ++++++++++++++++++++++++++++++++
>> drivers/gpu/nova-core/gpu.rs | 20 +-
>> drivers/gpu/nova-core/vbios.rs | 3 -
>> 5 files changed, 395 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
>> index
>> 9d90ae01d0044eaab4ddbc3eba216741d7a623ef..a12d0dff574aa38fb5eb8f4d759611af2f8ba3ec
>> 100644
>> --- a/drivers/gpu/nova-core/dma.rs
>> +++ b/drivers/gpu/nova-core/dma.rs
>> @@ -2,9 +2,6 @@
>>
>> //! Simple DMA object wrapper.
>>
>> -// To be removed when all code is used.
>> -#![expect(dead_code)]
>> -
>> use core::ops::{Deref, DerefMut};
>>
>> use kernel::device;
>> diff --git a/drivers/gpu/nova-core/firmware.rs
>> b/drivers/gpu/nova-core/firmware.rs
>> index
>> 960982174d834c7c66a47ecfb3a15bf47116b2c5..3945fd18499555ddd6fb2e0ea69535b40fcc4b08
>> 100644
>> --- a/drivers/gpu/nova-core/firmware.rs
>> +++ b/drivers/gpu/nova-core/firmware.rs
>> @@ -8,9 +8,12 @@
>> use kernel::prelude::*;
>> use kernel::str::CString;
>>
>> +use crate::dma::DmaObject;
>> use crate::gpu;
>> use crate::gpu::Chipset;
>>
>> +pub(crate) mod fwsec;
>> +
>> pub(crate) const FIRMWARE_VERSION: &str = "535.113.01";
>>
>> /// Structure encapsulating the firmware blobs required for the GPU to
>> operate.
>> @@ -86,6 +89,21 @@ pub(crate) fn size(&self) -> usize {
>> }
>> }
>>
>> +/// Patch the `ucode_dma` firmware at offset `sig_base_img` with
>> `signature`.
>> +fn patch_signature(ucode_dma: &mut DmaObject, signature: &[u8],
>> sig_base_img: usize) -> Result<()> {
>> + if sig_base_img + signature.len() > ucode_dma.size() {
>> + return Err(EINVAL);
>> + }
>> +
>> + // SAFETY: we are the only user of this object, so there cannot be any
>> race.
>> + let dst = unsafe { ucode_dma.start_ptr_mut().add(sig_base_img) };
>> +
>> + // SAFETY: `signature` and `dst` are valid, properly aligned, and do
>> not overlap.
>> + unsafe { core::ptr::copy_nonoverlapping(signature.as_ptr(), dst,
>> signature.len()) };
>> +
>> + Ok(())
>> +}
>
> Why is this not in firmware/fwsec.rs, like patch_command()?
Ah, there is no way to know it now, but this function will also be used
to patch the booter firmware that runs on sec2, so having it here makes
it available to both sub-modules. I'm fine with moving it into the fwsec
module temporarily if you prefer though.
>
> Also, please wrap the ucode DmaObject in its own type, i.e.
> `struct UcodeDma(DmaObject)` and make the patch_*() functions methods of this
> type. They're only applicable for the ucode DmaObject.
Indeed, good idea. We will event want to specialize that type against
the kind of firmware as not all patching methods may be applicable
depending on the firmware.
<snip>
>> +impl FwsecFirmware {
>> + /// Extract the Fwsec firmware from `bios` and patch it to run with the
>> `cmd` command.
>> + pub(crate) fn new(
>> + falcon: &Falcon<Gsp>,
>> + dev: &Device<device::Bound>,
>> + bar: &Bar0,
>> + bios: &Vbios,
>> + cmd: FwsecCommand,
>> + ) -> Result<Self> {
>> + let v3_desc = bios.fwsec_header(dev)?;
>> + let ucode = bios.fwsec_ucode(dev)?;
>> +
>> + let mut ucode_dma = DmaObject::from_data(dev, ucode)?;
>> + patch_command(&mut ucode_dma, v3_desc, cmd)?;
>> +
>> + const SIG_SIZE: usize = 96 * 4;
>
> 96 * 4? :-)
Mmmm let me look that up. ^_^; But I think it means that a signature is
made of 96 32-bit integers.
>
>> + let signatures = bios.fwsec_sigs(dev)?;
>> + let sig_base_img = (v3_desc.imem_load_size +
>> v3_desc.pkc_data_offset) as usize;
>> +
>> + if v3_desc.signature_count != 0 {
>> + // Patch signature.
>> + let desc_sig_versions = v3_desc.signature_versions as u32;
>> + let reg_fuse_version = falcon.get_signature_reg_fuse_version(
>> + bar,
>> + v3_desc.engine_id_mask,
>> + v3_desc.ucode_id,
>> + )?;
>> + dev_dbg!(
>> + dev,
>> + "desc_sig_versions: {:#x}, reg_fuse_version: {}\n",
>> + desc_sig_versions,
>> + reg_fuse_version
>> + );
>> + let signature_idx = {
>> + let reg_fuse_version_bit = 1 << reg_fuse_version;
>> +
>> + // Check if the fuse version is supported by the firmware.
>> + if desc_sig_versions & reg_fuse_version_bit == 0 {
>> + dev_warn!(
>> + dev,
>> + "no matching signature: {:#x} {:#x}\n",
>> + reg_fuse_version_bit,
>> + v3_desc.signature_versions
>> + );
>
> Looks like this should be dev_err!().
Indeed, fixed.
