| Message ID | 20220805093948.82561-10-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | QMP/HMP: add 'dumpdtb' and 'info fdt' commands | expand |
On 05/08/2022 11:39, Daniel Henrique Barboza wrote: > This will enable support for 'dumpdtb' and 'info fdt' HMP commands for > all powernv machines. > > Cc: Cédric Le Goater <clg@kaod.org> > Cc: Frederic Barrat <fbarrat@linux.ibm.com> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hw/ppc/pnv.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index d3f77c8367..f5162f8b7b 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -608,7 +608,11 @@ static void pnv_reset(MachineState *machine) > qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); > cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); > > - g_free(fdt); > + /* > + * Update the machine->fdt pointer to enable support for > + * 'dumpdtb' and 'info fdt' commands. > + */ > + machine->fdt = fdt; Can pnv_reset() be called several times in the same instance of the qemu process, in which case we leak memory? Fred > } > > static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
On 8/5/22 08:03, Frederic Barrat wrote: > > > On 05/08/2022 11:39, Daniel Henrique Barboza wrote: >> This will enable support for 'dumpdtb' and 'info fdt' HMP commands for >> all powernv machines. >> >> Cc: Cédric Le Goater <clg@kaod.org> >> Cc: Frederic Barrat <fbarrat@linux.ibm.com> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> --- >> hw/ppc/pnv.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index d3f77c8367..f5162f8b7b 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -608,7 +608,11 @@ static void pnv_reset(MachineState *machine) >> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); >> cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); >> - g_free(fdt); >> + /* >> + * Update the machine->fdt pointer to enable support for >> + * 'dumpdtb' and 'info fdt' commands. >> + */ >> + machine->fdt = fdt; > > > Can pnv_reset() be called several times in the same instance of the qemu process, in which case we leak memory? hmmm I think it's possible if we issue a 'system_reset' via the monitor. I'll put a g_free(machine->fdt) before the assignment. Daniel > > Fred > > >> } >> static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
On Fri, Aug 05, 2022 at 09:31:11AM -0300, Daniel Henrique Barboza wrote: > > > On 8/5/22 08:03, Frederic Barrat wrote: > > > > > > On 05/08/2022 11:39, Daniel Henrique Barboza wrote: > > > This will enable support for 'dumpdtb' and 'info fdt' HMP commands for > > > all powernv machines. > > > > > > Cc: Cédric Le Goater <clg@kaod.org> > > > Cc: Frederic Barrat <fbarrat@linux.ibm.com> > > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > > --- > > > hw/ppc/pnv.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > > > index d3f77c8367..f5162f8b7b 100644 > > > --- a/hw/ppc/pnv.c > > > +++ b/hw/ppc/pnv.c > > > @@ -608,7 +608,11 @@ static void pnv_reset(MachineState *machine) > > > qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); > > > cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); > > > - g_free(fdt); > > > + /* > > > + * Update the machine->fdt pointer to enable support for > > > + * 'dumpdtb' and 'info fdt' commands. > > > + */ > > > + machine->fdt = fdt; > > > > > > Can pnv_reset() be called several times in the same instance of the qemu process, in which case we leak memory? > > hmmm I think it's possible if we issue a 'system_reset' via the > monitor. Right. I'm not certain about pnv, but on most platforms there's a way to trigger system_reset from the guest side as well. > I'll put a g_free(machine->fdt) before the assignment. > > > Daniel > > > > > > Fred > > > > > > > } > > > static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) >
On 8/5/22 11:39, Daniel Henrique Barboza wrote: > This will enable support for 'dumpdtb' and 'info fdt' HMP commands for > all powernv machines. I might have missed some emails but dumpdtb is already suppported : commit 8d4092614161 ("ppc/pnv: activate the "dumpdtb" option on the powernv machine") Thanks, C. > > Cc: Cédric Le Goater <clg@kaod.org> > Cc: Frederic Barrat <fbarrat@linux.ibm.com> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hw/ppc/pnv.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index d3f77c8367..f5162f8b7b 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -608,7 +608,11 @@ static void pnv_reset(MachineState *machine) > qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); > cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); > > - g_free(fdt); > + /* > + * Update the machine->fdt pointer to enable support for > + * 'dumpdtb' and 'info fdt' commands. > + */ > + machine->fdt = fdt; > } > > static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
On 8/8/22 08:47, Cédric Le Goater wrote: > On 8/5/22 11:39, Daniel Henrique Barboza wrote: >> This will enable support for 'dumpdtb' and 'info fdt' HMP commands for >> all powernv machines. > > I might have missed some emails but dumpdtb is already suppported : > commit 8d4092614161 ("ppc/pnv: activate the "dumpdtb" option on the > powernv machine") ok. found the patchset "QMP/HMP: add 'dumpdtb' and 'info fdt' commands" 'info fdt' would have been of great help when we were developing the PowerNV machine. Initially, I was even using pmemsave to extract the FDT blob ... So this is a great idea ! (which needs a g_free() ) Do we have something similar to dump ACPI tables, btw ? Thanks, C. >> >> Cc: Cédric Le Goater <clg@kaod.org> >> Cc: Frederic Barrat <fbarrat@linux.ibm.com> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> --- >> hw/ppc/pnv.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index d3f77c8367..f5162f8b7b 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -608,7 +608,11 @@ static void pnv_reset(MachineState *machine) >> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); >> cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); >> - g_free(fdt); >> + /* >> + * Update the machine->fdt pointer to enable support for >> + * 'dumpdtb' and 'info fdt' commands. >> + */ >> + machine->fdt = fdt; >> } >> static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) >
On 8/8/22 04:13, Cédric Le Goater wrote: > On 8/8/22 08:47, Cédric Le Goater wrote: >> On 8/5/22 11:39, Daniel Henrique Barboza wrote: >>> This will enable support for 'dumpdtb' and 'info fdt' HMP commands for >>> all powernv machines. >> >> I might have missed some emails but dumpdtb is already suppported : >> commit 8d4092614161 ("ppc/pnv: activate the "dumpdtb" option on the >> powernv machine") > > ok. found the patchset "QMP/HMP: add 'dumpdtb' and 'info fdt' commands" > > 'info fdt' would have been of great help when we were developing the > PowerNV machine. Initially, I was even using pmemsave to extract the > FDT blob ... So this is a great idea ! (which needs a g_free() ) > > Do we have something similar to dump ACPI tables, btw ? In QEMU? No idea. I didn't find users of libfdt in x86 files so I didn't bothered checking. I am aware of something you can do in userland to dump the ACPI tables. I did it once for research when I was working in the NUMA FORM2 extension for pseries. This is the procedure do dump the ACPI SLIT table: danielhb@ubuntu-vm:~$ sudo acpidump > acpidata.dat [sudo] password for danielhb: danielhb@ubuntu-vm:~$ danielhb@ubuntu-vm:~$ sudo acpixtract -sSLIT acpidata.dat Intel ACPI Component Architecture ACPI Binary Table Extraction Utility version 20200925 Copyright (c) 2000 - 2020 Intel Corporation SLIT - 60 bytes written (0x0000003C) - slit.dat danielhb@ubuntu-vm:~$ danielhb@ubuntu-vm:~$ iasl -d slit.dat Intel ACPI Component Architecture ASL+ Optimizing Compiler/Disassembler version 20200925 Copyright (c) 2000 - 2020 Intel Corporation File appears to be binary: found 24 non-ASCII characters, disassembling Binary file appears to be a valid ACPI table, disassembling Input file slit.dat, Length 0x3C (60) bytes ACPI: SLIT 0x0000000000000000 00003C (v01 BOCHS BXPCSLIT 00000001 BXPC 00000001) Acpi Data Table [SLIT] decoded Formatted output: slit.dsl - 1489 bytes danielhb@ubuntu-vm:~$ danielhb@ubuntu-vm:~$ cat slit.dsl /* * Intel ACPI Component Architecture * AML/ASL+ Disassembler version 20200925 (64-bit version) * Copyright (c) 2000 - 2020 Intel Corporation * * Disassembly of slit.dat, Wed Jun 2 19:00:54 2021 * * ACPI Data Table [SLIT] * * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue */ [000h 0000 4] Signature : "SLIT" [System Locality Information Table] [004h 0004 4] Table Length : 0000003C [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : A0 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPCSLIT" [018h 0024 4] Oem Revision : 00000001 [01Ch 0028 4] Asl Compiler ID : "BXPC" [020h 0032 4] Asl Compiler Revision : 00000001 [024h 0036 8] Localities : 0000000000000004 [02Ch 0044 4] Locality 0 : 0A 16 16 16 [030h 0048 4] Locality 1 : 2C 0A 2C 2C [034h 0052 4] Locality 2 : 42 42 0A 42 [038h 0056 4] Locality 3 : 58 58 58 0A Raw Table Data: Length 60 (0x3C) 0000: 53 4C 49 54 3C 00 00 00 01 A0 42 4F 43 48 53 20 // SLIT<.....BOCHS 0010: 42 58 50 43 53 4C 49 54 01 00 00 00 42 58 50 43 // BXPCSLIT....BXPC 0020: 01 00 00 00 04 00 00 00 00 00 00 00 0A 16 16 16 // ................ 0030: 2C 0A 2C 2C 42 42 0A 42 58 58 58 0A // ,.,,BB.BXXX. danielhb@ubuntu-vm:~$ So basically a combination of acpidump and acpixtract commands in the guest. Daniel > > Thanks, > > C. > > >>> >>> Cc: Cédric Le Goater <clg@kaod.org> >>> Cc: Frederic Barrat <fbarrat@linux.ibm.com> >>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >>> --- >>> hw/ppc/pnv.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >>> index d3f77c8367..f5162f8b7b 100644 >>> --- a/hw/ppc/pnv.c >>> +++ b/hw/ppc/pnv.c >>> @@ -608,7 +608,11 @@ static void pnv_reset(MachineState *machine) >>> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); >>> cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); >>> - g_free(fdt); >>> + /* >>> + * Update the machine->fdt pointer to enable support for >>> + * 'dumpdtb' and 'info fdt' commands. >>> + */ >>> + machine->fdt = fdt; >>> } >>> static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) >> >
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index d3f77c8367..f5162f8b7b 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -608,7 +608,11 @@ static void pnv_reset(MachineState *machine) qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); - g_free(fdt); + /* + * Update the machine->fdt pointer to enable support for + * 'dumpdtb' and 'info fdt' commands. + */ + machine->fdt = fdt; } static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
This will enable support for 'dumpdtb' and 'info fdt' HMP commands for all powernv machines. Cc: Cédric Le Goater <clg@kaod.org> Cc: Frederic Barrat <fbarrat@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/pnv.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)