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=-8.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 F0D05C433E2 for ; Wed, 2 Sep 2020 11:04:07 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B993E2078E for ; Wed, 2 Sep 2020 11:04:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="X3S/iJRJ"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="n/YQSeRG" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B993E2078E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=gLEQFnEQXiRYzCiE1uETkQPIfvezwh6+xNVZvXkWAJ4=; b=X3S/iJRJIQ4PloSTb4CcgrORC UMRsBUI21bgjin2C0bK7/AyiU/6owc1TFmTNFnIBWkzA6MXRBn3gIAnUh2dzEuXdKm3igTND/GrMw ybqvMInc0Q7qeLPg9pw0qVblUMkZgRE0UbTEB9Ah8I5lu1IIbe9fV2wWtQ8pQKhYAMUkI3C8D1Xls Fk73+9lhrIAIW86goG2oqPooqLhowos7Tg5Y1eMrX9wZZ0EcP0MMq91qJ99Cojx3mKv5YJU8K72Cy w3LHvIFcsgJkwIMbisPhPQxVh9UaA69dYiKUwO3THNLJuzoxvUf13TjXXEsVLS+TxcMhu7BmldBbs xrVT3nwEA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kDQXJ-0000gZ-Ar; Wed, 02 Sep 2020 11:02:41 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kDQXH-0000gB-1X for linux-arm-kernel@lists.infradead.org; Wed, 02 Sep 2020 11:02:39 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id A63B82078E; Wed, 2 Sep 2020 11:02:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599044558; bh=rqUKktWH8aA1Aa5o8UuWV/lNmyL+HZCoUqv8egIn3zE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=n/YQSeRG/iZl1orn8jPuU6CmblgXr070xtfqBUGKBu7wrZG5ChM4nwdd2dwELZ92W NTLQTynfWC+N5O7UCW+x0VPne9dEVgqhRmnQ8oHltEUo7WPuZtAIbRXr8C2sQSp3Gi vDH49s4Sl/cVdyQLHISs8KcUcbTByXCYNO6jzlsE= Date: Wed, 2 Sep 2020 12:02:33 +0100 From: Will Deacon To: Gavin Shan Subject: Re: [PATCH v3 02/21] KVM: arm64: Add stand-alone page-table walker infrastructure Message-ID: <20200902110233.GE5567@willie-the-truck> References: <20200825093953.26493-1-will@kernel.org> <20200825093953.26493-3-will@kernel.org> <5ca35d7e-79c2-4e07-962c-cc2eb62f1acb@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5ca35d7e-79c2-4e07-962c-cc2eb62f1acb@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200902_070239_216730_5CE5B0AD X-CRM114-Status: GOOD ( 33.04 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Suzuki Poulose , Marc Zyngier , Quentin Perret , James Morse , Catalin Marinas , kernel-team@android.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Gavin, On Wed, Sep 02, 2020 at 04:31:32PM +1000, Gavin Shan wrote: > On 8/25/20 7:39 PM, Will Deacon wrote: > > The KVM page-table code is intricately tied into the kernel page-table > > code and re-uses the pte/pmd/pud/p4d/pgd macros directly in an attempt > > to reduce code duplication. Unfortunately, the reality is that there is > > an awful lot of code required to make this work, and at the end of the > > day you're limited to creating page-tables with the same configuration > > as the host kernel. Furthermore, lifting the page-table code to run > > directly at EL2 on a non-VHE system (as we plan to to do in future > > patches) is practically impossible due to the number of dependencies it > > has on the core kernel. > > > > Introduce a framework for walking Armv8 page-tables configured > > independently from the host kernel. > > > > Cc: Marc Zyngier > > Cc: Quentin Perret > > Signed-off-by: Will Deacon > > --- > > arch/arm64/include/asm/kvm_pgtable.h | 101 ++++++++++ > > arch/arm64/kvm/hyp/Makefile | 2 +- > > arch/arm64/kvm/hyp/pgtable.c | 290 +++++++++++++++++++++++++++ > > 3 files changed, 392 insertions(+), 1 deletion(-) > > create mode 100644 arch/arm64/include/asm/kvm_pgtable.h > > create mode 100644 arch/arm64/kvm/hyp/pgtable.c [...] > > +struct kvm_pgtable_walk_data { > > + struct kvm_pgtable *pgt; > > + struct kvm_pgtable_walker *walker; > > + > > + u64 addr; > > + u64 end; > > +}; > > + > > Some of the following function might be worthy to be inlined, considering > their complexity :) I'll leave that for the compiler to figure out :) > > +static u32 kvm_pgd_pages(u32 ia_bits, u32 start_level) > > +{ > > + struct kvm_pgtable pgt = { > > + .ia_bits = ia_bits, > > + .start_level = start_level, > > + }; > > + > > + return __kvm_pgd_page_idx(&pgt, -1ULL) + 1; > > +} > > + > > It seems @pgt.start_level is assigned with wrong value here. > For example, @start_level is 2 when @ia_bits and PAGE_SIZE > are 40 and 64KB separately. In this case, __kvm_pgd_page_idx() > always return zero. However, the extra page covers up the > issue. I think something like below might be needed: > > struct kvm_pgtable pgt = { > .ia_bits = ia_bits, > .start_level = KVM_PGTABLE_MAX_LEVELS - start_level + 1, > }; Hmm, we're pulling the start_level right out of the vtcr, so I don't see how it can be wrong. In your example, a start_level of 2 seems correct to me, as we'll translate 13 bits there, then 13 bits at level 3 which covers the 24 bits you need (with a 16-bit offset within the page). Your suggestion would give us a start_level of 1, which has a redundant level of translation. Maybe you're looking at the levels upside-down? The top level is level 0 and each time you walk to a new level, that number increases. But perhaps I'm missing something. Please could you elaborate if you think there's a problem here? > > +static int _kvm_pgtable_walk(struct kvm_pgtable_walk_data *data) > > +{ > > + u32 idx; > > + int ret = 0; > > + struct kvm_pgtable *pgt = data->pgt; > > + u64 limit = BIT(pgt->ia_bits); > > + > > + if (data->addr > limit || data->end > limit) > > + return -ERANGE; > > + > > + if (!pgt->pgd) > > + return -EINVAL; > > + > > + for (idx = kvm_pgd_page_idx(data); data->addr < data->end; ++idx) { > > + kvm_pte_t *ptep = &pgt->pgd[idx * PTRS_PER_PTE]; > > + > > + ret = __kvm_pgtable_walk(data, ptep, pgt->start_level); > > + if (ret) > > + break; > > + } > > + > > + return ret; > > +} > > + > > I guess we need bail on the following condition: > > if (data->addr >= limit || data->end >= limit) > return -ERANGE; What's wrong with the existing check? In particular, I think we _want_ to support data->end == limit (it's exclusive). If data->addr == limit, then we'll have a size of zero and the loop won't run. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel