On 25/06/2023 07:01, Duan, Zhenzhong wrote: >> -----Original Message----- >> From: Joao Martins <joao.m.mart...@oracle.com> >> Sent: Wednesday, June 21, 2023 7:09 PM >> To: Duan, Zhenzhong <zhenzhong.d...@intel.com> >> Cc: alex.william...@redhat.com; c...@redhat.com; qemu-devel@nongnu.org; >> avih...@nvidia.com; Peng, Chao P <chao.p.p...@intel.com> >> Subject: Re: [PATCH v3 2/3] vfio/pci: Fix a segfault in vfio_realize >> >> >> >> On 21/06/2023 09:02, Zhenzhong Duan wrote: >>> In case irqchip_change_notifier isn't added, removing it triggers segfault. >>> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com> >>> --- >>> hw/vfio/pci.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index >>> c71b0955d81c..82c4cf4f7609 100644 >>> --- a/hw/vfio/pci.c >>> +++ b/hw/vfio/pci.c >>> @@ -3222,7 +3222,9 @@ static void vfio_realize(PCIDevice *pdev, Error >>> **errp) >>> >>> out_deregister: >>> pci_device_set_intx_routing_notifier(&vdev->pdev, NULL); >>> - kvm_irqchip_remove_change_notifier(&vdev->irqchip_change_notifier); >>> + if (vdev->irqchip_change_notifier.notify) { >>> + kvm_irqchip_remove_change_notifier(&vdev- >>> irqchip_change_notifier); >>> + } >> >> If the first patch ends up being pursued (which I am not quite sure) it >> should >> be folded in the previous patch, as the out_deregister is used starting your >> patch 1. > Sorry for late response, just back from vacation. > > out_deregister isn't only for vfio migration, there are some other jump sites > to out_deregister in vfio_realize. Take below code for example: > > if (vdev->display_xres || vdev->display_yres) { > if (vdev->dpy == NULL) { > error_setg(errp, "xres and yres properties require display=on"); > goto out_deregister; > } > > I can reproduce a segmentation fault when hotplug a vfio device using below > cmd: > (qemu) device_add vfio-pci,host=81:11.1,id=vfio1,bus=root1,xres=1 Connection > closed by foreign host. > > After fix: > (qemu) device_add vfio-pci,host=81:11.1,id=vfio1,bus=root1,xres=1 > Error: vfio 0000:81:11.1: xres and yres properties require display=on > (qemu) >
Makes sense. Let's keep it separate then. > Thanks > Zhenzhong > >