[Qemu-devel] [PATCH 0/2] tpm: A fix and a cleanup

[Qemu-devel] [PATCH 0/2] tpm: A fix and a cleanup

[Stefan Berger]

The following two patches fix the resetting of the CRB interface
and wrap calls to st{w,l}_be_p in tpm_cmd_set_XYZ functions.

   Stefan

Stefan Berger (2):
  tpm: Split off tpm_crb_reset function
  tpm: wrap stX_be_p in tpm_cmd_set_XYZ functions

 hw/tpm/tpm_crb.c  |  6 ++++++
 hw/tpm/tpm_util.c |  6 +++---
 hw/tpm/tpm_util.h | 15 +++++++++++++++
 3 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH 1/2] tpm: Split off tpm_crb_reset function

[Stefan Berger]

Split off the tpm_crb_reset function part from tpm_crb_realize
that we need to run every time the machine resets.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_crb.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 687d255..624e2e9 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -232,6 +232,11 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
         TPM_CRB_ADDR_BASE, &s->mmio);
     memory_region_add_subregion(get_system_memory(),
         TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
+}
+
+static void tpm_crb_reset(DeviceState *dev)
+{
+    CRBState *s = CRB(dev);
 
     tpm_backend_reset(s->tpmbe);
 
@@ -274,6 +279,7 @@ static void tpm_crb_class_init(ObjectClass *klass, void *data)
 
     dc->realize = tpm_crb_realize;
     dc->props = tpm_crb_properties;
+    dc->reset = tpm_crb_reset;
     dc->vmsd  = &vmstate_tpm_crb;
     dc->user_creatable = true;
     tc->model = TPM_MODEL_TPM_CRB;
-- 
2.5.5

[Qemu-devel] [PATCH 2/2] tpm: wrap stX_be_p in tpm_cmd_set_XYZ functions

[Stefan Berger]

Wrap the calls to stl_be_p and stw_be_p in tpm_cmd_set_XYZ functions
that are similar to existing getters.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_util.c |  6 +++---
 hw/tpm/tpm_util.h | 15 +++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index 8abde59..2de52a0 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -106,9 +106,9 @@ const PropertyInfo qdev_prop_tpm = {
 void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
 {
     if (out_len >= sizeof(struct tpm_resp_hdr)) {
-        stw_be_p(out, TPM_TAG_RSP_COMMAND);
-        stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
-        stl_be_p(out + 6, TPM_FAIL);
+        tpm_cmd_set_tag(out, TPM_TAG_RSP_COMMAND);
+        tpm_cmd_set_size(out, sizeof(struct tpm_resp_hdr));
+        tpm_cmd_set_error(out, TPM_FAIL);
     }
 }
 
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index f003d15..f397ac2 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -36,11 +36,21 @@ static inline uint16_t tpm_cmd_get_tag(const void *b)
     return lduw_be_p(b);
 }
 
+static inline void tpm_cmd_set_tag(void *b, uint16_t tag)
+{
+    stw_be_p(b, tag);
+}
+
 static inline uint32_t tpm_cmd_get_size(const void *b)
 {
     return ldl_be_p(b + 2);
 }
 
+static inline void tpm_cmd_set_size(void *b, uint32_t size)
+{
+    stl_be_p(b + 2, size);
+}
+
 static inline uint32_t tpm_cmd_get_ordinal(const void *b)
 {
     return ldl_be_p(b + 6);
@@ -51,6 +61,11 @@ static inline uint32_t tpm_cmd_get_errcode(const void *b)
     return ldl_be_p(b + 6);
 }
 
+static inline void tpm_cmd_set_error(void *b, uint32_t error)
+{
+    stl_be_p(b + 6, error);
+}
+
 int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
                              size_t *buffersize);
 
-- 
2.5.5

Re: [Qemu-devel] [PATCH 1/2] tpm: Split off tpm_crb_reset function

[Stefan Berger]

On 02/01/2018 06:23 PM, Stefan Berger wrote:
> Split off the tpm_crb_reset function part from tpm_crb_realize
> that we need to run every time the machine resets.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>   hw/tpm/tpm_crb.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 687d255..624e2e9 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -232,6 +232,11 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>           TPM_CRB_ADDR_BASE, &s->mmio);
>       memory_region_add_subregion(get_system_memory(),
>           TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
> +}
> +
> +static void tpm_crb_reset(DeviceState *dev)
> +{
> +    CRBState *s = CRB(dev);
>
>       tpm_backend_reset(s->tpmbe);
>
> @@ -274,6 +279,7 @@ static void tpm_crb_class_init(ObjectClass *klass, void *data)
>
>       dc->realize = tpm_crb_realize;
>       dc->props = tpm_crb_properties;
> +    dc->reset = tpm_crb_reset;
>       dc->vmsd  = &vmstate_tpm_crb;
>       dc->user_creatable = true;
>       tc->model = TPM_MODEL_TPM_CRB;


So this splt-off unfortunately causes the tpm-crb-test to fail since the 
reset function is not called. I wonder how to trigger that? Do we need 
to call tpm_crb_reset now from tpm_crb_realize just to make the tests 
work ? Or is there a function we can call in the test case to trigger 
the device reset ?

Re: [Qemu-devel] [PATCH 2/2] tpm: wrap stX_be_p in tpm_cmd_set_XYZ functions

[Marc-Andre Lureau]

Hi

On Fri, Feb 2, 2018 at 12:23 AM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Wrap the calls to stl_be_p and stw_be_p in tpm_cmd_set_XYZ functions
> that are similar to existing getters.

why not,

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  hw/tpm/tpm_util.c |  6 +++---
>  hw/tpm/tpm_util.h | 15 +++++++++++++++
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 8abde59..2de52a0 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -106,9 +106,9 @@ const PropertyInfo qdev_prop_tpm = {
>  void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
>  {
>      if (out_len >= sizeof(struct tpm_resp_hdr)) {
> -        stw_be_p(out, TPM_TAG_RSP_COMMAND);
> -        stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
> -        stl_be_p(out + 6, TPM_FAIL);
> +        tpm_cmd_set_tag(out, TPM_TAG_RSP_COMMAND);
> +        tpm_cmd_set_size(out, sizeof(struct tpm_resp_hdr));
> +        tpm_cmd_set_error(out, TPM_FAIL);
>      }
>  }
>
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index f003d15..f397ac2 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -36,11 +36,21 @@ static inline uint16_t tpm_cmd_get_tag(const void *b)
>      return lduw_be_p(b);
>  }
>
> +static inline void tpm_cmd_set_tag(void *b, uint16_t tag)
> +{
> +    stw_be_p(b, tag);
> +}
> +
>  static inline uint32_t tpm_cmd_get_size(const void *b)
>  {
>      return ldl_be_p(b + 2);
>  }
>
> +static inline void tpm_cmd_set_size(void *b, uint32_t size)
> +{
> +    stl_be_p(b + 2, size);
> +}
> +
>  static inline uint32_t tpm_cmd_get_ordinal(const void *b)
>  {
>      return ldl_be_p(b + 6);
> @@ -51,6 +61,11 @@ static inline uint32_t tpm_cmd_get_errcode(const void *b)
>      return ldl_be_p(b + 6);
>  }
>
> +static inline void tpm_cmd_set_error(void *b, uint32_t error)
> +{
> +    stl_be_p(b + 6, error);
> +}
> +
>  int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
>                               size_t *buffersize);
>
> --
> 2.5.5
>