From mboxrd@z Thu Jan 1 00:00:00 1970 From: Balbir Singh Subject: Re: [PATCH v2] KVM/PPC Patch for KVM issue in real mode Date: Wed, 30 Nov 2016 20:09:33 +1100 Message-ID: <2df982da-32f6-2634-52cc-071addae3b23@gmail.com> References: <20a3e138-8a6e-6ad8-b9ba-ec8332f011a5@gmail.com> <8760n5icw5.fsf@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: linuxppc-dev , Paolo Bonzini , Paul Mackerras , Radim To: "Aneesh Kumar K.V" , kvm-ppc@vger.kernel.org, kvm@vger.kernel.org Return-path: Received: from mail-pg0-f67.google.com ([74.125.83.67]:34960 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757264AbcK3JJu (ORCPT ); Wed, 30 Nov 2016 04:09:50 -0500 In-Reply-To: <8760n5icw5.fsf@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 30/11/16 19:35, Aneesh Kumar K.V wrote: > Balbir Singh writes: > >> Some KVM functions for book3s_hv are called in real mode. >> In real mode the top 4 bits of the address space are ignored, >> hence an address beginning with 0xc0000000+offset is the >> same as 0xd0000000+offset. The issue was observed when >> a kvm memslot resolution lead to random values when >> access from kvmppc_h_enter(). The issue is hit if the >> KVM host is running with a page size of 4K, since >> kvzalloc() looks at size < PAGE_SIZE. On systems with >> 64K the issue is not observed easily, it largely depends >> on the size of the structure being allocated. >> >> The proposed fix moves all KVM allocations for book3s_hv >> to kzalloc() until all structures used in real mode are >> audited. For safety allocations are moved to kmalloc >> space. The impact is a large allocation on systems with >> 4K page size. > > We did such access using *real_vmalloc_addr(void *x). So you are > suggesting here is we don't do that for all code path ? > Yep.. that is true > Do you have a stack dump for which you identified the issue ? > I found it with kvm_memslots, don't have a stack dump, but IIRC, I saw it with search_memslots <-- __gfn_to_memslot() >> >> Signed-off-by: Balbir Singh >> --- >> Changelog v2: >> Fix build failures reported by the kbuild test robot >> http://www.spinics.net/lists/kvm/msg141727.html >> >> arch/powerpc/include/asm/kvm_host.h | 19 +++++++++++++++++++ >> include/linux/kvm_host.h | 11 +++++++++++ >> virt/kvm/kvm_main.c | 2 +- >> 3 files changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h >> index f15713a..53f5172 100644 >> --- a/arch/powerpc/include/asm/kvm_host.h >> +++ b/arch/powerpc/include/asm/kvm_host.h >> @@ -734,6 +734,25 @@ struct kvm_vcpu_arch { >> #define __KVM_HAVE_ARCH_WQP >> #define __KVM_HAVE_CREATE_DEVICE >> >> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE >> +#define __KVM_HAVE_ARCH_VZALLOC_OVERRIDE > > do we need that OVERRIDE ? We usually have HAVE_ARCH_KVM_VZALLOC > or just say #ifndef kvm_arch_vzalloc ? > I can move __KVM_HAVE_ARCH_VZALLOC_OVERRIDE to HAVE_ARCH_KVM_VZALLOC_OVERRIDE if it helps with clarity and convention Thanks for the review, Balbir