mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support
@ 2026-10-25 20:18 Arun Menon
  2026-10-25 20:18 ` [PATCH v4 1/6] tpm_crb: Add register definitions of TPM CRB chunking fields Arun Menon
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Arun Menon @ 2026-10-25 20:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Arun Menon

The new version of TCG TPM v185 [1] supports sending data/commands in
chunks for the CRB (Command Response Buffer) interface. This is in line
with the initiative to support PQC algorithms.

This series implements the logic to send and receive larger TPM
cmd/rsp between the linux guest and the TPM backend in chunks.
Currently, the TPM CRB driver is limited by the physical size of the
MMIO window. When userspace attempts to send a payload that exceeds this
size, the driver rejects it.

This series introduces chunking support. The driver now checks the CRB
interface capability for CRB_INTF_CAP_CRB_CHUNK. If supported by the
backend, the driver will slice oversized commands into MMIO-sized
chunks, signalling the backend via CRB_START_NEXT_CHUNK, and finalizing
with CRB_START_INVOKE. Responses are also read back in a similar chunked
manner.

If the backend does not support chunking, the driver retains its legacy
behaviour and enforces the standard size limits.

This feature also requires the QEMU to interpret the data in chunks and
forward it to the TPM backend and subsequently dispatch the TPM response
in chunks back to the linux guest. This is implemented in [2] and
already merged into the mainline.

The whole series applied on top of for-next-tpm (including prerequisite)
can be found here:
https://github.com/armenon-rh/linux/tree/tpm-crb-linux

[1] https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p07_Pub.pdf
[2] https://lore.kernel.org/qemu-devel/20260601215410.517009-1-stefanb@linux.ibm.com/

v4
--
- Rebased on master (or for-next-tpm, both point to the same commit).
- Resolved one conflict in PATCH v3 05/06, as TPM_BUFSIZE is declared in
  include/linux/tpm_command.h.

v3
--
- Split patch 2, so that the code is cleaner.
- Re-order the buffer size adjustment patch.
- Rename crb_trigger_tpm to tpm_crb_start.
- Add dispatching logic in send and recv functions. Chunking is
  separated from no-chunking for clarity.
- Increase TPM_BUFSIZE in the common file include/linux/tpm.h following,
  [PATCH v9 11/11] tpm-buf: Implement managed allocations
  https://lore.kernel.org/linux-integrity/20260125192526.782202-12-jarkko@kernel.org/

v2
--
- Add size checks before copying memory.
- Update TPM_BUFSIZE to 8KB.
- Commit messages updated to indicate motivation and logic of the change.

Arun Menon (6):
  tpm_crb: Add register definitions of TPM CRB chunking fields
  tpm_crb: Split start method into a separate header
  tpm_crb: Add start_cmd parameter to tpm_crb_start wrapper
  tpm: tis_i2c: Use local 4KB buffer to limit memory usage
  tpm: Increase TPM_BUFSIZE to 8kB for chunking support
  tpm_crb: Implement command and response chunking logic

 drivers/char/tpm/tpm_crb.c     | 257 +++++++++++++++++++++++++--------
 drivers/char/tpm/tpm_tis_i2c.c |   6 +-
 include/linux/tpm_command.h    |   2 +-
 3 files changed, 204 insertions(+), 61 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 1/6] tpm_crb: Add register definitions of TPM CRB chunking fields
  2026-10-25 20:18 [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support Arun Menon
@ 2026-10-25 20:18 ` Arun Menon
  2026-10-25 20:18 ` [PATCH v4 2/6] tpm_crb: Split start method into a separate header Arun Menon
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Arun Menon @ 2026-10-25 20:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Arun Menon

From: Arun Menon <armenon@redhat.com>

Post-quantum cryptographic (PQC) algorithms can require buffer sizes that
exceed the physical capacity of the TPM's Command/Response Buffer (CRB).
To support these larger payloads, the TPM 2.0 CRB specification [1]
allows for data chunking when the physical MMIO window is smaller than
the required buffer size.

To support this protocol, the TPM driver must be able to detect the
chunking capability, and signal the backend using specific start
method flags, also known as the control area start register bits.

As per sections 6.4.2.2 and 6.5.3.9 of the specification document [1]
Add 2 new bit flags to the existing enum crb_start and add the
capability bit.
- CRB_INTF_CAP_CRB_CHUNK: A capability bit used to detect if the backend
  supports chunking.
- CRB_START_NEXT_CHUNK: A control bit to signal the TPM to consume the
  current command buffer, or to get the next chunk from the response
  buffer.
- CRB_START_RESP_RETRY: A control bit to signal retransmission of a
  response buffer.

[1] https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p07_rc1_121225.pdf

Signed-off-by: Arun Menon <armenon@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm_crb.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index ceb4100ba400..d76f9e30f036 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -57,12 +57,18 @@ enum crb_ctrl_sts {
 
 enum crb_start {
 	CRB_START_INVOKE	= BIT(0),
+	CRB_START_RESP_RETRY = BIT(1),
+	CRB_START_NEXT_CHUNK = BIT(2),
 };
 
 enum crb_cancel {
 	CRB_CANCEL_INVOKE	= BIT(0),
 };
 
+enum crb_intf {
+	CRB_INTF_CAP_CRB_CHUNK = BIT(10),
+};
+
 struct crb_regs_head {
 	u32 loc_state;
 	u32 reserved1;
-- 
2.54.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 2/6] tpm_crb: Split start method into a separate header
  2026-10-25 20:18 [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support Arun Menon
  2026-10-25 20:18 ` [PATCH v4 1/6] tpm_crb: Add register definitions of TPM CRB chunking fields Arun Menon
@ 2026-10-25 20:18 ` Arun Menon
  2026-10-25 20:55   ` Jarkko Sakkinen
  2026-10-25 20:56   ` Jarkko Sakkinen
  2026-10-25 20:18 ` [PATCH v4 3/6] tpm_crb: Add start_cmd parameter to tpm_crb_start wrapper Arun Menon
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Arun Menon @ 2026-10-25 20:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Arun Menon

From: Arun Menon <armenon@redhat.com>

The current implementation handles different platform start methods
(ACPI, ARM SMC, and ARM FFA) directly within crb_send().
Move this logic into a new helper function, tpm_crb_start(). This is a
pure refactor with no functional changes intended.

Signed-off-by: Arun Menon <armenon@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm_crb.c | 50 ++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index d76f9e30f036..9a2f512b4ae3 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -446,6 +446,32 @@ static int tpm_crb_smc_start(struct device *dev, unsigned long func_id)
 }
 #endif
 
+static int tpm_crb_start(struct tpm_chip *chip)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	int rc = 0;
+	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
+	 * report only ACPI start but in practice seems to require both
+	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
+	 */
+	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
+	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
+	    !strcmp(priv->hid, "MSFT0101"))
+		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
+	if (priv->sm == ACPI_TPM2_START_METHOD ||
+	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
+		rc = crb_do_acpi_start(chip);
+	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
+		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
+		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
+	}
+	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
+		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
+		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
+	}
+	return rc;
+}
+
 static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
 {
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
@@ -471,29 +497,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
 	/* Make sure that cmd is populated before issuing start. */
 	wmb();
 
-	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
-	 * report only ACPI start but in practice seems to require both
-	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
-	 */
-	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
-	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
-	    !strcmp(priv->hid, "MSFT0101"))
-		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
-
-	if (priv->sm == ACPI_TPM2_START_METHOD ||
-	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
-		rc = crb_do_acpi_start(chip);
-
-	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
-		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
-		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
-	}
-
-	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
-		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
-		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
-	}
-
+	rc = tpm_crb_start(chip);
 	if (rc)
 		return rc;
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 3/6] tpm_crb: Add start_cmd parameter to tpm_crb_start wrapper
  2026-10-25 20:18 [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support Arun Menon
  2026-10-25 20:18 ` [PATCH v4 1/6] tpm_crb: Add register definitions of TPM CRB chunking fields Arun Menon
  2026-10-25 20:18 ` [PATCH v4 2/6] tpm_crb: Split start method into a separate header Arun Menon
@ 2026-10-25 20:18 ` Arun Menon
  2026-10-25 20:18 ` [PATCH v4 4/6] tpm: tis_i2c: Use local 4KB buffer to limit memory usage Arun Menon
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Arun Menon @ 2026-10-25 20:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Arun Menon

From: Arun Menon <armenon@redhat.com>

The current implementation of tpm_crb_start() is limited to triggering
the CRB_START_INVOKE bit. To support command and response chunking, the
driver must be able to send other control bits, like
CRB_START_NEXT_CHUNK, using the same platform-specific paths.

This commit adds the start_cmd parameter to tpm_crb_start() so the
caller can specify which command to send.

Signed-off-by: Arun Menon <armenon@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm_crb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 9a2f512b4ae3..31f530744e90 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -446,7 +446,7 @@ static int tpm_crb_smc_start(struct device *dev, unsigned long func_id)
 }
 #endif
 
