diff mbox series

[v2,3/5] tpm: report backend request error

Message ID 20180119141105.29095-4-marcandre.lureau@redhat.com
State New
Headers show
Series tpm: CRB device and cleanups | expand

Commit Message

Marc-André Lureau Jan. 19, 2018, 2:11 p.m. UTC
Use an Error** for request to let the caller handle error reporting.

This will also allow to inform the frontend of a backend error.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/sysemu/tpm.h         |  2 +-
 include/sysemu/tpm_backend.h |  3 ++-
 backends/tpm.c               |  9 +++++++--
 hw/tpm/tpm_emulator.c        | 21 ++++++---------------
 hw/tpm/tpm_passthrough.c     | 31 +++++++++++++++----------------
 hw/tpm/tpm_tis.c             |  3 ++-
 6 files changed, 33 insertions(+), 36 deletions(-)

Comments

Stefan Berger Jan. 19, 2018, 2:57 p.m. UTC | #1
On 01/19/2018 09:11 AM, Marc-André Lureau wrote:
> Use an Error** for request to let the caller handle error reporting.
>
> This will also allow to inform the frontend of a backend error.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

> ---
>   include/sysemu/tpm.h         |  2 +-
>   include/sysemu/tpm_backend.h |  3 ++-
>   backends/tpm.c               |  9 +++++++--
>   hw/tpm/tpm_emulator.c        | 21 ++++++---------------
>   hw/tpm/tpm_passthrough.c     | 31 +++++++++++++++----------------
>   hw/tpm/tpm_tis.c             |  3 ++-
>   6 files changed, 33 insertions(+), 36 deletions(-)
>
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 852e02687c..ac04a9d4a2 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -41,7 +41,7 @@ typedef struct TPMIfClass {
>       InterfaceClass parent_class;
>   
>       enum TpmModel model;
> -    void (*request_completed)(TPMIf *obj);
> +    void (*request_completed)(TPMIf *obj, int ret);
>       enum TPMVersion (*get_version)(TPMIf *obj);
>   } TPMIfClass;
>   
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index a69593e0cd..7e166ef954 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -18,6 +18,7 @@
>   #include "qapi-types.h"
>   #include "qemu/option.h"
>   #include "sysemu/tpm.h"
> +#include "qapi/error.h"
>   
>   #define TYPE_TPM_BACKEND "tpm-backend"
>   #define TPM_BACKEND(obj) \
> @@ -84,7 +85,7 @@ struct TPMBackendClass {
>   
>       TpmTypeOptions *(*get_tpm_options)(TPMBackend *t);
>   
> -    void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd);
> +    void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd, Error **errp);
>   };
>   
>   /**
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 143807aa37..d617ba7c52 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -27,7 +27,7 @@ static void tpm_backend_request_completed(void *opaque, int ret)
>       TPMBackend *s = TPM_BACKEND(opaque);
>       TPMIfClass *tic = TPM_IF_GET_CLASS(s->tpmif);
>   
> -    tic->request_completed(s->tpmif);
> +    tic->request_completed(s->tpmif, ret);
>   
>       /* no need for atomic, as long the BQL is taken */
>       s->cmd = NULL;
> @@ -38,8 +38,13 @@ static int tpm_backend_worker_thread(gpointer data)
>   {
>       TPMBackend *s = TPM_BACKEND(data);
>       TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +    Error *err = NULL;
>   
> -    k->handle_request(s, s->cmd);
> +    k->handle_request(s, s->cmd, &err);
> +    if (err) {
> +        error_report_err(err);
> +        return -1;
> +    }
>   
>       return 0;
>   }
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 35c78de5a9..52552af9f9 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -185,28 +185,19 @@ static int tpm_emulator_set_locality(TPMEmulator *tpm_emu, uint8_t locty_number,
>       return 0;
>   }
>   
> -static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
> +static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd,
> +                                        Error **errp)
>   {
>       TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> -    Error *err = NULL;
>   
>       DPRINTF("processing TPM command");
>   
> -    if (tpm_emulator_set_locality(tpm_emu, cmd->locty, &err) < 0) {
> -        goto error;
> -    }
> -
> -    if (tpm_emulator_unix_tx_bufs(tpm_emu, cmd->in, cmd->in_len,
> +    if (tpm_emulator_set_locality(tpm_emu, cmd->locty, errp) < 0 ||
> +        tpm_emulator_unix_tx_bufs(tpm_emu, cmd->in, cmd->in_len,
>                                     cmd->out, cmd->out_len,
> -                                  &cmd->selftest_done, &err) < 0) {
> -        goto error;
> +                                  &cmd->selftest_done, errp) < 0) {
> +        tpm_util_write_fatal_error_response(cmd->out, cmd->out_len);
>       }
> -
> -    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 f2563ac668..37dce15801 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -80,10 +80,11 @@ static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
>       }
>       return ret;
>   }
> -static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> -                                        const uint8_t *in, uint32_t in_len,
> -                                        uint8_t *out, uint32_t out_len,
> -                                        bool *selftest_done)
> +
> +static void tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> +                                         const uint8_t *in, uint32_t in_len,
> +                                         uint8_t *out, uint32_t out_len,
> +                                         bool *selftest_done, Error **errp)
>   {
>       ssize_t ret;
>       bool is_selftest;
> @@ -99,9 +100,8 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>       ret = qemu_write_full(tpm_pt->tpm_fd, in, in_len);
>       if (ret != in_len) {
>           if (!tpm_pt->tpm_op_canceled || errno != ECANCELED) {
> -            error_report("tpm_passthrough: error while transmitting data "
> -                         "to TPM: %s (%i)",
> -                         strerror(errno), errno);
> +            error_setg_errno(errp, errno, "tpm_passthrough: error while "
> +                             "transmitting data to TPM");
>           }
>           goto err_exit;
>       }
> @@ -111,15 +111,14 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>       ret = tpm_passthrough_unix_read(tpm_pt->tpm_fd, out, out_len);
>       if (ret < 0) {
>           if (!tpm_pt->tpm_op_canceled || errno != ECANCELED) {
> -            error_report("tpm_passthrough: error while reading data from "
> -                         "TPM: %s (%i)",
> -                         strerror(errno), errno);
> +            error_setg_errno(errp, errno, "tpm_passthrough: error while "
> +                             "reading data from TPM");
>           }
>       } else if (ret < sizeof(struct tpm_resp_hdr) ||
>                  be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
>           ret = -1;
> -        error_report("tpm_passthrough: received invalid response "
> -                     "packet from TPM");
> +        error_setg_errno(errp, errno, "tpm_passthrough: received invalid "
> +                     "response packet from TPM");
>       }
>   
>       if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
> @@ -133,18 +132,18 @@ err_exit:
>       }
>   
>       tpm_pt->tpm_executing = false;
> -
> -    return ret;
>   }
>   
> -static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
> +static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd *cmd,
> +                                           Error **errp)
>   {
>       TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>   
>       DPRINTF("tpm_passthrough: processing command %p\n", cmd);
>   
>       tpm_passthrough_unix_tx_bufs(tpm_pt, cmd->in, cmd->in_len,
> -                                 cmd->out, cmd->out_len, &cmd->selftest_done);
> +                                 cmd->out, cmd->out_len, &cmd->selftest_done,
> +                                 errp);
>   }
>   
>   static void tpm_passthrough_reset(TPMBackend *tb)
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 8b5eb01a2c..08f41d2707 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -393,7 +393,7 @@ static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, uint8_t newlocty)
>   /*
>    * Callback from the TPM to indicate that the response was received.
>    */
> -static void tpm_tis_request_completed(TPMIf *ti)
> +static void tpm_tis_request_completed(TPMIf *ti, int ret)
>   {
>       TPMState *s = TPM(ti);
>       uint8_t locty = s->cmd.locty;
> @@ -405,6 +405,7 @@ static void tpm_tis_request_completed(TPMIf *ti)
>           }
>       }
>   
> +    /* FIXME: report error if ret != 0 */
>       tpm_tis_sts_set(&s->loc[locty],
>                       TPM_TIS_STS_VALID | TPM_TIS_STS_DATA_AVAILABLE);
>       s->loc[locty].state = TPM_TIS_STATE_COMPLETION;
diff mbox series

