Re: [Qemu-devel] [PATCH 2/3] spapr/pci: Free MSIs during reset

[Greg Kurz <groug@kaod.org>]

On Fri, 26 Jul 2019 18:17:57 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Cc'ing qemu-stable@
> 

This patch relies on the previous one, otherwise g_hash_table_remove_all() will
just g_free() the spapr_pci_msi structures, but it will not tear down the MSIs
in the interrupt controller.

Also, this bug is so old that I'm not sure it qualifies for stable.

> On 7/26/19 4:44 PM, Greg Kurz wrote:
> > When the machine is reset, the MSI bitmap is cleared but the allocated
> > MSIs are not freed. Some operating systems, such as AIX, can detect the
> > previous configuration and assert.
> > 
> > Empty the MSI cache, this performs the needed cleanup.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/ppc/spapr_pci.c |    2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index bc22568bfa71..e45507bf2b53 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -2078,6 +2078,8 @@ static void spapr_phb_reset(DeviceState *qdev)
> >      if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) {
> >          spapr_phb_vfio_reset(qdev);
> >      }
> > +
> > +    g_hash_table_remove_all(sphb->msi);
> 
> It is not clear to my why spapr_phb_unrealize() doesn't require the same
> call, but this is not related to this patch.
> 

Because spapr_phb_unrealize() does this:

    if (sphb->msi) {
        g_hash_table_unref(sphb->msi);
        sphb->msi = NULL;
    }

and

https://developer.gnome.org/glib/stable/glib-Hash-Tables.html#g-hash-table-unref

g_hash_table_unref ()

void
g_hash_table_unref (GHashTable *hash_table);

Atomically decrements the reference count of hash_table by one. If the reference
count drops to 0, all keys and values will be destroyed, and all memory allocated
by the hash table is released. This function is MT-safe and may be called from any
thread.

If I have to re-post, I can make it clear with a comment.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> >  }
> >  
> >  static Property spapr_phb_properties[] = {
> > 
> >