-static int tpm_crb_start(struct tpm_chip *chip)
+static int tpm_crb_start(struct tpm_chip *chip, u32 start_cmd)
 {
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
 	int rc = 0;
@@ -457,16 +457,16 @@ static int tpm_crb_start(struct tpm_chip *chip)
 	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
 	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
 	    !strcmp(priv->hid, "MSFT0101"))
-		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
+		iowrite32(start_cmd, &priv->regs_t->ctrl_start);
 	if (priv->sm == ACPI_TPM2_START_METHOD ||
 	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
 		rc = crb_do_acpi_start(chip);
 	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
-		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
+		iowrite32(start_cmd, &priv->regs_t->ctrl_start);
 		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
 	}
 	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
-		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
+		iowrite32(start_cmd, &priv->regs_t->ctrl_start);
 		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
 	}
 	return rc;
@@ -497,7 +497,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
 	/* Make sure that cmd is populated before issuing start. */
 	wmb();
 
-	rc = tpm_crb_start(chip);
+	rc = tpm_crb_start(chip, CRB_START_INVOKE);
 	if (rc)
 		return rc;
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 4/6] tpm: tis_i2c: Use local 4KB buffer to limit memory usage
  2026-10-25 20:18 [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support Arun Menon
                   ` (2 preceding siblings ...)
  2026-10-25 20:18 ` [PATCH v4 3/6] tpm_crb: Add start_cmd parameter to tpm_crb_start wrapper Arun Menon
@ 2026-10-25 20:18 ` Arun Menon
  2026-10-25 20:18 ` [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support Arun Menon
  2026-10-25 20:18 ` [PATCH v4 6/6] tpm_crb: Implement command and response chunking logic Arun Menon
  5 siblings, 0 replies; 16+ messages in thread
From: Arun Menon @ 2026-10-25 20:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Arun Menon

From: Arun Menon <armenon@redhat.com>

The global increase of TPM_BUFSIZE to 8KB is necessary to support
Post-Quantum Cryptography (PQC) payloads. However, applying this increase
to the tpm_tis_i2c driver is unnecessary and wasteful due to physical
transport limitations as pointed out in [1]

This commit introduces a local buffer limit that is used in the i2c
driver.

[1] https://sashiko.dev/#/patchset/20260324071803.324774-1-armenon%40redhat.com?patch=8319

Signed-off-by: Arun Menon <armenon@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm_tis_i2c.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
index cde0b093ed58..4667eef15ff1 100644
--- a/drivers/char/tpm/tpm_tis_i2c.c
+++ b/drivers/char/tpm/tpm_tis_i2c.c
@@ -55,6 +55,8 @@
 #define TPM_INTF_CAPABILITY_ZERO 0x0FFFF000
 #define TPM_I2C_INTERFACE_CAPABILITY_ZERO 0x80000000
 
+#define TPM_I2C_BUFSIZE 4096
+
 struct tpm_tis_i2c_phy {
 	struct tpm_tis_data priv;
 	struct i2c_client *i2c_client;
@@ -233,7 +235,7 @@ static int tpm_tis_i2c_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
 	int ret;
 	u16 wrote = 0;
 
-	if (len > TPM_BUFSIZE - 1)
+	if (len > TPM_I2C_BUFSIZE - 1)
 		return -EIO;
 
 	phy->io_buf[0] = reg;
@@ -341,7 +343,7 @@ static int tpm_tis_i2c_probe(struct i2c_client *dev)
 	if (!phy)
 		return -ENOMEM;
 
-	phy->io_buf = devm_kzalloc(&dev->dev, TPM_BUFSIZE, GFP_KERNEL);
+	phy->io_buf = devm_kzalloc(&dev->dev, TPM_I2C_BUFSIZE, GFP_KERNEL);
 	if (!phy->io_buf)
 		return -ENOMEM;
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support
  2026-10-25 20:18 [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support Arun Menon
                   ` (3 preceding siblings ...)
  2026-10-25 20:18 ` [PATCH v4 4/6] tpm: tis_i2c: Use local 4KB buffer to limit memory usage Arun Menon
@ 2026-10-25 20:18 ` Arun Menon
  2026-10-25 20:59   ` Jarkko Sakkinen
  2026-10-25 20:18 ` [PATCH v4 6/6] tpm_crb: Implement command and response chunking logic Arun Menon
  5 siblings, 1 reply; 16+ messages in thread
From: Arun Menon @ 2026-10-25 20:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Arun Menon

From: Arun Menon <armenon@redhat.com>

The size of the command is checked against TPM_BUFSIZE early on before
even sending it to the backend. We therefore need to increase the
TPM_BUFSIZE to allow support for larger commands.

For now, 8KB seems sufficient for ML-KEM and ML-DSA algorithms and it is
also order-1 safe.

Signed-off-by: Arun Menon <armenon@redhat.com>
---
 include/linux/tpm_command.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/tpm_command.h b/include/linux/tpm_command.h
index fc446a1282e2..4ad11068b090 100644
--- a/include/linux/tpm_command.h
+++ b/include/linux/tpm_command.h
@@ -434,7 +434,7 @@ struct tpm2_context {
  */
 
 #define TPM_DIGEST_SIZE		20	/* Max TPM v1.2 PCR size */
-#define TPM_BUFSIZE		4096
+#define TPM_BUFSIZE		8192
 
 /*
  * SHA-512 is, as of today, the largest digest in the TCG algorithm repository.
-- 
2.54.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 6/6] tpm_crb: Implement command and response chunking logic
  2026-10-25 20:18 [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support Arun Menon
                   ` (4 preceding siblings ...)
  2026-10-25 20:18 ` [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support Arun Menon
@ 2026-10-25 20:18 ` Arun Menon
  2026-10-25 21:59   ` Jarkko Sakkinen
  5 siblings, 1 reply; 16+ messages in thread
From: Arun Menon @ 2026-10-25 20:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Arun Menon

From: Arun Menon <armenon@redhat.com>

With the introduction of support for Post Quantum Cryptography
algorithms in TPM, the commands and responses will grow in size.
Some TPMs have a physical hardware memory window (MMIO) that is
smaller than the commands we need to send. Therefore this commit
implements the core logic of sending/receiving data in chunks.

Instead of sending the whole command at once, the driver now sends it in
small chunks. After each chunk, it signals the TPM using a nextChunk
signal, and waits for the TPM to consume the data. Once the final piece
is delivered, the driver signals the TPM to begin execution by toggling
the start invoke bit. We use the same logic in reverse to read large
responses from the TPM.

This allows the driver to handle large payloads even when the hardware
interface has limited memory. This kernel-side support corresponds to
the backend implementation in QEMU [1]. QEMU reassembles the chunks
before passing them to the TPM emulator.

[1] https://lore.kernel.org/qemu-devel/20260506075813.120781-1-armenon@redhat.com/

Signed-off-by: Arun Menon <armenon@redhat.com>
---
 drivers/char/tpm/tpm_crb.c | 215 +++++++++++++++++++++++++++++--------
 1 file changed, 173 insertions(+), 42 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 31f530744e90..8b2aaa109fc4 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -105,11 +105,13 @@ struct crb_priv {
 	u8 __iomem *cmd;
 	u8 __iomem *rsp;
 	u32 cmd_size;
+	u32 rsp_size;
 	u32 smc_func_id;
 	u32 __iomem *pluton_start_addr;
 	u32 __iomem *pluton_reply_addr;
 	u8 ffa_flags;
 	u8 ffa_attributes;
+	u32 intf_id;
 };
 
 struct tpm2_crb_smc {
@@ -369,38 +371,6 @@ static u8 crb_status(struct tpm_chip *chip)
 	return sts;
 }
 
-static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
-{
-	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
-	unsigned int expected;
-
-	/* A sanity check that the upper layer wants to get at least the header
-	 * as that is the minimum size for any TPM response.
-	 */
-	if (count < TPM_HEADER_SIZE)
-		return -EIO;
-
-	/* If this bit is set, according to the spec, the TPM is in
-	 * unrecoverable condition.
-	 */
-	if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
-		return -EIO;
-
-	/* Read the first 8 bytes in order to get the length of the response.
-	 * We read exactly a quad word in order to make sure that the remaining
-	 * reads will be aligned.
-	 */
-	memcpy_fromio(buf, priv->rsp, 8);
-
-	expected = be32_to_cpup((__be32 *)&buf[2]);
-	if (expected > count || expected < TPM_HEADER_SIZE)
-		return -EIO;
-
-	memcpy_fromio(&buf[8], &priv->rsp[8], expected - 8);
-
-	return expected;
-}
-
 static int crb_do_acpi_start(struct tpm_chip *chip)
 {
 	union acpi_object *obj;
@@ -472,17 +442,71 @@ static int tpm_crb_start(struct tpm_chip *chip, u32 start_cmd)
 	return rc;
 }
 
+static int tpm_crb_send_no_chunks(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	int rc;
+
+	memcpy_toio(priv->cmd, buf, len);
+
+	/* Make sure that cmd is populated before issuing start. */
+	wmb();
+
+	rc = tpm_crb_start(chip, CRB_START_INVOKE);
+	if (rc)
+		return rc;
+
+	return crb_try_pluton_doorbell(priv, false);
+}
+
+static int tpm_crb_send_chunks(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	size_t offset = 0;
+	size_t chunk_size;
+	int rc;
+
+	while (offset < len) {
+		chunk_size = min_t(size_t, len - offset, priv->cmd_size);
+
+		if (chunk_size == 0)
+			break;
+
+		memcpy_toio(priv->cmd, buf + offset, chunk_size);
+		offset += chunk_size;
+
+		/* Make sure that cmd is populated before issuing start. */
+		wmb();
+		if (offset < len) {
+			rc = tpm_crb_start(chip, CRB_START_NEXT_CHUNK);
+			if (rc)
+				return rc;
+			if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_start,
+									 CRB_START_NEXT_CHUNK, 0,
+									 TPM2_TIMEOUT_C)) {
+				dev_err(&chip->dev,
+						"Timeout waiting for backend to consume chunk\n");
+				return -ETIME;
+			}
+		} else {
+			rc = tpm_crb_start(chip, CRB_START_INVOKE);
+			if (rc)
+				return rc;
+		}
+	}
+
+	return crb_try_pluton_doorbell(priv, false);
+}
 static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
 {
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
-	int rc = 0;
 
 	/* Zero the cancel register so that the next command will not get
 	 * canceled.
 	 */
 	iowrite32(0, &priv->regs_t->ctrl_cancel);
 
-	if (len > priv->cmd_size) {
+	if (len > priv->cmd_size && !(priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)) {
 		dev_err(&chip->dev, "invalid command count value %zd %d\n",
 			len, priv->cmd_size);
 		return -E2BIG;
@@ -492,16 +516,115 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
 	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON)
 		__crb_cmd_ready(&chip->dev, priv, chip->locality);
 
-	memcpy_toio(priv->cmd, buf, len);
+	if (len <= priv->cmd_size)
+		return tpm_crb_send_no_chunks(chip, buf, len);
 
-	/* Make sure that cmd is populated before issuing start. */
-	wmb();
+	return tpm_crb_send_chunks(chip, buf, len);
+}
 
-	rc = tpm_crb_start(chip, CRB_START_INVOKE);
-	if (rc)
-		return rc;
+static int tpm_crb_recv_no_chunks(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	unsigned int expected;
 
-	return crb_try_pluton_doorbell(priv, false);
+	/* Read the first 8 bytes in order to get the length of the response.
+	 * We read exactly a quad word in order to make sure that the remaining
+	 * reads will be aligned.
+	 */
+	memcpy_fromio(buf, priv->rsp, 8);
+
+	expected = be32_to_cpup((__be32 *)&buf[2]);
+	if (expected > count || expected < TPM_HEADER_SIZE)
+		return -EIO;
+
+	memcpy_fromio(&buf[8], &priv->rsp[8], expected - 8);
+
+	return expected;
+}
+
+static int tpm_crb_recv_chunks(struct tpm_chip *chip, u8 *buf, size_t count,
+							   unsigned int expected)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	size_t offset = 0;
+	size_t chunk_size;
+	size_t first_read;
+	int rc;
+
+	if (expected > count)
+		return -EIO;
+	/*
+	 * Set chunk_size by comparing the size of the buffer that the upper
+	 * layer has allocated (count) to the hardware tpm limit (priv->rsp_size).
+	 * This is to prevent buffer overflow while writing to buf.
+	 */
+	chunk_size = min_t(size_t, count, priv->rsp_size);
+	if (chunk_size < 8)
+		return -EIO;
+
+	memcpy_fromio(buf, priv->rsp, 8);
+
+	/*
+	 * Compare the actual size of the response we found in
+	 * the header to the chunk size
+	 */
+	first_read = min_t(size_t, expected, chunk_size);
+
+	memcpy_fromio(&buf[8], &priv->rsp[8], first_read - 8);
+	offset = first_read;
+
+	while (offset < expected) {
+		rc = tpm_crb_start(chip, CRB_START_NEXT_CHUNK);
+		if (rc)
+			return rc;
+
+		if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_start,
+								 CRB_START_NEXT_CHUNK, 0,
+								 TPM2_TIMEOUT_C)) {
+			dev_err(&chip->dev, "Timeout waiting for backend response\n");
+			return -ETIME;
+		}
+
+		chunk_size = min_t(size_t, expected - offset, priv->rsp_size);
+		memcpy_fromio(buf + offset, priv->rsp, chunk_size);
+		offset += chunk_size;
+	}
+
+	return expected;
+}
+
+static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	unsigned int expected;
+
+	/* A sanity check that the upper layer wants to get at least the header
+	 * as that is the minimum size for any TPM response.
+	 */
+	if (count < TPM_HEADER_SIZE)
+		return -EIO;
+
+	/* If this bit is set, according to the spec, the TPM is in
+	 * unrecoverable condition.
+	 */
+	if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
+		return -EIO;
+
+	/*
+	 * Peek at the first 8 bytes to determine the response size
+	 */
+	expected = be32_to_cpup((__be32 *)&priv->rsp[2]);
+
+	if (expected <= priv->rsp_size)
+		return tpm_crb_recv_no_chunks(chip, buf, count);
+
+	if (!(priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)) {
+		dev_err(&chip->dev,
+			    "Response larger than MMIO and chunking not supported\n");
+		return -EIO;
+	}
+
+	return tpm_crb_recv_chunks(chip, buf, count, expected);
 }
 
 static void crb_cancel(struct tpm_chip *chip)
