[PATCH] ppc/pnv: generate dtb after machine initialization is complete

[PATCH] ppc/pnv: generate dtb after machine initialization is complete

[Shivang Upadhyay]

Currently, the machine dtb is generated in pnv_init(), before all devices
are fully initialized. This can result in an incomplete dtb for the system,
as seen in bug [1].

Fix this by deferring dtb generation until machine initialization is complete,
using the machine_init_done_notifier hook.

[1] https://lore.kernel.org/all/20260323231612.GA2637687@ax162/

Cc: Aditya Gupta <adityag@linux.ibm.com>
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
Cc: BALATON Zoltan <balaton@eik.bme.hu>
Cc: qemu-stable@nongnu.org
Reported-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: a16d4c2f162a86d ("ppc/pnv: fix dumpdtb option")
Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
---
 hw/ppc/pnv.c         | 17 +++++++++++++----
 include/hw/ppc/pnv.h |  2 ++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7e54b6bc60..f13b2e3db8 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -984,6 +984,17 @@ static uint64_t pnv_chip_get_ram_size(PnvMachineState *pnv, int chip_id)
     return chip_id == 0 ? 1 * GiB : QEMU_ALIGN_DOWN(ram_per_chip, 1 * MiB);
 }
 
+static void pnv_machine_init_done(Notifier *notifier, void *data)
+{
+    PnvMachineState *pnv = container_of(notifier, PnvMachineState, machine_init_done);
+    MachineState *machine = MACHINE(pnv);
+
+    if (!machine->fdt) {
+        machine->fdt = pnv_dt_create(machine);
+        _FDT((fdt_pack(machine->fdt)));
+    }
+}
+
 static void pnv_init(MachineState *machine)
 {
     const char *bios_name = machine->firmware ?: FW_FILE_NAME;
@@ -1244,10 +1255,8 @@ static void pnv_init(MachineState *machine)
         pmc->i2c_init(pnv);
     }
 
