[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag |
Date: |
Tue, 3 Jul 2018 14:45:24 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 07/02/2018 05:57 AM, Peter Xu wrote:
> 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 <address@hidden>
>> ---
>>
>> 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?
So it is a little more complex than I thought.
The PCI device is a vfio PCI device and the PCI ROM region is initialized
in vfio_pci_size_rom() with memory_region_init_io(), which does not
allocate the RAMBlock, but has_rom is still set to true.
When the device is deleted, pci_del_option_rom() is called and with it,
vmstate_unregister_ram() because has_rom is set to true. Leading to the
SEGV.
I am not sure how to handle this case. It seems that the realize routine
of VFIOPCIDevice is hijacking a little the PCIDevice layer.
C.
- [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Cédric Le Goater, 2018/07/01
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Peter Xu, 2018/07/01
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag,
Cédric Le Goater <=
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Peter Xu, 2018/07/03
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Cédric Le Goater, 2018/07/04
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Paolo Bonzini, 2018/07/04
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Peter Xu, 2018/07/04
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Paolo Bonzini, 2018/07/04
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Cédric Le Goater, 2018/07/05
- Re: [Qemu-devel] [PATCH] exec.c: check RAMBlock validity before changing its flag, Paolo Bonzini, 2018/07/05