@@ -728,6 +851,12 @@ static int crb_map_io(struct device *dev, struct crb_priv *priv,
 		goto out;
 	}
 
+	if (priv->regs_h)
+		priv->intf_id = ioread32((u32 __iomem *)&priv->regs_h->intf_id);
+
+	if (priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)
+		dev_info(dev, "CRB Chunking is supported by backend\n");
+
 	memcpy_fromio(&__rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
 	rsp_pa = le64_to_cpu(__rsp_pa);
 	rsp_size = ioread32(&priv->regs_t->ctrl_rsp_size);
@@ -765,8 +894,10 @@ static int crb_map_io(struct device *dev, struct crb_priv *priv,
 	priv->rsp = priv->cmd;
 
 out:
-	if (!ret)
+	if (!ret) {
 		priv->cmd_size = cmd_size;
+		priv->rsp_size = rsp_size;
+	}
 
 	__crb_go_idle(dev, priv, 0);
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/6] tpm_crb: Split start method into a separate header
  2026-10-25 20:18 ` [PATCH v4 2/6] tpm_crb: Split start method into a separate header Arun Menon
@ 2026-10-25 20:55   ` Jarkko Sakkinen
  2026-10-26 14:28     ` Arun Menon
  2026-10-25 20:56   ` Jarkko Sakkinen
  1 sibling, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2026-10-25 20:55 UTC (permalink / raw)
  To: Arun Menon; +Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

On Mon, Oct 26, 2026 at 01:48:54AM +0530, Arun Menon wrote:
> From: Arun Menon <armenon@redhat.com>
> 
> The current implementation handles different platform start methods
> (ACPI, ARM SMC, and ARM FFA) directly within crb_send().
> Move this logic into a new helper function, tpm_crb_start(). This is a
> pure refactor with no functional changes intended.
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  drivers/char/tpm/tpm_crb.c | 50 ++++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index d76f9e30f036..9a2f512b4ae3 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -446,6 +446,32 @@ static int tpm_crb_smc_start(struct device *dev, unsigned long func_id)
>  }
>  #endif
>  
> +static int tpm_crb_start(struct tpm_chip *chip)
> +{
> +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> +	int rc = 0;

After declarations thre should be empty line. checkpatch.pl should
complain about this.

> +	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs

I probably did not spot this last time but:

	/*
	 * ...

https://docs.kernel.org/process/coding-style.html

> +	 * report only ACPI start but in practice seems to require both
> +	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
> +	 */
> +	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
> +	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
> +	    !strcmp(priv->hid, "MSFT0101"))
> +		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> +	if (priv->sm == ACPI_TPM2_START_METHOD ||
> +	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
> +		rc = crb_do_acpi_start(chip);
> +	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
> +		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> +		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
> +	}
> +	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
> +		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> +		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
> +	}
> +	return rc;
> +}
> +
>  static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
>  {
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> @@ -471,29 +497,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
>  	/* Make sure that cmd is populated before issuing start. */
>  	wmb();
>  
> -	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
> -	 * report only ACPI start but in practice seems to require both
> -	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
> -	 */
> -	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
> -	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
> -	    !strcmp(priv->hid, "MSFT0101"))
> -		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> -
> -	if (priv->sm == ACPI_TPM2_START_METHOD ||
> -	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
> -		rc = crb_do_acpi_start(chip);
> -
> -	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
> -		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> -		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
> -	}
> -
> -	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
> -		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> -		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
> -	}
> -
> +	rc = tpm_crb_start(chip);
>  	if (rc)
>  		return rc;
>  
> -- 
> 2.54.0
> 

