Re: [PATCH 12/16] arm64: drtm: Add macro definitions for DEN0113

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Thu, 24 Sep 2026 10:53:15 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> Taken from rev 1.4b of the spec, following the SMCC register layout and
> constant names.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  arch/arm64/include/asm/drtm.h | 196 ++++++++++++++++++++++++++++++++++
>  1 file changed, 196 insertions(+)
>  create mode 100644 arch/arm64/include/asm/drtm.h
> 
> diff --git a/arch/arm64/include/asm/drtm.h b/arch/arm64/include/asm/drtm.h
> new file mode 100644
> index 00000000000000..d6dd04a4ebcea5
> --- /dev/null
> +++ b/arch/arm64/include/asm/drtm.h
> @@ -0,0 +1,196 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES
> + *
> + * Definitions from ARM DEN 0113 "DRTM Architecture for Arm"
> + */
> +#ifndef __ASM_DRTM_H
> +#define __ASM_DRTM_H
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/bits.h>
> +
> +/* Offset 0x02 is reserved by DEN0113. */
> +#define ARM_DRTM_SMC_FN_BASE                                      \
> +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64, \
> +			   ARM_SMCCC_OWNER_STANDARD, 0x110)

Another wrapper for the same thing. Now which patch set did I last
moan about this in...  RMM 2.0 firmware series.
https://lore.kernel.org/all/d7ccaf22-5bde-4344-8bf0-a5a17dcac0f3@arm.com/

This one at least does it via base and sum.  But that then separates it from
the spec?   That is sort of the case anyway because the spec has the full
number.  0xC400_0110 so I guess not too bad.

> +#define ARM_DRTM_SMC_VERSION			(ARM_DRTM_SMC_FN_BASE + 0x00)



> +#define ARM_DRTM_FEATURE_SELECTOR	BIT_U64(63)
> +#define ARM_DRTM_FEATURE_TPM		0x01
> +#define ARM_DRTM_FEATURE_MIN_MEMORY	0x02
> +#define ARM_DRTM_FEATURE_DMA_PROTECTION	0x03
> +#define ARM_DRTM_FEATURE_BOOT_PE	0x04
> +#define ARM_DRTM_FEATURE_TCB_HASH	0x05
> +#define ARM_DRTM_FEATURE_IMAGE_AUTH	0x06

Nice to have a mask for the 8 bits of the feature field.
Also nice to keep order the same as the SMC defines which would put this after
version.  That also puts it next to the values returned for each feature.



> +
> +#define ARM_DRTM_TPM_ALG_MASK		GENMASK_U64(15, 0)
> +#define ARM_DRTM_TPM_HASHING		BIT_U64(32)
> +#define ARM_DRTM_PCR_SCHEMA_MASK	GENMASK_U64(36, 33)
> +#define ARM_DRTM_PCR_SCHEMA_DEFAULT	BIT_U64(33)
> +#define ARM_DRTM_PCR_SCHEMA_AUTHORITIES	BIT_U64(34)

Ah.  Always love a field made up of a bunch of bits. Hard to define
cleanly.   Do you actually need the PCR_SCHEMA_MASK?  Might be easier
to just not have it?  I think you only use it for a print. Maybe
build the bits we understand from the two specific bits?

Alternatively you could use the spec style definition and have 
#define ARM_DRTM_PCR_SCHEMA_DEFAULT BIT(0)
#define ARM_DRTM_PCR_SCHEMA_AUTHORITIES BIT(1)
or similar and have to check them via a FIELD_GET(FIELD_GET())



> +
> +#define ARM_DRTM_DLME_DATA_PAGES_MASK	GENMASK_U64(31, 0)
> +#define ARM_DRTM_NW_DCE_PAGES_MASK	GENMASK_U64(63, 32)
> +#define ARM_DRTM_PAGE_SIZE		4096
> +
> +#define ARM_DRTM_DMA_PROTECTION_MASK	GENMASK_U64(7, 0)
> +#define ARM_DRTM_DMA_PROTECTION_COMPLETE BIT_U64(0)
> +#define ARM_DRTM_DMA_PROTECTION_REGION	BIT_U64(1)

