Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On Mon, May 04, 2026 at 10:33:38AM -0400, Stefan Berger wrote:
>
>
> On 5/4/26 3:12 AM, Arun Menon wrote:
> > On Thu, Apr 30, 2026 at 07:46:33AM -0400, Stefan Berger wrote:
> > >
> > >
> > > On 4/30/26 1:11 AM, Arun Menon wrote:
> > > > On Wed, Apr 29, 2026 at 07:14:38PM -0400, Stefan Berger wrote:
> > > > >
> > > > >
> > > > > On 4/22/26 6:30 AM, Arun Menon wrote:
> > > > > > From: Arun Menon
> > > > > >
> > > > > > - Add subsection in VMState for TPM CRB with the newly introduced
> > > > > > command and response buffer GByteArrays, along with a needed
> > > > > > callback,
> > > > > > so that newer QEMU only sends the buffers if it is necessary.
> > > > > > - Implement a migration blocker to prevent migration of the VM if
> > > > > > the
> > > > > > user manually enables chunking capability, cap-chunk, but the
> > > > > > machine
> > > > > > type does not support it, using a new hw_compat property called
> > > > > > allow_chunk_migration.
> > > > > > - Add a post_load_errp hook so that during a migration, the buffers
> > > > > > are
> > > > > > validated before destination VM is started.
> > > > > >
> > > > > > Signed-off-by: Arun Menon
> > > > > > ---
> > > > > > hw/core/machine.c | 1 +
> > > > > > hw/tpm/tpm_crb.c | 71
> > > > > > +++
> > > > > > 2 files changed, 72 insertions(+)
> > > > > >
> > > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > > > index 6d27cf69a2..b590af0125 100644
> > > > > > --- a/hw/core/machine.c
> > > > > > +++ b/hw/core/machine.c
> > > > > > @@ -40,6 +40,7 @@
> > > > > >
> > > > > > GlobalProperty hw_compat_11_0[] = {
> > > > > > { "tpm-crb", "cap-chunk", "off"},
> > > > > > +{ "tpm-crb", "x-allow-chunk-migration", "off"},
> > > > > > };
> > > > > > const size_t hw_compat_11_0_len = G_N_ELEMENTS(hw_compat_11_0);
> > > > > >
> > > > > > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > > > > > index 29370d6f49..23e6948aee 100644
> > > > > > --- a/hw/tpm/tpm_crb.c
> > > > > > +++ b/hw/tpm/tpm_crb.c
> > > > > > @@ -24,6 +24,7 @@
> > > > > > #include "hw/pci/pci_ids.h"
> > > > > > #include "hw/acpi/tpm.h"
> > > > > > #include "migration/vmstate.h"
> > > > > > +#include "migration/blocker.h"
> > > > > > #include "system/tpm_backend.h"
> > > > > > #include "system/tpm_util.h"
> > > > > > #include "system/reset.h"
> > > > > > @@ -51,6 +52,8 @@ struct CRBState {
> > > > > > TPMPPI ppi;
> > > > > >
> > > > > > bool cap_chunk;
> > > > > > +bool allow_chunk_migration;
> > > > > > +Error *migration_blocker;
> > > > > > };
> > > > > > typedef struct CRBState CRBState;
> > > > > >
> > > > > > @@ -353,12 +356,63 @@ static int tpm_crb_pre_save(void *opaque)
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > +static bool tpm_crb_chunk_needed(void *opaque)
> > > > > > +{
> > > > > > +CRBState *s = opaque;
> > > > > > +
> > > > > > +if (!s->allow_chunk_migration) {
> > > > > > +return false;
> > > > > > +}
> > > > > > +
> > > > > > +return ((s->command_buffer && s->command_buffer->len > 0) ||
> > > > > > +(s->response_buffer && s->response_buffer->len > 0));
> > > > > > +}
> > > > > > +
> > > > > > +static bool tpm_crb_chunk_post_load(void *opaque, int version_id,
> > > > > > Error **errp)
> > > > > > +{
> > > > > > +CRBState *s = opaque;
> > > > > > +
> > > > > > +if (!s->response_buffer || !s->command_buffer) {
> > > > > > +error_setg(errp, "tpm-crb: Internal buffers are not
> > > > > > allocated");
> > > > >
> > > > > Could this happen that this state type is resumed but the buffers are
> > > > > not
> > > > > allocated?
> > > >
> > > > Yes, this is not required. Its just a defense. The tpm_realize()
> > > > function should allocate the command and response buffers.
> > > >
> > > > >
> > > > > > +return false;
> > > > > > +}
> > > > > > +if (s->response_offset > s->response_buffer->len) {
> > > > > > +error_setg(errp, "tpm-crb: Invalid response "
> > > > > > + "offset %" PRIu32 " in migration stream",
> > > > > > + s->response_offset);
> > > > > > +return false;
> > > > > > +}
> > > > >
> > > > > The check is correct but can this particular case occur other than
> > > > > through
> > > > > crafted input?
> > > >
> > > > Not that I can think of. The check is indeed added to address malicious
> > > > migration stream / crafted input.
> > >
> > > I think that devices are assuming trusted input for all devices resumed
> > > from
> > > their previously stored state...
> >
> > I see. Shall I remove these two checks at the beginning and keep only
> > the last one?
>
> I think so. For the last one I think you should write a description what
> role the external emulator can play here.
Sure. I have updated it in v6. Thank you.
>
>
> >
>
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On Thu, Apr 30, 2026 at 03:49:47PM -0400, Stefan Berger wrote:
>
>
> On 4/22/26 6:30 AM, Arun Menon wrote:
> > From: Arun Menon
> >
> > - Add subsection in VMState for TPM CRB with the newly introduced
> >command and response buffer GByteArrays, along with a needed callback,
> >so that newer QEMU only sends the buffers if it is necessary.
> > - Implement a migration blocker to prevent migration of the VM if the
> >user manually enables chunking capability, cap-chunk, but the machine
> >type does not support it, using a new hw_compat property called
> >allow_chunk_migration.
> > - Add a post_load_errp hook so that during a migration, the buffers are
> >validated before destination VM is started.
> >
> > Signed-off-by: Arun Menon
> > ---
> > hw/core/machine.c | 1 +
> > hw/tpm/tpm_crb.c | 71 +++
> > 2 files changed, 72 insertions(+)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 6d27cf69a2..b590af0125 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -40,6 +40,7 @@
> > GlobalProperty hw_compat_11_0[] = {
> > { "tpm-crb", "cap-chunk", "off"},
> > +{ "tpm-crb", "x-allow-chunk-migration", "off"},
>
> Nit: A space after the "off". Also in previous patch that added "cap-chunk".
Thanks. Changed it in v6.
>
>
Regards,
Arun Menon
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On 5/4/26 3:12 AM, Arun Menon wrote:
On Thu, Apr 30, 2026 at 07:46:33AM -0400, Stefan Berger wrote:
On 4/30/26 1:11 AM, Arun Menon wrote:
On Wed, Apr 29, 2026 at 07:14:38PM -0400, Stefan Berger wrote:
On 4/22/26 6:30 AM, Arun Menon wrote:
From: Arun Menon
- Add subsection in VMState for TPM CRB with the newly introduced
command and response buffer GByteArrays, along with a needed callback,
so that newer QEMU only sends the buffers if it is necessary.
- Implement a migration blocker to prevent migration of the VM if the
user manually enables chunking capability, cap-chunk, but the machine
type does not support it, using a new hw_compat property called
allow_chunk_migration.
- Add a post_load_errp hook so that during a migration, the buffers are
validated before destination VM is started.
Signed-off-by: Arun Menon
---
hw/core/machine.c | 1 +
hw/tpm/tpm_crb.c | 71 +++
2 files changed, 72 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6d27cf69a2..b590af0125 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,7 @@
GlobalProperty hw_compat_11_0[] = {
{ "tpm-crb", "cap-chunk", "off"},
+{ "tpm-crb", "x-allow-chunk-migration", "off"},
};
const size_t hw_compat_11_0_len = G_N_ELEMENTS(hw_compat_11_0);
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 29370d6f49..23e6948aee 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -24,6 +24,7 @@
#include "hw/pci/pci_ids.h"
#include "hw/acpi/tpm.h"
#include "migration/vmstate.h"
+#include "migration/blocker.h"
#include "system/tpm_backend.h"
#include "system/tpm_util.h"
#include "system/reset.h"
@@ -51,6 +52,8 @@ struct CRBState {
TPMPPI ppi;
bool cap_chunk;
+bool allow_chunk_migration;
+Error *migration_blocker;
};
typedef struct CRBState CRBState;
@@ -353,12 +356,63 @@ static int tpm_crb_pre_save(void *opaque)
return 0;
}
+static bool tpm_crb_chunk_needed(void *opaque)
+{
+CRBState *s = opaque;
+
+if (!s->allow_chunk_migration) {
+return false;
+}
+
+return ((s->command_buffer && s->command_buffer->len > 0) ||
+(s->response_buffer && s->response_buffer->len > 0));
+}
+
+static bool tpm_crb_chunk_post_load(void *opaque, int version_id, Error **errp)
+{
+CRBState *s = opaque;
+
+if (!s->response_buffer || !s->command_buffer) {
+error_setg(errp, "tpm-crb: Internal buffers are not allocated");
Could this happen that this state type is resumed but the buffers are not
allocated?
Yes, this is not required. Its just a defense. The tpm_realize()
function should allocate the command and response buffers.
+return false;
+}
+if (s->response_offset > s->response_buffer->len) {
+error_setg(errp, "tpm-crb: Invalid response "
+ "offset %" PRIu32 " in migration stream",
+ s->response_offset);
+return false;
+}
The check is correct but can this particular case occur other than through
crafted input?
Not that I can think of. The check is indeed added to address malicious
migration stream / crafted input.
I think that devices are assuming trusted input for all devices resumed from
their previously stored state...
I see. Shall I remove these two checks at the beginning and keep only
the last one?
I think so. For the last one I think you should write a description what
role the external emulator can play here.
+if (s->response_buffer->len > s->be_buffer_size ||
+s->command_buffer->len > s->be_buffer_size) {
I suppose this could happen if I was running with a PQC-enable swtpm/libtpms
and suspended at the right moment that when a chunked command or response
was in the buffer and now I am trying to resume with a non-PQC-enabled swtpm
that only has 4kb buffer, while the newer PQC one had 8kb buffer.
Yes. This check is for safety. We do not want to migrate to a VM that
has a tpm backend that does not support big buffers.
+error_setg(errp, "tpm-crb: Buffer sizes exceed backend capacity");
+return false;
+}
+return true;
+}
+
+static const VMStateDescription vmstate_tpm_crb_chunk = {
+.name = "tpm-crb/chunk",
+.version_id = 0,
+.needed = tpm_crb_chunk_needed,
+.post_load_errp = tpm_crb_chunk_post_load,
+.fields = (const VMStateField[]) {
+VMSTATE_GBYTEARRAY(command_buffer, CRBState, 0),
+VMSTATE_GBYTEARRAY(response_buffer, CRBState, 0),
+VMSTATE_UINT32(response_offset, CRBState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_tpm_crb = {
.name = "tpm-crb",
.pre_save = tpm_crb_pre_save,
.fields = (const VMStateField[]) {
VMSTATE_UINT32_ARRAY(regs, CRBState, TPM_CRB_R_MAX),
VMSTATE_END_OF_LIST(),
+},
+.subsections = (const
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On Thu, Apr 30, 2026 at 07:46:33AM -0400, Stefan Berger wrote:
>
>
> On 4/30/26 1:11 AM, Arun Menon wrote:
> > On Wed, Apr 29, 2026 at 07:14:38PM -0400, Stefan Berger wrote:
> > >
> > >
> > > On 4/22/26 6:30 AM, Arun Menon wrote:
> > > > From: Arun Menon
> > > >
> > > > - Add subsection in VMState for TPM CRB with the newly introduced
> > > > command and response buffer GByteArrays, along with a needed
> > > > callback,
> > > > so that newer QEMU only sends the buffers if it is necessary.
> > > > - Implement a migration blocker to prevent migration of the VM if the
> > > > user manually enables chunking capability, cap-chunk, but the
> > > > machine
> > > > type does not support it, using a new hw_compat property called
> > > > allow_chunk_migration.
> > > > - Add a post_load_errp hook so that during a migration, the buffers are
> > > > validated before destination VM is started.
> > > >
> > > > Signed-off-by: Arun Menon
> > > > ---
> > > >hw/core/machine.c | 1 +
> > > >hw/tpm/tpm_crb.c | 71
> > > > +++
> > > >2 files changed, 72 insertions(+)
> > > >
> > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > index 6d27cf69a2..b590af0125 100644
> > > > --- a/hw/core/machine.c
> > > > +++ b/hw/core/machine.c
> > > > @@ -40,6 +40,7 @@
> > > >
> > > >GlobalProperty hw_compat_11_0[] = {
> > > >{ "tpm-crb", "cap-chunk", "off"},
> > > > +{ "tpm-crb", "x-allow-chunk-migration", "off"},
> > > >};
> > > >const size_t hw_compat_11_0_len = G_N_ELEMENTS(hw_compat_11_0);
> > > >
> > > > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > > > index 29370d6f49..23e6948aee 100644
> > > > --- a/hw/tpm/tpm_crb.c
> > > > +++ b/hw/tpm/tpm_crb.c
> > > > @@ -24,6 +24,7 @@
> > > >#include "hw/pci/pci_ids.h"
> > > >#include "hw/acpi/tpm.h"
> > > >#include "migration/vmstate.h"
> > > > +#include "migration/blocker.h"
> > > >#include "system/tpm_backend.h"
> > > >#include "system/tpm_util.h"
> > > >#include "system/reset.h"
> > > > @@ -51,6 +52,8 @@ struct CRBState {
> > > >TPMPPI ppi;
> > > >
> > > >bool cap_chunk;
> > > > +bool allow_chunk_migration;
> > > > +Error *migration_blocker;
> > > >};
> > > >typedef struct CRBState CRBState;
> > > >
> > > > @@ -353,12 +356,63 @@ static int tpm_crb_pre_save(void *opaque)
> > > >return 0;
> > > >}
> > > >
> > > > +static bool tpm_crb_chunk_needed(void *opaque)
> > > > +{
> > > > +CRBState *s = opaque;
> > > > +
> > > > +if (!s->allow_chunk_migration) {
> > > > +return false;
> > > > +}
> > > > +
> > > > +return ((s->command_buffer && s->command_buffer->len > 0) ||
> > > > +(s->response_buffer && s->response_buffer->len > 0));
> > > > +}
> > > > +
> > > > +static bool tpm_crb_chunk_post_load(void *opaque, int version_id,
> > > > Error **errp)
> > > > +{
> > > > +CRBState *s = opaque;
> > > > +
> > > > +if (!s->response_buffer || !s->command_buffer) {
> > > > +error_setg(errp, "tpm-crb: Internal buffers are not
> > > > allocated");
> > >
> > > Could this happen that this state type is resumed but the buffers are not
> > > allocated?
> >
> > Yes, this is not required. Its just a defense. The tpm_realize()
> > function should allocate the command and response buffers.
> >
> > >
> > > > +return false;
> > > > +}
> > > > +if (s->response_offset > s->response_buffer->len) {
> > > > +error_setg(errp, "tpm-crb: Invalid response "
> > > > + "offset %" PRIu32 " in migration stream",
> > > > + s->response_offset);
> > > > +return false;
> > > > +}
> > >
> > > The check is correct but can this particular case occur other than through
> > > crafted input?
> >
> > Not that I can think of. The check is indeed added to address malicious
> > migration stream / crafted input.
>
> I think that devices are assuming trusted input for all devices resumed from
> their previously stored state...
I see. Shall I remove these two checks at the beginning and keep only
the last one?
>
> >
> > >
> > > > +if (s->response_buffer->len > s->be_buffer_size ||
> > > > +s->command_buffer->len > s->be_buffer_size) {
> > >
> > > I suppose this could happen if I was running with a PQC-enable
> > > swtpm/libtpms
> > > and suspended at the right moment that when a chunked command or response
> > > was in the buffer and now I am trying to resume with a non-PQC-enabled
> > > swtpm
> > > that only has 4kb buffer, while the newer PQC one had 8kb buffer.
> > >
> >
> > Yes. This check is for safety. We do not want to migrate to a VM that
> > has a tpm backend that does not support big buffers.
> >
> > > > +error_setg(errp, "tpm-crb: Buffer sizes exceed backend
> > > > capacity");
> > > > +return false;
> > > > +}
> > > > +return true;
> > > > +}
> > > > +
> > > >
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On 4/22/26 6:30 AM, Arun Menon wrote:
From: Arun Menon
- Add subsection in VMState for TPM CRB with the newly introduced
command and response buffer GByteArrays, along with a needed callback,
so that newer QEMU only sends the buffers if it is necessary.
- Implement a migration blocker to prevent migration of the VM if the
user manually enables chunking capability, cap-chunk, but the machine
type does not support it, using a new hw_compat property called
allow_chunk_migration.
- Add a post_load_errp hook so that during a migration, the buffers are
validated before destination VM is started.
Signed-off-by: Arun Menon
---
hw/core/machine.c | 1 +
hw/tpm/tpm_crb.c | 71 +++
2 files changed, 72 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6d27cf69a2..b590af0125 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,7 @@
GlobalProperty hw_compat_11_0[] = {
{ "tpm-crb", "cap-chunk", "off"},
+{ "tpm-crb", "x-allow-chunk-migration", "off"},
Nit: A space after the "off". Also in previous patch that added "cap-chunk".
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On 4/30/26 1:11 AM, Arun Menon wrote:
On Wed, Apr 29, 2026 at 07:14:38PM -0400, Stefan Berger wrote:
On 4/22/26 6:30 AM, Arun Menon wrote:
From: Arun Menon
- Add subsection in VMState for TPM CRB with the newly introduced
command and response buffer GByteArrays, along with a needed callback,
so that newer QEMU only sends the buffers if it is necessary.
- Implement a migration blocker to prevent migration of the VM if the
user manually enables chunking capability, cap-chunk, but the machine
type does not support it, using a new hw_compat property called
allow_chunk_migration.
- Add a post_load_errp hook so that during a migration, the buffers are
validated before destination VM is started.
Signed-off-by: Arun Menon
---
hw/core/machine.c | 1 +
hw/tpm/tpm_crb.c | 71 +++
2 files changed, 72 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6d27cf69a2..b590af0125 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,7 @@
GlobalProperty hw_compat_11_0[] = {
{ "tpm-crb", "cap-chunk", "off"},
+{ "tpm-crb", "x-allow-chunk-migration", "off"},
};
const size_t hw_compat_11_0_len = G_N_ELEMENTS(hw_compat_11_0);
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 29370d6f49..23e6948aee 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -24,6 +24,7 @@
#include "hw/pci/pci_ids.h"
#include "hw/acpi/tpm.h"
#include "migration/vmstate.h"
+#include "migration/blocker.h"
#include "system/tpm_backend.h"
#include "system/tpm_util.h"
#include "system/reset.h"
@@ -51,6 +52,8 @@ struct CRBState {
TPMPPI ppi;
bool cap_chunk;
+bool allow_chunk_migration;
+Error *migration_blocker;
};
typedef struct CRBState CRBState;
@@ -353,12 +356,63 @@ static int tpm_crb_pre_save(void *opaque)
return 0;
}
+static bool tpm_crb_chunk_needed(void *opaque)
+{
+CRBState *s = opaque;
+
+if (!s->allow_chunk_migration) {
+return false;
+}
+
+return ((s->command_buffer && s->command_buffer->len > 0) ||
+(s->response_buffer && s->response_buffer->len > 0));
+}
+
+static bool tpm_crb_chunk_post_load(void *opaque, int version_id, Error **errp)
+{
+CRBState *s = opaque;
+
+if (!s->response_buffer || !s->command_buffer) {
+error_setg(errp, "tpm-crb: Internal buffers are not allocated");
Could this happen that this state type is resumed but the buffers are not
allocated?
Yes, this is not required. Its just a defense. The tpm_realize()
function should allocate the command and response buffers.
+return false;
+}
+if (s->response_offset > s->response_buffer->len) {
+error_setg(errp, "tpm-crb: Invalid response "
+ "offset %" PRIu32 " in migration stream",
+ s->response_offset);
+return false;
+}
The check is correct but can this particular case occur other than through
crafted input?
Not that I can think of. The check is indeed added to address malicious
migration stream / crafted input.
I think that devices are assuming trusted input for all devices resumed
from their previously stored state...
+if (s->response_buffer->len > s->be_buffer_size ||
+s->command_buffer->len > s->be_buffer_size) {
I suppose this could happen if I was running with a PQC-enable swtpm/libtpms
and suspended at the right moment that when a chunked command or response
was in the buffer and now I am trying to resume with a non-PQC-enabled swtpm
that only has 4kb buffer, while the newer PQC one had 8kb buffer.
Yes. This check is for safety. We do not want to migrate to a VM that
has a tpm backend that does not support big buffers.
+error_setg(errp, "tpm-crb: Buffer sizes exceed backend capacity");
+return false;
+}
+return true;
+}
+
+static const VMStateDescription vmstate_tpm_crb_chunk = {
+.name = "tpm-crb/chunk",
+.version_id = 0,
+.needed = tpm_crb_chunk_needed,
+.post_load_errp = tpm_crb_chunk_post_load,
+.fields = (const VMStateField[]) {
+VMSTATE_GBYTEARRAY(command_buffer, CRBState, 0),
+VMSTATE_GBYTEARRAY(response_buffer, CRBState, 0),
+VMSTATE_UINT32(response_offset, CRBState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_tpm_crb = {
.name = "tpm-crb",
.pre_save = tpm_crb_pre_save,
.fields = (const VMStateField[]) {
VMSTATE_UINT32_ARRAY(regs, CRBState, TPM_CRB_R_MAX),
VMSTATE_END_OF_LIST(),
+},
+.subsections = (const VMStateDescription * const []) {
+&vmstate_tpm_crb_chunk,
+NULL,
}
};
@@ -366,6 +420,8 @@ static const Property tpm_crb_properties[] = {
DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe),
DEFINE_PROP_BOOL("ppi", CRBState, ppi_enabled, true),
DEFINE_PROP_BOOL("cap-chunk", CRBState, cap_chunk, true),
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On Wed, Apr 29, 2026 at 07:14:38PM -0400, Stefan Berger wrote:
>
>
> On 4/22/26 6:30 AM, Arun Menon wrote:
> > From: Arun Menon
> >
> > - Add subsection in VMState for TPM CRB with the newly introduced
> >command and response buffer GByteArrays, along with a needed callback,
> >so that newer QEMU only sends the buffers if it is necessary.
> > - Implement a migration blocker to prevent migration of the VM if the
> >user manually enables chunking capability, cap-chunk, but the machine
> >type does not support it, using a new hw_compat property called
> >allow_chunk_migration.
> > - Add a post_load_errp hook so that during a migration, the buffers are
> >validated before destination VM is started.
> >
> > Signed-off-by: Arun Menon
> > ---
> > hw/core/machine.c | 1 +
> > hw/tpm/tpm_crb.c | 71 +++
> > 2 files changed, 72 insertions(+)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 6d27cf69a2..b590af0125 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -40,6 +40,7 @@
> >
> > GlobalProperty hw_compat_11_0[] = {
> > { "tpm-crb", "cap-chunk", "off"},
> > +{ "tpm-crb", "x-allow-chunk-migration", "off"},
> > };
> > const size_t hw_compat_11_0_len = G_N_ELEMENTS(hw_compat_11_0);
> >
> > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > index 29370d6f49..23e6948aee 100644
> > --- a/hw/tpm/tpm_crb.c
> > +++ b/hw/tpm/tpm_crb.c
> > @@ -24,6 +24,7 @@
> > #include "hw/pci/pci_ids.h"
> > #include "hw/acpi/tpm.h"
> > #include "migration/vmstate.h"
> > +#include "migration/blocker.h"
> > #include "system/tpm_backend.h"
> > #include "system/tpm_util.h"
> > #include "system/reset.h"
> > @@ -51,6 +52,8 @@ struct CRBState {
> > TPMPPI ppi;
> >
> > bool cap_chunk;
> > +bool allow_chunk_migration;
> > +Error *migration_blocker;
> > };
> > typedef struct CRBState CRBState;
> >
> > @@ -353,12 +356,63 @@ static int tpm_crb_pre_save(void *opaque)
> > return 0;
> > }
> >
> > +static bool tpm_crb_chunk_needed(void *opaque)
> > +{
> > +CRBState *s = opaque;
> > +
> > +if (!s->allow_chunk_migration) {
> > +return false;
> > +}
> > +
> > +return ((s->command_buffer && s->command_buffer->len > 0) ||
> > +(s->response_buffer && s->response_buffer->len > 0));
> > +}
> > +
> > +static bool tpm_crb_chunk_post_load(void *opaque, int version_id, Error
> > **errp)
> > +{
> > +CRBState *s = opaque;
> > +
> > +if (!s->response_buffer || !s->command_buffer) {
> > +error_setg(errp, "tpm-crb: Internal buffers are not allocated");
>
> Could this happen that this state type is resumed but the buffers are not
> allocated?
Yes, this is not required. Its just a defense. The tpm_realize()
function should allocate the command and response buffers.
>
> > +return false;
> > +}
> > +if (s->response_offset > s->response_buffer->len) {
> > +error_setg(errp, "tpm-crb: Invalid response "
> > + "offset %" PRIu32 " in migration stream",
> > + s->response_offset);
> > +return false;
> > +}
>
> The check is correct but can this particular case occur other than through
> crafted input?
Not that I can think of. The check is indeed added to address malicious
migration stream / crafted input.
>
> > +if (s->response_buffer->len > s->be_buffer_size ||
> > +s->command_buffer->len > s->be_buffer_size) {
>
> I suppose this could happen if I was running with a PQC-enable swtpm/libtpms
> and suspended at the right moment that when a chunked command or response
> was in the buffer and now I am trying to resume with a non-PQC-enabled swtpm
> that only has 4kb buffer, while the newer PQC one had 8kb buffer.
>
Yes. This check is for safety. We do not want to migrate to a VM that
has a tpm backend that does not support big buffers.
> > +error_setg(errp, "tpm-crb: Buffer sizes exceed backend capacity");
> > +return false;
> > +}
> > +return true;
> > +}
> > +
> > +static const VMStateDescription vmstate_tpm_crb_chunk = {
> > +.name = "tpm-crb/chunk",
> > +.version_id = 0,
> > +.needed = tpm_crb_chunk_needed,
> > +.post_load_errp = tpm_crb_chunk_post_load,
> > +.fields = (const VMStateField[]) {
> > +VMSTATE_GBYTEARRAY(command_buffer, CRBState, 0),
> > +VMSTATE_GBYTEARRAY(response_buffer, CRBState, 0),
> > +VMSTATE_UINT32(response_offset, CRBState),
> > +VMSTATE_END_OF_LIST()
> > +}
> > +};
> > +
> > static const VMStateDescription vmstate_tpm_crb = {
> > .name = "tpm-crb",
> > .pre_save = tpm_crb_pre_save,
> > .fields = (const VMStateField[]) {
> > VMSTATE_UINT32_ARRAY(regs, CRBState, TPM_CRB_R_MAX),
> > VMSTATE_END_OF_LIST(),
> > +},
> > +.subsections = (const VMStateDescription * const []) {
> > +&vmstate_tpm_crb_chu
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On 4/22/26 6:30 AM, Arun Menon wrote:
From: Arun Menon
- Add subsection in VMState for TPM CRB with the newly introduced
command and response buffer GByteArrays, along with a needed callback,
so that newer QEMU only sends the buffers if it is necessary.
- Implement a migration blocker to prevent migration of the VM if the
user manually enables chunking capability, cap-chunk, but the machine
type does not support it, using a new hw_compat property called
allow_chunk_migration.
- Add a post_load_errp hook so that during a migration, the buffers are
validated before destination VM is started.
Signed-off-by: Arun Menon
---
hw/core/machine.c | 1 +
hw/tpm/tpm_crb.c | 71 +++
2 files changed, 72 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6d27cf69a2..b590af0125 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,7 @@
GlobalProperty hw_compat_11_0[] = {
{ "tpm-crb", "cap-chunk", "off"},
+{ "tpm-crb", "x-allow-chunk-migration", "off"},
};
const size_t hw_compat_11_0_len = G_N_ELEMENTS(hw_compat_11_0);
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 29370d6f49..23e6948aee 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -24,6 +24,7 @@
#include "hw/pci/pci_ids.h"
#include "hw/acpi/tpm.h"
#include "migration/vmstate.h"
+#include "migration/blocker.h"
#include "system/tpm_backend.h"
#include "system/tpm_util.h"
#include "system/reset.h"
@@ -51,6 +52,8 @@ struct CRBState {
TPMPPI ppi;
bool cap_chunk;
+bool allow_chunk_migration;
+Error *migration_blocker;
};
typedef struct CRBState CRBState;
@@ -353,12 +356,63 @@ static int tpm_crb_pre_save(void *opaque)
return 0;
}
+static bool tpm_crb_chunk_needed(void *opaque)
+{
+CRBState *s = opaque;
+
+if (!s->allow_chunk_migration) {
+return false;
+}
+
+return ((s->command_buffer && s->command_buffer->len > 0) ||
+(s->response_buffer && s->response_buffer->len > 0));
+}
+
+static bool tpm_crb_chunk_post_load(void *opaque, int version_id, Error **errp)
+{
+CRBState *s = opaque;
+
+if (!s->response_buffer || !s->command_buffer) {
+error_setg(errp, "tpm-crb: Internal buffers are not allocated");
Could this happen that this state type is resumed but the buffers are
not allocated?
+return false;
+}
+if (s->response_offset > s->response_buffer->len) {
+error_setg(errp, "tpm-crb: Invalid response "
+ "offset %" PRIu32 " in migration stream",
+ s->response_offset);
+return false;
+}
The check is correct but can this particular case occur other than
through crafted input?
+if (s->response_buffer->len > s->be_buffer_size ||
+s->command_buffer->len > s->be_buffer_size) {
I suppose this could happen if I was running with a PQC-enable
swtpm/libtpms and suspended at the right moment that when a chunked
command or response was in the buffer and now I am trying to resume with
a non-PQC-enabled swtpm that only has 4kb buffer, while the newer PQC
one had 8kb buffer.
+error_setg(errp, "tpm-crb: Buffer sizes exceed backend capacity");
+return false;
+}
+return true;
+}
+
+static const VMStateDescription vmstate_tpm_crb_chunk = {
+.name = "tpm-crb/chunk",
+.version_id = 0,
+.needed = tpm_crb_chunk_needed,
+.post_load_errp = tpm_crb_chunk_post_load,
+.fields = (const VMStateField[]) {
+VMSTATE_GBYTEARRAY(command_buffer, CRBState, 0),
+VMSTATE_GBYTEARRAY(response_buffer, CRBState, 0),
+VMSTATE_UINT32(response_offset, CRBState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_tpm_crb = {
.name = "tpm-crb",
.pre_save = tpm_crb_pre_save,
.fields = (const VMStateField[]) {
VMSTATE_UINT32_ARRAY(regs, CRBState, TPM_CRB_R_MAX),
VMSTATE_END_OF_LIST(),
+},
+.subsections = (const VMStateDescription * const []) {
+&vmstate_tpm_crb_chunk,
+NULL,
}
};
@@ -366,6 +420,8 @@ static const Property tpm_crb_properties[] = {
DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe),
DEFINE_PROP_BOOL("ppi", CRBState, ppi_enabled, true),
DEFINE_PROP_BOOL("cap-chunk", CRBState, cap_chunk, true),
+DEFINE_PROP_BOOL("x-allow-chunk-migration", CRBState,
+ allow_chunk_migration, true),
};
static void tpm_crb_reset(void *dev)
@@ -422,6 +478,7 @@ static void tpm_crb_reset(void *dev)
static void tpm_crb_realize(DeviceState *dev, Error **errp)
{
CRBState *s = CRB(dev);
+int ret;
if (!tpm_find()) {
error_setg(errp, "at most one TPM device is permitted");
@@ -431,6 +488,15 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
error_setg(errp, "'tpmdev' property is required");
return;
}
+if (s->cap_chunk && !s
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
Hi,
On Wed, Apr 29, 2026 at 11:36:39AM -0400, Stefan Berger wrote:
>
>
> On 4/22/26 6:30 AM, Arun Menon wrote:
> > From: Arun Menon
> >
> > - Add subsection in VMState for TPM CRB with the newly introduced
> >command and response buffer GByteArrays, along with a needed callback,
> >so that newer QEMU only sends the buffers if it is necessary.
> > - Implement a migration blocker to prevent migration of the VM if the
> >user manually enables chunking capability, cap-chunk, but the machine
> >type does not support it, using a new hw_compat property called
> >allow_chunk_migration.
> > - Add a post_load_errp hook so that during a migration, the buffers are
> >validated before destination VM is started.
> >
> > Signed-off-by: Arun Menon
> > ---
> > hw/core/machine.c | 1 +
> > hw/tpm/tpm_crb.c | 71 +++
> > 2 files changed, 72 insertions(+)
> >
> > @@ -422,6 +478,7 @@ static void tpm_crb_reset(void *dev)
> > static void tpm_crb_realize(DeviceState *dev, Error **errp)
> > {
> > CRBState *s = CRB(dev);
> > +int ret;
> > if (!tpm_find()) {
> > error_setg(errp, "at most one TPM device is permitted");
> > @@ -431,6 +488,15 @@ static void tpm_crb_realize(DeviceState *dev, Error
> > **errp)
> > error_setg(errp, "'tpmdev' property is required");
> > return;
> > }
> > +if (s->cap_chunk && !s->allow_chunk_migration) {
> > +error_setg(&s->migration_blocker,
> > + "The tpm-crb device does not support chunk migration
> > with "
> > + "machine version less than 11.1");
> > +ret = migrate_add_blocker_normal(&s->migration_blocker, errp);
> > +if (ret < 0) {
> > +return;
>
> Should this do an error_report() and exit(1)?
The tpm_crb_realize() function has always propagated errors in errp
back to the caller and returned in case of an error.
To be honest I followed the suit. A quick glance at the codebase shows
that the realize function in other devices also avoid calling exit().
Regards,
Arun Menon
>
>
Re: [PATCH v5 06/10] hw/tpm: Add support for VM migration with TPM CRB chunking
On 4/22/26 6:30 AM, Arun Menon wrote:
From: Arun Menon
- Add subsection in VMState for TPM CRB with the newly introduced
command and response buffer GByteArrays, along with a needed callback,
so that newer QEMU only sends the buffers if it is necessary.
- Implement a migration blocker to prevent migration of the VM if the
user manually enables chunking capability, cap-chunk, but the machine
type does not support it, using a new hw_compat property called
allow_chunk_migration.
- Add a post_load_errp hook so that during a migration, the buffers are
validated before destination VM is started.
Signed-off-by: Arun Menon
---
hw/core/machine.c | 1 +
hw/tpm/tpm_crb.c | 71 +++
2 files changed, 72 insertions(+)
@@ -422,6 +478,7 @@ static void tpm_crb_reset(void *dev)
static void tpm_crb_realize(DeviceState *dev, Error **errp)
{
CRBState *s = CRB(dev);
+int ret;
if (!tpm_find()) {
error_setg(errp, "at most one TPM device is permitted");
@@ -431,6 +488,15 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
error_setg(errp, "'tpmdev' property is required");
return;
}
+if (s->cap_chunk && !s->allow_chunk_migration) {
+error_setg(&s->migration_blocker,
+ "The tpm-crb device does not support chunk migration with "
+ "machine version less than 11.1");
+ret = migrate_add_blocker_normal(&s->migration_blocker, errp);
+if (ret < 0) {
+return;
Should this do an error_report() and exit(1)?
