Message-ID: <C4F54820-4FCE-4096-B341-DB9AB667D0B8@nvidia.com>
Date: Wed, 30 Apr 2025 06:58:44 +0000
From: Joel Fernandes <joelagnelf@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
CC: Alexandre Courbot <acourbot@...dia.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 <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>,
	Jonathan Corbet <corbet@....net>, John Hubbard <jhubbard@...dia.com>, Ben
 Skeggs <bskeggs@...dia.com>, Timur Tabi <ttabi@...dia.com>, Alistair Popple
	<apopple@...dia.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "rust-for-linux@...r.kernel.org"
	<rust-for-linux@...r.kernel.org>, "nouveau@...ts.freedesktop.org"
	<nouveau@...ts.freedesktop.org>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 11/16] gpu: nova-core: add falcon register definitions and
 base code



> On Apr 22, 2025, at 10:45 AM, Danilo Krummrich <dakr@...nel.org> wrote:
> […]
>> +
>> +    fn get_signature_reg_fuse_version(
>> +        &self,
>> +        bar: &Devres<Bar0>,
>> +        engine_id_mask: u16,
>> +        ucode_id: u8,
>> +    ) -> Result<u32>;
>> +
>> +    // Program the BROM registers prior to starting a secure firmware.
>> +    fn program_brom(&self, bar: &Devres<Bar0>, params: &FalconBromParams) -> Result<()>;
>> +}
>> +
>> +/// Returns a boxed falcon HAL adequate for the passed `chipset`.
>> +///
>> +/// We use this function and a heap-allocated trait object instead of statically defined trait
>> +/// objects because of the two-dimensional (Chipset, Engine) lookup required to return the
>> +/// requested HAL.
> 
> Do we really need the dynamic dispatch? AFAICS, there's only E::BASE that is
> relevant to FalconHal impls?
> 
> Can't we do something like I do in the following example [1]?
> 
> ```
> use std::marker::PhantomData;
> use std::ops::Deref;
> 
> trait Engine {
>    const BASE: u32;
> }
> 
> trait Hal<E: Engine> {
>    fn access(&self);
> }
> 
> struct Gsp;
> 
> impl Engine for Gsp {
>    const BASE: u32 = 0x1;
> }
> 
> struct Sec2;
> 
> impl Engine for Sec2 {
>    const BASE: u32 = 0x2;
> }
> 
> struct GA100<E: Engine>(PhantomData<E>);
> 
> impl<E: Engine> Hal<E> for GA100<E> {
>    fn access(&self) {
>        println!("Base: {}", E::BASE);
>    }
> }
> 
> impl<E: Engine> GA100<E> {
>    fn new() -> Self {
>        Self(PhantomData)
>    }
> }
> 
> //struct Falcon<E: Engine>(GA100<E>);
> 
> struct Falcon<H: Hal<E>, E: Engine>(H, PhantomData<E>);
> 
> impl<H: Hal<E>, E: Engine> Falcon<H, E> {
>    fn new(hal: H) -> Self {
>        Self(hal, PhantomData)
>    }
> }
> 
> impl<H: Hal<E>, E: Engine> Deref for Falcon<H, E> {
>    type Target = H;
> 
>    fn deref(&self) -> &Self::Target {
>        &self.0
>    }
> }
> 
> fn main() {
>    let gsp = Falcon::new(GA100::<Gsp>::new());
>    let sec2 = Falcon::new(GA100::<Sec2>::new());
> 
>    gsp.access();
>    sec2.access();
> }
> ```
> 
> [1] https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=bf7035a07e79a4047fb6834eac03a9f2

I am still researching this idea from a rust point of view, but quick question - will this even work if the chip type (GAxxx) is determined at runtime? That does need runtime polymorphism.

Thanks,

 - Joel


