| Message ID | 20240820103003.550735-1-adityag@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] hw/ppc: Implement -dtb support for PowerNV | expand |
Hi all, Just a ping for this. Any comments ? Thanks, Aditya Gupta On 20/08/24 16:00, 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 <adityag@linux.ibm.com> > > --- > Changelog > =========== > v4: > + use 'MachineState::fdt' instead of 'PnvMachineState::fdt' > + added an 'if' check at end of pnv_reset, so we don't free the fdt we > are using > v3: > + use 'load_device_tree' to read the device tree, instead of g_file_get_contents > + tested that passed dtb does NOT get ignored on system_reset > > v2: > + move reading dtb and warning to pnv_init > > v1: > + use 'g_file_get_contents' and add check for -append & -dtb as suggested by Daniel > --- > --- > hw/ppc/pnv.c | 51 ++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 40 insertions(+), 11 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 3526852685b4..5be15f748e45 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -736,21 +736,27 @@ static void pnv_reset(MachineState *machine, ShutdownCause reason) > } > } > > - fdt = pnv_dt_create(machine); > - > - /* Pack resulting tree */ > - _FDT((fdt_pack(fdt))); > + if (machine->fdt) { > + fdt = machine->fdt; > + } else { > + fdt = pnv_dt_create(machine); > + /* Pack resulting tree */ > + _FDT((fdt_pack(fdt))); > + } > > qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); > cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(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; > + /* 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; > + } > } > > static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) > @@ -952,6 +958,14 @@ static void pnv_init(MachineState *machine) > g_free(sz); > exit(EXIT_FAILURE); > } > + > + /* checks for invalid option combinations */ > + if (machine->dtb && (strlen(machine->kernel_cmdline) != 0)) { > + error_report("-append and -dtb cannot be used together, as passed" > + " command line is ignored in case of custom dtb"); > + exit(EXIT_FAILURE); > + } > + > memory_region_add_subregion(get_system_memory(), 0, machine->ram); > > /* > @@ -1003,6 +1017,21 @@ static void pnv_init(MachineState *machine) > } > } > > + /* load dtb if passed */ > + if (machine->dtb) { > + int fdt_size; > + > + warn_report("with manually passed dtb, some options like '-append'" > + " will get ignored and the dtb passed will be used as-is"); > + > + /* read the file 'machine->dtb', and load it into 'fdt' buffer */ > + machine->fdt = load_device_tree(machine->dtb, &fdt_size); > + if (!machine->fdt) { > + error_report("Could not load dtb '%s'", machine->dtb); > + exit(1); > + } > + } > + > /* MSIs are supported on this platform */ > msi_nonbroken = true; >
On Tue Aug 20, 2024 at 8:30 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 <adityag@linux.ibm.com> Sorry for getting to it late, Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
On 03/11/24 17:04, Nicholas Piggin wrote: > On Tue Aug 20, 2024 at 8:30 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 <adityag@linux.ibm.com> > Sorry for getting to it late, > > Reviewed-by: Nicholas Piggin <npiggin@gmail.com> That's okay. Thanks for the tag Nick ! - Aditya Gupta
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 3526852685b4..5be15f748e45 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -736,21 +736,27 @@ static void pnv_reset(MachineState *machine, ShutdownCause reason) } } - fdt = pnv_dt_create(machine); - - /* Pack resulting tree */ - _FDT((fdt_pack(fdt))); + if (machine->fdt) { + fdt = machine->fdt; + } else { + fdt = pnv_dt_create(machine); + /* Pack resulting tree */ + _FDT((fdt_pack(fdt))); + } qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(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; + /* 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; + } } static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) @@ -952,6 +958,14 @@ static void pnv_init(MachineState *machine) g_free(sz); exit(EXIT_FAILURE); } + + /* checks for invalid option combinations */ + if (machine->dtb && (strlen(machine->kernel_cmdline) != 0)) { + error_report("-append and -dtb cannot be used together, as passed" + " command line is ignored in case of custom dtb"); + exit(EXIT_FAILURE); + } + memory_region_add_subregion(get_system_memory(), 0, machine->ram); /* @@ -1003,6 +1017,21 @@ static void pnv_init(MachineState *machine) } } + /* load dtb if passed */ + if (machine->dtb) { + int fdt_size; + + warn_report("with manually passed dtb, some options like '-append'" + " will get ignored and the dtb passed will be used as-is"); + + /* read the file 'machine->dtb', and load it into 'fdt' buffer */ + machine->fdt = load_device_tree(machine->dtb, &fdt_size); + if (!machine->fdt) { + error_report("Could not load dtb '%s'", machine->dtb); + exit(1); + } + } + /* MSIs are supported on this platform */ msi_nonbroken = true;
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 <adityag@linux.ibm.com> --- Changelog =========== v4: + use 'MachineState::fdt' instead of 'PnvMachineState::fdt' + added an 'if' check at end of pnv_reset, so we don't free the fdt we are using v3: + use 'load_device_tree' to read the device tree, instead of g_file_get_contents + tested that passed dtb does NOT get ignored on system_reset v2: + move reading dtb and warning to pnv_init v1: + use 'g_file_get_contents' and add check for -append & -dtb as suggested by Daniel --- --- hw/ppc/pnv.c | 51 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 11 deletions(-)