[PATCH] gpu: nova-core: require little endian

Eliot Courtney posted 1 patch 2 months, 1 week ago
drivers/gpu/nova-core/Kconfig | 1 +
1 file changed, 1 insertion(+)
[PATCH] gpu: nova-core: require little endian
Posted by Eliot Courtney 2 months, 1 week ago
The driver already assumes little endian in a lot of locations. For
example, all the code that reads RPCs out of the command queue just
directly interprets the bytes.

Make this explicit in Kconfig.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
The current code assumes little endian in a bunch of places. I think we
should either explicitly decide to be generic on endianness or explicitly
decide not to - having some handling sprinkled around in various
locations seems confusing to me.

I believe that currently e.g. `RUST` transitively depends on
!CPU_BIG_ENDIAN, so this is more about making the decision explicit for
nova-core rather than fixing any kind of hole.
---
 drivers/gpu/nova-core/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
index a4f2380654e2..d8456f8eaa05 100644
--- a/drivers/gpu/nova-core/Kconfig
+++ b/drivers/gpu/nova-core/Kconfig
@@ -3,6 +3,7 @@ config NOVA_CORE
 	depends on 64BIT
 	depends on PCI
 	depends on RUST
+	depends on !CPU_BIG_ENDIAN
 	select AUXILIARY_BUS
 	select RUST_FW_LOADER_ABSTRACTIONS
 	default n

---
base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd
change-id: 20260406-fix-kconfig-3a059f622697

Best regards,
--  
Eliot Courtney <ecourtney@nvidia.com>
Re: [PATCH] gpu: nova-core: require little endian
Posted by Eliot Courtney 2 months, 1 week ago
On Mon Apr 6, 2026 at 3:52 PM JST, Eliot Courtney wrote:
> The driver already assumes little endian in a lot of locations. For
> example, all the code that reads RPCs out of the command queue just
> directly interprets the bytes.
>
> Make this explicit in Kconfig.
>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> The current code assumes little endian in a bunch of places. I think we
> should either explicitly decide to be generic on endianness or explicitly
> decide not to - having some handling sprinkled around in various
> locations seems confusing to me.
>
> I believe that currently e.g. `RUST` transitively depends on
> !CPU_BIG_ENDIAN, so this is more about making the decision explicit for
> nova-core rather than fixing any kind of hole.
> ---
>  drivers/gpu/nova-core/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> index a4f2380654e2..d8456f8eaa05 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -3,6 +3,7 @@ config NOVA_CORE
>  	depends on 64BIT
>  	depends on PCI
>  	depends on RUST
> +	depends on !CPU_BIG_ENDIAN
>  	select AUXILIARY_BUS
>  	select RUST_FW_LOADER_ABSTRACTIONS
>  	default n
>
> ---
> base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd
> change-id: 20260406-fix-kconfig-3a059f622697
>
> Best regards,
> --  
> Eliot Courtney <ecourtney@nvidia.com>

Thanks all. Will resend with the change to nova-drm as well.
Re: [PATCH] gpu: nova-core: require little endian
Posted by Danilo Krummrich 2 months, 1 week ago
On Mon Apr 6, 2026 at 8:52 AM CEST, Eliot Courtney wrote:
> The driver already assumes little endian in a lot of locations. For
> example, all the code that reads RPCs out of the command queue just
> directly interprets the bytes.
>
> Make this explicit in Kconfig.
>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> The current code assumes little endian in a bunch of places. I think we
> should either explicitly decide to be generic on endianness or explicitly
> decide not to - having some handling sprinkled around in various
> locations seems confusing to me.
>
> I believe that currently e.g. `RUST` transitively depends on
> !CPU_BIG_ENDIAN, so this is more about making the decision explicit for
> nova-core rather than fixing any kind of hole.
> ---
>  drivers/gpu/nova-core/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> index a4f2380654e2..d8456f8eaa05 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -3,6 +3,7 @@ config NOVA_CORE
>  	depends on 64BIT
>  	depends on PCI
>  	depends on RUST
> +	depends on !CPU_BIG_ENDIAN

Since we have DRM_NOVA select NOVA_CORE, this has to be added to DRM_NOVA as
well, otherwise you could forcefully select NOVA_CORE through DRM_NOVA without
considering !CPU_BIG_ENDIAN.

Note DRM_NOVA intentionally has select NOVA_CORE (instead of depends on), since
otherwise a user would need to enable NOVA_CORE in order to see DRM_NOVA in the
menuconfig in the first place.

>  	select AUXILIARY_BUS
>  	select RUST_FW_LOADER_ABSTRACTIONS
>  	default n
>
> ---
> base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd
> change-id: 20260406-fix-kconfig-3a059f622697
>
> Best regards,
> --  
> Eliot Courtney <ecourtney@nvidia.com>
Re: [PATCH] gpu: nova-core: require little endian
Posted by John Hubbard 2 months, 1 week ago
On 4/5/26 11:52 PM, Eliot Courtney wrote:
> The driver already assumes little endian in a lot of locations. For
> example, all the code that reads RPCs out of the command queue just
> directly interprets the bytes.

Yes, and that even understates the scope. The FromBytes-based parsing
of repr(C) structs happens in firmware loading, VBIOS parsing, and GSP
shared memory, not just command queue RPCs.

> 
> Make this explicit in Kconfig.
> 
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> The current code assumes little endian in a bunch of places. I think we
> should either explicitly decide to be generic on endianness or explicitly
> decide not to - having some handling sprinkled around in various
> locations seems confusing to me.
> 
> I believe that currently e.g. `RUST` transitively depends on
> !CPU_BIG_ENDIAN, so this is more about making the decision explicit for
> nova-core rather than fixing any kind of hole.

That's true today in practice, but config RUST does not directly depend
on !CPU_BIG_ENDIAN. The exclusion happens per-arch: ARM and ARM64 gate
HAVE_RUST on CPU_LITTLE_ENDIAN, and x86_64 and LoongArch are inherently
LE. 

RISC-V is more exciting because it selects HAVE_RUST without an explicit
endianness check.

So putting the dependency in nova-core directly is a good move, not just
for documentation but as a real guard.

> ---
>  drivers/gpu/nova-core/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> index a4f2380654e2..d8456f8eaa05 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -3,6 +3,7 @@ config NOVA_CORE
>  	depends on 64BIT
>  	depends on PCI
>  	depends on RUST
> +	depends on !CPU_BIG_ENDIAN
>  	select AUXILIARY_BUS
>  	select RUST_FW_LOADER_ABSTRACTIONS
>  	default n
> 
> ---
> base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd
> change-id: 20260406-fix-kconfig-3a059f622697
> 

Reviewed-by: John Hubbard <jhubbard@nvidia.com>

thanks,
-- 
John Hubbard
Re: [PATCH] gpu: nova-core: require little endian
Posted by Gary Guo 2 months, 1 week ago
On Mon Apr 6, 2026 at 7:52 AM BST, Eliot Courtney wrote:
> The driver already assumes little endian in a lot of locations. For
> example, all the code that reads RPCs out of the command queue just
> directly interprets the bytes.
>
> Make this explicit in Kconfig.
>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> The current code assumes little endian in a bunch of places. I think we
> should either explicitly decide to be generic on endianness or explicitly
> decide not to - having some handling sprinkled around in various
> locations seems confusing to me.
>
> I believe that currently e.g. `RUST` transitively depends on
> !CPU_BIG_ENDIAN, so this is more about making the decision explicit for
> nova-core rather than fixing any kind of hole.

IBM is adding PowerPC support which will be the first BE architecture that RfL
is going to support. However, only 32-bit BE is going to be added soon, so
`depends on 64BIT` will prevent Nova from supporting that.

So I think it's good that we put it in.

Acked-by: Gary Guo <gary@garyguo.net>

Best,
Gary

