On Fri Aug 16, 2024 at 3:52 AM AEST, Cédric Le Goater wrote: > On 8/15/24 09:31, Nicholas Piggin wrote: > > On Tue Aug 13, 2024 at 11:45 PM AEST, Aditya Gupta wrote: > >> Currently any device tree passed with -dtb option in QEMU, was ignored > >> by the PowerNV code. > >> > >> Read and pass the passed -dtb to the kernel, thus enabling easier > >> debugging with custom DTBs. > >> > >> The existing behaviour when -dtb is 'not' passed, is preserved as-is. > >> > >> But when a '-dtb' is passed, it completely overrides any dtb nodes or > >> changes QEMU might have done, such as '-append' arguments to the kernel > >> (which are mentioned in /chosen/bootargs in the dtb), hence add warning > >> when -dtb is being used > >> > >> Signed-off-by: Aditya Gupta <adit...@linux.ibm.com> > > > > This looks pretty good, I'm inclined to take it as a bug fix fo this > > release. > > I don't think this is a bug fix. is it ? AFAIUI, it is a debug > feature for skiboot. It's QEMU 9.2 material.
Okay. > > One little nit is MachineState.fdt vs PnvMachineState.fdt > > which is now confusing. I would call the new PnvMachineState member > > something like fdt_from_dtb, or fdt_override? > > I agree. this is confusing. machine->fdt could be used instead ? Yeah that could be another option. Test pnv.dtb or add a new bool to pnv if you need to check whether the fdt has been provided by cmdline. > > The other question... Some machines rebuild fdt at init, others at > > reset time. As far as I understood, spapr has to rebuild on reset > > because C-A-S call can update the fdt so you have to undo that on > > reset. > > C-A-S is a guest OS hcall. reset is called before the guest OS > is started. Right, but when you reboot it needs to be reverted to initial (pre-CAS) fdt. > > Did powernv just copy that without really needing it, I wonder? > > Maybe that rearranged to just do it at init time (e.g., see > > hw/riscv/virt.c which is simpler). > > The machine is aware of user created devices (on the command line) > only at reset time. Ah, I should have followed a bit closer. riscv, arm use a machine_done notifier for that (and x86, loongarch for ACPI / BIOS tables). So that avoids fdt rebuild after the first reset I think. Anyway I don't really mind then, following other archs would be okay, but keeping similar with spapr and avoiding code change is also good. Maybe add a small comment to we use reset rather than machine_done notifier of other archs to be similar to spapr. Thanks, Nick
