From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE Date: Tue, 30 Apr 2013 10:56:39 -0700 Message-ID: <20130430175639.GA6266@gmail.com> References: <1367331446-28030-1-git-send-email-marc.zyngier@arm.com> <1367331446-28030-5-git-send-email-marc.zyngier@arm.com> <518004D8.80801@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "kvmarm@lists.cs.columbia.edu" , linux-arm-kernel , KVM General To: Marc Zyngier Return-path: Received: from mail-pa0-f42.google.com ([209.85.220.42]:48274 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760807Ab3D3R4K (ORCPT ); Tue, 30 Apr 2013 13:56:10 -0400 Received: by mail-pa0-f42.google.com with SMTP id kl13so479960pab.29 for ; Tue, 30 Apr 2013 10:56:10 -0700 (PDT) Content-Disposition: inline In-Reply-To: <518004D8.80801@arm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Apr 30, 2013 at 06:52:24PM +0100, Marc Zyngier wrote: > On 30/04/13 18:40, Christoffer Dall wrote: > > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier wrote: > >> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, > >> not the size of the PGD. > >> > >> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE > >> is equal to 1. > >> > >> Signed-off-by: Marc Zyngier > >> --- > >> arch/arm/kvm/mmu.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > >> index b978ebe..09ece5c 100644 > >> --- a/arch/arm/kvm/mmu.c > >> +++ b/arch/arm/kvm/mmu.c > >> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > >> return -ENOMEM; > >> > >> /* stage-2 pgd must be aligned to its size */ > >> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); > >> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); > > > > I think the define is broken and should be fixed and/or renamed instead. > > To be fair, I wouldn't mind the whole check to be dropped altogether. If > __get_free_pages doesn't give you an aligned allocation, the whole > kernel is going the way of the dodo, and we shouldn't really care... > > Your call, really. > Yeah, this is from way back when, where the whole thing was different and we were debugging all sorts of issues, so we can get rid of both the check and the define in stead. Want to take care of it as part of your series? Thanks, -Christoffer