Re: [PATCH 03/26] hw/ppc: Avoid using Monitor in xive_tctx_pic_print_info()

[Philippe Mathieu-Daudé]

Hi Harsh,

On 17/6/24 12:19, Harsh Prateek Bora wrote:
> On 6/10/24 11:50, Philippe Mathieu-Daudé wrote:
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index fa23b27a2b..5854358f65 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1223,7 +1223,13 @@ static void 
> > pnv_chip_power9_intc_destroy(PnvChip *chip, PowerPCCPU *cpu)
> >   static void pnv_chip_power9_intc_print_info(PnvChip *chip, 
> > PowerPCCPU *cpu,
> >                                               Monitor *mon)
> >   {
> > -    xive_tctx_pic_print_info(XIVE_TCTX(pnv_cpu_state(cpu)->intc), mon);
> > +    g_autoptr(GString) buf = g_string_new("");
> > +    g_autoptr(HumanReadableText) info = NULL;
> > +
> > +    xive_tctx_pic_print_info(XIVE_TCTX(pnv_cpu_state(cpu)->intc), buf);
> > +
> > +    info = human_readable_text_from_str(buf);
> > +    monitor_puts(mon, info->human_readable_text);
> >   }
> >   static void pnv_chip_power10_intc_create(PnvChip *chip, PowerPCCPU 
> > *cpu,
> > @@ -1267,7 +1273,13 @@ static void 
> > pnv_chip_power10_intc_destroy(PnvChip *chip, PowerPCCPU *cpu)
> >   static void pnv_chip_power10_intc_print_info(PnvChip *chip, 
> > PowerPCCPU *cpu,
> >                                                Monitor *mon)
> >   {
> > -    xive_tctx_pic_print_info(XIVE_TCTX(pnv_cpu_state(cpu)->intc), mon);
> > +    g_autoptr(GString) buf = g_string_new("");
> > +    g_autoptr(HumanReadableText) info = NULL;
> > +
> > +    xive_tctx_pic_print_info(XIVE_TCTX(pnv_cpu_state(cpu)->intc), buf);
> > +
> > +    info = human_readable_text_from_str(buf);
> > +    monitor_puts(mon, info->human_readable_text);
> >   }
>
> We have an existing code duplication in above two routines which is 
> getting worse with these multi-lines getting duplicated. Could _power9_ 
> be changed to inline and called from _power10_ as well?

The duplicated lines get removed later. This series is a mere API
upgrade, the final diff is smaller (although harder to review, which
is why I did it with many trivial patches).

_power9_ / _power10_ simplification could be done but as a separate
patch from this series, since different matter IMHO.

Thanks for your reviews!

Phil.