> ---
>  drivers/gpu/nova-core/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> index a4f2380654e2..d8456f8eaa05 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -3,6 +3,7 @@ config NOVA_CORE
>  	depends on 64BIT
>  	depends on PCI
>  	depends on RUST
> +	depends on !CPU_BIG_ENDIAN
>  	select AUXILIARY_BUS
>  	select RUST_FW_LOADER_ABSTRACTIONS
>  	default n
>
> ---
> base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd
> change-id: 20260406-fix-kconfig-3a059f622697
>
> Best regards,
> --  
> Eliot Courtney <ecourtney@nvidia.com>
Re: [PATCH] gpu: nova-core: require little endian
Posted by Joel Fernandes 2 months, 1 week ago

On 4/6/2026 2:52 AM, Eliot Courtney wrote:
> The driver already assumes little endian in a lot of locations. For
> example, all the code that reads RPCs out of the command queue just
> directly interprets the bytes.

Oh yeah, its not just the driver making assumptions, it is the hardware itself.
The Nvidia GPU architecture is little-endian (including MMU structures in VRAM).

> 
> Make this explicit in Kconfig.
> 
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> The current code assumes little endian in a bunch of places. I think we
> should either explicitly decide to be generic on endianness or explicitly
> decide not to - having some handling sprinkled around in various
> locations seems confusing to me.
> 
> I believe that currently e.g. `RUST` transitively depends on
> !CPU_BIG_ENDIAN, so this is more about making the decision explicit for
> nova-core rather than fixing any kind of hole.
> ---
>  drivers/gpu/nova-core/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> index a4f2380654e2..d8456f8eaa05 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -3,6 +3,7 @@ config NOVA_CORE
>  	depends on 64BIT
>  	depends on PCI
>  	depends on RUST
> +	depends on !CPU_BIG_ENDIAN

If we want to do this, I am Ok. Although I am also Ok with the potential
transitive Rust dependency in which case the only value of adding it in Kconfig
is documentation. But just a side note (not a rejection comment), Nouveau does
not do this, and no other GPU driver afaics either.

thanks,

-- 
Joel Fernandes
Re: [PATCH] gpu: nova-core: require little endian
Posted by Danilo Krummrich 2 months, 1 week ago
On Mon Apr 6, 2026 at 6:30 PM CEST, Joel Fernandes wrote:
> Nouveau does not do this, and no other GPU driver afaics either.

Nouveau has nvkm_device_endianness() [1], which it configured before everything
else is done.

I assume that's not an option for later GPUs?

[1] https://elixir.bootlin.com/linux/v6.19.11/source/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c#L3111
Re: [PATCH] gpu: nova-core: require little endian
Posted by Timur Tabi 2 months, 1 week ago
On Tue, 2026-04-07 at 12:56 +0200, Danilo Krummrich wrote:
> On Mon Apr 6, 2026 at 6:30 PM CEST, Joel Fernandes wrote:
> > Nouveau does not do this, and no other GPU driver afaics either.
> 
> Nouveau has nvkm_device_endianness() [1], which it configured before everything
> else is done.
> 
> I assume that's not an option for later GPUs?
> 
> [1]
> https://elixir.bootlin.com/linux/v6.19.11/source/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c#L3111

Pascal is the last GPU to have NV_PMC_BOOT_1_ENDIAN0.  If nvkm_device_endianness() is called on
Turing, then that might be a bug.
Re: [PATCH] gpu: nova-core: require little endian
Posted by Joel Fernandes 2 months, 1 week ago

On 4/7/2026 2:02 PM, Timur Tabi wrote:
> On Tue, 2026-04-07 at 12:56 +0200, Danilo Krummrich wrote:
>> On Mon Apr 6, 2026 at 6:30 PM CEST, Joel Fernandes wrote:
>>> Nouveau does not do this, and no other GPU driver afaics either.
>>
>> Nouveau has nvkm_device_endianness() [1], which it configured before everything
>> else is done.
>>
>> I assume that's not an option for later GPUs?
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.19.11/source/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c#L3111
> 
> Pascal is the last GPU to have NV_PMC_BOOT_1_ENDIAN0.  If nvkm_device_endianness() is called on
> Turing, then that might be a bug.

Yes, this was on old/legacy GPUs, not GSP-based ones.

thanks,

--
Joel Fernandes