Hi Peter,
Thanks for review.
On 2/26/26 3:28 PM, Peter Maydell wrote:
On Wed, 25 Feb 2026 at 08:11, Shivang Upadhyay <[email protected]> wrote:
The '-machine dumpdtb' command line option stopped working on
PowerPC/pnv systems after recent design change [1].
Fixing this by generating fdt blob in `pnv_init`.
[1]
https://lore.kernel.org/qemu-devel/[email protected]/
Cc: Aditya Gupta <[email protected]>
Cc: Harsh Prateek Bora <[email protected]>
Signed-off-by: Shivang Upadhyay <[email protected]>
Thanks for catching this. I would suggest having
Cc: [email protected]
Fixes: 8fd2518ef2f8d34 ("hw: Centralize handling of -machine dumpdtb option")
Ack. Will add this.
---
hw/ppc/pnv.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 1513575b8f..9861714ad5 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1261,6 +1261,10 @@ static void pnv_init(MachineState *machine)
if (pmc->i2c_init) {
pmc->i2c_init(pnv);
}
+
+ if (!machine->fdt) {
+ machine->fdt = pnv_dt_create(machine);
+ }
So we now set the machine->fdt in pnv_init() (which is
the right place: the centralized dumpdtb handling assumes
the DTB is set after machine creation but before reset).
That means that this code in pnv_reset():
if (machine->fdt) {
fdt = machine->fdt;
} else {
fdt = pnv_dt_create(machine);
/* Pack resulting tree */
_FDT((fdt_pack(fdt)));
}
will never take the "else" path, and that code is dead.
It also suggests we should call fdt_pack() in pnv_init
before setting machine->fdt.
Correct. I missed this. Ill add this in v2.
This bit code in pnv_reset():
/* Update machine->fdt with latest fdt */
if (machine->fdt != fdt) {
/*
* Set machine->fdt for 'dumpdtb' QMP/HMP command. Free
* the existing machine->fdt to avoid leaking it during
* a reset.
*/
g_free(machine->fdt);
machine->fdt = fdt;
}
is also now dead and can be removed.
Q: can the generated DBT genuinely be different on a
subsequent reset than it was when QEMU first starts?
The comments in the pnv_reset() code seem to suggest it,
but on the other hand the code as written will only
call pnv_dt_create() and update machine->fdt once on first
reset, not on later resets. So perhaps the comment is
just confusingly worded ?
Yes, I am not much familiar with the design here. But, as per
my current understanding of this code, DTB content should not
not be changed.
thanks
-- PMM
~Shivang.
