From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40516) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atHrY-00060C-Q1 for qemu-devel@nongnu.org; Thu, 21 Apr 2016 12:53:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1atHrU-0004Yn-LJ for qemu-devel@nongnu.org; Thu, 21 Apr 2016 12:53:56 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:47222) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atHrU-0004Yh-DE for qemu-devel@nongnu.org; Thu, 21 Apr 2016 12:53:52 -0400 Received: from localhost by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 21 Apr 2016 10:53:50 -0600 References: <1460752385-13259-1-git-send-email-duanj@linux.vnet.ibm.com> <1460752385-13259-2-git-send-email-duanj@linux.vnet.ibm.com> <20160420043042.GE1133@voom> From: Jianjun Duan Message-ID: <5719058E.2070809@linux.vnet.ibm.com> Date: Thu, 21 Apr 2016 09:53:34 -0700 MIME-Version: 1.0 In-Reply-To: <20160420043042.GE1133@voom> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/5] spapr: ensure device trees are always associated with DRC List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On 04/19/2016 09:30 PM, David Gibson wrote: > On Fri, Apr 15, 2016 at 01:33:01PM -0700, Jianjun Duan wrote: >> There are possible racing situations involving hotplug events and >> guest migration. For cases where a hotplug event is migrated, or >> the guest is in the process of fetching device tree at the time of >> migration, we need to ensure the device tree is created and >> associated with the corresponding DRC for devices that were >> hotplugged on the source, but 'coldplugged' on the target. >> >> Signed-off-by: Jianjun Duan > This seems fairly sensible - should be harmless and a bit cleaner > whether or not we strictly speaking need it. > > However, it is likely to conflict with some of the device tree > construction cleanups I have in my pipeline, so it may well need > rework for that. Sure. It will need rework for CPU hotplug too. >> --- >> hw/ppc/spapr.c | 16 ++++++---------- >> hw/ppc/spapr_pci.c | 12 +++++------- >> 2 files changed, 11 insertions(+), 17 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index feaab08..af4745c 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -2132,15 +2132,6 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr, uint64_t size, >> int i, fdt_offset, fdt_size; >> void *fdt; >> >> - /* >> - * Check for DRC connectors and send hotplug notification to the >> - * guest only in case of hotplugged memory. This allows cold plugged >> - * memory to be specified at boot time. >> - */ >> - if (!dev->hotplugged) { >> - return; >> - } >> - >> for (i = 0; i < nr_lmbs; i++) { >> drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, >> addr/SPAPR_MEMORY_BLOCK_SIZE); >> @@ -2154,7 +2145,12 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr, uint64_t size, >> drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp); >> addr += SPAPR_MEMORY_BLOCK_SIZE; >> } >> - spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs); >> + /* send hotplug notification to the >> + * guest only in case of hotplugged memory >> + */ >> + if (dev->hotplugged) { >> + spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs); >> + } >> } >> >> static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index 8c20d34..b179e42 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -1092,13 +1092,11 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc, >> spapr_tce_set_need_vfio(tcet, true); >> } >> >> - if (dev->hotplugged) { >> - fdt = create_device_tree(&fdt_size); >> - fdt_start_offset = spapr_create_pci_child_dt(phb, pdev, fdt, 0); >> - if (!fdt_start_offset) { >> - error_setg(errp, "Failed to create pci child device tree node"); >> - goto out; >> - } >> + fdt = create_device_tree(&fdt_size); >> + fdt_start_offset = spapr_create_pci_child_dt(phb, pdev, fdt, 0); >> + if (!fdt_start_offset) { >> + error_setg(errp, "Failed to create pci child device tree node"); >> + goto out; >> } >> >> drck->attach(drc, DEVICE(pdev),