| Message ID | 1553605799-168605-11-git-send-email-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests: acpi: add UEFI (ARM) testing support | expand |
On 03/26/19 14:09, Igor Mammedov wrote: > adds simple arm/virt test case that starts guest with > bios-tables-test.aarch64.iso.qcow2 boot image which > initializes UefiTestSupport* structure in RAM once > guest is booted. > > * see commit: tests: acpi: add acpi_find_rsdp_address_uefi() helper > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v3: > * use firmware blobs directly from pc-bios directory > * use bios-tables-test.aarch64.iso.qcow2 as test boot image > v2: > * specify in test_data where board's RAM starts and RAM size > --- > tests/Makefile.include | 1 + > tests/bios-tables-test.c | 17 +++++++++++++++++ > 2 files changed, 18 insertions(+) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 0a0e1c3..0619751 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -269,6 +269,7 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF) > check-qtest-aarch64-y = tests/numa-test$(EXESUF) > check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF) > check-qtest-aarch64-y += tests/migration-test$(EXESUF) > +qtest-uefi-images-aarch64 = edk2-aarch64-code.fd.xz edk2-arm-vars.fd.xz > > check-qtest-microblazeel-y += $(check-qtest-microblaze-y) > What is this hunk needed for? I'm asking because it can't work on top of my v3 posting ("[PATCH for-4.1 v3 00/12] bundle edk2 platform firmware with QEMU") -- that version provides *.bz2 files, not *.xz files. In addition, the rest of your patch refers to the decompressed (*.fd) images -- and that is correct, in fact, given that my patches * [PATCH for-4.1 v3 10/12] tests: add missing dependency to build QTEST_QEMU_BINARY, round 2 https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06140.html * [PATCH for-4.1 v3 11/12] Makefile: install the edk2 firmware images and their descriptors https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06138.html make sure that the images are decompressed for testing too. So, in the end, this hunk should be dropped, IMO. > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > index 097ef03..b5cb425 100644 > --- a/tests/bios-tables-test.c > +++ b/tests/bios-tables-test.c > @@ -804,6 +804,21 @@ static void test_acpi_piix4_tcg_dimm_pxm(void) > test_acpi_tcg_dimm_pxm(MACHINE_PC); > } > > +static void test_acpi_virt_tcg(void) > +{ > + test_data data = { > + .machine = "virt", > + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", > + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", > + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", > + .ram_start = 0x40000000ULL, > + .scan_len = 128ULL * 1024 * 1024, > + }; (Feel free to ignore this one:) you might want to beautify both constants using the macros from "qemu/units.h": 1 * GiB, and 128 * MiB. > + > + test_acpi_one("-cpu cortex-a57 ", &data); > + free_test_data(&data); > +} > + > int main(int argc, char *argv[]) > { > const char *arch = qtest_get_arch(); > @@ -831,6 +846,8 @@ int main(int argc, char *argv[]) > qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem); > qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm); > qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm); > + } else if (strcmp(arch, "aarch64") == 0) { > + qtest_add_func("acpi/virt", test_acpi_virt_tcg); > } > ret = g_test_run(); > boot_sector_cleanup(disk); > With the Makefile.include hunk dropped (and regardless of the constants): Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks, Laszlo
On Tue, 26 Mar 2019 15:27:39 +0100 Laszlo Ersek <lersek@redhat.com> wrote: > On 03/26/19 14:09, Igor Mammedov wrote: > > adds simple arm/virt test case that starts guest with > > bios-tables-test.aarch64.iso.qcow2 boot image which > > initializes UefiTestSupport* structure in RAM once > > guest is booted. > > > > * see commit: tests: acpi: add acpi_find_rsdp_address_uefi() helper > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v3: > > * use firmware blobs directly from pc-bios directory > > * use bios-tables-test.aarch64.iso.qcow2 as test boot image > > v2: > > * specify in test_data where board's RAM starts and RAM size > > --- > > tests/Makefile.include | 1 + > > tests/bios-tables-test.c | 17 +++++++++++++++++ > > 2 files changed, 18 insertions(+) > > > > diff --git a/tests/Makefile.include b/tests/Makefile.include > > index 0a0e1c3..0619751 100644 > > --- a/tests/Makefile.include > > +++ b/tests/Makefile.include > > @@ -269,6 +269,7 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF) > > check-qtest-aarch64-y = tests/numa-test$(EXESUF) > > check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF) > > check-qtest-aarch64-y += tests/migration-test$(EXESUF) > > +qtest-uefi-images-aarch64 = edk2-aarch64-code.fd.xz edk2-arm-vars.fd.xz > > > > check-qtest-microblazeel-y += $(check-qtest-microblaze-y) > > > > What is this hunk needed for? > > I'm asking because it can't work on top of my v3 posting ("[PATCH > for-4.1 v3 00/12] bundle edk2 platform firmware with QEMU") -- that > version provides *.bz2 files, not *.xz files. > > In addition, the rest of your patch refers to the decompressed (*.fd) > images -- and that is correct, in fact, given that my patches > > * [PATCH for-4.1 v3 10/12] > tests: add missing dependency to build QTEST_QEMU_BINARY, round 2 > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06140.html > > * [PATCH for-4.1 v3 11/12] > Makefile: install the edk2 firmware images and their descriptors > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06138.html > > make sure that the images are decompressed for testing too. > > So, in the end, this hunk should be dropped, IMO. I know, I've just missed it during clean up. I'll fix it up. > > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > > index 097ef03..b5cb425 100644 > > --- a/tests/bios-tables-test.c > > +++ b/tests/bios-tables-test.c > > @@ -804,6 +804,21 @@ static void test_acpi_piix4_tcg_dimm_pxm(void) > > test_acpi_tcg_dimm_pxm(MACHINE_PC); > > } > > > > +static void test_acpi_virt_tcg(void) > > +{ > > + test_data data = { > > + .machine = "virt", > > + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", > > + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", > > + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", > > + .ram_start = 0x40000000ULL, > > + .scan_len = 128ULL * 1024 * 1024, > > + }; > > (Feel free to ignore this one:) > > you might want to beautify both constants using the macros from > "qemu/units.h": 1 * GiB, and 128 * MiB. I prefer to use a less as possible code from QEMU in tests lest we break the later so tests would notice. (it probably doesn't matter much in this case) > > > + > > + test_acpi_one("-cpu cortex-a57 ", &data); > > + free_test_data(&data); > > +} > > + > > int main(int argc, char *argv[]) > > { > > const char *arch = qtest_get_arch(); > > @@ -831,6 +846,8 @@ int main(int argc, char *argv[]) > > qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem); > > qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm); > > qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm); > > + } else if (strcmp(arch, "aarch64") == 0) { > > + qtest_add_func("acpi/virt", test_acpi_virt_tcg); > > } > > ret = g_test_run(); > > boot_sector_cleanup(disk); > > > > With the Makefile.include hunk dropped (and regardless of the constants): > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! > > Thanks, > Laszlo
Le mar. 26 mars 2019 15:28, Laszlo Ersek <lersek@redhat.com> a écrit : > On 03/26/19 14:09, Igor Mammedov wrote: > > adds simple arm/virt test case that starts guest with > > bios-tables-test.aarch64.iso.qcow2 boot image which > > initializes UefiTestSupport* structure in RAM once > > guest is booted. > > > > * see commit: tests: acpi: add acpi_find_rsdp_address_uefi() helper > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v3: > > * use firmware blobs directly from pc-bios directory > > * use bios-tables-test.aarch64.iso.qcow2 as test boot image > > v2: > > * specify in test_data where board's RAM starts and RAM size > > --- > > tests/Makefile.include | 1 + > > tests/bios-tables-test.c | 17 +++++++++++++++++ > > 2 files changed, 18 insertions(+) > > > > diff --git a/tests/Makefile.include b/tests/Makefile.include > > index 0a0e1c3..0619751 100644 > > --- a/tests/Makefile.include > > +++ b/tests/Makefile.include > > @@ -269,6 +269,7 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF) > > check-qtest-aarch64-y = tests/numa-test$(EXESUF) > > check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF) > > check-qtest-aarch64-y += tests/migration-test$(EXESUF) > > +qtest-uefi-images-aarch64 = edk2-aarch64-code.fd.xz edk2-arm-vars.fd.xz > > > > check-qtest-microblazeel-y += $(check-qtest-microblaze-y) > > > > What is this hunk needed for? > > I'm asking because it can't work on top of my v3 posting ("[PATCH > for-4.1 v3 00/12] bundle edk2 platform firmware with QEMU") -- that > version provides *.bz2 files, not *.xz files. > > In addition, the rest of your patch refers to the decompressed (*.fd) > images -- and that is correct, in fact, given that my patches > > * [PATCH for-4.1 v3 10/12] > tests: add missing dependency to build QTEST_QEMU_BINARY, round 2 > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06140.html > > * [PATCH for-4.1 v3 11/12] > Makefile: install the edk2 firmware images and their descriptors > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06138.html > > make sure that the images are decompressed for testing too. > > So, in the end, this hunk should be dropped, IMO. > > > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > > index 097ef03..b5cb425 100644 > > --- a/tests/bios-tables-test.c > > +++ b/tests/bios-tables-test.c > > @@ -804,6 +804,21 @@ static void test_acpi_piix4_tcg_dimm_pxm(void) > > test_acpi_tcg_dimm_pxm(MACHINE_PC); > > } > > > > +static void test_acpi_virt_tcg(void) > > +{ > > + test_data data = { > > + .machine = "virt", > > + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", > > + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", > > + .cd = > "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", > > + .ram_start = 0x40000000ULL, > > + .scan_len = 128ULL * 1024 * 1024, > > + }; > > (Feel free to ignore this one:) > > you might want to beautify both constants using the macros from > "qemu/units.h": 1 * GiB, and 128 * MiB. > When I suggested to enforce those macros, my concern was not beauty but ease review to QEMU newcomers not custom to read hexadecimal (or young developers/students for Bit-sized tasks) and to give a hint with the unit that we express memory size (not Hertz for frequency). 1 * GiB looks more natural than 0x40000000ULL. It was somehow funny that not everybody agreed, in particular the experienced developers. > > + > > + test_acpi_one("-cpu cortex-a57 ", &data); > > + free_test_data(&data); > > +} > > + > > int main(int argc, char *argv[]) > > { > > const char *arch = qtest_get_arch(); > > @@ -831,6 +846,8 @@ int main(int argc, char *argv[]) > > qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem); > > qtest_add_func("acpi/piix4/dimmpxm", > test_acpi_piix4_tcg_dimm_pxm); > > qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm); > > + } else if (strcmp(arch, "aarch64") == 0) { > > + qtest_add_func("acpi/virt", test_acpi_virt_tcg); > > } > > ret = g_test_run(); > > boot_sector_cleanup(disk); > > > > With the Makefile.include hunk dropped (and regardless of the constants): > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Thanks, > Laszlo > >
Le mar. 26 mars 2019 17:05, Igor Mammedov <imammedo@redhat.com> a écrit : > On Tue, 26 Mar 2019 15:27:39 +0100 > Laszlo Ersek <lersek@redhat.com> wrote: > > > On 03/26/19 14:09, Igor Mammedov wrote: > > > adds simple arm/virt test case that starts guest with > > > bios-tables-test.aarch64.iso.qcow2 boot image which > > > initializes UefiTestSupport* structure in RAM once > > > guest is booted. > > > > > > * see commit: tests: acpi: add acpi_find_rsdp_address_uefi() helper > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > v3: > > > * use firmware blobs directly from pc-bios directory > > > * use bios-tables-test.aarch64.iso.qcow2 as test boot image > > > v2: > > > * specify in test_data where board's RAM starts and RAM size > > > --- > > > tests/Makefile.include | 1 + > > > tests/bios-tables-test.c | 17 +++++++++++++++++ > > > 2 files changed, 18 insertions(+) > > > > > > diff --git a/tests/Makefile.include b/tests/Makefile.include > > > index 0a0e1c3..0619751 100644 > > > --- a/tests/Makefile.include > > > +++ b/tests/Makefile.include > > > @@ -269,6 +269,7 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF) > > > check-qtest-aarch64-y = tests/numa-test$(EXESUF) > > > check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF) > > > check-qtest-aarch64-y += tests/migration-test$(EXESUF) > > > +qtest-uefi-images-aarch64 = edk2-aarch64-code.fd.xz > edk2-arm-vars.fd.xz > > > > > > check-qtest-microblazeel-y += $(check-qtest-microblaze-y) > > > > > > > What is this hunk needed for? > > > > I'm asking because it can't work on top of my v3 posting ("[PATCH > > for-4.1 v3 00/12] bundle edk2 platform firmware with QEMU") -- that > > version provides *.bz2 files, not *.xz files. > > > > In addition, the rest of your patch refers to the decompressed (*.fd) > > images -- and that is correct, in fact, given that my patches > > > > * [PATCH for-4.1 v3 10/12] > > tests: add missing dependency to build QTEST_QEMU_BINARY, round 2 > > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06140.html > > > > * [PATCH for-4.1 v3 11/12] > > Makefile: install the edk2 firmware images and their descriptors > > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06138.html > > > > make sure that the images are decompressed for testing too. > > > > So, in the end, this hunk should be dropped, IMO. > > I know, I've just missed it during clean up. I'll fix it up. > > > > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > > > index 097ef03..b5cb425 100644 > > > --- a/tests/bios-tables-test.c > > > +++ b/tests/bios-tables-test.c > > > @@ -804,6 +804,21 @@ static void test_acpi_piix4_tcg_dimm_pxm(void) > > > test_acpi_tcg_dimm_pxm(MACHINE_PC); > > > } > > > > > > +static void test_acpi_virt_tcg(void) > > > +{ > > > + test_data data = { > > > + .machine = "virt", > > > + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", > > > + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", > > > + .cd = > "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", > > > + .ram_start = 0x40000000ULL, > > > + .scan_len = 128ULL * 1024 * 1024, > > > + }; > > > > (Feel free to ignore this one:) > > > > you might want to beautify both constants using the macros from > > "qemu/units.h": 1 * GiB, and 128 * MiB. > > I prefer to use a less as possible code from QEMU in tests lest we break > the later so tests would notice. > (it probably doesn't matter much in this case) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > > > + > > > + test_acpi_one("-cpu cortex-a57 ", &data); > > > + free_test_data(&data); > > > +} > > > + > > > int main(int argc, char *argv[]) > > > { > > > const char *arch = qtest_get_arch(); > > > @@ -831,6 +846,8 @@ int main(int argc, char *argv[]) > > > qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem); > > > qtest_add_func("acpi/piix4/dimmpxm", > test_acpi_piix4_tcg_dimm_pxm); > > > qtest_add_func("acpi/q35/dimmpxm", > test_acpi_q35_tcg_dimm_pxm); > > > + } else if (strcmp(arch, "aarch64") == 0) { > > > + qtest_add_func("acpi/virt", test_acpi_virt_tcg); > > > } > > > ret = g_test_run(); > > > boot_sector_cleanup(disk); > > > > > > > With the Makefile.include hunk dropped (and regardless of the constants): > > > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Thanks! > > > > > Thanks, > > Laszlo > > >
diff --git a/tests/Makefile.include b/tests/Makefile.include index 0a0e1c3..0619751 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -269,6 +269,7 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF) check-qtest-aarch64-y = tests/numa-test$(EXESUF) check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF) check-qtest-aarch64-y += tests/migration-test$(EXESUF) +qtest-uefi-images-aarch64 = edk2-aarch64-code.fd.xz edk2-arm-vars.fd.xz check-qtest-microblazeel-y += $(check-qtest-microblaze-y) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 097ef03..b5cb425 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -804,6 +804,21 @@ static void test_acpi_piix4_tcg_dimm_pxm(void) test_acpi_tcg_dimm_pxm(MACHINE_PC); } +static void test_acpi_virt_tcg(void) +{ + test_data data = { + .machine = "virt", + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", + .ram_start = 0x40000000ULL, + .scan_len = 128ULL * 1024 * 1024, + }; + + test_acpi_one("-cpu cortex-a57 ", &data); + free_test_data(&data); +} + int main(int argc, char *argv[]) { const char *arch = qtest_get_arch(); @@ -831,6 +846,8 @@ int main(int argc, char *argv[]) qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem); qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm); qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm); + } else if (strcmp(arch, "aarch64") == 0) { + qtest_add_func("acpi/virt", test_acpi_virt_tcg); } ret = g_test_run(); boot_sector_cleanup(disk);
adds simple arm/virt test case that starts guest with bios-tables-test.aarch64.iso.qcow2 boot image which initializes UefiTestSupport* structure in RAM once guest is booted. * see commit: tests: acpi: add acpi_find_rsdp_address_uefi() helper Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v3: * use firmware blobs directly from pc-bios directory * use bios-tables-test.aarch64.iso.qcow2 as test boot image v2: * specify in test_data where board's RAM starts and RAM size --- tests/Makefile.include | 1 + tests/bios-tables-test.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+)