Patch

diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 852e02687c..ac04a9d4a2 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -41,7 +41,7 @@  typedef struct TPMIfClass {
     InterfaceClass parent_class;
 
     enum TpmModel model;
-    void (*request_completed)(TPMIf *obj);
+    void (*request_completed)(TPMIf *obj, int ret);
     enum TPMVersion (*get_version)(TPMIf *obj);
 } TPMIfClass;
 
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index a69593e0cd..7e166ef954 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -18,6 +18,7 @@ 
 #include "qapi-types.h"
 #include "qemu/option.h"
 #include "sysemu/tpm.h"
+#include "qapi/error.h"
 
 #define TYPE_TPM_BACKEND "tpm-backend"
 #define TPM_BACKEND(obj) \
@@ -84,7 +85,7 @@  struct TPMBackendClass {
 
     TpmTypeOptions *(*get_tpm_options)(TPMBackend *t);
 
-    void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd);
+    void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd, Error **errp);
 };
 
 /**
diff --git a/backends/tpm.c b/backends/tpm.c
index 143807aa37..d617ba7c52 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -27,7 +27,7 @@  static void tpm_backend_request_completed(void *opaque, int ret)
     TPMBackend *s = TPM_BACKEND(opaque);
     TPMIfClass *tic = TPM_IF_GET_CLASS(s->tpmif);
 
-    tic->request_completed(s->tpmif);
+    tic->request_completed(s->tpmif, ret);
 
     /* no need for atomic, as long the BQL is taken */
     s->cmd = NULL;