> 
>> +///
>> +/// TODO: replace the return type with `KBox` once it gains the ability to host trait objects.
>> +pub(crate) fn create_falcon_hal<E: FalconEngine + 'static>(
>> +    chipset: Chipset,
>> +) -> Result<Arc<dyn FalconHal<E>>> {
>> +    let hal = match chipset {
>> +        Chipset::GA102 | Chipset::GA103 | Chipset::GA104 | Chipset::GA106 | Chipset::GA107 => {
>> +            Arc::new(ga102::Ga102::<E>::new(), GFP_KERNEL)? as Arc<dyn FalconHal<E>>
>> +        }
>> +        _ => return Err(ENOTSUPP),
>> +    };
>> +
>> +    Ok(hal)
>> +}
>> diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..747b02ca671f7d4a97142665a9ba64807c87391e
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
>> @@ -0,0 +1,111 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +use core::marker::PhantomData;
>> +use core::time::Duration;
>> +
>> +use kernel::devres::Devres;
>> +use kernel::prelude::*;
>> +
>> +use crate::driver::Bar0;
>> +use crate::falcon::{FalconBromParams, FalconEngine, FalconModSelAlgo, RiscvCoreSelect};
>> +use crate::regs;
>> +use crate::timer::Timer;
>> +
>> +use super::FalconHal;
>> +
>> +fn select_core_ga102<E: FalconEngine>(bar: &Devres<Bar0>, timer: &Timer) -> Result<()> {
>> +    let bcr_ctrl = with_bar!(bar, |b| regs::RiscvBcrCtrl::read(b, E::BASE))?;
>> +    if bcr_ctrl.core_select() != RiscvCoreSelect::Falcon {
>> +        with_bar!(bar, |b| regs::RiscvBcrCtrl::default()
>> +            .set_core_select(RiscvCoreSelect::Falcon)
>> +            .write(b, E::BASE))?;
>> +
>> +        timer.wait_on(bar, Duration::from_millis(10), || {
>> +            bar.try_access_with(|b| regs::RiscvBcrCtrl::read(b, E::BASE))
>> +                .and_then(|v| if v.valid() { Some(()) } else { None })
>> +        })?;
>> +    }
>> +
>> +    Ok(())
>> +}
>> +
>> +fn get_signature_reg_fuse_version_ga102(
>> +    bar: &Devres<Bar0>,
>> +    engine_id_mask: u16,
>> +    ucode_id: u8,
>> +) -> Result<u32> {
>> +    // The ucode fuse versions are contained in the FUSE_OPT_FPF_<ENGINE>_UCODE<X>_VERSION
>> +    // registers, which are an array. Our register definition macros do not allow us to manage them
>> +    // properly, so we need to hardcode their addresses for now.
>> +
>> +    // Each engine has 16 ucode version registers numbered from 1 to 16.
>> +    if ucode_id == 0 || ucode_id > 16 {
>> +        pr_warn!("invalid ucode id {:#x}", ucode_id);
>> +        return Err(EINVAL);
>> +    }
>> +    let reg_fuse = if engine_id_mask & 0x0001 != 0 {
>> +        // NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION
>> +        0x824140
>> +    } else if engine_id_mask & 0x0004 != 0 {
>> +        // NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION
>> +        0x824100
>> +    } else if engine_id_mask & 0x0400 != 0 {
>> +        // NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION
>> +        0x8241c0
>> +    } else {
>> +        pr_warn!("unexpected engine_id_mask {:#x}", engine_id_mask);
>> +        return Err(EINVAL);
>> +    } + ((ucode_id - 1) as usize * core::mem::size_of::<u32>());
>> +
>> +    let reg_fuse_version = with_bar!(bar, |b| { b.read32(reg_fuse) })?;
>> +
>> +    // Equivalent of Find Last Set bit.
>> +    Ok(u32::BITS - reg_fuse_version.leading_zeros())
>> +}
>> +
>> +fn program_brom_ga102<E: FalconEngine>(
>> +    bar: &Devres<Bar0>,
>> +    params: &FalconBromParams,
>> +) -> Result<()> {
>> +    with_bar!(bar, |b| {
>> +        regs::FalconBromParaaddr0::default()
>> +            .set_addr(params.pkc_data_offset)
>> +            .write(b, E::BASE);
>> +        regs::FalconBromEngidmask::default()
>> +            .set_mask(params.engine_id_mask as u32)
>> +            .write(b, E::BASE);
>> +        regs::FalconBromCurrUcodeId::default()
>> +            .set_ucode_id(params.ucode_id as u32)
>> +            .write(b, E::BASE);
>> +        regs::FalconModSel::default()
>> +            .set_algo(FalconModSelAlgo::Rsa3k)
>> +            .write(b, E::BASE)
>> +    })
>> +}
>> +
>> +pub(super) struct Ga102<E: FalconEngine>(PhantomData<E>);
>> +
>> +impl<E: FalconEngine> Ga102<E> {
>> +    pub(super) fn new() -> Self {
>> +        Self(PhantomData)
>> +    }
>> +}
>> +
>> +impl<E: FalconEngine> FalconHal<E> for Ga102<E> {
>> +    fn select_core(&self, bar: &Devres<Bar0>, timer: &Timer) -> Result<()> {
>> +        select_core_ga102::<E>(bar, timer)
>> +    }
>> +
>> +    fn get_signature_reg_fuse_version(
>> +        &self,
>> +        bar: &Devres<Bar0>,
>> +        engine_id_mask: u16,
>> +        ucode_id: u8,
>> +    ) -> Result<u32> {
>> +        get_signature_reg_fuse_version_ga102(bar, engine_id_mask, ucode_id)
>> +    }
>> +
>> +    fn program_brom(&self, bar: &Devres<Bar0>, params: &FalconBromParams) -> Result<()> {
>> +        program_brom_ga102::<E>(bar, params)
>> +    }
>> +}
>> diff --git a/drivers/gpu/nova-core/falcon/sec2.rs b/drivers/gpu/nova-core/falcon/sec2.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..85dda3e8380a3d31d34c92c4236c6f81c63ce772
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/falcon/sec2.rs
>> @@ -0,0 +1,9 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +use crate::falcon::{Falcon, FalconEngine};
>> +
>> +pub(crate) struct Sec2;
>> +impl FalconEngine for Sec2 {
>> +    const BASE: usize = 0x00840000;
>> +}
>> +pub(crate) type Sec2Falcon = Falcon<Sec2>;
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index 1b3e43e0412e2a2ea178c7404ea647c9e38d4e04..ec4c648c6e8b4aa7d06c627ed59c0e66a08c679e 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -5,6 +5,8 @@
>> use crate::devinit;
>> use crate::dma::DmaObject;
>> use crate::driver::Bar0;
>> +use crate::falcon::gsp::GspFalcon;
>> +use crate::falcon::sec2::Sec2Falcon;
>> use crate::firmware::Firmware;
>> use crate::regs;
>> use crate::timer::Timer;
>> @@ -221,6 +223,20 @@ pub(crate) fn new(
>> 
>>         let timer = Timer::new();
>> 
>> +        let gsp_falcon = GspFalcon::new(
>> +            pdev,
>> +            spec.chipset,
>> +            &bar,
>> +            if spec.chipset > Chipset::GA100 {
>> +                true
>> +            } else {
>> +                false
>> +            },
>> +        )?;
>> +        gsp_falcon.clear_swgen0_intr(&bar)?;
>> +
>> +        let _sec2_falcon = Sec2Falcon::new(pdev, spec.chipset, &bar, true)?;
>> +
>>         Ok(pin_init!(Self {
>>             spec,
>>             bar,
>> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
>> index df3468c92c6081b3e2db218d92fbe1c40a0a75c3..4dde8004d24882c60669b5acd6af9d6988c66a9c 100644
>> --- a/drivers/gpu/nova-core/nova_core.rs
>> +++ b/drivers/gpu/nova-core/nova_core.rs
>> @@ -23,6 +23,7 @@ macro_rules! with_bar {
>> mod devinit;
>> mod dma;
>> mod driver;
>> +mod falcon;
>> mod firmware;
>> mod gpu;
>> mod regs;
>> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
>> index f191cf4eb44c2b950e5cfcc6d04f95c122ce29d3..c76a16dc8e7267a4eb54cb71e1cca6fb9e00188f 100644
>> --- a/drivers/gpu/nova-core/regs.rs
>> +++ b/drivers/gpu/nova-core/regs.rs
>> @@ -6,6 +6,10 @@
>> #[macro_use]
>> mod macros;
>> 
>> +use crate::falcon::{
>> +    FalconCoreRev, FalconCoreRevSubversion, FalconFbifMemType, FalconFbifTarget, FalconModSelAlgo,
>> +    FalconSecurityModel, RiscvCoreSelect,
>> +};
>> use crate::gpu::Chipset;
>> 
>> register!(Boot0@...0000000, "Basic revision information about the GPU";
>> @@ -44,3 +48,188 @@
>> register!(Pgc6AonSecureScratchGroup05@...0118234;
>>     31:0    value => as u32
>> );
>> +
>> +/* PFALCON */
>> +
>> +register!(FalconIrqsclr@...00000004;
>> +    4:4     halt => as_bit bool;
>> +    6:6     swgen0 => as_bit bool;
>> +);
>> +
>> +register!(FalconIrqstat@...00000008;
>> +    4:4     halt => as_bit bool;
>> +    6:6     swgen0 => as_bit bool;
>> +);
>> +
>> +register!(FalconIrqmclr@...00000014;
>> +    31:0    val => as u32
>> +);
>> +
>> +register!(FalconIrqmask@...00000018;
>> +    31:0    val => as u32
>> +);
>> +
>> +register!(FalconRm@...00000084;
>> +    31:0    val => as u32
>> +);
>> +
>> +register!(FalconIrqdest@...0000001c;
>> +    31:0    val => as u32
>> +);
>> +
>> +register!(FalconMailbox0@...00000040;
>> +    31:0    mailbox0 => as u32
>> +);
>> +register!(FalconMailbox1@...00000044;
>> +    31:0    mailbox1 => as u32
>> +);
>> +
>> +register!(FalconHwcfg2@...000000f4;
>> +    10:10   riscv => as_bit bool;
>> +    12:12   mem_scrubbing => as_bit bool;
>> +    31:31   reset_ready => as_bit bool;
>> +);
>> +
>> +register!(FalconCpuCtl@...00000100;
>> +    1:1     start_cpu => as_bit bool;
>> +    4:4     halted => as_bit bool;
>> +    6:6     alias_en => as_bit bool;
>> +);
>> +
>> +register!(FalconBootVec@...00000104;
>> +    31:0    boot_vec => as u32
>> +);
>> +
>> +register!(FalconHwCfg@...00000108;
>> +    8:0     imem_size => as u32;
>> +    17:9    dmem_size => as u32;
>> +);
>> +
>> +register!(FalconDmaCtl@...0000010c;
>> +    0:0     require_ctx => as_bit bool;
>> +    1:1     dmem_scrubbing  => as_bit bool;
>> +    2:2     imem_scrubbing => as_bit bool;
>> +    6:3     dmaq_num => as_bit u8;
>> +    7:7     secure_stat => as_bit bool;
>> +);
>> +
>> +register!(FalconDmaTrfBase@...00000110;
>> +    31:0    base => as u32;
>> +);
>> +
>> +register!(FalconDmaTrfMOffs@...00000114;
>> +    23:0    offs => as u32;
>> +);
>> +
>> +register!(FalconDmaTrfCmd@...00000118;
>> +    0:0     full => as_bit bool;
>> +    1:1     idle => as_bit bool;
>> +    3:2     sec => as_bit u8;
>> +    4:4     imem => as_bit bool;
>> +    5:5     is_write => as_bit bool;
>> +    10:8    size => as u8;
>> +    14:12   ctxdma => as u8;
>> +    16:16   set_dmtag => as u8;
>> +);
>> +
>> +register!(FalconDmaTrfBOffs@...0000011c;
>> +    31:0    offs => as u32;
>> +);
>> +
>> +register!(FalconDmaTrfBase1@...00000128;
>> +    8:0     base => as u16;
>> +);
>> +
>> +register!(FalconHwcfg1@...0000012c;
>> +    3:0     core_rev => try_into FalconCoreRev, "core revision of the falcon";
>> +    5:4     security_model => try_into FalconSecurityModel, "security model of the falcon";
>> +    7:6     core_rev_subversion => into FalconCoreRevSubversion;
>> +    11:8    imem_ports => as u8;
>> +    15:12   dmem_ports => as u8;
>> +);
>> +
>> +register!(FalconCpuCtlAlias@...00000130;
>> +    1:1     start_cpu => as_bit bool;
>> +);
>> +
>> +/* TODO: this is an array of registers */
>> +register!(FalconImemC@...00000180;
>> +    7:2     offs => as u8;
>> +    23:8    blk => as u8;
>> +    24:24   aincw => as_bit bool;
>> +    25:25   aincr => as_bit bool;
>> +    28:28   secure => as_bit bool;
>> +    29:29   sec_atomic => as_bit bool;
>> +);
>> +
>> +register!(FalconImemD@...00000184;
>> +    31:0    data => as u32;
>> +);
>> +
>> +register!(FalconImemT@...00000188;
>> +    15:0    data => as u16;
>> +);
>> +
>> +register!(FalconDmemC@...000001c0;
>> +    7:2     offs => as u8;
>> +    23:0    addr => as u32;
>> +    23:8    blk => as u8;
>> +    24:24   aincw => as_bit bool;
>> +    25:25   aincr => as_bit bool;
>> +    26:26   settag => as_bit bool;
>> +    27:27   setlvl => as_bit bool;
>> +    28:28   va => as_bit bool;
>> +    29:29   miss => as_bit bool;
>> +);
>> +
>> +register!(FalconDmemD@...000001c4;
>> +    31:0    data => as u32;
>> +);
>> +
>> +register!(FalconModSel@...00001180;
>> +    7:0     algo => try_into FalconModSelAlgo;
>> +);
>> +register!(FalconBromCurrUcodeId@...00001198;
>> +    31:0    ucode_id => as u32;
>> +);
>> +register!(FalconBromEngidmask@...0000119c;
>> +    31:0    mask => as u32;
>> +);
>> +register!(FalconBromParaaddr0@...00001210;
>> +    31:0    addr => as u32;
>> +);
>> +
>> +register!(RiscvCpuctl@...00000388;
>> +    0:0     startcpu => as_bit bool;
>> +    4:4     halted => as_bit bool;
>> +    5:5     stopped => as_bit bool;
>> +    7:7     active_stat => as_bit bool;
>> +);
>> +
>> +register!(FalconEngine@...000003c0;
>> +    0:0     reset => as_bit bool;
>> +);
>> +
>> +register!(RiscvIrqmask@...00000528;
>> +    31:0    mask => as u32;
>> +);
>> +
>> +register!(RiscvIrqdest@...0000052c;
>> +    31:0    dest => as u32;
>> +);
>> +
>> +/* TODO: this is an array of registers */
>> +register!(FalconFbifTranscfg@...00000600;
>> +    1:0     target => try_into FalconFbifTarget;
>> +    2:2     mem_type => as_bit FalconFbifMemType;
>> +);
>> +
>> +register!(FalconFbifCtl@...00000624;
>> +    7:7     allow_phys_no_ctx => as_bit bool;
>> +);
>> +
>> +register!(RiscvBcrCtrl@...00001668;
>> +    0:0     valid => as_bit bool;
>> +    4:4     core_select => as_bit RiscvCoreSelect;
>> +    8:8     br_fetch => as_bit bool;
>> +);
>> diff --git a/drivers/gpu/nova-core/timer.rs b/drivers/gpu/nova-core/timer.rs
>> index 8987352f4192bc9b4b2fc0fb5f2e8e62ff27be68..c03a5c36d1230dfbf2bd6e02a793264280c6d509 100644
>> --- a/drivers/gpu/nova-core/timer.rs
>> +++ b/drivers/gpu/nova-core/timer.rs
>> @@ -2,9 +2,6 @@
>> 
>> //! Nova Core Timer subdevice
>> 
>> -// To be removed when all code is used.
>> -#![allow(dead_code)]
>> -
>> use core::fmt::Display;
>> use core::ops::{Add, Sub};
>> use core::time::Duration;
>> 
>> --
>> 2.49.0
>> 

Powered by blists - more mailing lists