[PATCH 0/8] tpm: Split hw/ vs backends/

[PATCH 0/8] tpm: Split hw/ vs backends/

[Philippe Mathieu-Daudé]

Hi,

Today I started to review some vTPM patches and got very
confused by the files under hw/tpm/. In particular after
running:

  $ git grep TYPE_TPM_BACKEND
  backends/tpm.c:188:    .name = TYPE_TPM_BACKEND,
  hw/tpm/tpm_emulator.c:985:    .parent = TYPE_TPM_BACKEND,
  hw/tpm/tpm_passthrough.c:393:    .parent = TYPE_TPM_BACKEND,
  include/sysemu/tpm_backend.h:21:#define TYPE_TPM_BACKEND "tpm-backend"

As there is no particular reason to keep this mixed, clean it up.

Philippe Mathieu-Daudé (8):
  hw/tpm: Do not include 'qemu/osdep.h' in header
  hw/tpm: Include missing 'qemu/option.h' header
  hw/tpm: Move 'hw/acpi/tpm.h' inclusion from header to sources
  hw/tpm: Remove unnecessary 'tpm_int.h' header inclusion
  hw/tpm: Move few declarations from 'tpm_util.h' to 'tpm_int.h'
  hw/tpm: Move DEFINE_PROP_TPMBE() macro to 'tmp_prop.h' local header
  hw/tpm: Make 'tpm_util.h' publicly accessible as "sysemu/tpm_util.h"
  tpm: Move backend code under the 'backends/' directory

 Makefile                               |  2 +-
 {hw/tpm => backends}/tpm_int.h         | 19 +++++++++++---
 {hw/tpm => backends}/tpm_ioctl.h       |  0
 hw/tpm/tpm_ppi.h                       |  1 -
 hw/tpm/tpm_prop.h                      | 31 +++++++++++++++++++++++
 hw/tpm/tpm_tis.h                       |  1 -
 {hw/tpm => include/sysemu}/tpm_util.h  | 19 +++-----------
 {hw/tpm => backends}/tpm_emulator.c    |  2 +-
 {hw/tpm => backends}/tpm_passthrough.c |  2 +-
 {hw/tpm => backends}/tpm_util.c        |  2 +-
 hw/tpm/tpm_crb.c                       |  4 +--
 hw/tpm/tpm_spapr.c                     |  4 +--
 hw/tpm/tpm_tis_common.c                |  4 +--
 hw/tpm/tpm_tis_isa.c                   |  3 ++-
 hw/tpm/tpm_tis_sysbus.c                |  3 ++-
 tests/qtest/tpm-emu.c                  |  2 +-
 MAINTAINERS                            |  2 +-
 backends/Kconfig                       | 14 +++++++++++
 backends/Makefile.objs                 |  3 +++
 backends/trace-events                  | 32 ++++++++++++++++++++++++
 hw/tpm/Kconfig                         | 15 ------------
 hw/tpm/Makefile.objs                   |  3 ---
 hw/tpm/trace-events                    | 34 +-------------------------
 23 files changed, 116 insertions(+), 86 deletions(-)
 rename {hw/tpm => backends}/tpm_int.h (81%)
 rename {hw/tpm => backends}/tpm_ioctl.h (100%)
 create mode 100644 hw/tpm/tpm_prop.h
 rename {hw/tpm => include/sysemu}/tpm_util.h (80%)
 rename {hw/tpm => backends}/tpm_emulator.c (99%)
 rename {hw/tpm => backends}/tpm_passthrough.c (99%)
 rename {hw/tpm => backends}/tpm_util.c (99%)
 create mode 100644 backends/Kconfig

-- 
2.21.3

[PATCH 1/8] hw/tpm: Do not include 'qemu/osdep.h' in header

[Philippe Mathieu-Daudé]

From CODING_STYLE.rst:

  Do not include "qemu/osdep.h" from header files since the .c
  file will have already included it.

Remove "qemu/osdep.h" from "tpm_tis.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_tis.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index 5554989395..f6b5872ba6 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -24,7 +24,6 @@
 #ifndef TPM_TPM_TIS_H
 #define TPM_TPM_TIS_H
 
-#include "qemu/osdep.h"
 #include "sysemu/tpm_backend.h"
 #include "tpm_ppi.h"
 
-- 
2.21.3

[PATCH 2/8] hw/tpm: Include missing 'qemu/option.h' header

[Philippe Mathieu-Daudé]

Files using the TPM_STANDARD_CMDLINE_OPTS macro declared in
"tpm_int.h" will use QEMU_OPT_STRING definition declared in
"qemu/option.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_int.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index 3fb28a9d6c..fd5ebc6489 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -12,6 +12,8 @@
 #ifndef TPM_TPM_INT_H
 #define TPM_TPM_INT_H
 
+#include "qemu/option.h"
+
 #define TPM_STANDARD_CMDLINE_OPTS \
     { \
         .name = "type", \
-- 
2.21.3

[PATCH 3/8] hw/tpm: Move 'hw/acpi/tpm.h' inclusion from header to sources

[Philippe Mathieu-Daudé]

Nothing in "tpm_ppi.h" require declarations from "hw/acpi/tpm.h".
Reduce dependencies and include it only in the files requiring it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_ppi.h        | 1 -
 hw/tpm/tpm_tis_isa.c    | 1 +
 hw/tpm/tpm_tis_sysbus.c | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
index d33ef27de6..6f773c25a0 100644
--- a/hw/tpm/tpm_ppi.h
+++ b/hw/tpm/tpm_ppi.h
@@ -12,7 +12,6 @@
 #ifndef TPM_TPM_PPI_H
 #define TPM_TPM_PPI_H
 
-#include "hw/acpi/tpm.h"
 #include "exec/address-spaces.h"
 
 typedef struct TPMPPI {
diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 30ba37079d..42f909ff1e 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -26,6 +26,7 @@
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
+#include "hw/acpi/tpm.h"
 #include "tpm_util.h"
 #include "tpm_tis.h"
 
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index eced1fc843..edca1dae0d 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
+#include "hw/acpi/tpm.h"
 #include "tpm_util.h"
 #include "hw/sysbus.h"
 #include "tpm_tis.h"
-- 
2.21.3

[PATCH 4/8] hw/tpm: Remove unnecessary 'tpm_int.h' header inclusion

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_crb.c        | 1 -
 hw/tpm/tpm_spapr.c      | 1 -
 hw/tpm/tpm_tis_common.c | 1 -
 3 files changed, 3 deletions(-)

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index cd004e7f8e..664ff70ef9 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -25,7 +25,6 @@
 #include "migration/vmstate.h"
 #include "sysemu/tpm_backend.h"
 #include "sysemu/reset.h"
-#include "tpm_int.h"
 #include "tpm_util.h"
 #include "tpm_ppi.h"
 #include "trace.h"
diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
index ce65eb2e45..ab1a86ad6e 100644
--- a/hw/tpm/tpm_spapr.c
+++ b/hw/tpm/tpm_spapr.c
@@ -20,7 +20,6 @@
 #include "migration/vmstate.h"
 
 #include "sysemu/tpm_backend.h"
-#include "tpm_int.h"
 #include "tpm_util.h"
 
 #include "hw/ppc/spapr.h"
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 1af4bce139..94704870f6 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -33,7 +33,6 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "sysemu/tpm_backend.h"
-#include "tpm_int.h"
 #include "tpm_util.h"
 #include "tpm_ppi.h"
 #include "trace.h"
-- 
2.21.3

[PATCH 5/8] hw/tpm: Move few declarations from 'tpm_util.h' to 'tpm_int.h'

[Philippe Mathieu-Daudé]

We are going to make "tpm_util.h" publicly accessible by
moving it to the include/ directory in a pair of commits.
Keep declarations internals to hw/tpm/ in "tpm_int.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_int.h  | 11 +++++++++++
 hw/tpm/tpm_util.h | 10 ----------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index fd5ebc6489..9f72879d89 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -13,6 +13,7 @@
 #define TPM_TPM_INT_H
 
 #include "qemu/option.h"
+#include "sysemu/tpm.h"
 
 #define TPM_STANDARD_CMDLINE_OPTS \
     { \
@@ -74,4 +75,14 @@ struct tpm_resp_hdr {
 #define TPM_RC_FAILURE            0x101
 #define TPM_RC_LOCALITY           0x907
 
+int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
+                             size_t *buffersize);
+
+typedef struct TPMSizedBuffer {
+    uint32_t size;
+    uint8_t  *buffer;
+} TPMSizedBuffer;
+
+void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
+
 #endif /* TPM_TPM_INT_H */
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index 7889081fba..d524935576 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -66,19 +66,9 @@ 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);
-
 #define DEFINE_PROP_TPMBE(_n, _s, _f)                     \
     DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *)
 
-typedef struct TPMSizedBuffer {
-    uint32_t size;
-    uint8_t  *buffer;
-} TPMSizedBuffer;
-
-void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
-
 void tpm_util_show_buffer(const unsigned char *buffer,
                           size_t buffer_size, const char *string);
 
-- 
2.21.3

[PATCH 6/8] hw/tpm: Move DEFINE_PROP_TPMBE() macro to 'tmp_prop.h' local header

[Philippe Mathieu-Daudé]

We are going to make "tpm_util.h" publicly accessible by
moving it to the include/ directory in the next commit.
The DEFINE_PROP_TPMBE() macro is only meaningful for the
TPM hardware files (in hw/tpm/), so keep this macro in a
local header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_prop.h       | 31 +++++++++++++++++++++++++++++++
 hw/tpm/tpm_util.h       |  3 ---
 hw/tpm/tpm_crb.c        |  1 +
 hw/tpm/tpm_spapr.c      |  1 +
 hw/tpm/tpm_tis_isa.c    |  2 +-
 hw/tpm/tpm_tis_sysbus.c |  2 +-
 6 files changed, 35 insertions(+), 5 deletions(-)
 create mode 100644 hw/tpm/tpm_prop.h

diff --git a/hw/tpm/tpm_prop.h b/hw/tpm/tpm_prop.h
new file mode 100644
index 0000000000..85e1ae5718
--- /dev/null
+++ b/hw/tpm/tpm_prop.h
@@ -0,0 +1,31 @@
+/*
+ * TPM utility functions
+ *
+ *  Copyright (c) 2010 - 2015 IBM Corporation
+ *  Authors:
+ *    Stefan Berger <stefanb@us.ibm.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef HW_TPM_PROP_H
+#define HW_TPM_PROP_H
+
+#include "sysemu/tpm_backend.h"
+#include "hw/qdev-properties.h"
+
+#define DEFINE_PROP_TPMBE(_n, _s, _f)                     \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *)
+
+#endif /* HW_TPM_PROP_H */
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index d524935576..cf61d830d7 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -66,9 +66,6 @@ static inline void tpm_cmd_set_error(void *b, uint32_t error)
     stl_be_p(b + 6, error);
 }
 
