[PATCH 0/7] gpu: nova-core: remove use of `as` for integer conversions

[PATCH 0/7] gpu: nova-core: remove use of `as` for integer conversions

[Alexandre Courbot]

Using the `as` operator for integer conversions is discouraged, as it
silently strips data if the destination type is smaller than the source.
Many such conversions can be replaced with `from`/`into` or (when
justified) `try_from`/`try_into`, but these traits cannot unfortunately
cover all conversions satisfyingly.

There is for instance the case of converting a `usize` to `u64`, which,
in the case of the kernel today, is completely lossless but cannot be
done because the Rust standard library does not provide a `From`
implementation for conversions that are not future-proof.

Still, in the kernel it is very practical to be able to perform such
conversions when they are safe to do for the current build target.

This patchset tries to eradicate the use of `as` in nova-core, by using
existing means and introducing new ones.

The first 4 patches use the already-available `From` and `TryFrom` trait
where it is possible or advisable.

The fifth patch introduces a new module that proposes conversion
functions for those that are infallible under the current build target.
This is done through a set of const functions, and the `FromAs` and
`IntoAs` extension traits which, as their names lightly suggest, offer
conversion for those types on which the `as` operator can be used
losslessly.

This new module is put to use in the sixth patch.

The idea was first suggested by Danilo, and I hope I captured it
properly. :)

As Danilo suggested, this could eventually find its place in the kernel
crate if the implementation is deemed to be fit, but for now let's
review and let it mature in nova-core.

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/DDK4KADWJHMG.1FUPL3SDR26XF@kernel.org/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Alexandre Courbot (7):
      gpu: nova-core: replace `as` with `from` conversions where possible
      gpu: nova-core: vbios: remove unneeded u8 conversions
      gpu: nova-core: vbios: add conversion to u8 for BiosImageType
      gpu: nova-core: use `try_from` instead of `as` for u32 conversions
      gpu: nova-core: add extra conversion functions and traits
      gpu: nova-core: replace use of `as` with functions from `num`
      gpu: nova-core: justify remaining uses of `as`

 drivers/gpu/nova-core/falcon.rs           |   9 +-
 drivers/gpu/nova-core/falcon/hal/ga102.rs |   6 +-
 drivers/gpu/nova-core/fb.rs               |   7 +-
 drivers/gpu/nova-core/fb/hal/ga100.rs     |   4 +
 drivers/gpu/nova-core/fb/hal/tu102.rs     |  16 ++-
 drivers/gpu/nova-core/firmware.rs         |   7 +-
 drivers/gpu/nova-core/firmware/booter.rs  |  31 +++---
 drivers/gpu/nova-core/firmware/fwsec.rs   |  23 ++---
 drivers/gpu/nova-core/firmware/gsp.rs     |   5 +-
 drivers/gpu/nova-core/firmware/riscv.rs   |   7 +-
 drivers/gpu/nova-core/nova_core.rs        |   1 +
 drivers/gpu/nova-core/num.rs              | 159 ++++++++++++++++++++++++++++++
 drivers/gpu/nova-core/regs.rs             |   5 +-
 drivers/gpu/nova-core/vbios.rs            |  60 ++++++-----
 14 files changed, 261 insertions(+), 79 deletions(-)
---
base-commit: ca16b15e78f4dee1631c0a68693f5e7d9b3bb3ec
change-id: 20251022-nova-as-8ece4142872a

Best regards,
-- 
Alexandre Courbot <acourbot@nvidia.com>

[PATCH 1/7] gpu: nova-core: replace `as` with `from` conversions where possible

[Alexandre Courbot]

The `as` operator is best avoided as it silently drops bits if the
destination type is smaller that the source.

