From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Date: Tue, 17 Dec 2019 22:37:34 +0000 Subject: Re: [PATCH v4 01/19] KVM: x86: Allocate new rmap and large page tracking when moving memslot Message-Id: <20191217223734.GL7258@xz-x1> List-Id: References: <20191217204041.10815-1-sean.j.christopherson@intel.com> <20191217204041.10815-2-sean.j.christopherson@intel.com> <20191217215640.GI7258@xz-x1> <20191217222058.GD11771@linux.intel.com> In-Reply-To: <20191217222058.GD11771@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Sean Christopherson Cc: James Hogan , Paul Mackerras , Christian Borntraeger , Janosch Frank , Paolo Bonzini , Marc Zyngier , linux-arm-kernel@lists.infradead.org, Wanpeng Li , Philippe =?utf-8?Q?Mathieu-Daud=C3=A9?= , kvm@vger.kernel.org, David Hildenbrand , Joerg Roedel , Cornelia Huck , linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, Vitaly Kuznetsov , kvmarm@lists.cs.columbia.edu, Jim Mattson , David Gibson On Tue, Dec 17, 2019 at 02:20:59PM -0800, Sean Christopherson wrote: > > For example, I see PPC has this: > > > > struct kvm_arch_memory_slot { > > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > > unsigned long *rmap; > > #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ > > }; > > > > I started to look into HV code of it a bit, then I see... > > > > - kvm_arch_create_memslot(kvmppc_core_create_memslot_hv) init slot->arch.rmap, > > - kvm_arch_flush_shadow_memslot(kvmppc_core_flush_memslot_hv) didn't free it, > > - kvm_arch_prepare_memory_region(kvmppc_core_prepare_memory_region_hv) is nop. > > > > So Does it have similar issue? > > No, KVM doesn't allow a memslot's size to be changed, and PPC's rmap > allocation is directly tied to the size of the memslot. The x86 bug exists > because the size of its metadata arrays varies based on the alignment of > the base gfn. Yes, I was actually thinking those rmap would be invalid rather than the size after the move. But I think kvm_arch_flush_shadow_memslot() will flush all of them anyways... So yes it seems fine. Thanks, -- Peter Xu