Ah. They are at it again. Fields within fields.
Maybe could use some white space to make it somewhat obvious
that is going on?  Indent the subfield a bit more?


> +#define ARM_DRTM_MAX_REGIONS_MASK	GENMASK_U64(23, 8)

blank line here probably just to make it obvious going to a
different u64.

> +#define ARM_DRTM_TCB_HASH_COUNT_MASK	GENMASK_U64(7, 0)

Same here.  I vaguely wonder if it is worth adding something
reflecting the relevant feature ID to each of these defines
so we know what the are referring to?

> +#define ARM_DRTM_IMAGE_AUTH_SUPPORTED	BIT_U64(0)
> +

Maybe a comment for next lot to where to find them in the spec
- took me a while.  Table 9 DTRM_Parameters, line for
LAUNCH_FEATURES if anyone is following along.

> +#define ARM_DRTM_LAUNCH_HASH_FIRMWARE	0
> +#define ARM_DRTM_LAUNCH_HASH_TPM	BIT_U32(0)

I'd rather see them as fields and field value pairs but
can see that is going to get a bit verbose.

> +#define ARM_DRTM_LAUNCH_PCR_DEFAULT	0
> +#define ARM_DRTM_LAUNCH_PCR_AUTHORITIES	BIT_U32(1)
> +#define ARM_DRTM_LAUNCH_DMA_COMPLETE	0
> +#define ARM_DRTM_LAUNCH_DMA_REGION	BIT_U32(3)
> +#define ARM_DRTM_LAUNCH_NO_AUTH		0
> +#define ARM_DRTM_LAUNCH_AUTH		BIT_U32(6)
> +#define ARM_DRTM_LAUNCH_KEEP_SECURE_IRQS 0
> +#define ARM_DRTM_LAUNCH_DISABLE_SECURE_IRQS BIT_U32(7)
> +
> +#define ARM_DRTM_SUCCESS		0
> +#define ARM_DRTM_NOT_SUPPORTED		-1
> +#define ARM_DRTM_INVALID_PARAMETERS	-2
> +#define ARM_DRTM_DENIED			-3
> +#define ARM_DRTM_NOT_FOUND		-4
> +#define ARM_DRTM_INTERNAL_ERROR		-5
> +#define ARM_DRTM_MEM_PROTECT_INVALID	-6

Hmm. the spec I pulled from arm.com has COPROCESSOR_ERROR for -7. Maybe
include it?

> +#define ARM_DRTM_OUT_OF_RESOURCES	-8
> +#define ARM_DRTM_INVALID_DATA		-9
> +#define ARM_DRTM_SECONDARY_PE_NOT_OFF	-10
> +#define ARM_DRTM_ALREADY_CLOSED		-11
> +#define ARM_DRTM_TPM_ERROR		-12
> +
> +/* Algorthim IDs are defined by TCG, in the kernel they are TPM_ALG_* */
> +
> +#define ARM_DRTM_PARAMETERS_REVISION	2
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <linux/bitfield.h>
> +#include <linux/build_bug.h>
> +#include <linux/stddef.h>
> +#include <linux/types.h>

> +
> +static inline s64 arm_drtm_features(u64 function_or_feature, u64 *value)
> +{
> +	struct arm_smccc_res res;
> +	s64 status;
> +
> +	arm_smccc_1_1_smc(ARM_DRTM_SMC_FEATURES, function_or_feature,
> +			  0, 0, 0, 0, 0, 0, &res);
> +	status = res.a0;
> +	if (status >= 0 && value)
> +		*value = res.a1;
If status == 0, is res.a1 useful?
You have a helpful comment at the call site in the final patch but none
the less I have read the spec section a couple of times and have no idea.

> +	return status;
> +}

Thanks,

Jonathan



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv