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
