Re: [PATCH] gpu: nova-core: define named constants for magic numbers

["Alexandre Courbot" <acourbot@nvidia.com>]

On Fri Jul 11, 2025 at 8:33 PM JST, Quaternions wrote:
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 5b5d9f38cbb3..d456c494374d 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -364,8 +364,9 @@ struct BitHeader {
>  }
>
>  impl BitHeader {
> +    const MIN_LEN: usize = 12;
>      fn new(data: &[u8]) -> Result<Self> {
> -        if data.len() < 12 {
> +        if data.len() < Self::MIN_LEN {
>              return Err(EINVAL);
>          }

Mmm looking at the these functions makes me think that maybe we should
rather:

- Make `BitHeader` (and the other structs affected by this patch)
  `#[repr(C)]`,
- Wait until the `FromBytes` patchset lands [1],
- ... and simply implement `FromBytes` on these structures to replace
  this code, which is basically a little-endian deserializer. Since Nova
  has no plan to run on big-endian targets at the moment, this should
  work fine, and removes the need to define constants for the sizes of
  these types and use indices that are always error-prone.

Or at the very least, we make these structs `#[repr(C)]` and change the
implementation to be done in terms of `size_of` and `offset_of`, which,
while more verbose, also promises better correctness.

[1] https://lore.kernel.org/rust-for-linux/20250624042802.105623-1-christiansantoslima21@gmail.com/