> On Apr 22, 2025, at 10:45 AM, Danilo Krummrich <d...@kernel.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@0x00000000, "Basic revision information about the GPU";
>> @@ -44,3 +48,188 @@
>> register!(Pgc6AonSecureScratchGroup05@0x00118234;
>> 31:0 value => as u32
>> );
>> +
>> +/* PFALCON */
>> +
>> +register!(FalconIrqsclr@+0x00000004;
>> + 4:4 halt => as_bit bool;
>> + 6:6 swgen0 => as_bit bool;
>> +);
>> +
>> +register!(FalconIrqstat@+0x00000008;
>> + 4:4 halt => as_bit bool;
>> + 6:6 swgen0 => as_bit bool;
>> +);
>> +
>> +register!(FalconIrqmclr@+0x00000014;
>> + 31:0 val => as u32
>> +);
>> +
>> +register!(FalconIrqmask@+0x00000018;
>> + 31:0 val => as u32
>> +);
>> +
>> +register!(FalconRm@+0x00000084;
>> + 31:0 val => as u32
>> +);
>> +
>> +register!(FalconIrqdest@+0x0000001c;
>> + 31:0 val => as u32
>> +);
>> +
>> +register!(FalconMailbox0@+0x00000040;
>> + 31:0 mailbox0 => as u32
>> +);
>> +register!(FalconMailbox1@+0x00000044;
>> + 31:0 mailbox1 => as u32
>> +);
>> +
>> +register!(FalconHwcfg2@+0x000000f4;
>> + 10:10 riscv => as_bit bool;
>> + 12:12 mem_scrubbing => as_bit bool;
>> + 31:31 reset_ready => as_bit bool;
>> +);
>> +
>> +register!(FalconCpuCtl@+0x00000100;
>> + 1:1 start_cpu => as_bit bool;
>> + 4:4 halted => as_bit bool;
>> + 6:6 alias_en => as_bit bool;
>> +);
>> +
>> +register!(FalconBootVec@+0x00000104;
>> + 31:0 boot_vec => as u32
>> +);
>> +
>> +register!(FalconHwCfg@+0x00000108;
>> + 8:0 imem_size => as u32;
>> + 17:9 dmem_size => as u32;
>> +);
>> +
>> +register!(FalconDmaCtl@+0x0000010c;
>> + 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@+0x00000110;
>> + 31:0 base => as u32;
>> +);
>> +
>> +register!(FalconDmaTrfMOffs@+0x00000114;
>> + 23:0 offs => as u32;
>> +);
>> +
>> +register!(FalconDmaTrfCmd@+0x00000118;
>> + 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@+0x0000011c;
>> + 31:0 offs => as u32;
>> +);
>> +
>> +register!(FalconDmaTrfBase1@+0x00000128;
>> + 8:0 base => as u16;
>> +);
>> +
>> +register!(FalconHwcfg1@+0x0000012c;
>> + 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@+0x00000130;
>> + 1:1 start_cpu => as_bit bool;
>> +);
>> +
>> +/* TODO: this is an array of registers */
>> +register!(FalconImemC@+0x00000180;
>> + 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@+0x00000184;
>> + 31:0 data => as u32;
>> +);
>> +
>> +register!(FalconImemT@+0x00000188;
>> + 15:0 data => as u16;
>> +);
>> +
>> +register!(FalconDmemC@+0x000001c0;
>> + 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@+0x000001c4;
>> + 31:0 data => as u32;
>> +);
>> +
>> +register!(FalconModSel@+0x00001180;
>> + 7:0 algo => try_into FalconModSelAlgo;
>> +);
>> +register!(FalconBromCurrUcodeId@+0x00001198;
>> + 31:0 ucode_id => as u32;
>> +);
>> +register!(FalconBromEngidmask@+0x0000119c;
>> + 31:0 mask => as u32;
>> +);
>> +register!(FalconBromParaaddr0@+0x00001210;
>> + 31:0 addr => as u32;
>> +);
>> +
>> +register!(RiscvCpuctl@+0x00000388;
>> + 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@+0x000003c0;
>> + 0:0 reset => as_bit bool;
>> +);
>> +
>> +register!(RiscvIrqmask@+0x00000528;
>> + 31:0 mask => as u32;
>> +);
>> +
>> +register!(RiscvIrqdest@+0x0000052c;
>> + 31:0 dest => as u32;
>> +);
>> +
>> +/* TODO: this is an array of registers */
>> +register!(FalconFbifTranscfg@+0x00000600;
>> + 1:0 target => try_into FalconFbifTarget;
>> + 2:2 mem_type => as_bit FalconFbifMemType;
>> +);
>> +
>> +register!(FalconFbifCtl@+0x00000624;
>> + 7:7 allow_phys_no_ctx => as_bit bool;
>> +);
>> +
>> +register!(RiscvBcrCtrl@+0x00001668;
>> + 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
>>