Br, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/6] tpm_crb: Split start method into a separate header
  2026-10-25 20:18 ` [PATCH v4 2/6] tpm_crb: Split start method into a separate header Arun Menon
  2026-10-25 20:55   ` Jarkko Sakkinen
@ 2026-10-25 20:56   ` Jarkko Sakkinen
  1 sibling, 0 replies; 16+ messages in thread
From: Jarkko Sakkinen @ 2026-10-25 20:56 UTC (permalink / raw)
  To: Arun Menon; +Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

On Mon, Oct 26, 2026 at 01:48:54AM +0530, Arun Menon wrote:
> -	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
> -	 * report only ACPI start but in practice seems to require both
> -	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
> -	 */

OK, it came from here and is probably my comment. Still, it should be
put to more appropriate format.

Br, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support
  2026-10-25 20:18 ` [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support Arun Menon
@ 2026-10-25 20:59   ` Jarkko Sakkinen
  2026-10-25 21:00     ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2026-10-25 20:59 UTC (permalink / raw)
  To: Arun Menon; +Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

On Mon, Oct 26, 2026 at 01:48:57AM +0530, Arun Menon wrote:
> From: Arun Menon <armenon@redhat.com>
> 
> The size of the command is checked against TPM_BUFSIZE early on before
> even sending it to the backend. We therefore need to increase the
> TPM_BUFSIZE to allow support for larger commands.
> 
> For now, 8KB seems sufficient for ML-KEM and ML-DSA algorithms and it is
> also order-1 safe.
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>

In what timeline? Or is it out of spectrum timeline?

> ---
>  include/linux/tpm_command.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/tpm_command.h b/include/linux/tpm_command.h
> index fc446a1282e2..4ad11068b090 100644
> --- a/include/linux/tpm_command.h
> +++ b/include/linux/tpm_command.h
> @@ -434,7 +434,7 @@ struct tpm2_context {
>   */
>  
>  #define TPM_DIGEST_SIZE		20	/* Max TPM v1.2 PCR size */
> -#define TPM_BUFSIZE		4096
> +#define TPM_BUFSIZE		8192
>  
>  /*
>   * SHA-512 is, as of today, the largest digest in the TCG algorithm repository.
> -- 
> 2.54.0
> 

Br, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support
  2026-10-25 20:59   ` Jarkko Sakkinen
@ 2026-10-25 21:00     ` Jarkko Sakkinen
  2026-10-26 14:30       ` Arun Menon
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2026-10-25 21:00 UTC (permalink / raw)
  To: Arun Menon; +Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

On Sun, Oct 25, 2026 at 10:59:09PM +0200, Jarkko Sakkinen wrote:
> On Mon, Oct 26, 2026 at 01:48:57AM +0530, Arun Menon wrote:
> > From: Arun Menon <armenon@redhat.com>
> > 
> > The size of the command is checked against TPM_BUFSIZE early on before
> > even sending it to the backend. We therefore need to increase the
> > TPM_BUFSIZE to allow support for larger commands.
> > 
> > For now, 8KB seems sufficient for ML-KEM and ML-DSA algorithms and it is
> > also order-1 safe.
> > 
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> 
> In what timeline? Or is it out of spectrum timeline?

"For now" part concerns me and I'm not educated enough on topic to
know better.

Br, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 6/6] tpm_crb: Implement command and response chunking logic
  2026-10-25 20:18 ` [PATCH v4 6/6] tpm_crb: Implement command and response chunking logic Arun Menon
@ 2026-10-25 21:59   ` Jarkko Sakkinen
  2026-10-26 14:31     ` Arun Menon
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2026-10-25 21:59 UTC (permalink / raw)
  To: Arun Menon; +Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

On Mon, Oct 26, 2026 at 01:48:58AM +0530, Arun Menon wrote:
> From: Arun Menon <armenon@redhat.com>
> 
> With the introduction of support for Post Quantum Cryptography
> algorithms in TPM, the commands and responses will grow in size.
> Some TPMs have a physical hardware memory window (MMIO) that is
> smaller than the commands we need to send.

That is too loose scope, and its truth value will be degrading as time
moves forward.

Here's what I think about the topic and would look up:

1. TCG has defined this in a spec. I would point out the spec and
   section(s), and optionally also date when it was published (like
   "09/2025" or "Spring of 2024", nothing more granular than this).
2. PQC will be probably a thing across the whole chip industry.
   This is why we need to support it not because of some random
   buffer shenanigans, right?
3. "Some TPMs" is inaccurate statement in a wrong way. I would be
   inaccurate too but how I would simplify the topic  would come
   from an utilitirian angle.

   Regardless what future holds i.e., I would represent the set
   of PQC enabled TPMs and the set of chunking enabled TPMs as they
   were the same set. This type of self-deception is what engineering
   really is all about, overall ;-)

Those are my verbose remarks or how I model this situation, which
would synthesize into maybe 2-4 not that long sentences.

> Therefore this commit
> implements the core logic of sending/receiving data in chunks.

I would remove this sentence because it has no utilitirian use and
does not make anyone smarter, other than you get the feeling that
"I should probably pretend that I know what the chunks are".

Add a short paragrap tell us what the heck they are reference to
section is great but putting the concept in nutshell without having
to immediately peak at it would be great.

> 
> Instead of sending the whole command at once, the driver now sends it in
> small chunks. After each chunk, it signals the TPM using a nextChunk

This is a bit loose form of expression and I'm not fan of qualitative
size definitions.

Example: "Send the command split into chunks."

That's really all information content of the first sentence.

> signal, and waits for the TPM to consume the data. Once the final piece
> is delivered, the driver signals the TPM to begin execution by toggling
> the start invoke bit. We use the same logic in reverse to read large
> responses from the TPM.

This is abstracting and encapsulating the meat of the code change.

Remarks:

- The list of the added constants.
- Speak how the CRB registers are used.
- Stuff is heavily moved around but I don't see anything that would
  help me to understand the reorg part. This is important because
  that gives better map to navigate and check if this patch
  could be split (I don't know yet).
- Go through each location where use the word "signal". It's used
  like a solution encapsulator.

When asked this much details and stuff I personally can recommend
more imperative writing style (see the examples). It has friction
because you can move sentence forward only when you get.

> 
> This allows the driver to handle large payloads even when the hardware
> interface has limited memory. This kernel-side support corresponds to
> the backend implementation in QEMU [1]. QEMU reassembles the chunks
> before passing them to the TPM emulator.
> 
> [1] https://lore.kernel.org/qemu-devel/20260506075813.120781-1-armenon@redhat.com/
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
>  drivers/char/tpm/tpm_crb.c | 215 +++++++++++++++++++++++++++++--------
>  1 file changed, 173 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 31f530744e90..8b2aaa109fc4 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -105,11 +105,13 @@ struct crb_priv {
>  	u8 __iomem *cmd;
>  	u8 __iomem *rsp;
>  	u32 cmd_size;
> +	u32 rsp_size;
>  	u32 smc_func_id;
>  	u32 __iomem *pluton_start_addr;
>  	u32 __iomem *pluton_reply_addr;
>  	u8 ffa_flags;
>  	u8 ffa_attributes;
> +	u32 intf_id;
>  };
>  
>  struct tpm2_crb_smc {
> @@ -369,38 +371,6 @@ static u8 crb_status(struct tpm_chip *chip)
>  	return sts;
>  }
>  
> -static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> -{
> -	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> -	unsigned int expected;
> -
> -	/* A sanity check that the upper layer wants to get at least the header
> -	 * as that is the minimum size for any TPM response.
> -	 */
> -	if (count < TPM_HEADER_SIZE)
> -		return -EIO;
> -
> -	/* If this bit is set, according to the spec, the TPM is in
> -	 * unrecoverable condition.
> -	 */
> -	if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
> -		return -EIO;
> -
> -	/* Read the first 8 bytes in order to get the length of the response.
> -	 * We read exactly a quad word in order to make sure that the remaining
> -	 * reads will be aligned.
> -	 */
> -	memcpy_fromio(buf, priv->rsp, 8);
> -
> -	expected = be32_to_cpup((__be32 *)&buf[2]);
> -	if (expected > count || expected < TPM_HEADER_SIZE)
> -		return -EIO;
> -
> -	memcpy_fromio(&buf[8], &priv->rsp[8], expected - 8);
> -
> -	return expected;
> -}
> -
>  static int crb_do_acpi_start(struct tpm_chip *chip)
>  {
>  	union acpi_object *obj;
> @@ -472,17 +442,71 @@ static int tpm_crb_start(struct tpm_chip *chip, u32 start_cmd)
>  	return rc;
>  }
>  
> +static int tpm_crb_send_no_chunks(struct tpm_chip *chip, u8 *buf, size_t len)
> +{
> +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> +	int rc;
> +
> +	memcpy_toio(priv->cmd, buf, len);
> +
> +	/* Make sure that cmd is populated before issuing start. */
> +	wmb();
> +
> +	rc = tpm_crb_start(chip, CRB_START_INVOKE);
> +	if (rc)
> +		return rc;
> +
> +	return crb_try_pluton_doorbell(priv, false);
> +}
> +
> +static int tpm_crb_send_chunks(struct tpm_chip *chip, u8 *buf, size_t len)
> +{
> +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> +	size_t offset = 0;
> +	size_t chunk_size;
> +	int rc;
> +
> +	while (offset < len) {
> +		chunk_size = min_t(size_t, len - offset, priv->cmd_size);
> +
> +		if (chunk_size == 0)
> +			break;
> +
> +		memcpy_toio(priv->cmd, buf + offset, chunk_size);
> +		offset += chunk_size;
> +
> +		/* Make sure that cmd is populated before issuing start. */
> +		wmb();
> +		if (offset < len) {
> +			rc = tpm_crb_start(chip, CRB_START_NEXT_CHUNK);
> +			if (rc)
> +				return rc;
> +			if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_start,
> +									 CRB_START_NEXT_CHUNK, 0,
> +									 TPM2_TIMEOUT_C)) {

I'd split these calls into two lines:

	expired = !crb_wait_for_reg_32( ... );
	if (expired) {



> +				dev_err(&chip->dev,
> +						"Timeout waiting for backend to consume chunk\n");
> +				return -ETIME;
> +			}
> +		} else {
> +			rc = tpm_crb_start(chip, CRB_START_INVOKE);
> +			if (rc)
> +				return rc;
> +		}
> +	}
> +
> +	return crb_try_pluton_doorbell(priv, false);
> +}
>  static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
>  {
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> -	int rc = 0;
>  
>  	/* Zero the cancel register so that the next command will not get
>  	 * canceled.
>  	 */
>  	iowrite32(0, &priv->regs_t->ctrl_cancel);
>  
> -	if (len > priv->cmd_size) {
> +	if (len > priv->cmd_size && !(priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)) {
>  		dev_err(&chip->dev, "invalid command count value %zd %d\n",
>  			len, priv->cmd_size);
>  		return -E2BIG;
> @@ -492,16 +516,115 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
>  	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON)
>  		__crb_cmd_ready(&chip->dev, priv, chip->locality);
>  
> -	memcpy_toio(priv->cmd, buf, len);
> +	if (len <= priv->cmd_size)
> +		return tpm_crb_send_no_chunks(chip, buf, len);
>  
> -	/* Make sure that cmd is populated before issuing start. */
> -	wmb();
> +	return tpm_crb_send_chunks(chip, buf, len);
> +}
>  
> -	rc = tpm_crb_start(chip, CRB_START_INVOKE);
> -	if (rc)
> -		return rc;
> +static int tpm_crb_recv_no_chunks(struct tpm_chip *chip, u8 *buf, size_t count)
> +{
> +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> +	unsigned int expected;
>  
> -	return crb_try_pluton_doorbell(priv, false);
> +	/* Read the first 8 bytes in order to get the length of the response.
> +	 * We read exactly a quad word in order to make sure that the remaining
> +	 * reads will be aligned.
> +	 */

When moving code not heavy mods should be done but we can fix this
cosmetic glitch from the block comments at least.

> +	memcpy_fromio(buf, priv->rsp, 8);
> +
> +	expected = be32_to_cpup((__be32 *)&buf[2]);
> +	if (expected > count || expected < TPM_HEADER_SIZE)
> +		return -EIO;
> +
> +	memcpy_fromio(&buf[8], &priv->rsp[8], expected - 8);
> +
> +	return expected;
> +}
> +
> +static int tpm_crb_recv_chunks(struct tpm_chip *chip, u8 *buf, size_t count,
> +							   unsigned int expected)
> +{
> +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> +	size_t offset = 0;
> +	size_t chunk_size;
> +	size_t first_read;
> +	int rc;
> +
> +	if (expected > count)
> +		return -EIO;
> +	/*
> +	 * Set chunk_size by comparing the size of the buffer that the upper
> +	 * layer has allocated (count) to the hardware tpm limit (priv->rsp_size).
> +	 * This is to prevent buffer overflow while writing to buf.
> +	 */
> +	chunk_size = min_t(size_t, count, priv->rsp_size);
> +	if (chunk_size < 8)
> +		return -EIO;
> +
> +	memcpy_fromio(buf, priv->rsp, 8);
> +
> +	/*
> +	 * Compare the actual size of the response we found in
> +	 * the header to the chunk size
> +	 */
> +	first_read = min_t(size_t, expected, chunk_size);
> +
> +	memcpy_fromio(&buf[8], &priv->rsp[8], first_read - 8);
> +	offset = first_read;
> +
> +	while (offset < expected) {
> +		rc = tpm_crb_start(chip, CRB_START_NEXT_CHUNK);
> +		if (rc)
> +			return rc;
> +
> +		if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_start,
> +								 CRB_START_NEXT_CHUNK, 0,
> +								 TPM2_TIMEOUT_C)) {
> +			dev_err(&chip->dev, "Timeout waiting for backend response\n");
> +			return -ETIME;
> +		}
> +
> +		chunk_size = min_t(size_t, expected - offset, priv->rsp_size);
> +		memcpy_fromio(buf + offset, priv->rsp, chunk_size);
> +		offset += chunk_size;
> +	}
> +
> +	return expected;
> +}
> +
> +static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> +{
> +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> +	unsigned int expected;
> +
> +	/* A sanity check that the upper layer wants to get at least the header
> +	 * as that is the minimum size for any TPM response.
> +	 */
> +	if (count < TPM_HEADER_SIZE)
> +		return -EIO;
> +
> +	/* If this bit is set, according to the spec, the TPM is in
> +	 * unrecoverable condition.
> +	 */
> +	if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
> +		return -EIO;
> +
> +	/*
> +	 * Peek at the first 8 bytes to determine the response size
> +	 */
> +	expected = be32_to_cpup((__be32 *)&priv->rsp[2]);
> +
> +	if (expected <= priv->rsp_size)
> +		return tpm_crb_recv_no_chunks(chip, buf, count);
> +
> +	if (!(priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)) {
> +		dev_err(&chip->dev,
> +			    "Response larger than MMIO and chunking not supported\n");
> +		return -EIO;
> +	}
> +
> +	return tpm_crb_recv_chunks(chip, buf, count, expected);
>  }
>  
>  static void crb_cancel(struct tpm_chip *chip)
> @@ -728,6 +851,12 @@ static int crb_map_io(struct device *dev, struct crb_priv *priv,
>  		goto out;
>  	}
>  
> +	if (priv->regs_h)
> +		priv->intf_id = ioread32((u32 __iomem *)&priv->regs_h->intf_id);
> +
> +	if (priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)
> +		dev_info(dev, "CRB Chunking is supported by backend\n");
> +
>  	memcpy_fromio(&__rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
>  	rsp_pa = le64_to_cpu(__rsp_pa);
>  	rsp_size = ioread32(&priv->regs_t->ctrl_rsp_size);
> @@ -765,8 +894,10 @@ static int crb_map_io(struct device *dev, struct crb_priv *priv,
>  	priv->rsp = priv->cmd;
>  
>  out:
> -	if (!ret)
> +	if (!ret) {
>  		priv->cmd_size = cmd_size;
> +		priv->rsp_size = rsp_size;
> +	}
>  
>  	__crb_go_idle(dev, priv, 0);
>  
> -- 
> 2.54.0
> 

Let's take this easy as it can land earliest v7.5. It would be hard
to miss that deadline so polishing all the details is the best way
to go.

Br, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/6] tpm_crb: Split start method into a separate header
  2026-10-25 20:55   ` Jarkko Sakkinen
@ 2026-10-26 14:28     ` Arun Menon
  0 siblings, 0 replies; 16+ messages in thread
From: Arun Menon @ 2026-10-26 14:28 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

Hi Jarkko,
Thanks for the review.

On Sun, Oct 25, 2026 at 10:55:20PM +0200, Jarkko Sakkinen wrote:
> On Mon, Oct 26, 2026 at 01:48:54AM +0530, Arun Menon wrote:
> > From: Arun Menon <armenon@redhat.com>
> > 
> > The current implementation handles different platform start methods
> > (ACPI, ARM SMC, and ARM FFA) directly within crb_send().
> > Move this logic into a new helper function, tpm_crb_start(). This is a
> > pure refactor with no functional changes intended.
> > 
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >  drivers/char/tpm/tpm_crb.c | 50 ++++++++++++++++++++------------------
> >  1 file changed, 27 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index d76f9e30f036..9a2f512b4ae3 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -446,6 +446,32 @@ static int tpm_crb_smc_start(struct device *dev, unsigned long func_id)
> >  }
> >  #endif
> >  
> > +static int tpm_crb_start(struct tpm_chip *chip)
> > +{
> > +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > +	int rc = 0;
> 
> After declarations thre should be empty line. checkpatch.pl should
> complain about this.
> 
> > +	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
> 
> I probably did not spot this last time but:
> 
> 	/*
> 	 * ...
> 
> https://docs.kernel.org/process/coding-style.html

I will amend the commit. Thanks.

> 
> > +	 * report only ACPI start but in practice seems to require both
> > +	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
> > +	 */
> > +	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
> > +	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
> > +	    !strcmp(priv->hid, "MSFT0101"))
> > +		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> > +	if (priv->sm == ACPI_TPM2_START_METHOD ||
> > +	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
> > +		rc = crb_do_acpi_start(chip);
> > +	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
> > +		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> > +		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
> > +	}
> > +	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
> > +		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> > +		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
> > +	}
> > +	return rc;
> > +}
> > +
> >  static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
> >  {
> >  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > @@ -471,29 +497,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
> >  	/* Make sure that cmd is populated before issuing start. */
> >  	wmb();
> >  
> > -	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
> > -	 * report only ACPI start but in practice seems to require both
> > -	 * CRB start, hence invoking CRB start method if hid == MSFT0101.
> > -	 */
> > -	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER ||
> > -	    priv->sm == ACPI_TPM2_MEMORY_MAPPED ||
> > -	    !strcmp(priv->hid, "MSFT0101"))
> > -		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> > -
> > -	if (priv->sm == ACPI_TPM2_START_METHOD ||
> > -	    priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
> > -		rc = crb_do_acpi_start(chip);
> > -
> > -	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
> > -		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> > -		rc = tpm_crb_smc_start(&chip->dev, priv->smc_func_id);
> > -	}
> > -
> > -	if (priv->sm == ACPI_TPM2_CRB_WITH_ARM_FFA) {
> > -		iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
> > -		rc = tpm_crb_ffa_start(CRB_FFA_START_TYPE_COMMAND, chip->locality);
> > -	}
> > -
> > +	rc = tpm_crb_start(chip);
> >  	if (rc)
> >  		return rc;
> >  
> > -- 
> > 2.54.0
> > 
> 
> Br, Jarkko
> 

Regards,
Arun Menon


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support
  2026-10-25 21:00     ` Jarkko Sakkinen
