* [Qemu-devel] [PATCH 1/5] tpm: Move getting TPM buffer size to backends
2017-11-07 0:58 [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Stefan Berger
@ 2017-11-07 0:58 ` Stefan Berger
2017-11-08 16:21 ` Marc-André Lureau
2017-11-07 0:58 ` [Qemu-devel] [PATCH 2/5] tpm: pull tpm_util_send() out of tpm_util_test() Stefan Berger
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Stefan Berger @ 2017-11-07 0:58 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: amarnath.valluri, Stefan Berger
Rather than setting the size of the TPM buffer in the front-end,
query the backend for the size of the buffer. In this patch we
just move the hard-coded buffer size of 4096 to the backends.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
backends/tpm.c | 9 +++++++++
hw/tpm/tpm_emulator.c | 6 ++++++
hw/tpm/tpm_passthrough.c | 6 ++++++
hw/tpm/tpm_tis.c | 12 +++++++-----
include/sysemu/tpm_backend.h | 11 +++++++++++
5 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/backends/tpm.c b/backends/tpm.c
index 7777467..e7d0f9a 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -139,6 +139,15 @@ TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
return k->get_tpm_version(s);
}
+uint32_t tpm_backend_get_buffer_size(TPMBackend *s)
+{
+ TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+
+ assert(k->get_buffer_size);
+
+ return k->get_buffer_size(s);
+}
+
TPMInfo *tpm_backend_query_tpm(TPMBackend *s)
{
TPMInfo *info = g_new0(TPMInfo, 1);
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 24cb611..5a6107e 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -356,6 +356,11 @@ static TPMVersion tpm_emulator_get_tpm_version(TPMBackend *tb)
return tpm_emu->tpm_version;
}
+static uint32_t tpm_emulator_get_buffer_size(TPMBackend *tb)
+{
+ return 4096;
+}
+
static int tpm_emulator_block_migration(TPMEmulator *tpm_emu)
{
Error *err = NULL;
@@ -556,6 +561,7 @@ static void tpm_emulator_class_init(ObjectClass *klass, void *data)
tbc->get_tpm_established_flag = tpm_emulator_get_tpm_established_flag;
tbc->reset_tpm_established_flag = tpm_emulator_reset_tpm_established_flag;
tbc->get_tpm_version = tpm_emulator_get_tpm_version;
+ tbc->get_buffer_size = tpm_emulator_get_buffer_size;
tbc->get_tpm_options = tpm_emulator_get_tpm_options;
tbc->handle_request = tpm_emulator_handle_request;
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 73554aa..7ff9249 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -199,6 +199,11 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
return tpm_pt->tpm_version;
}
+static uint32_t tpm_passthrough_get_buffer_size(TPMBackend *tb)
+{
+ return 4096;
+}
+
/*
* Unless path or file descriptor set has been provided by user,
* determine the sysfs cancel file following kernel documentation
@@ -354,6 +359,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
tbc->reset_tpm_established_flag =
tpm_passthrough_reset_tpm_established_flag;
tbc->get_tpm_version = tpm_passthrough_get_tpm_version;
+ tbc->get_buffer_size = tpm_passthrough_get_buffer_size;
tbc->get_tpm_options = tpm_passthrough_get_tpm_options;
tbc->handle_request = tpm_passthrough_handle_request;
}
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index cc32fcd..a3df40f 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -88,6 +88,8 @@ typedef struct TPMState {
TPMBackend *be_driver;
TPMVersion be_tpm_version;
+
+ uint32_t be_buffer_size;
} TPMState;
#define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
@@ -977,10 +979,9 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
return tpm_backend_startup_tpm(s->be_driver);
}
-static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb)
+static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb,
+ uint32_t wanted_size)
{
- size_t wanted_size = 4096; /* Linux tpm.c buffer size */
-
if (sb->size != wanted_size) {
sb->buffer = g_realloc(sb->buffer, wanted_size);
sb->size = wanted_size;
@@ -1007,6 +1008,7 @@ static void tpm_tis_reset(DeviceState *dev)
int c;
s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
+ s->be_buffer_size = tpm_backend_get_buffer_size(s->be_driver);
tpm_backend_reset(s->be_driver);
@@ -1033,9 +1035,9 @@ static void tpm_tis_reset(DeviceState *dev)
s->loc[c].state = TPM_TIS_STATE_IDLE;
s->loc[c].w_offset = 0;
- tpm_tis_realloc_buffer(&s->loc[c].w_buffer);
+ tpm_tis_realloc_buffer(&s->loc[c].w_buffer, s->be_buffer_size);
s->loc[c].r_offset = 0;
- tpm_tis_realloc_buffer(&s->loc[c].r_buffer);
+ tpm_tis_realloc_buffer(&s->loc[c].r_buffer, s->be_buffer_size);
}
tpm_tis_do_startup_tpm(s);
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 590e8b4..d23cef2 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -80,6 +80,7 @@ struct TPMBackendClass {
int (*reset_tpm_established_flag)(TPMBackend *t, uint8_t locty);
TPMVersion (*get_tpm_version)(TPMBackend *t);
+ uint32_t (*get_buffer_size)(TPMBackend *t);
TpmTypeOptions *(*get_tpm_options)(TPMBackend *t);
@@ -183,6 +184,16 @@ int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty);
TPMVersion tpm_backend_get_tpm_version(TPMBackend *s);
/**
+ * tpm_backend_get_buffer_size:
+ * @s: the backend to call into
+ *
+ * Get the TPM's buffer size.
+ *
+ * Returns buffer size.
+ */
+uint32_t tpm_backend_get_buffer_size(TPMBackend *s);
+
+/**
* tpm_backend_query_tpm:
* @s: the backend
*
--
2.5.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] tpm: Move getting TPM buffer size to backends
2017-11-07 0:58 ` [Qemu-devel] [PATCH 1/5] tpm: Move getting TPM buffer size to backends Stefan Berger
@ 2017-11-08 16:21 ` Marc-André Lureau
2017-11-08 18:19 ` Stefan Berger
0 siblings, 1 reply; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-08 16:21 UTC (permalink / raw)
To: Stefan Berger; +Cc: qemu-devel, amarnath.valluri
On Mon, Nov 06, 2017 at 07:58:52PM -0500, Stefan Berger wrote:
> Rather than setting the size of the TPM buffer in the front-end,
> query the backend for the size of the buffer. In this patch we
> just move the hard-coded buffer size of 4096 to the backends.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> backends/tpm.c | 9 +++++++++
> hw/tpm/tpm_emulator.c | 6 ++++++
> hw/tpm/tpm_passthrough.c | 6 ++++++
> hw/tpm/tpm_tis.c | 12 +++++++-----
> include/sysemu/tpm_backend.h | 11 +++++++++++
> 5 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 7777467..e7d0f9a 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -139,6 +139,15 @@ TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
> return k->get_tpm_version(s);
> }
>
> +uint32_t tpm_backend_get_buffer_size(TPMBackend *s)
We have a slight preference for size_t in qemu (even if underlying
protocol uses u32)
> +{
> + TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> + assert(k->get_buffer_size);
No need to have assert here to be consitent with qemu code, however it
is nice to have a comment on the interface declaration instead.
(I considered all function pointers mandatory unless explicitely
marked optional, as in "tpm-be: update optional function pointers")
> +
> + return k->get_buffer_size(s);
> +}
> +
> TPMInfo *tpm_backend_query_tpm(TPMBackend *s)
> {
> TPMInfo *info = g_new0(TPMInfo, 1);
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 24cb611..5a6107e 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -356,6 +356,11 @@ static TPMVersion tpm_emulator_get_tpm_version(TPMBackend *tb)
> return tpm_emu->tpm_version;
> }
>
> +static uint32_t tpm_emulator_get_buffer_size(TPMBackend *tb)
> +{
> + return 4096;
> +}
> +
> static int tpm_emulator_block_migration(TPMEmulator *tpm_emu)
> {
> Error *err = NULL;
> @@ -556,6 +561,7 @@ static void tpm_emulator_class_init(ObjectClass *klass, void *data)
> tbc->get_tpm_established_flag = tpm_emulator_get_tpm_established_flag;
> tbc->reset_tpm_established_flag = tpm_emulator_reset_tpm_established_flag;
> tbc->get_tpm_version = tpm_emulator_get_tpm_version;
> + tbc->get_buffer_size = tpm_emulator_get_buffer_size;
> tbc->get_tpm_options = tpm_emulator_get_tpm_options;
>
> tbc->handle_request = tpm_emulator_handle_request;
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 73554aa..7ff9249 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -199,6 +199,11 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
> return tpm_pt->tpm_version;
> }
>
> +static uint32_t tpm_passthrough_get_buffer_size(TPMBackend *tb)
> +{
> + return 4096;
> +}
> +
> /*
> * Unless path or file descriptor set has been provided by user,
> * determine the sysfs cancel file following kernel documentation
> @@ -354,6 +359,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
> tbc->reset_tpm_established_flag =
> tpm_passthrough_reset_tpm_established_flag;
> tbc->get_tpm_version = tpm_passthrough_get_tpm_version;
> + tbc->get_buffer_size = tpm_passthrough_get_buffer_size;
> tbc->get_tpm_options = tpm_passthrough_get_tpm_options;
> tbc->handle_request = tpm_passthrough_handle_request;
> }
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index cc32fcd..a3df40f 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -88,6 +88,8 @@ typedef struct TPMState {
>
> TPMBackend *be_driver;
> TPMVersion be_tpm_version;
> +
> + uint32_t be_buffer_size;
> } TPMState;
>
> #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
> @@ -977,10 +979,9 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
> return tpm_backend_startup_tpm(s->be_driver);
> }
>
> -static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb)
> +static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb,
> + uint32_t wanted_size)
> {
> - size_t wanted_size = 4096; /* Linux tpm.c buffer size */
> -
> if (sb->size != wanted_size) {
> sb->buffer = g_realloc(sb->buffer, wanted_size);
> sb->size = wanted_size;
> @@ -1007,6 +1008,7 @@ static void tpm_tis_reset(DeviceState *dev)
> int c;
>
> s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
> + s->be_buffer_size = tpm_backend_get_buffer_size(s->be_driver);
>
> tpm_backend_reset(s->be_driver);
>
> @@ -1033,9 +1035,9 @@ static void tpm_tis_reset(DeviceState *dev)
> s->loc[c].state = TPM_TIS_STATE_IDLE;
>
> s->loc[c].w_offset = 0;
> - tpm_tis_realloc_buffer(&s->loc[c].w_buffer);
> + tpm_tis_realloc_buffer(&s->loc[c].w_buffer, s->be_buffer_size);
> s->loc[c].r_offset = 0;
> - tpm_tis_realloc_buffer(&s->loc[c].r_buffer);
> + tpm_tis_realloc_buffer(&s->loc[c].r_buffer, s->be_buffer_size);
> }
>
> tpm_tis_do_startup_tpm(s);
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index 590e8b4..d23cef2 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -80,6 +80,7 @@ struct TPMBackendClass {
> int (*reset_tpm_established_flag)(TPMBackend *t, uint8_t locty);
>
> TPMVersion (*get_tpm_version)(TPMBackend *t);
> + uint32_t (*get_buffer_size)(TPMBackend *t);
>
> TpmTypeOptions *(*get_tpm_options)(TPMBackend *t);
>
> @@ -183,6 +184,16 @@ int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty);
> TPMVersion tpm_backend_get_tpm_version(TPMBackend *s);
>
> /**
> + * tpm_backend_get_buffer_size:
> + * @s: the backend to call into
> + *
> + * Get the TPM's buffer size.
> + *
> + * Returns buffer size.
> + */
> +uint32_t tpm_backend_get_buffer_size(TPMBackend *s);
> +
> +/**
> * tpm_backend_query_tpm:
> * @s: the backend
> *
> --
> 2.5.5
>
>
Looks good to me otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] tpm: Move getting TPM buffer size to backends
2017-11-08 16:21 ` Marc-André Lureau
@ 2017-11-08 18:19 ` Stefan Berger
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Berger @ 2017-11-08 18:19 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, amarnath.valluri
On 11/08/2017 11:21 AM, Marc-André Lureau wrote:
> On Mon, Nov 06, 2017 at 07:58:52PM -0500, Stefan Berger wrote:
>> Rather than setting the size of the TPM buffer in the front-end,
>> query the backend for the size of the buffer. In this patch we
>> just move the hard-coded buffer size of 4096 to the backends.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>> backends/tpm.c | 9 +++++++++
>> hw/tpm/tpm_emulator.c | 6 ++++++
>> hw/tpm/tpm_passthrough.c | 6 ++++++
>> hw/tpm/tpm_tis.c | 12 +++++++-----
>> include/sysemu/tpm_backend.h | 11 +++++++++++
>> 5 files changed, 39 insertions(+), 5 deletions(-)
>>
>> diff --git a/backends/tpm.c b/backends/tpm.c
>> index 7777467..e7d0f9a 100644
>> --- a/backends/tpm.c
>> +++ b/backends/tpm.c
>> @@ -139,6 +139,15 @@ TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
>> return k->get_tpm_version(s);
>> }
>>
>> +uint32_t tpm_backend_get_buffer_size(TPMBackend *s)
> We have a slight preference for size_t in qemu (even if underlying
> protocol uses u32)
I changed this now throughout the series. I'll collect the Reviewed-by's
next time.
>
>> +{
>> + TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>> +
>> + assert(k->get_buffer_size);
>
> No need to have assert here to be consitent with qemu code, however it
> is nice to have a comment on the interface declaration instead.
>
> (I considered all function pointers mandatory unless explicitely
> marked optional, as in "tpm-be: update optional function pointers")
Removed the assert.
Stefan
>
>> +
>> + return k->get_buffer_size(s);
>> +}
>> +
>> TPMInfo *tpm_backend_query_tpm(TPMBackend *s)
>> {
>> TPMInfo *info = g_new0(TPMInfo, 1);
>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>> index 24cb611..5a6107e 100644
>> --- a/hw/tpm/tpm_emulator.c
>> +++ b/hw/tpm/tpm_emulator.c
>> @@ -356,6 +356,11 @@ static TPMVersion tpm_emulator_get_tpm_version(TPMBackend *tb)
>> return tpm_emu->tpm_version;
>> }
>>
>> +static uint32_t tpm_emulator_get_buffer_size(TPMBackend *tb)
>> +{
>> + return 4096;
>> +}
>> +
>> static int tpm_emulator_block_migration(TPMEmulator *tpm_emu)
>> {
>> Error *err = NULL;
>> @@ -556,6 +561,7 @@ static void tpm_emulator_class_init(ObjectClass *klass, void *data)
>> tbc->get_tpm_established_flag = tpm_emulator_get_tpm_established_flag;
>> tbc->reset_tpm_established_flag = tpm_emulator_reset_tpm_established_flag;
>> tbc->get_tpm_version = tpm_emulator_get_tpm_version;
>> + tbc->get_buffer_size = tpm_emulator_get_buffer_size;
>> tbc->get_tpm_options = tpm_emulator_get_tpm_options;
>>
>> tbc->handle_request = tpm_emulator_handle_request;
>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>> index 73554aa..7ff9249 100644
>> --- a/hw/tpm/tpm_passthrough.c
>> +++ b/hw/tpm/tpm_passthrough.c
>> @@ -199,6 +199,11 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
>> return tpm_pt->tpm_version;
>> }
>>
>> +static uint32_t tpm_passthrough_get_buffer_size(TPMBackend *tb)
>> +{
>> + return 4096;
>> +}
>> +
>> /*
>> * Unless path or file descriptor set has been provided by user,
>> * determine the sysfs cancel file following kernel documentation
>> @@ -354,6 +359,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
>> tbc->reset_tpm_established_flag =
>> tpm_passthrough_reset_tpm_established_flag;
>> tbc->get_tpm_version = tpm_passthrough_get_tpm_version;
>> + tbc->get_buffer_size = tpm_passthrough_get_buffer_size;
>> tbc->get_tpm_options = tpm_passthrough_get_tpm_options;
>> tbc->handle_request = tpm_passthrough_handle_request;
>> }
>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>> index cc32fcd..a3df40f 100644
>> --- a/hw/tpm/tpm_tis.c
>> +++ b/hw/tpm/tpm_tis.c
>> @@ -88,6 +88,8 @@ typedef struct TPMState {
>>
>> TPMBackend *be_driver;
>> TPMVersion be_tpm_version;
>> +
>> + uint32_t be_buffer_size;
>> } TPMState;
>>
>> #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
>> @@ -977,10 +979,9 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
>> return tpm_backend_startup_tpm(s->be_driver);
>> }
>>
>> -static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb)
>> +static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb,
>> + uint32_t wanted_size)
>> {
>> - size_t wanted_size = 4096; /* Linux tpm.c buffer size */
>> -
>> if (sb->size != wanted_size) {
>> sb->buffer = g_realloc(sb->buffer, wanted_size);
>> sb->size = wanted_size;
>> @@ -1007,6 +1008,7 @@ static void tpm_tis_reset(DeviceState *dev)
>> int c;
>>
>> s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
>> + s->be_buffer_size = tpm_backend_get_buffer_size(s->be_driver);
>>
>> tpm_backend_reset(s->be_driver);
>>
>> @@ -1033,9 +1035,9 @@ static void tpm_tis_reset(DeviceState *dev)
>> s->loc[c].state = TPM_TIS_STATE_IDLE;
>>
>> s->loc[c].w_offset = 0;
>> - tpm_tis_realloc_buffer(&s->loc[c].w_buffer);
>> + tpm_tis_realloc_buffer(&s->loc[c].w_buffer, s->be_buffer_size);
>> s->loc[c].r_offset = 0;
>> - tpm_tis_realloc_buffer(&s->loc[c].r_buffer);
>> + tpm_tis_realloc_buffer(&s->loc[c].r_buffer, s->be_buffer_size);
>> }
>>
>> tpm_tis_do_startup_tpm(s);
>> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
>> index 590e8b4..d23cef2 100644
>> --- a/include/sysemu/tpm_backend.h
>> +++ b/include/sysemu/tpm_backend.h
>> @@ -80,6 +80,7 @@ struct TPMBackendClass {
>> int (*reset_tpm_established_flag)(TPMBackend *t, uint8_t locty);
>>
>> TPMVersion (*get_tpm_version)(TPMBackend *t);
>> + uint32_t (*get_buffer_size)(TPMBackend *t);
>>
>> TpmTypeOptions *(*get_tpm_options)(TPMBackend *t);
>>
>> @@ -183,6 +184,16 @@ int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty);
>> TPMVersion tpm_backend_get_tpm_version(TPMBackend *s);
>>
>> /**
>> + * tpm_backend_get_buffer_size:
>> + * @s: the backend to call into
>> + *
>> + * Get the TPM's buffer size.
>> + *
>> + * Returns buffer size.
>> + */
>> +uint32_t tpm_backend_get_buffer_size(TPMBackend *s);
>> +
>> +/**
>> * tpm_backend_query_tpm:
>> * @s: the backend
>> *
>> --
>> 2.5.5
>>
>>
> Looks good to me otherwise,
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/5] tpm: pull tpm_util_send() out of tpm_util_test()
2017-11-07 0:58 [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Stefan Berger
2017-11-07 0:58 ` [Qemu-devel] [PATCH 1/5] tpm: Move getting TPM buffer size to backends Stefan Berger
@ 2017-11-07 0:58 ` Stefan Berger
2017-11-08 16:22 ` Marc-André Lureau
2017-11-07 0:58 ` [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device Stefan Berger
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Stefan Berger @ 2017-11-07 0:58 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: amarnath.valluri, Stefan Berger
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
hw/tpm/tpm_util.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index daf1faa..396e793 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -53,10 +53,11 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
* A basic test of a TPM device. We expect a well formatted response header
* (error response is fine) within one second.
*/
-static int tpm_util_test(int fd,
- unsigned char *request,
- size_t requestlen,
- uint16_t *return_tag)
+static int tpm_util_tx(int fd,
+ unsigned char *request,
+ size_t requestlen,
+ unsigned char *response,
+ size_t responselen)
{
struct tpm_resp_hdr *resp;
fd_set readfds;
@@ -65,7 +66,6 @@ static int tpm_util_test(int fd,
.tv_sec = 1,
.tv_usec = 0,
};
- unsigned char buf[1024];
n = write(fd, request, requestlen);
if (n < 0) {
@@ -84,17 +84,36 @@ static int tpm_util_test(int fd,
return -errno;
}
- n = read(fd, &buf, sizeof(buf));
+ n = read(fd, response, responselen);
if (n < sizeof(struct tpm_resp_hdr)) {
return -EFAULT;
}
- resp = (struct tpm_resp_hdr *)buf;
+ resp = (struct tpm_resp_hdr *)response;
/* check the header */
if (be32_to_cpu(resp->len) != n) {
return -EMSGSIZE;
}
+ return 0;
+}
+
+static int tpm_util_test(int fd,
+ unsigned char *request,
+ size_t requestlen,
+ uint16_t *return_tag)
+{
+ struct tpm_resp_hdr *resp;
+ unsigned char buf[1024];
+ ssize_t ret;
+
+ ret = tpm_util_tx(fd, request, requestlen,
+ buf, sizeof(buf));
+ if (ret < 0) {
+ return ret;
+ }
+
+ resp = (struct tpm_resp_hdr *)buf;
*return_tag = be16_to_cpu(resp->tag);
return 0;
--
2.5.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 2/5] tpm: pull tpm_util_send() out of tpm_util_test()
2017-11-07 0:58 ` [Qemu-devel] [PATCH 2/5] tpm: pull tpm_util_send() out of tpm_util_test() Stefan Berger
@ 2017-11-08 16:22 ` Marc-André Lureau
0 siblings, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-08 16:22 UTC (permalink / raw)
To: Stefan Berger; +Cc: qemu-devel, amarnath.valluri
Hi Stefan,
On Mon, Nov 06, 2017 at 07:58:53PM -0500, Stefan Berger wrote:
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> hw/tpm/tpm_util.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index daf1faa..396e793 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -53,10 +53,11 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
> * A basic test of a TPM device. We expect a well formatted response header
> * (error response is fine) within one second.
> */
> -static int tpm_util_test(int fd,
> - unsigned char *request,
> - size_t requestlen,
> - uint16_t *return_tag)
> +static int tpm_util_tx(int fd,
The function is not just tx, but write & read. Call it
tpm_util_request() instead?
> + unsigned char *request,
> + size_t requestlen,
> + unsigned char *response,
> + size_t responselen)
> {
> struct tpm_resp_hdr *resp;
> fd_set readfds;
> @@ -65,7 +66,6 @@ static int tpm_util_test(int fd,
> .tv_sec = 1,
> .tv_usec = 0,
> };
> - unsigned char buf[1024];
>
> n = write(fd, request, requestlen);
> if (n < 0) {
> @@ -84,17 +84,36 @@ static int tpm_util_test(int fd,
> return -errno;
> }
>
> - n = read(fd, &buf, sizeof(buf));
> + n = read(fd, response, responselen);
> if (n < sizeof(struct tpm_resp_hdr)) {
> return -EFAULT;
> }
>
> - resp = (struct tpm_resp_hdr *)buf;
> + resp = (struct tpm_resp_hdr *)response;
> /* check the header */
> if (be32_to_cpu(resp->len) != n) {
> return -EMSGSIZE;
> }
>
> + return 0;
> +}
> +
> +static int tpm_util_test(int fd,
> + unsigned char *request,
> + size_t requestlen,
> + uint16_t *return_tag)
> +{
> + struct tpm_resp_hdr *resp;
> + unsigned char buf[1024];
> + ssize_t ret;
> +
> + ret = tpm_util_tx(fd, request, requestlen,
> + buf, sizeof(buf));
> + if (ret < 0) {
> + return ret;
> + }
> +
> + resp = (struct tpm_resp_hdr *)buf;
> *return_tag = be16_to_cpu(resp->tag);
>
> return 0;
> --
> 2.5.5
>
>
Looks fine otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device
2017-11-07 0:58 [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Stefan Berger
2017-11-07 0:58 ` [Qemu-devel] [PATCH 1/5] tpm: Move getting TPM buffer size to backends Stefan Berger
2017-11-07 0:58 ` [Qemu-devel] [PATCH 2/5] tpm: pull tpm_util_send() out of tpm_util_test() Stefan Berger
@ 2017-11-07 0:58 ` Stefan Berger
2017-11-07 12:28 ` Stefan Berger
2017-11-08 16:18 ` Marc-André Lureau
2017-11-07 0:58 ` [Qemu-devel] [PATCH 4/5] tpm: tpm_emulator: get and set buffer size of device Stefan Berger
` (2 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Stefan Berger @ 2017-11-07 0:58 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: amarnath.valluri, Stefan Berger
Rather than hard coding the buffer size in the tpm_passthrough
backend read the TPM I/O buffer size from the host device.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
hw/tpm/tpm_int.h | 9 ++++
hw/tpm/tpm_passthrough.c | 11 ++++-
hw/tpm/tpm_util.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++
hw/tpm/tpm_util.h | 3 ++
4 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index 1df5883..f838535 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -45,11 +45,20 @@ struct tpm_resp_hdr {
#define TPM_ORD_ContinueSelfTest 0x53
#define TPM_ORD_GetTicks 0xf1
+#define TPM_ORD_GetCapability 0x65
+#define TPM_CAP_PROPERTY 0x05
+
+#define TPM_CAP_PROP_INPUT_BUFFER 0x124
/* TPM2 defines */
#define TPM2_ST_NO_SESSIONS 0x8001
#define TPM2_CC_ReadClock 0x00000181
+#define TPM2_CC_GetCapability 0x0000017a
+
+#define TPM2_CAP_TPM_PROPERTIES 0x6
+
+#define TPM2_PT_INPUT_BUFFER 0x10d
#endif /* TPM_TPM_INT_H */
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 7ff9249..ec755fe 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -57,6 +57,7 @@ struct TPMPassthruState {
int cancel_fd;
TPMVersion tpm_version;
+ uint32_t tpm_buffersize;
};
typedef struct TPMPassthruState TPMPassthruState;
@@ -201,7 +202,15 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
static uint32_t tpm_passthrough_get_buffer_size(TPMBackend *tb)
{
- return 4096;
+ TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+ int ret;
+
+ ret = tpm_util_get_buffer_size(tpm_pt->tpm_fd, tpm_pt->tpm_version,
+ &tpm_pt->tpm_buffersize);
+ if (ret < 0) {
+ tpm_pt->tpm_buffersize = 4096;
+ }
+ return tpm_pt->tpm_buffersize;
}
/*
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index 396e793..3c861ab 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -20,10 +20,19 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "tpm_util.h"
#include "tpm_int.h"
#include "exec/memory.h"
+#define DEBUG_TPM 0
+
+#define DPRINTF(fmt, ...) do { \
+ if (DEBUG_TPM) { \
+ fprintf(stderr, "tpm-util:"fmt"\n", ## __VA_ARGS__); \
+ } \
+} while (0)
+
/*
* Write an error message in the given output buffer.
*/
@@ -170,3 +179,110 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
return 1;
}
+
+int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
+ uint32_t *buffersize)
+{
+ unsigned char buf[1024];
+ const struct tpm_req_get_buffer_size {
+ struct tpm_req_hdr hdr;
+ uint32_t capability;
+ uint32_t len;
+ uint32_t subcap;
+ } QEMU_PACKED tpm_get_buffer_size = {
+ .hdr = {
+ .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
+ .len = cpu_to_be32(sizeof(tpm_get_buffer_size)),
+ .ordinal = cpu_to_be32(TPM_ORD_GetCapability),
+ },
+ .capability = cpu_to_be32(TPM_CAP_PROPERTY),
+ .len = cpu_to_be32(sizeof(uint32_t)),
+ .subcap = cpu_to_be32(TPM_CAP_PROP_INPUT_BUFFER),
+ };
+ const struct tpm2_req_get_buffer_size {
+ struct tpm_req_hdr hdr;
+ uint32_t capability;
+ uint32_t property;
+ uint32_t count;
+ } QEMU_PACKED tpm2_get_buffer_size = {
+ .hdr = {
+ .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+ .len = cpu_to_be32(sizeof(tpm2_get_buffer_size)),
+ .ordinal = cpu_to_be32(TPM2_CC_GetCapability),
+ },
+ .capability = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES),
+ .property = cpu_to_be32(TPM2_PT_INPUT_BUFFER),
+ .count = cpu_to_be32(1),
+ };
+ struct tpm_resp_get_buffer_size {
+ struct tpm_resp_hdr hdr;
+ uint32_t len;
+ uint32_t buffersize;
+ } QEMU_PACKED *tpm_resp = (struct tpm_resp_get_buffer_size *)buf;
+ struct tpm2_resp_get_buffer_size {
+ struct tpm_resp_hdr hdr;
+ uint8_t more;
+ uint32_t capability;
+ uint32_t count;
+ uint32_t property;
+ uint32_t value;
+ } QEMU_PACKED *tpm2_resp = (struct tpm2_resp_get_buffer_size *)buf;
+ unsigned char *request = NULL;
+ size_t requestlen;
+ int ret;
+
+ switch (tpm_version) {
+ case TPM_VERSION_1_2:
+ request = (unsigned char *)&tpm_get_buffer_size;
+ requestlen = sizeof(tpm_get_buffer_size);
+ break;
+ case TPM_VERSION_2_0:
+ request = (unsigned char *)&tpm2_get_buffer_size;
+ requestlen = sizeof(tpm2_get_buffer_size);
+ break;
+ case TPM_VERSION_UNSPEC:
+ return -EFAULT;
+ }
+
+ ret = tpm_util_tx(tpm_fd, request, requestlen, buf, sizeof(buf));
+ if (ret < 0) {
+ return ret;
+ }
+
+ switch (tpm_version) {
+ case TPM_VERSION_1_2:
+ if (be32_to_cpu(tpm_resp->hdr.len) != sizeof(*tpm_resp) ||
+ be32_to_cpu(tpm_resp->len) != sizeof(uint32_t)) {
+ DPRINTF("tpm_resp->hdr.len = %u, expected = %zu\n",
+ be32_to_cpu(tpm_resp->hdr.len), sizeof(*tpm_resp));
+ DPRINTF("tpm_resp->len = %u, expected = %zu\n",
+ be32_to_cpu(tpm_resp->len), sizeof(uint32_t));
+ error_report("tpm_util: Got malformed response to "
+ "TPM_GetCapability; errcode: 0x%x",
+ be32_to_cpu(tpm_resp->hdr.errcode));
+ return -EFAULT;
+ }
+ *buffersize = be32_to_cpu(tpm_resp->buffersize);
+ break;
+ case TPM_VERSION_2_0:
+ if (be32_to_cpu(tpm2_resp->hdr.len) != sizeof(*tpm2_resp) ||
+ be32_to_cpu(tpm2_resp->count) != 1) {
+ DPRINTF("tpm2_resp->hdr.len = %u, expected = %zu\n",
+ be32_to_cpu(tpm2_resp->hdr.len), sizeof(*tpm2_resp));
+ DPRINTF("tpm2_resp->len = %u, expected = %u\n",
+ be32_to_cpu(tpm2_resp->count), 1);
+ error_report("tpm_util: Got malformed response to "
+ "TPM2_GetCapability; errcode: 0x%x",
+ be32_to_cpu(tpm2_resp->hdr.errcode));
+ return -EFAULT;
+ }
+ *buffersize = be32_to_cpu(tpm2_resp->value);
+ break;
+ case TPM_VERSION_UNSPEC:
+ break;
+ }
+
+ DPRINTF("buffersize of device: %u\n", *buffersize);
+
+ return 0;
+}
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index aca10c9..e4fba32 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -36,4 +36,7 @@ static inline uint32_t tpm_cmd_get_size(const void *b)
return be32_to_cpu(*(const uint32_t *)(b + 2));
}
+int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
+ uint32_t *buffersize);
+
#endif /* TPM_TPM_UTIL_H */
--
2.5.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device
2017-11-07 0:58 ` [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device Stefan Berger
@ 2017-11-07 12:28 ` Stefan Berger
2017-11-08 16:18 ` Marc-André Lureau
1 sibling, 0 replies; 16+ messages in thread
From: Stefan Berger @ 2017-11-07 12:28 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: amarnath.valluri
On 11/06/2017 07:58 PM, Stefan Berger wrote:
> Rather than hard coding the buffer size in the tpm_passthrough
> backend read the TPM I/O buffer size from the host device.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> hw/tpm/tpm_int.h | 9 ++++
> hw/tpm/tpm_passthrough.c | 11 ++++-
> hw/tpm/tpm_util.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++
> hw/tpm/tpm_util.h | 3 ++
> 4 files changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> index 1df5883..f838535 100644
> --- a/hw/tpm/tpm_int.h
> +++ b/hw/tpm/tpm_int.h
> @@ -45,11 +45,20 @@ struct tpm_resp_hdr {
>
> #define TPM_ORD_ContinueSelfTest 0x53
> #define TPM_ORD_GetTicks 0xf1
> +#define TPM_ORD_GetCapability 0x65
>
> +#define TPM_CAP_PROPERTY 0x05
> +
> +#define TPM_CAP_PROP_INPUT_BUFFER 0x124
>
> /* TPM2 defines */
> #define TPM2_ST_NO_SESSIONS 0x8001
>
> #define TPM2_CC_ReadClock 0x00000181
> +#define TPM2_CC_GetCapability 0x0000017a
> +
> +#define TPM2_CAP_TPM_PROPERTIES 0x6
> +
> +#define TPM2_PT_INPUT_BUFFER 0x10d
>
> #endif /* TPM_TPM_INT_H */
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 7ff9249..ec755fe 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -57,6 +57,7 @@ struct TPMPassthruState {
> int cancel_fd;
>
> TPMVersion tpm_version;
> + uint32_t tpm_buffersize;
> };
>
> typedef struct TPMPassthruState TPMPassthruState;
> @@ -201,7 +202,15 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
>
> static uint32_t tpm_passthrough_get_buffer_size(TPMBackend *tb)
> {
> - return 4096;
> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> + int ret;
> +
> + ret = tpm_util_get_buffer_size(tpm_pt->tpm_fd, tpm_pt->tpm_version,
> + &tpm_pt->tpm_buffersize);
> + if (ret < 0) {
> + tpm_pt->tpm_buffersize = 4096;
> + }
> + return tpm_pt->tpm_buffersize;
> }
>
> /*
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 396e793..3c861ab 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -20,10 +20,19 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> #include "tpm_util.h"
> #include "tpm_int.h"
> #include "exec/memory.h"
>
> +#define DEBUG_TPM 0
> +
> +#define DPRINTF(fmt, ...) do { \
> + if (DEBUG_TPM) { \
> + fprintf(stderr, "tpm-util:"fmt"\n", ## __VA_ARGS__); \
> + } \
> +} while (0)
> +
> /*
> * Write an error message in the given output buffer.
> */
> @@ -170,3 +179,110 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
>
> return 1;
> }
> +
> +int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> + uint32_t *buffersize)
> +{
> + unsigned char buf[1024];
> + const struct tpm_req_get_buffer_size {
> + struct tpm_req_hdr hdr;
> + uint32_t capability;
> + uint32_t len;
> + uint32_t subcap;
> + } QEMU_PACKED tpm_get_buffer_size = {
> + .hdr = {
> + .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
> + .len = cpu_to_be32(sizeof(tpm_get_buffer_size)),
> + .ordinal = cpu_to_be32(TPM_ORD_GetCapability),
> + },
> + .capability = cpu_to_be32(TPM_CAP_PROPERTY),
> + .len = cpu_to_be32(sizeof(uint32_t)),
> + .subcap = cpu_to_be32(TPM_CAP_PROP_INPUT_BUFFER),
> + };
> + const struct tpm2_req_get_buffer_size {
> + struct tpm_req_hdr hdr;
> + uint32_t capability;
> + uint32_t property;
> + uint32_t count;
> + } QEMU_PACKED tpm2_get_buffer_size = {
> + .hdr = {
> + .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> + .len = cpu_to_be32(sizeof(tpm2_get_buffer_size)),
> + .ordinal = cpu_to_be32(TPM2_CC_GetCapability),
> + },
> + .capability = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES),
> + .property = cpu_to_be32(TPM2_PT_INPUT_BUFFER),
I have to fix this to TPM_PT_MAX_COMMAND_SIZE (0x11e) and
TPM_PT_MAX_RESPONSE_SIZE (0x11f) and take the maximum of the two.
Stefan
> + .count = cpu_to_be32(1),
> + };
> + struct tpm_resp_get_buffer_size {
> + struct tpm_resp_hdr hdr;
> + uint32_t len;
> + uint32_t buffersize;
> + } QEMU_PACKED *tpm_resp = (struct tpm_resp_get_buffer_size *)buf;
> + struct tpm2_resp_get_buffer_size {
> + struct tpm_resp_hdr hdr;
> + uint8_t more;
> + uint32_t capability;
> + uint32_t count;
> + uint32_t property;
> + uint32_t value;
> + } QEMU_PACKED *tpm2_resp = (struct tpm2_resp_get_buffer_size *)buf;
> + unsigned char *request = NULL;
> + size_t requestlen;
> + int ret;
> +
> + switch (tpm_version) {
> + case TPM_VERSION_1_2:
> + request = (unsigned char *)&tpm_get_buffer_size;
> + requestlen = sizeof(tpm_get_buffer_size);
> + break;
> + case TPM_VERSION_2_0:
> + request = (unsigned char *)&tpm2_get_buffer_size;
> + requestlen = sizeof(tpm2_get_buffer_size);
> + break;
> + case TPM_VERSION_UNSPEC:
> + return -EFAULT;
> + }
> +
> + ret = tpm_util_tx(tpm_fd, request, requestlen, buf, sizeof(buf));
> + if (ret < 0) {
> + return ret;
> + }
> +
> + switch (tpm_version) {
> + case TPM_VERSION_1_2:
> + if (be32_to_cpu(tpm_resp->hdr.len) != sizeof(*tpm_resp) ||
> + be32_to_cpu(tpm_resp->len) != sizeof(uint32_t)) {
> + DPRINTF("tpm_resp->hdr.len = %u, expected = %zu\n",
> + be32_to_cpu(tpm_resp->hdr.len), sizeof(*tpm_resp));
> + DPRINTF("tpm_resp->len = %u, expected = %zu\n",
> + be32_to_cpu(tpm_resp->len), sizeof(uint32_t));
> + error_report("tpm_util: Got malformed response to "
> + "TPM_GetCapability; errcode: 0x%x",
> + be32_to_cpu(tpm_resp->hdr.errcode));
> + return -EFAULT;
> + }
> + *buffersize = be32_to_cpu(tpm_resp->buffersize);
> + break;
> + case TPM_VERSION_2_0:
> + if (be32_to_cpu(tpm2_resp->hdr.len) != sizeof(*tpm2_resp) ||
> + be32_to_cpu(tpm2_resp->count) != 1) {
> + DPRINTF("tpm2_resp->hdr.len = %u, expected = %zu\n",
> + be32_to_cpu(tpm2_resp->hdr.len), sizeof(*tpm2_resp));
> + DPRINTF("tpm2_resp->len = %u, expected = %u\n",
> + be32_to_cpu(tpm2_resp->count), 1);
> + error_report("tpm_util: Got malformed response to "
> + "TPM2_GetCapability; errcode: 0x%x",
> + be32_to_cpu(tpm2_resp->hdr.errcode));
> + return -EFAULT;
> + }
> + *buffersize = be32_to_cpu(tpm2_resp->value);
> + break;
> + case TPM_VERSION_UNSPEC:
> + break;
> + }
> +
> + DPRINTF("buffersize of device: %u\n", *buffersize);
> +
> + return 0;
> +}
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index aca10c9..e4fba32 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -36,4 +36,7 @@ static inline uint32_t tpm_cmd_get_size(const void *b)
> return be32_to_cpu(*(const uint32_t *)(b + 2));
> }
>
> +int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> + uint32_t *buffersize);
> +
> #endif /* TPM_TPM_UTIL_H */
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device
2017-11-07 0:58 ` [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device Stefan Berger
2017-11-07 12:28 ` Stefan Berger
@ 2017-11-08 16:18 ` Marc-André Lureau
1 sibling, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-08 16:18 UTC (permalink / raw)
To: Stefan Berger; +Cc: qemu-devel, marcandre.lureau, amarnath.valluri
Hi Stefan,
On Mon, Nov 06, 2017 at 07:58:54PM -0500, Stefan Berger wrote:
> Rather than hard coding the buffer size in the tpm_passthrough
> backend read the TPM I/O buffer size from the host device.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> hw/tpm/tpm_int.h | 9 ++++
> hw/tpm/tpm_passthrough.c | 11 ++++-
> hw/tpm/tpm_util.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++
> hw/tpm/tpm_util.h | 3 ++
> 4 files changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> index 1df5883..f838535 100644
> --- a/hw/tpm/tpm_int.h
> +++ b/hw/tpm/tpm_int.h
> @@ -45,11 +45,20 @@ struct tpm_resp_hdr {
>
> #define TPM_ORD_ContinueSelfTest 0x53
> #define TPM_ORD_GetTicks 0xf1
> +#define TPM_ORD_GetCapability 0x65
>
> +#define TPM_CAP_PROPERTY 0x05
> +
> +#define TPM_CAP_PROP_INPUT_BUFFER 0x124
>
> /* TPM2 defines */
> #define TPM2_ST_NO_SESSIONS 0x8001
>
> #define TPM2_CC_ReadClock 0x00000181
> +#define TPM2_CC_GetCapability 0x0000017a
> +
> +#define TPM2_CAP_TPM_PROPERTIES 0x6
> +
> +#define TPM2_PT_INPUT_BUFFER 0x10d
>
> #endif /* TPM_TPM_INT_H */
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 7ff9249..ec755fe 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -57,6 +57,7 @@ struct TPMPassthruState {
> int cancel_fd;
>
> TPMVersion tpm_version;
> + uint32_t tpm_buffersize;
> };
>
> typedef struct TPMPassthruState TPMPassthruState;
> @@ -201,7 +202,15 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
>
> static uint32_t tpm_passthrough_get_buffer_size(TPMBackend *tb)
> {
> - return 4096;
> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> + int ret;
> +
> + ret = tpm_util_get_buffer_size(tpm_pt->tpm_fd, tpm_pt->tpm_version,
> + &tpm_pt->tpm_buffersize);
> + if (ret < 0) {
> + tpm_pt->tpm_buffersize = 4096;
> + }
> + return tpm_pt->tpm_buffersize;
> }
>
> /*
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 396e793..3c861ab 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -20,10 +20,19 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> #include "tpm_util.h"
> #include "tpm_int.h"
> #include "exec/memory.h"
>
> +#define DEBUG_TPM 0
> +
> +#define DPRINTF(fmt, ...) do { \
> + if (DEBUG_TPM) { \
> + fprintf(stderr, "tpm-util:"fmt"\n", ## __VA_ARGS__); \
> + } \
> +} while (0)
> +
> /*
> * Write an error message in the given output buffer.
> */
> @@ -170,3 +179,110 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
>
> return 1;
> }
> +
> +int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> + uint32_t *buffersize)
> +{
> + unsigned char buf[1024];
> + const struct tpm_req_get_buffer_size {
> + struct tpm_req_hdr hdr;
> + uint32_t capability;
> + uint32_t len;
> + uint32_t subcap;
> + } QEMU_PACKED tpm_get_buffer_size = {
> + .hdr = {
> + .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
> + .len = cpu_to_be32(sizeof(tpm_get_buffer_size)),
> + .ordinal = cpu_to_be32(TPM_ORD_GetCapability),
> + },
> + .capability = cpu_to_be32(TPM_CAP_PROPERTY),
> + .len = cpu_to_be32(sizeof(uint32_t)),
> + .subcap = cpu_to_be32(TPM_CAP_PROP_INPUT_BUFFER),
> + };
> + const struct tpm2_req_get_buffer_size {
> + struct tpm_req_hdr hdr;
> + uint32_t capability;
> + uint32_t property;
> + uint32_t count;
> + } QEMU_PACKED tpm2_get_buffer_size = {
> + .hdr = {
> + .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> + .len = cpu_to_be32(sizeof(tpm2_get_buffer_size)),
> + .ordinal = cpu_to_be32(TPM2_CC_GetCapability),
> + },
> + .capability = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES),
> + .property = cpu_to_be32(TPM2_PT_INPUT_BUFFER),
> + .count = cpu_to_be32(1),
> + };
> + struct tpm_resp_get_buffer_size {
> + struct tpm_resp_hdr hdr;
> + uint32_t len;
> + uint32_t buffersize;
> + } QEMU_PACKED *tpm_resp = (struct tpm_resp_get_buffer_size *)buf;
> + struct tpm2_resp_get_buffer_size {
> + struct tpm_resp_hdr hdr;
> + uint8_t more;
> + uint32_t capability;
> + uint32_t count;
> + uint32_t property;
> + uint32_t value;
> + } QEMU_PACKED *tpm2_resp = (struct tpm2_resp_get_buffer_size *)buf;
> + unsigned char *request = NULL;
> + size_t requestlen;
> + int ret;
> +
> + switch (tpm_version) {
It may be more readable to split the function in 2 halfs, v1 & v2 versions.
> + case TPM_VERSION_1_2:
> + request = (unsigned char *)&tpm_get_buffer_size;
> + requestlen = sizeof(tpm_get_buffer_size);
> + break;
> + case TPM_VERSION_2_0:
> + request = (unsigned char *)&tpm2_get_buffer_size;
> + requestlen = sizeof(tpm2_get_buffer_size);
> + break;
> + case TPM_VERSION_UNSPEC:
> + return -EFAULT;
> + }
> +
> + ret = tpm_util_tx(tpm_fd, request, requestlen, buf, sizeof(buf));
> + if (ret < 0) {
> + return ret;
> + }
> +
> + switch (tpm_version) {
> + case TPM_VERSION_1_2:
> + if (be32_to_cpu(tpm_resp->hdr.len) != sizeof(*tpm_resp) ||
> + be32_to_cpu(tpm_resp->len) != sizeof(uint32_t)) {
> + DPRINTF("tpm_resp->hdr.len = %u, expected = %zu\n",
> + be32_to_cpu(tpm_resp->hdr.len), sizeof(*tpm_resp));
> + DPRINTF("tpm_resp->len = %u, expected = %zu\n",
> + be32_to_cpu(tpm_resp->len), sizeof(uint32_t));
> + error_report("tpm_util: Got malformed response to "
> + "TPM_GetCapability; errcode: 0x%x",
> + be32_to_cpu(tpm_resp->hdr.errcode));
> + return -EFAULT;
> + }
> + *buffersize = be32_to_cpu(tpm_resp->buffersize);
> + break;
> + case TPM_VERSION_2_0:
> + if (be32_to_cpu(tpm2_resp->hdr.len) != sizeof(*tpm2_resp) ||
> + be32_to_cpu(tpm2_resp->count) != 1) {
> + DPRINTF("tpm2_resp->hdr.len = %u, expected = %zu\n",
> + be32_to_cpu(tpm2_resp->hdr.len), sizeof(*tpm2_resp));
> + DPRINTF("tpm2_resp->len = %u, expected = %u\n",
> + be32_to_cpu(tpm2_resp->count), 1);
> + error_report("tpm_util: Got malformed response to "
> + "TPM2_GetCapability; errcode: 0x%x",
> + be32_to_cpu(tpm2_resp->hdr.errcode));
> + return -EFAULT;
> + }
> + *buffersize = be32_to_cpu(tpm2_resp->value);
> + break;
> + case TPM_VERSION_UNSPEC:
> + break;
> + }
> +
> + DPRINTF("buffersize of device: %u\n", *buffersize);
> +
> + return 0;
> +}
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index aca10c9..e4fba32 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -36,4 +36,7 @@ static inline uint32_t tpm_cmd_get_size(const void *b)
> return be32_to_cpu(*(const uint32_t *)(b + 2));
> }
>
> +int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> + uint32_t *buffersize);
> +
> #endif /* TPM_TPM_UTIL_H */
> --
> 2.5.5
>
>
Since you pointed out some issues with this patch, I'll review the
next iteration.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 4/5] tpm: tpm_emulator: get and set buffer size of device
2017-11-07 0:58 [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Stefan Berger
` (2 preceding siblings ...)
2017-11-07 0:58 ` [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device Stefan Berger
@ 2017-11-07 0:58 ` Stefan Berger
2017-11-08 16:22 ` Marc-André Lureau
2017-11-07 0:58 ` [Qemu-devel] [PATCH 5/5] tpm: tpm_passthrough: Fail startup if FE buffer size < BE buffer size Stefan Berger
2017-11-08 16:20 ` [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Marc-André Lureau
5 siblings, 1 reply; 16+ messages in thread
From: Stefan Berger @ 2017-11-07 0:58 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: amarnath.valluri, Stefan Berger
Convert the tpm_emulator backend to get the current buffer size
of the external device and set it to the buffer size that the
frontend (TIS) requests.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
backends/tpm.c | 4 +--
hw/tpm/tpm_emulator.c | 79 +++++++++++++++++++++++++++++++++++++++++---
hw/tpm/tpm_ioctl.h | 28 +++++++++++++++-
hw/tpm/tpm_tis.c | 6 ++--
include/sysemu/tpm_backend.h | 6 ++--
5 files changed, 111 insertions(+), 12 deletions(-)
diff --git a/backends/tpm.c b/backends/tpm.c
index e7d0f9a..f024c27 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -68,7 +68,7 @@ int tpm_backend_init(TPMBackend *s, TPMIf *tpmif, Error **errp)
return 0;
}
-int tpm_backend_startup_tpm(TPMBackend *s)
+int tpm_backend_startup_tpm(TPMBackend *s, uint32_t buffersize)
{
int res = 0;
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
@@ -79,7 +79,7 @@ int tpm_backend_startup_tpm(TPMBackend *s)
s->thread_pool = g_thread_pool_new(tpm_backend_worker_thread, s, 1, TRUE,
NULL);
- res = k->startup_tpm ? k->startup_tpm(s) : 0;
+ res = k->startup_tpm ? k->startup_tpm(s, buffersize) : 0;
s->had_startup_error = (res != 0);
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 5a6107e..a16de7a 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -232,13 +232,14 @@ static int tpm_emulator_check_caps(TPMEmulator *tpm_emu)
switch (tpm_emu->tpm_version) {
case TPM_VERSION_1_2:
caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
- PTM_CAP_SET_LOCALITY | PTM_CAP_SET_DATAFD;
+ PTM_CAP_SET_LOCALITY | PTM_CAP_SET_DATAFD | PTM_CAP_STOP |
+ PTM_CAP_SET_BUFFERSIZE;
tpm = "1.2";
break;
case TPM_VERSION_2_0:
caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
PTM_CAP_SET_LOCALITY | PTM_CAP_RESET_TPMESTABLISHED |
- PTM_CAP_SET_DATAFD;
+ PTM_CAP_SET_DATAFD | PTM_CAP_STOP | PTM_CAP_SET_BUFFERSIZE;
tpm = "2";
break;
case TPM_VERSION_UNSPEC:
@@ -255,12 +256,76 @@ static int tpm_emulator_check_caps(TPMEmulator *tpm_emu)
return 0;
}
-static int tpm_emulator_startup_tpm(TPMBackend *tb)
+static int tpm_emulator_stop_tpm(TPMBackend *tb)
+{
+ TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
+ ptm_res res;
+
+ if (tpm_emulator_ctrlcmd(tpm_emu, CMD_STOP, &res, 0, sizeof(res)) < 0) {
+ error_report("tpm-emulator: Could not stop TPM: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ res = be32_to_cpu(res);
+ if (res) {
+ error_report("tpm-emulator: TPM result for CMD_STOP: 0x%x", res);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int tpm_emulator_set_buffer_size(TPMBackend *tb,
+ uint32_t wanted_size,
+ uint32_t *actual_size)
+{
+ TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
+ ptm_setbuffersize psbs;
+
+ if (tpm_emulator_stop_tpm(tb) < 0) {
+ return -1;
+ }
+
+ psbs.u.req.buffersize = cpu_to_be32(wanted_size);
+
+ if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_BUFFERSIZE, &psbs,
+ sizeof(psbs.u.req), sizeof(psbs.u.resp)) < 0) {
+ error_report("tpm-emulator: Could not set buffer size: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ psbs.u.resp.tpm_result = be32_to_cpu(psbs.u.resp.tpm_result);
+ if (psbs.u.resp.tpm_result != 0) {
+ error_report("tpm-emulator: TPM result for set buffer size : 0x%x",
+ psbs.u.resp.tpm_result);
+ return -1;
+ }
+
+ if (actual_size) {
+ *actual_size = be32_to_cpu(psbs.u.resp.buffersize);
+ }
+
+ DPRINTF("buffer size: %u, min: %u, max: %u\n",
+ be32_to_cpu(psbs.u.resp.buffersize),
+ be32_to_cpu(psbs.u.resp.minsize),
+ be32_to_cpu(psbs.u.resp.maxsize));
+
+ return 0;
+}
+
+static int tpm_emulator_startup_tpm(TPMBackend *tb, uint32_t buffersize)
{
TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
ptm_init init;
ptm_res res;
+ if (buffersize != 0 &&
+ tpm_emulator_set_buffer_size(tb, buffersize, NULL) < 0) {
+ goto err_exit;
+ }
+
DPRINTF("%s", __func__);
if (tpm_emulator_ctrlcmd(tpm_emu, CMD_INIT, &init, sizeof(init),
sizeof(init)) < 0) {
@@ -358,7 +423,13 @@ static TPMVersion tpm_emulator_get_tpm_version(TPMBackend *tb)
static uint32_t tpm_emulator_get_buffer_size(TPMBackend *tb)
{
- return 4096;
+ uint32_t actual_size;
+
+ if (tpm_emulator_set_buffer_size(tb, 0, &actual_size) < 0) {
+ return 4096;
+ }
+
+ return actual_size;
}
static int tpm_emulator_block_migration(TPMEmulator *tpm_emu)
diff --git a/hw/tpm/tpm_ioctl.h b/hw/tpm/tpm_ioctl.h
index 33564b1..54c8d34 100644
--- a/hw/tpm/tpm_ioctl.h
+++ b/hw/tpm/tpm_ioctl.h
@@ -169,6 +169,28 @@ struct ptm_getconfig {
#define PTM_CONFIG_FLAG_FILE_KEY 0x1
#define PTM_CONFIG_FLAG_MIGRATION_KEY 0x2
+/*
+ * PTM_SET_BUFFERSIZE: Set the buffer size to be used by the TPM.
+ * A 0 on input queries for the current buffer size. Any other
+ * number will try to set the buffer size. The returned number is
+ * the buffer size that will be used, which can be larger than the
+ * requested one, if it was below the minimum, or smaller than the
+ * requested one, if it was above the maximum.
+ */
+struct ptm_setbuffersize {
+ union {
+ struct {
+ uint32_t buffersize; /* 0 to query for current buffer size */
+ } req; /* request */
+ struct {
+ ptm_res tpm_result;
+ uint32_t buffersize; /* buffer size in use */
+ uint32_t minsize; /* min. supported buffer size */
+ uint32_t maxsize; /* max. supported buffer size */
+ } resp; /* response */
+ } u;
+};
+
typedef uint64_t ptm_cap;
typedef struct ptm_est ptm_est;
@@ -179,6 +201,7 @@ typedef struct ptm_init ptm_init;
typedef struct ptm_getstate ptm_getstate;
typedef struct ptm_setstate ptm_setstate;
typedef struct ptm_getconfig ptm_getconfig;
+typedef struct ptm_setbuffersize ptm_setbuffersize;
/* capability flags returned by PTM_GET_CAPABILITY */
#define PTM_CAP_INIT (1)
@@ -194,6 +217,7 @@ typedef struct ptm_getconfig ptm_getconfig;
#define PTM_CAP_STOP (1 << 10)
#define PTM_CAP_GET_CONFIG (1 << 11)
#define PTM_CAP_SET_DATAFD (1 << 12)
+#define PTM_CAP_SET_BUFFERSIZE (1 << 13)
enum {
PTM_GET_CAPABILITY = _IOR('P', 0, ptm_cap),
@@ -212,6 +236,7 @@ enum {
PTM_STOP = _IOR('P', 13, ptm_res),
PTM_GET_CONFIG = _IOR('P', 14, ptm_getconfig),
PTM_SET_DATAFD = _IOR('P', 15, ptm_res),
+ PTM_SET_BUFFERSIZE = _IOWR('P', 16, ptm_setbuffersize),
};
/*
@@ -240,7 +265,8 @@ enum {
CMD_SET_STATEBLOB,
CMD_STOP,
CMD_GET_CONFIG,
- CMD_SET_DATAFD
+ CMD_SET_DATAFD,
+ CMD_SET_BUFFERSIZE,
};
#endif /* _TPM_IOCTL_H */
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index a3df40f..8d7310e 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -974,9 +974,9 @@ static const MemoryRegionOps tpm_tis_memory_ops = {
},
};
-static int tpm_tis_do_startup_tpm(TPMState *s)
+static int tpm_tis_do_startup_tpm(TPMState *s, uint32_t buffersize)
{
- return tpm_backend_startup_tpm(s->be_driver);
+ return tpm_backend_startup_tpm(s->be_driver, buffersize);
}
static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb,
@@ -1040,7 +1040,7 @@ static void tpm_tis_reset(DeviceState *dev)
tpm_tis_realloc_buffer(&s->loc[c].r_buffer, s->be_buffer_size);
}
- tpm_tis_do_startup_tpm(s);
+ tpm_tis_do_startup_tpm(s, s->be_buffer_size);
}
static const VMStateDescription vmstate_tpm_tis = {
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index d23cef2..3978d98 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -66,7 +66,7 @@ struct TPMBackendClass {
TPMBackend *(*create)(QemuOpts *opts);
/* start up the TPM on the backend - optional */
- int (*startup_tpm)(TPMBackend *t);
+ int (*startup_tpm)(TPMBackend *t, uint32_t buffersize);
/* optional */
void (*reset)(TPMBackend *t);
@@ -111,10 +111,12 @@ int tpm_backend_init(TPMBackend *s, TPMIf *tpmif, Error **errp);
/**
* tpm_backend_startup_tpm:
* @s: the backend whose TPM support is to be started
+ * @buffersize: the buffer size the TPM is supposed to use,
+ * 0 to leave it as-is
*
* Returns 0 on success.
*/
-int tpm_backend_startup_tpm(TPMBackend *s);
+int tpm_backend_startup_tpm(TPMBackend *s, uint32_t buffersize);
/**
* tpm_backend_had_startup_error:
--
2.5.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 4/5] tpm: tpm_emulator: get and set buffer size of device
2017-11-07 0:58 ` [Qemu-devel] [PATCH 4/5] tpm: tpm_emulator: get and set buffer size of device Stefan Berger
@ 2017-11-08 16:22 ` Marc-André Lureau
2017-11-08 17:50 ` Stefan Berger
0 siblings, 1 reply; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-08 16:22 UTC (permalink / raw)
To: Stefan Berger; +Cc: qemu-devel, marcandre.lureau, amarnath.valluri
On Mon, Nov 06, 2017 at 07:58:55PM -0500, Stefan Berger wrote:
> Convert the tpm_emulator backend to get the current buffer size
> of the external device and set it to the buffer size that the
> frontend (TIS) requests.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> backends/tpm.c | 4 +--
> hw/tpm/tpm_emulator.c | 79 +++++++++++++++++++++++++++++++++++++++++---
> hw/tpm/tpm_ioctl.h | 28 +++++++++++++++-
> hw/tpm/tpm_tis.c | 6 ++--
> include/sysemu/tpm_backend.h | 6 ++--
> 5 files changed, 111 insertions(+), 12 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index e7d0f9a..f024c27 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -68,7 +68,7 @@ int tpm_backend_init(TPMBackend *s, TPMIf *tpmif, Error **errp)
> return 0;
> }
>
> -int tpm_backend_startup_tpm(TPMBackend *s)
> +int tpm_backend_startup_tpm(TPMBackend *s, uint32_t buffersize)
> {
> int res = 0;
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> @@ -79,7 +79,7 @@ int tpm_backend_startup_tpm(TPMBackend *s)
> s->thread_pool = g_thread_pool_new(tpm_backend_worker_thread, s, 1, TRUE,
> NULL);
>
> - res = k->startup_tpm ? k->startup_tpm(s) : 0;
> + res = k->startup_tpm ? k->startup_tpm(s, buffersize) : 0;
>
> s->had_startup_error = (res != 0);
>
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 5a6107e..a16de7a 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -232,13 +232,14 @@ static int tpm_emulator_check_caps(TPMEmulator *tpm_emu)
> switch (tpm_emu->tpm_version) {
> case TPM_VERSION_1_2:
> caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
> - PTM_CAP_SET_LOCALITY | PTM_CAP_SET_DATAFD;
> + PTM_CAP_SET_LOCALITY | PTM_CAP_SET_DATAFD | PTM_CAP_STOP |
> + PTM_CAP_SET_BUFFERSIZE;
> tpm = "1.2";
> break;
> case TPM_VERSION_2_0:
> caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
> PTM_CAP_SET_LOCALITY | PTM_CAP_RESET_TPMESTABLISHED |
> - PTM_CAP_SET_DATAFD;
> + PTM_CAP_SET_DATAFD | PTM_CAP_STOP | PTM_CAP_SET_BUFFERSIZE;
> tpm = "2";
> break;
> case TPM_VERSION_UNSPEC:
> @@ -255,12 +256,76 @@ static int tpm_emulator_check_caps(TPMEmulator *tpm_emu)
> return 0;
> }
>
> -static int tpm_emulator_startup_tpm(TPMBackend *tb)
> +static int tpm_emulator_stop_tpm(TPMBackend *tb)
> +{
> + TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> + ptm_res res;
> +
> + if (tpm_emulator_ctrlcmd(tpm_emu, CMD_STOP, &res, 0, sizeof(res)) < 0) {
> + error_report("tpm-emulator: Could not stop TPM: %s",
> + strerror(errno));
> + return -1;
> + }
> +
> + res = be32_to_cpu(res);
> + if (res) {
> + error_report("tpm-emulator: TPM result for CMD_STOP: 0x%x", res);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int tpm_emulator_set_buffer_size(TPMBackend *tb,
> + uint32_t wanted_size,
> + uint32_t *actual_size)
> +{
> + TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> + ptm_setbuffersize psbs;
> +
> + if (tpm_emulator_stop_tpm(tb) < 0) {
> + return -1;
> + }
> +
> + psbs.u.req.buffersize = cpu_to_be32(wanted_size);
> +
> + if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_BUFFERSIZE, &psbs,
> + sizeof(psbs.u.req), sizeof(psbs.u.resp)) < 0) {
> + error_report("tpm-emulator: Could not set buffer size: %s",
> + strerror(errno));
> + return -1;
> + }
> +
> + psbs.u.resp.tpm_result = be32_to_cpu(psbs.u.resp.tpm_result);
> + if (psbs.u.resp.tpm_result != 0) {
> + error_report("tpm-emulator: TPM result for set buffer size : 0x%x",
> + psbs.u.resp.tpm_result);
> + return -1;
> + }
> +
> + if (actual_size) {
> + *actual_size = be32_to_cpu(psbs.u.resp.buffersize);
> + }
> +
> + DPRINTF("buffer size: %u, min: %u, max: %u\n",
> + be32_to_cpu(psbs.u.resp.buffersize),
> + be32_to_cpu(psbs.u.resp.minsize),
> + be32_to_cpu(psbs.u.resp.maxsize));
> +
> + return 0;
> +}
> +
> +static int tpm_emulator_startup_tpm(TPMBackend *tb, uint32_t buffersize)
> {
> TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> ptm_init init;
> ptm_res res;
>
> + if (buffersize != 0 &&
> + tpm_emulator_set_buffer_size(tb, buffersize, NULL) < 0) {
> + goto err_exit;
> + }
> +
> DPRINTF("%s", __func__);
> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_INIT, &init, sizeof(init),
> sizeof(init)) < 0) {
> @@ -358,7 +423,13 @@ static TPMVersion tpm_emulator_get_tpm_version(TPMBackend *tb)
>
> static uint32_t tpm_emulator_get_buffer_size(TPMBackend *tb)
> {
> - return 4096;
> + uint32_t actual_size;
> +
> + if (tpm_emulator_set_buffer_size(tb, 0, &actual_size) < 0) {
> + return 4096;
> + }
> +
> + return actual_size;
> }
>
> static int tpm_emulator_block_migration(TPMEmulator *tpm_emu)
> diff --git a/hw/tpm/tpm_ioctl.h b/hw/tpm/tpm_ioctl.h
> index 33564b1..54c8d34 100644
> --- a/hw/tpm/tpm_ioctl.h
> +++ b/hw/tpm/tpm_ioctl.h
> @@ -169,6 +169,28 @@ struct ptm_getconfig {
> #define PTM_CONFIG_FLAG_FILE_KEY 0x1
> #define PTM_CONFIG_FLAG_MIGRATION_KEY 0x2
>
> +/*
> + * PTM_SET_BUFFERSIZE: Set the buffer size to be used by the TPM.
> + * A 0 on input queries for the current buffer size. Any other
> + * number will try to set the buffer size. The returned number is
> + * the buffer size that will be used, which can be larger than the
> + * requested one, if it was below the minimum, or smaller than the
> + * requested one, if it was above the maximum.
> + */
> +struct ptm_setbuffersize {
> + union {
> + struct {
> + uint32_t buffersize; /* 0 to query for current buffer size */
> + } req; /* request */
> + struct {
> + ptm_res tpm_result;
> + uint32_t buffersize; /* buffer size in use */
> + uint32_t minsize; /* min. supported buffer size */
> + uint32_t maxsize; /* max. supported buffer size */
> + } resp; /* response */
> + } u;
> +};
> +
>
> typedef uint64_t ptm_cap;
> typedef struct ptm_est ptm_est;
> @@ -179,6 +201,7 @@ typedef struct ptm_init ptm_init;
> typedef struct ptm_getstate ptm_getstate;
> typedef struct ptm_setstate ptm_setstate;
> typedef struct ptm_getconfig ptm_getconfig;
> +typedef struct ptm_setbuffersize ptm_setbuffersize;
>
> /* capability flags returned by PTM_GET_CAPABILITY */
> #define PTM_CAP_INIT (1)
> @@ -194,6 +217,7 @@ typedef struct ptm_getconfig ptm_getconfig;
> #define PTM_CAP_STOP (1 << 10)
> #define PTM_CAP_GET_CONFIG (1 << 11)
> #define PTM_CAP_SET_DATAFD (1 << 12)
> +#define PTM_CAP_SET_BUFFERSIZE (1 << 13)
>
> enum {
> PTM_GET_CAPABILITY = _IOR('P', 0, ptm_cap),
> @@ -212,6 +236,7 @@ enum {
> PTM_STOP = _IOR('P', 13, ptm_res),
> PTM_GET_CONFIG = _IOR('P', 14, ptm_getconfig),
> PTM_SET_DATAFD = _IOR('P', 15, ptm_res),
> + PTM_SET_BUFFERSIZE = _IOWR('P', 16, ptm_setbuffersize),
> };
>
> /*
> @@ -240,7 +265,8 @@ enum {
> CMD_SET_STATEBLOB,
> CMD_STOP,
> CMD_GET_CONFIG,
> - CMD_SET_DATAFD
> + CMD_SET_DATAFD,
> + CMD_SET_BUFFERSIZE,
> };
>
> #endif /* _TPM_IOCTL_H */
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index a3df40f..8d7310e 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -974,9 +974,9 @@ static const MemoryRegionOps tpm_tis_memory_ops = {
> },
> };
>
> -static int tpm_tis_do_startup_tpm(TPMState *s)
> +static int tpm_tis_do_startup_tpm(TPMState *s, uint32_t buffersize)
> {
> - return tpm_backend_startup_tpm(s->be_driver);
> + return tpm_backend_startup_tpm(s->be_driver, buffersize);
> }
>
> static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb,
> @@ -1040,7 +1040,7 @@ static void tpm_tis_reset(DeviceState *dev)
> tpm_tis_realloc_buffer(&s->loc[c].r_buffer, s->be_buffer_size);
> }
>
> - tpm_tis_do_startup_tpm(s);
> + tpm_tis_do_startup_tpm(s, s->be_buffer_size);
It is a bit convoluted: get the buffer size from the backend to give
it back to startup, perhaps use 0/default value to make it more
explicit it doesn't intend to change it.
> }
>
> static const VMStateDescription vmstate_tpm_tis = {
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index d23cef2..3978d98 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -66,7 +66,7 @@ struct TPMBackendClass {
> TPMBackend *(*create)(QemuOpts *opts);
>
> /* start up the TPM on the backend - optional */
> - int (*startup_tpm)(TPMBackend *t);
> + int (*startup_tpm)(TPMBackend *t, uint32_t buffersize);
>
> /* optional */
> void (*reset)(TPMBackend *t);
> @@ -111,10 +111,12 @@ int tpm_backend_init(TPMBackend *s, TPMIf *tpmif, Error **errp);
> /**
> * tpm_backend_startup_tpm:
> * @s: the backend whose TPM support is to be started
> + * @buffersize: the buffer size the TPM is supposed to use,
> + * 0 to leave it as-is
> *
> * Returns 0 on success.
> */
> -int tpm_backend_startup_tpm(TPMBackend *s);
> +int tpm_backend_startup_tpm(TPMBackend *s, uint32_t buffersize);
>
> /**
> * tpm_backend_had_startup_error:
> --
> 2.5.5
>
>
looks ok overall,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 4/5] tpm: tpm_emulator: get and set buffer size of device
2017-11-08 16:22 ` Marc-André Lureau
@ 2017-11-08 17:50 ` Stefan Berger
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Berger @ 2017-11-08 17:50 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, amarnath.valluri
On 11/08/2017 11:22 AM, Marc-André Lureau wrote:
> On Mon, Nov 06, 2017 at 07:58:55PM -0500, Stefan Berger wrote:
>> Convert the tpm_emulator backend to get the current buffer size
>> of the external device and set it to the buffer size that the
>> frontend (TIS) requests.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>> backends/tpm.c | 4 +--
>> hw/tpm/tpm_emulator.c | 79 +++++++++++++++++++++++++++++++++++++++++---
>> hw/tpm/tpm_ioctl.h | 28 +++++++++++++++-
>> hw/tpm/tpm_tis.c | 6 ++--
>> include/sysemu/tpm_backend.h | 6 ++--
>> 5 files changed, 111 insertions(+), 12 deletions(-)
>>
>> diff --git a/backends/tpm.c b/backends/tpm.c
>> index e7d0f9a..f024c27 100644
>> --- a/backends/tpm.c
>> +++ b/backends/tpm.c
>> @@ -68,7 +68,7 @@ int tpm_backend_init(TPMBackend *s, TPMIf *tpmif, Error **errp)
>> return 0;
>> }
>>
>> -int tpm_backend_startup_tpm(TPMBackend *s)
>> +int tpm_backend_startup_tpm(TPMBackend *s, uint32_t buffersize)
>> {
>> int res = 0;
>> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>> @@ -79,7 +79,7 @@ int tpm_backend_startup_tpm(TPMBackend *s)
>> s->thread_pool = g_thread_pool_new(tpm_backend_worker_thread, s, 1, TRUE,
>> NULL);
>>
>> - res = k->startup_tpm ? k->startup_tpm(s) : 0;
>> + res = k->startup_tpm ? k->startup_tpm(s, buffersize) : 0;
>>
>> s->had_startup_error = (res != 0);
>>
>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>> index 5a6107e..a16de7a 100644
>> --- a/hw/tpm/tpm_emulator.c
>> +++ b/hw/tpm/tpm_emulator.c
>> @@ -232,13 +232,14 @@ static int tpm_emulator_check_caps(TPMEmulator *tpm_emu)
>> switch (tpm_emu->tpm_version) {
>> case TPM_VERSION_1_2:
>> caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
>> - PTM_CAP_SET_LOCALITY | PTM_CAP_SET_DATAFD;
>> + PTM_CAP_SET_LOCALITY | PTM_CAP_SET_DATAFD | PTM_CAP_STOP |
>> + PTM_CAP_SET_BUFFERSIZE;
>> tpm = "1.2";
>> break;
>> case TPM_VERSION_2_0:
>> caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
>> PTM_CAP_SET_LOCALITY | PTM_CAP_RESET_TPMESTABLISHED |
>> - PTM_CAP_SET_DATAFD;
>> + PTM_CAP_SET_DATAFD | PTM_CAP_STOP | PTM_CAP_SET_BUFFERSIZE;
>> tpm = "2";
>> break;
>> case TPM_VERSION_UNSPEC:
>> @@ -255,12 +256,76 @@ static int tpm_emulator_check_caps(TPMEmulator *tpm_emu)
>> return 0;
>> }
>>
>> -static int tpm_emulator_startup_tpm(TPMBackend *tb)
>> +static int tpm_emulator_stop_tpm(TPMBackend *tb)
>> +{
>> + TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
>> + ptm_res res;
>> +
>> + if (tpm_emulator_ctrlcmd(tpm_emu, CMD_STOP, &res, 0, sizeof(res)) < 0) {
>> + error_report("tpm-emulator: Could not stop TPM: %s",
>> + strerror(errno));
>> + return -1;
>> + }
>> +
>> + res = be32_to_cpu(res);
>> + if (res) {
>> + error_report("tpm-emulator: TPM result for CMD_STOP: 0x%x", res);
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int tpm_emulator_set_buffer_size(TPMBackend *tb,
>> + uint32_t wanted_size,
>> + uint32_t *actual_size)
>> +{
>> + TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
>> + ptm_setbuffersize psbs;
>> +
>> + if (tpm_emulator_stop_tpm(tb) < 0) {
>> + return -1;
>> + }
>> +
>> + psbs.u.req.buffersize = cpu_to_be32(wanted_size);
>> +
>> + if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_BUFFERSIZE, &psbs,
>> + sizeof(psbs.u.req), sizeof(psbs.u.resp)) < 0) {
>> + error_report("tpm-emulator: Could not set buffer size: %s",
>> + strerror(errno));
>> + return -1;
>> + }
>> +
>> + psbs.u.resp.tpm_result = be32_to_cpu(psbs.u.resp.tpm_result);
>> + if (psbs.u.resp.tpm_result != 0) {
>> + error_report("tpm-emulator: TPM result for set buffer size : 0x%x",
>> + psbs.u.resp.tpm_result);
>> + return -1;
>> + }
>> +
>> + if (actual_size) {
>> + *actual_size = be32_to_cpu(psbs.u.resp.buffersize);
>> + }
>> +
>> + DPRINTF("buffer size: %u, min: %u, max: %u\n",
>> + be32_to_cpu(psbs.u.resp.buffersize),
>> + be32_to_cpu(psbs.u.resp.minsize),
>> + be32_to_cpu(psbs.u.resp.maxsize));
>> +
>> + return 0;
>> +}
>> +
>> +static int tpm_emulator_startup_tpm(TPMBackend *tb, uint32_t buffersize)
>> {
>> TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
>> ptm_init init;
>> ptm_res res;
>>
>> + if (buffersize != 0 &&
>> + tpm_emulator_set_buffer_size(tb, buffersize, NULL) < 0) {
>> + goto err_exit;
>> + }
>> +
>> DPRINTF("%s", __func__);
>> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_INIT, &init, sizeof(init),
>> sizeof(init)) < 0) {
>> @@ -358,7 +423,13 @@ static TPMVersion tpm_emulator_get_tpm_version(TPMBackend *tb)
>>
>> static uint32_t tpm_emulator_get_buffer_size(TPMBackend *tb)
>> {
>> - return 4096;
>> + uint32_t actual_size;
>> +
>> + if (tpm_emulator_set_buffer_size(tb, 0, &actual_size) < 0) {
>> + return 4096;
>> + }
>> +
>> + return actual_size;
>> }
>>
>> static int tpm_emulator_block_migration(TPMEmulator *tpm_emu)
>> diff --git a/hw/tpm/tpm_ioctl.h b/hw/tpm/tpm_ioctl.h
>> index 33564b1..54c8d34 100644
>> --- a/hw/tpm/tpm_ioctl.h
>> +++ b/hw/tpm/tpm_ioctl.h
>> @@ -169,6 +169,28 @@ struct ptm_getconfig {
>> #define PTM_CONFIG_FLAG_FILE_KEY 0x1
>> #define PTM_CONFIG_FLAG_MIGRATION_KEY 0x2
>>
>> +/*
>> + * PTM_SET_BUFFERSIZE: Set the buffer size to be used by the TPM.
>> + * A 0 on input queries for the current buffer size. Any other
>> + * number will try to set the buffer size. The returned number is
>> + * the buffer size that will be used, which can be larger than the
>> + * requested one, if it was below the minimum, or smaller than the
>> + * requested one, if it was above the maximum.
>> + */
>> +struct ptm_setbuffersize {
>> + union {
>> + struct {
>> + uint32_t buffersize; /* 0 to query for current buffer size */
>> + } req; /* request */
>> + struct {
>> + ptm_res tpm_result;
>> + uint32_t buffersize; /* buffer size in use */
>> + uint32_t minsize; /* min. supported buffer size */
>> + uint32_t maxsize; /* max. supported buffer size */
>> + } resp; /* response */
>> + } u;
>> +};
>> +
>>
>> typedef uint64_t ptm_cap;
>> typedef struct ptm_est ptm_est;
>> @@ -179,6 +201,7 @@ typedef struct ptm_init ptm_init;
>> typedef struct ptm_getstate ptm_getstate;
>> typedef struct ptm_setstate ptm_setstate;
>> typedef struct ptm_getconfig ptm_getconfig;
>> +typedef struct ptm_setbuffersize ptm_setbuffersize;
>>
>> /* capability flags returned by PTM_GET_CAPABILITY */
>> #define PTM_CAP_INIT (1)
>> @@ -194,6 +217,7 @@ typedef struct ptm_getconfig ptm_getconfig;
>> #define PTM_CAP_STOP (1 << 10)
>> #define PTM_CAP_GET_CONFIG (1 << 11)
>> #define PTM_CAP_SET_DATAFD (1 << 12)
>> +#define PTM_CAP_SET_BUFFERSIZE (1 << 13)
>>
>> enum {
>> PTM_GET_CAPABILITY = _IOR('P', 0, ptm_cap),
>> @@ -212,6 +236,7 @@ enum {
>> PTM_STOP = _IOR('P', 13, ptm_res),
>> PTM_GET_CONFIG = _IOR('P', 14, ptm_getconfig),
>> PTM_SET_DATAFD = _IOR('P', 15, ptm_res),
>> + PTM_SET_BUFFERSIZE = _IOWR('P', 16, ptm_setbuffersize),
>> };
>>
>> /*
>> @@ -240,7 +265,8 @@ enum {
>> CMD_SET_STATEBLOB,
>> CMD_STOP,
>> CMD_GET_CONFIG,
>> - CMD_SET_DATAFD
>> + CMD_SET_DATAFD,
>> + CMD_SET_BUFFERSIZE,
>> };
>>
>> #endif /* _TPM_IOCTL_H */
>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>> index a3df40f..8d7310e 100644
>> --- a/hw/tpm/tpm_tis.c
>> +++ b/hw/tpm/tpm_tis.c
>> @@ -974,9 +974,9 @@ static const MemoryRegionOps tpm_tis_memory_ops = {
>> },
>> };
>>
>> -static int tpm_tis_do_startup_tpm(TPMState *s)
>> +static int tpm_tis_do_startup_tpm(TPMState *s, uint32_t buffersize)
>> {
>> - return tpm_backend_startup_tpm(s->be_driver);
>> + return tpm_backend_startup_tpm(s->be_driver, buffersize);
>> }
>>
>> static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb,
>> @@ -1040,7 +1040,7 @@ static void tpm_tis_reset(DeviceState *dev)
>> tpm_tis_realloc_buffer(&s->loc[c].r_buffer, s->be_buffer_size);
>> }
>>
>> - tpm_tis_do_startup_tpm(s);
>> + tpm_tis_do_startup_tpm(s, s->be_buffer_size);
> It is a bit convoluted: get the buffer size from the backend to give
> it back to startup, perhaps use 0/default value to make it more
> explicit it doesn't intend to change it.
Ok, passing '0' now, meaning that backend doesn't need to change.
Stefan
>
>> }
>>
>> static const VMStateDescription vmstate_tpm_tis = {
>> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
>> index d23cef2..3978d98 100644
>> --- a/include/sysemu/tpm_backend.h
>> +++ b/include/sysemu/tpm_backend.h
>> @@ -66,7 +66,7 @@ struct TPMBackendClass {
>> TPMBackend *(*create)(QemuOpts *opts);
>>
>> /* start up the TPM on the backend - optional */
>> - int (*startup_tpm)(TPMBackend *t);
>> + int (*startup_tpm)(TPMBackend *t, uint32_t buffersize);
>>
>> /* optional */
>> void (*reset)(TPMBackend *t);
>> @@ -111,10 +111,12 @@ int tpm_backend_init(TPMBackend *s, TPMIf *tpmif, Error **errp);
>> /**
>> * tpm_backend_startup_tpm:
>> * @s: the backend whose TPM support is to be started
>> + * @buffersize: the buffer size the TPM is supposed to use,
>> + * 0 to leave it as-is
>> *
>> * Returns 0 on success.
>> */
>> -int tpm_backend_startup_tpm(TPMBackend *s);
>> +int tpm_backend_startup_tpm(TPMBackend *s, uint32_t buffersize);
>>
>> /**
>> * tpm_backend_had_startup_error:
>> --
>> 2.5.5
>>
>>
> looks ok overall,
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/5] tpm: tpm_passthrough: Fail startup if FE buffer size < BE buffer size
2017-11-07 0:58 [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Stefan Berger
` (3 preceding siblings ...)
2017-11-07 0:58 ` [Qemu-devel] [PATCH 4/5] tpm: tpm_emulator: get and set buffer size of device Stefan Berger
@ 2017-11-07 0:58 ` Stefan Berger
2017-11-08 16:22 ` Marc-André Lureau
2017-11-08 16:20 ` [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Marc-André Lureau
5 siblings, 1 reply; 16+ messages in thread
From: Stefan Berger @ 2017-11-07 0:58 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: amarnath.valluri, Stefan Berger
If the requested buffer size of the frontend is smaller than the fixed
buffer size of the host's TPM, fail the startup_tpm() interface function,
which will make the device unusable. We fail it because the backend TPM
could produce larger packets than what the frontend could pass to the OS.
The current combination of TIS frontend and either passthrough or emulator
backend will not lead to this case since the TIS can support any size of
buffer.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
hw/tpm/tpm_passthrough.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index ec755fe..66d5098 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -304,6 +304,20 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts)
return TPM_BACKEND(obj);
}
+static int tpm_passthrough_startup_tpm(TPMBackend *tb, uint32_t buffersize)
+{
+ TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+
+ if (buffersize && buffersize < tpm_pt->tpm_buffersize) {
+ error_report("Requested buffer size of %u is smaller than host TPM's "
+ "fixed buffer size of %u",
+ buffersize, tpm_pt->tpm_buffersize);
+ return -1;
+ }
+
+ return 0;
+}
+
static TpmTypeOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
{
TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
@@ -362,6 +376,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
tbc->opts = tpm_passthrough_cmdline_opts;
tbc->desc = "Passthrough TPM backend driver";
tbc->create = tpm_passthrough_create;
+ tbc->startup_tpm = tpm_passthrough_startup_tpm;
tbc->reset = tpm_passthrough_reset;
tbc->cancel_cmd = tpm_passthrough_cancel_cmd;
tbc->get_tpm_established_flag = tpm_passthrough_get_tpm_established_flag;
--
2.5.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 5/5] tpm: tpm_passthrough: Fail startup if FE buffer size < BE buffer size
2017-11-07 0:58 ` [Qemu-devel] [PATCH 5/5] tpm: tpm_passthrough: Fail startup if FE buffer size < BE buffer size Stefan Berger
@ 2017-11-08 16:22 ` Marc-André Lureau
2017-11-08 18:20 ` Stefan Berger
0 siblings, 1 reply; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-08 16:22 UTC (permalink / raw)
To: Stefan Berger; +Cc: qemu-devel, marcandre.lureau, amarnath.valluri
Hi Stefan,
On Mon, Nov 06, 2017 at 07:58:56PM -0500, Stefan Berger wrote:
> If the requested buffer size of the frontend is smaller than the fixed
> buffer size of the host's TPM, fail the startup_tpm() interface function,
> which will make the device unusable. We fail it because the backend TPM
> could produce larger packets than what the frontend could pass to the OS.
>
> The current combination of TIS frontend and either passthrough or emulator
> backend will not lead to this case since the TIS can support any size of
> buffer.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> hw/tpm/tpm_passthrough.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index ec755fe..66d5098 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -304,6 +304,20 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts)
> return TPM_BACKEND(obj);
> }
>
> +static int tpm_passthrough_startup_tpm(TPMBackend *tb, uint32_t buffersize)
> +{
> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> +
> + if (buffersize && buffersize < tpm_pt->tpm_buffersize) {
> + error_report("Requested buffer size of %u is smaller than host TPM's "
> + "fixed buffer size of %u",
> + buffersize, tpm_pt->tpm_buffersize);
Looks ok
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static TpmTypeOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
> {
> TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
> @@ -362,6 +376,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
> tbc->opts = tpm_passthrough_cmdline_opts;
> tbc->desc = "Passthrough TPM backend driver";
> tbc->create = tpm_passthrough_create;
> + tbc->startup_tpm = tpm_passthrough_startup_tpm;
> tbc->reset = tpm_passthrough_reset;
> tbc->cancel_cmd = tpm_passthrough_cancel_cmd;
> tbc->get_tpm_established_flag = tpm_passthrough_get_tpm_established_flag;
> --
> 2.5.5
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH 5/5] tpm: tpm_passthrough: Fail startup if FE buffer size < BE buffer size
2017-11-08 16:22 ` Marc-André Lureau
@ 2017-11-08 18:20 ` Stefan Berger
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Berger @ 2017-11-08 18:20 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, amarnath.valluri
On 11/08/2017 11:22 AM, Marc-André Lureau wrote:
> Hi Stefan,
>
> On Mon, Nov 06, 2017 at 07:58:56PM -0500, Stefan Berger wrote:
>> If the requested buffer size of the frontend is smaller than the fixed
>> buffer size of the host's TPM, fail the startup_tpm() interface function,
>> which will make the device unusable. We fail it because the backend TPM
>> could produce larger packets than what the frontend could pass to the OS.
>>
>> The current combination of TIS frontend and either passthrough or emulator
>> backend will not lead to this case since the TIS can support any size of
>> buffer.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>> hw/tpm/tpm_passthrough.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>> index ec755fe..66d5098 100644
>> --- a/hw/tpm/tpm_passthrough.c
>> +++ b/hw/tpm/tpm_passthrough.c
>> @@ -304,6 +304,20 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts)
>> return TPM_BACKEND(obj);
>> }
>>
>> +static int tpm_passthrough_startup_tpm(TPMBackend *tb, uint32_t buffersize)
>> +{
>> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>> +
>> + if (buffersize && buffersize < tpm_pt->tpm_buffersize) {
>> + error_report("Requested buffer size of %u is smaller than host TPM's "
>> + "fixed buffer size of %u",
>> + buffersize, tpm_pt->tpm_buffersize);
> Looks ok
>
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static TpmTypeOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
>> {
>> TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
>> @@ -362,6 +376,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
>> tbc->opts = tpm_passthrough_cmdline_opts;
>> tbc->desc = "Passthrough TPM backend driver";
>> tbc->create = tpm_passthrough_create;
>> + tbc->startup_tpm = tpm_passthrough_startup_tpm;
>> tbc->reset = tpm_passthrough_reset;
>> tbc->cancel_cmd = tpm_passthrough_cancel_cmd;
>> tbc->get_tpm_established_flag = tpm_passthrough_get_tpm_established_flag;
>> --
>> 2.5.5
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
Thanks for the review. I'll post v2 shortly.
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11)
2017-11-07 0:58 [Qemu-devel] [PATCH 0/5] tpm: Match frontend and backend buffer sizes (not 2.11) Stefan Berger
` (4 preceding siblings ...)
2017-11-07 0:58 ` [Qemu-devel] [PATCH 5/5] tpm: tpm_passthrough: Fail startup if FE buffer size < BE buffer size Stefan Berger
@ 2017-11-08 16:20 ` Marc-André Lureau
5 siblings, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-08 16:20 UTC (permalink / raw)
To: Stefan Berger; +Cc: qemu-devel, amarnath.valluri
On Mon, Nov 06, 2017 at 07:58:51PM -0500, Stefan Berger wrote:
> This patch series does away with the hard coded buffer size in the TIS
> frontend and instead retrieves the buffer size from the device that's
> being used. So it gets it from the host device or the external emulator.
> In case the frontend (CRB) cannot support the backend's current buffer size
> (typically 4k) it can adjust the buffer size the emulator is working with
> so that we will not run into the problem that the backend produces packets
> that the frontend cannot deliver to due mismatching buffer sizes.
>
The approach looks ok to me. I don't have enough TPM/backend knowledge
to say how relevant that is in the long run. Modifying the CRB device
to set the buffer size to qemu CRB_CTRL_CMD_SIZE works. Windows 10 seems
happy at least.
Just a few remarks, I suppose you'll send a v2 soon,
thanks
> Stefan
>
> Stefan Berger (5):
> tpm: Move getting TPM buffer size to backends
> tpm: pull tpm_util_send() out of tpm_util_test()
> tpm: tpm_passthrough: Read the buffer size from the host device
> tpm: tpm_emulator: get and set buffer size of device
> tpm: tpm_passthrough: Fail startup if FE buffer size < BE buffer size
>
> backends/tpm.c | 13 +++-
> hw/tpm/tpm_emulator.c | 83 +++++++++++++++++++++++-
> hw/tpm/tpm_int.h | 9 +++
> hw/tpm/tpm_ioctl.h | 28 +++++++-
> hw/tpm/tpm_passthrough.c | 30 +++++++++
> hw/tpm/tpm_tis.c | 18 +++---
> hw/tpm/tpm_util.c | 149 +++++++++++++++++++++++++++++++++++++++++--
> hw/tpm/tpm_util.h | 3 +
> include/sysemu/tpm_backend.h | 17 ++++-
> 9 files changed, 327 insertions(+), 23 deletions(-)
>
> --
> 2.5.5
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread