From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [Qemu-devel] [PATCH v2 0/8] nvdimm: hotplug support Date: Mon, 10 Oct 2016 21:57:55 +0800 Message-ID: <560c2e37-ef5d-c0b0-0059-29d07cecd443@linux.intel.com> References: <1470984850-66891-1-git-send-email-guangrong.xiao@linux.intel.com> <20161003154800.17e3ba30@nial.brq.redhat.com> <20161010145931.59c0c5bb@nial.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net To: Igor Mammedov Return-path: Received: from mga14.intel.com ([192.55.52.115]:8875 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752736AbcJJOEc (ORCPT ); Mon, 10 Oct 2016 10:04:32 -0400 In-Reply-To: <20161010145931.59c0c5bb@nial.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 10/10/2016 08:59 PM, Igor Mammedov wrote: > On Sat, 8 Oct 2016 16:34:06 +0800 > Xiao Guangrong wrote: > >> On 10/03/2016 09:48 PM, Igor Mammedov wrote: >>> On Fri, 12 Aug 2016 14:54:02 +0800 >>> Xiao Guangrong wrote: >>> >>> General design issue in this series is regenerating >>> _FIT data every time inside of _FIT read loop. >>> >>> The issue here is that if FIT data doesn't fit in one page >>> RFIT would be called several times resulting in calling >>> nvdimm_dsm_func_read_fit() N-times, and in between >>> these N exits generated FIT data could change >>> (for example if another NVDIMM has been hotpluged in between) >>> resulting in corrupted concatenated FIT buffer returned by >>> _FIT method. >>> So one need either to block follow up hotplug or make sure >>> that FIT data regenerated/updated only once per _FIT >>> invocation and stay immutable until _FIT is completed >>> (maybe RCU protected buffer). >>> > > >>> Regenerating/updating inside qemu could also be shared >>> with NFIT table creation path so that both cold/hotplug >>> would reuse the same data which are updated only when >>> a NVDIMM is added/removed. > try to take into account this point so that FIT data > aren't regenerated on every RFIT invocation. Okay. The reason that the FIT was always regenerated is avoiding saving FIT during live-migration. Well, it seems that we are hard to avoid it now. :) > > >> Explicitly sync up QEMU and OSPM is hard as OSPM is running >> in a VM which is vulnerable and can not be expected always >> triggering correct sequence. >> >> So how about introducing generation-number which is updated >> for each hotplug event and a new DSM function reserved to >> get this number by OSPM. Then the ACPI code is like this: >> >> restart: >> old-number = DSM(Read_NUM); >> FIT = DSM(FIT) >> new-number = DSM(Read_NUM); >> >> if (old-number != new-number) >> goto restart; >> >> return FIT > It's a bit complicated. > Hmm, how about this algorithm: struct fit_data { u64 generation_number; GArray *fit; }; struct fit_data *fit; u64 current_number; The update-side (hotplug happens): new_fit = malloc(); old_fit = rcu_dereference(fit); new_fit->generation_number = old_fit->generation_number + 1; new_fit->fit = ...generate-fit...; rcu_assign(fit, new_fit); sync_rcu() free(old_fit); On the read-side (DSM issued by OSPM): { rcu_read_lock() fit = rcu_dereference(fit); /* if it is the first time issuing RFIT. */ if (fit-read-position == 0) current_number = fit->generation_number; else if (current_number != fit->generation_number) return FAIL_FIT_CHANGED; ... fill returned info with fit->fit...; rcu_read_unlock() } Of course, @fit and @current_number should be persistent during live migration.