Hi Bolaton,

Thanks for taking the time to review.

On Fri, Feb 27, 2026 at 04:53:56PM +0100, BALATON Zoltan wrote:
> On Fri, 27 Feb 2026, Aditya Gupta wrote:
> > On 26/02/27 02:05PM, BALATON Zoltan wrote:
> > > On Fri, 27 Feb 2026, Aditya Gupta wrote:
> > > > On 26/02/27 03:43PM, shivang upadhyay wrote:
> > > > > > <...snip...>
> > > > > > 
> > > > > > 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.
> > > > 
> > > > To add to this, yes, normally reset path doesn't change the device tree,
> > > > since all reboots are the same as the first boot.
> > > > 
> > > > As of upstream this is true, I don't see dtb changing between resets.
> > > 
> > > Isn't VOF allowing clients to do setprop that would be changing the device
> > > tree? Some settings are stored there so maybe it can be changed by the 
> > > guest
> > > that should be preserved between reboots?
> > 
> > By VOF did you mean the pseries f/w ?
> 
> I don't know much about these machines. I meant Virtual Open Firmware in
> QEMU or SLOF which it can replace. But maybe this is only used by spapr so
> does not affect pseries but in spapr there's spapr_vof_setprop() which looks
> like it can change the fdt. Maybe it's handled differently on pseries, I
> don't know.
Yes, Vof/Slof firmware are only used for pseries.
for pnv we use skiboot.lid firmware.
> 
> > i don't see anything changing the dt in powernv as of now, but
> > for some edge case, where that is possible (will be possible atleast
> > after mpipl support is merged), we can always update the fdt in below 2
> > if cases in pnv_reset:
> > 
> >    > if (machine->fdt) {
> >    >     fdt = machine->fdt;
> >    > } else {
> >    >     fdt = pnv_dt_create(machine);
> >    >     /* Pack resulting tree */
> >    >     _FDT((fdt_pack(fdt)));
> >    > }
> > 
> > machine->fdt will never be NULL since it's being set in pnv_init
> > should we just do pnv_dt_create on every pnv_reset ?
> 
> I don't know either but wasn't original code meant to either create an fdt
> or take what the user specified with -dtb option or similiar? In that case
> maybe you don't want to override the user specified dtb but I could be wrong
> about what this is meant to do as I have no idea whatsoever about these
> machines.
I think I should just remove all the fdt changing stuff from cpu_reset.
Becasue nothing is using it as of now. We can introduce it with mpipl patches.
That would make the code easier to understand there.
> 
> Regards,
> BALATON Zoltan


~Shivang.

Reply via email to