Re: [Qemu-devel] [PATCH 14/42] tpm: add TPMBackendCmd to hold the request state

[Stefan Berger]

On 10/10/2017 12:16 PM, Marc-André Lureau wrote:

Hi

- Original Message -

On 10/09/2017 06:55 PM, Marc-André Lureau wrote:

This simplifies a bit locality handling, and argument passing, and
could pave the way to queuing requests (if that makes sense).

We won't queue requests. The TPM interfaces all send one request and
expect the driver to wait until the response comes back.

Even on different localities? (I am not familiar enough with that part)


Yes, I believe so.




Signed-off-by: Marc-André Lureau 

Reviewed-by: Stefan Berger 


---
   hw/tpm/tpm_int.h |  1 +
   include/sysemu/tpm_backend.h | 16 +---
   backends/tpm.c   |  6 +++---
   hw/tpm/tpm_emulator.c| 29 +++--
   hw/tpm/tpm_passthrough.c | 24 +---
   hw/tpm/tpm_tis.c | 18 +-
   6 files changed, 50 insertions(+), 44 deletions(-)

diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index f2f285b3cc..6d7b3dc850 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -26,6 +26,7 @@ struct TPMState {
   
   uint8_t locty_number;

   TPMLocality *locty_data;
+TPMBackendCmd cmd;
   
   char *backend;

   TPMBackend *be_driver;
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 9c83a512e1..3bb90be3de 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -30,7 +30,16 @@
   typedef struct TPMBackendClass TPMBackendClass;
   typedef struct TPMBackend TPMBackend;
   
-typedef void (TPMRecvDataCB)(TPMState *, bool selftest_done);

+typedef void (TPMRecvDataCB)(TPMState *);
+
+typedef struct TPMBackendCmd {
+uint8_t locty;
+const uint8_t *in;
+uint32_t in_len;
+uint8_t *out;
+uint32_t out_len;
+bool selftest_done;
+} TPMBackendCmd;
   
   struct TPMBackend {

   Object parent;
@@ -76,7 +85,7 @@ struct TPMBackendClass {
   
   void (*opened)(TPMBackend *s, Error **errp);
   
-void (*handle_request)(TPMBackend *s);

+void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd);
   };
   
   /**

@@ -121,11 +130,12 @@ bool tpm_backend_had_startup_error(TPMBackend *s);
   /**
* tpm_backend_deliver_request:
* @s: the backend to send the request to
+ * @cmd: the command to deliver
*
* Send a request to the backend. The backend will then send the request
* to the TPM implementation.
*/
-void tpm_backend_deliver_request(TPMBackend *s);
+void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd);
   
   /**

* tpm_backend_reset:
diff --git a/backends/tpm.c b/backends/tpm.c
index 34e82085ec..dc7c831ff8 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -25,7 +25,7 @@ static void tpm_backend_worker_thread(gpointer data,
gpointer user_data)
   TPMBackendClass *k  = TPM_BACKEND_GET_CLASS(s);
   
   assert(k->handle_request != NULL);

-k->handle_request(s);
+k->handle_request(s, (TPMBackendCmd *)data);
   }
   
   static void tpm_backend_thread_end(TPMBackend *s)

@@ -76,9 +76,9 @@ bool tpm_backend_had_startup_error(TPMBackend *s)
   return s->had_startup_error;
   }
   
-void tpm_backend_deliver_request(TPMBackend *s)

+void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd)
   {
-g_thread_pool_push(s->thread_pool, NULL, NULL);
+g_thread_pool_push(s->thread_pool, cmd, NULL);
   }
   
   void tpm_backend_reset(TPMBackend *s)

diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 4fe405353a..788ab9876d 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -172,28 +172,29 @@ static int tpm_emulator_set_locality(TPMEmulator
*tpm_emu, uint8_t locty_number)
   return 0;
   }
   
-static void tpm_emulator_handle_request(TPMBackend *tb)

+static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd
*cmd)
   {
   TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
-TPMLocality *locty = NULL;
-bool selftest_done = false;
   Error *err = NULL;
   
   DPRINTF("processing TPM command");
   
-locty = tb->tpm_state->locty_data;

-if (tpm_emulator_set_locality(tpm_emu,
-  tb->tpm_state->locty_number) < 0 ||
-tpm_emulator_unix_tx_bufs(tpm_emu, locty->w_buffer.buffer,
-  locty->w_offset, locty->r_buffer.buffer,
-  locty->r_buffer.size, _done,
-  ) < 0) {
-tpm_util_write_fatal_error_response(locty->r_buffer.buffer,
-locty->r_buffer.size);
-error_report_err(err);
+if (tpm_emulator_set_locality(tpm_emu, tb->tpm_state->locty_number) <
0) {
+goto error;
+}
+
+if (tpm_emulator_unix_tx_bufs(tpm_emu, cmd->in, cmd->in_len,
+  cmd->out, cmd->out_len,
+  >selftest_done, ) < 0) {
+goto error;

Re: [Qemu-devel] [PATCH 14/42] tpm: add TPMBackendCmd to hold the request state

[Marc-André Lureau]

Hi

- Original Message -
> On 10/09/2017 06:55 PM, Marc-André Lureau wrote:
> > This simplifies a bit locality handling, and argument passing, and
> > could pave the way to queuing requests (if that makes sense).
> 
> We won't queue requests. The TPM interfaces all send one request and
> expect the driver to wait until the response comes back.

Even on different localities? (I am not familiar enough with that part)

> 
> >
> > Signed-off-by: Marc-André Lureau 
> 
> Reviewed-by: Stefan Berger 
> 
> > ---
> >   hw/tpm/tpm_int.h |  1 +
> >   include/sysemu/tpm_backend.h | 16 +---
> >   backends/tpm.c   |  6 +++---
> >   hw/tpm/tpm_emulator.c| 29 +++--
> >   hw/tpm/tpm_passthrough.c | 24 +---
> >   hw/tpm/tpm_tis.c | 18 +-
> >   6 files changed, 50 insertions(+), 44 deletions(-)
> >
> > diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> > index f2f285b3cc..6d7b3dc850 100644
> > --- a/hw/tpm/tpm_int.h
> > +++ b/hw/tpm/tpm_int.h
> > @@ -26,6 +26,7 @@ struct TPMState {
> >   
> >   uint8_t locty_number;
> >   TPMLocality *locty_data;
> > +TPMBackendCmd cmd;
> >   
> >   char *backend;
> >   TPMBackend *be_driver;
> > diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> > index 9c83a512e1..3bb90be3de 100644
> > --- a/include/sysemu/tpm_backend.h
> > +++ b/include/sysemu/tpm_backend.h
> > @@ -30,7 +30,16 @@
> >   typedef struct TPMBackendClass TPMBackendClass;
> >   typedef struct TPMBackend TPMBackend;
> >   
> > -typedef void (TPMRecvDataCB)(TPMState *, bool selftest_done);
> > +typedef void (TPMRecvDataCB)(TPMState *);
> > +
> > +typedef struct TPMBackendCmd {
> > +uint8_t locty;
> > +const uint8_t *in;
> > +uint32_t in_len;
> > +uint8_t *out;
> > +uint32_t out_len;
> > +bool selftest_done;
> > +} TPMBackendCmd;
> >   
> >   struct TPMBackend {
> >   Object parent;
> > @@ -76,7 +85,7 @@ struct TPMBackendClass {
> >   
> >   void (*opened)(TPMBackend *s, Error **errp);
> >   
> > -void (*handle_request)(TPMBackend *s);
> > +void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd);
> >   };
> >   
> >   /**
> > @@ -121,11 +130,12 @@ bool tpm_backend_had_startup_error(TPMBackend *s);
> >   /**
> >* tpm_backend_deliver_request:
> >* @s: the backend to send the request to
> > + * @cmd: the command to deliver
> >*
> >* Send a request to the backend. The backend will then send the request
> >* to the TPM implementation.
> >*/
> > -void tpm_backend_deliver_request(TPMBackend *s);
> > +void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd);
> >   
> >   /**
> >* tpm_backend_reset:
> > diff --git a/backends/tpm.c b/backends/tpm.c
> > index 34e82085ec..dc7c831ff8 100644
> > --- a/backends/tpm.c
> > +++ b/backends/tpm.c
> > @@ -25,7 +25,7 @@ static void tpm_backend_worker_thread(gpointer data,
> > gpointer user_data)
> >   TPMBackendClass *k  = TPM_BACKEND_GET_CLASS(s);
> >   
> >   assert(k->handle_request != NULL);
> > -k->handle_request(s);
> > +k->handle_request(s, (TPMBackendCmd *)data);
> >   }
> >   
> >   static void tpm_backend_thread_end(TPMBackend *s)
> > @@ -76,9 +76,9 @@ bool tpm_backend_had_startup_error(TPMBackend *s)
> >   return s->had_startup_error;
> >   }
> >   
> > -void tpm_backend_deliver_request(TPMBackend *s)
> > +void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd)
> >   {
> > -g_thread_pool_push(s->thread_pool, NULL, NULL);
> > +g_thread_pool_push(s->thread_pool, cmd, NULL);
> >   }
> >   
> >   void tpm_backend_reset(TPMBackend *s)
> > diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> > index 4fe405353a..788ab9876d 100644
> > --- a/hw/tpm/tpm_emulator.c
> > +++ b/hw/tpm/tpm_emulator.c
> > @@ -172,28 +172,29 @@ static int tpm_emulator_set_locality(TPMEmulator
> > *tpm_emu, uint8_t locty_number)
> >   return 0;
> >   }
> >   
> > -static void tpm_emulator_handle_request(TPMBackend *tb)
> > +static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd
> > *cmd)
> >   {
> >   TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> > -TPMLocality *locty = NULL;
> > -bool selftest_done = false;
> >   Error *err = NULL;
> >   
> >   DPRINTF("processing TPM command");
> >   
> > -locty = tb->tpm_state->locty_data;
> > -if (tpm_emulator_set_locality(tpm_emu,
> > -  tb->tpm_state->locty_number) < 0 ||
> > -tpm_emulator_unix_tx_bufs(tpm_emu, locty->w_buffer.buffer,
> > -  locty->w_offset, locty->r_buffer.buffer,
> > -  locty->r_buffer.size, _done,
> > -  ) < 0) {
> > -tpm_util_write_fatal_error_response(locty->r_buffer.buffer,
> > -

Re: [Qemu-devel] [PATCH 14/42] tpm: add TPMBackendCmd to hold the request state

[Stefan Berger]

On 10/09/2017 06:55 PM, Marc-André Lureau wrote:

This simplifies a bit locality handling, and argument passing, and
could pave the way to queuing requests (if that makes sense).


We won't queue requests. The TPM interfaces all send one request and 
expect the driver to wait until the response comes back.




Signed-off-by: Marc-André Lureau 


Reviewed-by: Stefan Berger 


---
  hw/tpm/tpm_int.h |  1 +
  include/sysemu/tpm_backend.h | 16 +---
  backends/tpm.c   |  6 +++---
  hw/tpm/tpm_emulator.c| 29 +++--
  hw/tpm/tpm_passthrough.c | 24 +---
  hw/tpm/tpm_tis.c | 18 +-
  6 files changed, 50 insertions(+), 44 deletions(-)

diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index f2f285b3cc..6d7b3dc850 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -26,6 +26,7 @@ struct TPMState {
  
  uint8_t locty_number;

  TPMLocality *locty_data;
+TPMBackendCmd cmd;
  
  char *backend;

  TPMBackend *be_driver;
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 9c83a512e1..3bb90be3de 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -30,7 +30,16 @@
  typedef struct TPMBackendClass TPMBackendClass;
  typedef struct TPMBackend TPMBackend;
  
-typedef void (TPMRecvDataCB)(TPMState *, bool selftest_done);

+typedef void (TPMRecvDataCB)(TPMState *);
+
+typedef struct TPMBackendCmd {
+uint8_t locty;
+const uint8_t *in;
+uint32_t in_len;
+uint8_t *out;
+uint32_t out_len;
+bool selftest_done;
+} TPMBackendCmd;
  
  struct TPMBackend {

  Object parent;
@@ -76,7 +85,7 @@ struct TPMBackendClass {
  
  void (*opened)(TPMBackend *s, Error **errp);
  
-void (*handle_request)(TPMBackend *s);

+void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd);
  };
  
  /**

@@ -121,11 +130,12 @@ bool tpm_backend_had_startup_error(TPMBackend *s);
  /**
   * tpm_backend_deliver_request:
   * @s: the backend to send the request to
+ * @cmd: the command to deliver
   *
   * Send a request to the backend. The backend will then send the request
   * to the TPM implementation.
   */
