Re: [PATCH v4 10/10] tests: Use QMP to check whether a TPM device model is available

[Philippe Mathieu-Daudé]

Hi Stefan,

On 7/12/21 10:47 PM, Stefan Berger wrote:
> Use QMP to check whether a given TPM device model is available
> and if it is not the case then skip a test that requires it.
> 
> Signed-off-by: Stefan Berger 
> ---
>  tests/qtest/bios-tables-test.c | 14 +++--
>  tests/qtest/tpm-emu.c  | 37 ++
>  tests/qtest/tpm-emu.h  |  2 ++
>  3 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index ddfd2d2b2a..64add7da72 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1094,7 +1094,6 @@ uint64_t tpm_tis_base_addr;
>  static void test_acpi_tcg_tpm(const char *machine, const char *tpm_if,
>uint64_t base, enum TPMVersion tpm_version)
>  {
> -#ifdef CONFIG_TPM
>  gchar *tmp_dir_name = g_strdup_printf("qemu-test_acpi_%s_tcg_%s.XX",
>machine, tpm_if);
>  char *tmp_path = g_dir_make_tmp(tmp_dir_name, NULL);
> @@ -1140,19 +1139,22 @@ static void test_acpi_tcg_tpm(const char *machine, 
> const char *tpm_if,
>  g_free(tmp_dir_name);
>  g_free(args);
>  free_test_data();
> -#else
> -g_test_skip("TPM disabled");
> -#endif
>  }
>  
>  static void test_acpi_q35_tcg_tpm2_tis(void)
>  {
> -test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_2_0);
> +if (!tpm_model_is_available("-machine q35", "tpm-tis"))

Style requires { }

> +g_test_skip("TPM disabled");
> +else
> +test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_2_0);
>  }
>  
>  static void test_acpi_q35_tcg_tpm12_tis(void)
>  {
> -test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_1_2);

Can you call this once in main() (before registering the tests)
and place this patch before patch #8 "tests: acpi: Add test cases
for TPM 1.2 with TCPA table" of this series? Otherwise looks good
(beside the unref you already mentioned).

> +if (!tpm_model_is_available("-machine q35", "tpm-tis"))
> +g_test_skip("TPM disabled");
> +else
> +test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_1_2);
>  }
>  
>  static void test_acpi_tcg_dimm_pxm(const char *machine)
> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
> index 32c704194b..553f1ad4ee 100644
> --- a/tests/qtest/tpm-emu.c
> +++ b/tests/qtest/tpm-emu.c
> @@ -16,6 +16,8 @@
>  #include "backends/tpm/tpm_ioctl.h"
>  #include "io/channel-socket.h"
>  #include "qapi/error.h"
> +#include "qapi/qmp/qlist.h"
> +#include "qapi/qmp/qstring.h"
>  #include "tpm-emu.h"
>  
>  void tpm_emu_test_wait_cond(TPMTestState *s)
> @@ -192,3 +194,38 @@ void *tpm_emu_ctrl_thread(void *data)
>  object_unref(OBJECT(lioc));
>  return NULL;
>  }
> +
> +bool tpm_model_is_available(const char *args, const char *tpm_if)
> +{
> +QTestState *qts;
> +QDict *rsp_tpm;
> +bool ret = false;
> +
> +qts = qtest_init(args);
> +if (!qts)
> +return false;
> +
> +rsp_tpm = qtest_qmp(qts, "{ 'execute': 'query-tpm'}");
> +if (!qdict_haskey(rsp_tpm, "error")) {
> +QDict *rsp_models = qtest_qmp(qts,
> +  "{ 'execute': 'query-tpm-models'}");
> +if (qdict_haskey(rsp_models, "return")) {
> +QList *models = qdict_get_qlist(rsp_models, "return");
> +QListEntry *e;
> +
> +QLIST_FOREACH_ENTRY(models, e) {
> +QString *s = qobject_to(QString, qlist_entry_obj(e));
> +const char *ename = qstring_get_str(s);
> +if (!strcmp(ename, tpm_if)) {
> +ret = true;
> +}
> +}
> +qobject_unref(models);
> +}
> +qobject_unref(rsp_models);
> +}
> +qobject_unref(rsp_tpm);
> +qtest_quit(qts);
> +
> +return ret;
> +}

Re: [PATCH v4 10/10] tests: Use QMP to check whether a TPM device model is available

[Stefan Berger]

On 7/12/21 4:47 PM, Stefan Berger wrote:

Use QMP to check whether a given TPM device model is available
and if it is not the case then skip a test that requires it.

Signed-off-by: Stefan Berger 

+rsp_tpm = qtest_qmp(qts, "{ 'execute': 'query-tpm'}");
+if (!qdict_haskey(rsp_tpm, "error")) {
+QDict *rsp_models = qtest_qmp(qts,
+  "{ 'execute': 'query-tpm-models'}");
+if (qdict_haskey(rsp_models, "return")) {
+QList *models = qdict_get_qlist(rsp_models, "return");
+QListEntry *e;
+
+QLIST_FOREACH_ENTRY(models, e) {
+QString *s = qobject_to(QString, qlist_entry_obj(e));
+const char *ename = qstring_get_str(s);
+if (!strcmp(ename, tpm_if)) {
+ret = true;
+}
+}
+qobject_unref(models);


I think this unref was too much. It tipped off s390x but none other (?).

https://travis-ci.com/github/stefanberger/qemu-tpm/builds/232529861


I fixed this in v5 (https://travis-ci.com/github/stefanberger/qemu-tpm)


I think this is generally the right approach for this test case, isn't it?



+}
+qobject_unref(rsp_models);
+}
+qobject_unref(rsp_tpm);
+qtest_quit(qts);
+
+return ret;
+}
diff --git a/tests/qtest/tpm-emu.h b/tests/qtest/tpm-emu.h
index fcb5d7a1d6..c33d99af37 100644
--- a/tests/qtest/tpm-emu.h
+++ b/tests/qtest/tpm-emu.h
@@ -22,6 +22,7 @@
  #include "qemu/sockets.h"
  #include "io/channel.h"
  #include "sysemu/tpm.h"