@ 2026-10-26 14:30       ` Arun Menon
  2026-10-26 19:32         ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: Arun Menon @ 2026-10-26 14:30 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

Hi Jarkko,
Thank you for the review.

On Sun, Oct 25, 2026 at 11:00:42PM +0200, Jarkko Sakkinen wrote:
> On Sun, Oct 25, 2026 at 10:59:09PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Oct 26, 2026 at 01:48:57AM +0530, Arun Menon wrote:
> > > From: Arun Menon <armenon@redhat.com>
> > > 
> > > The size of the command is checked against TPM_BUFSIZE early on before
> > > even sending it to the backend. We therefore need to increase the
> > > TPM_BUFSIZE to allow support for larger commands.
> > > 
> > > For now, 8KB seems sufficient for ML-KEM and ML-DSA algorithms and it is
> > > also order-1 safe.
> > > 
> > > Signed-off-by: Arun Menon <armenon@redhat.com>
> > 
> > In what timeline? Or is it out of spectrum timeline?
> 
> "For now" part concerns me and I'm not educated enough on topic to
> know better.
> 

TPM_BUFSIZE needs to hold the largest command the stack will ever
build. TPM 2.0 Library Part 3: Commands, v185, adds ML-DSA key
objects and the ML-KEM TPM2_Encapsulate()/TPM2_Decapsulate() commands
(sections 14.10, 14.11) [1].

NIST FIPS 204, Table 2, gives the sizes for each ML-DSA parameter
set; the largest, ML-DSA-87, has a 4896-byte private key and a
2592-byte public key [2]. Loading an ML-DSA-87 key pair carries both
(4896 + 2592 bytes); add the command header, a handle and a session,
and this comes to roughly 7.7KB. 8192 covers that with headroom and
keeps the kzalloc(TPM_BUFSIZE, ...) calls in the TPM stack at an
order-1 page allocation - something I figured after sashiko review.

This is sized to the largest parameter set defined today, not a
timeline, and there's no guarantee it stays sufficient. For example,
FIPS 205 (SLH-DSA), Table 2, already defines a 49856-byte signature for
SLH-DSA-SHA2-256f -- six times the current buffer. If a future
algorithm or parameter set needs more than 8192 bytes, this will need
revisiting again [3].

I will add this detail in the commit message.

[1] https://medias.cdn.trustedcomputinggroup.org/production/Trusted_Platform_Module_2_0_Library_Part_3_Commands_Version_185_pub_87a5c10230.pdf
[2] https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf
[3] https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.205.pdf


> Br, Jarkko
> 


Regards,
Arun Menon


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 6/6] tpm_crb: Implement command and response chunking logic
  2026-10-25 21:59   ` Jarkko Sakkinen
