From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42555) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsEra-00089e-C2 for qemu-devel@nongnu.org; Mon, 07 Jan 2013 10:43:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TsErY-0008SY-Bf for qemu-devel@nongnu.org; Mon, 07 Jan 2013 10:43:46 -0500 Received: from e8.ny.us.ibm.com ([32.97.182.138]:58523) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsErY-0008SB-7L for qemu-devel@nongnu.org; Mon, 07 Jan 2013 10:43:44 -0500 Received: from /spool/local by e8.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 7 Jan 2013 10:43:42 -0500 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 36E7D6E803C for ; Mon, 7 Jan 2013 10:43:13 -0500 (EST) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r07FhBVn109840 for ; Mon, 7 Jan 2013 10:43:12 -0500 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r07Fh7M7017455 for ; Mon, 7 Jan 2013 13:43:08 -0200 Message-ID: <50EAED06.5050501@linux.vnet.ibm.com> Date: Mon, 07 Jan 2013 10:43:02 -0500 From: "Jason J. Herne" MIME-Version: 1.0 References: <1356098191-4998-1-git-send-email-jjherne@us.ibm.com> <133FEF92-3C4F-48C8-BF67-E50066EEEF45@suse.de> <50E5D29B.6060804@linux.vnet.ibm.com> <20130104013812.GB23746@amt.cnet> <6A3DF150A5B70D4F9B66A25E3F7C888D06542905@039-SN2MPN1-022.039d.mgd.msft.net> <292DDE3D-7B6F-400E-954B-49CA3E284FDB@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06542EA8@039-SN2MPN1-022.039d.mgd.msft.net> <6A3DF150A5B70D4F9B66A25E3F7C888D06542EF7@039-SN2MPN1-022.039d.mgd.msft.net> <9B2CB541-8806-4BDF-A523-FD597BDFA08B@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06542FCA@039-SN2MPN1-022.039d.mgd.msft.net> <6501413C-7526-42DB-8824-C0638F59985A@suse.de> <50E6F479.4090002@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 7/7] KVM regsync: Fix do_kvm_cpu_synchronize_state data integrity issue Reply-To: jjherne@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Christian Borntraeger , Anthony Liguori , Marcelo Tosatti , "qemu-devel@nongnu.org qemu-devel" , Bhushan Bharat-R65777 On 01/04/2013 11:27 AM, Alexander Graf wrote: > > On 04.01.2013, at 16:25, Jason J. Herne wrote: > >> If I've followed the conversation correctly this is what needs to be done: >> >> 1. Remove the level parameters from kvm_arch_get_registers and kvm_arch_put_registers. >> >> 2. Add a new bitmap parameter to kvm_arch_get_registers and kvm_arch_put_registers. > > I would combine these into "replace levels with bitmap". > >> 3. Define a bit that correlates to our current notion of "all runtime registers". This bit, and all bits in this bitmap, would be architecture specific. > > Why would that bit be architecture specific? "All runtime registers" == "registers that gdb can access" IIRC. The implementation on what exactly that means obviously is architecture specific, but the bit itself would not be, as the gdbstub wants to be able to synchronize in arch independent code. > How do we want to define these bits? is it logical to break up the registers into smaller categories and then use masks to create RUNTIME_STATE, FULL_STATE, RESET_STATE? If so, how should we define them? Would they be arch specific and then we'd create the _STATE masks for each architecture? If we do simply define a bit for each of the above three states instead, they should probably be 100% mutually exclusive to provide the best protection against complicated data synchronization issues (like the original 7/7 patch was trying to prevent). Also, if we can assume 100% mutual exclusion the sync logic becomes trivial: static void do_kvm_cpu_synchronize_state(void *arg) { struct kvm_cpu_syncstate_args *args = arg; /* Do not sync regs that are already dirty */ int regs_to_get = args->regmap & ~cpu->kvm_vcpu_dirty; kvm_arch_get_registers(args->cpu, regs_to_get); args->cpu->kvm_vcpu_dirty |= regs_to_get; } Thoughts? -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)