* [RFC 0/6] TPM-TIS bios-tables-test
@ 2020-06-01 10:21 Eric Auger
2020-06-01 10:21 ` [RFC 1/6] test/tpm-emu: include sockets and channel headers in tpm-emu header Eric Auger
` (6 more replies)
0 siblings, 7 replies; 27+ messages in thread
From: Eric Auger @ 2020-06-01 10:21 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, stefanb, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
This series adds the test of the TPM2 and DSDT tables when the
TPM-TIS device gets instantiated.
The series also allows to validate changes in the TPM2 table
generation done in "[PATCH v3 0/4] vTPM/aarch64 ACPI support",
sent separately.
This depends on Stefan's "acpi: tpm: Do not build TCPA table
for TPM 2".
This is in RFC state because I am not very familiar with the
table generation process docommented in bios-tables-test.c.
I don't know how to get rid of the checkpatch errors:
"ERROR: Does not appear to be a unified-diff format patch"
Surprisingly, checkpatch argues when void tables are added in
the same time as bios-tables-test-allowed-diff.h changes.
Most importantly, to be able to reuse tpm-emu code, I was forced
to remove the TPM2_ST_NO_SESSIONS assert. I would be grateful
if I could get some advises on the best way to address the issue.
Best Regards
Eric
This series can be found at:
https://github.com/eauger/qemu/tree/v5.0-tpm-acpi-tests-v1
Eric Auger (6):
test/tpm-emu: include sockets and channel headers in tpm-emu header
tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test
tests/acpi: Ignore TPM2.tis and DSDT.tis
tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
bios-tables-test: Add Q35/TPM-TIS test
bios-tables-test: Generate reference tables for Q35/TPM-TIS
tests/qtest/tpm-emu.h | 3 ++
tests/qtest/bios-tables-test.c | 60 +++++++++++++++++++++++++++++++++
tests/qtest/tpm-emu.c | 1 -
tests/data/acpi/q35/DSDT.tis | Bin 0 -> 8468 bytes
tests/data/acpi/q35/TPM2.tis | Bin 0 -> 76 bytes
tests/qtest/Makefile.include | 1 +
6 files changed, 64 insertions(+), 1 deletion(-)
create mode 100644 tests/data/acpi/q35/DSDT.tis
create mode 100644 tests/data/acpi/q35/TPM2.tis
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFC 1/6] test/tpm-emu: include sockets and channel headers in tpm-emu header
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
@ 2020-06-01 10:21 ` Eric Auger
2020-06-02 14:08 ` Stefan Berger
2020-06-01 10:21 ` [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test Eric Auger
` (5 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Eric Auger @ 2020-06-01 10:21 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, stefanb, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Include sockets and channel headers to that the header is
self-contained.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
tests/qtest/tpm-emu.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qtest/tpm-emu.h b/tests/qtest/tpm-emu.h
index a4f1d64226..73f3bed0c4 100644
--- a/tests/qtest/tpm-emu.h
+++ b/tests/qtest/tpm-emu.h
@@ -16,6 +16,9 @@
#define TPM_RC_FAILURE 0x101
#define TPM2_ST_NO_SESSIONS 0x8001
+#include "qemu/sockets.h"
+#include "io/channel.h"
+
struct tpm_hdr {
uint16_t tag;
uint32_t len;
--
2.20.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
2020-06-01 10:21 ` [RFC 1/6] test/tpm-emu: include sockets and channel headers in tpm-emu header Eric Auger
@ 2020-06-01 10:21 ` Eric Auger
2020-06-02 14:08 ` Stefan Berger
2020-06-05 14:54 ` Igor Mammedov
2020-06-01 10:21 ` [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis Eric Auger
` (4 subsequent siblings)
6 siblings, 2 replies; 27+ messages in thread
From: Eric Auger @ 2020-06-01 10:21 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, stefanb, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Add placeholders for TPM and DSDT reference tables for
Q35 TPM-TIS tests.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
tests/data/acpi/q35/DSDT.tis | Bin
tests/data/acpi/q35/TPM2.tis | Bin
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 tests/data/acpi/q35/DSDT.tis
create mode 100644 tests/data/acpi/q35/TPM2.tis
diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
diff --git a/tests/data/acpi/q35/TPM2.tis b/tests/data/acpi/q35/TPM2.tis
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
2020-06-01 10:21 ` [RFC 1/6] test/tpm-emu: include sockets and channel headers in tpm-emu header Eric Auger
2020-06-01 10:21 ` [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test Eric Auger
@ 2020-06-01 10:21 ` Eric Auger
2020-06-02 14:09 ` Stefan Berger
2020-06-05 14:55 ` Igor Mammedov
2020-06-01 10:21 ` [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS Eric Auger
` (3 subsequent siblings)
6 siblings, 2 replies; 27+ messages in thread
From: Eric Auger @ 2020-06-01 10:21 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, stefanb, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Subsequent patches will alter the content of TPM2.tis
and DSDT.tis so let's ignore them until the references
are generated.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..a2a45d1d31 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/DSDT.tis",
+"tests/data/acpi/q35/TPM2.tis",
--
2.20.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
` (2 preceding siblings ...)
2020-06-01 10:21 ` [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis Eric Auger
@ 2020-06-01 10:21 ` Eric Auger
2020-06-02 13:39 ` Stefan Berger
2020-06-01 10:21 ` [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test Eric Auger
` (2 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Eric Auger @ 2020-06-01 10:21 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, stefanb, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
While writing tests for checking the content of TPM2 and DSDT
along with TPM-TIS instantiation I attempted to reuse the
framework used for TPM-TIS tests. However While dumping the
ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
is maybe the other tests did not execute long enough to encounter
this. So I tentatively propose to remove the assert as it
does not seem to break other tests and enable the new ones.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
tests/qtest/tpm-emu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
index c43ac4aef8..298d0eec74 100644
--- a/tests/qtest/tpm-emu.c
+++ b/tests/qtest/tpm-emu.c
@@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
- g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
s->tpm_msg = g_realloc(s->tpm_msg, s->tpm_msg->len);
qio_channel_read(ioc, (char *)&s->tpm_msg->code,
--
2.20.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
` (3 preceding siblings ...)
2020-06-01 10:21 ` [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS Eric Auger
@ 2020-06-01 10:21 ` Eric Auger
2020-06-02 14:38 ` Stefan Berger
2020-06-05 15:17 ` Igor Mammedov
2020-06-01 10:21 ` [RFC 6/6] bios-tables-test: Generate reference tables for Q35/TPM-TIS Eric Auger
2020-06-02 11:42 ` [RFC 0/6] TPM-TIS bios-tables-test Auger Eric
6 siblings, 2 replies; 27+ messages in thread
From: Eric Auger @ 2020-06-01 10:21 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, stefanb, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Test tables specific to the TPM-TIS instantiation.
The TPM2 is added in the framework. Also the DSDT
is updated with the TPM. The new function should be
be usable for CRB as well, later one.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
tests/qtest/bios-tables-test.c | 60 ++++++++++++++++++++++++++++++++++
tests/qtest/Makefile.include | 1 +
2 files changed, 61 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index c9843829b3..bbba98342c 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -57,6 +57,9 @@
#include "qemu/bitmap.h"
#include "acpi-utils.h"
#include "boot-sector.h"
+#include "tpm-emu.h"
+#include "hw/acpi/tpm.h"
+
#define MACHINE_PC "pc"
#define MACHINE_Q35 "q35"
@@ -874,6 +877,60 @@ static void test_acpi_piix4_tcg_numamem(void)
free_test_data(&data);
}
+uint64_t tpm_tis_base_addr;
+
+struct tpm_test_data {
+ const char *machine;
+ const char *tpm_if;
+};
+
+static void test_acpi_tcg_tpm(const void *context)
+{
+ struct tpm_test_data *c = (struct tpm_test_data *)context;
+ gchar *tmp_dir_name = g_strdup_printf("qemu-test_acpi_%s_tcg_%s.XXXXXX",
+ c->machine, c->tpm_if);
+ char *tmp_path = g_dir_make_tmp(tmp_dir_name, NULL);
+ TestState test;
+ test_data data;
+ GThread *thread;
+ char *args, *variant = g_strdup_printf(".%s", c->tpm_if);
+
+ tpm_tis_base_addr = TPM_TIS_ADDR_BASE;
+
+ module_call_init(MODULE_INIT_QOM);
+
+ test.addr = g_new0(SocketAddress, 1);
+ test.addr->type = SOCKET_ADDRESS_TYPE_UNIX;
+ test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
+ g_mutex_init(&test.data_mutex);
+ g_cond_init(&test.data_cond);
+ test.data_cond_signal = false;
+
+ thread = g_thread_new(NULL, tpm_emu_ctrl_thread, &test);
+ tpm_emu_test_wait_cond(&test);
+
+ memset(&data, 0, sizeof(data));
+ data.machine = c->machine;
+ data.variant = variant;
+
+ args = g_strdup_printf(
+ " -chardev socket,id=chr,path=%s"
+ " -tpmdev emulator,id=dev,chardev=chr"
+ " -device tpm-%s,tpmdev=dev",
+ test.addr->u.q_unix.path, c->tpm_if);
+
+ test_acpi_one(args, &data);
+
+ g_thread_join(thread);
+ g_unlink(test.addr->u.q_unix.path);
+ qapi_free_SocketAddress(test.addr);
+ g_rmdir(tmp_path);
+ g_free(variant);
+ g_free(tmp_path);
+ g_free(tmp_dir_name);
+ free_test_data(&data);
+}
+
static void test_acpi_tcg_dimm_pxm(const char *machine)
{
test_data data;
@@ -1028,6 +1085,7 @@ int main(int argc, char *argv[])
{
const char *arch = qtest_get_arch();
int ret;
+ struct tpm_test_data tpm_q35_tis = {MACHINE_Q35, "tis"};
g_test_init(&argc, &argv, NULL);
@@ -1037,6 +1095,8 @@ int main(int argc, char *argv[])
return ret;
}
+ qtest_add_data_func("acpi/q35/tpm-tis",
+ &tpm_q35_tis, test_acpi_tcg_tpm);
qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
qtest_add_func("acpi/q35", test_acpi_q35_tcg);
diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index 9e5a51d033..5023fa413d 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -262,6 +262,7 @@ tests/qtest/hd-geo-test$(EXESUF): tests/qtest/hd-geo-test.o $(libqos-obj-y)
tests/qtest/boot-order-test$(EXESUF): tests/qtest/boot-order-test.o $(libqos-obj-y)
tests/qtest/boot-serial-test$(EXESUF): tests/qtest/boot-serial-test.o $(libqos-obj-y)
tests/qtest/bios-tables-test$(EXESUF): tests/qtest/bios-tables-test.o \
+ tests/qtest/tpm-emu.o $(test-io-obj-y) \
tests/qtest/boot-sector.o tests/qtest/acpi-utils.o $(libqos-obj-y)
tests/qtest/pxe-test$(EXESUF): tests/qtest/pxe-test.o tests/qtest/boot-sector.o $(libqos-obj-y)
tests/qtest/microbit-test$(EXESUF): tests/qtest/microbit-test.o
--
2.20.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC 6/6] bios-tables-test: Generate reference tables for Q35/TPM-TIS
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
` (4 preceding siblings ...)
2020-06-01 10:21 ` [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test Eric Auger
@ 2020-06-01 10:21 ` Eric Auger
2020-06-02 14:39 ` Stefan Berger
2020-06-02 11:42 ` [RFC 0/6] TPM-TIS bios-tables-test Auger Eric
6 siblings, 1 reply; 27+ messages in thread
From: Eric Auger @ 2020-06-01 10:21 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, stefanb, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
TPM2, DSDT tables were generated using
tests/data/acpi/rebuild-expected-aml.sh
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 2 --
tests/data/acpi/q35/DSDT.tis | Bin 0 -> 8468 bytes
tests/data/acpi/q35/TPM2.tis | Bin 0 -> 76 bytes
3 files changed, 2 deletions(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index a2a45d1d31..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.tis",
-"tests/data/acpi/q35/TPM2.tis",
diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..34a3c915d27cab5d09190a0441e9511db0c147f2 100644
GIT binary patch
literal 8468
zcmb7JOKcm*8J^`!%jHs9QY-nHm~{*kY0`v}k~C<MG~_N{ktnUWw9|wxxN?(LwhM%C
zVj!szKvIA_476y>Cg`OA9S}utz4YFo=N@t}5CcUIy%y-DMG>P1v)@0%k!MH>h<VuE
zfA;(Sf4<qrx18}qzwLgDF{4@D^6Fus{G-70(Pl8l=$Q6SBXN;+_WW|KYh+>xqqCpk
z$Jmrj_esBezGnP(H~K7!-u^H$c6#j6=gy~>c6#rB6kTEjx_+r=S#;p6TivkS?HqVk
z;5CY7x8CsW(wCNLc0F+vH@hXj#mw&chHtnVbKR}bOWeH5JpYxI!Dh0*n_IM;PV;YT
zU!OVs(r2%{UHamMuYP*-s+9m>6?+r=wH|Gv8<BM;8aTK5myP#`&d;q37H{?WY0>7;
znO;OKw{6ixsaA$pou(=JS~2TEr7invpoi!i#-kpzo6KhH%ljFY=<GWbFJ7o@`h}q7
zcMI-w-LPF^J!*-1)U@-9$a12A{YgJ!{U|dU4cTD+ll~6-&5q6f@!#Z-4Xp)?L@UhY
ztoaYp7<wdU3C7-K&P#JoQFmzAFnp;-SnBejLI=*EGeM)Z9kU|K?(A*g3dXEGE7sXR
zLC4J7YL?>6z^;tlM$q<S994-~2ZUMzs=~ZdWz+D=*lYyYI4>_h9hViUTIb-qt+t1v
z;+kb}ji{%`t)-gLV?E}?&N8EBTw~W*EcGty96YmL_7CB43>$_+>f@}4Ma%%@b@n>;
zXJe4rIXKtcI)BJoSkzdil|0KFLcItTho}T|kBhOUo=1Ze#&^hWCk{u`v9dMeo%lO(
z(>%M!t5gygjTNaSX^ii;aT*$mqte`q--|Qzti#h(Db4Cc8Y?EdL!)_Qp4hz@@|ehD
z1`V&uT)T_C(c7iS`PHii4Zh?3lRqdys~zOm-`dM(61Z>J#^!p3iDLuDRhGA@-HIWo
z$smrP&hpuWJsROL7PxUlECA=lInD!AhQvgEj*W39u;YlB5*lNi2`FbGBqn;pv2o6n
zJ)k>}XXVhiU;-*bV#+GG;{}%INpPlwCInM<9-*o;sp(8|ri3O1Q+6JqsxzhOOmU`!
zrgWVMRh@I1&N)rzoURk0s?*eTnwm~i*NIToiFboC(rHa+TGxqC)oE!uElsDT>qMyP
z%xF3@n$C=_6QQaT?<nPZ+L}&V*NITonbmY=HJw>qCqh-{yry$r(>br}M5yXq&~z?n
zIu~@E2vwbqrqj`MI=W7Ts?MCIGpFgy={garI`f*&yrwg+>qMyPT-0<fYC0Enod{K(
zuBOw~bh^4ugsRRZP3MxPb4k~UP}RAt>0H)yF6%lGsydHpI*(~OkLfxQsydHrI*)5Q
zkLx-Ssya__W@-E;Ji(de@yqUnV0sTiV)|q3NsW0@W1iHR2vz1Ojd@CAp3<2JRpx1p
zd0Jzh)|m)Z<|7*O5smqX&P1p(ALUHxGLLeme8)d3nDWJqkeJd*&S*8yXf@C1H4%!M
zCJeNs7-%hQ92zK&QFv^i2vAl8aRjm`3{>E_B^mhA;Xvt_F#(k!F{LyZsK81CRiI>`
z0?L_Wpa{_$X`li-j;N;eS~5@pJv2~+=sL&3Km~Rj5mOE>8K{7ACK)I~sS^e&u$&14
zRiI>`0?L_Wpa`W-7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<ey
zXTm@gC>f}LawZulLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9uB|
zpaRR8Fi-_b1}dPONd}5g>V$y`EN8+%6(|{~fN~}oC_<?d1}d<e2?JH2WS|1dnPi{{
zrA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&RoJj_XQ0jz%3M^;BKouw%sDN@N
z87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!%b74x1xf}g
zpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-7^uK<CJa=8
zl7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZulLa7r5ibyd~
zM5=)zR1FlNVxS5W2C6X0Kouq#sKSJSDohxt!XyJ#m}H;|69%d<VW0|=3{+u~fhtTG
zsKSJSDoip^g-HggFkzsG<kf|NBEl8pgGmO85SKxQ#FS$T14Sgq76yt)jx8A|LOHf%
zpa^~Dp6tRx*}(4eKdVFfLwY+!yHaQW`%lN|Un<QK&^&|@w6;rd-0d=LJlgms!>q`5
zg^sJVsnMoxx{HnNux$*|H>|x~On;<@R>pm%vF)|e0JMs^c`NgAilu293jdUEQk#{;
zw$Zz>Yb`K3LMK|76WN%<H$qAN7q>ByLen%Rx*g`0t;LV$Svn4PV|afvyor&`UEs3@
zL+S#0Q*v`%zY((c=-ArhEekZq!GQ2uJU-S9TcL4tciaamPwnIC<%#N;R`|x%%S!b!
zuU<y~)aqq-cJ*@Sph?=jiRxDDA<BD7d5@R(Cdzw9l=sHv{mJr}@%hH;eWkq5%li}M
z{Ugfz<MNfs@`;BiUs1|ec=^gi`N|RHE93Ij$@0mEC|^~|S9$sBMEU9w<*VcJwaN0S
zhbUiD%GY@L+C=%<5#?*+^7YB`a}QCzu9UCy^7V=G^&`sHQ9ePhvdQv}C@)_g_^hC5
zHa=gex49|)$YV#(^*G(+V{GO0bokboN+;T8rki>$^LjdbfJ~(mZ8OtNJ!hCBrRPiv
z!&k{vI?*;W-PCjA>gn*QGL=rW%}h7-94zVS@clBCPPENTH+5ex>*?@OGnG!X%}h6S
zpB~fG;Y(*KooLh3Ij5A#_=T`p#!?2BE$lev;++jl{EIb?iw|bdgH|P!cd3Pk6W)~d
zg7by?n{W4ie*I^KYj3>u=JjhodE+hCF}z@7eSEiOEvshSweQ-7jcIRq63jAl*Tyve
z{$muBSiz&&datn=8eXe;!SE_C7Y&y1yryq<_VWaadtqgx0Hj}P7ckA-*ld_RMva|=
zttT#WDd>ynU$*20jbd@7NIej1GD$b*wX3hj23CKUec%;~#mjCz<)M1I^Vr3w6LOV+
zG*aJ3qu%ahGm7<CG&tFxJVCwhSqo9;+f!#Y)`yYXOp>C9D`ZTjLWY_7-TJ&2dYk4^
zGK}c+Ql=kaeIx3z$)p(JK`k*Qh+hc|*CZT+Z}g^m26WGeZm%Z>_32dndd^N|#iw%t
zj$E^aHq+i#*tU-nI`eu>bry<ymuKkeS+U7hf?k<y#1PEvUFZGh>8`@VhaYZG+*#(3
z?l0Vf%~Cs;u}p@)`t%X#&SOQeogp!7_wp6@6mH*IQ0Z2RtGGaX!HRBWx~h&#XZS-m
zzNw4PF{hamH@sMXq!p~HZT;n=EQ{e!Hhv@8U{UIxea|*bv>SM;@%tL5@7afO7OOCf
z{pUlx9mIM1upEu(!;*e6#?SE2R)pqgH2Nby!Pp<PGhd8Gf4(oyeDUJVCmv)IZIl4q
z5!7oxim^b5(|7K(fL>Zy6ih&_SX0dgn?bPzx#C4b?aCWQtf^&$AM!U$Ayr5jSXgr&
zzX(dW*3G9929o3WnQq=pSTPIJScsmbGt?932M>m^bMRF46;HhRv)h&Cd0fj?LSwy8
zt>Pq|B)!L+t;a9&#jV=oVmUJt7K>QTqlMqj!FR&q^CG~A8w_?7uJ~Bbqk~m>u*MHk
zfw$QPW$Ro!Xo}^)Y%{F3@%CC7*Vool)#2J}2f-?qZ@2H<w_fY)BLl)9Xl$C;1ce}I
z@y#|JH47Wq7COXPWA<y{&>}cRMsw@J1=C$^2i48=Abso8UCiqY@v6qj6?V;L!JYfN
zY>Zwp&{{Qze2Xrx<`XomI9kgm=`M3C$Gd*mv<Ed;taShWe}8}B)N_B@9G?57WBzVA
z9Ax@e*f5v0Vj}~u<15B6yYkc;8$8*+V#ToFewE=(MSY2OaQeoMg}Jq%(TmXi?H=T{
znpkuGA;9s$64u3g)cz6m#9-)Eb9L}o|1uKE<~4)Sag!ixxCIEaHZZ}*;%W@wD^{{c
zW5WRg987~#L)5Dzfu1|EkN9WFyKBV=>`>>FUQ=(;FVO;-hSYKbNliWJkmniZ;szT$
qD~9o3?dW@pf4yG)_37G$ul{}K$AdH<#<&q5EMgEPKcb5{Wd8@f&*N$U
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/TPM2.tis b/tests/data/acpi/q35/TPM2.tis
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..7878a6e79a6a406d99ca1f5e9a528eb392b8d793 100644
GIT binary patch
literal 76
wcmWFu@HO&bU|?V=a`Jcf2v%^42yhMoiZKGkKx`0=4A_u4U^Ym_e|8WP0Iiz{0RR91
literal 0
HcmV?d00001
--
2.20.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [RFC 0/6] TPM-TIS bios-tables-test
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
` (5 preceding siblings ...)
2020-06-01 10:21 ` [RFC 6/6] bios-tables-test: Generate reference tables for Q35/TPM-TIS Eric Auger
@ 2020-06-02 11:42 ` Auger Eric
6 siblings, 0 replies; 27+ messages in thread
From: Auger Eric @ 2020-06-02 11:42 UTC (permalink / raw)
To: eric.auger.pro, stefanb, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Hi,
On 6/1/20 12:21 PM, Eric Auger wrote:
> This series adds the test of the TPM2 and DSDT tables when the
> TPM-TIS device gets instantiated.
>
> The series also allows to validate changes in the TPM2 table
> generation done in "[PATCH v3 0/4] vTPM/aarch64 ACPI support",
> sent separately.
>
> This depends on Stefan's "acpi: tpm: Do not build TCPA table
> for TPM 2".
>
> This is in RFC state because I am not very familiar with the
> table generation process docommented in bios-tables-test.c.
>
> I don't know how to get rid of the checkpatch errors:
> "ERROR: Does not appear to be a unified-diff format patch"
>
> Surprisingly, checkpatch argues when void tables are added in
> the same time as bios-tables-test-allowed-diff.h changes.
Both issues reported above were resolved by Michael's
[PATCH] checkpatch: reversed logic with acpi test checks
Thank you for fixing that
Eric
>
> Most importantly, to be able to reuse tpm-emu code, I was forced
> to remove the TPM2_ST_NO_SESSIONS assert. I would be grateful
> if I could get some advises on the best way to address the issue.
>
> Best Regards
>
> Eric
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/v5.0-tpm-acpi-tests-v1
>
>
> Eric Auger (6):
> test/tpm-emu: include sockets and channel headers in tpm-emu header
> tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test
> tests/acpi: Ignore TPM2.tis and DSDT.tis
> tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
> bios-tables-test: Add Q35/TPM-TIS test
> bios-tables-test: Generate reference tables for Q35/TPM-TIS
>
> tests/qtest/tpm-emu.h | 3 ++
> tests/qtest/bios-tables-test.c | 60 +++++++++++++++++++++++++++++++++
> tests/qtest/tpm-emu.c | 1 -
> tests/data/acpi/q35/DSDT.tis | Bin 0 -> 8468 bytes
> tests/data/acpi/q35/TPM2.tis | Bin 0 -> 76 bytes
> tests/qtest/Makefile.include | 1 +
> 6 files changed, 64 insertions(+), 1 deletion(-)
> create mode 100644 tests/data/acpi/q35/DSDT.tis
> create mode 100644 tests/data/acpi/q35/TPM2.tis
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-01 10:21 ` [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS Eric Auger
@ 2020-06-02 13:39 ` Stefan Berger
2020-06-02 14:43 ` Stefan Berger
2020-06-02 16:13 ` Auger Eric
0 siblings, 2 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 13:39 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/1/20 6:21 AM, Eric Auger wrote:
> While writing tests for checking the content of TPM2 and DSDT
> along with TPM-TIS instantiation I attempted to reuse the
> framework used for TPM-TIS tests. However While dumping the
> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
> is maybe the other tests did not execute long enough to encounter
> this. So I tentatively propose to remove the assert as it
> does not seem to break other tests and enable the new ones.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> tests/qtest/tpm-emu.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
> index c43ac4aef8..298d0eec74 100644
> --- a/tests/qtest/tpm-emu.c
> +++ b/tests/qtest/tpm-emu.c
> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
You should not have to remove this. The tests are skipped if swtpm does
not support TPM 2 via --tpm2 option. This would be a very old swtpm
version, though. So, all tests are run with --tpm2 option and any
response received from the TPM would be a TPM 2 response that should
have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other value you
are seeing there.
>
> s->tpm_msg = g_realloc(s->tpm_msg, s->tpm_msg->len);
> qio_channel_read(ioc, (char *)&s->tpm_msg->code,
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 1/6] test/tpm-emu: include sockets and channel headers in tpm-emu header
2020-06-01 10:21 ` [RFC 1/6] test/tpm-emu: include sockets and channel headers in tpm-emu header Eric Auger
@ 2020-06-02 14:08 ` Stefan Berger
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 14:08 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/1/20 6:21 AM, Eric Auger wrote:
> Include sockets and channel headers to that the header is
> self-contained.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> tests/qtest/tpm-emu.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tests/qtest/tpm-emu.h b/tests/qtest/tpm-emu.h
> index a4f1d64226..73f3bed0c4 100644
> --- a/tests/qtest/tpm-emu.h
> +++ b/tests/qtest/tpm-emu.h
> @@ -16,6 +16,9 @@
> #define TPM_RC_FAILURE 0x101
> #define TPM2_ST_NO_SESSIONS 0x8001
>
> +#include "qemu/sockets.h"
> +#include "io/channel.h"
> +
> struct tpm_hdr {
> uint16_t tag;
> uint32_t len;
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test
2020-06-01 10:21 ` [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test Eric Auger
@ 2020-06-02 14:08 ` Stefan Berger
2020-06-05 14:54 ` Igor Mammedov
1 sibling, 0 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 14:08 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/1/20 6:21 AM, Eric Auger wrote:
> Add placeholders for TPM and DSDT reference tables for
> Q35 TPM-TIS tests.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> tests/data/acpi/q35/DSDT.tis | Bin
> tests/data/acpi/q35/TPM2.tis | Bin
> 2 files changed, 0 insertions(+), 0 deletions(-)
> create mode 100644 tests/data/acpi/q35/DSDT.tis
> create mode 100644 tests/data/acpi/q35/TPM2.tis
>
> diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
> new file mode 100644
> index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> diff --git a/tests/data/acpi/q35/TPM2.tis b/tests/data/acpi/q35/TPM2.tis
> new file mode 100644
> index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis
2020-06-01 10:21 ` [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis Eric Auger
@ 2020-06-02 14:09 ` Stefan Berger
2020-06-05 14:55 ` Igor Mammedov
1 sibling, 0 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 14:09 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/1/20 6:21 AM, Eric Auger wrote:
> Subsequent patches will alter the content of TPM2.tis
> and DSDT.tis so let's ignore them until the references
> are generated.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..a2a45d1d31 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,3 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT.tis",
> +"tests/data/acpi/q35/TPM2.tis",
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test
2020-06-01 10:21 ` [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test Eric Auger
@ 2020-06-02 14:38 ` Stefan Berger
2020-06-05 15:17 ` Igor Mammedov
1 sibling, 0 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 14:38 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/1/20 6:21 AM, Eric Auger wrote:
> Test tables specific to the TPM-TIS instantiation.
> The TPM2 is added in the framework. Also the DSDT
> is updated with the TPM. The new function should be
> be usable for CRB as well, later one.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 60 ++++++++++++++++++++++++++++++++++
> tests/qtest/Makefile.include | 1 +
> 2 files changed, 61 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index c9843829b3..bbba98342c 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -57,6 +57,9 @@
> #include "qemu/bitmap.h"
> #include "acpi-utils.h"
> #include "boot-sector.h"
> +#include "tpm-emu.h"
> +#include "hw/acpi/tpm.h"
> +
>
> #define MACHINE_PC "pc"
> #define MACHINE_Q35 "q35"
> @@ -874,6 +877,60 @@ static void test_acpi_piix4_tcg_numamem(void)
> free_test_data(&data);
> }
>
> +uint64_t tpm_tis_base_addr;
> +
> +struct tpm_test_data {
> + const char *machine;
> + const char *tpm_if;
> +};
> +
> +static void test_acpi_tcg_tpm(const void *context)
> +{
> + struct tpm_test_data *c = (struct tpm_test_data *)context;
> + gchar *tmp_dir_name = g_strdup_printf("qemu-test_acpi_%s_tcg_%s.XXXXXX",
> + c->machine, c->tpm_if);
> + char *tmp_path = g_dir_make_tmp(tmp_dir_name, NULL);
> + TestState test;
> + test_data data;
> + GThread *thread;
> + char *args, *variant = g_strdup_printf(".%s", c->tpm_if);
> +
> + tpm_tis_base_addr = TPM_TIS_ADDR_BASE;
> +
> + module_call_init(MODULE_INIT_QOM);
> +
> + test.addr = g_new0(SocketAddress, 1);
> + test.addr->type = SOCKET_ADDRESS_TYPE_UNIX;
> + test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
> + g_mutex_init(&test.data_mutex);
> + g_cond_init(&test.data_cond);
> + test.data_cond_signal = false;
> +
> + thread = g_thread_new(NULL, tpm_emu_ctrl_thread, &test);
> + tpm_emu_test_wait_cond(&test);
> +
> + memset(&data, 0, sizeof(data));
> + data.machine = c->machine;
> + data.variant = variant;
> +
> + args = g_strdup_printf(
> + " -chardev socket,id=chr,path=%s"
> + " -tpmdev emulator,id=dev,chardev=chr"
> + " -device tpm-%s,tpmdev=dev",
> + test.addr->u.q_unix.path, c->tpm_if);
> +
> + test_acpi_one(args, &data);
> +
> + g_thread_join(thread);
> + g_unlink(test.addr->u.q_unix.path);
> + qapi_free_SocketAddress(test.addr);
> + g_rmdir(tmp_path);
> + g_free(variant);
> + g_free(tmp_path);
> + g_free(tmp_dir_name);
> + free_test_data(&data);
> +}
> +
> static void test_acpi_tcg_dimm_pxm(const char *machine)
> {
> test_data data;
> @@ -1028,6 +1085,7 @@ int main(int argc, char *argv[])
> {
> const char *arch = qtest_get_arch();
> int ret;
> + struct tpm_test_data tpm_q35_tis = {MACHINE_Q35, "tis"};
>
> g_test_init(&argc, &argv, NULL);
>
> @@ -1037,6 +1095,8 @@ int main(int argc, char *argv[])
> return ret;
> }
>
> + qtest_add_data_func("acpi/q35/tpm-tis",
> + &tpm_q35_tis, test_acpi_tcg_tpm);
> qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
> qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
> qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
> index 9e5a51d033..5023fa413d 100644
> --- a/tests/qtest/Makefile.include
> +++ b/tests/qtest/Makefile.include
> @@ -262,6 +262,7 @@ tests/qtest/hd-geo-test$(EXESUF): tests/qtest/hd-geo-test.o $(libqos-obj-y)
> tests/qtest/boot-order-test$(EXESUF): tests/qtest/boot-order-test.o $(libqos-obj-y)
> tests/qtest/boot-serial-test$(EXESUF): tests/qtest/boot-serial-test.o $(libqos-obj-y)
> tests/qtest/bios-tables-test$(EXESUF): tests/qtest/bios-tables-test.o \
> + tests/qtest/tpm-emu.o $(test-io-obj-y) \
> tests/qtest/boot-sector.o tests/qtest/acpi-utils.o $(libqos-obj-y)
> tests/qtest/pxe-test$(EXESUF): tests/qtest/pxe-test.o tests/qtest/boot-sector.o $(libqos-obj-y)
> tests/qtest/microbit-test$(EXESUF): tests/qtest/microbit-test.o
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 6/6] bios-tables-test: Generate reference tables for Q35/TPM-TIS
2020-06-01 10:21 ` [RFC 6/6] bios-tables-test: Generate reference tables for Q35/TPM-TIS Eric Auger
@ 2020-06-02 14:39 ` Stefan Berger
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 14:39 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/1/20 6:21 AM, Eric Auger wrote:
> TPM2, DSDT tables were generated using
> tests/data/acpi/rebuild-expected-aml.sh
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 2 --
> tests/data/acpi/q35/DSDT.tis | Bin 0 -> 8468 bytes
> tests/data/acpi/q35/TPM2.tis | Bin 0 -> 76 bytes
> 3 files changed, 2 deletions(-)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index a2a45d1d31..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,3 +1 @@
> /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/q35/DSDT.tis",
> -"tests/data/acpi/q35/TPM2.tis",
> diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
> index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..34a3c915d27cab5d09190a0441e9511db0c147f2 100644
> GIT binary patch
> literal 8468
> zcmb7JOKcm*8J^`!%jHs9QY-nHm~{*kY0`v}k~C<MG~_N{ktnUWw9|wxxN?(LwhM%C
> zVj!szKvIA_476y>Cg`OA9S}utz4YFo=N@t}5CcUIy%y-DMG>P1v)@0%k!MH>h<VuE
> zfA;(Sf4<qrx18}qzwLgDF{4@D^6Fus{G-70(Pl8l=$Q6SBXN;+_WW|KYh+>xqqCpk
> z$Jmrj_esBezGnP(H~K7!-u^H$c6#j6=gy~>c6#rB6kTEjx_+r=S#;p6TivkS?HqVk
> z;5CY7x8CsW(wCNLc0F+vH@hXj#mw&chHtnVbKR}bOWeH5JpYxI!Dh0*n_IM;PV;YT
> zU!OVs(r2%{UHamMuYP*-s+9m>6?+r=wH|Gv8<BM;8aTK5myP#`&d;q37H{?WY0>7;
> znO;OKw{6ixsaA$pou(=JS~2TEr7invpoi!i#-kpzo6KhH%ljFY=<GWbFJ7o@`h}q7
> zcMI-w-LPF^J!*-1)U@-9$a12A{YgJ!{U|dU4cTD+ll~6-&5q6f@!#Z-4Xp)?L@UhY
> ztoaYp7<wdU3C7-K&P#JoQFmzAFnp;-SnBejLI=*EGeM)Z9kU|K?(A*g3dXEGE7sXR
> zLC4J7YL?>6z^;tlM$q<S994-~2ZUMzs=~ZdWz+D=*lYyYI4>_h9hViUTIb-qt+t1v
> z;+kb}ji{%`t)-gLV?E}?&N8EBTw~W*EcGty96YmL_7CB43>$_+>f@}4Ma%%@b@n>;
> zXJe4rIXKtcI)BJoSkzdil|0KFLcItTho}T|kBhOUo=1Ze#&^hWCk{u`v9dMeo%lO(
> z(>%M!t5gygjTNaSX^ii;aT*$mqte`q--|Qzti#h(Db4Cc8Y?EdL!)_Qp4hz@@|ehD
> z1`V&uT)T_C(c7iS`PHii4Zh?3lRqdys~zOm-`dM(61Z>J#^!p3iDLuDRhGA@-HIWo
> z$smrP&hpuWJsROL7PxUlECA=lInD!AhQvgEj*W39u;YlB5*lNi2`FbGBqn;pv2o6n
> zJ)k>}XXVhiU;-*bV#+GG;{}%INpPlwCInM<9-*o;sp(8|ri3O1Q+6JqsxzhOOmU`!
> zrgWVMRh@I1&N)rzoURk0s?*eTnwm~i*NIToiFboC(rHa+TGxqC)oE!uElsDT>qMyP
> z%xF3@n$C=_6QQaT?<nPZ+L}&V*NITonbmY=HJw>qCqh-{yry$r(>br}M5yXq&~z?n
> zIu~@E2vwbqrqj`MI=W7Ts?MCIGpFgy={garI`f*&yrwg+>qMyPT-0<fYC0Enod{K(
> zuBOw~bh^4ugsRRZP3MxPb4k~UP}RAt>0H)yF6%lGsydHpI*(~OkLfxQsydHrI*)5Q
> zkLx-Ssya__W@-E;Ji(de@yqUnV0sTiV)|q3NsW0@W1iHR2vz1Ojd@CAp3<2JRpx1p
> zd0Jzh)|m)Z<|7*O5smqX&P1p(ALUHxGLLeme8)d3nDWJqkeJd*&S*8yXf@C1H4%!M
> zCJeNs7-%hQ92zK&QFv^i2vAl8aRjm`3{>E_B^mhA;Xvt_F#(k!F{LyZsK81CRiI>`
> z0?L_Wpa{_$X`li-j;N;eS~5@pJv2~+=sL&3Km~Rj5mOE>8K{7ACK)I~sS^e&u$&14
> zRiI>`0?L_Wpa`W-7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<ey
> zXTm@gC>f}LawZulLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9uB|
> zpaRR8Fi-_b1}dPONd}5g>V$y`EN8+%6(|{~fN~}oC_<?d1}d<e2?JH2WS|1dnPi{{
> zrA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&RoJj_XQ0jz%3M^;BKouw%sDN@N
> z87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!%b74x1xf}g
> zpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-7^uK<CJa=8
> zl7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZulLa7r5ibyd~
> zM5=)zR1FlNVxS5W2C6X0Kouq#sKSJSDohxt!XyJ#m}H;|69%d<VW0|=3{+u~fhtTG
> zsKSJSDoip^g-HggFkzsG<kf|NBEl8pgGmO85SKxQ#FS$T14Sgq76yt)jx8A|LOHf%
> zpa^~Dp6tRx*}(4eKdVFfLwY+!yHaQW`%lN|Un<QK&^&|@w6;rd-0d=LJlgms!>q`5
> zg^sJVsnMoxx{HnNux$*|H>|x~On;<@R>pm%vF)|e0JMs^c`NgAilu293jdUEQk#{;
> zw$Zz>Yb`K3LMK|76WN%<H$qAN7q>ByLen%Rx*g`0t;LV$Svn4PV|afvyor&`UEs3@
> zL+S#0Q*v`%zY((c=-ArhEekZq!GQ2uJU-S9TcL4tciaamPwnIC<%#N;R`|x%%S!b!
> zuU<y~)aqq-cJ*@Sph?=jiRxDDA<BD7d5@R(Cdzw9l=sHv{mJr}@%hH;eWkq5%li}M
> z{Ugfz<MNfs@`;BiUs1|ec=^gi`N|RHE93Ij$@0mEC|^~|S9$sBMEU9w<*VcJwaN0S
> zhbUiD%GY@L+C=%<5#?*+^7YB`a}QCzu9UCy^7V=G^&`sHQ9ePhvdQv}C@)_g_^hC5
> zHa=gex49|)$YV#(^*G(+V{GO0bokboN+;T8rki>$^LjdbfJ~(mZ8OtNJ!hCBrRPiv
> z!&k{vI?*;W-PCjA>gn*QGL=rW%}h7-94zVS@clBCPPENTH+5ex>*?@OGnG!X%}h6S
> zpB~fG;Y(*KooLh3Ij5A#_=T`p#!?2BE$lev;++jl{EIb?iw|bdgH|P!cd3Pk6W)~d
> zg7by?n{W4ie*I^KYj3>u=JjhodE+hCF}z@7eSEiOEvshSweQ-7jcIRq63jAl*Tyve
> z{$muBSiz&&datn=8eXe;!SE_C7Y&y1yryq<_VWaadtqgx0Hj}P7ckA-*ld_RMva|=
> zttT#WDd>ynU$*20jbd@7NIej1GD$b*wX3hj23CKUec%;~#mjCz<)M1I^Vr3w6LOV+
> zG*aJ3qu%ahGm7<CG&tFxJVCwhSqo9;+f!#Y)`yYXOp>C9D`ZTjLWY_7-TJ&2dYk4^
> zGK}c+Ql=kaeIx3z$)p(JK`k*Qh+hc|*CZT+Z}g^m26WGeZm%Z>_32dndd^N|#iw%t
> zj$E^aHq+i#*tU-nI`eu>bry<ymuKkeS+U7hf?k<y#1PEvUFZGh>8`@VhaYZG+*#(3
> z?l0Vf%~Cs;u}p@)`t%X#&SOQeogp!7_wp6@6mH*IQ0Z2RtGGaX!HRBWx~h&#XZS-m
> zzNw4PF{hamH@sMXq!p~HZT;n=EQ{e!Hhv@8U{UIxea|*bv>SM;@%tL5@7afO7OOCf
> z{pUlx9mIM1upEu(!;*e6#?SE2R)pqgH2Nby!Pp<PGhd8Gf4(oyeDUJVCmv)IZIl4q
> z5!7oxim^b5(|7K(fL>Zy6ih&_SX0dgn?bPzx#C4b?aCWQtf^&$AM!U$Ayr5jSXgr&
> zzX(dW*3G9929o3WnQq=pSTPIJScsmbGt?932M>m^bMRF46;HhRv)h&Cd0fj?LSwy8
> zt>Pq|B)!L+t;a9&#jV=oVmUJt7K>QTqlMqj!FR&q^CG~A8w_?7uJ~Bbqk~m>u*MHk
> zfw$QPW$Ro!Xo}^)Y%{F3@%CC7*Vool)#2J}2f-?qZ@2H<w_fY)BLl)9Xl$C;1ce}I
> z@y#|JH47Wq7COXPWA<y{&>}cRMsw@J1=C$^2i48=Abso8UCiqY@v6qj6?V;L!JYfN
> zY>Zwp&{{Qze2Xrx<`XomI9kgm=`M3C$Gd*mv<Ed;taShWe}8}B)N_B@9G?57WBzVA
> z9Ax@e*f5v0Vj}~u<15B6yYkc;8$8*+V#ToFewE=(MSY2OaQeoMg}Jq%(TmXi?H=T{
> znpkuGA;9s$64u3g)cz6m#9-)Eb9L}o|1uKE<~4)Sag!ixxCIEaHZZ}*;%W@wD^{{c
> zW5WRg987~#L)5Dzfu1|EkN9WFyKBV=>`>>FUQ=(;FVO;-hSYKbNliWJkmniZ;szT$
> qD~9o3?dW@pf4yG)_37G$ul{}K$AdH<#<&q5EMgEPKcb5{Wd8@f&*N$U
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/data/acpi/q35/TPM2.tis b/tests/data/acpi/q35/TPM2.tis
> index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..7878a6e79a6a406d99ca1f5e9a528eb392b8d793 100644
> GIT binary patch
> literal 76
> wcmWFu@HO&bU|?V=a`Jcf2v%^42yhMoiZKGkKx`0=4A_u4U^Ym_e|8WP0Iiz{0RR91
>
> literal 0
> HcmV?d00001
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-02 13:39 ` Stefan Berger
@ 2020-06-02 14:43 ` Stefan Berger
2020-06-02 16:13 ` Auger Eric
1 sibling, 0 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 14:43 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/2/20 9:39 AM, Stefan Berger wrote:
> On 6/1/20 6:21 AM, Eric Auger wrote:
>> While writing tests for checking the content of TPM2 and DSDT
>> along with TPM-TIS instantiation I attempted to reuse the
>> framework used for TPM-TIS tests. However While dumping the
>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>> is maybe the other tests did not execute long enough to encounter
>> this. So I tentatively propose to remove the assert as it
>> does not seem to break other tests and enable the new ones.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> tests/qtest/tpm-emu.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>> index c43ac4aef8..298d0eec74 100644
>> --- a/tests/qtest/tpm-emu.c
>> +++ b/tests/qtest/tpm-emu.c
>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
> You should not have to remove this. The tests are skipped if swtpm
> does not support TPM 2 via --tpm2 option. This would be a very old
> swtpm version, though. So, all tests are run with --tpm2 option and
> any response received from the TPM would be a TPM 2 response that
> should have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other
> value you are seeing there.
In hw/tpm/tpm_util.c tpm_util_test_tpmdev() we have a probing function
that tries to determine whether the attached TPM is a TPM 1.2 or TPM 2.
It sends a TPM 2 command first and I would say this is the only command
we should see in this (fake) TPM emulation here. We respond with a TPM 2
command header [indicating failure], but the probing function should
then not proceed to probe with the TPM 1.2 command.
>> s->tpm_msg = g_realloc(s->tpm_msg, s->tpm_msg->len);
>> qio_channel_read(ioc, (char *)&s->tpm_msg->code,
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-02 13:39 ` Stefan Berger
2020-06-02 14:43 ` Stefan Berger
@ 2020-06-02 16:13 ` Auger Eric
2020-06-02 16:17 ` Stefan Berger
1 sibling, 1 reply; 27+ messages in thread
From: Auger Eric @ 2020-06-02 16:13 UTC (permalink / raw)
To: Stefan Berger, eric.auger.pro, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Hi Stefan,
On 6/2/20 3:39 PM, Stefan Berger wrote:
> On 6/1/20 6:21 AM, Eric Auger wrote:
>> While writing tests for checking the content of TPM2 and DSDT
>> along with TPM-TIS instantiation I attempted to reuse the
>> framework used for TPM-TIS tests. However While dumping the
>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>> is maybe the other tests did not execute long enough to encounter
>> this. So I tentatively propose to remove the assert as it
>> does not seem to break other tests and enable the new ones.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> tests/qtest/tpm-emu.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>> index c43ac4aef8..298d0eec74 100644
>> --- a/tests/qtest/tpm-emu.c
>> +++ b/tests/qtest/tpm-emu.c
>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
> You should not have to remove this. The tests are skipped if swtpm does
> not support TPM 2 via --tpm2 option. This would be a very old swtpm
> version, though. So, all tests are run with --tpm2 option and any
> response received from the TPM would be a TPM 2 response that should
> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other value you
> are seeing there.
If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
Thanks
Eric
>> s->tpm_msg = g_realloc(s->tpm_msg, s->tpm_msg->len);
>> qio_channel_read(ioc, (char *)&s->tpm_msg->code,
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-02 16:13 ` Auger Eric
@ 2020-06-02 16:17 ` Stefan Berger
2020-06-02 17:15 ` Auger Eric
2020-06-05 9:35 ` Auger Eric
0 siblings, 2 replies; 27+ messages in thread
From: Stefan Berger @ 2020-06-02 16:17 UTC (permalink / raw)
To: Auger Eric, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/2/20 12:13 PM, Auger Eric wrote:
> Hi Stefan,
>
> On 6/2/20 3:39 PM, Stefan Berger wrote:
>> On 6/1/20 6:21 AM, Eric Auger wrote:
>>> While writing tests for checking the content of TPM2 and DSDT
>>> along with TPM-TIS instantiation I attempted to reuse the
>>> framework used for TPM-TIS tests. However While dumping the
>>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>>> is maybe the other tests did not execute long enough to encounter
>>> this. So I tentatively propose to remove the assert as it
>>> does not seem to break other tests and enable the new ones.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> ---
>>> tests/qtest/tpm-emu.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>>> index c43ac4aef8..298d0eec74 100644
>>> --- a/tests/qtest/tpm-emu.c
>>> +++ b/tests/qtest/tpm-emu.c
>>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
>> You should not have to remove this. The tests are skipped if swtpm does
>> not support TPM 2 via --tpm2 option. This would be a very old swtpm
>> version, though. So, all tests are run with --tpm2 option and any
>> response received from the TPM would be a TPM 2 response that should
>> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other value you
>> are seeing there.
> If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
Stefan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-02 16:17 ` Stefan Berger
@ 2020-06-02 17:15 ` Auger Eric
2020-06-05 9:35 ` Auger Eric
1 sibling, 0 replies; 27+ messages in thread
From: Auger Eric @ 2020-06-02 17:15 UTC (permalink / raw)
To: Stefan Berger, eric.auger.pro, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Stefan,
On 6/2/20 6:17 PM, Stefan Berger wrote:
> On 6/2/20 12:13 PM, Auger Eric wrote:
>> Hi Stefan,
>>
>> On 6/2/20 3:39 PM, Stefan Berger wrote:
>>> On 6/1/20 6:21 AM, Eric Auger wrote:
>>>> While writing tests for checking the content of TPM2 and DSDT
>>>> along with TPM-TIS instantiation I attempted to reuse the
>>>> framework used for TPM-TIS tests. However While dumping the
>>>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>>>> is maybe the other tests did not execute long enough to encounter
>>>> this. So I tentatively propose to remove the assert as it
>>>> does not seem to break other tests and enable the new ones.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> ---
>>>> tests/qtest/tpm-emu.c | 1 -
>>>> 1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>>>> index c43ac4aef8..298d0eec74 100644
>>>> --- a/tests/qtest/tpm-emu.c
>>>> +++ b/tests/qtest/tpm-emu.c
>>>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>>>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>>>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>>>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>>>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
>>> You should not have to remove this. The tests are skipped if swtpm does
>>> not support TPM 2 via --tpm2 option. This would be a very old swtpm
>>> version, though. So, all tests are run with --tpm2 option and any
>>> response received from the TPM would be a TPM 2 response that should
>>> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other value you
>>> are seeing there.
>> If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
>
> Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
From what I understand
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
tests/qtest/bios-tables-test
launches
x86_64-softmmu/qemu-system-x86_64 \
-qtest unix:/tmp/qtest-20181.sock \
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-20181.qmp,id=char0
-mon chardev=char0,mode=control \
-display none -machine q35,kernel-irqchip=off \
-accel kvm -accel tcg \
-net none -display none \
-chardev socket,id=chr,path=/tmp/qemu-test_acpi_q35_tcg_tpm2.RJJCL0/sock
-tpmdev emulator,id=dev,chardev=chr \
-device tpm-tis,tpmdev=dev \
-drive id=hd0,if=none,file=tests/acpi-test-disk-RAnagW,format=raw
-device ide-hd,drive=hd0 -accel qtest
acpi-test-disk-RAnagW consists in an x86 boot sector code, created by
boot_sector_init (x86_boot_sector)
Thanks
Eric
>
> Stefan
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-02 16:17 ` Stefan Berger
2020-06-02 17:15 ` Auger Eric
@ 2020-06-05 9:35 ` Auger Eric
2020-06-05 15:25 ` Stefan Berger
1 sibling, 1 reply; 27+ messages in thread
From: Auger Eric @ 2020-06-05 9:35 UTC (permalink / raw)
To: Stefan Berger, eric.auger.pro, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Hi Stefan,
On 6/2/20 6:17 PM, Stefan Berger wrote:
> On 6/2/20 12:13 PM, Auger Eric wrote:
>> Hi Stefan,
>>
>> On 6/2/20 3:39 PM, Stefan Berger wrote:
>>> On 6/1/20 6:21 AM, Eric Auger wrote:
>>>> While writing tests for checking the content of TPM2 and DSDT
>>>> along with TPM-TIS instantiation I attempted to reuse the
>>>> framework used for TPM-TIS tests. However While dumping the
>>>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>>>> is maybe the other tests did not execute long enough to encounter
>>>> this. So I tentatively propose to remove the assert as it
>>>> does not seem to break other tests and enable the new ones.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> ---
>>>> tests/qtest/tpm-emu.c | 1 -
>>>> 1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>>>> index c43ac4aef8..298d0eec74 100644
>>>> --- a/tests/qtest/tpm-emu.c
>>>> +++ b/tests/qtest/tpm-emu.c
>>>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>>>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>>>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>>>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>>>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
>>> You should not have to remove this. The tests are skipped if swtpm does
>>> not support TPM 2 via --tpm2 option. This would be a very old swtpm
>>> version, though. So, all tests are run with --tpm2 option and any
>>> response received from the TPM would be a TPM 2 response that should
>>> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other value you
>>> are seeing there.
>> If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
>
> Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
So it looks SeaBIOS is in use (bios-256k.bin loaded).
I can see MMIO accesses to the TPM and the following commands are
observable:
tpm_emu_tpm_thread code=0x181 tag=0x8001 len=0xa
tpm_emu_tpm_thread code=0x144 tag=0x8001 len=0xc
tpm_emu_tpm_thread code=0x121 tag=0x8002 len=0x20
This last one causes the assert (TPM2_CC_HierarchyControl)
I checked in Seabios and effectively tpm20_hierarchycontrol() tags the
TPM2_CC_HierarchyControl command with TPM2_ST_SESSIONS
Due to our emulation, maybe tpm_set_failure() gets called, inducing
tpm20_hierarchycontrol() call.
That being said, what do you recommend? Remove the assert, improve the
emulation, other?
Thank you in advance
Best Regards
Eric
>
> Stefan
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test
2020-06-01 10:21 ` [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test Eric Auger
2020-06-02 14:08 ` Stefan Berger
@ 2020-06-05 14:54 ` Igor Mammedov
1 sibling, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2020-06-05 14:54 UTC (permalink / raw)
To: Eric Auger
Cc: peter.maydell, drjones, mst, philmd, qemu-devel, shannon.zhaosl,
qemu-arm, marcandre.lureau, eric.auger.pro, lersek, ardb,
stefanb
On Mon, 1 Jun 2020 12:21:09 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> Add placeholders for TPM and DSDT reference tables for
> Q35 TPM-TIS tests.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/data/acpi/q35/DSDT.tis | Bin
> tests/data/acpi/q35/TPM2.tis | Bin
> 2 files changed, 0 insertions(+), 0 deletions(-)
> create mode 100644 tests/data/acpi/q35/DSDT.tis
> create mode 100644 tests/data/acpi/q35/TPM2.tis
>
> diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
> new file mode 100644
> index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> diff --git a/tests/data/acpi/q35/TPM2.tis b/tests/data/acpi/q35/TPM2.tis
> new file mode 100644
> index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis
2020-06-01 10:21 ` [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis Eric Auger
2020-06-02 14:09 ` Stefan Berger
@ 2020-06-05 14:55 ` Igor Mammedov
1 sibling, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2020-06-05 14:55 UTC (permalink / raw)
To: Eric Auger
Cc: peter.maydell, drjones, mst, philmd, qemu-devel, shannon.zhaosl,
qemu-arm, marcandre.lureau, eric.auger.pro, lersek, ardb,
stefanb
On Mon, 1 Jun 2020 12:21:10 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> Subsequent patches will alter the content of TPM2.tis
> and DSDT.tis so let's ignore them until the references
> are generated.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..a2a45d1d31 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,3 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT.tis",
> +"tests/data/acpi/q35/TPM2.tis",
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test
2020-06-01 10:21 ` [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test Eric Auger
2020-06-02 14:38 ` Stefan Berger
@ 2020-06-05 15:17 ` Igor Mammedov
2020-06-09 12:10 ` Auger Eric
1 sibling, 1 reply; 27+ messages in thread
From: Igor Mammedov @ 2020-06-05 15:17 UTC (permalink / raw)
To: Eric Auger
Cc: peter.maydell, drjones, mst, philmd, qemu-devel, shannon.zhaosl,
qemu-arm, marcandre.lureau, eric.auger.pro, lersek, ardb,
stefanb
On Mon, 1 Jun 2020 12:21:12 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> Test tables specific to the TPM-TIS instantiation.
> The TPM2 is added in the framework. Also the DSDT
> is updated with the TPM. The new function should be
> be usable for CRB as well, later one.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 60 ++++++++++++++++++++++++++++++++++
> tests/qtest/Makefile.include | 1 +
> 2 files changed, 61 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index c9843829b3..bbba98342c 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -57,6 +57,9 @@
> #include "qemu/bitmap.h"
> #include "acpi-utils.h"
> #include "boot-sector.h"
> +#include "tpm-emu.h"
> +#include "hw/acpi/tpm.h"
> +
>
> #define MACHINE_PC "pc"
> #define MACHINE_Q35 "q35"
> @@ -874,6 +877,60 @@ static void test_acpi_piix4_tcg_numamem(void)
> free_test_data(&data);
> }
>
> +uint64_t tpm_tis_base_addr;
> +
> +struct tpm_test_data {
> + const char *machine;
> + const char *tpm_if;
> +};
> +
> +static void test_acpi_tcg_tpm(const void *context)
> +{
s/test_acpi_tcg_tpm/test_acpi_q35_tcg_tpm/
I'd try to keep test specific parameter within test function isnstead of pushing it up to main(),
drawback would be some code duplication for intializing test data and calling runner
but it's trivial and worked well so far. See for example test_acpi_piix4_tcg_bridge/test_acpi_q35_tcg_bridge. I might seem a waste but it's consictent with what we were doing
with bios tests.
> + struct tpm_test_data *c = (struct tpm_test_data *)context;
> + gchar *tmp_dir_name = g_strdup_printf("qemu-test_acpi_%s_tcg_%s.XXXXXX",
> + c->machine, c->tpm_if);
> + char *tmp_path = g_dir_make_tmp(tmp_dir_name, NULL);
> + TestState test;
> + test_data data;
> + GThread *thread;
> + char *args, *variant = g_strdup_printf(".%s", c->tpm_if);
maybe derive tpm_if from '.variant' if it's necessary at all?
> +
> + tpm_tis_base_addr = TPM_TIS_ADDR_BASE;
hardcode it here, so in case QEMU regresses, test could notice?
> +
> + module_call_init(MODULE_INIT_QOM);
why it's here?
> +
> + test.addr = g_new0(SocketAddress, 1);
> + test.addr->type = SOCKET_ADDRESS_TYPE_UNIX;
> + test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
> + g_mutex_init(&test.data_mutex);
> + g_cond_init(&test.data_cond);
> + test.data_cond_signal = false;
> +
> + thread = g_thread_new(NULL, tpm_emu_ctrl_thread, &test);
> + tpm_emu_test_wait_cond(&test);
perhaps make a separate helper function from this chunk
so it could be reused from other TMP test functions.
> +
> + memset(&data, 0, sizeof(data));
I'd init fields with initializer, see test_acpi_virt_tcg_numamem()
> + data.machine = c->machine;
> + data.variant = variant;
> +
> + args = g_strdup_printf(
> + " -chardev socket,id=chr,path=%s"
> + " -tpmdev emulator,id=dev,chardev=chr"
> + " -device tpm-%s,tpmdev=dev",
> + test.addr->u.q_unix.path, c->tpm_if);
> +
> + test_acpi_one(args, &data);
> +
> + g_thread_join(thread);
> + g_unlink(test.addr->u.q_unix.path);
> + qapi_free_SocketAddress(test.addr);
> + g_rmdir(tmp_path);
> + g_free(variant);
> + g_free(tmp_path);
> + g_free(tmp_dir_name);
> + free_test_data(&data);
> +}
> +
> static void test_acpi_tcg_dimm_pxm(const char *machine)
> {
> test_data data;
> @@ -1028,6 +1085,7 @@ int main(int argc, char *argv[])
> {
> const char *arch = qtest_get_arch();
> int ret;
> + struct tpm_test_data tpm_q35_tis = {MACHINE_Q35, "tis"};
I'd hide this within test_acpi_tcg_tpm() as it's done in other test functions
>
> g_test_init(&argc, &argv, NULL);
>
> @@ -1037,6 +1095,8 @@ int main(int argc, char *argv[])
> return ret;
> }
>
> + qtest_add_data_func("acpi/q35/tpm-tis",
> + &tpm_q35_tis, test_acpi_tcg_tpm);
> qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
> qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
> qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
> index 9e5a51d033..5023fa413d 100644
> --- a/tests/qtest/Makefile.include
> +++ b/tests/qtest/Makefile.include
> @@ -262,6 +262,7 @@ tests/qtest/hd-geo-test$(EXESUF): tests/qtest/hd-geo-test.o $(libqos-obj-y)
> tests/qtest/boot-order-test$(EXESUF): tests/qtest/boot-order-test.o $(libqos-obj-y)
> tests/qtest/boot-serial-test$(EXESUF): tests/qtest/boot-serial-test.o $(libqos-obj-y)
> tests/qtest/bios-tables-test$(EXESUF): tests/qtest/bios-tables-test.o \
> + tests/qtest/tpm-emu.o $(test-io-obj-y) \
> tests/qtest/boot-sector.o tests/qtest/acpi-utils.o $(libqos-obj-y)
> tests/qtest/pxe-test$(EXESUF): tests/qtest/pxe-test.o tests/qtest/boot-sector.o $(libqos-obj-y)
> tests/qtest/microbit-test$(EXESUF): tests/qtest/microbit-test.o
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-05 9:35 ` Auger Eric
@ 2020-06-05 15:25 ` Stefan Berger
2020-06-05 15:47 ` Auger Eric
0 siblings, 1 reply; 27+ messages in thread
From: Stefan Berger @ 2020-06-05 15:25 UTC (permalink / raw)
To: Auger Eric, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
On 6/5/20 5:35 AM, Auger Eric wrote:
> Hi Stefan,
>
> On 6/2/20 6:17 PM, Stefan Berger wrote:
>> On 6/2/20 12:13 PM, Auger Eric wrote:
>>> Hi Stefan,
>>>
>>> On 6/2/20 3:39 PM, Stefan Berger wrote:
>>>> On 6/1/20 6:21 AM, Eric Auger wrote:
>>>>> While writing tests for checking the content of TPM2 and DSDT
>>>>> along with TPM-TIS instantiation I attempted to reuse the
>>>>> framework used for TPM-TIS tests. However While dumping the
>>>>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>>>>> is maybe the other tests did not execute long enough to encounter
>>>>> this. So I tentatively propose to remove the assert as it
>>>>> does not seem to break other tests and enable the new ones.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>> ---
>>>>> tests/qtest/tpm-emu.c | 1 -
>>>>> 1 file changed, 1 deletion(-)
>>>>>
>>>>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>>>>> index c43ac4aef8..298d0eec74 100644
>>>>> --- a/tests/qtest/tpm-emu.c
>>>>> +++ b/tests/qtest/tpm-emu.c
>>>>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>>>>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>>>>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>>>>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>>>>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
>>>> You should not have to remove this. The tests are skipped if swtpm does
>>>> not support TPM 2 via --tpm2 option. This would be a very old swtpm
>>>> version, though. So, all tests are run with --tpm2 option and any
>>>> response received from the TPM would be a TPM 2 response that should
>>>> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other value you
>>>> are seeing there.
>>> If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
>> Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
> So it looks SeaBIOS is in use (bios-256k.bin loaded).
>
> I can see MMIO accesses to the TPM and the following commands are
> observable:
> tpm_emu_tpm_thread code=0x181 tag=0x8001 len=0xa
> tpm_emu_tpm_thread code=0x144 tag=0x8001 len=0xc
> tpm_emu_tpm_thread code=0x121 tag=0x8002 len=0x20
> This last one causes the assert (TPM2_CC_HierarchyControl)
>
> I checked in Seabios and effectively tpm20_hierarchycontrol() tags the
> TPM2_CC_HierarchyControl command with TPM2_ST_SESSIONS
>
> Due to our emulation, maybe tpm_set_failure() gets called, inducing
> tpm20_hierarchycontrol() call.
>
> That being said, what do you recommend? Remove the assert, improve the
> emulation, other?
So this is an ACPI test. What role does the firmware play for success of
the test? If the test relies on the firmware showing some sort of
expected result, then I would recommend only running this test with an
attached swtpm, like we run some other tests. If we don't need the
firmware to succeed then I would just get rid of the assert. Probably no
other test we have implemented so far was running the firmware...
Stefan
>
> Thank you in advance
>
> Best Regards
>
> Eric
>
>> Stefan
>>
>>
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-05 15:25 ` Stefan Berger
@ 2020-06-05 15:47 ` Auger Eric
2020-06-08 8:34 ` Igor Mammedov
0 siblings, 1 reply; 27+ messages in thread
From: Auger Eric @ 2020-06-05 15:47 UTC (permalink / raw)
To: Stefan Berger, eric.auger.pro, qemu-devel, qemu-arm,
peter.maydell, mst, shannon.zhaosl, imammedo
Cc: marcandre.lureau, drjones, lersek, ardb, philmd
Hi Stefan,
On 6/5/20 5:25 PM, Stefan Berger wrote:
> On 6/5/20 5:35 AM, Auger Eric wrote:
>> Hi Stefan,
>>
>> On 6/2/20 6:17 PM, Stefan Berger wrote:
>>> On 6/2/20 12:13 PM, Auger Eric wrote:
>>>> Hi Stefan,
>>>>
>>>> On 6/2/20 3:39 PM, Stefan Berger wrote:
>>>>> On 6/1/20 6:21 AM, Eric Auger wrote:
>>>>>> While writing tests for checking the content of TPM2 and DSDT
>>>>>> along with TPM-TIS instantiation I attempted to reuse the
>>>>>> framework used for TPM-TIS tests. However While dumping the
>>>>>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>>>>>> is maybe the other tests did not execute long enough to encounter
>>>>>> this. So I tentatively propose to remove the assert as it
>>>>>> does not seem to break other tests and enable the new ones.
>>>>>>
>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>> ---
>>>>>> tests/qtest/tpm-emu.c | 1 -
>>>>>> 1 file changed, 1 deletion(-)
>>>>>>
>>>>>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>>>>>> index c43ac4aef8..298d0eec74 100644
>>>>>> --- a/tests/qtest/tpm-emu.c
>>>>>> +++ b/tests/qtest/tpm-emu.c
>>>>>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>>>>>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>>>>>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>>>>>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>>>>>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
>>>>> You should not have to remove this. The tests are skipped if swtpm
>>>>> does
>>>>> not support TPM 2 via --tpm2 option. This would be a very old swtpm
>>>>> version, though. So, all tests are run with --tpm2 option and any
>>>>> response received from the TPM would be a TPM 2 response that should
>>>>> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other
>>>>> value you
>>>>> are seeing there.
>>>> If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
>>> Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
>> So it looks SeaBIOS is in use (bios-256k.bin loaded).
>>
>> I can see MMIO accesses to the TPM and the following commands are
>> observable:
>> tpm_emu_tpm_thread code=0x181 tag=0x8001 len=0xa
>> tpm_emu_tpm_thread code=0x144 tag=0x8001 len=0xc
>> tpm_emu_tpm_thread code=0x121 tag=0x8002 len=0x20
>> This last one causes the assert (TPM2_CC_HierarchyControl)
>>
>> I checked in Seabios and effectively tpm20_hierarchycontrol() tags the
>> TPM2_CC_HierarchyControl command with TPM2_ST_SESSIONS
>>
>> Due to our emulation, maybe tpm_set_failure() gets called, inducing
>> tpm20_hierarchycontrol() call.
>>
>> That being said, what do you recommend? Remove the assert, improve the
>> emulation, other?
>
> So this is an ACPI test. What role does the firmware play for success of
> the test? If the test relies on the firmware showing some sort of
> expected result, then I would recommend only running this test with an
> attached swtpm, like we run some other tests. If we don't need the
> firmware to succeed then I would just get rid of the assert. Probably no
> other test we have implemented so far was running the firmware...
FWIU The goal of this test is to compare the acpi tables generated by
qemu against reference ones. I dont think we expect from the FW any
specific result but I would prefer Igor or Michael to confirm.
In that case, removing the assert() allows to compare the specific DSDT
and TPM2 tables and that's our expectation here I think.
Thanks
Eric
>
>
> Stefan
>
>
>>
>> Thank you in advance
>>
>> Best Regards
>>
>> Eric
>>
>>> Stefan
>>>
>>>
>>>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-05 15:47 ` Auger Eric
@ 2020-06-08 8:34 ` Igor Mammedov
2020-06-08 9:11 ` Auger Eric
0 siblings, 1 reply; 27+ messages in thread
From: Igor Mammedov @ 2020-06-08 8:34 UTC (permalink / raw)
To: Auger Eric
Cc: peter.maydell, drjones, mst, philmd, qemu-devel, shannon.zhaosl,
qemu-arm, marcandre.lureau, Stefan Berger, lersek, ardb,
eric.auger.pro
On Fri, 5 Jun 2020 17:47:08 +0200
Auger Eric <eric.auger@redhat.com> wrote:
> Hi Stefan,
>
> On 6/5/20 5:25 PM, Stefan Berger wrote:
> > On 6/5/20 5:35 AM, Auger Eric wrote:
> >> Hi Stefan,
> >>
> >> On 6/2/20 6:17 PM, Stefan Berger wrote:
> >>> On 6/2/20 12:13 PM, Auger Eric wrote:
> >>>> Hi Stefan,
> >>>>
> >>>> On 6/2/20 3:39 PM, Stefan Berger wrote:
> >>>>> On 6/1/20 6:21 AM, Eric Auger wrote:
> >>>>>> While writing tests for checking the content of TPM2 and DSDT
> >>>>>> along with TPM-TIS instantiation I attempted to reuse the
> >>>>>> framework used for TPM-TIS tests. However While dumping the
> >>>>>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
> >>>>>> is maybe the other tests did not execute long enough to encounter
> >>>>>> this. So I tentatively propose to remove the assert as it
> >>>>>> does not seem to break other tests and enable the new ones.
> >>>>>>
> >>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>>>>> ---
> >>>>>> tests/qtest/tpm-emu.c | 1 -
> >>>>>> 1 file changed, 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
> >>>>>> index c43ac4aef8..298d0eec74 100644
> >>>>>> --- a/tests/qtest/tpm-emu.c
> >>>>>> +++ b/tests/qtest/tpm-emu.c
> >>>>>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
> >>>>>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
> >>>>>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
> >>>>>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
> >>>>>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
> >>>>> You should not have to remove this. The tests are skipped if swtpm
> >>>>> does
> >>>>> not support TPM 2 via --tpm2 option. This would be a very old swtpm
> >>>>> version, though. So, all tests are run with --tpm2 option and any
> >>>>> response received from the TPM would be a TPM 2 response that should
> >>>>> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other
> >>>>> value you
> >>>>> are seeing there.
> >>>> If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
> >>> Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
> >> So it looks SeaBIOS is in use (bios-256k.bin loaded).
> >>
> >> I can see MMIO accesses to the TPM and the following commands are
> >> observable:
> >> tpm_emu_tpm_thread code=0x181 tag=0x8001 len=0xa
> >> tpm_emu_tpm_thread code=0x144 tag=0x8001 len=0xc
> >> tpm_emu_tpm_thread code=0x121 tag=0x8002 len=0x20
> >> This last one causes the assert (TPM2_CC_HierarchyControl)
> >>
> >> I checked in Seabios and effectively tpm20_hierarchycontrol() tags the
> >> TPM2_CC_HierarchyControl command with TPM2_ST_SESSIONS
> >>
> >> Due to our emulation, maybe tpm_set_failure() gets called, inducing
> >> tpm20_hierarchycontrol() call.
> >>
> >> That being said, what do you recommend? Remove the assert, improve the
> >> emulation, other?
> >
> > So this is an ACPI test. What role does the firmware play for success of
> > the test? If the test relies on the firmware showing some sort of
> > expected result, then I would recommend only running this test with an
> > attached swtpm, like we run some other tests. If we don't need the
> > firmware to succeed then I would just get rid of the assert. Probably no
> > other test we have implemented so far was running the firmware...
> FWIU The goal of this test is to compare the acpi tables generated by
> qemu against reference ones. I dont think we expect from the FW any
> specific result but I would prefer Igor or Michael to confirm.
Firmware is needed to fetch tables from QEMU and place them in guest RAM,
it will also patch cross table pointers accordingly.
So bios-tables-test checks both QEMU and firmware at the same time,
and reference tables are in form guest OS will see them.
>
> In that case, removing the assert() allows to compare the specific DSDT
> and TPM2 tables and that's our expectation here I think.
>
> Thanks
>
> Eric
> >
> >
> > Stefan
> >
> >
> >>
> >> Thank you in advance
> >>
> >> Best Regards
> >>
> >> Eric
> >>
> >>> Stefan
> >>>
> >>>
> >>>
> >
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS
2020-06-08 8:34 ` Igor Mammedov
@ 2020-06-08 9:11 ` Auger Eric
0 siblings, 0 replies; 27+ messages in thread
From: Auger Eric @ 2020-06-08 9:11 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, drjones, mst, lersek, qemu-devel, shannon.zhaosl,
qemu-arm, marcandre.lureau, eric.auger.pro, philmd, ardb,
Stefan Berger
Hi Igor,
On 6/8/20 10:34 AM, Igor Mammedov wrote:
> On Fri, 5 Jun 2020 17:47:08 +0200
> Auger Eric <eric.auger@redhat.com> wrote:
>
>> Hi Stefan,
>>
>> On 6/5/20 5:25 PM, Stefan Berger wrote:
>>> On 6/5/20 5:35 AM, Auger Eric wrote:
>>>> Hi Stefan,
>>>>
>>>> On 6/2/20 6:17 PM, Stefan Berger wrote:
>>>>> On 6/2/20 12:13 PM, Auger Eric wrote:
>>>>>> Hi Stefan,
>>>>>>
>>>>>> On 6/2/20 3:39 PM, Stefan Berger wrote:
>>>>>>> On 6/1/20 6:21 AM, Eric Auger wrote:
>>>>>>>> While writing tests for checking the content of TPM2 and DSDT
>>>>>>>> along with TPM-TIS instantiation I attempted to reuse the
>>>>>>>> framework used for TPM-TIS tests. However While dumping the
>>>>>>>> ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
>>>>>>>> is maybe the other tests did not execute long enough to encounter
>>>>>>>> this. So I tentatively propose to remove the assert as it
>>>>>>>> does not seem to break other tests and enable the new ones.
>>>>>>>>
>>>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>>>> ---
>>>>>>>> tests/qtest/tpm-emu.c | 1 -
>>>>>>>> 1 file changed, 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
>>>>>>>> index c43ac4aef8..298d0eec74 100644
>>>>>>>> --- a/tests/qtest/tpm-emu.c
>>>>>>>> +++ b/tests/qtest/tpm-emu.c
>>>>>>>> @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
>>>>>>>> s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
>>>>>>>> s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
>>>>>>>> g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
>>>>>>>> - g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
>>>>>>> You should not have to remove this. The tests are skipped if swtpm
>>>>>>> does
>>>>>>> not support TPM 2 via --tpm2 option. This would be a very old swtpm
>>>>>>> version, though. So, all tests are run with --tpm2 option and any
>>>>>>> response received from the TPM would be a TPM 2 response that should
>>>>>>> have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other
>>>>>>> value you
>>>>>>> are seeing there.
>>>>>> If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
>>>>> Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
>>>> So it looks SeaBIOS is in use (bios-256k.bin loaded).
>>>>
>>>> I can see MMIO accesses to the TPM and the following commands are
>>>> observable:
>>>> tpm_emu_tpm_thread code=0x181 tag=0x8001 len=0xa
>>>> tpm_emu_tpm_thread code=0x144 tag=0x8001 len=0xc
>>>> tpm_emu_tpm_thread code=0x121 tag=0x8002 len=0x20
>>>> This last one causes the assert (TPM2_CC_HierarchyControl)
>>>>
>>>> I checked in Seabios and effectively tpm20_hierarchycontrol() tags the
>>>> TPM2_CC_HierarchyControl command with TPM2_ST_SESSIONS
>>>>
>>>> Due to our emulation, maybe tpm_set_failure() gets called, inducing
>>>> tpm20_hierarchycontrol() call.
>>>>
>>>> That being said, what do you recommend? Remove the assert, improve the
>>>> emulation, other?
>>>
>>> So this is an ACPI test. What role does the firmware play for success of
>>> the test? If the test relies on the firmware showing some sort of
>>> expected result, then I would recommend only running this test with an
>>> attached swtpm, like we run some other tests. If we don't need the
>>> firmware to succeed then I would just get rid of the assert. Probably no
>>> other test we have implemented so far was running the firmware...
>> FWIU The goal of this test is to compare the acpi tables generated by
>> qemu against reference ones. I dont think we expect from the FW any
>> specific result but I would prefer Igor or Michael to confirm.
>
> Firmware is needed to fetch tables from QEMU and place them in guest RAM,
> it will also patch cross table pointers accordingly.
>
> So bios-tables-test checks both QEMU and firmware at the same time,
> and reference tables are in form guest OS will see them.
Thank you for your input. I guess Stefan's concern was: is that an issue
if the FW accesses to the TPM silently fail at some (assumed late)
point. I guess the job we expect from the FW, ie. copy the ACPI tables
and patch tables, is done prior to that failure, hence the test success.
So this functional failure should not be an issue for bios-table-test,
right?
Thanks
Eric
>
>>
>> In that case, removing the assert() allows to compare the specific DSDT
>> and TPM2 tables and that's our expectation here I think.
>>
>> Thanks
>>
>> Eric
>>>
>>>
>>> Stefan
>>>
>>>
>>>>
>>>> Thank you in advance
>>>>
>>>> Best Regards
>>>>
>>>> Eric
>>>>
>>>>> Stefan
>>>>>
>>>>>
>>>>>
>>>
>>>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test
2020-06-05 15:17 ` Igor Mammedov
@ 2020-06-09 12:10 ` Auger Eric
0 siblings, 0 replies; 27+ messages in thread
From: Auger Eric @ 2020-06-09 12:10 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, drjones, mst, philmd, qemu-devel, shannon.zhaosl,
qemu-arm, marcandre.lureau, eric.auger.pro, lersek, ardb,
stefanb
Hi Igor,
On 6/5/20 5:17 PM, Igor Mammedov wrote:
> On Mon, 1 Jun 2020 12:21:12 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Test tables specific to the TPM-TIS instantiation.
>> The TPM2 is added in the framework. Also the DSDT
>> is updated with the TPM. The new function should be
>> be usable for CRB as well, later one.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> tests/qtest/bios-tables-test.c | 60 ++++++++++++++++++++++++++++++++++
>> tests/qtest/Makefile.include | 1 +
>> 2 files changed, 61 insertions(+)
>>
>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>> index c9843829b3..bbba98342c 100644
>> --- a/tests/qtest/bios-tables-test.c
>> +++ b/tests/qtest/bios-tables-test.c
>> @@ -57,6 +57,9 @@
>> #include "qemu/bitmap.h"
>> #include "acpi-utils.h"
>> #include "boot-sector.h"
>> +#include "tpm-emu.h"
>> +#include "hw/acpi/tpm.h"
>> +
>>
>> #define MACHINE_PC "pc"
>> #define MACHINE_Q35 "q35"
>> @@ -874,6 +877,60 @@ static void test_acpi_piix4_tcg_numamem(void)
>> free_test_data(&data);
>> }
>>
>> +uint64_t tpm_tis_base_addr;
>> +
>> +struct tpm_test_data {
>> + const char *machine;
>> + const char *tpm_if;
>> +};
>> +
>> +static void test_acpi_tcg_tpm(const void *context)
>> +{
>
> s/test_acpi_tcg_tpm/test_acpi_q35_tcg_tpm/
>
> I'd try to keep test specific parameter within test function isnstead of pushing it up to main(),
> drawback would be some code duplication for intializing test data and calling runner
> but it's trivial and worked well so far. See for example test_acpi_piix4_tcg_bridge/test_acpi_q35_tcg_bridge. I might seem a waste but it's consictent with what we were doing
> with bios tests.
OK
>
>
>> + struct tpm_test_data *c = (struct tpm_test_data *)context;
>> + gchar *tmp_dir_name = g_strdup_printf("qemu-test_acpi_%s_tcg_%s.XXXXXX",
>> + c->machine, c->tpm_if);
>> + char *tmp_path = g_dir_make_tmp(tmp_dir_name, NULL);
>> + TestState test;
>> + test_data data;
>> + GThread *thread;
>> + char *args, *variant = g_strdup_printf(".%s", c->tpm_if);
> maybe derive tpm_if from '.variant' if it's necessary at all?
>
>> +
>> + tpm_tis_base_addr = TPM_TIS_ADDR_BASE;
> hardcode it here, so in case QEMU regresses, test could notice?
>
>> +
>> + module_call_init(MODULE_INIT_QOM);
> why it's here
Without it, I get
/x86_64/acpi/q35/tpm-tis: **
ERROR:qom/object.c:677:object_new_with_type: assertion failed: (type !=
NULL)
Thanks
Eric
>
>> +
>> + test.addr = g_new0(SocketAddress, 1);
>> + test.addr->type = SOCKET_ADDRESS_TYPE_UNIX;
>> + test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
>> + g_mutex_init(&test.data_mutex);
>> + g_cond_init(&test.data_cond);
>> + test.data_cond_signal = false;
>> +
>> + thread = g_thread_new(NULL, tpm_emu_ctrl_thread, &test);
>> + tpm_emu_test_wait_cond(&test);
> perhaps make a separate helper function from this chunk
> so it could be reused from other TMP test functions.
>
>> +
>> + memset(&data, 0, sizeof(data));
> I'd init fields with initializer, see test_acpi_virt_tcg_numamem()
>
>> + data.machine = c->machine;
>> + data.variant = variant;
>> +
>> + args = g_strdup_printf(
>> + " -chardev socket,id=chr,path=%s"
>> + " -tpmdev emulator,id=dev,chardev=chr"
>> + " -device tpm-%s,tpmdev=dev",
>> + test.addr->u.q_unix.path, c->tpm_if);
>> +
>> + test_acpi_one(args, &data);
>> +
>> + g_thread_join(thread);
>> + g_unlink(test.addr->u.q_unix.path);
>> + qapi_free_SocketAddress(test.addr);
>> + g_rmdir(tmp_path);
>> + g_free(variant);
>> + g_free(tmp_path);
>> + g_free(tmp_dir_name);
>> + free_test_data(&data);
>> +}
>> +
>> static void test_acpi_tcg_dimm_pxm(const char *machine)
>> {
>> test_data data;
>> @@ -1028,6 +1085,7 @@ int main(int argc, char *argv[])
>> {
>> const char *arch = qtest_get_arch();
>> int ret;
>> + struct tpm_test_data tpm_q35_tis = {MACHINE_Q35, "tis"};
>
> I'd hide this within test_acpi_tcg_tpm() as it's done in other test functions
>
>>
>> g_test_init(&argc, &argv, NULL);
>>
>> @@ -1037,6 +1095,8 @@ int main(int argc, char *argv[])
>> return ret;
>> }
>>
>> + qtest_add_data_func("acpi/q35/tpm-tis",
>> + &tpm_q35_tis, test_acpi_tcg_tpm);
>> qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
>> qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
>> qtest_add_func("acpi/q35", test_acpi_q35_tcg);
>> diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
>> index 9e5a51d033..5023fa413d 100644
>> --- a/tests/qtest/Makefile.include
>> +++ b/tests/qtest/Makefile.include
>> @@ -262,6 +262,7 @@ tests/qtest/hd-geo-test$(EXESUF): tests/qtest/hd-geo-test.o $(libqos-obj-y)
>> tests/qtest/boot-order-test$(EXESUF): tests/qtest/boot-order-test.o $(libqos-obj-y)
>> tests/qtest/boot-serial-test$(EXESUF): tests/qtest/boot-serial-test.o $(libqos-obj-y)
>> tests/qtest/bios-tables-test$(EXESUF): tests/qtest/bios-tables-test.o \
>> + tests/qtest/tpm-emu.o $(test-io-obj-y) \
>> tests/qtest/boot-sector.o tests/qtest/acpi-utils.o $(libqos-obj-y)
>> tests/qtest/pxe-test$(EXESUF): tests/qtest/pxe-test.o tests/qtest/boot-sector.o $(libqos-obj-y)
>> tests/qtest/microbit-test$(EXESUF): tests/qtest/microbit-test.o
>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-06-09 12:11 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-01 10:21 [RFC 0/6] TPM-TIS bios-tables-test Eric Auger
2020-06-01 10:21 ` [RFC 1/6] test/tpm-emu: include sockets and channel headers in tpm-emu header Eric Auger
2020-06-02 14:08 ` Stefan Berger
2020-06-01 10:21 ` [RFC 2/6] tests/acpi: Add void tables for Q35/TPM-TIS bios-tables-test Eric Auger
2020-06-02 14:08 ` Stefan Berger
2020-06-05 14:54 ` Igor Mammedov
2020-06-01 10:21 ` [RFC 3/6] tests/acpi: Ignore TPM2.tis and DSDT.tis Eric Auger
2020-06-02 14:09 ` Stefan Berger
2020-06-05 14:55 ` Igor Mammedov
2020-06-01 10:21 ` [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS Eric Auger
2020-06-02 13:39 ` Stefan Berger
2020-06-02 14:43 ` Stefan Berger
2020-06-02 16:13 ` Auger Eric
2020-06-02 16:17 ` Stefan Berger
2020-06-02 17:15 ` Auger Eric
2020-06-05 9:35 ` Auger Eric
2020-06-05 15:25 ` Stefan Berger
2020-06-05 15:47 ` Auger Eric
2020-06-08 8:34 ` Igor Mammedov
2020-06-08 9:11 ` Auger Eric
2020-06-01 10:21 ` [RFC 5/6] bios-tables-test: Add Q35/TPM-TIS test Eric Auger
2020-06-02 14:38 ` Stefan Berger
2020-06-05 15:17 ` Igor Mammedov
2020-06-09 12:10 ` Auger Eric
2020-06-01 10:21 ` [RFC 6/6] bios-tables-test: Generate reference tables for Q35/TPM-TIS Eric Auger
2020-06-02 14:39 ` Stefan Berger
2020-06-02 11:42 ` [RFC 0/6] TPM-TIS bios-tables-test Auger Eric
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.