-    if (!machine->fdt) {
-        machine->fdt = pnv_dt_create(machine);
-        _FDT((fdt_pack(machine->fdt)));
-    }
+    pnv->machine_init_done.notify = pnv_machine_init_done;
+    qemu_add_machine_init_done_notifier(&pnv->machine_init_done);
 }
 
 /*
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 24f8843a40..90028f974d 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -111,6 +111,8 @@ struct PnvMachineState {
 
     bool         big_core;
     bool         lpar_per_core;
+
+    Notifier     machine_init_done;
 };
 
 PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id);
-- 
2.53.0

Re: [PATCH] ppc/pnv: generate dtb after machine initialization is complete

[Peter Maydell]

On Tue, 24 Mar 2026 at 13:51, Shivang Upadhyay <shivangu@linux.ibm.com> wrote:
>
> Currently, the machine dtb is generated in pnv_init(), before all devices
> are fully initialized. This can result in an incomplete dtb for the system,
> as seen in bug [1].
>
> Fix this by deferring dtb generation until machine initialization is complete,
> using the machine_init_done_notifier hook.
>
> [1] https://lore.kernel.org/all/20260323231612.GA2637687@ax162/
>
> Cc: Aditya Gupta <adityag@linux.ibm.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: BALATON Zoltan <balaton@eik.bme.hu>
> Cc: qemu-stable@nongnu.org
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: a16d4c2f162a86d ("ppc/pnv: fix dumpdtb option")
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH] ppc/pnv: generate dtb after machine initialization is complete

[Nathan Chancellor]

On Tue, Mar 24, 2026 at 07:20:26PM +0530, Shivang Upadhyay wrote:
> Currently, the machine dtb is generated in pnv_init(), before all devices
> are fully initialized. This can result in an incomplete dtb for the system,
> as seen in bug [1].
> 
> Fix this by deferring dtb generation until machine initialization is complete,
> using the machine_init_done_notifier hook.
> 
> [1] https://lore.kernel.org/all/20260323231612.GA2637687@ax162/
> 
> Cc: Aditya Gupta <adityag@linux.ibm.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: BALATON Zoltan <balaton@eik.bme.hu>
> Cc: qemu-stable@nongnu.org
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: a16d4c2f162a86d ("ppc/pnv: fix dumpdtb option")
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>

Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  hw/ppc/pnv.c         | 17 +++++++++++++----
>  include/hw/ppc/pnv.h |  2 ++
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7e54b6bc60..f13b2e3db8 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -984,6 +984,17 @@ static uint64_t pnv_chip_get_ram_size(PnvMachineState *pnv, int chip_id)
>      return chip_id == 0 ? 1 * GiB : QEMU_ALIGN_DOWN(ram_per_chip, 1 * MiB);
>  }
>  
> +static void pnv_machine_init_done(Notifier *notifier, void *data)
> +{
> +    PnvMachineState *pnv = container_of(notifier, PnvMachineState, machine_init_done);
> +    MachineState *machine = MACHINE(pnv);
> +
> +    if (!machine->fdt) {
> +        machine->fdt = pnv_dt_create(machine);
> +        _FDT((fdt_pack(machine->fdt)));
> +    }
> +}
> +
>  static void pnv_init(MachineState *machine)
>  {
>      const char *bios_name = machine->firmware ?: FW_FILE_NAME;
> @@ -1244,10 +1255,8 @@ static void pnv_init(MachineState *machine)
>          pmc->i2c_init(pnv);
>      }
>  
> -    if (!machine->fdt) {
> -        machine->fdt = pnv_dt_create(machine);
> -        _FDT((fdt_pack(machine->fdt)));
> -    }
> +    pnv->machine_init_done.notify = pnv_machine_init_done;
> +    qemu_add_machine_init_done_notifier(&pnv->machine_init_done);
>  }
>  
>  /*
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 24f8843a40..90028f974d 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -111,6 +111,8 @@ struct PnvMachineState {
>  
>      bool         big_core;
>      bool         lpar_per_core;
> +
> +    Notifier     machine_init_done;
>  };
>  
>  PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id);
> -- 
> 2.53.0
> 

Re: [PATCH] ppc/pnv: generate dtb after machine initialization is complete

[Aditya Gupta]

On 26/03/24 07:20PM, Shivang Upadhyay wrote:
> <...snip...>
>
> +static void pnv_machine_init_done(Notifier *notifier, void *data)
> +{
> +    PnvMachineState *pnv = container_of(notifier, PnvMachineState, machine_init_done);
> +    MachineState *machine = MACHINE(pnv);
> +
> +    if (!machine->fdt) {
> +        machine->fdt = pnv_dt_create(machine);
> +        _FDT((fdt_pack(machine->fdt)));
> +    }
> +}
> +
>  static void pnv_init(MachineState *machine)
>  {
>      const char *bios_name = machine->firmware ?: FW_FILE_NAME;
> @@ -1244,10 +1255,8 @@ static void pnv_init(MachineState *machine)
>          pmc->i2c_init(pnv);
>      }
>  
> -    if (!machine->fdt) {
> -        machine->fdt = pnv_dt_create(machine);
> -        _FDT((fdt_pack(machine->fdt)));
> -    }
> +    pnv->machine_init_done.notify = pnv_machine_init_done;
> +    qemu_add_machine_init_done_notifier(&pnv->machine_init_done);
>  }
>  
>  /*

Thanks for fixing it. Just adding few more info from what I see.
OPAL was stuck at cec-power-down, since it expects an ipmi-bt device for
shutdown to work in QEMU.

So, the issue was not directly due to missing BMC device, rather due to
missing 'isa-ipmi-bt' device, which the patch does fix, and hence the
issue is fixed.

But still the dts we have is different from what we should have at
pnv_reset. Adding a pnv_dt_create at the end of pnv_reset, as it used to
be before, gives me a 'bmc' node also in device tree.

	$ diff /tmp/dts.machinedone /tmp/dts.pnvreset 
	16a17,33
	>       bmc {
	> 
	>               sensors {
	>                       #size-cells = <0x00>;
	>                       #address-cells = <0x01>;
	> 
	>                       sensor@0 {
	>                               ipmi-sensor-type = <0x23>;
	>                               ipmi-entity-instance = <0x01>;
	>                               ipmi-entity-id = <0x23>;
	>                               ipmi-sensor-reading-type = <0x6f>;
	>                               compatible = "ibm,ipmi-sensor";
	>                               reg = <0x00>;
	>                       };
	>               };
	>       };
	> 

This isn't handled at machine_init_done as 'pnv->bmc' is not set at this
time

Two ways to fix it:
1. Have pnv_dt_create in pnv_reset as well, like previously
2. Move the pnv_bmc_find code in pnv_reset to pnv_init

What do you say ?

> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -111,6 +111,8 @@ struct PnvMachineState {
>  
>      bool         big_core;
>      bool         lpar_per_core;
> +
> +    Notifier     machine_init_done;

One minor note, all other machines in qemu use 'machine_done' as the
notifier name, but I am good with 'machine_init_done' also, as this
makes more sense to me.

Thanks,
- Aditya G

Re: [PATCH] ppc/pnv: generate dtb after machine initialization is complete

[Peter Maydell]

On Thu, 26 Mar 2026 at 11:16, Aditya Gupta <adityag@linux.ibm.com> wrote:
> But still the dts we have is different from what we should have at
> pnv_reset. Adding a pnv_dt_create at the end of pnv_reset, as it used to
> be before, gives me a 'bmc' node also in device tree.
>
>         $ diff /tmp/dts.machinedone /tmp/dts.pnvreset
>         16a17,33
>         >       bmc {
>         >
>         >               sensors {
>         >                       #size-cells = <0x00>;
>         >                       #address-cells = <0x01>;
>         >
>         >                       sensor@0 {
>         >                               ipmi-sensor-type = <0x23>;
>         >                               ipmi-entity-instance = <0x01>;
>         >                               ipmi-entity-id = <0x23>;
>         >                               ipmi-sensor-reading-type = <0x6f>;
>         >                               compatible = "ibm,ipmi-sensor";
>         >                               reg = <0x00>;
>         >                       };
>         >               };
>         >       };
>         >
>
> This isn't handled at machine_init_done as 'pnv->bmc' is not set at this
> time
>
> Two ways to fix it:
> 1. Have pnv_dt_create in pnv_reset as well, like previously
> 2. Move the pnv_bmc_find code in pnv_reset to pnv_init
>
> What do you say ?

My feeling would be that the pnv_bmc_find() handling
should move to the machine-init-done notifier. (pnv_init()
would be too early -- the pnv_bmc_find() code is looking
for a device that has been added via the -device command
line option, so it won't be there yet in pnv_init().)

-- PMM

Re: [PATCH] ppc/pnv: generate dtb after machine initialization is complete

[Aditya Gupta]

On 26/03/26 11:25AM, Peter Maydell wrote:
> On Thu, 26 Mar 2026 at 11:16, Aditya Gupta <adityag@linux.ibm.com> wrote:
> > But still the dts we have is different from what we should have at
> > pnv_reset. Adding a pnv_dt_create at the end of pnv_reset, as it used to
> > be before, gives me a 'bmc' node also in device tree.
> >
> >         $ diff /tmp/dts.machinedone /tmp/dts.pnvreset
> >         16a17,33
> >         >       bmc {
> >         >
> >         >               sensors {
> >         >                       #size-cells = <0x00>;
> >         >                       #address-cells = <0x01>;
> >         >
> >         >                       sensor@0 {
> >         >                               ipmi-sensor-type = <0x23>;
> >         >                               ipmi-entity-instance = <0x01>;
> >         >                               ipmi-entity-id = <0x23>;
> >         >                               ipmi-sensor-reading-type = <0x6f>;
> >         >                               compatible = "ibm,ipmi-sensor";
> >         >                               reg = <0x00>;
> >         >                       };
> >         >               };
> >         >       };
> >         >
> >
> > This isn't handled at machine_init_done as 'pnv->bmc' is not set at this
> > time
> >
> > Two ways to fix it:
> > 1. Have pnv_dt_create in pnv_reset as well, like previously
> > 2. Move the pnv_bmc_find code in pnv_reset to pnv_init
> >
> > What do you say ?
> 
> My feeling would be that the pnv_bmc_find() handling
> should move to the machine-init-done notifier. (pnv_init()
> would be too early -- the pnv_bmc_find() code is looking
> for a device that has been added via the -device command
> line option, so it won't be there yet in pnv_init().)

thanks! agreed, pnv_bmc_find wouldn't work in pnv_init.
moving it to machine-init-done seems okay. if shivang decides to move
pnv_bmc_find this should be good

- Aditya G