Re: [PATCH v4 4/7] gpu: nova-core: add NVKV decoder

[John Hubbard <jhubbard@nvidia.com>]

On 10/15/26 7:00 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] gpu: nova-core: endianness bug when extracting u8/u32 arrays from native &[u64] stream
...
>> +++ b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
> [ ... ]
>> +    /// Takes `count` bytes. If `count` is not a multiple of 8 (`u64` size), bytes are discarded up
>> +    /// to the next multiple.
>> +    fn take_u8s(&mut self, count: usize) -> Result<&'a [u8]> {
>> +        let values = self.take_u64s(count.div_ceil(size_of::<u64>()))?;
>> +        values.as_bytes().get(..count).ok_or(EINVAL)
>> +    }
> 
> [Severity: High]
> Does casting a native u64 slice to bytes extract the wrong payload on
> big-endian architectures?

Roman, or anyone who knows,

How do we update sashiko to make it stop reporting on this issue, which
we have decided is a non-issue (Nova is actually configured to only ever
be little-endian)?


thanks,
John Hubbard

> 
> Because Cursor::take_u64s() reads host-native 64-bit integers, using
> .as_bytes() reverses the byte layout on big-endian hosts compared to
> little-endian.
> 
> For example, if a 3-byte payload is stored inside a 64-bit word, a big-endian
> host will have the payload at the end of the word and zero-padding at the
> beginning, causing .get(..count) to extract the zeros instead of the actual
> data payload.
> 
>> +
>> +    /// Takes `count` 32-bit values. If `count` is not a multiple of 2 (`u32` values per `u64`),
>> +    /// bytes are discarded up to the next multiple.
>> +    fn take_u32s(&mut self, count: usize) -> Result<&'a [u32]> {
>> +        let values = self.take_u64s(count.div_ceil(size_of::<u64>() / size_of::<u32>()))?;
>> +        <[u32]>::ref_from_prefix_with_elems(values.as_bytes(), count)
>> +            .map(|(elems, _)| elems)
>> +            .map_err(|_| EINVAL)
>> +    }
> 
> [Severity: High]
> Can this result in swapped 32-bit words on big-endian systems?
> 
> Similar to the byte array extraction above, passing the native memory
> representation through .as_bytes() to recreate a [u32] slice will swap
> the order of the 32-bit halves on big-endian architectures compared to
> little-endian ones.
> 
> [ ... ]
>> +    /// Decodes every pair into `schema` and returns the result of [`Schema::finish`].
>> +    pub(crate) fn decode<'s, S: Schema + Visit<'a>>(
>> +        &self,
>> +        schema: &'s mut S,
>> +    ) -> Result<impl Init<S::Target, Error> + 's> {
>> +        let mut cursor = Cursor::new(self.data);
>> +        while !cursor.is_empty() {
>> +            let op: Op = cursor.take_u64()?.into();
> 
> [Severity: High]
> Does this misalign the operation bitfields on big-endian hosts?
> 
> The 64-bit operation word is read as a host-native integer via take_u64().
> If the NVKV stream comes from a little-endian GSP, parsing it directly as a
> native u64 on a big-endian host will misalign the key, index, and value
> fields inside Op.
> 
> Should this explicitly handle endianness (e.g., using little-endian read
> functions) instead of relying on native host integers?
>