diff mbox series

[v3,04/13] tpm_tis: move buffers from localities into common location

Message ID 1510323112-2207-5-git-send-email-stefanb@linux.vnet.ibm.com
State New
Headers show
Series tpm: Extend TPM with state migration support (not 2.11) | expand

Commit Message

Stefan Berger Nov. 10, 2017, 2:11 p.m. UTC
One read buffer and one write buffer is sufficient for all localities.
The localities cannot all be active at the same time, and only the active
locality can use the r/w buffers. Inactive localities will require the
COMMAND_READY flag to be set on the STS register to move to the READY
state, which then enables access to using the buffer for writing of a
command, while all other localities are inactive.

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

Comments

Marc-André Lureau Dec. 21, 2017, 2:11 p.m. UTC | #1
Hi

On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> One read buffer and one write buffer is sufficient for all localities.
> The localities cannot all be active at the same time, and only the active
> locality can use the r/w buffers. Inactive localities will require the
> COMMAND_READY flag to be set on the STS register to move to the READY
> state, which then enables access to using the buffer for writing of a
> command, while all other localities are inactive.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  hw/tpm/tpm_tis.c | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index ddfcfc9..f6f5f17 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -64,16 +64,14 @@ typedef struct TPMLocality {
>
>      uint16_t w_offset;
>      uint16_t r_offset;
> -    unsigned char w_buffer[TPM_TIS_BUFFER_MAX];
> -    unsigned char r_buffer[TPM_TIS_BUFFER_MAX];
>  } TPMLocality;
>
>  typedef struct TPMState {
>      ISADevice busdev;
>      MemoryRegion mmio;
>
> -    uint32_t offset;
> -    uint8_t buf[TPM_TIS_BUFFER_MAX];

Looks like those were never used. Make it a seperate cleanup?

> +    unsigned char w_buffer[TPM_TIS_BUFFER_MAX];
> +    unsigned char r_buffer[TPM_TIS_BUFFER_MAX];
>
>      uint8_t active_locty;
>      uint8_t aborting_locty;
> @@ -259,7 +257,7 @@ static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
>  {
>      TPMLocality *locty_data = &s->loc[locty];
>
> -    tpm_tis_show_buffer(s->loc[locty].w_buffer, s->be_buffer_size,
> +    tpm_tis_show_buffer(s->w_buffer, s->be_buffer_size,
>                          "tpm_tis: To TPM");
>
>      /*
> @@ -270,9 +268,9 @@ static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
>
>      s->cmd = (TPMBackendCmd) {
>          .locty = locty,
> -        .in = locty_data->w_buffer,
> +        .in = s->w_buffer,
>          .in_len = locty_data->w_offset,
> -        .out = locty_data->r_buffer,
> +        .out = s->r_buffer,
>          .out_len = s->be_buffer_size,
>      };
>
> @@ -424,7 +422,7 @@ static void tpm_tis_request_completed(TPMIf *ti)
>      s->loc[locty].r_offset = 0;
>      s->loc[locty].w_offset = 0;
>
> -    tpm_tis_show_buffer(s->loc[locty].r_buffer, s->be_buffer_size,
> +    tpm_tis_show_buffer(s->r_buffer, s->be_buffer_size,
>                          "tpm_tis: From TPM");
>
>      if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
> @@ -444,10 +442,10 @@ static uint32_t tpm_tis_data_read(TPMState *s, uint8_t locty)
>      uint16_t len;
>
>      if ((s->loc[locty].sts & TPM_TIS_STS_DATA_AVAILABLE)) {
> -        len = MIN(tpm_cmd_get_size(&s->loc[locty].r_buffer),
> +        len = MIN(tpm_cmd_get_size(&s->r_buffer),
>                    s->be_buffer_size);
>
> -        ret = s->loc[locty].r_buffer[s->loc[locty].r_offset++];
> +        ret = s->r_buffer[s->loc[locty].r_offset++];
>          if (s->loc[locty].r_offset >= len) {
>              /* got last byte */
>              tpm_tis_sts_set(&s->loc[locty], TPM_TIS_STS_VALID);
> @@ -493,12 +491,11 @@ static void tpm_tis_dump_state(void *opaque, hwaddr addr)
>              "tpm_tis: result buffer : ",
>              s->loc[locty].r_offset);
>      for (idx = 0;
> -         idx < MIN(tpm_cmd_get_size(&s->loc[locty].r_buffer),
> -                   s->be_buffer_size);
> +         idx < MIN(tpm_cmd_get_size(&s->r_buffer), s->be_buffer_size);
>           idx++) {
>          DPRINTF("%c%02x%s",
>                  s->loc[locty].r_offset == idx ? '>' : ' ',
> -                s->loc[locty].r_buffer[idx],
> +                s->r_buffer[idx],
>                  ((idx & 0xf) == 0xf) ? "\ntpm_tis:                 " : "");
>      }
>      DPRINTF("\n"
> @@ -506,12 +503,11 @@ static void tpm_tis_dump_state(void *opaque, hwaddr addr)
>              "tpm_tis: request buffer: ",
>              s->loc[locty].w_offset);
>      for (idx = 0;
> -         idx < MIN(tpm_cmd_get_size(s->loc[locty].w_buffer),
> -                   s->be_buffer_size);
> +         idx < MIN(tpm_cmd_get_size(s->w_buffer), s->be_buffer_size);
>           idx++) {
>          DPRINTF("%c%02x%s",
>                  s->loc[locty].w_offset == idx ? '>' : ' ',
> -                s->loc[locty].w_buffer[idx],
> +                s->w_buffer[idx],
>                  ((idx & 0xf) == 0xf) ? "\ntpm_tis:                 " : "");
>      }
>      DPRINTF("\n");
> @@ -573,7 +569,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
>          if (s->active_locty == locty) {
>              if ((s->loc[locty].sts & TPM_TIS_STS_DATA_AVAILABLE)) {
>                  val = TPM_TIS_BURST_COUNT(
> -                       MIN(tpm_cmd_get_size(&s->loc[locty].r_buffer),
> +                       MIN(tpm_cmd_get_size(&s->r_buffer),
>                             s->be_buffer_size)
>                         - s->loc[locty].r_offset) | s->loc[locty].sts;
>              } else {
> @@ -926,7 +922,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
>
>              while ((s->loc[locty].sts & TPM_TIS_STS_EXPECT) && size > 0) {
>                  if (s->loc[locty].w_offset < s->be_buffer_size) {
> -                    s->loc[locty].w_buffer[s->loc[locty].w_offset++] =
> +                    s->w_buffer[s->loc[locty].w_offset++] =
>                          (uint8_t)val;
>                      val >>= 8;
>                      size--;
> @@ -941,7 +937,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
>                  /* we have a packet length - see if we have all of it */
>                  bool need_irq = !(s->loc[locty].sts & TPM_TIS_STS_VALID);
>
> -                len = tpm_cmd_get_size(&s->loc[locty].w_buffer);
> +                len = tpm_cmd_get_size(&s->w_buffer);
>                  if (len > s->loc[locty].w_offset) {
>                      tpm_tis_sts_set(&s->loc[locty],
>                                      TPM_TIS_STS_EXPECT | TPM_TIS_STS_VALID);
> --
> 2.5.5
>
>


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

Patch

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index ddfcfc9..f6f5f17 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -64,16 +64,14 @@  typedef struct TPMLocality {
 
     uint16_t w_offset;
     uint16_t r_offset;
-    unsigned char w_buffer[TPM_TIS_BUFFER_MAX];
-    unsigned char r_buffer[TPM_TIS_BUFFER_MAX];
 } TPMLocality;
 
 typedef struct TPMState {
     ISADevice busdev;
     MemoryRegion mmio;
 
-    uint32_t offset;
-    uint8_t buf[TPM_TIS_BUFFER_MAX];
+    unsigned char w_buffer[TPM_TIS_BUFFER_MAX];
+    unsigned char r_buffer[TPM_TIS_BUFFER_MAX];
 
     uint8_t active_locty;
     uint8_t aborting_locty;
@@ -259,7 +257,7 @@  static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
 {
     TPMLocality *locty_data = &s->loc[locty];
 
-    tpm_tis_show_buffer(s->loc[locty].w_buffer, s->be_buffer_size,
+    tpm_tis_show_buffer(s->w_buffer, s->be_buffer_size,
                         "tpm_tis: To TPM");
 
     /*
@@ -270,9 +268,9 @@  static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
 
     s->cmd = (TPMBackendCmd) {
         .locty = locty,
-        .in = locty_data->w_buffer,
+        .in = s->w_buffer,
         .in_len = locty_data->w_offset,
-        .out = locty_data->r_buffer,
+        .out = s->r_buffer,
         .out_len = s->be_buffer_size,
     };
 
@@ -424,7 +422,7 @@  static void tpm_tis_request_completed(TPMIf *ti)
     s->loc[locty].r_offset = 0;
     s->loc[locty].w_offset = 0;
 
-    tpm_tis_show_buffer(s->loc[locty].r_buffer, s->be_buffer_size,
+    tpm_tis_show_buffer(s->r_buffer, s->be_buffer_size,
                         "tpm_tis: From TPM");
 
     if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
@@ -444,10 +442,10 @@  static uint32_t tpm_tis_data_read(TPMState *s, uint8_t locty)
     uint16_t len;
 
     if ((s->loc[locty].sts & TPM_TIS_STS_DATA_AVAILABLE)) {
-        len = MIN(tpm_cmd_get_size(&s->loc[locty].r_buffer),
+        len = MIN(tpm_cmd_get_size(&s->r_buffer),
                   s->be_buffer_size);
 
-        ret = s->loc[locty].r_buffer[s->loc[locty].r_offset++];
+        ret = s->r_buffer[s->loc[locty].r_offset++];
         if (s->loc[locty].r_offset >= len) {
             /* got last byte */
             tpm_tis_sts_set(&s->loc[locty], TPM_TIS_STS_VALID);
@@ -493,12 +491,11 @@  static void tpm_tis_dump_state(void *opaque, hwaddr addr)
             "tpm_tis: result buffer : ",
             s->loc[locty].r_offset);
     for (idx = 0;
-         idx < MIN(tpm_cmd_get_size(&s->loc[locty].r_buffer),
-                   s->be_buffer_size);
+         idx < MIN(tpm_cmd_get_size(&s->r_buffer), s->be_buffer_size);
          idx++) {
         DPRINTF("%c%02x%s",
                 s->loc[locty].r_offset == idx ? '>' : ' ',
-                s->loc[locty].r_buffer[idx],
+                s->r_buffer[idx],
                 ((idx & 0xf) == 0xf) ? "\ntpm_tis:                 " : "");
     }
     DPRINTF("\n"
@@ -506,12 +503,11 @@  static void tpm_tis_dump_state(void *opaque, hwaddr addr)
             "tpm_tis: request buffer: ",
             s->loc[locty].w_offset);
     for (idx = 0;
-         idx < MIN(tpm_cmd_get_size(s->loc[locty].w_buffer),
-                   s->be_buffer_size);
+         idx < MIN(tpm_cmd_get_size(s->w_buffer), s->be_buffer_size);
          idx++) {
         DPRINTF("%c%02x%s",
                 s->loc[locty].w_offset == idx ? '>' : ' ',
-                s->loc[locty].w_buffer[idx],
+                s->w_buffer[idx],
                 ((idx & 0xf) == 0xf) ? "\ntpm_tis:                 " : "");
     }
     DPRINTF("\n");
@@ -573,7 +569,7 @@  static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
         if (s->active_locty == locty) {
             if ((s->loc[locty].sts & TPM_TIS_STS_DATA_AVAILABLE)) {
                 val = TPM_TIS_BURST_COUNT(
-                       MIN(tpm_cmd_get_size(&s->loc[locty].r_buffer),
+                       MIN(tpm_cmd_get_size(&s->r_buffer),
                            s->be_buffer_size)
                        - s->loc[locty].r_offset) | s->loc[locty].sts;
             } else {
@@ -926,7 +922,7 @@  static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
 
             while ((s->loc[locty].sts & TPM_TIS_STS_EXPECT) && size > 0) {
                 if (s->loc[locty].w_offset < s->be_buffer_size) {
-                    s->loc[locty].w_buffer[s->loc[locty].w_offset++] =
+                    s->w_buffer[s->loc[locty].w_offset++] =
                         (uint8_t)val;
                     val >>= 8;
                     size--;
@@ -941,7 +937,7 @@  static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
                 /* we have a packet length - see if we have all of it */
                 bool need_irq = !(s->loc[locty].sts & TPM_TIS_STS_VALID);
 
-                len = tpm_cmd_get_size(&s->loc[locty].w_buffer);
+                len = tpm_cmd_get_size(&s->w_buffer);
                 if (len > s->loc[locty].w_offset) {
                     tpm_tis_sts_set(&s->loc[locty],
                                     TPM_TIS_STS_EXPECT | TPM_TIS_STS_VALID);