For data types where this is clearly not the case, use `from` to
unambiguously signal that these conversions are lossless.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/falcon/hal/ga102.rs |  6 ++---
 drivers/gpu/nova-core/firmware/fwsec.rs   |  4 ++--
 drivers/gpu/nova-core/vbios.rs            | 38 +++++++++++++++----------------
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index f2ae9537321d..afed353b24d2 100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -40,11 +40,9 @@ fn signature_reg_fuse_version_ga102(
     engine_id_mask: u16,
     ucode_id: u8,
 ) -> Result<u32> {
-    const NV_FUSE_OPT_FPF_SIZE: u8 = regs::NV_FUSE_OPT_FPF_SIZE as u8;
-
     // Each engine has 16 ucode version registers numbered from 1 to 16.
-    let ucode_idx = match ucode_id {
-        1..=NV_FUSE_OPT_FPF_SIZE => (ucode_id - 1) as usize,
+    let ucode_idx = match usize::from(ucode_id) {
+        ucode_id @ 1..=regs::NV_FUSE_OPT_FPF_SIZE => ucode_id - 1,
         _ => {
             dev_err!(dev, "invalid ucode id {:#x}", ucode_id);
             return Err(EINVAL);
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index 8edbb5c0572c..dd3420aaa2bf 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -259,13 +259,13 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
         }
 
         // Find the DMEM mapper section in the firmware.
-        for i in 0..hdr.entry_count as usize {
+        for i in 0..usize::from(hdr.entry_count) {
             let app: &FalconAppifV1 =
             // SAFETY: we have exclusive access to `dma_object`.
             unsafe {
                 transmute(
                     &dma_object,
-                    hdr_offset + hdr.header_size as usize + i * hdr.entry_size as usize
+                    hdr_offset + usize::from(hdr.header_size) + i * usize::from(hdr.entry_size)
                 )
             }?;
 
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index ad070a0420ca..943b0dac31df 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -361,7 +361,7 @@ fn is_last(&self) -> bool {
 
     /// Calculate image size in bytes from 512-byte blocks.
     fn image_size_bytes(&self) -> usize {
-        self.image_len as usize * 512
+        usize::from(self.image_len) * 512
     }
 }
 
@@ -439,13 +439,13 @@ fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> {
         let header = &image.bit_header;
 
         // Offset to the first token entry
-        let tokens_start = image.bit_offset + header.header_size as usize;
+        let tokens_start = image.bit_offset + usize::from(header.header_size);
 
-        for i in 0..header.token_entries as usize {
-            let entry_offset = tokens_start + (i * header.token_size as usize);
+        for i in 0..usize::from(header.token_entries) {
+            let entry_offset = tokens_start + (i * usize::from(header.token_size));
 
             // Make sure we don't go out of bounds
-            if entry_offset + header.token_size as usize > image.base.data.len() {
+            if entry_offset + usize::from(header.token_size) > image.base.data.len() {
                 return Err(EINVAL);
             }
 
@@ -601,7 +601,7 @@ fn is_last(&self) -> bool {
 
     /// Calculate image size in bytes from 512-byte blocks.
     fn image_size_bytes(&self) -> usize {
-        self.subimage_len as usize * 512
+        usize::from(self.subimage_len) * 512
     }
 
     /// Try to find NPDE in the data, the NPDE is right after the PCIR.
@@ -613,8 +613,8 @@ fn find_in_data(
     ) -> Option<Self> {
         // Calculate the offset where NPDE might be located
         // NPDE should be right after the PCIR structure, aligned to 16 bytes
-        let pcir_offset = rom_header.pci_data_struct_offset as usize;
-        let npde_start = (pcir_offset + pcir.pci_data_struct_len as usize + 0x0F) & !0x0F;
+        let pcir_offset = usize::from(rom_header.pci_data_struct_offset);
+        let npde_start = (pcir_offset + usize::from(pcir.pci_data_struct_len) + 0x0F) & !0x0F;
 
         // Check if we have enough data
         if npde_start + core::mem::size_of::<Self>() > data.len() {
@@ -737,7 +737,7 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
             .inspect_err(|e| dev_err!(dev, "Failed to create PciRomHeader: {:?}\n", e))?;
 
         // Get the PCI Data Structure using the pointer from the ROM header.
-        let pcir_offset = rom_header.pci_data_struct_offset as usize;
+        let pcir_offset = usize::from(rom_header.pci_data_struct_offset);
         let pcir_data = data
             .get(pcir_offset..pcir_offset + core::mem::size_of::<PcirStruct>())
             .ok_or(EINVAL)
@@ -805,12 +805,12 @@ fn falcon_data_ptr(&self) -> Result<u32> {
         let token = self.get_bit_token(BIT_TOKEN_ID_FALCON_DATA)?;
 
         // Make sure we don't go out of bounds
-        if token.data_offset as usize + 4 > self.base.data.len() {
+        if usize::from(token.data_offset) + 4 > self.base.data.len() {
             return Err(EINVAL);
         }
 
         // read the 4 bytes at the offset specified in the token
-        let offset = token.data_offset as usize;
+        let offset = usize::from(token.data_offset);
         let bytes: [u8; 4] = self.base.data[offset..offset + 4].try_into().map_err(|_| {
             dev_err!(self.base.dev, "Failed to convert data slice to array");
             EINVAL
@@ -818,7 +818,7 @@ fn falcon_data_ptr(&self) -> Result<u32> {
 
         let data_ptr = u32::from_le_bytes(bytes);
 
-        if (data_ptr as usize) < self.base.data.len() {
+        if (usize::from_u32(data_ptr)) < self.base.data.len() {
             dev_err!(self.base.dev, "Falcon data pointer out of bounds\n");
             return Err(EINVAL);
         }
@@ -886,9 +886,9 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
             return Err(EINVAL);
         }
 
-        let header_len = data[1] as usize;
-        let entry_len = data[2] as usize;
-        let entry_count = data[3] as usize;
+        let header_len = usize::from(data[1]);
+        let entry_len = usize::from(data[2]);
+        let entry_count = usize::from(data[3]);
 
         let required_bytes = header_len + (entry_count * entry_len);
 
@@ -923,7 +923,7 @@ fn lookup_index(&self, idx: u8) -> Result<PmuLookupTableEntry> {
             return Err(EINVAL);
         }
 
-        let index = (idx as usize) * self.entry_len as usize;
+        let index = (usize::from(idx)) * usize::from(self.entry_len);
         PmuLookupTableEntry::new(&self.table_data[index..])
     }
 
@@ -1092,8 +1092,8 @@ pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) -> Result<&[u8]> {
     pub(crate) fn sigs(&self, desc: &FalconUCodeDescV3) -> Result<&[Bcrt30Rsa3kSignature]> {
         // The signatures data follows the descriptor.
         let sigs_data_offset = self.falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>();
-        let sigs_size =
-            desc.signature_count as usize * core::mem::size_of::<Bcrt30Rsa3kSignature>();
+        let sigs_count = usize::from(desc.signature_count);
+        let sigs_size = sigs_count * core::mem::size_of::<Bcrt30Rsa3kSignature>();
 
         // Make sure the data is within bounds.
         if sigs_data_offset + sigs_size > self.base.data.len() {
@@ -1113,7 +1113,7 @@ pub(crate) fn sigs(&self, desc: &FalconUCodeDescV3) -> Result<&[Bcrt30Rsa3kSigna
                     .as_ptr()
                     .add(sigs_data_offset)
                     .cast::<Bcrt30Rsa3kSignature>(),
-                desc.signature_count as usize,
+                sigs_count,
             )
         })
     }

-- 
2.51.0

[PATCH 2/7] gpu: nova-core: vbios: remove unneeded u8 conversions

[Alexandre Courbot]

These variables were read from the u8 array `data` and converted to a
`usize`, before being converted back to a `u8`. Just re-read them from
the source to avoid using `as`.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 943b0dac31df..dbe0d6e4a015 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -911,9 +911,9 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
 
         Ok(PmuLookupTable {
             version: data[0],
-            header_len: header_len as u8,
-            entry_len: entry_len as u8,
-            entry_count: entry_count as u8,
+            header_len: data[1],
+            entry_len: data[2],
+            entry_count: data[3],
             table_data,
         })
     }

-- 
2.51.0

[PATCH 3/7] gpu: nova-core: vbios: add conversion to u8 for BiosImageType

[Alexandre Courbot]

Since `BiosImageType` is `repr(u8)`, if can safely be converted into a
`u8` but this is not obvious when doing this in the code.

Instead, implement `From<BiosImageType>` for `u8` so the cast can be
done in a single place, with a justifying comment.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index dbe0d6e4a015..a521c0a4df0f 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -50,6 +50,13 @@ fn try_from(code: u8) -> Result<Self> {
     }
 }
 
+impl From<BiosImageType> for u8 {
+    fn from(value: BiosImageType) -> Self {
+        // `BiosImageType` is `repr(u8)` and thus convertible without loss.
+        value as u8
+    }
+}
+
 // PMU lookup table entry types. Used to locate PMU table entries
 // in the Fwsec image, corresponding to falcon ucodes.
 #[expect(dead_code)]
@@ -711,7 +718,7 @@ fn image_type(&self) -> Result<BiosImageType> {
     fn is_last(&self) -> bool {
         // For NBSI images (type == 0x70), return true as they're
         // considered the last image
-        if self.pcir.code_type == BiosImageType::Nbsi as u8 {
+        if self.pcir.code_type == BiosImageType::Nbsi.into() {
             return true;
         }
 

-- 
2.51.0

[PATCH 4/7] gpu: nova-core: use `try_from` instead of `as` for u32 conversions

[Alexandre Courbot]

There are a few situations in the driver where we convert a `usize` into
a `u32` using `as`. Even though most of these are obviously correct, use
`try_from` and let the compiler optimize wherever it is safe to do so.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/fb/hal/tu102.rs   | 16 +++++++---------
 drivers/gpu/nova-core/firmware/fwsec.rs |  8 ++++----
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/nova-core/fb/hal/tu102.rs b/drivers/gpu/nova-core/fb/hal/tu102.rs
index b022c781caf4..32114c3b3686 100644
--- a/drivers/gpu/nova-core/fb/hal/tu102.rs
+++ b/drivers/gpu/nova-core/fb/hal/tu102.rs
@@ -15,15 +15,13 @@ pub(super) fn read_sysmem_flush_page_gm107(bar: &Bar0) -> u64 {
 
 pub(super) fn write_sysmem_flush_page_gm107(bar: &Bar0, addr: u64) -> Result {
     // Check that the address doesn't overflow the receiving 32-bit register.
-    if addr >> (u32::BITS + FLUSH_SYSMEM_ADDR_SHIFT) == 0 {
-        regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::default()
-            .set_adr_39_08((addr >> FLUSH_SYSMEM_ADDR_SHIFT) as u32)
-            .write(bar);
-
-        Ok(())
-    } else {
-        Err(EINVAL)
-    }
+    u32::try_from(addr >> FLUSH_SYSMEM_ADDR_SHIFT)
+        .map_err(|_| EINVAL)
+        .map(|addr| {
+            regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::default()
+                .set_adr_39_08(addr)
+                .write(bar)
+        })
 }
 
 pub(super) fn display_enabled_gm107(bar: &Bar0) -> bool {
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index dd3420aaa2bf..ce78c1563754 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -291,7 +291,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
 
             frts_cmd.read_vbios = ReadVbios {
                 ver: 1,
-                hdr: size_of::<ReadVbios>() as u32,
+                hdr: u32::try_from(size_of::<ReadVbios>())?,
                 addr: 0,
                 size: 0,
                 flags: 2,
@@ -304,9 +304,9 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
                 } => {
                     frts_cmd.frts_region = FrtsRegion {
                         ver: 1,
-                        hdr: size_of::<FrtsRegion>() as u32,
-                        addr: (frts_addr >> 12) as u32,
-                        size: (frts_size >> 12) as u32,
+                        hdr: u32::try_from(size_of::<FrtsRegion>())?,
+                        addr: u32::try_from(frts_addr >> 12)?,
+                        size: u32::try_from(frts_size >> 12)?,
                         ftype: NVFW_FRTS_CMD_REGION_TYPE_FB,
                     };
 

-- 
2.51.0

[PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Alexandre Courbot]

The core library's `From` implementations do not cover conversions
that are not portable or future-proof. For instance, even though it is
safe today, `From<usize>` is not implemented for `u64` because of the
possibility to support larger-than-64bit architectures in the future.

However, the kernel supports a narrower set of architectures, and in the
case of Nova we only support 64-bit. This makes it helpful and desirable
to provide more infallible conversions, lest we need to rely on the `as`
keyword and carry the risk of silently losing data.

Thus, introduce a new module `num` that provides safe const functions
performing more conversions allowed by the build target, as well as
`FromAs` and `IntoAs` traits that are just extensions of `From` and
`Into` to conversions that are known to be lossless.

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/DDK4KADWJHMG.1FUPL3SDR26XF@kernel.org/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/nova_core.rs |   1 +
 drivers/gpu/nova-core/num.rs       | 159 +++++++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+)

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index e130166c1086..9180ec9c27ef 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -13,6 +13,7 @@
 mod gfw;
 mod gpu;
 mod gsp;
+mod num;
 mod regs;
 mod vbios;
 
diff --git a/drivers/gpu/nova-core/num.rs b/drivers/gpu/nova-core/num.rs
new file mode 100644
index 000000000000..6b9d4e89dbf5
--- /dev/null
+++ b/drivers/gpu/nova-core/num.rs
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Numerical helpers functions and traits.
+//!
+//! This is essentially a staging module for code to mature until it can be moved to the `kernel`
+//! crate.
+
+/// Infallibly converts a `usize` to `u64`.
+///
+/// This conversion is always lossless as Linux only supports 32-bit and 64-bit platforms, thus a
+/// `usize` is always smaller than or of the same size as a `u64`.
+///
+/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
+///
+/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
+/// [`IntoAs`] traits.
+pub(crate) const fn usize_as_u64(value: usize) -> u64 {
+    kernel::static_assert!(size_of::<u64>() >= size_of::<usize>());
+
+    value as u64
+}
+
+#[cfg(CONFIG_32BIT)]
+/// Infallibly converts a `usize` to `u32` on 32-bit platforms.
+///
+/// This conversion is always lossless on 32-bit platforms, where a `usize` is the same size as a
+/// `u32`.
+///
+/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
+///
+/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
+/// [`IntoAs`] traits.
+pub(crate) const fn usize_as_u32(value: usize) -> u32 {
+    kernel::static_assert!(size_of::<u32>() >= size_of::<usize>());
+
+    value as u32
+}
+
+/// Infallibly converts a `u32` to `usize`.
+///
+/// This conversion is always lossless as Linux only supports 32-bit and 64-bit platforms, thus a
+/// `u32` is always smaller than or of the same size as a `usize`.
+///
+/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
+///
+/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
+/// [`IntoAs`] traits.
+pub(crate) const fn u32_as_usize(value: u32) -> usize {
+    kernel::static_assert!(size_of::<usize>() >= size_of::<u32>());
+
+    value as usize
+}
+
+#[cfg(CONFIG_64BIT)]
+/// Infallibly converts a `u64` to `usize` on 64-bit platforms.
+///
+/// This conversion is always lossless on 64-bit platforms, where a `usize` is the same size as a
+/// `u64`.
+///
+/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
+///
+/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
+/// [`IntoAs`] traits.
+pub(crate) const fn u64_as_usize(value: u64) -> usize {
+    kernel::static_assert!(size_of::<usize>() >= size_of::<u64>());
+
+    value as usize
+}
+
+/// Extension trait providing guaranteed lossless conversion to `Self` from `T`.
+///
+/// The standard library's `From` implementations do not cover conversions that are not portable or
+/// future-proof. For instance, even though it is safe today, `From<usize>` is not implemented for
+/// `u64` because of the possibility to support larger-than-64bit architectures in the future.
+///
+/// The workaround is to either deal with the error handling of `TryFrom` for an operation that
+/// technically cannot fail, or to use the `as` keyword, which can silently strip data if the
+/// destination type is smaller than the source.
+///
+/// Both options are hardly acceptable for the kernel. It is also a much more architecture
+/// dependent environment, supporting only 32 and 64 bit architectures, with some modules
+/// explicitly depending on a specific bus witdth that could greatly benefit from infallible
+/// conversion operations.
+///
+/// Thus this extension trait that provides, for the architecture the kernel is built for, safe
+/// conversion between types for which such conversion is lossless.
+///
+/// In other words, this trait is implemented if, for the current build target and with `t: T`, the
+/// `t as Self` operation is completely lossless.
+///
+/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
+///
+/// If you need to perform a conversion in `const` context, use [`u64_as_usize`],
+/// [`u32_as_usize`], [`usize_as_u64`], or [`usize_as_u32`].
+///
+/// # Examples
+///
+/// ```
+/// use crate::num::FromAs;
+///
+/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
+/// ```
+pub(crate) trait FromAs<T> {
+    /// Create a `Self` from `value`. This operation is guaranteed to be lossless.
+    fn from_as(value: T) -> Self;
+}
+
+impl FromAs<usize> for u64 {
+    fn from_as(value: usize) -> Self {
+        usize_as_u64(value)
+    }
+}
+
+#[cfg(CONFIG_32BIT)]
+impl FromAs<usize> for u32 {
+    fn from_as(value: usize) -> Self {
+        usize_as_u32(value)
+    }
+}
+
+impl FromAs<u32> for usize {
+    fn from_as(value: u32) -> Self {
+        u32_as_usize(value)
+    }
+}
+
+#[cfg(CONFIG_64BIT)]
+impl FromAs<u64> for usize {
+    fn from_as(value: u64) -> Self {
+        u64_as_usize(value)
+    }
+}
+
+/// Counterpart to the [`FromAs`] trait, i.e. this trait is to [`FromAs`] what [`Into`] is to
+/// [`From`].
+///
+/// See the documentation of [`FromAs`] for the motivation.
+///
+/// # Examples
+///
+/// ```
+/// use crate::num::IntoAs;
+///
+/// assert_eq!(0xf00u32.into_as(), 0xf00u32 as usize);
+/// ```
+pub(crate) trait IntoAs<T> {
+    /// Convert `self` into a `T`. This operation is guaranteed to be lossless.
+    fn into_as(self) -> T;
+}
+
+/// Reverse operation for types implementing [`FromAs`].
+impl<S, T> IntoAs<T> for S
+where
+    T: FromAs<S>,
+{
+    fn into_as(self) -> T {
+        T::from_as(self)
+    }
+}

-- 
2.51.0

[PATCH 6/7] gpu: nova-core: replace use of `as` with functions from `num`

[Alexandre Courbot]

Use the newly-introduced `num` module to replace the use of `as`
wherever it is safe to do. This ensures that a given conversion cannot
lose data if its source or destination type ever changes.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/falcon.rs          |  5 +++--
 drivers/gpu/nova-core/fb.rs              |  7 ++++---
 drivers/gpu/nova-core/firmware.rs        |  7 ++++---
 drivers/gpu/nova-core/firmware/booter.rs | 31 +++++++++++++++++--------------
 drivers/gpu/nova-core/firmware/fwsec.rs  | 11 ++++++-----
 drivers/gpu/nova-core/firmware/gsp.rs    |  5 +++--
 drivers/gpu/nova-core/firmware/riscv.rs  |  7 ++++---
 drivers/gpu/nova-core/regs.rs            |  5 +++--
 drivers/gpu/nova-core/vbios.rs           |  9 +++++----
 9 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index fb3561cc9746..a44df1ac8873 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -15,6 +15,7 @@
 use crate::dma::DmaObject;
 use crate::driver::Bar0;
 use crate::gpu::Chipset;
+use crate::num::{FromAs, IntoAs};
 use crate::regs;
 use crate::regs::macros::RegisterBase;
 
@@ -442,7 +443,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
             FalconMem::Imem => (load_offsets.src_start, fw.dma_handle()),
             FalconMem::Dmem => (
                 0,
-                fw.dma_handle_with_offset(load_offsets.src_start as usize)?,
+                fw.dma_handle_with_offset(load_offsets.src_start.into_as())?,
             ),
         };
         if dma_start % DmaAddress::from(DMA_LEN) > 0 {
@@ -468,7 +469,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
                 dev_err!(self.dev, "DMA transfer length overflow");
                 return Err(EOVERFLOW);
             }
-            Some(upper_bound) if upper_bound as usize > fw.size() => {
+            Some(upper_bound) if usize::from_as(upper_bound) > fw.size() => {
                 dev_err!(self.dev, "DMA transfer goes beyond range of DMA object");
                 return Err(EINVAL);
             }
diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 27d9edab8347..1461dd643cae 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -11,6 +11,7 @@
 use crate::dma::DmaObject;
 use crate::driver::Bar0;
 use crate::gpu::Chipset;
+use crate::num::usize_as_u64;
 use crate::regs;
 
 mod hal;
@@ -105,14 +106,14 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0) -> Result<Self> {
 
         let vga_workspace = {
             let vga_base = {
-                const NV_PRAMIN_SIZE: u64 = SZ_1M as u64;
+                const NV_PRAMIN_SIZE: u64 = usize_as_u64(SZ_1M);
                 let base = fb.end - NV_PRAMIN_SIZE;
 
                 if hal.supports_display(bar) {
                     match regs::NV_PDISP_VGA_WORKSPACE_BASE::read(bar).vga_workspace_addr() {
                         Some(addr) => {
                             if addr < base {
-                                const VBIOS_WORKSPACE_SIZE: u64 = SZ_128K as u64;
+                                const VBIOS_WORKSPACE_SIZE: u64 = usize_as_u64(SZ_128K);
 
                                 // Point workspace address to end of framebuffer.
                                 fb.end - VBIOS_WORKSPACE_SIZE
@@ -132,7 +133,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0) -> Result<Self> {
 
         let frts = {
             const FRTS_DOWN_ALIGN: Alignment = Alignment::new::<SZ_128K>();
-            const FRTS_SIZE: u64 = SZ_1M as u64;
+            const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
             let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
 
             frts_base..frts_base + FRTS_SIZE
diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 4179a74a2342..ae6dbefd9e5a 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -15,6 +15,7 @@
 use crate::dma::DmaObject;
 use crate::falcon::FalconFirmware;
 use crate::gpu;
+use crate::num::{FromAs, IntoAs};
 
 pub(crate) mod booter;
 pub(crate) mod fwsec;
@@ -75,7 +76,7 @@ pub(crate) fn size(&self) -> usize {
         const HDR_SIZE_SHIFT: u32 = 16;
         const HDR_SIZE_MASK: u32 = 0xffff0000;
 
-        ((self.hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT) as usize
+        ((self.hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_as()
     }
 }
 
@@ -190,8 +191,8 @@ fn new(fw: &'a firmware::Firmware) -> Result<Self> {
     /// Returns the data payload of the firmware, or `None` if the data range is out of bounds of
     /// the firmware image.
     fn data(&self) -> Option<&[u8]> {
-        let fw_start = self.hdr.data_offset as usize;
-        let fw_size = self.hdr.data_size as usize;
+        let fw_start = usize::from_as(self.hdr.data_offset);
+        let fw_size = usize::from_as(self.hdr.data_size);
 
         self.fw.get(fw_start..fw_start + fw_size)
     }
diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index b4ff1b17e4a0..bab66ed85f10 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -18,6 +18,7 @@
 use crate::falcon::{Falcon, FalconBromParams, FalconFirmware, FalconLoadParams, FalconLoadTarget};
 use crate::firmware::{BinFirmware, FirmwareDmaObject, FirmwareSignature, Signed, Unsigned};
 use crate::gpu::Chipset;
+use crate::num::{FromAs, IntoAs};
 
 /// Local convenience function to return a copy of `S` by reinterpreting the bytes starting at
 /// `offset` in `slice`.
@@ -74,7 +75,7 @@ impl<'a> HsFirmwareV2<'a> {
     ///
     /// Fails if the header pointed at by `bin_fw` is not within the bounds of the firmware image.
     fn new(bin_fw: &BinFirmware<'a>) -> Result<Self> {
-        frombytes_at::<HsHeaderV2>(bin_fw.fw, bin_fw.hdr.header_offset as usize)
+        frombytes_at::<HsHeaderV2>(bin_fw.fw, bin_fw.hdr.header_offset.into_as())
             .map(|hdr| Self { hdr, fw: bin_fw.fw })
     }
 
@@ -83,7 +84,7 @@ fn new(bin_fw: &BinFirmware<'a>) -> Result<Self> {
     /// Fails if the offset of the patch location is outside the bounds of the firmware
     /// image.
     fn patch_location(&self) -> Result<u32> {
-        frombytes_at::<u32>(self.fw, self.hdr.patch_loc_offset as usize)
+        frombytes_at::<u32>(self.fw, self.hdr.patch_loc_offset.into_as())
     }
 
     /// Returns an iterator to the signatures of the firmware. The iterator can be empty if the
@@ -91,19 +92,21 @@ fn patch_location(&self) -> Result<u32> {
     ///
     /// Fails if the pointed signatures are outside the bounds of the firmware image.
     fn signatures_iter(&'a self) -> Result<impl Iterator<Item = BooterSignature<'a>>> {
-        let num_sig = frombytes_at::<u32>(self.fw, self.hdr.num_sig_offset as usize)?;
+        let num_sig = frombytes_at::<u32>(self.fw, self.hdr.num_sig_offset.into_as())?;
         let iter = match self.hdr.sig_prod_size.checked_div(num_sig) {
             // If there are no signatures, return an iterator that will yield zero elements.
             None => (&[] as &[u8]).chunks_exact(1),
             Some(sig_size) => {
-                let patch_sig = frombytes_at::<u32>(self.fw, self.hdr.patch_sig_offset as usize)?;
-                let signatures_start = (self.hdr.sig_prod_offset + patch_sig) as usize;
+                let patch_sig = frombytes_at::<u32>(self.fw, self.hdr.patch_sig_offset.into_as())?;
+                let signatures_start = usize::from_as(self.hdr.sig_prod_offset + patch_sig);
 
                 self.fw
                     // Get signatures range.
-                    .get(signatures_start..signatures_start + self.hdr.sig_prod_size as usize)
+                    .get(
+                        signatures_start..signatures_start + usize::from_as(self.hdr.sig_prod_size),
+                    )
                     .ok_or(EINVAL)?
-                    .chunks_exact(sig_size as usize)
+                    .chunks_exact(sig_size.into_as())
             }
         };
 
@@ -132,9 +135,9 @@ impl HsSignatureParams {
     /// Fails if the meta data parameter of `hs_fw` is outside the bounds of the firmware image, or
     /// if its size doesn't match that of [`HsSignatureParams`].
     fn new(hs_fw: &HsFirmwareV2<'_>) -> Result<Self> {
-        let start = hs_fw.hdr.meta_data_offset as usize;
+        let start = usize::from_as(hs_fw.hdr.meta_data_offset);
         let end = start
-            .checked_add(hs_fw.hdr.meta_data_size as usize)
+            .checked_add(hs_fw.hdr.meta_data_size.into_as())
             .ok_or(EINVAL)?;
 
         hs_fw
@@ -169,7 +172,7 @@ impl HsLoadHeaderV2 {
     ///
     /// Fails if the header pointed at by `hs_fw` is not within the bounds of the firmware image.
     fn new(hs_fw: &HsFirmwareV2<'_>) -> Result<Self> {
-        frombytes_at::<Self>(hs_fw.fw, hs_fw.hdr.header_offset as usize)
+        frombytes_at::<Self>(hs_fw.fw, hs_fw.hdr.header_offset.into_as())
     }
 }
 
@@ -198,12 +201,12 @@ fn new(hs_fw: &HsFirmwareV2<'_>, idx: u32) -> Result<Self> {
         } else {
             frombytes_at::<Self>(
                 hs_fw.fw,
-                (hs_fw.hdr.header_offset as usize)
+                usize::from_as(hs_fw.hdr.header_offset)
                     // Skip the load header...
                     .checked_add(size_of::<HsLoadHeaderV2>())
                     // ... and jump to app header `idx`.
                     .and_then(|offset| {
-                        offset.checked_add((idx as usize).checked_mul(size_of::<Self>())?)
+                        offset.checked_add(usize::from_as(idx).checked_mul(size_of::<Self>())?)
                     })
                     .ok_or(EINVAL)?,
             )
@@ -318,12 +321,12 @@ pub(crate) fn new(
                             dev_err!(dev, "invalid fuse version for Booter firmware\n");
                             return Err(EINVAL);
                         };
-                        signatures.nth(idx as usize)
+                        signatures.nth(idx.into_as())
                     }
                 }
                 .ok_or(EINVAL)?;
 
-                ucode.patch_signature(&signature, patch_loc as usize)?
+                ucode.patch_signature(&signature, patch_loc.into_as())?
             }
         };
 
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index ce78c1563754..ed82e74ccdc9 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -23,6 +23,7 @@
 use crate::falcon::gsp::Gsp;
 use crate::falcon::{Falcon, FalconBromParams, FalconFirmware, FalconLoadParams, FalconLoadTarget};
 use crate::firmware::{FalconUCodeDescV3, FirmwareDmaObject, FirmwareSignature, Signed, Unsigned};
+use crate::num::{FromAs, IntoAs};
 use crate::vbios::Vbios;
 
 const NVFW_FALCON_APPIF_ID_DMEMMAPPER: u32 = 0x4;
@@ -250,7 +251,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
         let ucode = bios.fwsec_image().ucode(desc)?;
         let mut dma_object = DmaObject::from_data(dev, ucode)?;
 
-        let hdr_offset = (desc.imem_load_size + desc.interface_offset) as usize;
+        let hdr_offset = usize::from_as(desc.imem_load_size + desc.interface_offset);
         // SAFETY: we have exclusive access to `dma_object`.
         let hdr: &FalconAppifHdrV1 = unsafe { transmute(&dma_object, hdr_offset) }?;
 
@@ -277,7 +278,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
             let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe {
                 transmute_mut(
                     &mut dma_object,
-                    (desc.imem_load_size + app.dmem_base) as usize,
+                    (desc.imem_load_size + app.dmem_base).into_as(),
                 )
             }?;
 
@@ -285,7 +286,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
             let frts_cmd: &mut FrtsCmd = unsafe {
                 transmute_mut(
                     &mut dma_object,
-                    (desc.imem_load_size + dmem_mapper.cmd_in_buffer_offset) as usize,
+                    (desc.imem_load_size + dmem_mapper.cmd_in_buffer_offset).into_as(),
                 )
             }?;
 
@@ -338,7 +339,7 @@ pub(crate) fn new(
         // Patch signature if needed.
         let desc = bios.fwsec_image().header()?;
         let ucode_signed = if desc.signature_count != 0 {
-            let sig_base_img = (desc.imem_load_size + desc.pkc_data_offset) as usize;
+            let sig_base_img = usize::from_as(desc.imem_load_size + desc.pkc_data_offset);
             let desc_sig_versions = u32::from(desc.signature_versions);
             let reg_fuse_version =
                 falcon.signature_reg_fuse_version(bar, desc.engine_id_mask, desc.ucode_id)?;
@@ -369,7 +370,7 @@ pub(crate) fn new(
                 // Mask of the bits of `desc_sig_versions` to preserve.
                 let reg_fuse_version_mask = reg_fuse_version_bit.wrapping_sub(1);
 
-                (desc_sig_versions & reg_fuse_version_mask).count_ones() as usize
+                usize::from_as((desc_sig_versions & reg_fuse_version_mask).count_ones())
             };
 
             dev_dbg!(dev, "patching signature with index {}\n", signature_idx);
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 24c3ea698940..637adf989ee8 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -12,6 +12,7 @@
 use crate::firmware::riscv::RiscvFirmware;
 use crate::gpu::{Architecture, Chipset};
 use crate::gsp::GSP_PAGE_SIZE;
+use crate::num::FromAs;
 
 /// Ad-hoc and temporary module to extract sections from ELF images.
 ///
@@ -232,10 +233,10 @@ pub(crate) fn radix3_dma_handle(&self) -> DmaAddress {
 fn map_into_lvl(sg_table: &SGTable<Owned<VVec<u8>>>, mut dst: VVec<u8>) -> Result<VVec<u8>> {
     for sg_entry in sg_table.iter() {
         // Number of pages we need to map.
-        let num_pages = (sg_entry.dma_len() as usize).div_ceil(GSP_PAGE_SIZE);
+        let num_pages = usize::from_as(sg_entry.dma_len()).div_ceil(GSP_PAGE_SIZE);
 
         for i in 0..num_pages {
-            let entry = sg_entry.dma_address() + (i as u64 * GSP_PAGE_SIZE as u64);
+            let entry = sg_entry.dma_address() + (u64::from_as(i) * u64::from_as(GSP_PAGE_SIZE));
             dst.extend_from_slice(&entry.to_le_bytes(), GFP_KERNEL)?;
         }
     }
diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
index afb08f5bc4ba..fabb38fa8c55 100644
--- a/drivers/gpu/nova-core/firmware/riscv.rs
+++ b/drivers/gpu/nova-core/firmware/riscv.rs
@@ -12,6 +12,7 @@
 
 use crate::dma::DmaObject;
 use crate::firmware::BinFirmware;
+use crate::num::FromAs;
 
 /// Descriptor for microcode running on a RISC-V core.
 #[repr(C)]
@@ -41,7 +42,7 @@ impl RmRiscvUCodeDesc {
     ///
     /// Fails if the header pointed at by `bin_fw` is not within the bounds of the firmware image.
     fn new(bin_fw: &BinFirmware<'_>) -> Result<Self> {
-        let offset = bin_fw.hdr.header_offset as usize;
+        let offset = usize::from_as(bin_fw.hdr.header_offset);
 
         bin_fw
             .fw
@@ -74,8 +75,8 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
         let riscv_desc = RmRiscvUCodeDesc::new(&bin_fw)?;
 
         let ucode = {
-            let start = bin_fw.hdr.data_offset as usize;
-            let len = bin_fw.hdr.data_size as usize;
+            let start = usize::from_as(bin_fw.hdr.data_offset);
+            let len = usize::from_as(bin_fw.hdr.data_size);
 
             DmaObject::from_data(dev, fw.data().get(start..start + len).ok_or(EINVAL)?)?
         };
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..b3ed164aa2ac 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -12,6 +12,7 @@
     FalconModSelAlgo, FalconSecurityModel, PFalcon2Base, PFalconBase, PeregrineCoreSelect,
 };
 use crate::gpu::{Architecture, Chipset};
+use crate::num::FromAs;
 use kernel::prelude::*;
 
 // PMC
@@ -75,7 +76,7 @@ impl NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE {
     /// Returns the usable framebuffer size, in bytes.
     pub(crate) fn usable_fb_size(self) -> u64 {
         let size = (u64::from(self.lower_mag()) << u64::from(self.lower_scale()))
-            * kernel::sizes::SZ_1M as u64;
+            * u64::from_as(kernel::sizes::SZ_1M);
 
         if self.ecc_mode_enabled() {
             // Remove the amount of memory reserved for ECC (one per 16 units).
@@ -158,7 +159,7 @@ pub(crate) fn completed(self) -> bool {
 impl NV_USABLE_FB_SIZE_IN_MB {
     /// Returns the usable framebuffer size, in bytes.
     pub(crate) fn usable_fb_size(self) -> u64 {
-        u64::from(self.value()) * kernel::sizes::SZ_1M as u64
+        u64::from(self.value()) * u64::from_as(kernel::sizes::SZ_1M)
     }
 }
 
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index a521c0a4df0f..234a9f29787b 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -5,6 +5,7 @@
 use crate::driver::Bar0;
 use crate::firmware::fwsec::Bcrt30Rsa3kSignature;
 use crate::firmware::FalconUCodeDescV3;
+use crate::num::FromAs;
 use core::convert::TryFrom;
 use kernel::device;
 use kernel::error::Result;
@@ -825,7 +826,7 @@ fn falcon_data_ptr(&self) -> Result<u32> {
 
         let data_ptr = u32::from_le_bytes(bytes);
 
-        if (usize::from_u32(data_ptr)) < self.base.data.len() {
+        if (usize::from_as(data_ptr)) < self.base.data.len() {
             dev_err!(self.base.dev, "Falcon data pointer out of bounds\n");
             return Err(EINVAL);
         }
@@ -953,7 +954,7 @@ fn setup_falcon_data(
         pci_at_image: &PciAtBiosImage,
         first_fwsec: &FwSecBiosBuilder,
     ) -> Result {
-        let mut offset = pci_at_image.falcon_data_ptr()? as usize;
+        let mut offset = usize::from_as(pci_at_image.falcon_data_ptr()?);
         let mut pmu_in_first_fwsec = false;
 
         // The falcon data pointer assumes that the PciAt and FWSEC images
@@ -994,7 +995,7 @@ fn setup_falcon_data(
             .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
         {
             Ok(entry) => {
-                let mut ucode_offset = entry.data as usize;
+                let mut ucode_offset = usize::from_as(entry.data);
                 ucode_offset -= pci_at_image.base.data.len();
                 if ucode_offset < first_fwsec.base.data.len() {
                     dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
@@ -1080,7 +1081,7 @@ pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) -> Result<&[u8]> {
 
         // The ucode data follows the descriptor.
         let ucode_data_offset = falcon_ucode_offset + desc.size();
-        let size = (desc.imem_load_size + desc.dmem_load_size) as usize;
+        let size = usize::from_as(desc.imem_load_size + desc.dmem_load_size);
 
         // Get the data slice, checking bounds in a single operation.
         self.base

-- 
2.51.0

[PATCH 7/7] gpu: nova-core: justify remaining uses of `as`

[Alexandre Courbot]

There are a few remaining cases where we *do* want to use `as`,
because we specifically want to strip the data that does not fit into
the destination type. Comment these uses to clear confusion about the
intent.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/falcon.rs       | 4 ++++
 drivers/gpu/nova-core/fb/hal/ga100.rs | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index a44df1ac8873..701c08d0cb2a 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -479,9 +479,13 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
         // Set up the base source DMA address.
 
         regs::NV_PFALCON_FALCON_DMATRFBASE::default()
+            // `as u32` is used on purpose since we do want to strip the upper bits, which will be
+            // written to `NV_PFALCON_FALCON_DMATRFBASE1`.
             .set_base((dma_start >> 8) as u32)
             .write(bar, &E::ID);
         regs::NV_PFALCON_FALCON_DMATRFBASE1::default()
+            // `as u16` is used on purpose since the remaining bits are guaranteed to fit within a
+            // `u16`.
             .set_base((dma_start >> 40) as u16)
             .write(bar, &E::ID);
 
diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
index 871c42bf033a..6a59c96febc1 100644
--- a/drivers/gpu/nova-core/fb/hal/ga100.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
@@ -18,9 +18,13 @@ pub(super) fn read_sysmem_flush_page_ga100(bar: &Bar0) -> u64 {
 
 pub(super) fn write_sysmem_flush_page_ga100(bar: &Bar0, addr: u64) {
     regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI::default()
+        // `as u32` is used on purpose since the remaining bits are guaranteed to fit within a
+        // `u32`.
         .set_adr_63_40((addr >> FLUSH_SYSMEM_ADDR_SHIFT_HI) as u32)
         .write(bar);
     regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::default()
+        // `as u32` is used on purpose since we want to strip the upper bits that have been written
+        // to `NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI`.
         .set_adr_39_08((addr >> FLUSH_SYSMEM_ADDR_SHIFT) as u32)
         .write(bar);
 }

-- 
2.51.0

Re: [PATCH 0/7] gpu: nova-core: remove use of `as` for integer conversions

[Danilo Krummrich]

On 10/26/25 3:39 PM, Alexandre Courbot wrote:
> This patchset tries to eradicate the use of `as` in nova-core, by using
> existing means and introducing new ones.

Thanks a lot for working on this cleanup!
> The first 4 patches use the already-available `From` and `TryFrom` trait
> where it is possible or advisable.
> 
> The fifth patch introduces a new module that proposes conversion
> functions for those that are infallible under the current build target.
> This is done through a set of const functions, and the `FromAs` and
> `IntoAs` extension traits which, as their names lightly suggest, offer
> conversion for those types on which the `as` operator can be used
> losslessly.
> 
> This new module is put to use in the sixth patch.
> 
> The idea was first suggested by Danilo, and I hope I captured it
> properly. :)

Yes, this is what I thought of.
> As Danilo suggested, this could eventually find its place in the kernel
> crate if the implementation is deemed to be fit, but for now let's
> review and let it mature in nova-core.

Yeah, I think this approach makes sense. Though, I think we want to move this to
the kernel crate sooner than later, it's definitely something we want for core
code and other drivers as well.

Depending on the initial feedback, it might even make sense to do it right away.

I won't have time for a thorough review, but at a first glance the patches all
look good!

Acked-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Danilo Krummrich]

On 10/26/25 3:39 PM, Alexandre Courbot wrote:
> +#[cfg(CONFIG_32BIT)]
> +/// Infallibly converts a `usize` to `u32` on 32-bit platforms.
> +///
> +/// This conversion is always lossless on 32-bit platforms, where a `usize` is the same size as a
> +/// `u32`.
> +///
> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
> +///
> +/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
> +/// [`IntoAs`] traits.
> +pub(crate) const fn usize_as_u32(value: usize) -> u32 {
> +    kernel::static_assert!(size_of::<u32>() >= size_of::<usize>());
> +
> +    value as u32
> +}

Given that nova-core depends on CONFIG_64BIT, I assume you added this predicting
that we'll move this to the kernel crate.

For core code (and maybe other drivers) I think we also want the same thing for
signed types, but I think it's also fine to add them when moving things into the
kernel crate.

Re: [PATCH 7/7] gpu: nova-core: justify remaining uses of `as`

[Miguel Ojeda]

On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> +            // `as u32` is used on purpose since we do want to strip the upper bits, which will be
> +            // written to `NV_PFALCON_FALCON_DMATRFBASE1`.
>              .set_base((dma_start >> 8) as u32)
>              .write(bar, &E::ID);

We are not very consistent on this yet, but I would suggest using `//
CAST:`. We are working on getting `// PANIC:` in Clippy, and we could
get others like this one eventually.

Cheers,
Miguel

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Miguel Ojeda]

On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> +/// Infallibly converts a `usize` to `u64`.
> +///
> +/// This conversion is always lossless as Linux only supports 32-bit and 64-bit platforms, thus a
> +/// `usize` is always smaller than or of the same size as a `u64`.
> +///
> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
> +///
> +/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
> +/// [`IntoAs`] traits.
> +pub(crate) const fn usize_as_u64(value: usize) -> u64 {
> +    kernel::static_assert!(size_of::<u64>() >= size_of::<usize>());
> +
> +    value as u64
> +}

Since you have the static asserts, this is fine today.

However, we may actually get 128-bit architectures in the
not-so-distant future -- Matthew suggests to be ready by 2035:

    https://lwn.net/Articles/908026/

So this one in particular may actually not be true "soon" -- we also
had related discussions about these assumptions, e.g.:

    https://lore.kernel.org/rust-for-linux/CANiq72m9AeqFKHrRniQ5Nr9vPv2MmUMHFTuuj5ydmqo+OYn60A@mail.gmail.com/

So we should consider having the `cfg` already to prevent people from
assuming it will be always available, and likely a note in its docs,
i.e. we may introducing trouble to port later on to new architectures.
Similarly, the docs of the trait may need rewording.

What do you think?

Regarding the `.into_as()` name, it makes sense, but it can be a bit
surprising when reading out of context... The standalone functions are
super clear, in comparison. But I am not sure what could be better.
`into_in_this_arch()` or similar could emphasize that this will only
work in certain architectures, i.e. it is "an `into()` for this arch"
rather than the general one.
That would go well with the idea that you didn't implement it for
other obvious types, which I guess was to avoid developers using this
instead of `into()` by mistake, right?

(By the way, please use intra-doc links on the primitives too.)

Thanks!

Cheers,
Miguel

Re: [PATCH 7/7] gpu: nova-core: justify remaining uses of `as`

[Alexandre Courbot]

On Mon Oct 27, 2025 at 1:11 AM JST, Miguel Ojeda wrote:
> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> +            // `as u32` is used on purpose since we do want to strip the upper bits, which will be
>> +            // written to `NV_PFALCON_FALCON_DMATRFBASE1`.
>>              .set_base((dma_start >> 8) as u32)
>>              .write(bar, &E::ID);
>
> We are not very consistent on this yet, but I would suggest using `//
> CAST:`. We are working on getting `// PANIC:` in Clippy, and we could
> get others like this one eventually.

Sounds great, will do!

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Alexandre Courbot]

On Mon Oct 27, 2025 at 12:40 AM JST, Danilo Krummrich wrote:
> On 10/26/25 3:39 PM, Alexandre Courbot wrote:
>> +#[cfg(CONFIG_32BIT)]
>> +/// Infallibly converts a `usize` to `u32` on 32-bit platforms.
>> +///
>> +/// This conversion is always lossless on 32-bit platforms, where a `usize` is the same size as a
>> +/// `u32`.
>> +///
>> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
>> +///
>> +/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
>> +/// [`IntoAs`] traits.
>> +pub(crate) const fn usize_as_u32(value: usize) -> u32 {
>> +    kernel::static_assert!(size_of::<u32>() >= size_of::<usize>());
>> +
>> +    value as u32
>> +}
>
> Given that nova-core depends on CONFIG_64BIT, I assume you added this predicting
> that we'll move this to the kernel crate.

That's correct.

>
> For core code (and maybe other drivers) I think we also want the same thing for
> signed types, but I think it's also fine to add them when moving things into the
> kernel crate.

We'll definitely want this. Nova-core does not use require signed types,
which is why I omitted them for now - and also so save some labor as we
are still discussing the fitness of this design.

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Alexandre Courbot]

On Mon Oct 27, 2025 at 1:44 AM JST, Miguel Ojeda wrote:
> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> +/// Infallibly converts a `usize` to `u64`.
>> +///
>> +/// This conversion is always lossless as Linux only supports 32-bit and 64-bit platforms, thus a
>> +/// `usize` is always smaller than or of the same size as a `u64`.
>> +///
>> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
>> +///
>> +/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and
>> +/// [`IntoAs`] traits.
>> +pub(crate) const fn usize_as_u64(value: usize) -> u64 {
>> +    kernel::static_assert!(size_of::<u64>() >= size_of::<usize>());
>> +
>> +    value as u64
>> +}
>
> Since you have the static asserts, this is fine today.
>
> However, we may actually get 128-bit architectures in the
> not-so-distant future -- Matthew suggests to be ready by 2035:
>
>     https://lwn.net/Articles/908026/
>
> So this one in particular may actually not be true "soon" -- we also
> had related discussions about these assumptions, e.g.:
>
>     https://lore.kernel.org/rust-for-linux/CANiq72m9AeqFKHrRniQ5Nr9vPv2MmUMHFTuuj5ydmqo+OYn60A@mail.gmail.com/
>
> So we should consider having the `cfg` already to prevent people from
> assuming it will be always available, and likely a note in its docs,
> i.e. we may introducing trouble to port later on to new architectures.
> Similarly, the docs of the trait may need rewording.
>
> What do you think?

Do you mean adding a `#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]`? That
sounds like a good idea.

The static asserts will break whenever one of these functions needs to
be protected by more conditional compilation anyway, but for consistency
I agree it would make sense to add it now.

>
> Regarding the `.into_as()` name, it makes sense, but it can be a bit
> surprising when reading out of context... The standalone functions are
> super clear, in comparison. But I am not sure what could be better.
> `into_in_this_arch()` or similar could emphasize that this will only
> work in certain architectures, i.e. it is "an `into()` for this arch"
> rather than the general one.
> That would go well with the idea that you didn't implement it for
> other obvious types, which I guess was to avoid developers using this
> instead of `into()` by mistake, right?

Exactly, the trait implementation is limited to conversions not already
covered by `From` (because if there is a `From` implementation, it is
obviously the preferred way to do it).

The const functions, by contrast, need to cover all safe conversions as
we cannot use `From` in a const context yet.

I am happy to take suggestions for naming (I also think the current name
is not great) - we could also consider dropping the trait altogether,
but I find it more convenient for non-const contexts.

>
> (By the way, please use intra-doc links on the primitives too.)

Thanks, it never occured to me that we could. ^_^;

Re: [PATCH 0/7] gpu: nova-core: remove use of `as` for integer conversions

[Alexandre Courbot]

On Mon Oct 27, 2025 at 12:35 AM JST, Danilo Krummrich wrote:
> On 10/26/25 3:39 PM, Alexandre Courbot wrote:
>> This patchset tries to eradicate the use of `as` in nova-core, by using
>> existing means and introducing new ones.
>
> Thanks a lot for working on this cleanup!
>> The first 4 patches use the already-available `From` and `TryFrom` trait
>> where it is possible or advisable.
>> 
>> The fifth patch introduces a new module that proposes conversion
>> functions for those that are infallible under the current build target.
>> This is done through a set of const functions, and the `FromAs` and
>> `IntoAs` extension traits which, as their names lightly suggest, offer
>> conversion for those types on which the `as` operator can be used
>> losslessly.
>> 
>> This new module is put to use in the sixth patch.
>> 
>> The idea was first suggested by Danilo, and I hope I captured it
>> properly. :)
>
> Yes, this is what I thought of.

Great!

>> As Danilo suggested, this could eventually find its place in the kernel
>> crate if the implementation is deemed to be fit, but for now let's
>> review and let it mature in nova-core.
>
> Yeah, I think this approach makes sense. Though, I think we want to move this to
> the kernel crate sooner than later, it's definitely something we want for core
> code and other drivers as well.
>
> Depending on the initial feedback, it might even make sense to do it right away.

Moving to the kernel crate sounds good to me, but this work has been
initiated from the Nova GSP boot series and is now a dependency of it -
so taking it through another tree than `drm-rust` would delay the GSP
boot some more.

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[John Hubbard]

On 10/26/25 9:44 AM, Miguel Ojeda wrote:
> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
...

> Regarding the `.into_as()` name, it makes sense, but it can be a bit
> surprising when reading out of context... The standalone functions are
> super clear, in comparison. But I am not sure what could be better.
> `into_in_this_arch()` or similar could emphasize that this will only
> work in certain architectures, i.e. it is "an `into()` for this arch"
> rather than the general one.
> That would go well with the idea that you didn't implement it for
> other obvious types, which I guess was to avoid developers using this
> instead of `into()` by mistake, right?
> 

Exactly: the into-as, from-as naming suffers from *appearing* to be
familiar and readable, but actually, the naming gives no hint as to 
what it is really doing--nor how it is subtly different from the
basic from/as/into standard conversions.

Instead, we need to add something (almost anything) to the name, to
make it clearly different from the from/as/into.

into_for_arch() goes in that direction, for example.

thanks,
-- 
John Hubbard

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Joel Fernandes]

On 10/27/2025 2:46 PM, John Hubbard wrote:
> On 10/26/25 9:44 AM, Miguel Ojeda wrote:
>> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
> ...
> 
>> Regarding the `.into_as()` name, it makes sense, but it can be a bit
>> surprising when reading out of context... The standalone functions are
>> super clear, in comparison. But I am not sure what could be better.
>> `into_in_this_arch()` or similar could emphasize that this will only
>> work in certain architectures, i.e. it is "an `into()` for this arch"
>> rather than the general one.
>> That would go well with the idea that you didn't implement it for
>> other obvious types, which I guess was to avoid developers using this
>> instead of `into()` by mistake, right?
>>
> 
> Exactly: the into-as, from-as naming suffers from *appearing* to be
> familiar and readable, but actually, the naming gives no hint as to 
> what it is really doing--nor how it is subtly different from the
> basic from/as/into standard conversions.
> 
> Instead, we need to add something (almost anything) to the name, to
> make it clearly different from the from/as/into.
> 
> into_for_arch() goes in that direction, for example.

100% agree with John. Thanks,

 - Joel

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Alexandre Courbot]

On Tue Oct 28, 2025 at 3:46 AM JST, John Hubbard wrote:
> On 10/26/25 9:44 AM, Miguel Ojeda wrote:
>> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
> ...
>
>> Regarding the `.into_as()` name, it makes sense, but it can be a bit
>> surprising when reading out of context... The standalone functions are
>> super clear, in comparison. But I am not sure what could be better.
>> `into_in_this_arch()` or similar could emphasize that this will only
>> work in certain architectures, i.e. it is "an `into()` for this arch"
>> rather than the general one.
>> That would go well with the idea that you didn't implement it for
>> other obvious types, which I guess was to avoid developers using this
>> instead of `into()` by mistake, right?
>> 
>
> Exactly: the into-as, from-as naming suffers from *appearing* to be
> familiar and readable, but actually, the naming gives no hint as to 
> what it is really doing--nor how it is subtly different from the
> basic from/as/into standard conversions.
>
> Instead, we need to add something (almost anything) to the name, to
> make it clearly different from the from/as/into.
>
> into_for_arch() goes in that direction, for example.

I'd like to get more input on that, for I am not sure how we can stay
succint in the naming, while carrying the relevant information.
`into_arch` does not sound much more explanatory than `into_as` - the
intent with the latter was to say "I would normally have done an `as`,
but instead here is a method that attests that this operations is indeed
lossless and safe".

The best naming scheme I could think of is to have the methods carry the
source or destination types: e.g. `from_usize` or `into_usize` (like the
standalone functions), but that would require defining as many traits,
and increase the number of imports - if we go that way, we might just as
well drop the traits completely and use the standalone functions.

`into_native` also comes to mind, but like `arch`, it can mean many
things depending on the context.

... I think I still believe that `into_as` is the clearest name, once
one has read the documentation for the trait - which one should be
expected to do anyway. :)

Re: [PATCH 7/7] gpu: nova-core: justify remaining uses of `as`

[Miguel Ojeda]

On Mon, Oct 27, 2025 at 1:07 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Sounds great, will do!

Thanks!

For reference, the other day I opened a couple issues for this on Clippy:

    https://github.com/rust-lang/rust-clippy/issues/15963
    https://github.com/rust-lang/rust-clippy/issues/15964

and added them to our usual list:

    https://github.com/Rust-for-Linux/linux/issues/349

Cheers,
Miguel

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Miguel Ojeda]

On Tue, Oct 28, 2025 at 3:44 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> intent with the latter was to say "I would normally have done an `as`,
> but instead here is a method that attests that this operations is indeed
> lossless and safe".

Yeah, so you want that, when we see `_as`, we are reminded that this
is the equivalent but lossless of that, which sounds OK. Though I
wouldn't say "I would normally have done ...", but rather turn it
around and say "Since we want to avoid `as`, here is a method ...".

Now, I suggested a change because typically I would expect names to
mention what they are about/do, rather than how they are implemented.
The implementation in this case also doesn't say much, i.e. a type
cast expression can be used for many things, and worse, the operator
may or may not be lossless, so it isn't a big hint.

But, yeah, I understand that you want to evoke the relationship above.

I also suggested it because when I wrote the message I was thinking
about the `cfg`s message, i.e. most of these depend on the
architecture, and thus having an `arch` or similar does evoke a
"careful, my code may not be portable anymore".

On the other hand, it is true that the `u32_as_usize` case will most
likely always be available, in practice, unlike the others. So an
`arch` for that one isn't great (but I guess it could still matter for
someone wanting to reuse the code in a different project/domain).

In any case, personally I don't mind it too much either way --
whatever looks best for most.

Cheers,
Miguel

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[John Hubbard]

On 10/28/25 7:44 AM, Alexandre Courbot wrote:
> On Tue Oct 28, 2025 at 3:46 AM JST, John Hubbard wrote:
>> On 10/26/25 9:44 AM, Miguel Ojeda wrote:
>>> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>> ...
>>
>>> Regarding the `.into_as()` name, it makes sense, but it can be a bit
>>> surprising when reading out of context... The standalone functions are
>>> super clear, in comparison. But I am not sure what could be better.
>>> `into_in_this_arch()` or similar could emphasize that this will only
>>> work in certain architectures, i.e. it is "an `into()` for this arch"
>>> rather than the general one.
>>> That would go well with the idea that you didn't implement it for
>>> other obvious types, which I guess was to avoid developers using this
>>> instead of `into()` by mistake, right?
>>>
>>
>> Exactly: the into-as, from-as naming suffers from *appearing* to be
>> familiar and readable, but actually, the naming gives no hint as to
>> what it is really doing--nor how it is subtly different from the
>> basic from/as/into standard conversions.
>>
>> Instead, we need to add something (almost anything) to the name, to
>> make it clearly different from the from/as/into.
>>
>> into_for_arch() goes in that direction, for example.
> 
> I'd like to get more input on that, for I am not sure how we can stay
> succint in the naming, while carrying the relevant information.

That's too many constraints: if you want an extremely short name
that carries information, *and* avoids (as requested here) confusion
with existing "as" methods, then...you can't.

But you are allowed to be less succinct here, because the more
specialized and rare a case is, the longer you can make the name.
And here, you are definitely allowed a few more characters.


> `into_arch` does not sound much more explanatory than `into_as` - the
> intent with the latter was to say "I would normally have done an `as`,
> but instead here is a method that attests that this operations is indeed
> lossless and safe".
> 
> The best naming scheme I could think of is to have the methods carry the
> source or destination types: e.g. `from_usize` or `into_usize` (like the
> standalone functions), but that would require defining as many traits,
> and increase the number of imports - if we go that way, we might just as
> well drop the traits completely and use the standalone functions.

Accurate names are really desirable; maybe we shouldn't completely
close the door to the above approach.

thanks,
John Hubbard

> 
> `into_native` also comes to mind, but like `arch`, it can mean many
> things depending on the context.
> 
> ... I think I still believe that `into_as` is the clearest name, once
> one has read the documentation for the trait - which one should be
> expected to do anyway. :)

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[Alexandre Courbot]

On Wed Oct 29, 2025 at 2:18 AM JST, John Hubbard wrote:
> On 10/28/25 7:44 AM, Alexandre Courbot wrote:
>> On Tue Oct 28, 2025 at 3:46 AM JST, John Hubbard wrote:
>>> On 10/26/25 9:44 AM, Miguel Ojeda wrote:
>>>> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>> ...
>>>
>>>> Regarding the `.into_as()` name, it makes sense, but it can be a bit
>>>> surprising when reading out of context... The standalone functions are
>>>> super clear, in comparison. But I am not sure what could be better.
>>>> `into_in_this_arch()` or similar could emphasize that this will only
>>>> work in certain architectures, i.e. it is "an `into()` for this arch"
>>>> rather than the general one.
>>>> That would go well with the idea that you didn't implement it for
>>>> other obvious types, which I guess was to avoid developers using this
>>>> instead of `into()` by mistake, right?
>>>>
>>>
>>> Exactly: the into-as, from-as naming suffers from *appearing* to be
>>> familiar and readable, but actually, the naming gives no hint as to
>>> what it is really doing--nor how it is subtly different from the
>>> basic from/as/into standard conversions.
>>>
>>> Instead, we need to add something (almost anything) to the name, to
>>> make it clearly different from the from/as/into.
>>>
>>> into_for_arch() goes in that direction, for example.
>> 
>> I'd like to get more input on that, for I am not sure how we can stay
>> succint in the naming, while carrying the relevant information.
>
> That's too many constraints: if you want an extremely short name
> that carries information, *and* avoids (as requested here) confusion
> with existing "as" methods, then...you can't.
>
> But you are allowed to be less succinct here, because the more
> specialized and rare a case is, the longer you can make the name.
> And here, you are definitely allowed a few more characters.
>
>
>> `into_arch` does not sound much more explanatory than `into_as` - the
>> intent with the latter was to say "I would normally have done an `as`,
>> but instead here is a method that attests that this operations is indeed
>> lossless and safe".
>> 
>> The best naming scheme I could think of is to have the methods carry the
>> source or destination types: e.g. `from_usize` or `into_usize` (like the
>> standalone functions), but that would require defining as many traits,
>> and increase the number of imports - if we go that way, we might just as
>> well drop the traits completely and use the standalone functions.
>
> Accurate names are really desirable; maybe we shouldn't completely
> close the door to the above approach.

I think we have reached the stage where any responsible adult would
shove this whole discussion into a LLM and see what it comes up with.

And the candidate is... `FromSafeCast`/`IntoSafeCast`. Which I have to
say sounds like a good middle ground? :) The intent is definitely to
perform a safe cast here.

Re: [PATCH 5/7] gpu: nova-core: add extra conversion functions and traits

[John Hubbard]

On 10/28/25 3:32 PM, Alexandre Courbot wrote:
> On Wed Oct 29, 2025 at 2:18 AM JST, John Hubbard wrote:
>> On 10/28/25 7:44 AM, Alexandre Courbot wrote:
>>> On Tue Oct 28, 2025 at 3:46 AM JST, John Hubbard wrote:
>>>> On 10/26/25 9:44 AM, Miguel Ojeda wrote:
>>>>> On Sun, Oct 26, 2025 at 3:40 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>>> ...
...
> I think we have reached the stage where any responsible adult would
> shove this whole discussion into a LLM and see what it comes up with.
> 
> And the candidate is... `FromSafeCast`/`IntoSafeCast`. Which I have to
> say sounds like a good middle ground? :) The intent is definitely to
> perform a safe cast here.

Ship it! :)

thanks,
-- 
John Hubbard