From patchwork Wed Jun 10 07:45:37 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Han, Weidong" X-Patchwork-Id: 29207 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n5A7kAk6000706 for ; Wed, 10 Jun 2009 07:46:10 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754741AbZFJHqG (ORCPT ); Wed, 10 Jun 2009 03:46:06 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754689AbZFJHqF (ORCPT ); Wed, 10 Jun 2009 03:46:05 -0400 Received: from mga01.intel.com ([192.55.52.88]:7798 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754700AbZFJHqE convert rfc822-to-8bit (ORCPT ); Wed, 10 Jun 2009 03:46:04 -0400 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 10 Jun 2009 00:33:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.41,339,1241420400"; d="scan'208";a="465056425" Received: from pgsmsx602.gar.corp.intel.com ([10.221.43.81]) by fmsmga002.fm.intel.com with ESMTP; 10 Jun 2009 00:40:05 -0700 Received: from pgsmsx601.gar.corp.intel.com (10.221.43.69) by pgsmsx602.gar.corp.intel.com (10.221.43.81) with Microsoft SMTP Server (TLS) id 8.1.358.0; Wed, 10 Jun 2009 15:45:40 +0800 Received: from pdsmsx601.ccr.corp.intel.com (172.16.12.94) by pgsmsx601.gar.corp.intel.com (10.221.43.69) with Microsoft SMTP Server (TLS) id 8.1.358.0; Wed, 10 Jun 2009 15:45:39 +0800 Received: from pdsmsx503.ccr.corp.intel.com ([172.16.12.95]) by pdsmsx601.ccr.corp.intel.com ([172.16.12.94]) with mapi; Wed, 10 Jun 2009 15:45:39 +0800 From: "Han, Weidong" To: "'Gerd Hoffmann'" , "'Paul Brook'" CC: "'avi@redhat.com'" , "'kvm@vger.kernel.org'" Date: Wed, 10 Jun 2009 15:45:37 +0800 Subject: RE: [PATCH RFC] qemu: fix hot remove assigned device Thread-Topic: [PATCH RFC] qemu: fix hot remove assigned device Thread-Index: AcnpGHbOg88ubPFxSyyvegaIDLOs7QAX56Dw Message-ID: <715D42877B251141A38726ABF5CABF2C054596A9FF@pdsmsx503.ccr.corp.intel.com> References: <1244481435-17224-1-git-send-email-weidong.han@intel.com> <200906081538.22186.paul@codesourcery.com> <715D42877B251141A38726ABF5CABF2C054590C496@pdsmsx503.ccr.corp.intel.com> <200906091551.41674.paul@codesourcery.com> <4A2E81A9.4060501@redhat.com> In-Reply-To: <4A2E81A9.4060501@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: en-US MIME-Version: 1.0 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Gerd Hoffmann wrote: > On 06/09/09 16:51, Paul Brook wrote: >> On Tuesday 09 June 2009, Han, Weidong wrote: >>> Paul Brook wrote: >>>> On Monday 08 June 2009, Weidong Han wrote: >>>>> When hot remove an assigned device, segmentation fault was >>>>> triggered by qemu_free(&pci_dev->qdev) in pci_unregister_device(). >>>>> pci_register_device() doesn't initialize or set pci_dev->qdev. >>>>> For an assigned device, qdev variable isn't touched at all. So >>>>> segmentation fault happens when to free a non-initialized qdev. >>>> Better would be to just disable hot remove for devices still using >>>> the legacy pci_register_device API. >>> PCI passthrough uses pci_register_device to register assigned >>> device to qemu. Is there newer API to do so? >> >> Yes. See e.g. LSI scsi emulation. > > Well. Except that you can't (yet) register pci config read/write > callbacks using the qdev-based API. > So pci passthrough have to use pci_register_device now. I cooked a new patch (post below) to fix this issue. Thanks. When hot remove an assigned device, segmentation fault was triggered by qdev_free(&pci_dev->qdev) in pci_unregister_device(). pci_register_device() doesn't initialize or set pci_dev->qdev. For an assigned device, qdev variable isn't touched at all. So segmentation fault happens when to free a non-initialized qdev. This patch adds a parameter in pci_unregister_device to check if it's an assigned device. For assgined device, free pci_dev instead of qdev. No changes for other devices. Signed-off-by: Weidong Han --- hw/device-assignment.c | 2 +- hw/pci-hotplug.c | 2 +- hw/pci.c | 8 ++++++-- hw/pci.h | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index 624d15a..65920d0 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -581,7 +581,7 @@ static void free_assigned_device(AssignedDevInfo *adev) dev->real_device.config_fd = 0; } - pci_unregister_device(&dev->dev); + pci_unregister_device(&dev->dev, 1); #ifdef KVM_CAP_IRQ_ROUTING free_dev_irq_entries(dev); #endif diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c index d7c8b84..18a4912 100644 --- a/hw/pci-hotplug.c +++ b/hw/pci-hotplug.c @@ -271,6 +271,6 @@ void pci_device_hot_remove_success(int pcibus, int slot) break; } - pci_unregister_device(d); + pci_unregister_device(d, 0); } diff --git a/hw/pci.c b/hw/pci.c index 25581a4..35fd064 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -363,7 +363,7 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev) } } -int pci_unregister_device(PCIDevice *pci_dev) +int pci_unregister_device(PCIDevice *pci_dev, int assigned) { int ret = 0; @@ -377,7 +377,11 @@ int pci_unregister_device(PCIDevice *pci_dev) qemu_free_irqs(pci_dev->irq); pci_irq_index--; pci_dev->bus->devices[pci_dev->devfn] = NULL; - qdev_free(&pci_dev->qdev); + + if (assigned) + qemu_free(pci_dev); + else + qdev_free(&pci_dev->qdev); return 0; } diff --git a/hw/pci.h b/hw/pci.h index f2dccb5..f658e78 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -199,7 +199,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name, int instance_size, int devfn, PCIConfigReadFunc *config_read, PCIConfigWriteFunc *config_write); -int pci_unregister_device(PCIDevice *pci_dev); +int pci_unregister_device(PCIDevice *pci_dev, int assigned); void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type,