* [PATCH v3 1/6] gpu: nova-core: replace `as` with `from` conversions where possible
2025-10-28 23:12 [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
@ 2025-10-28 23:12 ` Alexandre Courbot
2025-10-28 23:31 ` Alexandre Courbot
2025-10-28 23:12 ` [PATCH v3 2/6] gpu: nova-core: vbios: do not use `as` when comparing BiosImageType Alexandre Courbot
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Alexandre Courbot @ 2025-10-28 23:12 UTC (permalink / raw)
To: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, 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.
Acked-by: Danilo Krummrich <dakr@kernel.org>
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 | 42 +++++++++++++++----------------
3 files changed, 25 insertions(+), 27 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..74ed6d61e6cc 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
@@ -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);
@@ -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,
})
}
@@ -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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 1/6] gpu: nova-core: replace `as` with `from` conversions where possible
2025-10-28 23:12 ` [PATCH v3 1/6] gpu: nova-core: replace `as` with `from` conversions where possible Alexandre Courbot
@ 2025-10-28 23:31 ` Alexandre Courbot
0 siblings, 0 replies; 11+ messages in thread
From: Alexandre Courbot @ 2025-10-28 23:31 UTC (permalink / raw)
To: Alexandre Courbot, Alice Ryhl, David Airlie, Simona Vetter,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, Nouveau
On Wed Oct 29, 2025 at 8:12 AM JST, Alexandre Courbot wrote:
> 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.
>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Unless someeone screams, I will probably merge this one patch today - it
doesn't do anything controversial and is required for another series.
Reviews welcome still.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/6] gpu: nova-core: vbios: do not use `as` when comparing BiosImageType
2025-10-28 23:12 [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
2025-10-28 23:12 ` [PATCH v3 1/6] gpu: nova-core: replace `as` with `from` conversions where possible Alexandre Courbot
@ 2025-10-28 23:12 ` Alexandre Courbot
2025-11-03 20:31 ` Joel Fernandes
2025-10-28 23:12 ` [PATCH v3 3/6] gpu: nova-core: use `try_from` instead of `as` for u32 conversions Alexandre Courbot
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Alexandre Courbot @ 2025-10-28 23:12 UTC (permalink / raw)
To: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, Alexandre Courbot
Use the `image_type` method and compare its result to avoid using `as`.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/vbios.rs | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 74ed6d61e6cc..7c1bf10b2eac 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -709,9 +709,8 @@ fn image_type(&self) -> Result<BiosImageType> {
/// Check if this is the last image.
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 {
+ // For NBSI images, return true as they're considered the last image
+ if self.image_type() == Ok(BiosImageType::Nbsi) {
return true;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/6] gpu: nova-core: vbios: do not use `as` when comparing BiosImageType
2025-10-28 23:12 ` [PATCH v3 2/6] gpu: nova-core: vbios: do not use `as` when comparing BiosImageType Alexandre Courbot
@ 2025-11-03 20:31 ` Joel Fernandes
2025-11-04 13:34 ` Alexandre Courbot
0 siblings, 1 reply; 11+ messages in thread
From: Joel Fernandes @ 2025-11-03 20:31 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, nouveau, dri-devel,
linux-kernel, rust-for-linux, Danilo Krummrich
On Wed, Oct 29, 2025 at 08:12:10AM +0900, Alexandre Courbot wrote:
> Use the `image_type` method and compare its result to avoid using `as`.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/gpu/nova-core/vbios.rs | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 74ed6d61e6cc..7c1bf10b2eac 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -709,9 +709,8 @@ fn image_type(&self) -> Result<BiosImageType> {
>
> /// Check if this is the last image.
> 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 {
> + // For NBSI images, return true as they're considered the last image
> + if self.image_type() == Ok(BiosImageType::Nbsi) {
nit: Could you add period at the end of the comment sentence as Miguel
suggested in the other thread (which I admit the initial code also didn't
have)?
With that change,
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
thanks,
- Joel
> return true;
> }
>
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/6] gpu: nova-core: vbios: do not use `as` when comparing BiosImageType
2025-11-03 20:31 ` Joel Fernandes
@ 2025-11-04 13:34 ` Alexandre Courbot
0 siblings, 0 replies; 11+ messages in thread
From: Alexandre Courbot @ 2025-11-04 13:34 UTC (permalink / raw)
To: Joel Fernandes, Alexandre Courbot
Cc: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, nouveau, dri-devel,
linux-kernel, rust-for-linux, Danilo Krummrich, Nouveau
On Tue Nov 4, 2025 at 5:31 AM JST, Joel Fernandes wrote:
> On Wed, Oct 29, 2025 at 08:12:10AM +0900, Alexandre Courbot wrote:
>> Use the `image_type` method and compare its result to avoid using `as`.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> drivers/gpu/nova-core/vbios.rs | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>> index 74ed6d61e6cc..7c1bf10b2eac 100644
>> --- a/drivers/gpu/nova-core/vbios.rs
>> +++ b/drivers/gpu/nova-core/vbios.rs
>> @@ -709,9 +709,8 @@ fn image_type(&self) -> Result<BiosImageType> {
>>
>> /// Check if this is the last image.
>> 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 {
>> + // For NBSI images, return true as they're considered the last image
>> + if self.image_type() == Ok(BiosImageType::Nbsi) {
>
> nit: Could you add period at the end of the comment sentence as Miguel
> suggested in the other thread (which I admit the initial code also didn't
> have)?
>
> With that change,
>
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Done and merged this patch and patch 3/6, thanks! I will follow with the
rest this week unless someone screams at the kernel-wide RFC I send for
patch 4/6 [1].
[1] https://lore.kernel.org/rust-for-linux/20251104-as_casts-v1-1-0a0e95bd2a9f@nvidia.com/T/#u
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/6] gpu: nova-core: use `try_from` instead of `as` for u32 conversions
2025-10-28 23:12 [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
2025-10-28 23:12 ` [PATCH v3 1/6] gpu: nova-core: replace `as` with `from` conversions where possible Alexandre Courbot
2025-10-28 23:12 ` [PATCH v3 2/6] gpu: nova-core: vbios: do not use `as` when comparing BiosImageType Alexandre Courbot
@ 2025-10-28 23:12 ` Alexandre Courbot
2025-10-28 23:12 ` [PATCH v3 4/6] gpu: nova-core: add functions and traits for lossless integer conversions Alexandre Courbot
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Alexandre Courbot @ 2025-10-28 23:12 UTC (permalink / raw)
To: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, 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.
Acked-by: Danilo Krummrich <dakr@kernel.org>
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 4/6] gpu: nova-core: add functions and traits for lossless integer conversions
2025-10-28 23:12 [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (2 preceding siblings ...)
2025-10-28 23:12 ` [PATCH v3 3/6] gpu: nova-core: use `try_from` instead of `as` for u32 conversions Alexandre Courbot
@ 2025-10-28 23:12 ` Alexandre Courbot
2025-10-28 23:12 ` [PATCH v3 5/6] gpu: nova-core: replace use of `as` with functions from `num` Alexandre Courbot
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Alexandre Courbot @ 2025-10-28 23:12 UTC (permalink / raw)
To: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, 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
`FromSafeCast` and `IntoSafeCast` 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/
Acked-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/nova_core.rs | 1 +
drivers/gpu/nova-core/num.rs | 165 +++++++++++++++++++++++++++++++++++++
2 files changed, 166 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..3f7daaedee0b
--- /dev/null
+++ b/drivers/gpu/nova-core/num.rs
@@ -0,0 +1,165 @@
+// 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.
+
+use kernel::macros::paste;
+use kernel::prelude::*;
+
+/// Implements safe `as` conversion functions from a given type into a series of target types.
+///
+/// These functions can be used in place of `as`, with the guarantee that they will be lossless.
+macro_rules! impl_safe_as {
+ ($from:ty as { $($into:ty),* }) => {
+ $(
+ paste! {
+ #[doc = ::core::concat!(
+ "Losslessly converts a [`",
+ ::core::stringify!($from),
+ "`] into a [`",
+ ::core::stringify!($into),
+ "`].")]
+ ///
+ /// This conversion is allowed as it is always lossless. Prefer this over the `as`
+ /// keyword to ensure no lossy casts are performed.
+ ///
+ /// This is for use from a `const` context. For non `const` use, prefer the
+ /// [`FromSafeCast`] and [`IntoSafeCast`] traits.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use crate::num;
+ ///
+ #[doc = ::core::concat!(
+ "assert_eq!(num::",
+ ::core::stringify!($from),
+ "_as_",
+ ::core::stringify!($into),
+ "(1",
+ ::core::stringify!($from),
+ "), 1",
+ ::core::stringify!($into),
+ ");")]
+ /// ```
+ #[allow(unused)]
+ #[inline(always)]
+ pub(crate) const fn [<$from _as_ $into>](value: $from) -> $into {
+ kernel::static_assert!(size_of::<$into>() >= size_of::<$from>());
+
+ value as $into
+ }
+ }
+ )*
+ };
+}
+
+impl_safe_as!(u8 as { u16, u32, u64, usize });
+impl_safe_as!(u16 as { u32, u64, usize });
+impl_safe_as!(u32 as { u64, usize } );
+// `u64` and `usize` have the same size on 64-bit platforms.
+#[cfg(CONFIG_64BIT)]
+impl_safe_as!(u64 as { usize } );
+
+// A `usize` fits into a `u64` on 32 and 64-bit platforms.
+#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
+impl_safe_as!(usize as { u64 });
+
+// A `usize` fits into a `u32` on 32-bit platforms.
+#[cfg(CONFIG_32BIT)]
+impl_safe_as!(usize as { u32 });
+
+/// Extension trait providing guaranteed lossless cast 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 width 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 cast 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 casts are performed.
+///
+/// If you need to perform a conversion in `const` context, use [`u64_as_usize`], [`u32_as_usize`],
+/// [`usize_as_u64`], etc.
+///
+/// # Examples
+///
+/// ```
+/// use crate::num::FromSafeCast;
+///
+/// assert_eq!(usize::from_safe_cast(0xf00u32), 0xf00u32 as usize);
+/// ```
+#[expect(unused)]
+pub(crate) trait FromSafeCast<T> {
+ /// Create a `Self` from `value`. This operation is guaranteed to be lossless.
+ fn from_safe_cast(value: T) -> Self;
+}
+
+impl FromSafeCast<usize> for u64 {
+ fn from_safe_cast(value: usize) -> Self {
+ usize_as_u64(value)
+ }
+}
+
+#[cfg(CONFIG_32BIT)]
+impl FromSafeCast<usize> for u32 {
+ fn from_safe_cast(value: usize) -> Self {
+ usize_as_u32(value)
+ }
+}
+
+impl FromSafeCast<u32> for usize {
+ fn from_safe_cast(value: u32) -> Self {
+ u32_as_usize(value)
+ }
+}
+
+#[cfg(CONFIG_64BIT)]
+impl FromSafeCast<u64> for usize {
+ fn from_safe_cast(value: u64) -> Self {
+ u64_as_usize(value)
+ }
+}
+
+/// Counterpart to the [`FromSafeCast`] trait, i.e. this trait is to [`FromSafeCast`] what [`Into`]
+/// is to [`From`].
+///
+/// See the documentation of [`FromSafeCast`] for the motivation.
+///
+/// # Examples
+///
+/// ```
+/// use crate::num::IntoSafeCast;
+///
+/// assert_eq!(0xf00u32.into_safe_cast(), 0xf00u32 as usize);
+/// ```
+#[expect(unused)]
+pub(crate) trait IntoSafeCast<T> {
+ /// Convert `self` into a `T`. This operation is guaranteed to be lossless.
+ fn into_safe_cast(self) -> T;
+}
+
+/// Reverse operation for types implementing [`FromSafeCast`].
+impl<S, T> IntoSafeCast<T> for S
+where
+ T: FromSafeCast<S>,
+{
+ fn into_safe_cast(self) -> T {
+ T::from_safe_cast(self)
+ }
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 5/6] gpu: nova-core: replace use of `as` with functions from `num`
2025-10-28 23:12 [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (3 preceding siblings ...)
2025-10-28 23:12 ` [PATCH v3 4/6] gpu: nova-core: add functions and traits for lossless integer conversions Alexandre Courbot
@ 2025-10-28 23:12 ` Alexandre Courbot
2025-10-28 23:12 ` [PATCH v3 6/6] gpu: nova-core: justify remaining uses of `as` Alexandre Courbot
2025-11-07 23:30 ` [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
6 siblings, 0 replies; 11+ messages in thread
From: Alexandre Courbot @ 2025-10-28 23:12 UTC (permalink / raw)
To: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, 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.
Acked-by: Danilo Krummrich <dakr@kernel.org>
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 | 34 +++++++++++++++++++-------------
drivers/gpu/nova-core/firmware/fwsec.rs | 11 ++++++-----
drivers/gpu/nova-core/firmware/gsp.rs | 6 ++++--
drivers/gpu/nova-core/firmware/riscv.rs | 7 ++++---
drivers/gpu/nova-core/num.rs | 2 --
drivers/gpu/nova-core/regs.rs | 5 +++--
drivers/gpu/nova-core/vbios.rs | 9 +++++----
10 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index fb3561cc9746..a0ae197a1489 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::{FromSafeCast, IntoSafeCast};
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_safe_cast())?,
),
};
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_safe_cast(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..667d5681e347 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::{FromSafeCast, IntoSafeCast};
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_safe_cast()
}
}
@@ -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_safe_cast(self.hdr.data_offset);
+ let fw_size = usize::from_safe_cast(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..cdd6e971f19e 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::{FromSafeCast, IntoSafeCast};
/// 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_safe_cast())
.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_safe_cast())
}
/// Returns an iterator to the signatures of the firmware. The iterator can be empty if the
@@ -91,19 +92,23 @@ 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_safe_cast())?;
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_safe_cast())?;
+ let signatures_start = usize::from_safe_cast(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_safe_cast(self.hdr.sig_prod_size),
+ )
.ok_or(EINVAL)?
- .chunks_exact(sig_size as usize)
+ .chunks_exact(sig_size.into_safe_cast())
}
};
@@ -132,9 +137,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_safe_cast(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_safe_cast())
.ok_or(EINVAL)?;
hs_fw
@@ -169,7 +174,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_safe_cast())
}
}
@@ -198,12 +203,13 @@ 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_safe_cast(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_safe_cast(idx).checked_mul(size_of::<Self>())?)
})
.ok_or(EINVAL)?,
)
@@ -318,12 +324,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_safe_cast())
}
}
.ok_or(EINVAL)?;
- ucode.patch_signature(&signature, patch_loc as usize)?
+ ucode.patch_signature(&signature, patch_loc.into_safe_cast())?
}
};
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index ce78c1563754..96bedada91bc 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::{FromSafeCast, IntoSafeCast};
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_safe_cast(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_safe_cast(),
)
}?;
@@ -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_safe_cast(),
)
}?;
@@ -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_safe_cast(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_safe_cast((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..b78311f32752 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::FromSafeCast;
/// Ad-hoc and temporary module to extract sections from ELF images.
///
@@ -232,10 +233,11 @@ 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_safe_cast(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_safe_cast(i) * u64::from_safe_cast(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..0bae8c74328a 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::FromSafeCast;
/// 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_safe_cast(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_safe_cast(bin_fw.hdr.data_offset);
+ let len = usize::from_safe_cast(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/num.rs b/drivers/gpu/nova-core/num.rs
index 3f7daaedee0b..b08139ec667c 100644
--- a/drivers/gpu/nova-core/num.rs
+++ b/drivers/gpu/nova-core/num.rs
@@ -104,7 +104,6 @@ pub(crate) const fn [<$from _as_ $into>](value: $from) -> $into {
///
/// assert_eq!(usize::from_safe_cast(0xf00u32), 0xf00u32 as usize);
/// ```
-#[expect(unused)]
pub(crate) trait FromSafeCast<T> {
/// Create a `Self` from `value`. This operation is guaranteed to be lossless.
fn from_safe_cast(value: T) -> Self;
@@ -148,7 +147,6 @@ fn from_safe_cast(value: u64) -> Self {
///
/// assert_eq!(0xf00u32.into_safe_cast(), 0xf00u32 as usize);
/// ```
-#[expect(unused)]
pub(crate) trait IntoSafeCast<T> {
/// Convert `self` into a `T`. This operation is guaranteed to be lossless.
fn into_safe_cast(self) -> T;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..2e7cebed684b 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::FromSafeCast;
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_safe_cast(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_safe_cast(kernel::sizes::SZ_1M)
}
}
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 7c1bf10b2eac..cb54d96c9535 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::FromSafeCast;
use core::convert::TryFrom;
use kernel::device;
use kernel::error::Result;
@@ -817,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_safe_cast(data_ptr)) < self.base.data.len() {
dev_err!(self.base.dev, "Falcon data pointer out of bounds\n");
return Err(EINVAL);
}
@@ -945,7 +946,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_safe_cast(pci_at_image.falcon_data_ptr()?);
let mut pmu_in_first_fwsec = false;
// The falcon data pointer assumes that the PciAt and FWSEC images
@@ -986,7 +987,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_safe_cast(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");
@@ -1072,7 +1073,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_safe_cast(desc.imem_load_size + desc.dmem_load_size);
// Get the data slice, checking bounds in a single operation.
self.base
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 6/6] gpu: nova-core: justify remaining uses of `as`
2025-10-28 23:12 [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (4 preceding siblings ...)
2025-10-28 23:12 ` [PATCH v3 5/6] gpu: nova-core: replace use of `as` with functions from `num` Alexandre Courbot
@ 2025-10-28 23:12 ` Alexandre Courbot
2025-11-07 23:30 ` [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
6 siblings, 0 replies; 11+ messages in thread
From: Alexandre Courbot @ 2025-10-28 23:12 UTC (permalink / raw)
To: Alice Ryhl, David Airlie, Simona Vetter, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, 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.
Acked-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 4 ++++
drivers/gpu/nova-core/fb/hal/ga100.rs | 4 ++++
drivers/gpu/nova-core/firmware/fwsec.rs | 2 ++
3 files changed, 10 insertions(+)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index a0ae197a1489..9b5269843af5 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()
+ // CAST: `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()
+ // CAST: `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..b9389fa382e5 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()
+ // CAST: `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()
+ // CAST: `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);
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index 96bedada91bc..90b372850062 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -161,6 +161,7 @@ unsafe fn transmute<'a, 'b, T: Sized + FromBytes>(
if offset + size_of::<T>() > fw.size() {
return Err(EINVAL);
}
+ // CAST: `usize` has the same size as pointers.
if (fw.start_ptr() as usize + offset) % align_of::<T>() != 0 {
return Err(EINVAL);
}
@@ -184,6 +185,7 @@ unsafe fn transmute_mut<'a, 'b, T: Sized + FromBytes>(
if offset + size_of::<T>() > fw.size() {
return Err(EINVAL);
}
+ // CAST: `usize` has the same size as pointers.
if (fw.start_ptr_mut() as usize + offset) % align_of::<T>() != 0 {
return Err(EINVAL);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions
2025-10-28 23:12 [PATCH v3 0/6] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (5 preceding siblings ...)
2025-10-28 23:12 ` [PATCH v3 6/6] gpu: nova-core: justify remaining uses of `as` Alexandre Courbot
@ 2025-11-07 23:30 ` Alexandre Courbot
6 siblings, 0 replies; 11+ messages in thread
From: Alexandre Courbot @ 2025-11-07 23:30 UTC (permalink / raw)
To: Alexandre Courbot, Alice Ryhl, David Airlie, Simona Vetter,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux,
Danilo Krummrich, Nouveau
On Wed Oct 29, 2025 at 8:12 AM JST, Alexandre Courbot wrote:
> 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 `FromSafeCast`
> and `IntoSafeCast` 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.
>
> 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.
All patches pushed to drm-rust-next, thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread