| Message ID | 20240610062105.49848-2-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/ppc: Prefer HumanReadableText over Monitor | expand |
On 6/10/24 11:50, Philippe Mathieu-Daudé wrote: > Replace Monitor API by HumanReadableText one. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > --- > include/hw/pci-host/pnv_phb3.h | 2 +- > hw/pci-host/pnv_phb3_msi.c | 21 ++++++++++----------- > hw/ppc/pnv.c | 8 +++++++- > 3 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h > index d62b3091ac..24ca3dddaa 100644 > --- a/include/hw/pci-host/pnv_phb3.h > +++ b/include/hw/pci-host/pnv_phb3.h > @@ -40,7 +40,7 @@ void pnv_phb3_msi_update_config(Phb3MsiState *msis, uint32_t base, > void pnv_phb3_msi_send(Phb3MsiState *msis, uint64_t addr, uint16_t data, > int32_t dev_pe); > void pnv_phb3_msi_ffi(Phb3MsiState *msis, uint64_t val); > -void pnv_phb3_msi_pic_print_info(Phb3MsiState *msis, Monitor *mon); > +void pnv_phb3_msi_pic_print_info(Phb3MsiState *msis, GString *buf); > > > /* > diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c > index a6d827f903..77d673da54 100644 > --- a/hw/pci-host/pnv_phb3_msi.c > +++ b/hw/pci-host/pnv_phb3_msi.c > @@ -13,7 +13,6 @@ > #include "hw/pci-host/pnv_phb3.h" > #include "hw/ppc/pnv.h" > #include "hw/pci/msi.h" > -#include "monitor/monitor.h" > #include "hw/irq.h" > #include "hw/qdev-properties.h" > #include "sysemu/reset.h" > @@ -316,13 +315,13 @@ static void pnv_phb3_msi_register_types(void) > > type_init(pnv_phb3_msi_register_types); > > -void pnv_phb3_msi_pic_print_info(Phb3MsiState *msi, Monitor *mon) > +void pnv_phb3_msi_pic_print_info(Phb3MsiState *msi, GString *buf) > { > ICSState *ics = ICS(msi); > int i; > > - monitor_printf(mon, "ICS %4x..%4x %p\n", > - ics->offset, ics->offset + ics->nr_irqs - 1, ics); > + g_string_append_printf(buf, "ICS %4x..%4x %p\n", > + ics->offset, ics->offset + ics->nr_irqs - 1, ics); > > for (i = 0; i < ics->nr_irqs; i++) { > uint64_t ive; > @@ -335,12 +334,12 @@ void pnv_phb3_msi_pic_print_info(Phb3MsiState *msi, Monitor *mon) > continue; > } > > - monitor_printf(mon, " %4x %c%c server=%04x prio=%02x gen=%d\n", > - ics->offset + i, > - GETFIELD(IODA2_IVT_P, ive) ? 'P' : '-', > - GETFIELD(IODA2_IVT_Q, ive) ? 'Q' : '-', > - (uint32_t) GETFIELD(IODA2_IVT_SERVER, ive) >> 2, > - (uint32_t) GETFIELD(IODA2_IVT_PRIORITY, ive), > - (uint32_t) GETFIELD(IODA2_IVT_GEN, ive)); > + g_string_append_printf(buf, " %4x %c%c server=%04x prio=%02x gen=%d\n", > + ics->offset + i, > + GETFIELD(IODA2_IVT_P, ive) ? 'P' : '-', > + GETFIELD(IODA2_IVT_Q, ive) ? 'Q' : '-', > + (uint32_t) GETFIELD(IODA2_IVT_SERVER, ive) >> 2, > + (uint32_t) GETFIELD(IODA2_IVT_PRIORITY, ive), > + (uint32_t) GETFIELD(IODA2_IVT_GEN, ive)); > } > } > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 6e3a5ccdec..5356a4e295 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -38,6 +38,7 @@ > #include "hw/loader.h" > #include "hw/nmi.h" > #include "qapi/visitor.h" > +#include "qapi/type-helpers.h" > #include "monitor/monitor.h" > #include "hw/intc/intc.h" > #include "hw/ipmi/ipmi.h" > @@ -774,8 +775,13 @@ static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon) > for (i = 0; i < chip8->num_phbs; i++) { > PnvPHB *phb = chip8->phbs[i]; > PnvPHB3 *phb3 = PNV_PHB3(phb->backend); > + g_autoptr(GString) buf = g_string_new(""); > + g_autoptr(HumanReadableText) info = NULL; > + > + pnv_phb3_msi_pic_print_info(&phb3->msis, buf); > + info = human_readable_text_from_str(buf); > + monitor_puts(mon, info->human_readable_text); > > - pnv_phb3_msi_pic_print_info(&phb3->msis, mon); > ics_pic_print_info(&phb3->lsis, mon); > } > }
Hi Phillipe, One query below: On 6/17/24 15:41, Harsh Prateek Bora wrote: > > > On 6/10/24 11:50, Philippe Mathieu-Daudé wrote: >> Replace Monitor API by HumanReadableText one. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > >> --- >> include/hw/pci-host/pnv_phb3.h | 2 +- >> hw/pci-host/pnv_phb3_msi.c | 21 ++++++++++----------- >> hw/ppc/pnv.c | 8 +++++++- >> 3 files changed, 18 insertions(+), 13 deletions(-) >> <snip> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index 6e3a5ccdec..5356a4e295 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -38,6 +38,7 @@ >> #include "hw/loader.h" >> #include "hw/nmi.h" >> #include "qapi/visitor.h" >> +#include "qapi/type-helpers.h" >> #include "monitor/monitor.h" >> #include "hw/intc/intc.h" >> #include "hw/ipmi/ipmi.h" >> @@ -774,8 +775,13 @@ static void >> pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon) >> for (i = 0; i < chip8->num_phbs; i++) { >> PnvPHB *phb = chip8->phbs[i]; >> PnvPHB3 *phb3 = PNV_PHB3(phb->backend); >> + g_autoptr(GString) buf = g_string_new(""); >> + g_autoptr(HumanReadableText) info = NULL; >> + >> + pnv_phb3_msi_pic_print_info(&phb3->msis, buf); >> + info = human_readable_text_from_str(buf); >> + monitor_puts(mon, info->human_readable_text); >> - pnv_phb3_msi_pic_print_info(&phb3->msis, mon); >> ics_pic_print_info(&phb3->lsis, mon); How is the memory allocated to info in human_readable_text_from_str being reclaimed here? Isnt it a mem leak ? Thanks Harsh >> } >> } >
On 6/17/24 15:49, Harsh Prateek Bora wrote: > Hi Phillipe, > > One query below: > > On 6/17/24 15:41, Harsh Prateek Bora wrote: >> >> >> On 6/10/24 11:50, Philippe Mathieu-Daudé wrote: >>> Replace Monitor API by HumanReadableText one. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> >> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> >> >>> --- >>> include/hw/pci-host/pnv_phb3.h | 2 +- >>> hw/pci-host/pnv_phb3_msi.c | 21 ++++++++++----------- >>> hw/ppc/pnv.c | 8 +++++++- >>> 3 files changed, 18 insertions(+), 13 deletions(-) >>> > > <snip> > >>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >>> index 6e3a5ccdec..5356a4e295 100644 >>> --- a/hw/ppc/pnv.c >>> +++ b/hw/ppc/pnv.c >>> @@ -38,6 +38,7 @@ >>> #include "hw/loader.h" >>> #include "hw/nmi.h" >>> #include "qapi/visitor.h" >>> +#include "qapi/type-helpers.h" >>> #include "monitor/monitor.h" >>> #include "hw/intc/intc.h" >>> #include "hw/ipmi/ipmi.h" >>> @@ -774,8 +775,13 @@ static void >>> pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon) >>> for (i = 0; i < chip8->num_phbs; i++) { >>> PnvPHB *phb = chip8->phbs[i]; >>> PnvPHB3 *phb3 = PNV_PHB3(phb->backend); >>> + g_autoptr(GString) buf = g_string_new(""); >>> + g_autoptr(HumanReadableText) info = NULL; >>> + >>> + pnv_phb3_msi_pic_print_info(&phb3->msis, buf); >>> + info = human_readable_text_from_str(buf); >>> + monitor_puts(mon, info->human_readable_text); >>> - pnv_phb3_msi_pic_print_info(&phb3->msis, mon); >>> ics_pic_print_info(&phb3->lsis, mon); > > How is the memory allocated to info in human_readable_text_from_str > being reclaimed here? Isnt it a mem leak ? > Ok, I see, g_autoptr takes care of auto cleanup. Please ignore. > Thanks > Harsh > >>> } >>> } >>
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h index d62b3091ac..24ca3dddaa 100644 --- a/include/hw/pci-host/pnv_phb3.h +++ b/include/hw/pci-host/pnv_phb3.h @@ -40,7 +40,7 @@ void pnv_phb3_msi_update_config(Phb3MsiState *msis, uint32_t base, void pnv_phb3_msi_send(Phb3MsiState *msis, uint64_t addr, uint16_t data, int32_t dev_pe); void pnv_phb3_msi_ffi(Phb3MsiState *msis, uint64_t val); -void pnv_phb3_msi_pic_print_info(Phb3MsiState *msis, Monitor *mon); +void pnv_phb3_msi_pic_print_info(Phb3MsiState *msis, GString *buf); /* diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c index a6d827f903..77d673da54 100644 --- a/hw/pci-host/pnv_phb3_msi.c +++ b/hw/pci-host/pnv_phb3_msi.c @@ -13,7 +13,6 @@ #include "hw/pci-host/pnv_phb3.h" #include "hw/ppc/pnv.h" #include "hw/pci/msi.h" -#include "monitor/monitor.h" #include "hw/irq.h" #include "hw/qdev-properties.h" #include "sysemu/reset.h" @@ -316,13 +315,13 @@ static void pnv_phb3_msi_register_types(void) type_init(pnv_phb3_msi_register_types); -void pnv_phb3_msi_pic_print_info(Phb3MsiState *msi, Monitor *mon) +void pnv_phb3_msi_pic_print_info(Phb3MsiState *msi, GString *buf) { ICSState *ics = ICS(msi); int i; - monitor_printf(mon, "ICS %4x..%4x %p\n", - ics->offset, ics->offset + ics->nr_irqs - 1, ics); + g_string_append_printf(buf, "ICS %4x..%4x %p\n", + ics->offset, ics->offset + ics->nr_irqs - 1, ics); for (i = 0; i < ics->nr_irqs; i++) { uint64_t ive; @@ -335,12 +334,12 @@ void pnv_phb3_msi_pic_print_info(Phb3MsiState *msi, Monitor *mon) continue; } - monitor_printf(mon, " %4x %c%c server=%04x prio=%02x gen=%d\n", - ics->offset + i, - GETFIELD(IODA2_IVT_P, ive) ? 'P' : '-', - GETFIELD(IODA2_IVT_Q, ive) ? 'Q' : '-', - (uint32_t) GETFIELD(IODA2_IVT_SERVER, ive) >> 2, - (uint32_t) GETFIELD(IODA2_IVT_PRIORITY, ive), - (uint32_t) GETFIELD(IODA2_IVT_GEN, ive)); + g_string_append_printf(buf, " %4x %c%c server=%04x prio=%02x gen=%d\n", + ics->offset + i, + GETFIELD(IODA2_IVT_P, ive) ? 'P' : '-', + GETFIELD(IODA2_IVT_Q, ive) ? 'Q' : '-', + (uint32_t) GETFIELD(IODA2_IVT_SERVER, ive) >> 2, + (uint32_t) GETFIELD(IODA2_IVT_PRIORITY, ive), + (uint32_t) GETFIELD(IODA2_IVT_GEN, ive)); } } diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 6e3a5ccdec..5356a4e295 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -38,6 +38,7 @@ #include "hw/loader.h" #include "hw/nmi.h" #include "qapi/visitor.h" +#include "qapi/type-helpers.h" #include "monitor/monitor.h" #include "hw/intc/intc.h" #include "hw/ipmi/ipmi.h" @@ -774,8 +775,13 @@ static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon) for (i = 0; i < chip8->num_phbs; i++) { PnvPHB *phb = chip8->phbs[i]; PnvPHB3 *phb3 = PNV_PHB3(phb->backend); + g_autoptr(GString) buf = g_string_new(""); + g_autoptr(HumanReadableText) info = NULL; + + pnv_phb3_msi_pic_print_info(&phb3->msis, buf); + info = human_readable_text_from_str(buf); + monitor_puts(mon, info->human_readable_text); - pnv_phb3_msi_pic_print_info(&phb3->msis, mon); ics_pic_print_info(&phb3->lsis, mon); } }
Replace Monitor API by HumanReadableText one. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/pci-host/pnv_phb3.h | 2 +- hw/pci-host/pnv_phb3_msi.c | 21 ++++++++++----------- hw/ppc/pnv.c | 8 +++++++- 3 files changed, 18 insertions(+), 13 deletions(-)