-#define DEFINE_PROP_TPMBE(_n, _s, _f)                     \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *)
-
 void tpm_util_show_buffer(const unsigned char *buffer,
                           size_t buffer_size, const char *string);
 
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 664ff70ef9..1cac4d671d 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -25,6 +25,7 @@
 #include "migration/vmstate.h"
 #include "sysemu/tpm_backend.h"
 #include "sysemu/reset.h"
+#include "tpm_prop.h"
 #include "tpm_util.h"
 #include "tpm_ppi.h"
 #include "trace.h"
diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
index ab1a86ad6e..65672048c7 100644
--- a/hw/tpm/tpm_spapr.c
+++ b/hw/tpm/tpm_spapr.c
@@ -21,6 +21,7 @@
 
 #include "sysemu/tpm_backend.h"
 #include "tpm_util.h"
+#include "tpm_prop.h"
 
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 42f909ff1e..5faf6231c0 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -27,7 +27,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/tpm.h"
-#include "tpm_util.h"
+#include "tpm_prop.h"
 #include "tpm_tis.h"
 
 typedef struct TPMStateISA {
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index edca1dae0d..4a3bc70625 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -26,7 +26,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/tpm.h"
-#include "tpm_util.h"
+#include "tpm_prop.h"
 #include "hw/sysbus.h"
 #include "tpm_tis.h"
 
-- 
2.21.3

[PATCH 7/8] hw/tpm: Make 'tpm_util.h' publicly accessible as "sysemu/tpm_util.h"

[Philippe Mathieu-Daudé]

We are going to split the TPM backends from the TPM emulated
hardware in the next commit. Make the TPM util helpers accessible
by moving local "tpm_util.h" to global "sysemu/tpm_util.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 {hw/tpm => include/sysemu}/tpm_util.h | 6 +++---
 hw/tpm/tpm_crb.c                      | 2 +-
 hw/tpm/tpm_emulator.c                 | 2 +-
 hw/tpm/tpm_passthrough.c              | 2 +-
 hw/tpm/tpm_spapr.c                    | 2 +-
 hw/tpm/tpm_tis_common.c               | 2 +-
 hw/tpm/tpm_util.c                     | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)
 rename {hw/tpm => include/sysemu}/tpm_util.h (95%)

diff --git a/hw/tpm/tpm_util.h b/include/sysemu/tpm_util.h
similarity index 95%
rename from hw/tpm/tpm_util.h
rename to include/sysemu/tpm_util.h
index cf61d830d7..63e872c3b2 100644
--- a/hw/tpm/tpm_util.h
+++ b/include/sysemu/tpm_util.h
@@ -19,8 +19,8 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>
  */
 
-#ifndef TPM_TPM_UTIL_H
-#define TPM_TPM_UTIL_H
+#ifndef SYSEMU_TPM_UTIL_H
+#define SYSEMU_TPM_UTIL_H
 
 #include "sysemu/tpm.h"
 #include "qemu/bswap.h"
@@ -69,4 +69,4 @@ static inline void tpm_cmd_set_error(void *b, uint32_t error)
 void tpm_util_show_buffer(const unsigned char *buffer,
                           size_t buffer_size, const char *string);
 
-#endif /* TPM_TPM_UTIL_H */
+#endif /* SYSEMU_TPM_UTIL_H */
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 1cac4d671d..60247295d4 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -24,9 +24,9 @@
 #include "hw/acpi/tpm.h"
 #include "migration/vmstate.h"
 #include "sysemu/tpm_backend.h"
+#include "sysemu/tpm_util.h"
 #include "sysemu/reset.h"
 #include "tpm_prop.h"
-#include "tpm_util.h"
 #include "tpm_ppi.h"
 #include "trace.h"
 
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 3a0fc442f3..9605339f93 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -32,8 +32,8 @@
 #include "qemu/sockets.h"
 #include "io/channel-socket.h"
 #include "sysemu/tpm_backend.h"
+#include "sysemu/tpm_util.h"
 #include "tpm_int.h"
-#include "tpm_util.h"
 #include "tpm_ioctl.h"
 #include "migration/blocker.h"
 #include "migration/vmstate.h"
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index f67244b5d4..7403807ec4 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -28,10 +28,10 @@
 #include "qemu/module.h"
 #include "qemu/sockets.h"
 #include "sysemu/tpm_backend.h"
+#include "sysemu/tpm_util.h"
 #include "tpm_int.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/qapi-visit-tpm.h"
-#include "tpm_util.h"
 #include "trace.h"
 
 #define TYPE_TPM_PASSTHROUGH "tpm-passthrough"
diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
index 65672048c7..cb4dfd1e6a 100644
--- a/hw/tpm/tpm_spapr.c
+++ b/hw/tpm/tpm_spapr.c
@@ -20,7 +20,7 @@
 #include "migration/vmstate.h"
 
 #include "sysemu/tpm_backend.h"
-#include "tpm_util.h"
+#include "sysemu/tpm_util.h"
 #include "tpm_prop.h"
 
 #include "hw/ppc/spapr.h"
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 94704870f6..64206a6a3b 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -33,7 +33,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "sysemu/tpm_backend.h"
-#include "tpm_util.h"
+#include "sysemu/tpm_util.h"
 #include "tpm_ppi.h"
 #include "trace.h"
 
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index c0a0f3d71f..d0ec2a8235 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -23,11 +23,11 @@
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
-#include "tpm_util.h"
 #include "tpm_int.h"
 #include "exec/memory.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/tpm_backend.h"
+#include "sysemu/tpm_util.h"
 #include "trace.h"
 
 /* tpm backend property */
-- 
2.21.3

[RFC PATCH 8/8] tpm: Move backend code under the 'backends/' directory

[Philippe Mathieu-Daudé]