@ 2026-10-26 14:31     ` Arun Menon
  0 siblings, 0 replies; 16+ messages in thread
From: Arun Menon @ 2026-10-26 14:31 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

Hi Jarkko,
Thank you for the detailed feedback.

On Sun, Oct 25, 2026 at 11:59:23PM +0200, Jarkko Sakkinen wrote:
> On Mon, Oct 26, 2026 at 01:48:58AM +0530, Arun Menon wrote:
> > From: Arun Menon <armenon@redhat.com>
> > 
> > With the introduction of support for Post Quantum Cryptography
> > algorithms in TPM, the commands and responses will grow in size.
> > Some TPMs have a physical hardware memory window (MMIO) that is
> > smaller than the commands we need to send.
> 
> That is too loose scope, and its truth value will be degrading as time
> moves forward.
> 
> Here's what I think about the topic and would look up:
> 
> 1. TCG has defined this in a spec. I would point out the spec and
>    section(s), and optionally also date when it was published (like
>    "09/2025" or "Spring of 2024", nothing more granular than this).

Sure, I will add the spec detail, citing the TPM PC Client PTP (Platform
TPM Profile) with section number and date.

> 2. PQC will be probably a thing across the whole chip industry.
>    This is why we need to support it not because of some random
>    buffer shenanigans, right?
> 3. "Some TPMs" is inaccurate statement in a wrong way. I would be
>    inaccurate too but how I would simplify the topic  would come
>    from an utilitirian angle.

yes, I understand. Any CRB TPM that gets the updated PQC-sized payloads
will hit the same MMIO-window limit.

> 
>    Regardless what future holds i.e., I would represent the set
>    of PQC enabled TPMs and the set of chunking enabled TPMs as they
>    were the same set. This type of self-deception is what engineering
>    really is all about, overall ;-)

yes, its a PQC transition generally. I will mention that.

> 
> Those are my verbose remarks or how I model this situation, which
> would synthesize into maybe 2-4 not that long sentences.
> 
> > Therefore this commit
> > implements the core logic of sending/receiving data in chunks.
> 
> I would remove this sentence because it has no utilitirian use and
> does not make anyone smarter, other than you get the feeling that
> "I should probably pretend that I know what the chunks are".

yes, will remove this line.

> 
> Add a short paragrap tell us what the heck they are reference to
> section is great but putting the concept in nutshell without having
> to immediately peak at it would be great.
> 
> > 
> > Instead of sending the whole command at once, the driver now sends it in
> > small chunks. After each chunk, it signals the TPM using a nextChunk
> 
> This is a bit loose form of expression and I'm not fan of qualitative
> size definitions.
> 
> Example: "Send the command split into chunks."
> 
> That's really all information content of the first sentence.
> 
> > signal, and waits for the TPM to consume the data. Once the final piece
> > is delivered, the driver signals the TPM to begin execution by toggling
> > the start invoke bit. We use the same logic in reverse to read large
> > responses from the TPM.
> 
> This is abstracting and encapsulating the meat of the code change.
> 
> Remarks:
> 
> - The list of the added constants.
yes, will add them
> - Speak how the CRB registers are used.
yes.
> - Stuff is heavily moved around but I don't see anything that would
>   help me to understand the reorg part. This is important because
>   that gives better map to navigate and check if this patch
>   could be split (I don't know yet).

I will split the 6th patch in to 2. The preparation patch will put the
existing crb_send()/crb_recv() into
tpm_crb_send_no_chunks()/tpm_crb_recv_no_chunks().

Also, I will forward declare tpm_crb_start() (required by 
tpm_crb_recv_chunks()) so that the diff is clean.

> - Go through each location where use the word "signal". It's used
>   like a solution encapsulator.

I will explain the use of registers briefly. The registers were
introduced in a former patch.

> 
> When asked this much details and stuff I personally can recommend
> more imperative writing style (see the examples). It has friction
> because you can move sentence forward only when you get.
> 
> > 
> > This allows the driver to handle large payloads even when the hardware
> > interface has limited memory. This kernel-side support corresponds to
> > the backend implementation in QEMU [1]. QEMU reassembles the chunks
> > before passing them to the TPM emulator.
> > 
> > [1] https://lore.kernel.org/qemu-devel/20260506075813.120781-1-armenon@redhat.com/
> > 
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> > ---
> >  drivers/char/tpm/tpm_crb.c | 215 +++++++++++++++++++++++++++++--------
> >  1 file changed, 173 insertions(+), 42 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index 31f530744e90..8b2aaa109fc4 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -105,11 +105,13 @@ struct crb_priv {
> >  	u8 __iomem *cmd;
> >  	u8 __iomem *rsp;
> >  	u32 cmd_size;
> > +	u32 rsp_size;
> >  	u32 smc_func_id;
> >  	u32 __iomem *pluton_start_addr;
> >  	u32 __iomem *pluton_reply_addr;
> >  	u8 ffa_flags;
> >  	u8 ffa_attributes;
> > +	u32 intf_id;
> >  };
> >  
> >  struct tpm2_crb_smc {
> > @@ -369,38 +371,6 @@ static u8 crb_status(struct tpm_chip *chip)
> >  	return sts;
> >  }
> >  
> > -static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> > -{
> > -	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > -	unsigned int expected;
> > -
> > -	/* A sanity check that the upper layer wants to get at least the header
> > -	 * as that is the minimum size for any TPM response.
> > -	 */
> > -	if (count < TPM_HEADER_SIZE)
> > -		return -EIO;
> > -
> > -	/* If this bit is set, according to the spec, the TPM is in
> > -	 * unrecoverable condition.
> > -	 */
> > -	if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
> > -		return -EIO;
> > -
> > -	/* Read the first 8 bytes in order to get the length of the response.
> > -	 * We read exactly a quad word in order to make sure that the remaining
> > -	 * reads will be aligned.
> > -	 */
> > -	memcpy_fromio(buf, priv->rsp, 8);
> > -
> > -	expected = be32_to_cpup((__be32 *)&buf[2]);
> > -	if (expected > count || expected < TPM_HEADER_SIZE)
> > -		return -EIO;
> > -
> > -	memcpy_fromio(&buf[8], &priv->rsp[8], expected - 8);
> > -
> > -	return expected;
> > -}
> > -
> >  static int crb_do_acpi_start(struct tpm_chip *chip)
> >  {
> >  	union acpi_object *obj;
> > @@ -472,17 +442,71 @@ static int tpm_crb_start(struct tpm_chip *chip, u32 start_cmd)
> >  	return rc;
> >  }
> >  
> > +static int tpm_crb_send_no_chunks(struct tpm_chip *chip, u8 *buf, size_t len)
> > +{
> > +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > +	int rc;
> > +
> > +	memcpy_toio(priv->cmd, buf, len);
> > +
> > +	/* Make sure that cmd is populated before issuing start. */
> > +	wmb();
> > +
> > +	rc = tpm_crb_start(chip, CRB_START_INVOKE);
> > +	if (rc)
> > +		return rc;
> > +
> > +	return crb_try_pluton_doorbell(priv, false);
> > +}
> > +
> > +static int tpm_crb_send_chunks(struct tpm_chip *chip, u8 *buf, size_t len)
> > +{
> > +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > +	size_t offset = 0;
> > +	size_t chunk_size;
> > +	int rc;
> > +
> > +	while (offset < len) {
> > +		chunk_size = min_t(size_t, len - offset, priv->cmd_size);
> > +
> > +		if (chunk_size == 0)
> > +			break;
> > +
> > +		memcpy_toio(priv->cmd, buf + offset, chunk_size);
> > +		offset += chunk_size;
> > +
> > +		/* Make sure that cmd is populated before issuing start. */
> > +		wmb();
> > +		if (offset < len) {
> > +			rc = tpm_crb_start(chip, CRB_START_NEXT_CHUNK);
> > +			if (rc)
> > +				return rc;
> > +			if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_start,
> > +									 CRB_START_NEXT_CHUNK, 0,
> > +									 TPM2_TIMEOUT_C)) {
> 
> I'd split these calls into two lines:
> 
> 	expired = !crb_wait_for_reg_32( ... );
> 	if (expired) {
> 
> 
> 

yes will do it.

> > +				dev_err(&chip->dev,
> > +						"Timeout waiting for backend to consume chunk\n");
> > +				return -ETIME;
> > +			}
> > +		} else {
> > +			rc = tpm_crb_start(chip, CRB_START_INVOKE);
> > +			if (rc)
> > +				return rc;
> > +		}
> > +	}
> > +
> > +	return crb_try_pluton_doorbell(priv, false);
> > +}
> >  static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
> >  {
> >  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > -	int rc = 0;
> >  
> >  	/* Zero the cancel register so that the next command will not get
> >  	 * canceled.
> >  	 */
> >  	iowrite32(0, &priv->regs_t->ctrl_cancel);
> >  
> > -	if (len > priv->cmd_size) {
> > +	if (len > priv->cmd_size && !(priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)) {
> >  		dev_err(&chip->dev, "invalid command count value %zd %d\n",
> >  			len, priv->cmd_size);
> >  		return -E2BIG;
> > @@ -492,16 +516,115 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz, size_t len)
> >  	if (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON)
> >  		__crb_cmd_ready(&chip->dev, priv, chip->locality);
> >  
> > -	memcpy_toio(priv->cmd, buf, len);
> > +	if (len <= priv->cmd_size)
> > +		return tpm_crb_send_no_chunks(chip, buf, len);
> >  
> > -	/* Make sure that cmd is populated before issuing start. */
> > -	wmb();
> > +	return tpm_crb_send_chunks(chip, buf, len);
> > +}
> >  
> > -	rc = tpm_crb_start(chip, CRB_START_INVOKE);
> > -	if (rc)
> > -		return rc;
> > +static int tpm_crb_recv_no_chunks(struct tpm_chip *chip, u8 *buf, size_t count)
> > +{
> > +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > +	unsigned int expected;
> >  
> > -	return crb_try_pluton_doorbell(priv, false);
> > +	/* Read the first 8 bytes in order to get the length of the response.
> > +	 * We read exactly a quad word in order to make sure that the remaining
> > +	 * reads will be aligned.
> > +	 */
> 
> When moving code not heavy mods should be done but we can fix this
> cosmetic glitch from the block comments at least.
> 
> > +	memcpy_fromio(buf, priv->rsp, 8);
> > +
> > +	expected = be32_to_cpup((__be32 *)&buf[2]);
> > +	if (expected > count || expected < TPM_HEADER_SIZE)
> > +		return -EIO;
> > +
> > +	memcpy_fromio(&buf[8], &priv->rsp[8], expected - 8);
> > +
> > +	return expected;
> > +}
> > +
> > +static int tpm_crb_recv_chunks(struct tpm_chip *chip, u8 *buf, size_t count,
> > +							   unsigned int expected)
> > +{
> > +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > +	size_t offset = 0;
> > +	size_t chunk_size;
> > +	size_t first_read;
> > +	int rc;
> > +
> > +	if (expected > count)
> > +		return -EIO;
> > +	/*
> > +	 * Set chunk_size by comparing the size of the buffer that the upper
> > +	 * layer has allocated (count) to the hardware tpm limit (priv->rsp_size).
> > +	 * This is to prevent buffer overflow while writing to buf.
> > +	 */
> > +	chunk_size = min_t(size_t, count, priv->rsp_size);
> > +	if (chunk_size < 8)
> > +		return -EIO;
> > +
> > +	memcpy_fromio(buf, priv->rsp, 8);
> > +
> > +	/*
> > +	 * Compare the actual size of the response we found in
> > +	 * the header to the chunk size
> > +	 */
> > +	first_read = min_t(size_t, expected, chunk_size);
> > +
> > +	memcpy_fromio(&buf[8], &priv->rsp[8], first_read - 8);
> > +	offset = first_read;
> > +
> > +	while (offset < expected) {
> > +		rc = tpm_crb_start(chip, CRB_START_NEXT_CHUNK);
> > +		if (rc)
> > +			return rc;
> > +
> > +		if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_start,
> > +								 CRB_START_NEXT_CHUNK, 0,
> > +								 TPM2_TIMEOUT_C)) {
> > +			dev_err(&chip->dev, "Timeout waiting for backend response\n");
> > +			return -ETIME;
> > +		}
> > +
> > +		chunk_size = min_t(size_t, expected - offset, priv->rsp_size);
> > +		memcpy_fromio(buf + offset, priv->rsp, chunk_size);
> > +		offset += chunk_size;
> > +	}
> > +
> > +	return expected;
> > +}
> > +
> > +static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> > +{
> > +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> > +	unsigned int expected;
> > +
> > +	/* A sanity check that the upper layer wants to get at least the header
> > +	 * as that is the minimum size for any TPM response.
> > +	 */
> > +	if (count < TPM_HEADER_SIZE)
> > +		return -EIO;
> > +
> > +	/* If this bit is set, according to the spec, the TPM is in
> > +	 * unrecoverable condition.
> > +	 */
> > +	if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
> > +		return -EIO;
> > +
> > +	/*
> > +	 * Peek at the first 8 bytes to determine the response size
> > +	 */
> > +	expected = be32_to_cpup((__be32 *)&priv->rsp[2]);
> > +
> > +	if (expected <= priv->rsp_size)
> > +		return tpm_crb_recv_no_chunks(chip, buf, count);
> > +
> > +	if (!(priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)) {
> > +		dev_err(&chip->dev,
> > +			    "Response larger than MMIO and chunking not supported\n");
> > +		return -EIO;
> > +	}
> > +
> > +	return tpm_crb_recv_chunks(chip, buf, count, expected);
> >  }
> >  
> >  static void crb_cancel(struct tpm_chip *chip)
> > @@ -728,6 +851,12 @@ static int crb_map_io(struct device *dev, struct crb_priv *priv,
> >  		goto out;
> >  	}
> >  
> > +	if (priv->regs_h)
> > +		priv->intf_id = ioread32((u32 __iomem *)&priv->regs_h->intf_id);
> > +
> > +	if (priv->intf_id & CRB_INTF_CAP_CRB_CHUNK)
> > +		dev_info(dev, "CRB Chunking is supported by backend\n");
> > +
> >  	memcpy_fromio(&__rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
> >  	rsp_pa = le64_to_cpu(__rsp_pa);
> >  	rsp_size = ioread32(&priv->regs_t->ctrl_rsp_size);
> > @@ -765,8 +894,10 @@ static int crb_map_io(struct device *dev, struct crb_priv *priv,
> >  	priv->rsp = priv->cmd;
> >  
> >  out:
> > -	if (!ret)
> > +	if (!ret) {
> >  		priv->cmd_size = cmd_size;
> > +		priv->rsp_size = rsp_size;
> > +	}
> >  
> >  	__crb_go_idle(dev, priv, 0);
> >  
> > -- 
> > 2.54.0
> > 
> 
> Let's take this easy as it can land earliest v7.5. It would be hard
> to miss that deadline so polishing all the details is the best way
> to go.

I agree. Thanks again.

> 
> Br, Jarkko
> 

Regards,
Arun Menon


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support
  2026-10-26 14:30       ` Arun Menon
