Re: [PATCH v3 10/10] gpu: nova-core: mm: Add PRAMIN aperture self-tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The misalignment selftest for `u64` will fail on 32-bit architectures because `align_of::<u64>()` is 4, not 8.
- [Low] Compilation error when CONFIG_NOVA_CORE_SELFTESTS is enabled: calling `run_selftests` on an unpinned mutable reference.
--

commit 0fa1da2e573acbda3f74539dc8e29525a6f4dfba
Author: Joel Fernandes <joelagnelf@nvidia.com>

gpu: nova-core: mm: Add PRAMIN aperture self-tests

This commit adds self-tests for the PRAMIN aperture mechanism to verify
correct operation during GPU probe. The tests validate various alignment
requirements and corner cases, and are gated behind the
CONFIG_NOVA_CORE_SELFTESTS configuration.

> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -85,6 +85,9 @@ fn probe<'bound>(
>                  // (try_pin_init!() initializes fields in declaration order), lives at a pinned
>                  // stable address, and is dropped after gpu (struct field drop order).
>                  gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }),
> +                // Run optional GPU selftests.
> +                #[cfg(CONFIG_NOVA_CORE_SELFTESTS)]
> +                _: { gpu.run_selftests(pdev) },

[Severity: Low]
Will this cause a compilation error in the probe function when
CONFIG_NOVA_CORE_SELFTESTS is enabled?

The gpu field exposed within the try_pin_init closure is a standard mutable
reference, but the run_selftests method expects a pinned mutable reference.
Since these types do not automatically coerce, this looks like it will result
in a build failure during probe execution.

[ ... ]
> diff --git a/drivers/gpu/nova-core/mm/pramin.rs b/drivers/gpu/nova-core/mm/pramin.rs
> --- a/drivers/gpu/nova-core/mm/pramin.rs
> +++ b/drivers/gpu/nova-core/mm/pramin.rs
[ ... ]
> +    /// Test that misaligned accesses are rejected.
> +    fn test_misaligned_access(
> +        dev: &device::Device<device::Bound>,
> +        pramin: &mut Pramin<'_>,
> +        base: VramAddress,
> +    ) -> Result {
> +        // u16 at odd offset (not 2-byte aligned).
> +        selftest_assert!(dev, pramin.window_at::<u16>(base + 0x21).is_err());
> +
> +        // u32 at 2-byte-aligned (not 4-byte-aligned) offset.
> +        selftest_assert!(dev, pramin.window_at::<u32>(base + 2).is_err());
> +
> +        // u64 at a 4-byte-aligned (not 8-byte-aligned) address.
> +        selftest_assert!(dev, pramin.window_at::<u64>(base + 0x44).is_err());

[Severity: Medium]
Will this assertion in test_misaligned_access fail on 32-bit architectures?

On 32-bit systems, the memory alignment requirement for a u64 is typically 4
bytes rather than 8 bytes. Since the offset 0x44 is a multiple of 4, the
window_at call would succeed rather than returning an error, which seems
like it would cause a spurious selftest failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-pramin-split-v3-0-24b24d7afc52@nvidia.com?part=10