Re: [PATCH] tpm: Dynamically allocate tpm-tis buffer

[Stefan Berger]

On 4/28/26 3:01 AM, Arun Menon wrote:
Hi Stefan,

Thank you for taking a look.

On Mon, Apr 27, 2026 at 04:49:04PM -0400, Stefan Berger wrote:


On 4/27/26 4:01 PM, Arun Menon wrote:
From: Arun Menon <arme...@redhat.com>

The TPM TIS buffer is currently a fixed-size static array. Change this
to a dynamically allocated heap block. The buffer size is now determined
at runtime by querying the TPM backend.

Do we really need this? I mean for the forseeable future 8kb should be
sufficient.

You are right that 8kb should be sufficient.
I implemented this to address the TODO mentioned here:
https://github.com/qemu/qemu/commit/e5f62d87e3c03bda6006085cf6303736fb57f5c5

That is why this patch is  deliberately posted outside the v5 series.
It is more about future-proofing than a strict requirement. I thought it
was worth addressing while the context was fresh. I am happy to leave it
out if we prefer that.

Actually, go ahead. It's better than hard coding the size.




To support VM migration,
1. Replace the static VMSTATE_BUFFER macro with pointer-based variant
     VMSTATE_BUFFER_POINTER_UNSAFE, explicitly mentioning the size.
2. Introduce ext_buffer and ext_size in the migration subsection to
     track allocation exceeding TPM_TIS_BUFFER_MAX. Allocate ext_buffer
     using VMSTATE_VBUFFER_ALLOC_UINT32 only to be freed later after it is
     appended to the main buffer.

This allows us to migrate to a destination host without breaking
backward compatibility. Old QEMU does not include a size field along
with the buffer in the migration stream, and therefore the
new QEMU is also forced to keep expecting exactly 4096 bytes.

Implement a post_load hook that will validate the incoming data size
from the migration stream, failing the migration if it exceeds the
destination backend capacity. Add unrealize functions for the TIS interface
types ISA, SysBus and I2C to ensure that the buffer is safely freed on
device destruction.

Signed-off-by: Arun Menon <arme...@redhat.com>
---
Dependencies:
This patch depends on the following patch currently in the mailing list:
https://lore.kernel.org/qemu-devel/20260422103018.123608-10-arme...@redhat.com/

I had tried to apply your v5 series to master but could not. Do you have a
git repo where you keep your patches?

Yes. Here is the repo: 
https://gitlab.com/armenon/qemu-dev/-/tree/pqc_tpm?ref_type=heads
I have applied the GByteArray change first, followed by the 10 commits.



Depends-on: <20260422103018.123608-10-arme...@redhat.com>

   hw/tpm/tpm_tis.h        |  6 ++++-
   hw/tpm/tpm_tis_common.c | 56 +++++++++++++++++++++++++++++++++++------
   hw/tpm/tpm_tis_i2c.c    | 28 +++++++++++++++++++--
   hw/tpm/tpm_tis_isa.c    | 31 +++++++++++++++++++++--
   hw/tpm/tpm_tis_sysbus.c | 32 +++++++++++++++++++++--
   5 files changed, 138 insertions(+), 15 deletions(-)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index b2d9c0116c..c736ecedc1 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -56,7 +56,9 @@ typedef struct TPMLocality {
   typedef struct TPMState {
       MemoryRegion mmio;
-    unsigned char buffer[TPM_TIS_BUFFER_MAX];
+    uint8_t *buffer;
+    uint8_t *ext_buffer;
+    uint32_t ext_size;
       uint16_t rw_offset;
       uint8_t active_locty;
@@ -82,6 +84,8 @@ extern const VMStateDescription vmstate_locty;
   extern const MemoryRegionOps tpm_tis_memory_ops;
   int tpm_tis_pre_save(TPMState *s);
+int tpm_tis_post_load(TPMState *s);
+int tpm_tis_ext_buffer_post_load(TPMState *s);
   void tpm_tis_reset(TPMState *s, bool ppi_enabled);
   enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
   void tpm_tis_request_completed(TPMState *s, int ret);
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 43e68410f8..c9c4dd1190 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -270,7 +270,7 @@ static uint32_t tpm_tis_data_read(TPMState *s, uint8_t 
locty)
       uint16_t len;
       if ((s->loc[locty].sts & TPM_TIS_STS_DATA_AVAILABLE)) {
-        len = MIN(tpm_cmd_get_size(&s->buffer),
+        len = MIN(tpm_cmd_get_size(s->buffer),
                     s->be_buffer_size);
           ret = s->buffer[s->rw_offset++];
@@ -317,7 +317,7 @@ static void tpm_tis_dump_state(TPMState *s, hwaddr addr)
              "tpm_tis: result buffer : ",
              s->rw_offset);
       for (idx = 0;
-         idx < MIN(tpm_cmd_get_size(&s->buffer), s->be_buffer_size);
+         idx < MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size);
            idx++) {
           printf("%c%02x%s",
                  s->rw_offset == idx ? '>' : ' ',
@@ -383,7 +383,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
           if (s->active_locty == locty) {
               if ((s->loc[locty].sts & TPM_TIS_STS_DATA_AVAILABLE)) {
                   val = TPM_TIS_BURST_COUNT(
-                       MIN(tpm_cmd_get_size(&s->buffer),
+                       MIN(tpm_cmd_get_size(s->buffer),
                              s->be_buffer_size)
                          - s->rw_offset) | s->loc[locty].sts;
               } else {
@@ -754,7 +754,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
                   /* we have a packet length - see if we have all of it */
                   bool need_irq = !(s->loc[locty].sts & TPM_TIS_STS_VALID);
-                len = tpm_cmd_get_size(&s->buffer);
+                len = tpm_cmd_get_size(s->buffer);
                   if (len > s->rw_offset) {
                       tpm_tis_sts_set(&s->loc[locty],
                                       TPM_TIS_STS_EXPECT | TPM_TIS_STS_VALID);
@@ -818,9 +818,10 @@ void tpm_tis_reset(TPMState *s, bool ppi_enabled)
       int c;
       s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
-    s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
-                            TPM_TIS_BUFFER_MAX);
+    s->be_buffer_size = tpm_backend_get_buffer_size(s->be_driver);
+    s->buffer = g_realloc(s->buffer, MAX(s->be_buffer_size,
+                          TPM_TIS_BUFFER_MAX));

With MAX() it can now be bigger than TPM_TIS_BUFFER_MAX if the backend says
so -- hm...

TPM_TIS_BUFFER_MAX is still 4096.

Oh, I had still based my patches on your TPM_TIS_BUFFER_MAX increase to 
8192 bytes. Let me fixes this along with a few other things. I will let 
you know.

I have another series that I will post now that can be applied to 
master. It's adding a test for TIS over I2C but I will need to extend 
that one also with the large transfer test case then.