Re: [PATCH 2/2] gpu: nova-core: add support for GPU timer block. (v2)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpu: nova-core: GH100 FSP bug 3957833 WAR applied to Blackwell in timer_time()
--

commit fd289ae7ae729480f9ef4d6e43c04d6aa8f3dbe4
Author: Dave Airlie <airlied@redhat.com>

gpu: nova-core: add support for GPU timer block. (v2)

This commit adds GPU timer block initialization for nova-core to compute and
program the system timer offset passed to GSP initialization, with HAL
implementations for TU102 (Turing/Ampere/Ada) and GH100 (Hopper/Blackwell).

> diff --git a/drivers/gpu/nova-core/timer/hal.rs b/drivers/gpu/nova-core/timer/hal.rs
> new file mode 100644
> index 0000000000000..7aeff6cdc6787
> --- /dev/null
> +++ b/drivers/gpu/nova-core/timer/hal.rs
> @@ -0,0 +1,28 @@

[ ... ]

> +pub(super) fn timer_hal(chipset: Chipset) -> &'static dyn TimerHal {
> +    match chipset.arch() {
> +        Architecture::Turing | Architecture::Ampere | Architecture::Ada => tu102::TU102_HAL,
> +        Architecture::Hopper | Architecture::BlackwellGB10x | Architecture::BlackwellGB20x => {
> +            gh100::GH100_HAL
> +        }
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/timer/hal/gh100.rs b/drivers/gpu/nova-core/timer/hal/gh100.rs
> new file mode 100644
> index 0000000000000..7f55bf04a3f35
> --- /dev/null
> +++ b/drivers/gpu/nova-core/timer/hal/gh100.rs
> @@ -0,0 +1,76 @@

[ ... ]

> +/// FSP ucode version from which on the fix for bug 3957833 is included.
> +const FSP_BUG_3957833_FIX_VERSION: u32 = 0x44c;
> +
> +/// kfspRequiresBug3957833WAR_HAL: returns `true` if the FSP ucode predates the fix for bug
> +/// 3957833 and hence the sys timer offset must not be programmed.
> +fn fsp_requires_bug3957833_war(bar: Bar0<'_>) -> bool {
> +    bar.read(regs::gh100::NV_GFW_FSP_UCODE_VERSION).full().get() < FSP_BUG_3957833_FIX_VERSION
> +}
> +
> +fn timer_time(bar: Bar0<'_>, time_ns: i64) -> Result<i64> {

[ ... ]

> +    if !fsp_requires_bug3957833_war(bar) {
> +        let offset_ns = sys_timer_offset_ns as u64;
> +        bar.write_reg(
> +            regs::gh100::NV_PGC6_SCI_SYS_TIMER_OFFSET_1::zeroed()
> +                .try_with_nsec(((offset_ns >> 32) & 0x1fff_ffff) as u32)?,
> +        );
> +        bar.write_reg(
> +            regs::gh100::NV_PGC6_SCI_SYS_TIMER_OFFSET_0::zeroed()
> +                .try_with_nsec(((offset_ns >> 5) & 0x7ff_ffff) as u32)?
> +                .with_update(true),
> +        );
> +    }
> +    Ok(sys_timer_offset_ns)
> +}

[Severity: High]
Should fsp_requires_bug3957833_war() in drivers/gpu/nova-core/timer/hal/gh100.rs
be restricted to Hopper (GH100) rather than also running on Blackwell?

In drivers/gpu/nova-core/timer/hal.rs, timer_hal() routes Architecture::Hopper,
Architecture::BlackwellGB10x, and Architecture::BlackwellGB20x to
gh100::GH100_HAL:

    Gpu::new()
      -> Timer::new()
        -> hal::timer_hal(chipset).init(bar)
          -> Gh100::init()
            -> timer_time()
              -> fsp_requires_bug3957833_war()

In OpenRM, kfspRequiresBug3957833WAR_HAL is an FSP HAL method implemented only
for GH100 (comparing GH100's FSP ucode version against 0x44c) and stubbed to
return false on Blackwell, which has separate FSP firmware version numbering.

When running on Blackwell (BlackwellGB10x or BlackwellGB20x), if the value in
NV_GFW_FSP_UCODE_VERSION is below 0x44c, won't timer_time() falsely trigger
the GH100 workaround and skip programming NV_PGC6_SCI_SYS_TIMER_OFFSET_1 and
NV_PGC6_SCI_SYS_TIMER_OFFSET_0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-nova-timer-v1-0-b347ead64469@redhat.com?part=2