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

On Thu, Sep 24, 2026 at 05:29:52PM -0700, Jonathan Cameron wrote:

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.

I wasn't sure what to do with this since the spec had the full hex and
we have these helper macros.. I made it look more like something else
with the offset style instead of the function call style:

quoted

+#define ARM_DRTM_SMC_VERSION			(ARM_DRTM_SMC_FN_BASE + 0x00)

IDK, maybe ARM_DRTM_SMC_FN_BASE(0x110) is more connected to the spec,
but I'm used to doing it like this from ioctl land which is very
similar.

quoted

+#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.

OK

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.

I reorganized everything

quoted

+#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?  

I did it with a field content which gets reused in the launch struct:

/*
 * PCR schema numbers. ARM_DRTM_PCR_SCHEMA_MASK is a bitmap indexed by these,
 * and the Launch Features ARM_DRTM_LAUNCH_PCR_SCHEMA_MASK field holds one.
 */
#define ARM_DRTM_PCR_SCHEMA_DEFAULT		0
#define ARM_DRTM_PCR_SCHEMA_AUTHORITIES		1

/*
 * DMA protection types. ARM_DRTM_DMA_PROTECTION_MASK is a bitmap indexed by
 * these, and the Launch Features ARM_DRTM_LAUNCH_DMA_PROTECTION_MASK field
 * holds one.
 */
#define ARM_DRTM_DMA_PROTECTION_COMPLETE	0
#define ARM_DRTM_DMA_PROTECTION_REGION		1

Have to use BIT() to get the bitmap encoding.

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?

I added comments and spec references for everything

quoted

+#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.

I think that would be too verbose in this case, and the usage is very
limited.

quoted

+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.

I read the spec as 0 is not allowed.

I changed this around to have two wrapper functions, which works a bit
better.

static inline s64 arm_drtm_query_function(u64 function_id)
{
	struct arm_smccc_res res;

	arm_smccc_1_1_smc(ARM_DRTM_SMC_FEATURES, function_id, 0, 0, 0, 0, 0, 0,
			  &res);
	return res.a0;
}

/*
 * v1.4b Section 3.3.1 says:
 *   A return value of greater than 0 means the feature ID is
 *   implemented, and there are other feature-specific
 *   feature or capability bits available in other return value
 *   registers.
 * And for FEATURE_SELECTOR all defined features return data in other
 * registers, so 0 is not an allowed return.
 */
static inline s64 arm_drtm_query_feature(u64 feature, u64 *value)
{
	struct arm_smccc_res res;

	arm_smccc_1_1_smc(ARM_DRTM_SMC_FEATURES,
			  ARM_DRTM_FEATURE_SELECTOR |
				  FIELD_PREP(ARM_DRTM_FEATURE_ID_MASK, feature),
			  0, 0, 0, 0, 0, 0, &res);
	*value = res.a1;
	return res.a0;
}

Thanks,
Jason