| Message ID | 156415228410.1064338.4486161194061636096.stgit@bahia.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr/pci: Improve MSI tracking | expand |
On 26/07/2019 16:44, 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. This is fixing the reset bug. Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > > 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); > } > > static Property spapr_phb_properties[] = { >
Cc'ing qemu-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. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > } > > static Property spapr_phb_properties[] = { > >
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[] = { > > > >
On Fri, Jul 26, 2019 at 04:44:44PM +0200, 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> Applied to ppc-for-4.2, thanks. > --- > 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); > } > > static Property spapr_phb_properties[] = { >
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); } static Property spapr_phb_properties[] = {
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(+)