[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 11/16] boot-order-test: Better separate targe
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH v3 11/16] boot-order-test: Better separate target-specific and generic parts |
Date: |
Fri, 14 Jun 2013 08:52:09 -0500 |
User-agent: |
Notmuch/0.15.2+77~g661dcf8 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Markus Armbruster <address@hidden> writes:
> The initial version did just PC. I didn't bother to separate out
> generic parts, because I don't like to abstract from a single case.
>
> Now we have two cases, PC and PowerMac, and I'm about to add two more.
> Time to do it right.
>
> To ease review, this commit changes the code without in-place, and
> only the next commit reorders it for better readability.
>
> Cc: Andreas Färber <address@hidden>
> Cc: Alexander Graf <address@hidden>
> Cc: address@hidden
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> tests/boot-order-test.c | 160
> ++++++++++++++++++++++++++++++------------------
> 1 file changed, 99 insertions(+), 61 deletions(-)
>
> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
> index f6dece6..23edacf 100644
> --- a/tests/boot-order-test.c
> +++ b/tests/boot-order-test.c
> @@ -15,106 +15,141 @@
> #include "libqtest.h"
> #include "qemu/bswap.h"
>
> -static void test_pc_cmos_byte(int reg, int expected)
> +static uint8_t read_mc146818(uint16_t port, uint8_t reg)
> {
> - int actual;
> -
> - outb(0x70, reg);
> - actual = inb(0x71);
> - g_assert_cmphex(actual, ==, expected);
> + outb(port, reg);
> + return inb(port + 1);
> }
It's worth looking at the rtc-test case and pulling out
cmos_read/write() into libqos I suspect.
Regards,
Anthony Liguori
>
> -static void test_pc_cmos(uint8_t boot1, uint8_t boot2)
> +static uint64_t read_boot_order_pc(void)
> {
> - test_pc_cmos_byte(0x38, boot1);
> - test_pc_cmos_byte(0x3d, boot2);
> + uint8_t b1 = read_mc146818(0x70, 0x38);
> + uint8_t b2 = read_mc146818(0x70, 0x3d);
> +
> + return b1 | (b2 << 8);
> }
>
> -static void test_pc_with_args(const char *test_args,
> - uint8_t boot1, uint8_t boot2,
> - uint8_t reboot1, uint8_t reboot2)
> +static void test_a_boot_order(const char *machine,
> + const char *test_args,
> + uint64_t (*read_boot_order)(void),
> + uint64_t expected_boot,
> + uint64_t expected_reboot)
> {
> - char *args = g_strdup_printf("-nodefaults -display none %s", test_args);
> + char *args;
> + uint64_t actual;
>
> + args = g_strdup_printf("-nodefaults -display none%s%s %s",
> + machine ? " -M " : "",
> + machine ?: "",
> + test_args);
> qtest_start(args);
> - test_pc_cmos(boot1, boot2);
> + actual = read_boot_order();
> + g_assert_cmphex(actual, ==, expected_boot);
> qmp("{ 'execute': 'system_reset' }");
> - test_pc_cmos(reboot1, reboot2);
> + actual = read_boot_order();
> + g_assert_cmphex(actual, ==, expected_reboot);
> qtest_quit(global_qtest);
> g_free(args);
> }
>
> +typedef struct {
> + const char *args;
> + uint64_t expected_boot;
> + uint64_t expected_reboot;
> +} boot_order_test;
> +
> +static void test_boot_orders(const char *machine,
> + uint64_t (*read_boot_order)(void),
> + const boot_order_test *tests)
> +{
> + int i;
> +
> + for (i = 0; tests[i].args; i++) {
> + test_a_boot_order(machine, tests[i].args,
> + read_boot_order,
> + tests[i].expected_boot,
> + tests[i].expected_reboot);
> + }
> +}
> +
> +static const boot_order_test test_cases_pc[] = {
> + { "",
> + 0x1230, 0x1230 },
> + { "-no-fd-bootchk",
> + 0x1231, 0x1231 },
> + { "-boot c",
> + 0x0200, 0x0200 },
> + { "-boot nda",
> + 0x3410, 0x3410 },
> + { "-boot order=",
> + 0, 0 },
> + { "-boot order= -boot order=c",
> + 0x0200, 0x0200 },
> + { "-boot once=a",
> + 0x0100, 0x1230 },
> + { "-boot once=a -no-fd-bootchk",
> + 0x0101, 0x1231 },
> + { "-boot once=a,order=c",
> + 0x0100, 0x0200 },
> + { "-boot once=d -boot order=nda",
> + 0x0300, 0x3410 },
> + { "-boot once=a -boot once=b -boot once=c",
> + 0x0200, 0x1230 },
> + {}
> +};
> +
> static void test_pc_boot_order(void)
> {
> - test_pc_with_args("", 0x30, 0x12, 0x30, 0x12);
> - test_pc_with_args("-no-fd-bootchk", 0x31, 0x12, 0x31, 0x12);
> - test_pc_with_args("-boot c", 0, 0x02, 0, 0x02);
> - test_pc_with_args("-boot nda", 0x10, 0x34, 0x10, 0x34);
> - test_pc_with_args("-boot order=", 0, 0, 0, 0);
> - test_pc_with_args("-boot order= -boot order=c", 0, 0x02, 0, 0x02);
> - test_pc_with_args("-boot once=a", 0, 0x01, 0x30, 0x12);
> - test_pc_with_args("-boot once=a -no-fd-bootchk", 0x01, 0x01, 0x31, 0x12);
> - test_pc_with_args("-boot once=a,order=c", 0, 0x01, 0, 0x02);
> - test_pc_with_args("-boot once=d -boot order=nda", 0, 0x03, 0x10, 0x34);
> - test_pc_with_args("-boot once=a -boot once=b -boot once=c",
> - 0, 0x02, 0x30, 0x12);
> + test_boot_orders(NULL, read_boot_order_pc, test_cases_pc);
> }
>
> -#define G3BEIGE_CFG_ADDR 0xf0000510
> -#define MAC99_CFG_ADDR 0xf0000510
> +#define PMAC_CFG_ADDR 0xf0000510
>
> #define NO_QEMU_PROTOS
> #include "hw/nvram/fw_cfg.h"
> #undef NO_QEMU_PROTOS
>
> -static void powermac_fw_cfg_read(bool newworld, uint16_t cmd,
> - uint8_t *buf, unsigned int len)
> +static void read_fw_cfg(uint64_t cfg_addr, uint16_t cmd,
> + void *buf, size_t len)
> {
> - unsigned int i;
> + uint8_t *p = buf;
> + size_t i;
>
> - writew(newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR, cmd);
> + writew(cfg_addr, cmd);
> for (i = 0; i < len; i++) {
> - buf[i] = readb((newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR) + 2);
> + p[i] = readb(cfg_addr + 2);
> }
> }
>
> -static uint16_t powermac_fw_cfg_read16(bool newworld, uint16_t cmd)
> +static uint16_t read_fw_cfg_i16(uint64_t cfg_addr, uint16_t cmd)
> {
> uint16_t value;
>
> - powermac_fw_cfg_read(newworld, cmd, (uint8_t *)&value, sizeof(value));
> + read_fw_cfg(cfg_addr, cmd, &value, sizeof(value));
> return le16_to_cpu(value);
> }
>
> -static void test_powermac_with_args(bool newworld, const char *extra_args,
> - uint16_t expected_boot,
> - uint16_t expected_reboot)
> +static uint64_t read_boot_order_pmac(void)
> {
> - char *args = g_strdup_printf("-nodefaults -display none -machine %s %s",
> - newworld ? "mac99" : "g3beige", extra_args);
> - uint16_t actual;
> - qtest_start(args);
> - actual = powermac_fw_cfg_read16(newworld, FW_CFG_BOOT_DEVICE);
> - g_assert_cmphex(actual, ==, expected_boot);
> - qmp("{ 'execute': 'system_reset' }");
> - actual = powermac_fw_cfg_read16(newworld, FW_CFG_BOOT_DEVICE);
> - g_assert_cmphex(actual, ==, expected_reboot);
> - qtest_quit(global_qtest);
> - g_free(args);
> + return read_fw_cfg_i16(PMAC_CFG_ADDR, FW_CFG_BOOT_DEVICE);
> }
>
> -static void test_powermac_boot_order(void)
> -{
> - int i;
> +static const boot_order_test test_cases_fw_cfg[] = {
> + { "", 'c', 'c' },
> + { "-boot c", 'c', 'c' },
> + { "-boot d", 'd', 'd' },
> + { "-boot once=d,order=c", 'd', 'c' },
> + {}
> +};
>
> - for (i = 0; i < 2; i++) {
> - bool newworld = (i == 1);
> +static void test_pmac_oldworld_boot_order(void)
> +{
> + test_boot_orders("g3beige", read_boot_order_pmac, test_cases_fw_cfg);
> +}
>
> - test_powermac_with_args(newworld, "", 'c', 'c');
> - test_powermac_with_args(newworld, "-boot c", 'c', 'c');
> - test_powermac_with_args(newworld, "-boot d", 'd', 'd');
> - test_powermac_with_args(newworld, "-boot once=d,order=c", 'd', 'c');
> - }
> +static void test_pmac_newworld_boot_order(void)
> +{
> + test_boot_orders("mac99", read_boot_order_pmac, test_cases_fw_cfg);
> }
>
> int main(int argc, char *argv[])
> @@ -126,7 +161,10 @@ int main(int argc, char *argv[])
> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> qtest_add_func("boot-order/pc", test_pc_boot_order);
> } else if (strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) {
> - qtest_add_func("boot-order/powermac", test_powermac_boot_order);
> + qtest_add_func("boot-order/pmac_oldworld",
> + test_pmac_oldworld_boot_order);
> + qtest_add_func("boot-order/pmac_newworld",
> + test_pmac_newworld_boot_order);
> }
>
> return g_test_run();
> --
> 1.7.11.7
- Re: [Qemu-devel] [PATCH v3 08/16] boot-order-test: New; covering just PC for now, (continued)
[Qemu-devel] [PATCH v3 03/16] vl: Fix -boot order and once regressions, and related bugs, Markus Armbruster, 2013/06/14
[Qemu-devel] [PATCH v3 12/16] boot-order-test: Code motion for better readability, Markus Armbruster, 2013/06/14
[Qemu-devel] [PATCH v3 06/16] doc: Drop ref to Bochs from -no-fd-bootchk documentation, Markus Armbruster, 2013/06/14
[Qemu-devel] [PATCH v3 14/16] boot-order-test: Add tests for Sun4m, Markus Armbruster, 2013/06/14
[Qemu-devel] [PATCH v3 11/16] boot-order-test: Better separate target-specific and generic parts, Markus Armbruster, 2013/06/14
- Re: [Qemu-devel] [PATCH v3 11/16] boot-order-test: Better separate target-specific and generic parts,
Anthony Liguori <=
[Qemu-devel] [PATCH v3 09/16] boot-order-test: Add tests for PowerMacs, Markus Armbruster, 2013/06/14
[Qemu-devel] [PATCH v3 15/16] boot-order-test: Support fw_cfg in I/O space, Markus Armbruster, 2013/06/14
[Qemu-devel] [PATCH v3 16/16] boot-order-test: Add tests for Sun4u, Markus Armbruster, 2013/06/14
Re: [Qemu-devel] [PATCH v3 00/16] -boot and -no-fd-bootchk fixes, Anthony Liguori, 2013/06/21