Re: [PATCH v5 2/5] tsm-mr: Add tsm-mr sample code

Dan Williams
2025-05-01T19:36:31Z

Cedric Xing wrote:
> This sample kernel module demonstrates how to make MRs accessible to user
> mode through the tsm-mr library.
>
> Once loaded, this module registers a `miscdevice` that host a set of
> emulated measurement registers as shown in the directory tree below.
>
> /sys/class/misc/tsm_mr_sample
> └── mr
> ├── config_mr
> ├── report_digest:sha512
> ├── rtmr0:sha256
> ├── rtmr1:sha384
> ├── rtmr_crypto_agile:sha256
> ├── rtmr_crypto_agile:sha384
> └── static_mr:sha384
>
> Among the MRs in this example:
>
> - `config_mr` demonstrates a hashless MR, like MRCONFIGID in Intel TDX or
> HOSTDATA in AMD SEV.
> - `static_mr` demonstrates a static MR. The suffix `:sha384` indicates its
> value is a sha384 digest.
> - `rtmr0` is an RTMR with `TSM_MR_F_WRITABLE` **cleared**, preventing
> direct extensions; as a result, the attribute `rtmr0:sha256` is
> read-only.
> - `rtmr1` is an RTMR with `TSM_MR_F_WRITABLE` **set**, permitting direct
> extensions; thus, the attribute `rtmr1:sha384` is writable.
> - `rtmr_crypto_agile` demonstrates a "single" MR that supports multiple
> hash algorithms. Each supported algorithm has a corresponding digest,
> usually referred to as a "bank" in TCG terminology. In this specific
> sample, the 2 banks are aliased to `rtmr0` and `rtmr1`, respectively.
> - `report_digest` contains the digest of the internal report structure
> living in this sample module's memory. It is to demonstrate the use of
> the `TSM_MR_F_LIVE` flag. Its value changes each time an RTMR is
> extended.
>
> Signed-off-by: Cedric Xing <cedric.xing@intel.com>
> ---
> MAINTAINERS | 1 +
> samples/Kconfig | 11 ++++
> samples/Makefile | 1 +
> samples/tsm-mr/Makefile | 2 +
> samples/tsm-mr/tsm_mr_sample.c | 137 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 152 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1d1426af3a68..fd5bbf78ec8b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24618,6 +24618,7 @@ S: Maintained
> F: Documentation/ABI/testing/configfs-tsm
> F: drivers/virt/coco/tsm*.c
> F: include/linux/tsm*.h
> +F: samples/tsm/*
>
> TRUSTED SERVICES TEE DRIVER
> M: Balint Dobszay <balint.dobszay@arm.com>
> diff --git a/samples/Kconfig b/samples/Kconfig
> index 09011be2391a..6ade17cb16b4 100644
> --- a/samples/Kconfig
> +++ b/samples/Kconfig
> @@ -184,6 +184,17 @@ config SAMPLE_TIMER
> bool "Timer sample"
> depends on CC_CAN_LINK && HEADERS_INSTALL
>
> +config SAMPLE_TSM_MR
> + tristate "TSM measurement sample"
> + select TSM_MEASUREMENTS
> + select VIRT_DRIVERS
> + help
> + Build a sample module that emulates MRs (Measurement Registers) and
> + exposes them to user mode applications through the TSM sysfs
> + interface (/sys/class/misc/tsm_mr_sample/emulated_mr/).
> +
> + The module name will be tsm-mr-sample when built as a module.
> +
> config SAMPLE_UHID
> bool "UHID sample"
> depends on CC_CAN_LINK && HEADERS_INSTALL
> diff --git a/samples/Makefile b/samples/Makefile
> index bf6e6fca5410..c95bac31851c 100644
> --- a/samples/Makefile
> +++ b/samples/Makefile
> @@ -43,3 +43,4 @@ obj-$(CONFIG_SAMPLES_RUST) += rust/
> obj-$(CONFIG_SAMPLE_DAMON_WSSE) += damon/
> obj-$(CONFIG_SAMPLE_DAMON_PRCL) += damon/
> obj-$(CONFIG_SAMPLE_HUNG_TASK) += hung_task/
> +obj-$(CONFIG_SAMPLE_TSM_MR) += tsm-mr/
> diff --git a/samples/tsm-mr/Makefile b/samples/tsm-mr/Makefile
> new file mode 100644
> index 000000000000..587c3947b3a7
> --- /dev/null
> +++ b/samples/tsm-mr/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_SAMPLE_TSM_MR) += tsm_mr_sample.o
> diff --git a/samples/tsm-mr/tsm_mr_sample.c b/samples/tsm-mr/tsm_mr_sample.c
> new file mode 100644
> index 000000000000..3ee35d87d275
> --- /dev/null
> +++ b/samples/tsm-mr/tsm_mr_sample.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */

2024-2025

> +
> +#define DEBUG

No need for this. Either the person trying out this sample driver will
define DEBUG at compile time, manually add this line, or otherwise use
dynamic-debug to enable the pr_debug() statements when loading the
module.

If you put #define DEBUG directly in the file then pr_debug() just
becomes printk() which defeats the purpose.

> +#define pr_fmt(x) KBUILD_MODNAME ": " x
> +
> +#include <linux/module.h>
> +#include <linux/tsm-mr.h>
> +#include <linux/miscdevice.h>
> +#include <crypto/hash.h>
> +
> +struct {
> + u8 static_mr[SHA384_DIGEST_SIZE];
> + u8 config_mr[SHA512_DIGEST_SIZE];
> + u8 rtmr0[SHA256_DIGEST_SIZE];
> + u8 rtmr1[SHA384_DIGEST_SIZE];
> + u8 report_digest[SHA512_DIGEST_SIZE];
> +} sample_report = {
> + .static_mr = "static_mr",
> + .config_mr = "config_mr",
> + .rtmr0 = "rtmr0",
> + .rtmr1 = "rtmr1",
> +};
> +
> +static int sample_report_refresh(const struct tsm_measurements *tm,
> + const struct tsm_measurement_register *mr)
> +{
> + struct crypto_shash *tfm;
> + int rc;
> +
> + pr_debug("%s(%s) is called\n", __func__, mr ? mr->mr_name : "<nil>");

With dynamic-debug the function name can be enabled, so no need to
include that in the debug statement.

See "Decorator flags add to the message-prefix" in
Documentation/admin-guide/dynamic-debug-howto.rst.

Other than that this looks good to me:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>