From 3606620b316c29e3de8ff87b40828c722086a9c9 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Thu, 19 Jun 2025 22:24:08 +0900 Subject: [PATCH] gpu: nova-core: update and annotate TODO list A few new dependencies are required to remove some of the TODO items: - A way to safely convert from byte slices to types implementing `FromBytes`, - A way to obtain slices and write into a `CoherentAllocation`, - Several improvements to the `register!()` macro, - Alignment operations to powers of two, and an equivalent to the C `fls`, - Support for `xa_alloc` in the XAlloc bindings. Some items have also become obsolete: - The auxiliary bus abstractions have been implemented and are in use, - The ELF utilities are not considered for being part of the core kernel bindings anymore. - VBIOS, falcon and GPU timer have been completed. We now have quite a few TODO entries in the code, so annotate them with a 4 letter code representing the corresponding task in `todo.rst`. This allows to easily find which part of the code corresponds to a given entry (and conversely). Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250619-nova-frts-v6-24-ecf41ef99252@nvidia.com Signed-off-by: Danilo Krummrich --- Documentation/gpu/nova/core/todo.rst | 107 ++++++++++++---------- drivers/gpu/nova-core/dma.rs | 2 +- drivers/gpu/nova-core/driver.rs | 2 +- drivers/gpu/nova-core/falcon.rs | 8 +- drivers/gpu/nova-core/falcon/hal/ga102.rs | 10 +- drivers/gpu/nova-core/fb.rs | 2 +- drivers/gpu/nova-core/firmware/fwsec.rs | 6 +- drivers/gpu/nova-core/gfw.rs | 2 +- drivers/gpu/nova-core/gpu.rs | 2 +- drivers/gpu/nova-core/regs.rs | 8 +- drivers/gpu/nova-core/regs/macros.rs | 2 +- drivers/gpu/nova-core/util.rs | 2 +- drivers/gpu/nova-core/vbios.rs | 2 +- 13 files changed, 84 insertions(+), 71 deletions(-) diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst index 8a459fc088121..894a1e9c3741a 100644 --- a/Documentation/gpu/nova/core/todo.rst +++ b/Documentation/gpu/nova/core/todo.rst @@ -14,14 +14,17 @@ Tasks may have the following fields: - ``Contact``: The person that can be contacted for further information about the task. +A task might have `[ABCD]` code after its name. This code can be used to grep +into the code for `TODO` entries related to it. + Enablement (Rust) ================= Tasks that are not directly related to nova-core, but are preconditions in terms of required APIs. -FromPrimitive API ------------------ +FromPrimitive API [FPRI] +------------------------ Sometimes the need arises to convert a number to a value of an enum or a structure. @@ -41,8 +44,27 @@ automatically generates the corresponding mappings between a value and a number. | Complexity: Beginner | Link: https://docs.rs/num/latest/num/trait.FromPrimitive.html -Generic register abstraction ----------------------------- +Conversion from byte slices for types implementing FromBytes [TRSM] +------------------------------------------------------------------- + +We retrieve several structures from byte streams coming from the BIOS or loaded +firmware. At the moment converting the bytes slice into the proper type require +an inelegant `unsafe` operation; this will go away once `FromBytes` implements +a proper `from_bytes` method. + +| Complexity: Beginner + +CoherentAllocation improvements [COHA] +-------------------------------------- + +`CoherentAllocation` needs a safe way to write into the allocation, and to +obtain slices within the allocation. + +| Complexity: Beginner +| Contact: Abdiel Janulgue + +Generic register abstraction [REGA] +----------------------------------- Work out how register constants and structures can be automatically generated through generalized macros. @@ -102,16 +124,40 @@ Usage: let boot0 = Boot0::read(&bar); pr_info!("Revision: {}\n", boot0.revision()); -Note: a work-in-progress implementation currently resides in +A work-in-progress implementation currently resides in `drivers/gpu/nova-core/regs/macros.rs` and is used in nova-core. It would be nice to improve it (possibly using proc macros) and move it to the `kernel` crate so it can be used by other components as well. +Features desired before this happens: + +* Relative register with build-time base address validation, +* Arrays of registers with build-time index validation, +* Make I/O optional I/O (for field values that are not registers), +* Support other sizes than `u32`, +* Allow visibility control for registers and individual fields, +* Use Rust slice syntax to express fields ranges. + | Complexity: Advanced | Contact: Alexandre Courbot -Delay / Sleep abstractions --------------------------- +Numerical operations [NUMM] +--------------------------- + +Nova uses integer operations that are not part of the standard library (or not +implemented in an optimized way for the kernel). These include: + +- Aligning up and down to a power of two, +- The "Find Last Set Bit" (`fls` function of the C part of the kernel) + operation. + +A `num` core kernel module is being designed to provide these operations. + +| Complexity: Intermediate +| Contact: Alexandre Courbot + +Delay / Sleep abstractions [DLAY] +--------------------------------- Rust abstractions for the kernel's delay() and sleep() functions. @@ -159,18 +205,6 @@ mailing list yet. | Complexity: Intermediate | Contact: Abdiel Janulgue -ELF utils ---------- - -Rust implementation of ELF header representation to retrieve section header -tables, names, and data from an ELF-formatted images. - -There is preceding work from Abdiel Janulgue, which hasn't made it to the -mailing list yet. - -| Complexity: Beginner -| Contact: Abdiel Janulgue - PCI MISC APIs ------------- @@ -179,12 +213,11 @@ capability, MSI API abstractions. | Complexity: Beginner -Auxiliary bus abstractions --------------------------- - -Rust abstraction for the auxiliary bus APIs. +XArray bindings [XARR] +---------------------- -This is needed to connect nova-core to the nova-drm driver. +We need bindings for `xa_alloc`/`xa_alloc_cyclic` in order to generate the +auxiliary device IDs. | Complexity: Intermediate @@ -216,15 +249,6 @@ Build the radix3 page table to map the firmware. | Complexity: Intermediate | Contact: Abdiel Janulgue -vBIOS support -------------- - -Parse the vBIOS and probe the structures required for driver initialization. - -| Contact: Dave Airlie -| Reference: Vec extensions -| Complexity: Intermediate - Initial Devinit support ----------------------- @@ -234,23 +258,6 @@ configuration. | Contact: Dave Airlie | Complexity: Beginner -Boot Falcon controller ----------------------- - -Infrastructure to load and execute falcon (sec2) firmware images; handle the -GSP falcon processor and fwsec loading. - -| Complexity: Advanced -| Contact: Dave Airlie - -GPU Timer support ------------------ - -Support for the GPU's internal timer peripheral. - -| Complexity: Beginner -| Contact: Dave Airlie - MMU / PT management ------------------- diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs index 1f1f8c378d8e2..94f44bcfd748d 100644 --- a/drivers/gpu/nova-core/dma.rs +++ b/drivers/gpu/nova-core/dma.rs @@ -26,7 +26,7 @@ impl DmaObject { pub(crate) fn from_data(dev: &device::Device, data: &[u8]) -> Result { Self::new(dev, data.len()).map(|mut dma_obj| { - // TODO: replace with `CoherentAllocation::write()` once available. + // TODO[COHA]: replace with `CoherentAllocation::write()` once available. // SAFETY: // - `dma_obj`'s size is at least `data.len()`. // - We have just created this object and there is no other user at this stage. diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs index ffe25c7a2fdad..518ef8739550f 100644 --- a/drivers/gpu/nova-core/driver.rs +++ b/drivers/gpu/nova-core/driver.rs @@ -42,7 +42,7 @@ impl pci::Driver for NovaCore { _reg: auxiliary::Registration::new( pdev.as_ref(), c_str!("nova-drm"), - 0, // TODO: Once it lands, use XArray; for now we don't use the ID. + 0, // TODO[XARR]: Once it lands, use XArray; for now we don't use the ID. crate::MODULE_NAME )?, }), diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs index fe4d3d458a6b1..07be1c30668c4 100644 --- a/drivers/gpu/nova-core/falcon.rs +++ b/drivers/gpu/nova-core/falcon.rs @@ -35,6 +35,7 @@ pub(crate) enum FalconCoreRev { Rev7 = 7, } +// TODO[FPRI]: replace with `FromPrimitive`. impl TryFrom for FalconCoreRev { type Error = Error; @@ -68,6 +69,7 @@ pub(crate) enum FalconCoreRevSubversion { Subversion3 = 3, } +// TODO[FPRI]: replace with `FromPrimitive`. impl TryFrom for FalconCoreRevSubversion { type Error = Error; @@ -101,6 +103,7 @@ pub(crate) enum FalconSecurityModel { Heavy = 3, } +// TODO[FPRI]: replace with `FromPrimitive`. impl TryFrom for FalconSecurityModel { type Error = Error; @@ -128,6 +131,7 @@ pub(crate) enum FalconModSelAlgo { Rsa3k = 1, } +// TODO[FPRI]: replace with `FromPrimitive`. impl TryFrom for FalconModSelAlgo { type Error = Error; @@ -148,6 +152,7 @@ pub(crate) enum DmaTrfCmdSize { Size256B = 0x6, } +// TODO[FPRI]: replace with `FromPrimitive`. impl TryFrom for DmaTrfCmdSize { type Error = Error; @@ -199,6 +204,7 @@ pub(crate) enum FalconFbifTarget { NoncoherentSysmem = 2, } +// TODO[FPRI]: replace with `FromPrimitive`. impl TryFrom for FalconFbifTarget { type Error = Error; @@ -354,7 +360,7 @@ impl Falcon { regs::NV_PFALCON_FALCON_ENGINE::alter(bar, E::BASE, |v| v.set_reset(true)); - // TODO: replace with udelay() or equivalent once available. + // TODO[DLAY]: replace with udelay() or equivalent once available. // TIMEOUT: falcon engine should not take more than 10us to reset. let _: Result = util::wait_on(Duration::from_micros(10), || None); diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs index 0a4e5e7adf8cb..664327f75cf41 100644 --- a/drivers/gpu/nova-core/falcon/hal/ga102.rs +++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs @@ -42,10 +42,10 @@ fn signature_reg_fuse_version_ga102( engine_id_mask: u16, ucode_id: u8, ) -> Result { - // TODO: The ucode fuse versions are contained in the FUSE_OPT_FPF__UCODE_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. Clean this up once we support - // register arrays. + // TODO[REGA]: The ucode fuse versions are contained in the + // FUSE_OPT_FPF__UCODE_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. Clean this up once we support register arrays. // Each engine has 16 ucode version registers numbered from 1 to 16. if ucode_id == 0 || ucode_id > 16 { @@ -69,7 +69,7 @@ fn signature_reg_fuse_version_ga102( let reg_fuse_version = bar.read32(reg_fuse_base + ((ucode_id - 1) as usize * core::mem::size_of::())); - // TODO: replace with `last_set_bit` once it lands. + // TODO[NUMM]: replace with `last_set_bit` once it lands. Ok(u32::BITS - reg_fuse_version.leading_zeros()) } diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs index 5719de5c47593..172b4a12ba2af 100644 --- a/drivers/gpu/nova-core/fb.rs +++ b/drivers/gpu/nova-core/fb.rs @@ -122,7 +122,7 @@ impl FbLayout { let frts = { const FRTS_DOWN_ALIGN: u64 = SZ_128K as u64; const FRTS_SIZE: u64 = SZ_1M as u64; - // TODO: replace with `align_down` once it lands. + // TODO[NUMM]: replace with `align_down` once it lands. let frts_base = (vga_workspace.start & !(FRTS_DOWN_ALIGN - 1)) - FRTS_SIZE; frts_base..frts_base + FRTS_SIZE diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs index 6058598ce76e2..047aab76470ec 100644 --- a/drivers/gpu/nova-core/firmware/fwsec.rs +++ b/drivers/gpu/nova-core/firmware/fwsec.rs @@ -150,8 +150,8 @@ impl FirmwareSignature for Bcrt30Rsa3kSignature {} /// Callers must ensure that the region of memory returned is not written for as long as the /// returned reference is alive. /// -/// TODO: Remove this and `transmute_mut` once `CoherentAllocation::as_slice` is available and we -/// have a way to transmute objects implementing FromBytes, e.g.: +/// TODO[TRSM][COHA]: Remove this and `transmute_mut` once `CoherentAllocation::as_slice` is +/// available and we have a way to transmute objects implementing FromBytes, e.g.: /// https://lore.kernel.org/lkml/20250330234039.29814-1-christiansantoslima21@gmail.com/ unsafe fn transmute<'a, 'b, T: Sized + FromBytes>( fw: &'a DmaObject, @@ -218,7 +218,7 @@ impl FalconLoadParams for FwsecFirmware { FalconLoadTarget { src_start: self.desc.imem_load_size, dst_start: self.desc.dmem_phys_base, - // TODO: replace with `align_up` once it lands. + // TODO[NUMM]: replace with `align_up` once it lands. len: self .desc .dmem_load_size diff --git a/drivers/gpu/nova-core/gfw.rs b/drivers/gpu/nova-core/gfw.rs index 937e820e00fca..ce03ac9f4d9d6 100644 --- a/drivers/gpu/nova-core/gfw.rs +++ b/drivers/gpu/nova-core/gfw.rs @@ -29,7 +29,7 @@ pub(crate) fn wait_gfw_boot_completion(bar: &Bar0) -> Result { if gfw_booted { Some(()) } else { - // TODO: replace with [1] once merged. + // TODO[DLAY]: replace with [1] once it merges. // [1] https://lore.kernel.org/rust-for-linux/20250423192857.199712-6-fujita.tomonori@gmail.com/ // // SAFETY: `msleep()` is safe to call with any parameter. diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 43c8120559a76..8e32af16b669c 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -37,7 +37,7 @@ macro_rules! define_chipset { ]; } - // TODO replace with something like derive(FromPrimitive) + // TODO[FPRI]: replace with something like derive(FromPrimitive) impl TryFrom for Chipset { type Error = kernel::error::Error; diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs index ccfaeed55cff9..707f87d6828df 100644 --- a/drivers/gpu/nova-core/regs.rs +++ b/drivers/gpu/nova-core/regs.rs @@ -44,7 +44,7 @@ impl NV_PMC_BOOT_0 { /* PBUS */ -// TODO: this is an array of registers. +// TODO[REGA]: this is an array of registers. register!(NV_PBUS_SW_SCRATCH_0E@0x00001438 { 31:16 frts_err_code as u16; }); @@ -110,7 +110,7 @@ register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK @ 0x00118128 { 0:0 read_protection_level0 as bool, "Set after FWSEC lowers its protection level"; }); -// TODO: This is an array of registers. +// TODO[REGA]: This is an array of registers. register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05 @ 0x00118234 { 31:0 value as u32; }); @@ -272,7 +272,7 @@ register!(NV_PFALCON_FALCON_ENGINE @ +0x000003c0 { 0:0 reset as bool; }); -// TODO: this is an array of registers. +// TODO[REGA]: this is an array of registers. register!(NV_PFALCON_FBIF_TRANSCFG @ +0x00000600 { 1:0 target as u8 ?=> FalconFbifTarget; 2:2 mem_type as bool => FalconFbifMemType; @@ -294,7 +294,7 @@ register!(NV_PFALCON2_FALCON_BROM_ENGIDMASK @ +0x0000119c { 31:0 value as u32; }); -// TODO: this is an array of registers. +// TODO[REGA]: this is an array of registers. register!(NV_PFALCON2_FALCON_BROM_PARAADDR @ +0x00001210 { 31:0 value as u32; }); diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs index e0e6fef3796f9..cdf668073480e 100644 --- a/drivers/gpu/nova-core/regs/macros.rs +++ b/drivers/gpu/nova-core/regs/macros.rs @@ -147,7 +147,7 @@ macro_rules! register { pub(crate) const OFFSET: usize = $offset; } - // TODO: display the raw hex value, then the value of all the fields. This requires + // TODO[REGA]: display the raw hex value, then the value of all the fields. This requires // matching the fields, which will complexify the syntax considerably... impl ::core::fmt::Debug for $name { fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-core/util.rs index 69f29238b25ed..5cafe0797cd6f 100644 --- a/drivers/gpu/nova-core/util.rs +++ b/drivers/gpu/nova-core/util.rs @@ -32,7 +32,7 @@ pub(crate) const fn const_bytes_to_str(bytes: &[u8]) -> &str { /// `Err(ETIMEDOUT)` is returned if `timeout` has been reached without `cond` evaluating to /// `Some`. /// -/// TODO: replace with `read_poll_timeout` once it is available. +/// TODO[DLAY]: replace with `read_poll_timeout` once it is available. /// (https://lore.kernel.org/lkml/20250220070611.214262-8-fujita.tomonori@gmail.com/) pub(crate) fn wait_on Option>(timeout: Duration, cond: F) -> Result { let start_time = Instant::now(); diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs index 0b47ddb057447..feb80c8470778 100644 --- a/drivers/gpu/nova-core/vbios.rs +++ b/drivers/gpu/nova-core/vbios.rs @@ -177,7 +177,7 @@ impl<'a> Iterator for VbiosIterator<'a> { // Advance to next image (aligned to 512 bytes). self.current_offset += image_size; - // TODO: replace with `align_up` once it lands. + // TODO[NUMM]: replace with `align_up` once it lands. self.current_offset = self.current_offset.next_multiple_of(512); Some(Ok(full_image)) -- 2.47.2