-void tpm_backend_deliver_request(TPMBackend *s);
+void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd);
  
  /**

   * tpm_backend_reset:
diff --git a/backends/tpm.c b/backends/tpm.c
index 34e82085ec..dc7c831ff8 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -25,7 +25,7 @@ static void tpm_backend_worker_thread(gpointer data, gpointer 
user_data)
  TPMBackendClass *k  = TPM_BACKEND_GET_CLASS(s);
  
  assert(k->handle_request != NULL);

-k->handle_request(s);
+k->handle_request(s, (TPMBackendCmd *)data);
  }
  
  static void tpm_backend_thread_end(TPMBackend *s)

@@ -76,9 +76,9 @@ bool tpm_backend_had_startup_error(TPMBackend *s)
  return s->had_startup_error;
  }
  
-void tpm_backend_deliver_request(TPMBackend *s)

+void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd)
  {
-g_thread_pool_push(s->thread_pool, NULL, NULL);
+g_thread_pool_push(s->thread_pool, cmd, NULL);
  }
  
  void tpm_backend_reset(TPMBackend *s)

diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 4fe405353a..788ab9876d 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -172,28 +172,29 @@ static int tpm_emulator_set_locality(TPMEmulator 
*tpm_emu, uint8_t locty_number)
  return 0;
  }
  
-static void tpm_emulator_handle_request(TPMBackend *tb)

+static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
  {
  TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
-TPMLocality *locty = NULL;
-bool selftest_done = false;
  Error *err = NULL;
  
  DPRINTF("processing TPM command");
  
-locty = tb->tpm_state->locty_data;

-if (tpm_emulator_set_locality(tpm_emu,
-  tb->tpm_state->locty_number) < 0 ||
-tpm_emulator_unix_tx_bufs(tpm_emu, locty->w_buffer.buffer,
-  locty->w_offset, locty->r_buffer.buffer,
-  locty->r_buffer.size, _done,
-  ) < 0) {
-tpm_util_write_fatal_error_response(locty->r_buffer.buffer,
-locty->r_buffer.size);
-error_report_err(err);
+if (tpm_emulator_set_locality(tpm_emu, tb->tpm_state->locty_number) < 0) {
+goto error;
+}
+
+if (tpm_emulator_unix_tx_bufs(tpm_emu, cmd->in, cmd->in_len,
+  cmd->out, cmd->out_len,
+  >selftest_done, ) < 0) {
+goto error;
  }
  
-tb->recv_data_callback(tb->tpm_state, selftest_done);

+tb->recv_data_callback(tb->tpm_state);
+return;
+
+error:
+tpm_util_write_fatal_error_response(cmd->out, cmd->out_len);
+

[Qemu-devel] [PATCH 14/42] tpm: add TPMBackendCmd to hold the request state

[Marc-André Lureau]

This simplifies a bit locality handling, and argument passing, and
could pave the way to queuing requests (if that makes sense).

Signed-off-by: Marc-André Lureau 
---
 hw/tpm/tpm_int.h |  1 +
 include/sysemu/tpm_backend.h | 16 +---
 backends/tpm.c   |  6 +++---
 hw/tpm/tpm_emulator.c| 29 +++--
 hw/tpm/tpm_passthrough.c | 24 +---
 hw/tpm/tpm_tis.c | 18 +-
 6 files changed, 50 insertions(+), 44 deletions(-)

diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index f2f285b3cc..6d7b3dc850 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -26,6 +26,7 @@ struct TPMState {
 
 uint8_t locty_number;
 TPMLocality *locty_data;
+TPMBackendCmd cmd;
 
 char *backend;
 TPMBackend *be_driver;
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 9c83a512e1..3bb90be3de 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -30,7 +30,16 @@
 typedef struct TPMBackendClass TPMBackendClass;
 typedef struct TPMBackend TPMBackend;
 
-typedef void (TPMRecvDataCB)(TPMState *, bool selftest_done);
+typedef void (TPMRecvDataCB)(TPMState *);
+
+typedef struct TPMBackendCmd {
+uint8_t locty;
+const uint8_t *in;
+uint32_t in_len;
+uint8_t *out;
+uint32_t out_len;
+bool selftest_done;
+} TPMBackendCmd;
 
 struct TPMBackend {
 Object parent;
@@ -76,7 +85,7 @@ struct TPMBackendClass {
 
 void (*opened)(TPMBackend *s, Error **errp);
 
-void (*handle_request)(TPMBackend *s);
+void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd);
 };
 
 /**
@@ -121,11 +130,12 @@ bool tpm_backend_had_startup_error(TPMBackend *s);
 /**
  * tpm_backend_deliver_request:
  * @s: the backend to send the request to
+ * @cmd: the command to deliver
  *
  * Send a request to the backend. The backend will then send the request
  * to the TPM implementation.
  */
