This is on top of my "[PATCH v2 0/8] -boot and -no-fd-bootchk fixes".
Since I need to respin my series anyway, I can include your patch in the
respin.
Andreas Färber <afaer...@suse.de> writes:
> They set the boot device via fw_cfg, which is then translated to a boot
> path of "hd" or "cd" in OpenBIOS.
>
> Signed-off-by: Andreas Färber <afaer...@suse.de>
> Cc: Markus Armbruster <arm...@redhat.com>
> Cc: Alexander Graf <ag...@suse.de>
> Cc: qemu-...@nongnu.org
> ---
> tests/Makefile | 2 ++
> tests/boot-order-test.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++-
> 2 Dateien geändert, 67 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 1305642..f84d466 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -76,6 +76,8 @@ gcov-files-sparc-y += hw/m48t59.c
> gcov-files-sparc64-y += hw/m48t59.c
> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
> gcov-files-arm-y += hw/tmp105.c
> +check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
> +check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
>
> GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h
> tests/test-qmp-commands.h
>
> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
> index 60412ad..84dd694 100644
> --- a/tests/boot-order-test.c
> +++ b/tests/boot-order-test.c
> @@ -14,8 +14,10 @@
> * Covers only PC. Better than nothing. Improvements welcome.
> */
Comment is no longer correct. Feel free to drop it entirely.
>
> +#include <string.h>
> #include <glib.h>
> #include "libqtest.h"
> +#include "qemu/bswap.h"
>
> static void test_cmos_byte(int reg, int expected)
> {
> @@ -61,11 +63,73 @@ static void test_pc_boot_order(void)
> 0, 0x02, 0x30, 0x12);
> }
>
> +#define G3BEIGE_CFG_ADDR 0xf0000510
> +#define MAC99_CFG_ADDR 0xf0000510
> +
> +#define NO_QEMU_PROTOS
> +#include "hw/fw_cfg.h"
> +#undef NO_QEMU_PROTOS
Looks like fw_cfg.h should be split up. Out of this patch's scope.
> +
> +static void powermac_fw_cfg_read(bool newworld, uint16_t cmd,
> + uint8_t *buf, unsigned int len)
> +{
> + unsigned int i;
> +
> + writew(newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR, cmd);
> + for (i = 0; i < len; i++) {
> + buf[i] = readb((newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR) + 2);
> + }
> +}
More general than necessary (readw() would do), but that's okay.
> +
> +static uint16_t powermac_fw_cfg_read16(bool newworld, uint16_t cmd)
> +{
> + uint16_t value;
> +
> + powermac_fw_cfg_read(newworld, cmd, (uint8_t *)&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)
> +{
> + 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);
> +}
> +
> +static void test_powermac_boot_order(void)
> +{
> + int i;
> +
> + for (i = 0; i < 2; i++) {
> + bool newworld = (i == 1);
> +
> + 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');
Doesn't test -boot once. Tolerable, because test_pc_boot_order() tests
it, and the only machine-specific part is the boot set handler.
Hmm, test_powermac_with_args() is prepared for -boot once testing.
Throw in "-boot once=d,order=c"?
> + }
> +}
> +
> int main(int argc, char *argv[])
> {
> + const char *arch = qtest_get_arch();
> +
> g_test_init(&argc, &argv, NULL);
>
> - qtest_add_func("boot-order/pc", test_pc_boot_order);
> + 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);
> + }
>
> return g_test_run();
> }
If you don't want to respin, I can take this patch as is, and fix up the
bogus comment in my tree.
