Re: [PATCH v5 1/5] tsm-mr: Add TVM Measurement Register support
From: Dan Williams <hidden>
Date: 2025-05-01 19:25:33
Also in:
lkml
Cedric Xing wrote:
Introduce new TSM Measurement helper library (tsm-mr) for TVM guest drivers
to expose MRs (Measurement Registers) as sysfs attributes, with Crypto
Agility support.
Add the following new APIs (see include/linux/tsm-mr.h for details):
- tsm_mr_create_attribute_group(): Take on input a `struct
tsm_measurements` instance, which includes one `struct
tsm_measurement_register` per MR with properties like `TSM_MR_F_READABLE`
and `TSM_MR_F_WRITABLE`, to determine the supported operations and create
the sysfs attributes accordingly. On success, return a `struct
attribute_group` instance that will typically be included by the guest
driver into `miscdevice.groups` before calling misc_register().
- tsm_mr_free_attribute_group(): Free the memory allocated to the attrubute
group returned by tsm_mr_create_attribute_group().
tsm_mr_create_attribute_group() creates one attribute for each MR, with
names following this pattern:
MRNAME[:HASH]
- MRNAME - Placeholder for the MR name, as specified by
`tsm_measurement_register.mr_name`.
- :HASH - Optional suffix indicating the hash algorithm associated with
this MR, as specified by `tsm_measurement_register.mr_hash`.
Support Crypto Agility by allowing multiple definitions of the same MR
(i.e., with the same `mr_name`) with distinct HASH algorithms.
NOTE: Crypto Agility, introduced in TPM 2.0, allows new hash algorithms to
be introduced without breaking compatibility with applications using older
algorithms. CC architectures may face the same challenge in the future,
needing new hashes for security while retaining compatibility with older
hashes, hence the need for Crypto Agility.
Signed-off-by: Cedric Xing <redacted>Looking good, some minor fixups below.
quoted hunk ↗ jump to hunk
--- Documentation/driver-api/coco/index.rst | 12 + .../driver-api/coco/measurement-registers.rst | 12 + Documentation/driver-api/index.rst | 1 + MAINTAINERS | 4 +- drivers/virt/coco/Kconfig | 5 + drivers/virt/coco/Makefile | 1 + drivers/virt/coco/tsm-mr.c | 247 +++++++++++++++++++++ include/linux/tsm-mr.h | 94 ++++++++ include/trace/events/tsm_mr.h | 80 +++++++ 9 files changed, 454 insertions(+), 2 deletions(-)diff --git a/Documentation/driver-api/coco/index.rst b/Documentation/driver-api/coco/index.rst new file mode 100644 index 000000000000..af9f08ca0cfd --- /dev/null +++ b/Documentation/driver-api/coco/index.rst@@ -0,0 +1,12 @@ +.. SPDX-License-Identifier: GPL-2.0 + +====================== +Confidential Computing +====================== + +.. toctree:: + :maxdepth: 1 + + measurement-registers + +.. only:: subproject and htmldiff --git a/Documentation/driver-api/coco/measurement-registers.rst b/Documentation/driver-api/coco/measurement-registers.rst new file mode 100644 index 000000000000..cef85945a9a7 --- /dev/null +++ b/Documentation/driver-api/coco/measurement-registers.rst@@ -0,0 +1,12 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. include:: <isonum.txt> + +===================== +Measurement Registers +===================== + +.. kernel-doc:: include/linux/tsm-mr.h + :internal: + +.. kernel-doc:: drivers/virt/coco/tsm-mr.c + :export:diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst index 16e2c4ec3c01..3e2a270bd828 100644 --- a/Documentation/driver-api/index.rst +++ b/Documentation/driver-api/index.rst@@ -81,6 +81,7 @@ Subsystem-specific APIs acpi/index backlight/lp855x-driver.rst clk + coco/index console crypto/index dmaengine/indexdiff --git a/MAINTAINERS b/MAINTAINERS index fa1e04e87d1d..1d1426af3a68 100644 --- a/MAINTAINERS +++ b/MAINTAINERS@@ -24616,8 +24616,8 @@ M: Dan Williams <dan.j.williams@intel.com> L: linux-coco@lists.linux.dev S: Maintained F: Documentation/ABI/testing/configfs-tsm -F: drivers/virt/coco/tsm.c -F: include/linux/tsm.h +F: drivers/virt/coco/tsm*.c +F: include/linux/tsm*.h
Should also add Documentation/driver-api/coco/ here.
quoted hunk ↗ jump to hunk
TRUSTED SERVICES TEE DRIVER M: Balint Dobszay [off-list ref]diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig index ff869d883d95..737106d5dcbc 100644 --- a/drivers/virt/coco/Kconfig +++ b/drivers/virt/coco/Kconfig@@ -7,6 +7,11 @@ config TSM_REPORTS select CONFIGFS_FS tristate +config TSM_MEASUREMENTS + select CRYPTO_HASH_INFO + select CRYPTO + bool + source "drivers/virt/coco/efi_secret/Kconfig" source "drivers/virt/coco/pkvm-guest/Kconfig"diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile index c3d07cfc087e..eb6ec5c1d2e1 100644 --- a/drivers/virt/coco/Makefile +++ b/drivers/virt/coco/Makefile@@ -3,6 +3,7 @@ # Confidential computing related collateral # obj-$(CONFIG_TSM_REPORTS) += tsm.o +obj-$(CONFIG_TSM_MEASUREMENTS) += tsm-mr.o obj-$(CONFIG_EFI_SECRET) += efi_secret/ obj-$(CONFIG_ARM_PKVM_GUEST) += pkvm-guest/ obj-$(CONFIG_SEV_GUEST) += sev-guest/diff --git a/drivers/virt/coco/tsm-mr.c b/drivers/virt/coco/tsm-mr.c new file mode 100644 index 000000000000..adc32328018e --- /dev/null +++ b/drivers/virt/coco/tsm-mr.c@@ -0,0 +1,247 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2024-2025 Intel Corporation. All rights reserved. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/sysfs.h> + +#define CREATE_TRACE_POINTS +#include <trace/events/tsm_mr.h> + +/* + * struct tm_context - contains everything necessary to implement sysfs + * attributes for MRs. + * @rwsem: protects the MR cache from concurrent access. + * @agrp: contains all MR attributes created by tsm_mr_create_attribute_group(). + * @tm: input to tsm_mr_create_attribute_group() containing MR definitions/ops. + * @in_sync: %true if MR cache is up-to-date. + * @mrs: array of &struct bin_attribute, one for each MR. + * + * This internal structure contains everything needed to implement + * tm_digest_read() and tm_digest_write(). + * + * Given tm->refresh() is potentially expensive, tm_digest_read() caches MR + * values and calls tm->refresh() only when necessary. Only live MRs (i.e., with + * %TSM_MR_F_LIVE set) can trigger tm->refresh(), while others are assumed to + * retain their values from the last tm->write(). @in_sync tracks if there have + * been tm->write() calls since the last tm->refresh(). That is, tm->refresh() + * will be called only when a live MR is being read and the cache is stale + * (@in_sync is %false). + * + * tm_digest_write() sets @in_sync to %false and calls tm->write(), whose + * semantics is arch and MR specific. Most (if not all) writable MRs support the + * extension semantics (i.e., tm->write() extends the input buffer into the MR). + */ +struct tm_context { + struct rw_semaphore rwsem; + struct attribute_group agrp; + const struct tsm_measurements *tm; + bool in_sync; + struct bin_attribute mrs[]; +}; + +static ssize_t tm_digest_read(struct file *filp, struct kobject *kobj, + const struct bin_attribute *attr, char *buffer, + loff_t off, size_t count) +{ + struct tm_context *ctx; + const struct tsm_measurement_register *mr; + int rc; + + ctx = attr->private; + rc = down_read_interruptible(&ctx->rwsem); + if (rc) + return rc; + + mr = &ctx->tm->mrs[attr - ctx->mrs]; + + /* + * @ctx->in_sync indicates if the MR cache is stale. It is a global + * instead of a per-MR flag for simplicity, as most (if not all) archs + * allow reading all MRs in oneshot. + * + * ctx->refresh() is necessary only for LIVE MRs, while others retain + * their values from their respective last ctx->write(). + */ + if ((mr->mr_flags & TSM_MR_F_LIVE) && !ctx->in_sync) { + up_read(&ctx->rwsem); + + rc = down_write_killable(&ctx->rwsem); + if (rc) + return rc; + + if (!ctx->in_sync) { + rc = ctx->tm->refresh(ctx->tm, mr); + ctx->in_sync = !rc; + trace_tsm_mr_refresh(mr, rc); + } + + downgrade_write(&ctx->rwsem); + } + + memcpy(buffer, mr->mr_value + off, count); + trace_tsm_mr_read(mr); + + up_read(&ctx->rwsem); + return rc ?: count; +} + +static ssize_t tm_digest_write(struct file *filp, struct kobject *kobj, + const struct bin_attribute *attr, char *buffer, + loff_t off, size_t count) +{ + struct tm_context *ctx; + const struct tsm_measurement_register *mr; + ssize_t rc; + + /* partial writes are not supported */ + if (off != 0 || count != attr->size) + return -EINVAL; + + ctx = attr->private; + mr = &ctx->tm->mrs[attr - ctx->mrs]; + trace_tsm_mr_write(mr, buffer);
I would have expected the trace to be emitted after the write succeeds, otherwise this tracepoint is "may have written" vs "write".
+ + rc = down_write_killable(&ctx->rwsem); + if (rc) + return rc; + + rc = ctx->tm->write(ctx->tm, mr, buffer); + + /* mark MR cache stale */ + if (!rc) + ctx->in_sync = false;
...also, putting the trace here means the trace event is known to be ordered by the rwsem. [..] Other than the above you can add: Reviewed-by: Dan Williams <redacted>