-void tpm_backend_deliver_request(TPMBackend *s);
+void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd);
 
 /**
  * tpm_backend_reset:
diff --git a/backends/tpm.c b/backends/tpm.c
index 34e82085ec..dc7c831ff8 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -25,7 +25,7 @@ static void tpm_backend_worker_thread(gpointer data, gpointer 
user_data)
 TPMBackendClass *k  = TPM_BACKEND_GET_CLASS(s);
 
 assert(k->handle_request != NULL);
-k->handle_request(s);
+k->handle_request(s, (TPMBackendCmd *)data);
 }
 
 static void tpm_backend_thread_end(TPMBackend *s)
@@ -76,9 +76,9 @@ bool tpm_backend_had_startup_error(TPMBackend *s)
 return s->had_startup_error;
 }
 
-void tpm_backend_deliver_request(TPMBackend *s)
+void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd)
 {
-g_thread_pool_push(s->thread_pool, NULL, NULL);
+g_thread_pool_push(s->thread_pool, cmd, NULL);
 }
 
 void tpm_backend_reset(TPMBackend *s)
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 4fe405353a..788ab9876d 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -172,28 +172,29 @@ static int tpm_emulator_set_locality(TPMEmulator 
*tpm_emu, uint8_t locty_number)
 return 0;
 }
 
-static void tpm_emulator_handle_request(TPMBackend *tb)
+static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
 {
 TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
-TPMLocality *locty = NULL;
-bool selftest_done = false;
 Error *err = NULL;
 
 DPRINTF("processing TPM command");
 
-locty = tb->tpm_state->locty_data;
-if (tpm_emulator_set_locality(tpm_emu,
-  tb->tpm_state->locty_number) < 0 ||
-tpm_emulator_unix_tx_bufs(tpm_emu, locty->w_buffer.buffer,
-  locty->w_offset, locty->r_buffer.buffer,
-  locty->r_buffer.size, _done,
-  ) < 0) {
-tpm_util_write_fatal_error_response(locty->r_buffer.buffer,
-locty->r_buffer.size);
-error_report_err(err);
+if (tpm_emulator_set_locality(tpm_emu, tb->tpm_state->locty_number) < 0) {
+goto error;
+}
+
+if (tpm_emulator_unix_tx_bufs(tpm_emu, cmd->in, cmd->in_len,
+  cmd->out, cmd->out_len,
+  >selftest_done, ) < 0) {
+goto error;
 }
 
-tb->recv_data_callback(tb->tpm_state, selftest_done);
+tb->recv_data_callback(tb->tpm_state);
+return;
+
+error:
+tpm_util_write_fatal_error_response(cmd->out, cmd->out_len);
+error_report_err(err);
 }
 
 static int tpm_emulator_probe_caps(TPMEmulator *tpm_emu)
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 0ae4596932..93d72b8e9e 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -137,30 +137,16 @@ err_exit:
 return ret;
 }
 
-static int