From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D154FC0044C for ; Thu, 1 Nov 2018 08:36:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 96F7E204FD for ; Thu, 1 Nov 2018 08:36:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 96F7E204FD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728258AbeKARi0 (ORCPT ); Thu, 1 Nov 2018 13:38:26 -0400 Received: from foss.arm.com ([217.140.101.70]:53084 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728161AbeKARiZ (ORCPT ); Thu, 1 Nov 2018 13:38:25 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4498480D; Thu, 1 Nov 2018 01:36:25 -0700 (PDT) Received: from localhost (e113682-lin.copenhagen.arm.com [10.32.144.41]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9E50B3F557; Thu, 1 Nov 2018 01:36:24 -0700 (PDT) Date: Thu, 1 Nov 2018 09:36:22 +0100 From: Christoffer Dall To: Suzuki K Poulose Cc: linux-arm-kernel@lists.infradead.org, cdall@kernel.org, kvm@vger.kernel.org, marc.zyngier@arm.com, will.deacon@arm.com, linux-kernel@vger.kernel.org, dave.martin@arm.com, pbonzini@redhat.com, kvmarm@lists.cs.columbia.edu, Punit Agrawal Subject: Re: [PATCH v6 18/18] kvm: arm64: Allow tuning the physical address size for VM Message-ID: <20181101083622.GH12057@e113682-lin.lund.arm.com> References: <20180926163258.20218-1-suzuki.poulose@arm.com> <20180926163258.20218-19-suzuki.poulose@arm.com> <20181031142216.GD12057@e113682-lin.lund.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 31, 2018 at 05:55:13PM +0000, Suzuki K Poulose wrote: > Hi Christoffer, > > On 31/10/18 14:22, Christoffer Dall wrote: > >On Wed, Sep 26, 2018 at 05:32:54PM +0100, Suzuki K Poulose wrote: > >>Allow specifying the physical address size limit for a new > >>VM via the kvm_type argument for the KVM_CREATE_VM ioctl. This > >>allows us to finalise the stage2 page table as early as possible > >>and hence perform the right checks on the memory slots > >>without complication. The size is encoded as Log2(PA_Size) in > >>bits[7:0] of the type field. For backward compatibility the > >>value 0 is reserved and implies 40bits. Also, lift the limit > >>of the IPA to host limit and allow lower IPA sizes (e.g, 32). > >> > >>The userspace could check the extension KVM_CAP_ARM_VM_IPA_SIZE > >>for the availability of this feature. The cap check returns the > >>maximum limit for the physical address shift supported by the host. > >> > >>Cc: Marc Zyngier > >>Cc: Christoffer Dall > >>Cc: Peter Maydell > >>Cc: Paolo Bonzini > >>Cc: Radim Krčmář > >>Signed-off-by: Suzuki K Poulose > >>--- > > >>@@ -192,17 +195,23 @@ int kvm_arm_config_vm(struct kvm *kvm, unsigned long type) > >> u32 parange, phys_shift; > >> u8 lvls; > >>- if (type) > >>+ if (type & ~KVM_VM_TYPE_ARM_IPA_SIZE_MASK) > >> return -EINVAL; > >>+ phys_shift = KVM_VM_TYPE_ARM_IPA_SIZE(type); > >>+ if (phys_shift) { > >>+ if (phys_shift > kvm_ipa_limit || > >>+ phys_shift < 32) > >>+ return -EINVAL; > > > >I am concerned here that if we allow the user to set the phys_size to 32 > >bits, then we end up with 2 levels of stage2 page tables, which means > >that the size of a stage2 pmd mapping becomes the size of a stage2 pgd > >mapping, yet we can still decide in user_mem_abort() that a stage2 fault > >is backed by PMD size mappings on the host, and attempt a huge mapping > >at stage2, which then becomes a PGD level block map, I think. > > Yes, you're right. We will have a pgd-level block map in that case. > This should work transparently as PMD at stage2 is folded into PGD and > we endup marking the PGD entry as huge and the stage2 accessors deal > with it appropriately. This is similar to having a PMD huge page with > 64K + 42bit VA (2 level page table) on stage1. > > > > >Is this handled somehow? If so, how? > > We don't prevent this. We have a guaranteed minimum number of levels > at 2, which implies you can map a stage1 PMD huge page at stage2. > I acknowledge that the Linux naming convention does cause some confusion > for a "level" at stage1 and stage2 levels. But if you think of it > from the hardware level (and like the ARM ARM defines it , Level 0-3), > it is much simpler. i.e, you can map a huge page at level N in stage1 > into stage2 if you have that level N. It doesn't matter if stage2 has > more or less number of levels than stage1, as long as stage2 table can > deal with it. > That is indeed a good way to reason about it. > > > >I can't see user_mem_abort() being modified to explicitly handle this > >in your code, but perhaps the stage2_set_pmd_huge() call ends up > >actually mapping at the stage2 pte level, but I can't tell that it does. > > The stage2_set_pmd_huge() installs it at the PGD (level 2, which would > have been PMD if we had levels > 2) slot. > > pmd = stage2_get_pmd(addr) > \-> pud = stage2_get_pud(addr) > \-> pgd = kvm->arch.pgd + stage2_pgd_index(addr); > \-> (we have stage2_pgd_none(x) = 0 and > \-> stage2_pud_offset(pgd, addr) = pgd > \->returns (kvm->arch.pgd + stage2_pgd_index(addr); > \-> stage_pud_none(x) = 0 & stage2_pmd_offset(pud, addr) = pud > \-> returns pud (kvm->arch.pgd + stage2_pgd_index(addr)) > > and we install the PMD huge mapping at the location. > > >In any case, I think user_mem_abort() should give up on pmd/pud huge > >mappings if the size mapped by the stage2/stage1 pmd/pud levels don't > >line up. What do you think? > > Does it matter ? Personally I don't think it matters much as long as we > are able to map the "huge" page at stage1 in the stage2 as huge, even if > the stage2 has lesser levels and manage it well. Given that PMD huge > pages are quite common, it would be good to exploit it when we can. What I couldn't convince myself of was whether having 2 levels at stage2 implied the entry level block mapping being of the same size as the stage1 block mapping, but given your explanation above, I think that's fine. > > On the other hand, for stage2 PUD we are checking if the stage2 has a > PUD level (kvm_has_stage2_pud()). May be we should relax it just like > we do for PMD to check (kvm_stage2_levels > 2). > Depends on how the code ends up looking like I suppose, but the more symmetry we can have between the approach for host PMD and host PUD and host PTE mappings, the better. Thanks, Christoffer