Re: [PATCH 16/16] efi/arm64: Implement ARM64 DRTM in the stub

From: Jason Gunthorpe <jgg@nvidia.com>
To: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Cc: Alexandre Ghiti <alex@ghiti.fr>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Ard Biesheuvel <ardb@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jonathan Corbet <corbet@lwn.net>, David Sterba <dsterba@suse.com>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-doc@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-riscv@lists.infradead.org,
	Mark Rutland <mark.rutland@arm.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <pjw@kernel.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Simon Glass <sjg@chromium.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Nick Terrell <terrelln@fb.com>, Will Deacon <will@kernel.org>,
	Alexandre Ghiti <alexghiti@rivosinc.com>,
	Conor Dooley <conor.dooley@microchip.com>,
	linux-integrity@vger.kernel.org,
	Palmer Dabbelt <palmer@rivosinc.com>,
	patches@lists.linux.dev,
	Ross Philipson <ross.philipson@gmail.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Song Shuai <songshuaishuai@tinylab.org>
Subject: Re: [PATCH 16/16] efi/arm64: Implement ARM64 DRTM in the stub
Date: Sat, 26 Sep 2026 15:45:58 -0300	[thread overview]
Message-ID: <20260926184558.GB473004@nvidia.com> (raw)
In-Reply-To: <20260925113709.00005cd1@oss.qualcomm.com>

On Fri, Sep 25, 2026 at 11:37:09AM -0700, Jonathan Cameron wrote:
> > +efi_status_t efi_drtm_prepare(void)
> > +{
> > +	s64 feature_status;
> > +	u16 major, minor;
> > +	s32 status;
> > +
> > +	if (efi_drtm_policy == EFI_DRTM_OFF)
> > +		return EFI_SUCCESS;
> > +
> > +	if (!efi_arm64_psci_smccc_compatible())
> > +		return efi_drtm_failure();
> > +
> > +	/* VERSION must be the first DRTM call. */
> 
> Words like 'must' should be backed by a specific spec reference.
> I couldn't immediately fine one other than common sense suggesting it should
> be called to check we have a version we understand ho to talk to.

	/*
	 * v1.4B section 3.2.1 "DRTM_VERSION usage" explains that a new major
	 * ABI version may "Change behavior of existing functions". Verify the
	 * major version before calling anything so we don't trigger unknown
	 * behavior.
	 */
	status = arm_drtm_version(&major, &minor); 

> > +	/*
> > +	 * See arch/arm64/kernel/vmlinux.lds.S for the DRTM Memory layout. After
> > +	 * the DLME we choose to place the Normal World DCE region followed by
> > +	 * the aligned DRTM_PARAMETERS structure.
> > +	 */
> > +	memset((void *)dlme_start, 0, efi_drtm_get_extra_size());
> 
> Is that efi_drtm_get_extra_size() adding much? 

Yeah, it is the one place we compute the extra amount that was
allocated. How about:

	/*
	 * For robustness zero all the trailing space at the end of the
	 * alocation. This contains the DRTM_PARAMETERS structure too, so
	 * must be done before filling it.
	 */
	memset((void *)dlme_start, 0, efi_drtm_get_extra_size());

> Maybe it is worth keeping for the big comment in there, but it does
> feel like that and what we have here could be combined.

efi_drtm_get_extra_size() is called by the generic EFI code, I can't
remove it.

> > +	/*
> > +	 * DEN0113 section 3.4 returns from DYNAMIC_LAUNCH only on error. Boot
> 
> I'd use a spec version for references + ideally title of section.
> As much as folk may try, sometimes these things move around.

I tightened all of this

Thanks,
Jason