@ 2026-10-26 19:32         ` Jarkko Sakkinen
  0 siblings, 0 replies; 16+ messages in thread
From: Jarkko Sakkinen @ 2026-10-26 19:32 UTC (permalink / raw)
  To: Arun Menon; +Cc: linux-kernel, linux-integrity, Peter Huewe, Jason Gunthorpe

On Mon, Oct 26, 2026 at 08:00:10PM +0530, Arun Menon wrote:
> Hi Jarkko,
> Thank you for the review.
> 
> On Sun, Oct 25, 2026 at 11:00:42PM +0200, Jarkko Sakkinen wrote:
> > On Sun, Oct 25, 2026 at 10:59:09PM +0200, Jarkko Sakkinen wrote:
> > > On Mon, Oct 26, 2026 at 01:48:57AM +0530, Arun Menon wrote:
> > > > From: Arun Menon <armenon@redhat.com>
> > > > 
> > > > The size of the command is checked against TPM_BUFSIZE early on before
> > > > even sending it to the backend. We therefore need to increase the
> > > > TPM_BUFSIZE to allow support for larger commands.
> > > > 
> > > > For now, 8KB seems sufficient for ML-KEM and ML-DSA algorithms and it is
> > > > also order-1 safe.
> > > > 
> > > > Signed-off-by: Arun Menon <armenon@redhat.com>
> > > 
> > > In what timeline? Or is it out of spectrum timeline?
> > 
> > "For now" part concerns me and I'm not educated enough on topic to
> > know better.
> > 
> 
> TPM_BUFSIZE needs to hold the largest command the stack will ever
> build. TPM 2.0 Library Part 3: Commands, v185, adds ML-DSA key
> objects and the ML-KEM TPM2_Encapsulate()/TPM2_Decapsulate() commands
> (sections 14.10, 14.11) [1].
> 
> NIST FIPS 204, Table 2, gives the sizes for each ML-DSA parameter
> set; the largest, ML-DSA-87, has a 4896-byte private key and a
> 2592-byte public key [2]. Loading an ML-DSA-87 key pair carries both
> (4896 + 2592 bytes); add the command header, a handle and a session,
> and this comes to roughly 7.7KB. 8192 covers that with headroom and
> keeps the kzalloc(TPM_BUFSIZE, ...) calls in the TPM stack at an
> order-1 page allocation - something I figured after sashiko review.
> 
> This is sized to the largest parameter set defined today, not a
> timeline, and there's no guarantee it stays sufficient. For example,
> FIPS 205 (SLH-DSA), Table 2, already defines a 49856-byte signature for
> SLH-DSA-SHA2-256f -- six times the current buffer. If a future
> algorithm or parameter set needs more than 8192 bytes, this will need
> revisiting again [3].
> 
> I will add this detail in the commit message.

Thanks!

> 
> [1] https://medias.cdn.trustedcomputinggroup.org/production/Trusted_Platform_Module_2_0_Library_Part_3_Commands_Version_185_pub_87a5c10230.pdf
> [2] https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf
> [3] https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.205.pdf
> 
> 
> > Br, Jarkko
> > 
> 
> 
> Regards,
> Arun Menon
> 

Br, Jarkko

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-10-26 19:32 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-25 20:18 [PATCH v4 0/6] tpm_crb: Add command and response buffer chunking support Arun Menon
2026-10-25 20:18 ` [PATCH v4 1/6] tpm_crb: Add register definitions of TPM CRB chunking fields Arun Menon
2026-10-25 20:18 ` [PATCH v4 2/6] tpm_crb: Split start method into a separate header Arun Menon
2026-10-25 20:55   ` Jarkko Sakkinen
2026-10-26 14:28     ` Arun Menon
2026-10-25 20:56   ` Jarkko Sakkinen
2026-10-25 20:18 ` [PATCH v4 3/6] tpm_crb: Add start_cmd parameter to tpm_crb_start wrapper Arun Menon
2026-10-25 20:18 ` [PATCH v4 4/6] tpm: tis_i2c: Use local 4KB buffer to limit memory usage Arun Menon
2026-10-25 20:18 ` [PATCH v4 5/6] tpm: Increase TPM_BUFSIZE to 8kB for chunking support Arun Menon
2026-10-25 20:59   ` Jarkko Sakkinen
2026-10-25 21:00     ` Jarkko Sakkinen
2026-10-26 14:30       ` Arun Menon
2026-10-26 19:32         ` Jarkko Sakkinen
2026-10-25 20:18 ` [PATCH v4 6/6] tpm_crb: Implement command and response chunking logic Arun Menon
2026-10-25 21:59   ` Jarkko Sakkinen
2026-10-26 14:31     ` Arun Menon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®