* [PATCH v2 1/7] gpu: nova-core: replace `as` with `from` conversions where possible
2025-10-27 12:54 [PATCH v2 0/7] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
@ 2025-10-27 12:54 ` Alexandre Courbot
2025-10-27 12:54 ` [PATCH v2 2/7] gpu: nova-core: vbios: remove unneeded u8 conversions Alexandre Courbot
` (5 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-27 12:54 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 | 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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 2/7] gpu: nova-core: vbios: remove unneeded u8 conversions
2025-10-27 12:54 [PATCH v2 0/7] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
2025-10-27 12:54 ` [PATCH v2 1/7] gpu: nova-core: replace `as` with `from` conversions where possible Alexandre Courbot
@ 2025-10-27 12:54 ` Alexandre Courbot
2025-10-27 17:41 ` Joel Fernandes
2025-10-27 12:54 ` [PATCH v2 3/7] gpu: nova-core: vbios: add conversion to u8 for BiosImageType Alexandre Courbot
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-27 12:54 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
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`.
Acked-by: Danilo Krummrich <dakr@kernel.org>
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 2/7] gpu: nova-core: vbios: remove unneeded u8 conversions
2025-10-27 12:54 ` [PATCH v2 2/7] gpu: nova-core: vbios: remove unneeded u8 conversions Alexandre Courbot
@ 2025-10-27 17:41 ` Joel Fernandes
2025-10-28 7:22 ` Alexandre Courbot
0 siblings, 1 reply; 18+ messages in thread
From: Joel Fernandes @ 2025-10-27 17:41 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, Timur Tabi, Edwin Peer, nouveau,
dri-devel, linux-kernel, rust-for-linux, Danilo Krummrich
Hello Alex,
On 10/27/2025 8:54 AM, Alexandre Courbot wrote:
> 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`.
>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> 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,
Why not just change PmuLookupTable to:
struct PmuLookupTable {
version: u8,
header_len: usize,
entry_len: usize,
entry_count: usize,
table_data: KVec<u8>,
}
That is cleaner and removes the issue while allowing to use the local variables
(and also makes sense to be usize as these 3 fields are size-like fields).
thanks,
- Joel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 2/7] gpu: nova-core: vbios: remove unneeded u8 conversions
2025-10-27 17:41 ` Joel Fernandes
@ 2025-10-28 7:22 ` Alexandre Courbot
0 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-28 7:22 UTC (permalink / raw)
To: Joel Fernandes, 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, Timur Tabi, Edwin Peer, nouveau,
dri-devel, linux-kernel, rust-for-linux, Danilo Krummrich,
Nouveau
On Tue Oct 28, 2025 at 2:41 AM JST, Joel Fernandes wrote:
> Hello Alex,
>
> On 10/27/2025 8:54 AM, Alexandre Courbot wrote:
>> 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`.
>>
>> Acked-by: Danilo Krummrich <dakr@kernel.org>
>> 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,
>
> Why not just change PmuLookupTable to:
>
> struct PmuLookupTable {
> version: u8,
> header_len: usize,
> entry_len: usize,
> entry_count: usize,
> table_data: KVec<u8>,
> }
>
> That is cleaner and removes the issue while allowing to use the local variables
> (and also makes sense to be usize as these 3 fields are size-like fields).
That would work! But while trying I also figured we could just split the
header into its own `repr(C)` struct and use `FromBytes` on it, which
would achieve the same result with less array indexing.
Actually, we could do that for a bunch of structures in this file, so I
think I'll just try and do it that way.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 3/7] gpu: nova-core: vbios: add conversion to u8 for BiosImageType
2025-10-27 12:54 [PATCH v2 0/7] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
2025-10-27 12:54 ` [PATCH v2 1/7] gpu: nova-core: replace `as` with `from` conversions where possible Alexandre Courbot
2025-10-27 12:54 ` [PATCH v2 2/7] gpu: nova-core: vbios: remove unneeded u8 conversions Alexandre Courbot
@ 2025-10-27 12:54 ` Alexandre Courbot
2025-10-27 17:37 ` Joel Fernandes
2025-10-27 12:54 ` [PATCH v2 4/7] gpu: nova-core: use `try_from` instead of `as` for u32 conversions Alexandre Courbot
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-27 12:54 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
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.
Acked-by: Danilo Krummrich <dakr@kernel.org>
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/7] gpu: nova-core: vbios: add conversion to u8 for BiosImageType
2025-10-27 12:54 ` [PATCH v2 3/7] gpu: nova-core: vbios: add conversion to u8 for BiosImageType Alexandre Courbot
@ 2025-10-27 17:37 ` Joel Fernandes
2025-10-28 7:23 ` Alexandre Courbot
0 siblings, 1 reply; 18+ messages in thread
From: Joel Fernandes @ 2025-10-27 17:37 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
Hello Alex,
On Mon, Oct 27, 2025 at 09:54:43PM +0900, Alexandre Courbot wrote:
> 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.
>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> 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() {
I strongly prefer u8::from(BiosImageType::Nbsi) here so there is no loss of
readability of the type. Can we please use ::from()?
thanks,
- Joel
> return true;
> }
>
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/7] gpu: nova-core: vbios: add conversion to u8 for BiosImageType
2025-10-27 17:37 ` Joel Fernandes
@ 2025-10-28 7:23 ` Alexandre Courbot
0 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-28 7:23 UTC (permalink / raw)
To: Joel Fernandes
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 Oct 28, 2025 at 2:37 AM JST, Joel Fernandes wrote:
> Hello Alex,
>
> On Mon, Oct 27, 2025 at 09:54:43PM +0900, Alexandre Courbot wrote:
>> 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.
>>
>> Acked-by: Danilo Krummrich <dakr@kernel.org>
>> 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() {
>
> I strongly prefer u8::from(BiosImageType::Nbsi) here so there is no loss of
> readability of the type. Can we please use ::from()?
What benefit do we get from knowing the representing type of
BiosImageType? We are only interested in knowing whether the two values
are equal or not.
But you have a point that `::from()` is generally easier to read, only I
would apply it on the left value:
if BiosImageType::try_from(self.pcir.code_type) == Ok(BiosImageType::Nbsi)
or reusing the already existing code:
if self.image_type() == Ok(BiosImageType::Nbsi)
That reads even more naturally imho.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 4/7] gpu: nova-core: use `try_from` instead of `as` for u32 conversions
2025-10-27 12:54 [PATCH v2 0/7] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (2 preceding siblings ...)
2025-10-27 12:54 ` [PATCH v2 3/7] gpu: nova-core: vbios: add conversion to u8 for BiosImageType Alexandre Courbot
@ 2025-10-27 12:54 ` Alexandre Courbot
2025-10-27 12:54 ` [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits Alexandre Courbot
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-27 12:54 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] 18+ messages in thread* [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
2025-10-27 12:54 [PATCH v2 0/7] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (3 preceding siblings ...)
2025-10-27 12:54 ` [PATCH v2 4/7] gpu: nova-core: use `try_from` instead of `as` for u32 conversions Alexandre Courbot
@ 2025-10-27 12:54 ` Alexandre Courbot
2025-10-27 19:09 ` Joel Fernandes
2025-10-27 12:54 ` [PATCH v2 6/7] gpu: nova-core: replace use of `as` with functions from `num` Alexandre Courbot
2025-10-27 12:54 ` [PATCH v2 7/7] gpu: nova-core: justify remaining uses of `as` Alexandre Courbot
6 siblings, 1 reply; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-27 12:54 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
`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/
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 | 158 +++++++++++++++++++++++++++++++++++++
2 files changed, 159 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..adb5a92f0d51
--- /dev/null
+++ b/drivers/gpu/nova-core/num.rs
@@ -0,0 +1,158 @@
+// 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::{build_error, macros::paste};
+
+macro_rules! impl_lossless_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 conversions are performed.
+ ///
+ /// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`]
+ /// and [`IntoAs`] 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)]
+ pub(crate) const fn [<$from _as_ $into>](value: $from) -> $into {
+ kernel::static_assert!(size_of::<$into>() >= size_of::<$from>());
+
+ value as $into
+ }
+ }
+ )*
+ };
+}
+
+impl_lossless_as!(u8 as { u16, u32, u64, usize });
+impl_lossless_as!(u16 as { u32, u64, usize });
+impl_lossless_as!(u32 as { u64, usize } );
+// `u64` and `usize` have the same size on 64-bit platforms.
+#[cfg(CONFIG_64BIT)]
+impl_lossless_as!(u64 as { usize } );
+
+// A `usize` fits into a `u64` on 32 and 64-bit platforms.
+#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
+impl_lossless_as!(usize as { u64 });
+
+// A `usize` fits into a `u32` on 32-bit platforms.
+#[cfg(CONFIG_32BIT)]
+impl_lossless_as!(usize as { u32 });
+
+/// 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 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 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`], etc.
+///
+/// # 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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
2025-10-27 12:54 ` [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits Alexandre Courbot
@ 2025-10-27 19:09 ` Joel Fernandes
2025-10-27 19:11 ` Joel Fernandes
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Joel Fernandes @ 2025-10-27 19:09 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, Timur Tabi, Edwin Peer, nouveau,
dri-devel, linux-kernel, rust-for-linux, Danilo Krummrich
Hello Alex,
On 10/27/2025 8:54 AM, Alexandre Courbot wrote:
> 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.
Why not just implement `From` and `Into` for the types missing it then, with
adequate comments about why such conversions are Ok for the kernel, instead of
introducing a new trait? This is exactly what `From`/`Into` is for right?
> 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 | 158 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 159 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..adb5a92f0d51
> --- /dev/null
> +++ b/drivers/gpu/nova-core/num.rs
> @@ -0,0 +1,158 @@
> +// 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::{build_error, macros::paste};
> +
> +macro_rules! impl_lossless_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 conversions are performed.
> + ///
> + /// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`]
> + /// and [`IntoAs`] 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)]
> + pub(crate) const fn [<$from _as_ $into>](value: $from) -> $into {
> + kernel::static_assert!(size_of::<$into>() >= size_of::<$from>());
> +
> + value as $into
> + }
> + }
> + )*
> + };
> +}
> +
> +impl_lossless_as!(u8 as { u16, u32, u64, usize });
> +impl_lossless_as!(u16 as { u32, u64, usize });
> +impl_lossless_as!(u32 as { u64, usize } );
I prefer consistency with the notation in num.rs in the rust standard library:
impl_from!(u16 => usize)
But I am Ok with your notation as well, which is concise.
> +// `u64` and `usize` have the same size on 64-bit platforms.
> +#[cfg(CONFIG_64BIT)]
> +impl_lossless_as!(u64 as { usize } );
> +
> +// A `usize` fits into a `u64` on 32 and 64-bit platforms.
> +#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
> +impl_lossless_as!(usize as { u64 });
> +
> +// A `usize` fits into a `u32` on 32-bit platforms.
> +#[cfg(CONFIG_32BIT)]
> +impl_lossless_as!(usize as { u32 });
> +
> +/// 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 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 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`], etc.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use crate::num::FromAs;
> +///
> +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
This `from_as()` syntax will be very confusing for users IMO, honestly we should
just keep it as `from()`, otherwise there is confusion and ambiguity around
whether someone should use `::from()` or `::from_as()`. Please let us just keep
all infallible conversions to use `from()`/`into()` and all infallible ones to
use `try_from()`/`try_into()`. No need to additional `_as()` prefix, as it
creates confusion. In the end of the day, the fact the conversion is lossless is
not relevant, all conversions that don't use the `as` keyword should be expected
to be lossless right?
thanks,
- Joel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
2025-10-27 19:09 ` Joel Fernandes
@ 2025-10-27 19:11 ` Joel Fernandes
2025-10-27 19:23 ` Danilo Krummrich
2025-10-28 7:23 ` Alexandre Courbot
2 siblings, 0 replies; 18+ messages in thread
From: Joel Fernandes @ 2025-10-27 19:11 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, Timur Tabi, Edwin Peer, nouveau,
dri-devel, linux-kernel, rust-for-linux, Danilo Krummrich
On 10/27/2025 3:09 PM, Joel Fernandes wrote:
[...]
>> +/// 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 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 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`], etc.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// use crate::num::FromAs;
>> +///
>> +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
>
> This `from_as()` syntax will be very confusing for users IMO, honestly we should
> just keep it as `from()`, otherwise there is confusion and ambiguity around
> whether someone should use `::from()` or `::from_as()`. Please let us just keep
> all infallible conversions to use `from()`/`into()` and all infallible ones to
s/infallible ones/fallible ones/.
Sorry, thanks,
- Joel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
2025-10-27 19:09 ` Joel Fernandes
2025-10-27 19:11 ` Joel Fernandes
@ 2025-10-27 19:23 ` Danilo Krummrich
2025-10-27 19:28 ` Danilo Krummrich
2025-10-28 7:23 ` Alexandre Courbot
2 siblings, 1 reply; 18+ messages in thread
From: Danilo Krummrich @ 2025-10-27 19:23 UTC (permalink / raw)
To: Joel Fernandes
Cc: 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, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux
On 10/27/25 8:09 PM, Joel Fernandes wrote:
> Why not just implement `From` and `Into` for the types missing it then, with
> adequate comments about why such conversions are Ok for the kernel, instead of
> introducing a new trait? This is exactly what `From`/`Into` is for right?
https://doc.rust-lang.org/reference/items/implementations.html#r-items.impl.trait.orphan-rule.intro
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
2025-10-27 19:23 ` Danilo Krummrich
@ 2025-10-27 19:28 ` Danilo Krummrich
2025-10-27 19:37 ` Joel Fernandes
0 siblings, 1 reply; 18+ messages in thread
From: Danilo Krummrich @ 2025-10-27 19:28 UTC (permalink / raw)
To: Joel Fernandes
Cc: 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, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux
On 10/27/25 8:23 PM, Danilo Krummrich wrote:
> On 10/27/25 8:09 PM, Joel Fernandes wrote:
>> Why not just implement `From` and `Into` for the types missing it then, with
>> adequate comments about why such conversions are Ok for the kernel, instead of
>> introducing a new trait? This is exactly what `From`/`Into` is for right?
>
> https://doc.rust-lang.org/reference/items/implementations.html#r-items.impl.trait.orphan-rule.intro
(Sorry, I didn't mean to send the link without additional comment.)
We can't do this due to the orphan rule, but even if we could I think a separate
trait indicating the reason for the conversions to be valid is a good thing.
- Danilo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
2025-10-27 19:28 ` Danilo Krummrich
@ 2025-10-27 19:37 ` Joel Fernandes
0 siblings, 0 replies; 18+ messages in thread
From: Joel Fernandes @ 2025-10-27 19:37 UTC (permalink / raw)
To: Danilo Krummrich
Cc: 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, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, nouveau, dri-devel, linux-kernel, rust-for-linux
On 10/27/2025 3:28 PM, Danilo Krummrich wrote:
> On 10/27/25 8:23 PM, Danilo Krummrich wrote:
>> On 10/27/25 8:09 PM, Joel Fernandes wrote:
>>> Why not just implement `From` and `Into` for the types missing it then, with
>>> adequate comments about why such conversions are Ok for the kernel, instead of
>>> introducing a new trait? This is exactly what `From`/`Into` is for right?
>>
>> https://doc.rust-lang.org/reference/items/implementations.html#r-items.impl.trait.orphan-rule.intro
>
> (Sorry, I didn't mean to send the link without additional comment.)
>
> We can't do this due to the orphan rule, but even if we could I think a separate
> trait indicating the reason for the conversions to be valid is a good thing.
>
Thanks for clarifying, yeah its not ideal then as the user of the conversion
needs to remember when to use from vs from_as. I don't think its terrible, just
a bit confusing.
Thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
2025-10-27 19:09 ` Joel Fernandes
2025-10-27 19:11 ` Joel Fernandes
2025-10-27 19:23 ` Danilo Krummrich
@ 2025-10-28 7:23 ` Alexandre Courbot
2 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-28 7:23 UTC (permalink / raw)
To: Joel Fernandes, 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, Timur Tabi, Edwin Peer, nouveau,
dri-devel, linux-kernel, rust-for-linux, Danilo Krummrich,
Nouveau
On Tue Oct 28, 2025 at 4:09 AM JST, Joel Fernandes wrote:
> Hello Alex,
>
> On 10/27/2025 8:54 AM, Alexandre Courbot wrote:
>> 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.
>
> Why not just implement `From` and `Into` for the types missing it then, with
> adequate comments about why such conversions are Ok for the kernel, instead of
> introducing a new trait? This is exactly what `From`/`Into` is for right?
I wish we could. :) The link Danilo sent should clarify why this is not
possible.
<snip>
>> +impl_lossless_as!(u8 as { u16, u32, u64, usize });
>> +impl_lossless_as!(u16 as { u32, u64, usize });
>> +impl_lossless_as!(u32 as { u64, usize } );
>
> I prefer consistency with the notation in num.rs in the rust standard library:
> impl_from!(u16 => usize)
> But I am Ok with your notation as well, which is concise.
You're right, something like `impl_as` is both shorter and more
consistent.
>
>> +// `u64` and `usize` have the same size on 64-bit platforms.
>> +#[cfg(CONFIG_64BIT)]
>> +impl_lossless_as!(u64 as { usize } );
>> +
>> +// A `usize` fits into a `u64` on 32 and 64-bit platforms.
>> +#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
>> +impl_lossless_as!(usize as { u64 });
>> +
>> +// A `usize` fits into a `u32` on 32-bit platforms.
>> +#[cfg(CONFIG_32BIT)]
>> +impl_lossless_as!(usize as { u32 });
>> +
>> +/// 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 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 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`], etc.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// use crate::num::FromAs;
>> +///
>> +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
>
> This `from_as()` syntax will be very confusing for users IMO, honestly we should
> just keep it as `from()`, otherwise there is confusion and ambiguity around
> whether someone should use `::from()` or `::from_as()`. Please let us just keep
> all infallible conversions to use `from()`/`into()` and all infallible ones to
> use `try_from()`/`try_into()`. No need to additional `_as()` prefix, as it
> creates confusion. In the end of the day, the fact the conversion is lossless is
> not relevant, all conversions that don't use the `as` keyword should be expected
> to be lossless right?
I wanted to use `from`/`into` initially but this unfortunately clashes
with the `From` and `Into` traits and forces callers to disambiguate
which trait we want to use with a turbofish, which goes against the
intent of keeping the syntax short. I'm happy to consider better names
though.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 6/7] gpu: nova-core: replace use of `as` with functions from `num`
2025-10-27 12:54 [PATCH v2 0/7] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (4 preceding siblings ...)
2025-10-27 12:54 ` [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits Alexandre Courbot
@ 2025-10-27 12:54 ` Alexandre Courbot
2025-10-27 12:54 ` [PATCH v2 7/7] gpu: nova-core: justify remaining uses of `as` Alexandre Courbot
6 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-27 12:54 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 | 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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 7/7] gpu: nova-core: justify remaining uses of `as`
2025-10-27 12:54 [PATCH v2 0/7] gpu: nova-core: remove use of `as` for integer conversions Alexandre Courbot
` (5 preceding siblings ...)
2025-10-27 12:54 ` [PATCH v2 6/7] gpu: nova-core: replace use of `as` with functions from `num` Alexandre Courbot
@ 2025-10-27 12:54 ` Alexandre Courbot
6 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2025-10-27 12:54 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 a44df1ac8873..7f6c7091c9c3 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 ed82e74ccdc9..ecef4cc78942 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] 18+ messages in thread