TPM subsytem is split into backends (see commit f4ede81eed2)
and frontends (see i.e. 3676bc69b35). Keep the emulated
hardware 'frontends' under hw/tpm/, but move the backends
in the backends/ directory.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC due to a FIXME in tpm_tis_common.c, it uses
TRACE_TPM_UTIL_SHOW_BUFFER which is now generated
by backends/trace-events...
---
 Makefile                               |  2 +-
 {hw/tpm => backends}/tpm_int.h         |  6 ++---
 {hw/tpm => backends}/tpm_ioctl.h       |  0
 {hw/tpm => backends}/tpm_emulator.c    |  0
 {hw/tpm => backends}/tpm_passthrough.c |  0
 {hw/tpm => backends}/tpm_util.c        |  0
 hw/tpm/tpm_tis_common.c                |  1 +
 tests/qtest/tpm-emu.c                  |  2 +-
 MAINTAINERS                            |  2 +-
 backends/Kconfig                       | 14 +++++++++++
 backends/Makefile.objs                 |  3 +++
 backends/trace-events                  | 32 ++++++++++++++++++++++++
 hw/tpm/Kconfig                         | 15 ------------
 hw/tpm/Makefile.objs                   |  3 ---
 hw/tpm/trace-events                    | 34 +-------------------------
 15 files changed, 57 insertions(+), 57 deletions(-)
 rename {hw/tpm => backends}/tpm_int.h (96%)
 rename {hw/tpm => backends}/tpm_ioctl.h (100%)
 rename {hw/tpm => backends}/tpm_emulator.c (100%)
 rename {hw/tpm => backends}/tpm_passthrough.c (100%)
 rename {hw/tpm => backends}/tpm_util.c (100%)
 create mode 100644 backends/Kconfig

diff --git a/Makefile b/Makefile
index d1af126ea1..0ab6e12dd6 100644
--- a/Makefile
+++ b/Makefile
@@ -418,7 +418,7 @@ MINIKCONF_ARGS = \
     CONFIG_LINUX=$(CONFIG_LINUX) \
     CONFIG_PVRDMA=$(CONFIG_PVRDMA)
 
-MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/hw/Kconfig
+MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/backends/Kconfig $(SRC_PATH)/hw/Kconfig
 MINIKCONF_DEPS = $(MINIKCONF_INPUTS) $(wildcard $(SRC_PATH)/hw/*/Kconfig)
 MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py \
 
diff --git a/hw/tpm/tpm_int.h b/backends/tpm_int.h
similarity index 96%
rename from hw/tpm/tpm_int.h
rename to backends/tpm_int.h
index 9f72879d89..ba6109306e 100644
--- a/hw/tpm/tpm_int.h
+++ b/backends/tpm_int.h
@@ -9,8 +9,8 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
-#ifndef TPM_TPM_INT_H
-#define TPM_TPM_INT_H
+#ifndef BACKENDS_TPM_INT_H
+#define BACKENDS_TPM_INT_H
 
 #include "qemu/option.h"
 #include "sysemu/tpm.h"
@@ -85,4 +85,4 @@ typedef struct TPMSizedBuffer {
 
 void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
 
-#endif /* TPM_TPM_INT_H */
+#endif /* BACKENDS_TPM_INT_H */
diff --git a/hw/tpm/tpm_ioctl.h b/backends/tpm_ioctl.h
similarity index 100%
rename from hw/tpm/tpm_ioctl.h
rename to backends/tpm_ioctl.h
diff --git a/hw/tpm/tpm_emulator.c b/backends/tpm_emulator.c
similarity index 100%
rename from hw/tpm/tpm_emulator.c
rename to backends/tpm_emulator.c
diff --git a/hw/tpm/tpm_passthrough.c b/backends/tpm_passthrough.c
similarity index 100%
rename from hw/tpm/tpm_passthrough.c
rename to backends/tpm_passthrough.c
diff --git a/hw/tpm/tpm_util.c b/backends/tpm_util.c
similarity index 100%
rename from hw/tpm/tpm_util.c
rename to backends/tpm_util.c
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 64206a6a3b..70ca5bc7a9 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -36,6 +36,7 @@
 #include "sysemu/tpm_util.h"
 #include "tpm_ppi.h"
 #include "trace.h"
+#include "../backends/trace.h" /* FIXME TRACE_TPM_UTIL_SHOW_BUFFER */
 
 #include "tpm_tis.h"
 
diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
index c43ac4aef8..fd6a2a9aff 100644
--- a/tests/qtest/tpm-emu.c
+++ b/tests/qtest/tpm-emu.c
@@ -13,7 +13,7 @@
 #include "qemu/osdep.h"
 #include <glib/gstdio.h>
 
-#include "hw/tpm/tpm_ioctl.h"
+#include "backends/tpm_ioctl.h"
 #include "io/channel-socket.h"
 #include "qapi/error.h"
 #include "tpm-emu.h"
diff --git a/MAINTAINERS b/MAINTAINERS
index 3abe3faa4e..3a9425e3df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2385,7 +2385,7 @@ F: hw/tpm/*
 F: include/hw/acpi/tpm.h
 F: include/sysemu/tpm*
 F: qapi/tpm.json
-F: backends/tpm.c
+F: backends/tpm*.c
 F: tests/qtest/*tpm*
 T: git https://github.com/stefanberger/qemu-tpm.git tpm-next
 
diff --git a/backends/Kconfig b/backends/Kconfig
new file mode 100644
index 0000000000..4ac943957a
--- /dev/null
+++ b/backends/Kconfig
@@ -0,0 +1,14 @@
+config TPMDEV
+    bool
+    depends on TPM
+
+config TPM_PASSTHROUGH
+    bool
+    default y
+    # FIXME: should check for x86 host as well
+    depends on TPMDEV && LINUX
+
+config TPM_EMULATOR
+    bool
+    default y
+    depends on TPMDEV
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 28a847cd57..e3f244808e 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -2,6 +2,9 @@ common-obj-y += rng.o rng-egd.o rng-builtin.o
 common-obj-$(CONFIG_POSIX) += rng-random.o
 
 common-obj-$(CONFIG_TPM) += tpm.o
+common-obj-$(CONFIG_TPM) += tpm_util.o
+common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
+common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o
 
 common-obj-y += hostmem.o hostmem-ram.o
 common-obj-$(CONFIG_POSIX) += hostmem-file.o
diff --git a/backends/trace-events b/backends/trace-events
index 59058f7630..e6f7b3215b 100644
--- a/backends/trace-events
+++ b/backends/trace-events
@@ -5,3 +5,35 @@ dbus_vmstate_pre_save(void)
 dbus_vmstate_post_load(int version_id) "version_id: %d"
 dbus_vmstate_loading(const char *id) "id: %s"
 dbus_vmstate_saving(const char *id) "id: %s"
+
+# tpm_passthrough.c
+tpm_passthrough_handle_request(void *cmd) "processing command %p"
+tpm_passthrough_reset(void) "reset"
+
+# tpm_emulator.c
+tpm_emulator_set_locality(uint8_t locty) "setting locality to %d"
+tpm_emulator_handle_request(void) "processing TPM command"
+tpm_emulator_probe_caps(uint64_t caps) "capabilities: 0x%"PRIx64
+tpm_emulator_set_buffer_size(uint32_t buffersize, uint32_t minsize, uint32_t maxsize) "buffer size: %u, min: %u, max: %u"
+tpm_emulator_startup_tpm_resume(bool is_resume, size_t buffersize) "is_resume: %d, buffer size: %zu"
+tpm_emulator_get_tpm_established_flag(uint8_t flag) "got established flag: %d"
+tpm_emulator_cancel_cmd_not_supt(void) "Backend does not support CANCEL_TPM_CMD"
+tpm_emulator_handle_device_opts_tpm12(void) "TPM Version 1.2"
+tpm_emulator_handle_device_opts_tpm2(void) "TPM Version 2"
+tpm_emulator_handle_device_opts_unspec(void) "TPM Version Unspecified"
+tpm_emulator_handle_device_opts_startup_error(void) "Startup error"
+tpm_emulator_get_state_blob(uint8_t type, uint32_t size, uint32_t flags) "got state blob type %d, %u bytes, flags 0x%08x"
+tpm_emulator_set_state_blob(uint8_t type, uint32_t size, uint32_t flags) "set state blob type %d, %u bytes, flags 0x%08x"
+tpm_emulator_set_state_blobs(void) "setting state blobs"
+tpm_emulator_set_state_blobs_error(const char *msg) "error while setting state blobs: %s"
+tpm_emulator_set_state_blobs_done(void) "Done setting state blobs"
+tpm_emulator_pre_save(void) ""
+tpm_emulator_inst_init(void) ""
+
+# tpm_util.c
+tpm_util_get_buffer_size_hdr_len(uint32_t len, size_t expected) "tpm_resp->hdr.len = %u, expected = %zu"
+tpm_util_get_buffer_size_len(uint32_t len, size_t expected) "tpm_resp->len = %u, expected = %zu"
+tpm_util_get_buffer_size_hdr_len2(uint32_t len, size_t expected) "tpm2_resp->hdr.len = %u, expected = %zu"
+tpm_util_get_buffer_size_len2(uint32_t len, size_t expected) "tpm2_resp->len = %u, expected = %zu"
+tpm_util_get_buffer_size(size_t len) "buffersize of device: %zu"
+tpm_util_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\n%s"
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 4794e7fe28..3a9fd73a4b 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -1,7 +1,3 @@
-config TPMDEV
-    bool
-    depends on TPM
-
 config TPM_TIS_ISA
     bool
     depends on TPM && ISA_BUS
@@ -22,17 +18,6 @@ config TPM_CRB
     depends on TPM && PC
     select TPMDEV
 
-config TPM_PASSTHROUGH
-    bool
-    default y
-    # FIXME: should check for x86 host as well
-    depends on TPMDEV && LINUX
-
-config TPM_EMULATOR
-    bool
-    default y
-    depends on TPMDEV
-
 config TPM_SPAPR
     bool
     default y
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index f1ec4beb95..6fc05be67c 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,9 +1,6 @@
-common-obj-$(CONFIG_TPM) += tpm_util.o
 obj-$(call lor,$(CONFIG_TPM_TIS),$(CONFIG_TPM_CRB)) += tpm_ppi.o
 common-obj-$(CONFIG_TPM_TIS_ISA) += tpm_tis_isa.o
 common-obj-$(CONFIG_TPM_TIS_SYSBUS) += tpm_tis_sysbus.o
 common-obj-$(CONFIG_TPM_TIS) += tpm_tis_common.o
 common-obj-$(CONFIG_TPM_CRB) += tpm_crb.o
-common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
-common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o
 obj-$(CONFIG_TPM_SPAPR) += tpm_spapr.o
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 439e514787..de9bf1e01b 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -4,38 +4,6 @@
 tpm_crb_mmio_read(uint64_t addr, unsigned size, uint32_t val) "CRB read 0x%016" PRIx64 " len:%u val: 0x%" PRIx32
 tpm_crb_mmio_write(uint64_t addr, unsigned size, uint32_t val) "CRB write 0x%016" PRIx64 " len:%u val: 0x%" PRIx32
 
-# tpm_passthrough.c
-tpm_passthrough_handle_request(void *cmd) "processing command %p"
-tpm_passthrough_reset(void) "reset"
-
-# tpm_util.c
-tpm_util_get_buffer_size_hdr_len(uint32_t len, size_t expected) "tpm_resp->hdr.len = %u, expected = %zu"
-tpm_util_get_buffer_size_len(uint32_t len, size_t expected) "tpm_resp->len = %u, expected = %zu"
-tpm_util_get_buffer_size_hdr_len2(uint32_t len, size_t expected) "tpm2_resp->hdr.len = %u, expected = %zu"
-tpm_util_get_buffer_size_len2(uint32_t len, size_t expected) "tpm2_resp->len = %u, expected = %zu"
-tpm_util_get_buffer_size(size_t len) "buffersize of device: %zu"
-tpm_util_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\n%s"
-
-# tpm_emulator.c
-tpm_emulator_set_locality(uint8_t locty) "setting locality to %d"
-tpm_emulator_handle_request(void) "processing TPM command"
-tpm_emulator_probe_caps(uint64_t caps) "capabilities: 0x%"PRIx64
-tpm_emulator_set_buffer_size(uint32_t buffersize, uint32_t minsize, uint32_t maxsize) "buffer size: %u, min: %u, max: %u"
-tpm_emulator_startup_tpm_resume(bool is_resume, size_t buffersize) "is_resume: %d, buffer size: %zu"
-tpm_emulator_get_tpm_established_flag(uint8_t flag) "got established flag: %d"
-tpm_emulator_cancel_cmd_not_supt(void) "Backend does not support CANCEL_TPM_CMD"
-tpm_emulator_handle_device_opts_tpm12(void) "TPM Version 1.2"
-tpm_emulator_handle_device_opts_tpm2(void) "TPM Version 2"
-tpm_emulator_handle_device_opts_unspec(void) "TPM Version Unspecified"
-tpm_emulator_handle_device_opts_startup_error(void) "Startup error"
-tpm_emulator_get_state_blob(uint8_t type, uint32_t size, uint32_t flags) "got state blob type %d, %u bytes, flags 0x%08x"
-tpm_emulator_set_state_blob(uint8_t type, uint32_t size, uint32_t flags) "set state blob type %d, %u bytes, flags 0x%08x"
-tpm_emulator_set_state_blobs(void) "setting state blobs"
-tpm_emulator_set_state_blobs_error(const char *msg) "error while setting state blobs: %s"
-tpm_emulator_set_state_blobs_done(void) "Done setting state blobs"
-tpm_emulator_pre_save(void) ""
-tpm_emulator_inst_init(void) ""
-
 # tpm_tis.c
 tpm_tis_raise_irq(uint32_t irqmask) "Raising IRQ for flag 0x%08x"
 tpm_tis_new_active_locality(uint8_t locty) "Active locality is now %d"
@@ -56,7 +24,7 @@ tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
 # tpm_ppi.c
 tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
 
-# hw/tpm/tpm_spapr.c
+# tpm_spapr.c
 tpm_spapr_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\n%s"
 tpm_spapr_do_crq(uint8_t raw1, uint8_t raw2) "1st 2 bytes in CRQ: 0x%02x 0x%02x"
 tpm_spapr_do_crq_crq_result(void) "SPAPR_VTPM_INIT_CRQ_RESULT"
-- 
2.21.3

Re: [PATCH 0/8] tpm: Split hw/ vs backends/

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

Hi

On Thu, Jun 11, 2020 at 12:03 AM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Hi,
>
> Today I started to review some vTPM patches and got very
> confused by the files under hw/tpm/. In particular after
> running:
>
>   $ git grep TYPE_TPM_BACKEND
>   backends/tpm.c:188:    .name = TYPE_TPM_BACKEND,
>   hw/tpm/tpm_emulator.c:985:    .parent = TYPE_TPM_BACKEND,
>   hw/tpm/tpm_passthrough.c:393:    .parent = TYPE_TPM_BACKEND,
>   include/sysemu/tpm_backend.h:21:#define TYPE_TPM_BACKEND "tpm-backend"
>
> As there is no particular reason to keep this mixed, clean it up.
>

While at it, why not start to organize backends/ ?  I would move that under
backends/tpm/


> Philippe Mathieu-Daudé (8):
>   hw/tpm: Do not include 'qemu/osdep.h' in header
>   hw/tpm: Include missing 'qemu/option.h' header
>   hw/tpm: Move 'hw/acpi/tpm.h' inclusion from header to sources
>   hw/tpm: Remove unnecessary 'tpm_int.h' header inclusion
>   hw/tpm: Move few declarations from 'tpm_util.h' to 'tpm_int.h'
>   hw/tpm: Move DEFINE_PROP_TPMBE() macro to 'tmp_prop.h' local header
>   hw/tpm: Make 'tpm_util.h' publicly accessible as "sysemu/tpm_util.h"
>   tpm: Move backend code under the 'backends/' directory
>
>  Makefile                               |  2 +-
>  {hw/tpm => backends}/tpm_int.h         | 19 +++++++++++---
>  {hw/tpm => backends}/tpm_ioctl.h       |  0
>  hw/tpm/tpm_ppi.h                       |  1 -
>  hw/tpm/tpm_prop.h                      | 31 +++++++++++++++++++++++
>  hw/tpm/tpm_tis.h                       |  1 -
>  {hw/tpm => include/sysemu}/tpm_util.h  | 19 +++-----------
>  {hw/tpm => backends}/tpm_emulator.c    |  2 +-
>  {hw/tpm => backends}/tpm_passthrough.c |  2 +-
>  {hw/tpm => backends}/tpm_util.c        |  2 +-
>  hw/tpm/tpm_crb.c                       |  4 +--
>  hw/tpm/tpm_spapr.c                     |  4 +--
>  hw/tpm/tpm_tis_common.c                |  4 +--
>  hw/tpm/tpm_tis_isa.c                   |  3 ++-
>  hw/tpm/tpm_tis_sysbus.c                |  3 ++-
>  tests/qtest/tpm-emu.c                  |  2 +-
>  MAINTAINERS                            |  2 +-
>  backends/Kconfig                       | 14 +++++++++++
>  backends/Makefile.objs                 |  3 +++
>  backends/trace-events                  | 32 ++++++++++++++++++++++++
>  hw/tpm/Kconfig                         | 15 ------------
>  hw/tpm/Makefile.objs                   |  3 ---
>  hw/tpm/trace-events                    | 34 +-------------------------
>  23 files changed, 116 insertions(+), 86 deletions(-)
>  rename {hw/tpm => backends}/tpm_int.h (81%)
>  rename {hw/tpm => backends}/tpm_ioctl.h (100%)
>  create mode 100644 hw/tpm/tpm_prop.h
>  rename {hw/tpm => include/sysemu}/tpm_util.h (80%)
>  rename {hw/tpm => backends}/tpm_emulator.c (99%)
>  rename {hw/tpm => backends}/tpm_passthrough.c (99%)
>  rename {hw/tpm => backends}/tpm_util.c (99%)
>  create mode 100644 backends/Kconfig
>
> --
> 2.21.3
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 4093 bytes --]

Re: [PATCH 0/8] tpm: Split hw/ vs backends/

[Philippe Mathieu-Daudé]

On 6/10/20 10:02 PM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> Today I started to review some vTPM patches and got very
> confused by the files under hw/tpm/. In particular after
> running:
> 
>   $ git grep TYPE_TPM_BACKEND
>   backends/tpm.c:188:    .name = TYPE_TPM_BACKEND,
>   hw/tpm/tpm_emulator.c:985:    .parent = TYPE_TPM_BACKEND,
>   hw/tpm/tpm_passthrough.c:393:    .parent = TYPE_TPM_BACKEND,
>   include/sysemu/tpm_backend.h:21:#define TYPE_TPM_BACKEND "tpm-backend"
> 
> As there is no particular reason to keep this mixed, clean it up.
> 
> Philippe Mathieu-Daudé (8):
>   hw/tpm: Do not include 'qemu/osdep.h' in header
>   hw/tpm: Include missing 'qemu/option.h' header
>   hw/tpm: Move 'hw/acpi/tpm.h' inclusion from header to sources
>   hw/tpm: Remove unnecessary 'tpm_int.h' header inclusion
>   hw/tpm: Move few declarations from 'tpm_util.h' to 'tpm_int.h'
>   hw/tpm: Move DEFINE_PROP_TPMBE() macro to 'tmp_prop.h' local header
>   hw/tpm: Make 'tpm_util.h' publicly accessible as "sysemu/tpm_util.h"
>   tpm: Move backend code under the 'backends/' directory
> 
>  Makefile                               |  2 +-
>  {hw/tpm => backends}/tpm_int.h         | 19 +++++++++++---
>  {hw/tpm => backends}/tpm_ioctl.h       |  0
>  hw/tpm/tpm_ppi.h                       |  1 -
>  hw/tpm/tpm_prop.h                      | 31 +++++++++++++++++++++++
>  hw/tpm/tpm_tis.h                       |  1 -
>  {hw/tpm => include/sysemu}/tpm_util.h  | 19 +++-----------
>  {hw/tpm => backends}/tpm_emulator.c    |  2 +-
>  {hw/tpm => backends}/tpm_passthrough.c |  2 +-
>  {hw/tpm => backends}/tpm_util.c        |  2 +-
>  hw/tpm/tpm_crb.c                       |  4 +--
>  hw/tpm/tpm_spapr.c                     |  4 +--
>  hw/tpm/tpm_tis_common.c                |  4 +--
>  hw/tpm/tpm_tis_isa.c                   |  3 ++-
>  hw/tpm/tpm_tis_sysbus.c                |  3 ++-
>  tests/qtest/tpm-emu.c                  |  2 +-
>  MAINTAINERS                            |  2 +-
>  backends/Kconfig                       | 14 +++++++++++
>  backends/Makefile.objs                 |  3 +++
>  backends/trace-events                  | 32 ++++++++++++++++++++++++
>  hw/tpm/Kconfig                         | 15 ------------
>  hw/tpm/Makefile.objs                   |  3 ---
>  hw/tpm/trace-events                    | 34 +-------------------------
>  23 files changed, 116 insertions(+), 86 deletions(-)
>  rename {hw/tpm => backends}/tpm_int.h (81%)
>  rename {hw/tpm => backends}/tpm_ioctl.h (100%)
>  create mode 100644 hw/tpm/tpm_prop.h
>  rename {hw/tpm => include/sysemu}/tpm_util.h (80%)
>  rename {hw/tpm => backends}/tpm_emulator.c (99%)
>  rename {hw/tpm => backends}/tpm_passthrough.c (99%)
>  rename {hw/tpm => backends}/tpm_util.c (99%)
>  create mode 100644 backends/Kconfig

Beh I forgot to commit the docs/ changes...

-- >8 --
diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 5e61238bc5..546109ebf7 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -199,8 +199,8 @@ to be used with the passthrough backend or the swtpm
backend.

 QEMU files related to TPM backends:
  - ``backends/tpm.c``
+ - ``include/sysemu/tpm.h``
  - ``include/sysemu/tpm_backend.h``
- - ``include/sysemu/tpm_backend_int.h``

 The QEMU TPM passthrough device
 -------------------------------
@@ -232,9 +232,9 @@ Integrity Measurement Architecture (IMA), are not
expecting to share
 PCRs.

 QEMU files related to the TPM passthrough device:
- - ``hw/tpm/tpm_passthrough.c``
- - ``hw/tpm/tpm_util.c``
- - ``hw/tpm/tpm_util.h``
+ - ``backends/tpm_passthrough.c``
+ - ``backends/tpm_util.c``
+ - ``include/sysemu/tpm_util.h``


 Command line to start QEMU with the TPM passthrough device using the host's
@@ -292,9 +292,9 @@ instrumented to initialize a TPM 1.2 or TPM 2 device
using this
 command.

 QEMU files related to the TPM emulator device:
- - ``hw/tpm/tpm_emulator.c``
- - ``hw/tpm/tpm_util.c``
- - ``hw/tpm/tpm_util.h``
+ - ``backends/tpm_emulator.c``
+ - ``backends/tpm_util.c``
+ - ``include/sysemu/tpm_util.h``

 The following commands start the swtpm with a UnixIO control channel over
 a socket interface. They do not need to be run as root.

---

I'll wait for some review before respining.

Re: [PATCH 0/8] tpm: Split hw/ vs backends/

[Philippe Mathieu-Daudé]

On 6/10/20 10:09 PM, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jun 11, 2020 at 12:03 AM Philippe Mathieu-Daudé
> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> 
>     Hi,
> 
>     Today I started to review some vTPM patches and got very
>     confused by the files under hw/tpm/. In particular after
>     running:
> 
>       $ git grep TYPE_TPM_BACKEND
>       backends/tpm.c:188:    .name = TYPE_TPM_BACKEND,
>       hw/tpm/tpm_emulator.c:985:    .parent = TYPE_TPM_BACKEND,
>       hw/tpm/tpm_passthrough.c:393:    .parent = TYPE_TPM_BACKEND,
>       include/sysemu/tpm_backend.h:21:#define TYPE_TPM_BACKEND "tpm-backend"
> 
>     As there is no particular reason to keep this mixed, clean it up.
> 
> 
> While at it, why not start to organize backends/ ?  I would move that
> under backends/tpm/

Sure, if the overall of this series looks good, I can do that.
I'll wait for positive/negative feedback first.

> 
> 
>     Philippe Mathieu-Daudé (8):
>       hw/tpm: Do not include 'qemu/osdep.h' in header
>       hw/tpm: Include missing 'qemu/option.h' header
>       hw/tpm: Move 'hw/acpi/tpm.h' inclusion from header to sources
>       hw/tpm: Remove unnecessary 'tpm_int.h' header inclusion
>       hw/tpm: Move few declarations from 'tpm_util.h' to 'tpm_int.h'
>       hw/tpm: Move DEFINE_PROP_TPMBE() macro to 'tmp_prop.h' local header
>       hw/tpm: Make 'tpm_util.h' publicly accessible as "sysemu/tpm_util.h"
>       tpm: Move backend code under the 'backends/' directory
> 
>      Makefile                               |  2 +-
>      {hw/tpm => backends}/tpm_int.h         | 19 +++++++++++---
>      {hw/tpm => backends}/tpm_ioctl.h       |  0
>      hw/tpm/tpm_ppi.h                       |  1 -
>      hw/tpm/tpm_prop.h                      | 31 +++++++++++++++++++++++
>      hw/tpm/tpm_tis.h                       |  1 -
>      {hw/tpm => include/sysemu}/tpm_util.h  | 19 +++-----------
>      {hw/tpm => backends}/tpm_emulator.c    |  2 +-
>      {hw/tpm => backends}/tpm_passthrough.c |  2 +-
>      {hw/tpm => backends}/tpm_util.c        |  2 +-
>      hw/tpm/tpm_crb.c                       |  4 +--
>      hw/tpm/tpm_spapr.c                     |  4 +--
>      hw/tpm/tpm_tis_common.c                |  4 +--
>      hw/tpm/tpm_tis_isa.c                   |  3 ++-
>      hw/tpm/tpm_tis_sysbus.c                |  3 ++-
>      tests/qtest/tpm-emu.c                  |  2 +-
>      MAINTAINERS                            |  2 +-
>      backends/Kconfig                       | 14 +++++++++++
>      backends/Makefile.objs                 |  3 +++
>      backends/trace-events                  | 32 ++++++++++++++++++++++++
>      hw/tpm/Kconfig                         | 15 ------------
>      hw/tpm/Makefile.objs                   |  3 ---
>      hw/tpm/trace-events                    | 34 +-------------------------
>      23 files changed, 116 insertions(+), 86 deletions(-)
>      rename {hw/tpm => backends}/tpm_int.h (81%)
>      rename {hw/tpm => backends}/tpm_ioctl.h (100%)
>      create mode 100644 hw/tpm/tpm_prop.h
>      rename {hw/tpm => include/sysemu}/tpm_util.h (80%)
>      rename {hw/tpm => backends}/tpm_emulator.c (99%)
>      rename {hw/tpm => backends}/tpm_passthrough.c (99%)
>      rename {hw/tpm => backends}/tpm_util.c (99%)
>      create mode 100644 backends/Kconfig
> 
>     -- 
>     2.21.3
> 
> 
> 
> 
> -- 
> Marc-André Lureau

Re: [PATCH 0/8] tpm: Split hw/ vs backends/

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200610200247.21378-1-philmd@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      x86_64-softmmu/hw/virtio/vhost-user.o
In file included from /tmp/qemu-test/src/hw/tpm/tpm_ppi.c:14:0:
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c: In function 'tpm_ppi_init':
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c:47:49: error: 'TPM_PPI_ADDR_SIZE' undeclared (first use in this function)
                                 HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
                                                 ^
/tmp/qemu-test/src/include/qemu/osdep.h:278:27: note: in definition of macro 'ROUND_UP'
---
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c:47:33: note: in expansion of macro 'HOST_PAGE_ALIGN'
                                 HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
                                 ^
make[1]: *** [hw/tpm/tpm_ppi.o] Error 1
make[1]: *** Waiting for unfinished jobs....
  CC      x86_64-softmmu/hw/virtio/virtio-crypto.o
  CC      x86_64-softmmu/hw/virtio/virtio-balloon.o
---
  CC      x86_64-softmmu/hw/virtio/vhost-vsock-pci.o
  CC      x86_64-softmmu/hw/virtio/vhost-vsock.o
  CC      x86_64-softmmu/hw/virtio/vhost-user-blk-pci.o
make: *** [aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  CC      x86_64-softmmu/hw/virtio/vhost-user-input-pci.o
  CC      x86_64-softmmu/hw/virtio/vhost-user-scsi-pci.o
---
  CC      x86_64-softmmu/hw/i386/pc_q35.o
In file included from /tmp/qemu-test/src/hw/tpm/tpm_ppi.c:14:0:
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c: In function 'tpm_ppi_init':
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c:47:49: error: 'TPM_PPI_ADDR_SIZE' undeclared (first use in this function)
                                 HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
                                                 ^
/tmp/qemu-test/src/include/qemu/osdep.h:278:27: note: in definition of macro 'ROUND_UP'
---
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c:47:33: note: in expansion of macro 'HOST_PAGE_ALIGN'
                                 HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
                                 ^
make[1]: *** [hw/tpm/tpm_ppi.o] Error 1
make[1]: *** Waiting for unfinished jobs....
  CC      x86_64-softmmu/hw/i386/microvm.o
  CC      x86_64-softmmu/hw/i386/fw_cfg.o
make: *** [x86_64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 665, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=1006c01dc5a7450baddc2f02dba11b1d', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-4syr9jsb/src/docker-src.2020-06-10-17.23.57.23699:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=1006c01dc5a7450baddc2f02dba11b1d
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4syr9jsb/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m46.752s
user    0m8.110s


The full log is available at
http://patchew.org/logs/20200610200247.21378-1-philmd@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 0/8] tpm: Split hw/ vs backends/

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200610200247.21378-1-philmd@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      x86_64-softmmu/qapi/qapi-visit.o
  CC      x86_64-softmmu/qapi/qapi-events-machine-target.o
  CC      x86_64-softmmu/qapi/qapi-events-misc-target.o
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c:47:49: error: use of undeclared identifier 'TPM_PPI_ADDR_SIZE'
                                HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
                                                ^
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c:47:49: error: use of undeclared identifier 'TPM_PPI_ADDR_SIZE'
/tmp/qemu-test/src/hw/tpm/tpm_ppi.c:49:39: error: use of undeclared identifier 'TPM_PPI_ADDR_SIZE'
                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);
                                      ^
3 errors generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: hw/tpm/tpm_ppi.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:527: x86_64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 665, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=f66fe6459f56422185999e40bcbe2abb', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-b4vqotdt/src/docker-src.2020-06-10-17.27.35.514:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=f66fe6459f56422185999e40bcbe2abb
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-b4vqotdt/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    3m55.750s
user    0m7.893s


The full log is available at
http://patchew.org/logs/20200610200247.21378-1-philmd@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 1/8] hw/tpm: Do not include 'qemu/osdep.h' in header

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
>  From CODING_STYLE.rst:
>
>    Do not include "qemu/osdep.h" from header files since the .c
>    file will have already included it.
>
> Remove "qemu/osdep.h" from "tpm_tis.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>


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


> ---
>   hw/tpm/tpm_tis.h | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
> index 5554989395..f6b5872ba6 100644
> --- a/hw/tpm/tpm_tis.h
> +++ b/hw/tpm/tpm_tis.h
> @@ -24,7 +24,6 @@
>   #ifndef TPM_TPM_TIS_H
>   #define TPM_TPM_TIS_H
>   
> -#include "qemu/osdep.h"
>   #include "sysemu/tpm_backend.h"
>   #include "tpm_ppi.h"
>   

Re: [PATCH 2/8] hw/tpm: Include missing 'qemu/option.h' header

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
> Files using the TPM_STANDARD_CMDLINE_OPTS macro declared in
> "tpm_int.h" will use QEMU_OPT_STRING definition declared in
> "qemu/option.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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


> ---
>   hw/tpm/tpm_int.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> index 3fb28a9d6c..fd5ebc6489 100644
> --- a/hw/tpm/tpm_int.h
> +++ b/hw/tpm/tpm_int.h
> @@ -12,6 +12,8 @@
>   #ifndef TPM_TPM_INT_H
>   #define TPM_TPM_INT_H
>   
> +#include "qemu/option.h"
> +
>   #define TPM_STANDARD_CMDLINE_OPTS \
>       { \
>           .name = "type", \

Re: [PATCH 3/8] hw/tpm: Move 'hw/acpi/tpm.h' inclusion from header to sources

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
> Nothing in "tpm_ppi.h" require declarations from "hw/acpi/tpm.h".
> Reduce dependencies and include it only in the files requiring it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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


> ---
>   hw/tpm/tpm_ppi.h        | 1 -
>   hw/tpm/tpm_tis_isa.c    | 1 +
>   hw/tpm/tpm_tis_sysbus.c | 1 +
>   3 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> index d33ef27de6..6f773c25a0 100644
> --- a/hw/tpm/tpm_ppi.h
> +++ b/hw/tpm/tpm_ppi.h
> @@ -12,7 +12,6 @@
>   #ifndef TPM_TPM_PPI_H
>   #define TPM_TPM_PPI_H
>   
> -#include "hw/acpi/tpm.h"
>   #include "exec/address-spaces.h"
>   
>   typedef struct TPMPPI {
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index 30ba37079d..42f909ff1e 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -26,6 +26,7 @@
>   #include "hw/isa/isa.h"
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
> +#include "hw/acpi/tpm.h"
>   #include "tpm_util.h"
>   #include "tpm_tis.h"
>   
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index eced1fc843..edca1dae0d 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -25,6 +25,7 @@
>   #include "qemu/osdep.h"
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
> +#include "hw/acpi/tpm.h"
>   #include "tpm_util.h"
>   #include "hw/sysbus.h"
>   #include "tpm_tis.h"

Re: [PATCH 4/8] hw/tpm: Remove unnecessary 'tpm_int.h' header inclusion

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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


> ---
>   hw/tpm/tpm_crb.c        | 1 -
>   hw/tpm/tpm_spapr.c      | 1 -
>   hw/tpm/tpm_tis_common.c | 1 -
>   3 files changed, 3 deletions(-)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index cd004e7f8e..664ff70ef9 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -25,7 +25,6 @@
>   #include "migration/vmstate.h"
>   #include "sysemu/tpm_backend.h"
>   #include "sysemu/reset.h"
> -#include "tpm_int.h"
>   #include "tpm_util.h"
>   #include "tpm_ppi.h"
>   #include "trace.h"
> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
> index ce65eb2e45..ab1a86ad6e 100644
> --- a/hw/tpm/tpm_spapr.c
> +++ b/hw/tpm/tpm_spapr.c
> @@ -20,7 +20,6 @@
>   #include "migration/vmstate.h"
>   
>   #include "sysemu/tpm_backend.h"
> -#include "tpm_int.h"
>   #include "tpm_util.h"
>   
>   #include "hw/ppc/spapr.h"
> diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
> index 1af4bce139..94704870f6 100644
> --- a/hw/tpm/tpm_tis_common.c
> +++ b/hw/tpm/tpm_tis_common.c
> @@ -33,7 +33,6 @@
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
>   #include "sysemu/tpm_backend.h"
> -#include "tpm_int.h"
>   #include "tpm_util.h"
>   #include "tpm_ppi.h"
>   #include "trace.h"

Re: [PATCH 5/8] hw/tpm: Move few declarations from 'tpm_util.h' to 'tpm_int.h'

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
> We are going to make "tpm_util.h" publicly accessible by
> moving it to the include/ directory in a pair of commits.
> Keep declarations internals to hw/tpm/ in "tpm_int.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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


> ---
>   hw/tpm/tpm_int.h  | 11 +++++++++++
>   hw/tpm/tpm_util.h | 10 ----------
>   2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> index fd5ebc6489..9f72879d89 100644
> --- a/hw/tpm/tpm_int.h
> +++ b/hw/tpm/tpm_int.h
> @@ -13,6 +13,7 @@
>   #define TPM_TPM_INT_H
>   
>   #include "qemu/option.h"
> +#include "sysemu/tpm.h"
>   
>   #define TPM_STANDARD_CMDLINE_OPTS \
>       { \
> @@ -74,4 +75,14 @@ struct tpm_resp_hdr {
>   #define TPM_RC_FAILURE            0x101
>   #define TPM_RC_LOCALITY           0x907
>   
> +int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> +                             size_t *buffersize);
> +
> +typedef struct TPMSizedBuffer {
> +    uint32_t size;
> +    uint8_t  *buffer;
> +} TPMSizedBuffer;
> +
> +void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
> +
>   #endif /* TPM_TPM_INT_H */
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index 7889081fba..d524935576 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -66,19 +66,9 @@ 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);
> -
>   #define DEFINE_PROP_TPMBE(_n, _s, _f)                     \
>       DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *)
>   
> -typedef struct TPMSizedBuffer {
> -    uint32_t size;
> -    uint8_t  *buffer;
> -} TPMSizedBuffer;
> -
> -void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
> -
>   void tpm_util_show_buffer(const unsigned char *buffer,
>                             size_t buffer_size, const char *string);
>   