+#include "libqos/libqtest.h"
  
  struct tpm_hdr {

  uint16_t tag;
@@ -50,5 +51,6 @@ typedef struct TPMTestState {
  
  void tpm_emu_test_wait_cond(TPMTestState *s);

  void *tpm_emu_ctrl_thread(void *data);
+bool tpm_model_is_available(const char *args, const char *tpm_if);
  
  #endif /* TESTS_TPM_EMU_H */

[PATCH v4 10/10] tests: Use QMP to check whether a TPM device model is available

[Stefan Berger]

Use QMP to check whether a given TPM device model is available
and if it is not the case then skip a test that requires it.

Signed-off-by: Stefan Berger 
---
 tests/qtest/bios-tables-test.c | 14 +++--
 tests/qtest/tpm-emu.c  | 37 ++
 tests/qtest/tpm-emu.h  |  2 ++
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index ddfd2d2b2a..64add7da72 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1094,7 +1094,6 @@ uint64_t tpm_tis_base_addr;
 static void test_acpi_tcg_tpm(const char *machine, const char *tpm_if,
   uint64_t base, enum TPMVersion tpm_version)
 {
-#ifdef CONFIG_TPM
 gchar *tmp_dir_name = g_strdup_printf("qemu-test_acpi_%s_tcg_%s.XX",
   machine, tpm_if);
 char *tmp_path = g_dir_make_tmp(tmp_dir_name, NULL);
@@ -1140,19 +1139,22 @@ static void test_acpi_tcg_tpm(const char *machine, 
const char *tpm_if,
 g_free(tmp_dir_name);
 g_free(args);
 free_test_data();
-#else
-g_test_skip("TPM disabled");
-#endif
 }
 
 static void test_acpi_q35_tcg_tpm2_tis(void)
 {
-test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_2_0);
+if (!tpm_model_is_available("-machine q35", "tpm-tis"))
+g_test_skip("TPM disabled");
+else
+test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_2_0);
 }
 
 static void test_acpi_q35_tcg_tpm12_tis(void)
 {
-test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_1_2);
+if (!tpm_model_is_available("-machine q35", "tpm-tis"))
+g_test_skip("TPM disabled");
+else
+test_acpi_tcg_tpm("q35", "tis", 0xFED4, TPM_VERSION_1_2);
 }
 
 static void test_acpi_tcg_dimm_pxm(const char *machine)
diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
index 32c704194b..553f1ad4ee 100644
--- a/tests/qtest/tpm-emu.c
+++ b/tests/qtest/tpm-emu.c
@@ -16,6 +16,8 @@
 #include "backends/tpm/tpm_ioctl.h"
 #include "io/channel-socket.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qstring.h"
 #include "tpm-emu.h"
 
 void tpm_emu_test_wait_cond(TPMTestState *s)
@@ -192,3 +194,38 @@ void *tpm_emu_ctrl_thread(void *data)
 object_unref(OBJECT(lioc));
 return NULL;
 }
+
+bool tpm_model_is_available(const char *args, const char *tpm_if)
+{
+QTestState *qts;
+QDict *rsp_tpm;
+bool ret = false;
+
+qts = qtest_init(args);
+if (!qts)
+return false;
+
+rsp_tpm = qtest_qmp(qts, "{ 'execute': 'query-tpm'}");
+if (!qdict_haskey(rsp_tpm, "error")) {
+QDict *rsp_models = qtest_qmp(qts,
+  "{ 'execute': 'query-tpm-models'}");
+if (qdict_haskey(rsp_models, "return")) {
+QList *models = qdict_get_qlist(rsp_models, "return");
+QListEntry *e;
+
+QLIST_FOREACH_ENTRY(models, e) {
+QString *s = qobject_to(QString, qlist_entry_obj(e));
+const char *ename = qstring_get_str(s);
+if (!strcmp(ename, tpm_if)) {
+ret = true;
+}
+}
+qobject_unref(models);
+}
+qobject_unref(rsp_models);
+}
+qobject_unref(rsp_tpm);
+qtest_quit(qts);
+
+return ret;
+}
diff --git a/tests/qtest/tpm-emu.h b/tests/qtest/tpm-emu.h
index fcb5d7a1d6..c33d99af37 100644
--- a/tests/qtest/tpm-emu.h
+++ b/tests/qtest/tpm-emu.h
@@ -22,6 +22,7 @@
 #include "qemu/sockets.h"
 #include "io/channel.h"
 #include "sysemu/tpm.h"
+#include "libqos/libqtest.h"
 
 struct tpm_hdr {
 uint16_t tag;
@@ -50,5 +51,6 @@ typedef struct TPMTestState {
 
 void tpm_emu_test_wait_cond(TPMTestState *s);
 void *tpm_emu_ctrl_thread(void *data);
+bool tpm_model_is_available(const char *args, const char *tpm_if);
 
 #endif /* TESTS_TPM_EMU_H */
-- 
2.31.1