[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ppc / sparc: Add a tester for checking whether
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH] ppc / sparc: Add a tester for checking whether OpenBIOS runs successfully |
Date: |
Tue, 14 Jun 2016 15:46:45 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 |
On 14.06.2016 13:31, Markus Armbruster wrote:
> Thomas Huth <address@hidden> writes:
>[...]
>> diff --git a/tests/prom-env-test.c b/tests/prom-env-test.c
>> new file mode 100644
>> index 0000000..b4d5d68
>> --- /dev/null
>> +++ b/tests/prom-env-test.c
>> @@ -0,0 +1,93 @@
>> +/*
>> + * Test OpenBIOS-based machines.
>> + *
>> + * Copyright (c) 2016 Red Hat Inc.
>> + *
>> + * Author:
>> + * Thomas Huth <address@hidden>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2
>> + * or later. See the COPYING file in the top-level directory.
>> + *
>> + * This test is used to check that some OpenBIOS machines can be started
>> + * successfully in TCG mode. To do this, we first put some Forth code into
>> + * the "boot-command" Open Firmware environment variable. This Forth code
>> + * writes a well-known magic value to a known location in memory. Then we
>> + * start the guest so that OpenBIOS can boot and finally run the Forth code.
>> + * The testing code here then can finally check whether the value has been
>> + * successfully written into the guest memory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/compiler.h"
>> +#include <glib/gstdio.h>
>> +#include "qemu-common.h"
>
> These three includes seem redundant.
True. That's a copy-n-paste error from when I was creating the file
using another test as a template ;-)
I'll send a v2 with these lines removed.
>> +#include "libqtest.h"
>> +
>> +#define MAGIC 0xcafec0de
>> +#define ADDRESS 0x4000
>> +
>> +static void check_guest_memory(void)
>> +{
>> + uint32_t signature;
>> + int i;
>> +
>> + /* Poll until code has run and modified memory. Wait at most 30 seconds
>> */
>> + for (i = 0; i < 3000; ++i) {
>> + signature = readl(ADDRESS);
>> + if (signature == MAGIC) {
>> + break;
>> + }
>> + g_usleep(10000);
>> + }
>> +
>> + g_assert_cmphex(signature, ==, MAGIC);
>> +}
>
> I guess there's no way around waiting and an arbitrary time limit here.
> 30s is perhaps a bit long. Dunno...
Currently, the test takes 1 - 2 seconds on my idle laptop, and 4 to 6
seconds when I'm doing a lot of GCC compilation task in the background.
Now imagine somebody tries to run this test on a very old host machine
that is already loaded with a lot of other background tasks ... I guess
that could easily take more than 10 or even 20 seconds. So 30 seconds
should be a realistic value for a timeout here, I think.
>> +
>> +static void test_machine(const void *machine)
>> +{
>> + char *args;
>> +
>> + args = g_strdup_printf("-M %s,accel=tcg -prom-env 'boot-command=%x %x
>> l!'",
>> + (const char *)machine, MAGIC, ADDRESS);
>> +
>> + qtest_start(args);
>> + check_guest_memory();
>> + qtest_quit(global_qtest);
>> +
>> + g_free(args);
>> +}
>> +
>> +static void add_tests(const char *machines[])
>> +{
>> + int i;
>> + char *name;
>> +
>> + for (i = 0; machines[i] != NULL; i++) {
>> + name = g_strdup_printf("prom-env/%s", machines[i]);
>> + qtest_add_data_func(name, machines[i], test_machine);
>> + g_free(name);
>> + }
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + const char *sparc_machines[] = { "SPARCbook", "Voyager", "SS-20", NULL
>> };
>> + const char *sparc64_machines[] = { "sun4u", "sun4v", NULL };
>> + const char *mac_machines[] = { "mac99", "g3beige", NULL };
>> + const char *arch = qtest_get_arch();
>> +
>> + g_test_init(&argc, &argv, NULL);
>> +
>> + if (!strcmp(arch, "ppc") || !strcmp(arch, "ppc64")) {
>> + add_tests(mac_machines);
>> + } else if (!strcmp(arch, "sparc")) {
>> + add_tests(sparc_machines);
>> + } else if (!strcmp(arch, "sparc64")) {
>> + add_tests(sparc64_machines);
>> + } else {
>> + g_assert_not_reached();
>> + }
>> +
>> + return g_test_run();
>> +}
>
> With redundant includes dropped:
> Reviewed-by: Markus Armbruster <address@hidden>
Thanks for the review!
Thomas