Re: [PATCH 6/8] hw/tpm: Move DEFINE_PROP_TPMBE() macro to 'tmp_prop.h' local header

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
> We are going to make "tpm_util.h" publicly accessible by
> moving it to the include/ directory in the next commit.
> The DEFINE_PROP_TPMBE() macro is only meaningful for the
> TPM hardware files (in hw/tpm/), so keep this macro in a
> local header.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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


> ---
>   hw/tpm/tpm_prop.h       | 31 +++++++++++++++++++++++++++++++
>   hw/tpm/tpm_util.h       |  3 ---
>   hw/tpm/tpm_crb.c        |  1 +
>   hw/tpm/tpm_spapr.c      |  1 +
>   hw/tpm/tpm_tis_isa.c    |  2 +-
>   hw/tpm/tpm_tis_sysbus.c |  2 +-
>   6 files changed, 35 insertions(+), 5 deletions(-)
>   create mode 100644 hw/tpm/tpm_prop.h
>
> diff --git a/hw/tpm/tpm_prop.h b/hw/tpm/tpm_prop.h
> new file mode 100644
> index 0000000000..85e1ae5718
> --- /dev/null
> +++ b/hw/tpm/tpm_prop.h
> @@ -0,0 +1,31 @@
> +/*
> + * TPM utility functions
> + *
> + *  Copyright (c) 2010 - 2015 IBM Corporation
> + *  Authors:
> + *    Stefan Berger <stefanb@us.ibm.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> + */
> +
> +#ifndef HW_TPM_PROP_H
> +#define HW_TPM_PROP_H
> +
> +#include "sysemu/tpm_backend.h"
> +#include "hw/qdev-properties.h"
> +
> +#define DEFINE_PROP_TPMBE(_n, _s, _f)                     \
> +    DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *)
> +
> +#endif /* HW_TPM_PROP_H */
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index d524935576..cf61d830d7 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -66,9 +66,6 @@ static inline void tpm_cmd_set_error(void *b, uint32_t error)
>       stl_be_p(b + 6, error);
>   }
>   
> -#define DEFINE_PROP_TPMBE(_n, _s, _f)                     \
> -    DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *)
> -
>   void tpm_util_show_buffer(const unsigned char *buffer,
>                             size_t buffer_size, const char *string);
>   
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 664ff70ef9..1cac4d671d 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -25,6 +25,7 @@
>   #include "migration/vmstate.h"
>   #include "sysemu/tpm_backend.h"
>   #include "sysemu/reset.h"
> +#include "tpm_prop.h"
>   #include "tpm_util.h"
>   #include "tpm_ppi.h"
>   #include "trace.h"
> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
> index ab1a86ad6e..65672048c7 100644
> --- a/hw/tpm/tpm_spapr.c
> +++ b/hw/tpm/tpm_spapr.c
> @@ -21,6 +21,7 @@
>   
>   #include "sysemu/tpm_backend.h"
>   #include "tpm_util.h"
> +#include "tpm_prop.h"
>   
>   #include "hw/ppc/spapr.h"
>   #include "hw/ppc/spapr_vio.h"
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index 42f909ff1e..5faf6231c0 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -27,7 +27,7 @@
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
>   #include "hw/acpi/tpm.h"
> -#include "tpm_util.h"
> +#include "tpm_prop.h"
>   #include "tpm_tis.h"
>   
>   typedef struct TPMStateISA {
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index edca1dae0d..4a3bc70625 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -26,7 +26,7 @@
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
>   #include "hw/acpi/tpm.h"
> -#include "tpm_util.h"
> +#include "tpm_prop.h"
>   #include "hw/sysbus.h"
>   #include "tpm_tis.h"
>   

Re: [PATCH 7/8] hw/tpm: Make 'tpm_util.h' publicly accessible as "sysemu/tpm_util.h"

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
> We are going to split the TPM backends from the TPM emulated
> hardware in the next commit. Make the TPM util helpers accessible
> by moving local "tpm_util.h" to global "sysemu/tpm_util.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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


> ---
>   {hw/tpm => include/sysemu}/tpm_util.h | 6 +++---
>   hw/tpm/tpm_crb.c                      | 2 +-
>   hw/tpm/tpm_emulator.c                 | 2 +-
>   hw/tpm/tpm_passthrough.c              | 2 +-
>   hw/tpm/tpm_spapr.c                    | 2 +-
>   hw/tpm/tpm_tis_common.c               | 2 +-
>   hw/tpm/tpm_util.c                     | 2 +-
>   7 files changed, 9 insertions(+), 9 deletions(-)
>   rename {hw/tpm => include/sysemu}/tpm_util.h (95%)
>
> diff --git a/hw/tpm/tpm_util.h b/include/sysemu/tpm_util.h
> similarity index 95%
> rename from hw/tpm/tpm_util.h
> rename to include/sysemu/tpm_util.h
> index cf61d830d7..63e872c3b2 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/include/sysemu/tpm_util.h
> @@ -19,8 +19,8 @@
>    * License along with this library; if not, see <http://www.gnu.org/licenses/>
>    */
>   
> -#ifndef TPM_TPM_UTIL_H
> -#define TPM_TPM_UTIL_H
> +#ifndef SYSEMU_TPM_UTIL_H
> +#define SYSEMU_TPM_UTIL_H
>   
>   #include "sysemu/tpm.h"
>   #include "qemu/bswap.h"
> @@ -69,4 +69,4 @@ static inline void tpm_cmd_set_error(void *b, uint32_t error)
>   void tpm_util_show_buffer(const unsigned char *buffer,
>                             size_t buffer_size, const char *string);
>   
> -#endif /* TPM_TPM_UTIL_H */
> +#endif /* SYSEMU_TPM_UTIL_H */
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 1cac4d671d..60247295d4 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -24,9 +24,9 @@
>   #include "hw/acpi/tpm.h"
>   #include "migration/vmstate.h"
>   #include "sysemu/tpm_backend.h"
> +#include "sysemu/tpm_util.h"
>   #include "sysemu/reset.h"
>   #include "tpm_prop.h"
> -#include "tpm_util.h"
>   #include "tpm_ppi.h"
>   #include "trace.h"
>   
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 3a0fc442f3..9605339f93 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -32,8 +32,8 @@
>   #include "qemu/sockets.h"
>   #include "io/channel-socket.h"
>   #include "sysemu/tpm_backend.h"
> +#include "sysemu/tpm_util.h"
>   #include "tpm_int.h"
> -#include "tpm_util.h"
>   #include "tpm_ioctl.h"
>   #include "migration/blocker.h"
>   #include "migration/vmstate.h"
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index f67244b5d4..7403807ec4 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -28,10 +28,10 @@
>   #include "qemu/module.h"
>   #include "qemu/sockets.h"
>   #include "sysemu/tpm_backend.h"
> +#include "sysemu/tpm_util.h"
>   #include "tpm_int.h"
>   #include "qapi/clone-visitor.h"
>   #include "qapi/qapi-visit-tpm.h"
> -#include "tpm_util.h"
>   #include "trace.h"
>   
>   #define TYPE_TPM_PASSTHROUGH "tpm-passthrough"
> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
> index 65672048c7..cb4dfd1e6a 100644
> --- a/hw/tpm/tpm_spapr.c
> +++ b/hw/tpm/tpm_spapr.c
> @@ -20,7 +20,7 @@
>   #include "migration/vmstate.h"
>   
>   #include "sysemu/tpm_backend.h"
> -#include "tpm_util.h"
> +#include "sysemu/tpm_util.h"
>   #include "tpm_prop.h"
>   
>   #include "hw/ppc/spapr.h"
> diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
> index 94704870f6..64206a6a3b 100644
> --- a/hw/tpm/tpm_tis_common.c
> +++ b/hw/tpm/tpm_tis_common.c
> @@ -33,7 +33,7 @@
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
>   #include "sysemu/tpm_backend.h"
> -#include "tpm_util.h"
> +#include "sysemu/tpm_util.h"
>   #include "tpm_ppi.h"
>   #include "trace.h"
>   
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index c0a0f3d71f..d0ec2a8235 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -23,11 +23,11 @@
>   #include "qemu/error-report.h"
>   #include "qapi/error.h"
>   #include "qapi/visitor.h"
> -#include "tpm_util.h"
>   #include "tpm_int.h"
>   #include "exec/memory.h"
>   #include "hw/qdev-properties.h"
>   #include "sysemu/tpm_backend.h"
> +#include "sysemu/tpm_util.h"
>   #include "trace.h"
>   
>   /* tpm backend property */

Re: [RFC PATCH 8/8] tpm: Move backend code under the 'backends/' directory

[Stefan Berger]

On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
> TPM subsytem is split into backends (see commit f4ede81eed2)
> and frontends (see i.e. 3676bc69b35). Keep the emulated
> hardware 'frontends' under hw/tpm/, but move the backends
> in the backends/ directory.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> RFC due to a FIXME in tpm_tis_common.c, it uses
> TRACE_TPM_UTIL_SHOW_BUFFER which is now generated
> by backends/trace-events...
> ---
>   Makefile                               |  2 +-
>   {hw/tpm => backends}/tpm_int.h         |  6 ++---
>   {hw/tpm => backends}/tpm_ioctl.h       |  0
>   {hw/tpm => backends}/tpm_emulator.c    |  0
>   {hw/tpm => backends}/tpm_passthrough.c |  0
>   {hw/tpm => backends}/tpm_util.c        |  0

I don't understand this move. Why not keep everything TPM related in one 
directory even though there may be a backend directory where 'nothing 
else fits but the name.' All we need to remember is that 'emulator' and 
'passthrough' are the backends.

Re: [RFC PATCH 8/8] tpm: Move backend code under the 'backends/' directory

[Thomas Huth]

On 11/06/2020 14.00, Stefan Berger wrote:
> On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
>> TPM subsytem is split into backends (see commit f4ede81eed2)
>> and frontends (see i.e. 3676bc69b35). Keep the emulated
>> hardware 'frontends' under hw/tpm/, but move the backends
>> in the backends/ directory.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> RFC due to a FIXME in tpm_tis_common.c, it uses
>> TRACE_TPM_UTIL_SHOW_BUFFER which is now generated
>> by backends/trace-events...
>> ---
>>   Makefile                               |  2 +-
>>   {hw/tpm => backends}/tpm_int.h         |  6 ++---
>>   {hw/tpm => backends}/tpm_ioctl.h       |  0
>>   {hw/tpm => backends}/tpm_emulator.c    |  0
>>   {hw/tpm => backends}/tpm_passthrough.c |  0
>>   {hw/tpm => backends}/tpm_util.c        |  0
> 
> I don't understand this move. Why not keep everything TPM related in one
> directory even though there may be a backend directory where 'nothing
> else fits but the name.' All we need to remember is that 'emulator' and
> 'passthrough' are the backends.

We try to have a clean separation between frontends and backends in
QEMU. The concepts have been mixed in the past (see e.g. the -drive
parameter) and that led only to confusion and trouble later. The hw/
directory is clearly for emulated hardware device frontends only, we
should not include any backend code here.

 Thomas

Re: [RFC PATCH 8/8] tpm: Move backend code under the 'backends/' directory

[Stefan Berger]

On 6/11/20 8:05 AM, Thomas Huth wrote:
> On 11/06/2020 14.00, Stefan Berger wrote:
>> On 6/10/20 4:02 PM, Philippe Mathieu-Daudé wrote:
>>> TPM subsytem is split into backends (see commit f4ede81eed2)
>>> and frontends (see i.e. 3676bc69b35). Keep the emulated
>>> hardware 'frontends' under hw/tpm/, but move the backends
>>> in the backends/ directory.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> RFC due to a FIXME in tpm_tis_common.c, it uses
>>> TRACE_TPM_UTIL_SHOW_BUFFER which is now generated
>>> by backends/trace-events...
>>> ---
>>>    Makefile                               |  2 +-
>>>    {hw/tpm => backends}/tpm_int.h         |  6 ++---
>>>    {hw/tpm => backends}/tpm_ioctl.h       |  0
>>>    {hw/tpm => backends}/tpm_emulator.c    |  0
>>>    {hw/tpm => backends}/tpm_passthrough.c |  0
>>>    {hw/tpm => backends}/tpm_util.c        |  0
>> I don't understand this move. Why not keep everything TPM related in one
>> directory even though there may be a backend directory where 'nothing
>> else fits but the name.' All we need to remember is that 'emulator' and
>> 'passthrough' are the backends.
> We try to have a clean separation between frontends and backends in
> QEMU. The concepts have been mixed in the past (see e.g. the -drive
> parameter) and that led only to confusion and trouble later. The hw/
> directory is clearly for emulated hardware device frontends only, we
> should not include any backend code here.

Then move it into backends/tpm/ as Marc-Andre suggested.

>
>   Thomas
>