Re: [PATCH] pnv/psi: Add VMSTATE information to PnvPsi

[Miles Glenn <milesg@linux.ibm.com>]

On Mon, 2025-12-15 at 07:07 -0600, Caleb Schlossin wrote:
> I can do that, if others agree. I see the commit msg should really be "hw/ppc: ...", so it would fit with the others.
> 
> - Caleb
> 

Even if that wasn't the case, a commit series is not required to
address a single component (they often don't) and the cover letter
subject doesn't even need to have the "hw/ppc:" prefix in it.

-Glenn

> On 12/12/25 11:05 AM, Miles Glenn wrote:
> > Caleb, why not add this commit to your other submission of patches
> > titled "Snapshot support for several ppc devices"?
> > 
> > -Glenn
> > 
> > On Thu, 2025-12-11 at 11:00 -0600, Caleb Schlossin wrote:
> > > PnvPsi needs to be able to save/load snapshots.  Add VMSTATE information
> > > to the device class and a post_load() method to restore dynamic data items and
> > > memory region mappings.
> > > 
> > > Signed-off-by: Michael Kowal <kowal@linux.ibm.com>
> > > Signed-off-by: Caleb Schlossin <calebs@linux.ibm.com>
> > > ---
> > >  hw/ppc/pnv_psi.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 44 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> > > index 5d947d8b52..88d5f1d45d 100644
> > > --- a/hw/ppc/pnv_psi.c
> > > +++ b/hw/ppc/pnv_psi.c
> > > @@ -25,6 +25,7 @@
> > >  #include "qemu/module.h"
> > >  #include "system/reset.h"
> > >  #include "qapi/error.h"
> > > +#include "migration/vmstate.h"
> > >  
> > >  
> > >  #include "hw/ppc/fdt.h"
> > > @@ -35,6 +36,8 @@
> > >  
> > >  #include <libfdt.h>
> > >  
> > > +#undef PSI_DEBUG
> > > +
> > >  #define PSIHB_XSCOM_FIR_RW      0x00
> > >  #define PSIHB_XSCOM_FIR_AND     0x01
> > >  #define PSIHB_XSCOM_FIR_OR      0x02
> > > @@ -130,12 +133,11 @@ static void pnv_psi_set_bar(PnvPsi *psi, uint64_t bar)
> > >  {
> > >      PnvPsiClass *ppc = PNV_PSI_GET_CLASS(psi);
> > >      MemoryRegion *sysmem = get_system_memory();
> > > -    uint64_t old = psi->regs[PSIHB_XSCOM_BAR];
> > >  
> > >      psi->regs[PSIHB_XSCOM_BAR] = bar & (ppc->bar_mask | PSIHB_BAR_EN);
> > >  
> > >      /* Update MR, always remove it first */
> > > -    if (old & PSIHB_BAR_EN) {
> > > +    if (memory_region_is_mapped(&psi->regs_mr)) {
> > >          memory_region_del_subregion(sysmem, &psi->regs_mr);
> > >      }
> > >  
> > > @@ -975,6 +977,40 @@ static void pnv_psi_register_types(void)
> > >  
> > >  type_init(pnv_psi_register_types);
> > >  
> > > +#ifdef PSI_DEBUG
> > > +static void psi_regs_pic_print_info(uint64_t *regs, uint32_t nr_regs,
> > > +                                    GString *buf) {
> > > +    uint i, prev_idx = -1;
> > > +    uint64_t  reg1, prev_reg1 = -1;
> > > +    uint64_t  reg2, prev_reg2 = -1;
> > > +    uint64_t  reg3, prev_reg3 = -1;
> > > +    uint64_t  reg4, prev_reg4 = -1;
> > > +    for (i = 0; i < nr_regs; i = i + 4) {
> > > +        /* Don't print if values do not change, but print last*/
> > > +        reg1 = regs[i];
> > > +        reg2 = regs[i + 1];
> > > +        reg3 = regs[i + 2];
> > > +        reg4 = regs[i + 3];
> > > +        if (reg1 == prev_reg1 && reg2 == prev_reg2 &&
> > > +            reg3 == prev_reg3 && reg4 == prev_reg4 &&
> > > +            i < (nr_regs - 4)) {
> > > +            if (i == (prev_idx + 4)) {
> > > +                g_string_append_printf(buf, "        . . .\n");
> > > +            }
> > > +            continue;
> > > +        }
> > > +
> > > +        g_string_append_printf(buf, "  [%03X] 0x%016lX %016lX %016lX %016lX\n",
> > > +            i, reg1, reg2, reg3, reg4);
> > > +        prev_idx = i;
> > > +        prev_reg1 = reg1;
> > > +        prev_reg2 = reg2;
> > > +        prev_reg3 = reg3;
> > > +        prev_reg4 = reg4;
> > > +    }
> > > +}
> > > +#endif
> > > +
> > >  void pnv_psi_pic_print_info(Pnv9Psi *psi9, GString *buf)
> > >  {
> > >      PnvPsi *psi = PNV_PSI(psi9);
> > > @@ -985,4 +1021,10 @@ void pnv_psi_pic_print_info(Pnv9Psi *psi9, GString *buf)
> > >      g_string_append_printf(buf, "PSIHB Source %08x .. %08x\n",
> > >                             offset, offset + psi9->source.nr_irqs - 1);
> > >      xive_source_pic_print_info(&psi9->source, offset, buf);
> > > +#ifdef PSI_DEBUG
> > > +    /* Print PSI registers */
> > > +    g_string_append_printf(buf, "\nPSI Regs[0x0..%X]\n",
> > > +                           PSIHB_XSCOM_MAX);
> > > +    psi_regs_pic_print_info(psi->regs, PSIHB_XSCOM_MAX, buf);
> > > +#endif
> > >  }