Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Thu, 18 Dec 2025 16:48:13 +0900
Message-Id: <DF16DSHX4SV0.MI79333PZ8NE@nvidia.com>
Subject: Re: [PATCH v4 08/11] gpu: nova-core: add Falcon HAL method
 supports_dma()
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Timur Tabi" <ttabi@nvidia.com>, "Danilo Krummrich" <dakr@kernel.org>,
 "Alexandre Courbot" <acourbot@nvidia.com>, "Joel Fernandes"
 <joelagnelf@nvidia.com>, "John Hubbard" <jhubbard@nvidia.com>,
 <nouveau@lists.freedesktop.org>, <rust-for-linux@vger.kernel.org>
References: <20251218032955.979623-1-ttabi@nvidia.com>
 <20251218032955.979623-9-ttabi@nvidia.com>
In-Reply-To: <20251218032955.979623-9-ttabi@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?TFdGVVd0K05XSmJ4R0pNblZnZHlMY2FkZVkvQ2ZITFV3amtVQnl1dXNVclVD?=
 =?utf-8?B?V3F2VnlhWWU4SGRMY2IrZ2tpYWgxcVp1WSs3dkZ2WkVrcHlDMy9VRXh0TUNZ?=
 =?utf-8?B?QzZjMWE3U21TWm1CTjFYN25zQks3SXJaMVRFNUhzMmZYbUkyNTBzQXBaTXpp?=
 =?utf-8?B?Rmg0S3FLU2wwYXJFZHkxV1ZGRFJ3Ny96MUVqLzE3MzNyK1pneEdNQ1VRMWV2?=
 =?utf-8?B?OFEvV2w2MjkxNzl4dGJBbU9ERWdTanoyR0F5V1F4NTRqdDVPcXFRcnpTYTRH?=
 =?utf-8?B?ZzFaNER0dEdacDI0bnpKRTJ6MGd3ampLQXVEUWk2RDJ3QjJ4U2oyY2NaUmMr?=
 =?utf-8?B?RGxQSmJZK2d1Mm9nT2hpRmU2T0o1czNJeTZaRS9iM2pIdW43RG1aQytHaDdT?=
 =?utf-8?B?b1QxV0plMnBGOGIyY3ptc0ZVNWpmajZZa0g2czhVcmFoQ3I1UkNIK2xqRmFL?=
 =?utf-8?B?UFI0T25ZZUQ1OG82NDFZZlRJUDRud2dRTXU4Uy91Y1N1aVJwV2pRSUYrUFBU?=
 =?utf-8?B?R3FodG90aUdQc2tzanV0T0NiLzA4VU9FaWNLN1NnT3gxbDUrMktPb2pjem5J?=
 =?utf-8?B?Z0JJL3o5UlhwdlJqMC9MZFpmcW44WlcwelpjV29kYVBNdDBrd25GbTBMVmFt?=
 =?utf-8?B?alZzTXBCRGw2UFNlUnlMNzVXeVg1Y01FNGRIN2xSdk9LeStiN2dkNUdCZW14?=
 =?utf-8?B?eTJ1RGJJMFA0MnRZQmdOL2d5MTZIekNVOXIvdUxGbThMWFh3bERnUmNnbFN3?=
 =?utf-8?B?cFZLQ1M5Z3dFSWZXcEp0WFNiYkJvdGFFTDRXeFpCYzV0SGZZcnoyeGVBUXRz?=
 =?utf-8?B?MTlucCtGSlpyNHBMTWZKNC9vaTNUVnIrV0l0WWkrK0J1RU94RUFvVkdzZlJv?=
 =?utf-8?B?bzA2SWNoRml1TEwzOHhFU0tEUTZhR0JWL3ZGL2hCbXQ0dW81N3FJdzF4c1c1?=
 =?utf-8?B?dGlLeGdMeW16NTk1MkEzM3hjZnRCMHB0OUZCRFNZZjNHaTJ4dnJMS1dtMW9I?=
 =?utf-8?B?V3J3TmY1SW8vOTFqVUF0RnUyc2tyTVgvcHc3dG5UWVM2N3hMb080cU1Pb2w5?=
 =?utf-8?B?T1h1dmd6c1luS3pEZnhlK3VhMHZFYzBINTlZbWpTYm5SQWVJMVlrLzR5ZVIx?=
 =?utf-8?B?Q1BWb09ndXhHbEJxWlhURkw0M2pTRlRKdmk2OWVCazM4ajQwNW5oSzdlZjlU?=
 =?utf-8?B?aVNiK3N1TkZteFBXYjlhWGdheGtrK1ZYMXo2aDRaQ3VYQ0IwRFNydERXOGF2?=
 =?utf-8?B?M0pOaVBzc2w5cHU0Y2Q3bHlRdDZwSUsyMTZpcmsraWJ2cFFKR0o5OExFd0xB?=
 =?utf-8?B?anY3Ulk3aGgwQlFTMVVjdjlVdFdnUE5ZaGtZWnZyOEtwblN0Y0E1RlFNdnNG?=
 =?utf-8?B?MDZnWmwwTFpNRFNvcXkwWXdwbUFscEQ1aCtJdVFIcHkwNko5RjBQRS95U3NS?=
 =?utf-8?B?TnRTOFhnQ01lUFFSUSt2M3JCazcxV1JDeHhPUmpjbkU4MCtBRmp6c2NXdGVB?=
 =?utf-8?B?c3NIRWFELzY2c1NkRGhzMTcvWlJYT29tSFRzRE5qNnVPazFGYThtbjlRNTlP?=
 =?utf-8?B?MXZZRUFIYTRDRWUxeGJBdHZhNERVcHJLY3pMNnNyRHRCbmVoWWlEVy9keDh4?=
 =?utf-8?B?bXloVHVDMGtTcHBjSVlOT2R5dnlZcXNGemNZUk5IbGZTeEg2U3EyejhROFdv?=
 =?utf-8?B?Ti9wdDJwZy9wWTROaEN2Mm9PaHFnUlRaRXJ0dWhRTUhLbnJKMHhiUHExaEVQ?=
 =?utf-8?B?a1J4VnRNb280T0pMY3RtZGlmajlBNGtib21hbXhvSVhlQlUrOXc5eFBUUW53?=
 =?utf-8?B?amo0VFlZZXMxMHBNbHVxVlV6empWRXp0VThWSzJUQzhaRFp5M1VFVmFWaEp6?=
 =?utf-8?B?RkpaZm4zU1cxcVlsaU5Nc3FEZk5yYmtZUGU0cUFtcDFLbHlOWEJ4L0xFQ2Ru?=
 =?utf-8?B?bkFXVHh4QXdxbnh2dkpMS1lSMnQ2Z3V3UUd5S25JZlN1bnNOREI0YSs0cHcw?=
 =?utf-8?B?TzlwOEtVYksrWTBRcXlncHBGRUVIVUVHVUEyMGNua3NVVXFtSkRQSmFFbExN?=
 =?utf-8?B?YkQyYzJYL01kSDVYU2lEbExMbzRVeDY3WVNzOG1QVEpaeXU2SGNhOVhiNGVX?=
 =?utf-8?B?OVFuc1hhb2Y2MWRMUmxYeE53czREK1Avd2lOTmR0S29RcnJtZDQ3NUZ1TnJm?=
 =?utf-8?Q?fqKY1B1PyhrRrVpPTl1CAgu6kFlr73z3Yirz60eOmczN?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fd84f504-7283-4403-f3da-08de3e09ceaa
X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB3990.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Dec 2025 07:48:17.8623
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gOzlnVn6YPnQuHtI094YQ119bFaUyeiGvwtKXKNiN4WLJ0va2SF8Prp+m97N15js4RzgOgctbE+ahRBpdm+lLQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB7550

On Thu Dec 18, 2025 at 12:29 PM JST, Timur Tabi wrote:
> Some GPUs do not support using DMA to transfer code/data from system
> memory to Falcon memory, and instead must use programmed I/O (PIO).
> Add a function to the Falcon HAL to indicate whether a given GPU's
> Falcons support DMA for this purpose.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/falcon.rs           | 7 +++++++
>  drivers/gpu/nova-core/falcon/hal.rs       | 3 +++
>  drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 ++++
>  drivers/gpu/nova-core/falcon/hal/tu102.rs | 4 ++++
>  4 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falc=
on.rs
> index 6b54c0ca458a..50c87dadf2ea 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -630,6 +630,13 @@ pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> =
bool {
>          self.hal.is_riscv_active(bar)
>      }
> =20
> +    /// Check if this Falcon supports DMA for loading firmware.
> +    ///
> +    /// Returns `true` if DMA is supported, `false` if PIO must be used =
instead.
> +    pub(crate) fn supports_dma(&self) -> bool {
> +        self.hal.supports_dma()
> +    }

Rather than this, we should make the selection of the loading method
transparent to the caller. Right now `boot.rs` needs to calls
`supports_dma` itself and choose the right method, which leaves room for
error.

I'd suggest the following:

- A `fn load(bar, fw) -> Result` method in the Falcon HAL that redirects
  to either `load_dma` or `load_pio`.
- A similarly-named public function in `Falcon` for the boot code to
  call,
- `load_dma` and `load_pio` becomes private - that way the HAL can call
  them, but not the user code.

Now we have the problem that the generic bootloader argument is only
useful for PIO, but the way it is handled is a bit too ad-hoc to my
taste. I wonder if we could pass a different `FalconFirmware` structure
when it is used so the same code can do the correct thing.