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") > --- > 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. 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 ? thanks -- PMM
