From 8fd2518ef2f8d34dc9ee53d6915a2a610eb1a659 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 6 Feb 2025 15:12:13 +0000 Subject: hw: Centralize handling of -machine dumpdtb option Currently we handle the 'dumpdtb' machine sub-option ad-hoc in every board model that has an FDT. It's up to the board code to make sure it calls qemu_fdt_dumpdtb() in the right place. This means we're inconsistent and often just ignore the user's command line argument: * if the board doesn't have an FDT at all * if the board supports FDT, but there happens not to be one present (usually because of a missing -fdt option) This isn't very helpful because it gives the user no clue why their option was ignored. However, in order to support the QMP/HMP dumpdtb commands we require now that every FDT machine stores a pointer to the FDT in MachineState::fdt. This means we can handle -machine dumpdtb centrally by calling the qmp_dumpdtb() function, unifying its handling with the QMP/HMP commands. All the board code calls to qemu_fdt_dumpdtb() can then be removed. For this commit we retain the existing behaviour that if there is no FDT we silently ignore the -machine dumpdtb option. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/core/machine.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'hw/core/machine.c') diff --git a/hw/core/machine.c b/hw/core/machine.c index 02cff735b3..61c22f723a 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -19,6 +19,7 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "qapi/qapi-visit-machine.h" +#include "qapi/qapi-commands-machine.h" #include "qemu/madvise.h" #include "qom/object_interfaces.h" #include "system/cpus.h" @@ -1696,6 +1697,24 @@ void qemu_remove_machine_init_done_notifier(Notifier *notify) notifier_remove(notify); } +static void handle_machine_dumpdtb(MachineState *ms) +{ + if (!ms->dumpdtb) { + return; + } + if (!ms->fdt) { + /* Silently ignore dumpdtb option if there is nothing to dump */ + return; + } +#ifdef CONFIG_FDT + qmp_dumpdtb(ms->dumpdtb, &error_fatal); + exit(0); +#else + error_report("This machine doesn't have an FDT"); + exit(1); +#endif +} + void qdev_machine_creation_done(void) { cpu_synchronize_all_post_init(); @@ -1712,6 +1731,12 @@ void qdev_machine_creation_done(void) phase_advance(PHASE_MACHINE_READY); qdev_assert_realized_properly(); + /* + * If the user used -machine dumpdtb=file.dtb to request that we + * dump the DTB to a file, do it now, and exit. + */ + handle_machine_dumpdtb(current_machine); + /* TODO: once all bus devices are qdevified, this should be done * when bus is created by qdev.c */ /* -- cgit 1.4.1 From bb09b7bfd37024381970744c71646e0239428897 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 6 Feb 2025 15:12:14 +0000 Subject: hw/core/machine.c: Make -machine dumpdtb=file.dtb with no DTB an error Currently if the user requests via -machine dumpdtb=file.dtb that we dump the DTB, but the machine doesn't have a DTB, we silently ignore the option. This is confusing to users, and is a legacy of the old board-specific implementation of the option, where if the execution codepath didn't go via a call to qemu_fdt_dumpdtb() we would never handle the option. Now we handle the option in one place in machine.c, we can provide the user with a useful message if they asked us to dump a DTB when none exists. qmp_dumpdtb() already produces this error; remove the logic in handle_machine_dumpdtb() that was there specifically to avoid hitting it. While we're here, beef up the error message a bit with a hint, and make it consistent about "an FDT" rather than "a FDT". (In the qmp_dumpdtb() case this needs an ERRP_GUARD to make error_append_hint() work when the caller passes error_fatal.) Note that the three places where we might report "doesn't have an FDT" are hit in different situations: (1) in handle_machine_dumpdtb(), if CONFIG_FDT is not set: this is because the QEMU binary was built without libfdt at all. The build system will not let you build with a machine type that needs an FDT but no libfdt, so here we know both that the machine doesn't use FDT and that QEMU doesn't have the support: (2) in the device_tree-stub.c qmp_dumpdtb(): this is used when we had libfdt at build time but the target architecture didn't enable any machines which did "select DEVICE_TREE", so here we know that the machine doesn't use FDT. (3) in qmp_dumpdtb(), if current_machine->fdt is NULL all we know is that this machine never set it. That might be because it doesn't use FDT, or it might be because the user didn't pass an FDT on the command line and the machine doesn't autogenerate an FDT. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2733 Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20250206151214.2947842-7-peter.maydell@linaro.org --- hw/core/machine.c | 6 ++---- system/device_tree-stub.c | 5 ++++- system/device_tree.c | 7 ++++++- 3 files changed, 12 insertions(+), 6 deletions(-) (limited to 'hw/core/machine.c') diff --git a/hw/core/machine.c b/hw/core/machine.c index 61c22f723a..b68b8b94a3 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -1702,15 +1702,13 @@ static void handle_machine_dumpdtb(MachineState *ms) if (!ms->dumpdtb) { return; } - if (!ms->fdt) { - /* Silently ignore dumpdtb option if there is nothing to dump */ - return; - } #ifdef CONFIG_FDT qmp_dumpdtb(ms->dumpdtb, &error_fatal); exit(0); #else error_report("This machine doesn't have an FDT"); + error_printf("(this machine type definitely doesn't use FDT, and " + "this QEMU doesn't have FDT support compiled in)\n"); exit(1); #endif } diff --git a/system/device_tree-stub.c b/system/device_tree-stub.c index bddda6fa37..428330b0fe 100644 --- a/system/device_tree-stub.c +++ b/system/device_tree-stub.c @@ -5,6 +5,9 @@ #ifdef CONFIG_FDT void qmp_dumpdtb(const char *filename, Error **errp) { - error_setg(errp, "This machine doesn't have a FDT"); + ERRP_GUARD(); + + error_setg(errp, "This machine doesn't have an FDT"); + error_append_hint(errp, "(this machine type definitely doesn't use FDT)\n"); } #endif diff --git a/system/device_tree.c b/system/device_tree.c index d605ed2a21..aa3fe9516f 100644 --- a/system/device_tree.c +++ b/system/device_tree.c @@ -635,11 +635,16 @@ out: void qmp_dumpdtb(const char *filename, Error **errp) { + ERRP_GUARD(); + g_autoptr(GError) err = NULL; uint32_t size; if (!current_machine->fdt) { - error_setg(errp, "This machine doesn't have a FDT"); + error_setg(errp, "This machine doesn't have an FDT"); + error_append_hint(errp, + "(Perhaps it doesn't support FDT at all, or perhaps " + "you need to provide an FDT with the -fdt option?)\n"); return; } -- cgit 1.4.1