Laszlo Ersek <ler...@redhat.com> writes:
> Check whether the firmware is not hidden by other memory regions.
>
> Qemu is started in paused mode: it shouldn't try to interpret generated
> garbage.
>
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
> tests/i440fx-test.c | 81
> +++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
> index 5506421..6de16e9 100644
> --- a/tests/i440fx-test.c
> +++ b/tests/i440fx-test.c
> @@ -35,6 +35,11 @@ typedef struct TestData
> int num_cpus;
> } TestData;
>
> +typedef struct FirmwareTestFixture {
> + /* decides whether we're testing -bios or -pflash */
> + bool is_bios;
> +} FirmwareTestFixture;
> +
> static QPCIBus *test_start_get_bus(const TestData *s)
> {
> char *cmdline;
> @@ -278,6 +283,7 @@ static void test_i440fx_pam(gconstpointer opaque)
> }
>
> #define FW_SIZE ((size_t)65536)
> +#define ISA_BIOS_MAXSZ ((size_t)(128 * 1024))
>
> /* Create a temporary firmware blob, and return its absolute pathname as a
> * dynamically allocated string.
> @@ -328,23 +334,86 @@ static char *create_firmware(void)
> return ret == -1 ? NULL : pathname;
> }
>
> -int main(int argc, char **argv)
> +static void test_i440fx_firmware(FirmwareTestFixture *fixture,
> + gconstpointer user_data)
> {
> - char *fw_pathname;
> - TestData data;
> - int ret;
> -
> - g_test_init(&argc, &argv, NULL);
> + char *fw_pathname, *cmdline;
> + char unsigned *buf;
unsigned char *buf, please.
> + size_t i, isa_bios_size;
>
> fw_pathname = create_firmware();
> g_assert(fw_pathname != NULL);
> +
> + /* Better hope the user didn't put metacharacters in TMPDIR and co. */
Putting meta-characters in TMPDIR would be... adventurous.
You could add quotes. Then it breaks only when the user puts quotes in ;-P
> + cmdline = g_strdup_printf("-S %s %s",
> + fixture->is_bios ? "-bios" : "-pflash",
> + fw_pathname);
> + g_test_message("qemu cmdline: %s", cmdline);
> + qtest_start(cmdline);
> + g_free(cmdline);
> +
> + /* Qemu has loaded the firmware (because qtest_start() only returns after
> + * the QMP handshake completes). We must unlink the firmware blob right
> + * here, because any assertion firing below would leak it in the
> + * filesystem. This is also the reason why we recreate the blob every
> time
> + * this function is invoked.
> + */
> unlink(fw_pathname);
> g_free(fw_pathname);
>
> + /* check below 4G */
> + buf = g_malloc0(FW_SIZE);
> + memread(0x100000000ULL - FW_SIZE, buf, FW_SIZE);
Zero-fill immediately followed by read. Suggest g_malloc().
> + for (i = 0; i < FW_SIZE; ++i) {
> + g_assert_cmphex(buf[i], ==, (char unsigned)i);
(unsigned char), please.
> + }
> +
> + /* check in ISA space too */
> + memset(buf, 0, FW_SIZE);
> + isa_bios_size = ISA_BIOS_MAXSZ < FW_SIZE ? ISA_BIOS_MAXSZ : FW_SIZE;
> + memread(0x100000 - isa_bios_size, buf, isa_bios_size);
Zero-fill immediately followed by read. Suggest to drop memset().
> + for (i = 0; i < isa_bios_size; ++i) {
> + g_assert_cmphex(buf[i], ==,
> + (char unsigned)((FW_SIZE - isa_bios_size) + i));
Again.
> + }
> +
> + g_free(buf);
> + qtest_end();
> +}
> +
> +static void add_firmware_test(const char *testpath,
> + void (*setup_fixture)(FirmwareTestFixture *f,
> + gconstpointer test_data))
> +{
> + g_test_add(testpath, FirmwareTestFixture, NULL, setup_fixture,
> + test_i440fx_firmware, NULL);
> +}
> +
> +static void request_bios(FirmwareTestFixture *fixture,
> + gconstpointer user_data)
> +{
> + fixture->is_bios = true;
> +}
> +
> +static void request_pflash(FirmwareTestFixture *fixture,
> + gconstpointer user_data)
> +{
> + fixture->is_bios = false;
> +}
> +
I'm not sure fixtures are justified here. Perhaps having the test
function's test data argument point to the flag would be simpler.
> +int main(int argc, char **argv)
> +{
> + TestData data;
> + int ret;
> +
> + g_test_init(&argc, &argv, NULL);
> +
> data.num_cpus = 1;
>
> g_test_add_data_func("/i440fx/defaults", &data, test_i440fx_defaults);
> g_test_add_data_func("/i440fx/pam", &data, test_i440fx_pam);
> + add_firmware_test("/i440fx/firmware/bios", request_bios);
> + add_firmware_test("/i440fx/firmware/pflash", request_pflash);
>
> ret = g_test_run();
> return ret;