@@ -38,8 +38,13 @@  static int tpm_backend_worker_thread(gpointer data)
 {
     TPMBackend *s = TPM_BACKEND(data);
     TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+    Error *err = NULL;
 
-    k->handle_request(s, s->cmd);
+    k->handle_request(s, s->cmd, &err);
+    if (err) {
+        error_report_err(err);
+        return -1;
+    }
 
     return 0;
 }
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 35c78de5a9..52552af9f9 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -185,28 +185,19 @@  static int tpm_emulator_set_locality(TPMEmulator *tpm_emu, uint8_t locty_number,
     return 0;
 }
 
-static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
+static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd,
+                                        Error **errp)
 {
     TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
-    Error *err = NULL;
 
     DPRINTF("processing TPM command");
 
-    if (tpm_emulator_set_locality(tpm_emu, cmd->locty, &err) < 0) {
-        goto error;
-    }
-
-    if (tpm_emulator_unix_tx_bufs(tpm_emu, cmd->in, cmd->in_len,
+    if (tpm_emulator_set_locality(tpm_emu, cmd->locty, errp) < 0 ||
+        tpm_emulator_unix_tx_bufs(tpm_emu, cmd->in, cmd->in_len,
                                   cmd->out, cmd->out_len,
-                                  &cmd->selftest_done, &err) < 0) {
-        goto error;
+                                  &cmd->selftest_done, errp) < 0) {
+        tpm_util_write_fatal_error_response(cmd->out, cmd->out_len);
     }
-
-    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 f2563ac668..37dce15801 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -80,10 +80,11 @@  static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
     }
     return ret;
 }
-static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
-                                        const uint8_t *in, uint32_t in_len,
-                                        uint8_t *out, uint32_t out_len,
-                                        bool *selftest_done)
+
+static void tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
+                                         const uint8_t *in, uint32_t in_len,
+                                         uint8_t *out, uint32_t out_len,
+                                         bool *selftest_done, Error **errp)
 {
     ssize_t ret;
     bool is_selftest;
@@ -99,9 +100,8 @@  static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
     ret = qemu_write_full(tpm_pt->tpm_fd, in, in_len);
     if (ret != in_len) {
         if (!tpm_pt->tpm_op_canceled || errno != ECANCELED) {
-            error_report("tpm_passthrough: error while transmitting data "
-                         "to TPM: %s (%i)",
-                         strerror(errno), errno);
+            error_setg_errno(errp, errno, "tpm_passthrough: error while "
+                             "transmitting data to TPM");
         }
         goto err_exit;
     }
@@ -111,15 +111,14 @@  static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
     ret = tpm_passthrough_unix_read(tpm_pt->tpm_fd, out, out_len);
     if (ret < 0) {
         if (!tpm_pt->tpm_op_canceled || errno != ECANCELED) {
-            error_report("tpm_passthrough: error while reading data from "
-                         "TPM: %s (%i)",
-                         strerror(errno), errno);
+            error_setg_errno(errp, errno, "tpm_passthrough: error while "
+                             "reading data from TPM");
         }
     } else if (ret < sizeof(struct tpm_resp_hdr) ||
                be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
         ret = -1;
-        error_report("tpm_passthrough: received invalid response "
-                     "packet from TPM");
+        error_setg_errno(errp, errno, "tpm_passthrough: received invalid "
+                     "response packet from TPM");
     }
 
     if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
@@ -133,18 +132,18 @@  err_exit:
     }
 
     tpm_pt->tpm_executing = false;
-
-    return ret;
 }
 
-static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
+static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd *cmd,
+                                           Error **errp)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
 
     DPRINTF("tpm_passthrough: processing command %p\n", cmd);
 
     tpm_passthrough_unix_tx_bufs(tpm_pt, cmd->in, cmd->in_len,
-                                 cmd->out, cmd->out_len, &cmd->selftest_done);
+                                 cmd->out, cmd->out_len, &cmd->selftest_done,
+                                 errp);
 }
 
 static void tpm_passthrough_reset(TPMBackend *tb)
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 8b5eb01a2c..08f41d2707 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -393,7 +393,7 @@  static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, uint8_t newlocty)
 /*
  * Callback from the TPM to indicate that the response was received.
  */
-static void tpm_tis_request_completed(TPMIf *ti)
+static void tpm_tis_request_completed(TPMIf *ti, int ret)
 {
     TPMState *s = TPM(ti);
     uint8_t locty = s->cmd.locty;
@@ -405,6 +405,7 @@  static void tpm_tis_request_completed(TPMIf *ti)
         }
     }
 
+    /* FIXME: report error if ret != 0 */
     tpm_tis_sts_set(&s->loc[locty],
                     TPM_TIS_STS_VALID | TPM_TIS_STS_DATA_AVAILABLE);
     s->loc[locty].state = TPM_TIS_STATE_COMPLETION;