> From 639a42987b36852de2813ce35d221d730467c709 Mon Sep 17 00:00:00 2001 > From: Jiri Pirko > Date: Thu, 15 Oct 2026 16:50:51 +0200 > Subject: [PATCH 18/20] tdx-guest: Expose the native attestation service API > > Share TDREPORT, Quote and measurement-register operations with one The commit message does not mention three things the diff does: it changes the TSM report path to allocate a full `TDX_QUOTE_MAX_LEN` (~128 KiB) buffer for every report instead of an exact-size copy; it adds MAINTAINERS F: lines; and it adds a duplicated set of length constants. Also "Tie clients to service initialization and removal" is misleading, see the exit-path comment. > in-kernel client while preserving the existing misc, TSM report and TSM > MR interfaces. Tie clients to service initialization and removal. > > Assisted-by: Cursor: GPT-5.6 Sol > Assisted-by: Cursor: Claude Opus 5.5 > Signed-off-by: Jiri Pirko > --- > Documentation/virt/coco/tdx-guest.rst | 18 ++ > MAINTAINERS | 3 + > drivers/virt/coco/tdx-guest/tdx-guest.c | 258 +++++++++++++++++++++--- > include/linux/tdx-guest.h | 75 +++++++ > 4 files changed, 326 insertions(+), 28 deletions(-) > create mode 100644 include/linux/tdx-guest.h > > diff --git a/Documentation/virt/coco/tdx-guest.rst b/Documentation/virt/coco/tdx-guest.rst > index 46e316db6bb4..759497b1434a 100644 > --- a/Documentation/virt/coco/tdx-guest.rst > +++ b/Documentation/virt/coco/tdx-guest.rst > @@ -50,3 +50,21 @@ TDX reference material is collected here: > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html > > The driver is based on TDX module specification v1.0 and TDX GHCI specification v1.0. > + > +Kernel clients > +============== > + > +```` exposes the same native service to one in-kernel > +client. The client receives the misc device as its lifetime anchor and a > +complete ``struct tdx_guest_ops``. The operations retrieve TDREPORT or Quote > +bytes, read MRTD/RTMR values, extend an RTMR, and return immutable construction > +fields. > + > +Registering a client attaches it immediately when the service is already Please state the real scenarios. The client (`attestation_tdx`) calls symbols exported by `tdx_guest`, so the module dependency forces `tdx_guest` to be initialized before the client can even load, and prevents `tdx_guest` from being unloaded while the client is loaded. The "attach at init" and "driver removal calls the client's remove callback" cases are therefore only reachable with both built in (init-order), and never at removal. The text suggests more generality than exists. > +available. Otherwise attachment occurs when the TDX guest driver finishes > +initialization. Driver removal calls the client's remove callback before > +freeing report or Quote buffers. Client unregistration similarly drains the > +attachment before returning. > + > +The misc ioctl, TSM-report configfs interface, and TSM-MR sysfs attributes > +remain independent frontends over the same native serialization and buffers. > diff --git a/MAINTAINERS b/MAINTAINERS > index 773ba70accbf..528b1f91a243 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -29689,6 +29689,9 @@ L: linux-coco@lists.linux.dev > L: kvm@vger.kernel.org > S: Supported > T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx > +F: Documentation/virt/coco/tdx-guest.rst These MAINTAINERS additions are unrelated to exposing a native API and are not in the commit message. They also claim the whole `drivers/virt/coco/tdx-guest/` directory and the doc for the x86 TDX entry (Kiryl/Rick), who are not cc'd/ack'd on this series presumably; split into a separate patch and cc the maintainers. > +F: drivers/virt/coco/tdx-guest/ > +F: include/linux/tdx-guest.h > N: tdx > K: \b(tdx) > > diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c > index d0303e31e816..0d5317060246 100644 > --- a/drivers/virt/coco/tdx-guest/tdx-guest.c > +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include Include order: `linux/tdx-guest.h` sorts before `linux/tsm.h` / `tsm-mr.h`. > > #include > > @@ -27,10 +28,21 @@ > > /* TDREPORT buffer */ > static u8 *tdx_report_buf; > +static struct tdx_guest_info tdx_guest_info; `tdx_guest_info` is written in `tdx_mr_init()` with no lock and read from `tdx_guest_info_get()` without one. It is only written before the client can attach, so OK, but then also say it is immutable after init. Also the data is copied here (mrconfigid/mrowner/mrownerconfig) even when no client ever registers. > +static struct miscdevice tdx_misc_dev; > + > +static_assert(TDX_REPORTDATA_LEN == TDX_GUEST_REPORTDATA_LEN); Duplicated constants: the new header re-defines `TDX_GUEST_REPORTDATA_LEN`, `_REPORT_LEN` and `_QUOTE_MAX_LEN`, with `static_asserts` to keep them in sync with the uapi header. Include `` from the new header (or move the definitions) instead of copying numbers, and the three `static_asserts` disappear. The hard-coded (128 * 1024) - 24 in a public header is particularly fragile. > +static_assert(TDX_REPORT_LEN == TDX_GUEST_REPORT_LEN); > > /* Lock to serialize TDG.MR.REPORT and TDG.MR.RTMR.EXTEND TDCALLs */ > static DEFINE_MUTEX(mr_lock); > > +/* Serializes native client registration and service attachment. */ > +static DEFINE_MUTEX(client_lock); > +static const struct tdx_guest_client *tdx_client; > +static void *tdx_client_priv; > +static bool tdx_service_ready; Four separate statics (`tdx_client`, `tdx_client_priv`, `tdx_service_ready`, and `tdx_client_attached` declared much further down) describe one state machine. Group them into one struct and declare them together. `tdx_client_priv` being non-`NULL` could replace `tdx_client_attached`, if `add()` cannot return `NULL` (it returns `ERR_PTR` only on failure, `NULL` would be a valid 'priv'). > + > /* TDREPORT fields */ > enum { > TDREPORT_reportdata = 128, > @@ -49,24 +61,30 @@ enum { > TDREPORT_servtd_hash = TDREPORT_rtmr3 + SHA384_DIGEST_SIZE, > }; > > -static int tdx_do_report(sockptr_t data, sockptr_t tdreport) > +static int __tdx_do_report(sockptr_t data, sockptr_t tdreport) Nit: `__` prefix for "caller holds `mr_lock`" is a common kernel convention (e.g. `__xa_store()`), but this file already uses a `_locked` suffix (`tdx_report_new_locked()`); `tdx_do_report_locked()` would be more consistent. > { > - scoped_cond_guard(mutex_intr, return -EINTR, &mr_lock) { > - u8 *reportdata = tdx_report_buf + TDREPORT_reportdata; > - int ret; > + u8 *reportdata = tdx_report_buf + TDREPORT_reportdata; > + int ret; > > - if (!sockptr_is_null(data) && > - copy_from_sockptr(reportdata, data, TDX_REPORTDATA_LEN)) > - return -EFAULT; > + lockdep_assert_held(&mr_lock); > + if (!sockptr_is_null(data) && > + copy_from_sockptr(reportdata, data, TDX_REPORTDATA_LEN)) > + return -EFAULT; > > - ret = tdx_mcall_get_report0(reportdata, tdx_report_buf); > - if (WARN_ONCE(ret, "tdx_mcall_get_report0() failed: %d", ret)) > - return ret; > + ret = tdx_mcall_get_report0(reportdata, tdx_report_buf); > + if (WARN_ONCE(ret, "tdx_mcall_get_report0() failed: %d", ret)) > + return ret; > > - if (!sockptr_is_null(tdreport) && > - copy_to_sockptr(tdreport, tdx_report_buf, TDX_REPORT_LEN)) > - return -EFAULT; > - } > + if (!sockptr_is_null(tdreport) && > + copy_to_sockptr(tdreport, tdx_report_buf, TDX_REPORT_LEN)) > + return -EFAULT; > + return 0; > +} > + > +static int tdx_do_report(sockptr_t data, sockptr_t tdreport) > +{ > + scoped_cond_guard(mutex_intr, return -EINTR, &mr_lock) > + return __tdx_do_report(data, tdreport); > return 0; > } > > @@ -90,6 +108,76 @@ static int tdx_do_extend(u8 mr_ind, const u8 *data) > return 0; > } > > +static bool tdx_guest_device_valid(struct device *dev) This check plus the `-ENODEV` in every op is the only protection against a stale 'dev'. Since the ops take no reference and the client is removed before anything is freed, the check can only fail if the client is buggy, and a `NULL` dev is a programming error. A device pointer parameter on a singleton service is itself superfluous: there is exactly one TDX guest device (`tdx_client` is limited to one), so '`struct device` *dev' only carries information the callee already has. Speculative generality. > +{ > + return dev && dev == tdx_misc_dev.this_device; > +} > + > +static const struct tdx_guest_info *tdx_guest_info_get(struct device *dev) > +{ > + return tdx_guest_device_valid(dev) ? &tdx_guest_info : NULL; > +} > + > +static int tdx_guest_report_get(struct device *dev, const u8 *reportdata, Each of these ops re-validates length/`NULL` arguments that the sole in-tree caller (`providers/tdx.c`) has already checked (e.g. challenge length is checked against caps in `ctlv.c`). The (length, capacity, `out_length`) triple for fixed-size outputs is also overkill: `report_get` always returns exactly `TDX_REPORT_LEN` bytes. Also: the TDREPORT is memcpy'd out of the shared `tdx_report_buf` under `mr_lock`, good, but `tdx_report_buf` also holds the previous reportdata when callers pass `NULL` (see `mr_read`). > + size_t length, void *out, size_t capacity, > + size_t *out_length) > +{ > + int ret; > + > + if (!tdx_guest_device_valid(dev)) > + return -ENODEV; > + if (!reportdata || length != TDX_REPORTDATA_LEN || !out || !out_length) > + return -EINVAL; > + if (capacity < TDX_REPORT_LEN) > + return -EMSGSIZE; > + scoped_cond_guard(mutex_intr, return -EINTR, &mr_lock) { > + ret = __tdx_do_report(KERNEL_SOCKPTR((void *)reportdata), Casting away const to feed `KERNEL_SOCKPTR()`; `sockptr_t` has no const variant. Same in the quote path below. Better to have the lower layer take 'const `u8` *' for the kernel path. > + KERNEL_SOCKPTR(NULL)); > + if (ret) > + return ret; > + memcpy(out, tdx_report_buf, TDX_REPORT_LEN); > + } > + *out_length = TDX_REPORT_LEN; > + return 0; > +} > + > +static int tdx_guest_mr_read(struct device *dev, enum tdx_guest_mr mr, Every `mr_read` issues a full TDG.MR.REPORT TDCALL with reportdata = `NULL`, which reuses whatever `tdx_report_buf` currently holds in the reportdata field (possibly the last RTMR extend value, since `tdx_do_extend()` uses that same area as its staging buffer). Harmless functionally, but the result of one TDCALL is thrown away except for 48 bytes; reading several MRs (the attestation MR dump path) will call it five times. The existing `tdx_mr_refresh()` in this file is what TSM-MR does; reuse the same refresh and cache instead of a new code path. Also hard-coding the offsets table duplicates the `TDREPORT_*` enum mapping already in `tdx_mrs[]`. > + u8 value[SHA384_DIGEST_SIZE]) > +{ > + static const size_t offsets[] = { > + [TDX_GUEST_MR_MRTD] = TDREPORT_mrtd, > + [TDX_GUEST_MR_RTMR0] = TDREPORT_rtmr0, > + [TDX_GUEST_MR_RTMR1] = TDREPORT_rtmr1, > + [TDX_GUEST_MR_RTMR2] = TDREPORT_rtmr2, > + [TDX_GUEST_MR_RTMR3] = TDREPORT_rtmr3, > + }; > + int ret; > + > + if (!tdx_guest_device_valid(dev)) > + return -ENODEV; > + if (mr >= ARRAY_SIZE(offsets) || !value) > + return -EINVAL; > + scoped_cond_guard(mutex_intr, return -EINTR, &mr_lock) { > + ret = __tdx_do_report(KERNEL_SOCKPTR(NULL), > + KERNEL_SOCKPTR(NULL)); > + if (ret) > + return ret; > + memcpy(value, tdx_report_buf + offsets[mr], > + SHA384_DIGEST_SIZE); > + } > + return 0; > +} > + > +static int tdx_guest_rtmr_extend(struct device *dev, unsigned int rtmr, > + const u8 value[SHA384_DIGEST_SIZE]) > +{ > + if (!tdx_guest_device_valid(dev)) > + return -ENODEV; > + if (rtmr >= 4 || !value) Magic number 4: use `ARRAY_SIZE(tdx_mrs)` or a named define for the number of RTMRs (and note the provider calls it with `mr_id` - 1). > + return -EINVAL; > + return tdx_do_extend(rtmr, value); > +} > + > #define TDX_MR_(r) .mr_value = (void *)TDREPORT_##r, TSM_MR_(r, SHA384) > static struct tsm_measurement_register tdx_mrs[] = { > { TDX_MR_(rtmr0) | TSM_MR_F_RTMR }, > @@ -135,6 +223,12 @@ static const struct attribute_group *tdx_mr_init(void) > rc = tdx_mr_refresh(&tdx_measurements); > if (rc) > return ERR_PTR(rc); > + memcpy(tdx_guest_info.mrconfigid, The three `memcpy()`s are placed between `tdx_mr_refresh()` and the comment that belongs to the code after it, splitting an unrelated logical block; move them to a small helper or before the refresh comment. If the refresh succeeded but a later step in `tdx_mr_init()` fails, `tdx_guest_info` is left populated; harmless. > + buf + TDREPORT_mrconfigid, SHA384_DIGEST_SIZE); > + memcpy(tdx_guest_info.mrowner, > + buf + TDREPORT_mrowner, SHA384_DIGEST_SIZE); > + memcpy(tdx_guest_info.mrownerconfig, > + buf + TDREPORT_mrownerconfig, SHA384_DIGEST_SIZE); > > /* > * @mr_value was initialized with the offset only, while the base > @@ -191,6 +285,8 @@ struct tdx_quote_buf { > u8 data[]; > }; > > +static_assert(TDX_QUOTE_MAX_LEN == TDX_GUEST_QUOTE_MAX_LEN); See comment on the other `static_asserts`; drop the duplicated constant. > + > /* Quote data buffer */ > static void *quote_data; > > @@ -265,15 +361,18 @@ static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeou > return (i == timeout) ? -ETIMEDOUT : 0; > } > > -static int tdx_report_new_locked(struct tsm_report *report, void *data) > +static int __tdx_guest_quote_get(const u8 *reportdata, size_t length, Behavior change in the existing TSM path: previously the `IN_FLIGHT` check (`-EBUSY`) came before the `inblob_len` validation; now argument validation comes first. Harmless, but the commit message claims the existing interfaces are preserved. The function name also does not match the other helpers (it is the `quote_new_locked` body). > + void *out, size_t capacity, > + size_t *out_length) > { > - u8 *buf; > struct tdx_quote_buf *quote_buf = quote_data; > - struct tsm_report_desc *desc = &report->desc; > u32 out_len; > int ret; > u64 err; > > + lockdep_assert_held("e_lock); > + if (!reportdata || length != TDX_REPORTDATA_LEN || !out || !out_length) > + return -EINVAL; > /* > * If the previous request is timedout or interrupted, and the > * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by > @@ -282,16 +381,13 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data) > if (quote_buf->status == GET_QUOTE_IN_FLIGHT) > return -EBUSY; > > - if (desc->inblob_len != TDX_REPORTDATA_LEN) > - return -EINVAL; > - > memset(quote_data, 0, GET_QUOTE_BUF_SIZE); > > /* Update Quote buffer header */ > quote_buf->version = GET_QUOTE_CMD_VER; > quote_buf->in_len = TDX_REPORT_LEN; > > - ret = tdx_do_report(KERNEL_SOCKPTR(desc->inblob), > + ret = tdx_do_report(KERNEL_SOCKPTR((void *)reportdata), > KERNEL_SOCKPTR(quote_buf->data)); > if (ret) > return ret; > @@ -317,13 +413,10 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data) > > if (out_len > TDX_QUOTE_MAX_LEN) > return -EFBIG; > - > - buf = kvmemdup(quote_buf->data, out_len, GFP_KERNEL); > - if (!buf) > - return -ENOMEM; > - > - report->outblob = buf; > - report->outblob_len = out_len; > + if (out_len > capacity) For the TSM path capacity == `TDX_QUOTE_MAX_LEN` so this is unreachable there; it only exists for the kernel client. Fine, but then the preceding `-EFBIG` check and this `-EMSGSIZE` overlap: pick one error for 'quote bigger than what the caller can take'. > + return -EMSGSIZE; > + memcpy(out, quote_buf->data, out_len); > + *out_length = out_len; > > /* > * TODO: parse the PEM-formatted cert chain out of the quote buffer when > @@ -333,6 +426,39 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data) > return ret; > } > > +static int tdx_guest_quote_get(struct device *dev, const u8 *reportdata, Unreachable trailing 'return 0' after `scoped_cond_guard()` (same pattern as the existing `tdx_report_new()`; it is needed to silence the compiler, but a plain `mutex_lock_interruptible()`/guard would be clearer). Also an in-kernel caller is blocked for up to the quote timeout while holding `quote_lock`; the kernel client calls this from within `att->lock` context, so every attestation op stalls behind it, and it is interruptible by signals of whichever userspace task issued the CTLV op; `-EINTR` then propagates to the CTLV client. Is that intended? > + size_t length, void *out, size_t capacity, > + size_t *out_length) > +{ > + if (!tdx_guest_device_valid(dev)) > + return -ENODEV; > + scoped_cond_guard(mutex_intr, return -EINTR, "e_lock) > + return __tdx_guest_quote_get(reportdata, length, out, > + capacity, out_length); > + return 0; > +} > + > +static int tdx_report_new_locked(struct tsm_report *report, void *data) > +{ > + struct tsm_report_desc *desc = &report->desc; > + size_t length; > + void *buf; > + int ret; > + > + buf = kvmalloc(TDX_QUOTE_MAX_LEN, GFP_KERNEL); Regression: the TSM report path used to `kvmemdup()` exactly `out_len` bytes (typically ~5 KiB for a Quote). It now always allocates `TDX_QUOTE_MAX_LEN` (~128 KiB, vmalloc territory) per report and keeps it for the lifetime of `report->outblob`, plus an extra memcpy. Either allocate-and-trim (krealloc/kvmemdup after the fact) or have `__tdx_guest_quote_get()` return the pointer into `quote_buf->data` so the TSM path can `kvmemdup()` as before. Also mixing 'void *buf' and the success path leaves kvmalloc'ed memory that tsm frees with `kvfree()`, fine. > + if (!buf) > + return -ENOMEM; > + ret = __tdx_guest_quote_get(desc->inblob, desc->inblob_len, > + buf, TDX_QUOTE_MAX_LEN, &length); > + if (ret) { > + kvfree(buf); > + return ret; > + } > + report->outblob = buf; > + report->outblob_len = length; > + return 0; > +} > + > static int tdx_report_new(struct tsm_report *report, void *data) > { > scoped_cond_guard(mutex_intr, return -EINTR, "e_lock) > @@ -402,6 +528,68 @@ static const struct tsm_report_ops tdx_tsm_ops = { > .report_bin_attr_visible = tdx_report_bin_attr_visible, > }; > > +static const struct tdx_guest_ops tdx_guest_ops = { > + .info_get = tdx_guest_info_get, > + .report_get = tdx_guest_report_get, > + .quote_get = tdx_guest_quote_get, > + .mr_read = tdx_guest_mr_read, > + .rtmr_extend = tdx_guest_rtmr_extend, > +}; > + > +static bool tdx_client_attached; Declared in the middle of the file, away from the other client state. > + > +static int tdx_guest_client_attach(void) > +{ > + void *priv; > + > + lockdep_assert_held(&client_lock); > + if (!tdx_client || !tdx_service_ready) > + return 0; > + priv = tdx_client->add(tdx_misc_dev.this_device, &tdx_guest_ops); `client->add()` is called with `client_lock` held, and `add()` for the attestation provider registers a CTLV device and probes ops. If `add()` (or the callbacks that follow it) ever calls `tdx_guest_client_unregister()` or another path that takes `client_lock`, this deadlocks. Document that client callbacks must not call back into register/unregister. Also `add()` runs before the op table is guaranteed immutable; fine. > + if (IS_ERR(priv)) > + return PTR_ERR(priv); > + tdx_client_priv = priv; > + tdx_client_attached = true; > + return 0; > +} > + > +static void tdx_guest_client_detach(void) > +{ > + lockdep_assert_held(&client_lock); > + if (!tdx_client_attached) > + return; > + tdx_client->remove(tdx_client_priv); State the draining guarantee in the header kernel-doc for `remove()`: it is called under `client_lock` and must wait for running operations, so unregistering can block for as long as an in-flight Quote holds `quote_lock` (up to the completion timeout). > + tdx_client_priv = NULL; > + tdx_client_attached = false; > +} > + > +int tdx_guest_client_register(const struct tdx_guest_client *client) > +{ > + int ret; > + > + if (!client || !client->add || !client->remove) The kernel-doc says nothing about the contexts and locking; add Context: and the single-client `EBUSY` semantics to the header. > + return -EINVAL; > + guard(mutex)(&client_lock); > + if (tdx_client) > + return -EBUSY; > + tdx_client = client; If attach fails (`add()` returns an error) the failure is returned to the registering client, good, but when registration happens before the service is ready, `add()` is deferred to `tdx_guest_init()`, where a failing `add()` fails the whole `tdx_guest` module init (see below). The same client therefore gets different failure semantics depending on init order, and in the built-in case the registrant is never told (it saw success). > + ret = tdx_guest_client_attach(); > + if (ret) > + tdx_client = NULL; > + return ret; > +} > +EXPORT_SYMBOL_NS_GPL(tdx_guest_client_register, "TDX_GUEST"); Two changes requested. 1. Say in the commit message that the new `TDX_GUEST` export namespace has one user, `providers/tdx.c` in patch 20, which imports it with `MODULE_IMPORT_NS("TDX_GUEST")`, and that nothing here is exercised or build-tested until then. 2. Add `!CONFIG_TDX_GUEST_DRIVER` stubs to the header, or document that any other user must depend on the Kconfig symbol. > + > +void tdx_guest_client_unregister(const struct tdx_guest_client *client) > +{ > + guard(mutex)(&client_lock); > + if (WARN_ON_ONCE(client != tdx_client)) A WARN on a caller mismatch, but `tdx_client_attached`/priv state is left as is; that is fine since we return. Note that unregistering a client that was registered but never attached (service not ready) correctly clears `tdx_client`. > + return; > + tdx_guest_client_detach(); > + tdx_client = NULL; > +} > +EXPORT_SYMBOL_NS_GPL(tdx_guest_client_unregister, "TDX_GUEST"); > + > static int __init tdx_guest_init(void) > { > int ret; > @@ -427,9 +615,19 @@ static int __init tdx_guest_init(void) > ret = tsm_report_register(&tdx_tsm_ops, NULL); > if (ret) > goto free_quote; > + scoped_guard(mutex, &client_lock) { > + tdx_service_ready = true; Do not fail `tdx_guest_init()` when `tdx_guest_client_attach()` fails. Log the error and continue, so the misc device, configfs TSM report and TSM-MR interfaces that the commit message promises to preserve stay available; an optional in-kernel consumer should not disable them. This also removes the new `unregister_tsm` unwind. > + ret = tdx_guest_client_attach(); > + if (ret) > + tdx_service_ready = false; > + } > + if (ret) > + goto unregister_tsm; > > return 0; > > +unregister_tsm: > + tsm_report_unregister(&tdx_tsm_ops); > free_quote: > free_quote_buf(quote_data); > free_misc: > @@ -443,6 +641,10 @@ module_init(tdx_guest_init); > > static void __exit tdx_guest_exit(void) > { > + scoped_guard(mutex, &client_lock) { > + tdx_guest_client_detach(); As noted in the Documentation comment, this is unreachable in the module case: the client module holds a reference on `tdx_guest` through its symbol dependency, so module exit cannot run while a client is attached. For built-in the exit function is not compiled in. This hunk (and the `tdx_service_ready` = false below) is dead code. Drop it, or make the dependency real (e.g. attach via `try_module_get`). > + tdx_service_ready = false; > + } > tsm_report_unregister(&tdx_tsm_ops); > free_quote_buf(quote_data); > misc_deregister(&tdx_misc_dev); > diff --git a/include/linux/tdx-guest.h b/include/linux/tdx-guest.h > new file mode 100644 > index 000000000000..65f367e4956a > --- /dev/null > +++ b/include/linux/tdx-guest.h > @@ -0,0 +1,75 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +#ifndef _LINUX_TDX_GUEST_H > +#define _LINUX_TDX_GUEST_H > + > +#include > +#include > + > +struct device; > + > +#define TDX_GUEST_REPORTDATA_LEN 64 > +#define TDX_GUEST_REPORT_LEN 1024 > +#define TDX_GUEST_QUOTE_MAX_LEN ((128 * 1024) - 24) > + > +struct tdx_guest_info { > + u8 mrconfigid[SHA384_DIGEST_SIZE]; > + u8 mrowner[SHA384_DIGEST_SIZE]; > + u8 mrownerconfig[SHA384_DIGEST_SIZE]; > +}; > + > +enum tdx_guest_mr { > + TDX_GUEST_MR_MRTD, > + TDX_GUEST_MR_RTMR0, > + TDX_GUEST_MR_RTMR1, > + TDX_GUEST_MR_RTMR2, > + TDX_GUEST_MR_RTMR3, > +}; > + > +/** > + * struct tdx_guest_ops - Native TDX guest operations kernel-doc: the member docs are one-liners that omit the key contract: `@report_get`/`@quote_get` output size (exactly `TDX_GUEST_REPORT_LEN` bytes? at most `TDX_GUEST_QUOTE_MAX_LEN`?), that @capacity must be >= that, locking (sleeps, interruptible), the lifetime of the pointer returned by `@info_get`, and the valid range of @rtmr (0..3, while `enum tdx_guest_mr` numbers RTMR0 as 1). Also the member order in the struct should follow the doc. > + * @info_get: Return immutable TD construction fields > + * @report_get: Get a TDREPORT bound to report data > + * @quote_get: Get a Quote bound to report data > + * @mr_read: Read MRTD or an RTMR > + * @rtmr_extend: Extend an RTMR > + */ > +struct tdx_guest_ops { > + const struct tdx_guest_info *(*info_get)(struct device *dev); > + int (*report_get)(struct device *dev, const u8 *reportdata, > + size_t length, void *out, size_t capacity, > + size_t *out_length); > + int (*quote_get)(struct device *dev, const u8 *reportdata, > + size_t length, void *out, size_t capacity, > + size_t *out_length); > + int (*mr_read)(struct device *dev, enum tdx_guest_mr mr, > + u8 value[SHA384_DIGEST_SIZE]); > + int (*rtmr_extend)(struct device *dev, unsigned int rtmr, > + const u8 value[SHA384_DIGEST_SIZE]); > +}; > + > +/** > + * struct tdx_guest_client - Native TDX service consumer > + * @add: Attach to the service and return client private data or an ERR_PTR > + * @remove: Detach the client private data returned by @add > + */ > +struct tdx_guest_client { > + void *(*add)(struct device *parent_dev, > + const struct tdx_guest_ops *ops); > + void (*remove)(void *client_priv); > +}; > + > +/** > + * tdx_guest_client_register - Register one native TDX service consumer The Return: line is vacuous: list `-EBUSY` (already a client), `-EINVAL`, and errors from @add. > + * @client: Immutable add/remove callback pair > + * > + * Return: 0, or a negative errno. > + */ > +int tdx_guest_client_register(const struct tdx_guest_client *client); > + > +/** > + * tdx_guest_client_unregister - Unregister a native TDX service consumer > + * @client: Descriptor passed to tdx_guest_client_register() > + */ > +void tdx_guest_client_unregister(const struct tdx_guest_client *client); > + > +#endif /* _LINUX_TDX_GUEST_H */ > -- > 2.55.0 > Summary of the most serious issues: - TSM report path regression: always `kvmalloc()`s `TDX_QUOTE_MAX_LEN` (~128 KiB) per report instead of an exact-size copy. - A failing in-kernel client attach at init now fails the whole `tdx_guest` module init, killing the misc/TSM interfaces the commit message says are preserved. - Exit-path detach and `tdx_service_ready` handling are unreachable given the module symbol dependency; the documentation overstates the lifecycle. - Constants duplicated in the new public header with `static_asserts` instead of sharing the uapi definitions; `struct device` *dev parameter on every op is speculative generality; magic number 4 for RTMRs. - Unrelated MAINTAINERS additions not in the commit message. - `mr_read` does a full TDCALL per call and discards most of it; kernel-doc lacks the size/locking/context contract.