| Message ID | 20200601102113.1207-6-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | TPM-TIS bios-tables-test | expand |
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>
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
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 >
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
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(+)