On Sun, Jul 01, 2018 at 07:19:53PM +0200, Cédric Le Goater wrote:
> When a PCI device is unplugged, the PCI memory regions are deleted
> before the optional ROM RAMBlock is flagged non-migratable. But, when
> this is done, the RAMBlock has already been cleared from the region,
> leading to a segv.
>
> Fix the issue by testing the RAMBlock before flagging it, as it is
> done in qemu_ram_unset_idstr()
>
> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> ---
>
> I caught this bug while deleting a passthrough device from a pseries
> machine. Here is the stack:
>
> #0 qemu_ram_unset_migratable (rb=0x0) at
> /home/legoater/work/qemu/qemu-xive-3.0.git/exec.c:1994
> #1 0x000000010072def0 in vmstate_unregister_ram (mr=0x101796af0,
> dev=<optimized out>)
> #2 0x0000000100694e5c in pci_del_option_rom (pdev=0x101796330)
> #3 pci_qdev_unrealize (dev=<optimized out>, errp=<optimized out>)
> #4 0x00000001005ff910 in device_set_realized (obj=0x101796330,
> value=<optimized out>, errp=0x0)
> #5 0x00000001007a487c in property_set_bool (obj=0x101796330, v=<optimized
> out>, name=<optimized out>,
> #6 0x00000001007a7878 in object_property_set (obj=0x101796330,
> v=0x7fff70033110,
> #7 0x00000001007aaf1c in object_property_set_qobject (obj=0x101796330,
> value=<optimized out>,
> #8 0x00000001007a7b90 in object_property_set_bool (obj=0x101796330,
> value=<optimized out>,
> #9 0x00000001005fcdd8 in device_unparent (obj=0x101796330)
> #10 0x00000001007a6dd0 in object_finalize_child_property (obj=<optimized
> out>, name=<optimized out>,
> #11 0x00000001007a50c0 in object_property_del_child (obj=0x10111f800,
> child=0x101796330,
> #12 0x0000000100425cc0 in spapr_phb_remove_pci_device_cb (dev=0x101796330)
> #13 0x0000000100427974 in spapr_drc_release (drc=0x1017e2df0)
> #14 0x0000000100429098 in spapr_drc_detach (drc=0x1017e2df0)
> #15 0x00000001004294e0 in drc_isolate_physical (drc=0x1017e2df0)
> #16 0x000000010042a50c in rtas_set_isolation_state (state=0,
> idx=<optimized out>)
>
> May be we should call pci_del_option_rom() before
> pci_unregister_io_regions() ?
This seems to make more sense to me.
Meanwhile I assume the name pci_del_option_rom() is a bit misleading -
it's not really deleting the ROM but unregistering the ROM only.
Instead IIUC it's pci_unregister_io_regions() which deleted that. So
maybe we can either rename the function pci_del_option_rom(), or we
can pick the ROM destruction out of pci_unregister_io_regions() and
put it into pci_del_option_rom() to make sure it's done as the last
step?
Regards,
>
> exec.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/exec.c b/exec.c
> index ebadc0e302ad..0dabf7380438 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1992,7 +1992,9 @@ void qemu_ram_set_migratable(RAMBlock *rb)
>
> void qemu_ram_unset_migratable(RAMBlock *rb)
> {
> - rb->flags &= ~RAM_MIGRATABLE;
> + if (rb) {
> + rb->flags &= ~RAM_MIGRATABLE;
> + }
> }
>
> /* Called with iothread lock held. */
> --
> 2.13.6
>
>
--
Peter Xu
