From patchwork Wed Dec 31 03:33:13 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Li, Liang Z" X-Patchwork-Id: 424723 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id F2A0A1400B7 for ; Wed, 31 Dec 2014 14:38:49 +1100 (AEDT) Received: from localhost ([::1]:39057 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y6A7S-0003Fn-Jd for incoming@patchwork.ozlabs.org; Tue, 30 Dec 2014 22:38:46 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43565) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y6A7D-0002ov-1m for qemu-devel@nongnu.org; Tue, 30 Dec 2014 22:38:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y6A77-0007Qx-VB for qemu-devel@nongnu.org; Tue, 30 Dec 2014 22:38:30 -0500 Received: from mga03.intel.com ([134.134.136.65]:20916) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y6A77-0007Qf-Pi for qemu-devel@nongnu.org; Tue, 30 Dec 2014 22:38:25 -0500 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP; 30 Dec 2014 19:35:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,670,1413270000"; d="scan'208";a="655193734" Received: from pgsmsx102.gar.corp.intel.com ([10.221.44.80]) by fmsmga002.fm.intel.com with ESMTP; 30 Dec 2014 19:38:11 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by PGSMSX102.gar.corp.intel.com (10.221.44.80) with Microsoft SMTP Server (TLS) id 14.3.195.1; Wed, 31 Dec 2014 11:33:47 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.216]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.182]) with mapi id 14.03.0195.001; Wed, 31 Dec 2014 11:33:14 +0800 From: "Li, Liang Z" To: "qemu-devel@nongnu.org" , Paolo Bonzini Thread-Topic: about the re-attach more than one pci devices failed Thread-Index: AdAkpUWE3YGlP96FTMC+TU4OvqogbA== Date: Wed, 31 Dec 2014 03:33:13 +0000 Message-ID: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 134.134.136.65 Cc: "Zhang, Yang Z" Subject: [Qemu-devel] about the re-attach more than one pci devices failed X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Hi Paolo, We have found a bug in all the xen-4.4 and xen-4.5-rcx, the bug can be reproduced by the following steps: Use the 'xl pci-attach $DomU $BDF' command to attach more then one PCI devices to the guest, then detach the devices with 'xl pci-detach $DomU $BDF', after that, re-attach these PCI devices again, an error message will be reported like following: libxl: error: libxl_qmp.c:287:qmp_handle_error_response: receive an error message from QMP server: Duplicate ID 'pci-pt-03_10.1' for device. By debugging, I found the count of calling xen_pt_region_add and xen_pt_region_del are not the same, and this may cause the XenPCIPassthroughState and it's related QemuOpts object not be released properly. I don't know how this happened, but the following patch can fix this bug. By debugging, I found when using 'address_space_memory', xen_pt_region_del won't be called when the memory region is not ' xen-pci-pt-*', when using ' s->dev.bus_master_as ', there is no such issue. I am not sure use 's->dev.bus_master_as' instead of 'address_space_memory' is right. Could you give some suggestion? Liang diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index be4220b..a418c53 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -607,7 +607,6 @@ static void xen_pt_region_add(MemoryListener *l, MemoryRegionSection *sec) XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState, memory_listener); - memory_region_ref(sec->mr); xen_pt_region_update(s, sec, true); } @@ -617,7 +616,6 @@ static void xen_pt_region_del(MemoryListener *l, MemoryRegionSection *sec) memory_listener); xen_pt_region_update(s, sec, false); - memory_region_unref(sec->mr); } static void xen_pt_io_region_add(MemoryListener *l, MemoryRegionSection *sec) @@ -625,7 +623,6 @@ static void xen_pt_io_region_add(MemoryListener *l, MemoryRegionSection *sec) XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState, io_listener); - memory_region_ref(sec->mr); xen_pt_region_update(s, sec, true); } @@ -635,7 +632,6 @@ static void xen_pt_io_region_del(MemoryListener *l, MemoryRegionSection *sec) io_listener); xen_pt_region_update(s, sec, false); - memory_region_unref(sec->mr); } static const MemoryListener xen_pt_memory_listener = { After reading other parts of the source code, I don't think the above patch is a good fix. I have verified the following patch can work too: diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index c1bf357..f2893b2 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -736,7 +736,7 @@ static int xen_pt_initfn(PCIDevice *d) } out: - memory_listener_register(&s->memory_listener, &address_space_memory); + memory_listener_register(&s->memory_listener, &s->dev.bus_master_as); memory_listener_register(&s->io_listener, &address_space_io); XEN_PT_LOG(d, "Real physical device %02x:%02x.%d registered successfully!\n",