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=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 DBA5CC07E95 for ; Tue, 20 Jul 2021 10:13:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C13D5610D2 for ; Tue, 20 Jul 2021 10:13:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231809AbhGTJdK (ORCPT ); Tue, 20 Jul 2021 05:33:10 -0400 Received: from mail.kernel.org ([198.145.29.99]:56906 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233914AbhGTJc7 (ORCPT ); Tue, 20 Jul 2021 05:32:59 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (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 8B0D0610C7; Tue, 20 Jul 2021 10:13:33 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1m5mkl-00EU1u-Gx; Tue, 20 Jul 2021 11:13:31 +0100 Date: Tue, 20 Jul 2021 11:13:31 +0100 Message-ID: <8735s99ttg.wl-maz@kernel.org> From: Marc Zyngier To: Quentin Perret Cc: james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, tabba@google.com, dbrazdil@google.com, kernel-team@android.com Subject: Re: [PATCH 08/14] KVM: arm64: Add support for tagging shared pages in page-table In-Reply-To: References: <20210719104735.3681732-1-qperret@google.com> <20210719104735.3681732-9-qperret@google.com> <87fswajre1.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: qperret@google.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, tabba@google.com, dbrazdil@google.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 19 Jul 2021 16:49:13 +0100, Quentin Perret wrote: > > On Monday 19 Jul 2021 at 15:43:34 (+0100), Marc Zyngier wrote: > > On Mon, 19 Jul 2021 11:47:29 +0100, > > Quentin Perret wrote: > > > > > > The hypervisor will soon be in charge of tracking ownership of all > > > memory pages in the system. The current page-tracking infrastructure at > > > EL2 only allows binary states: a page is either owned or not by an > > > entity. But a number of use-cases will require more complex states for > > > pages that are shared between two entities (host, hypervisor, or guests). > > > > > > In preparation for supporting these use-cases, introduce in the KVM > > > page-table library some infrastructure allowing to tag shared pages > > > using ignored bits (a.k.a. software bits) in PTEs. > > > > > > Signed-off-by: Quentin Perret > > > --- > > > arch/arm64/include/asm/kvm_pgtable.h | 5 +++++ > > > arch/arm64/kvm/hyp/pgtable.c | 25 +++++++++++++++++++++++++ > > > 2 files changed, 30 insertions(+) > > > > > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h > > > index dd72653314c7..f6d3d5c8910d 100644 > > > --- a/arch/arm64/include/asm/kvm_pgtable.h > > > +++ b/arch/arm64/include/asm/kvm_pgtable.h > > > @@ -81,6 +81,8 @@ enum kvm_pgtable_stage2_flags { > > > * @KVM_PGTABLE_PROT_W: Write permission. > > > * @KVM_PGTABLE_PROT_R: Read permission. > > > * @KVM_PGTABLE_PROT_DEVICE: Device attributes. > > > + * @KVM_PGTABLE_STATE_SHARED: Page shared with another entity. > > > + * @KVM_PGTABLE_STATE_BORROWED: Page borrowed from another entity. > > > */ > > > enum kvm_pgtable_prot { > > > KVM_PGTABLE_PROT_X = BIT(0), > > > @@ -88,6 +90,9 @@ enum kvm_pgtable_prot { > > > KVM_PGTABLE_PROT_R = BIT(2), > > > > > > KVM_PGTABLE_PROT_DEVICE = BIT(3), > > > + > > > + KVM_PGTABLE_STATE_SHARED = BIT(4), > > > + KVM_PGTABLE_STATE_BORROWED = BIT(5), > > > > I'd rather have some indirection here, as we have other potential > > users for the SW bits outside of pKVM (see the NV series, which uses > > some of these SW bits as the backend for TTL-based TLB invalidation). > > > > Can we instead only describe the SW bit states in this enum, and let > > the users map the semantic they require onto that state? See [1] for > > what I carry in the NV branch. > > Works for me -- I just wanted to make sure we don't have users in > different places that use the same bits without knowing, but no strong > opinions, so happy to change. > > > > }; > > > > > > #define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W) > > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c > > > index 5bdbe7a31551..51598b79dafc 100644 > > > --- a/arch/arm64/kvm/hyp/pgtable.c > > > +++ b/arch/arm64/kvm/hyp/pgtable.c > > > @@ -211,6 +211,29 @@ static kvm_pte_t kvm_init_invalid_leaf_owner(u8 owner_id) > > > return FIELD_PREP(KVM_INVALID_PTE_OWNER_MASK, owner_id); > > > } > > > > > > +static kvm_pte_t pte_ignored_bit_prot(enum kvm_pgtable_prot prot) > > > > Can we call these sw rather than ignored? > > Sure. > > > > +{ > > > + kvm_pte_t ignored_bits = 0; > > > + > > > + /* > > > + * Ignored bits 0 and 1 are reserved to track the memory ownership > > > + * state of each page: > > > + * 00: The page is owned solely by the page-table owner. > > > + * 01: The page is owned by the page-table owner, but is shared > > > + * with another entity. > > > + * 10: The page is shared with, but not owned by the page-table owner. > > > + * 11: Reserved for future use (lending). > > > + */ > > > + if (prot & KVM_PGTABLE_STATE_SHARED) { > > > + if (prot & KVM_PGTABLE_STATE_BORROWED) > > > + ignored_bits |= BIT(1); > > > + else > > > + ignored_bits |= BIT(0); > > > + } > > > + > > > + return FIELD_PREP(KVM_PTE_LEAF_ATTR_IGNORED, ignored_bits); > > > +} > > > + > > > static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data, u64 addr, > > > u32 level, kvm_pte_t *ptep, > > > enum kvm_pgtable_walk_flags flag) > > > @@ -357,6 +380,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep) > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_AP, ap); > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_SH, sh); > > > attr |= KVM_PTE_LEAF_ATTR_LO_S1_AF; > > > + attr |= pte_ignored_bit_prot(prot); > > > *ptep = attr; > > > > > > return 0; > > > @@ -558,6 +582,7 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p > > > > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh); > > > attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF; > > > + attr |= pte_ignored_bit_prot(prot); > > > *ptep = attr; > > > > > > return 0; > > > > How about kvm_pgtable_stage2_relax_perms()? > > It should leave SW bits untouched, and it really felt like a path were > we want to change permissions and nothing else. What did you have in > mind? It isn't clear to me that it would not (cannot?) be used to change other bits, given that it takes an arbitrary 'prot' set. If there is such an intended restriction, we definitely should document it. M. -- Without deviation from the norm, progress is not possible. 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=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 A2886C07E95 for ; Tue, 20 Jul 2021 10:13:41 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 18D59610D2 for ; Tue, 20 Jul 2021 10:13:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 18D59610D2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id A019F4B098; Tue, 20 Jul 2021 06:13:40 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DToZd0+8UCuE; Tue, 20 Jul 2021 06:13:38 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 9BD8C4B0B1; Tue, 20 Jul 2021 06:13:38 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 7A1CE4A51D for ; Tue, 20 Jul 2021 06:13:37 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id b37wrCb5QAm1 for ; Tue, 20 Jul 2021 06:13:34 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id A70624A1AF for ; Tue, 20 Jul 2021 06:13:34 -0400 (EDT) Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (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 8B0D0610C7; Tue, 20 Jul 2021 10:13:33 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1m5mkl-00EU1u-Gx; Tue, 20 Jul 2021 11:13:31 +0100 Date: Tue, 20 Jul 2021 11:13:31 +0100 Message-ID: <8735s99ttg.wl-maz@kernel.org> From: Marc Zyngier To: Quentin Perret Subject: Re: [PATCH 08/14] KVM: arm64: Add support for tagging shared pages in page-table In-Reply-To: References: <20210719104735.3681732-1-qperret@google.com> <20210719104735.3681732-9-qperret@google.com> <87fswajre1.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: qperret@google.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, tabba@google.com, dbrazdil@google.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: kernel-team@android.com, qwandor@google.com, will@kernel.org, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Mon, 19 Jul 2021 16:49:13 +0100, Quentin Perret wrote: > > On Monday 19 Jul 2021 at 15:43:34 (+0100), Marc Zyngier wrote: > > On Mon, 19 Jul 2021 11:47:29 +0100, > > Quentin Perret wrote: > > > > > > The hypervisor will soon be in charge of tracking ownership of all > > > memory pages in the system. The current page-tracking infrastructure at > > > EL2 only allows binary states: a page is either owned or not by an > > > entity. But a number of use-cases will require more complex states for > > > pages that are shared between two entities (host, hypervisor, or guests). > > > > > > In preparation for supporting these use-cases, introduce in the KVM > > > page-table library some infrastructure allowing to tag shared pages > > > using ignored bits (a.k.a. software bits) in PTEs. > > > > > > Signed-off-by: Quentin Perret > > > --- > > > arch/arm64/include/asm/kvm_pgtable.h | 5 +++++ > > > arch/arm64/kvm/hyp/pgtable.c | 25 +++++++++++++++++++++++++ > > > 2 files changed, 30 insertions(+) > > > > > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h > > > index dd72653314c7..f6d3d5c8910d 100644 > > > --- a/arch/arm64/include/asm/kvm_pgtable.h > > > +++ b/arch/arm64/include/asm/kvm_pgtable.h > > > @@ -81,6 +81,8 @@ enum kvm_pgtable_stage2_flags { > > > * @KVM_PGTABLE_PROT_W: Write permission. > > > * @KVM_PGTABLE_PROT_R: Read permission. > > > * @KVM_PGTABLE_PROT_DEVICE: Device attributes. > > > + * @KVM_PGTABLE_STATE_SHARED: Page shared with another entity. > > > + * @KVM_PGTABLE_STATE_BORROWED: Page borrowed from another entity. > > > */ > > > enum kvm_pgtable_prot { > > > KVM_PGTABLE_PROT_X = BIT(0), > > > @@ -88,6 +90,9 @@ enum kvm_pgtable_prot { > > > KVM_PGTABLE_PROT_R = BIT(2), > > > > > > KVM_PGTABLE_PROT_DEVICE = BIT(3), > > > + > > > + KVM_PGTABLE_STATE_SHARED = BIT(4), > > > + KVM_PGTABLE_STATE_BORROWED = BIT(5), > > > > I'd rather have some indirection here, as we have other potential > > users for the SW bits outside of pKVM (see the NV series, which uses > > some of these SW bits as the backend for TTL-based TLB invalidation). > > > > Can we instead only describe the SW bit states in this enum, and let > > the users map the semantic they require onto that state? See [1] for > > what I carry in the NV branch. > > Works for me -- I just wanted to make sure we don't have users in > different places that use the same bits without knowing, but no strong > opinions, so happy to change. > > > > }; > > > > > > #define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W) > > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c > > > index 5bdbe7a31551..51598b79dafc 100644 > > > --- a/arch/arm64/kvm/hyp/pgtable.c > > > +++ b/arch/arm64/kvm/hyp/pgtable.c > > > @@ -211,6 +211,29 @@ static kvm_pte_t kvm_init_invalid_leaf_owner(u8 owner_id) > > > return FIELD_PREP(KVM_INVALID_PTE_OWNER_MASK, owner_id); > > > } > > > > > > +static kvm_pte_t pte_ignored_bit_prot(enum kvm_pgtable_prot prot) > > > > Can we call these sw rather than ignored? > > Sure. > > > > +{ > > > + kvm_pte_t ignored_bits = 0; > > > + > > > + /* > > > + * Ignored bits 0 and 1 are reserved to track the memory ownership > > > + * state of each page: > > > + * 00: The page is owned solely by the page-table owner. > > > + * 01: The page is owned by the page-table owner, but is shared > > > + * with another entity. > > > + * 10: The page is shared with, but not owned by the page-table owner. > > > + * 11: Reserved for future use (lending). > > > + */ > > > + if (prot & KVM_PGTABLE_STATE_SHARED) { > > > + if (prot & KVM_PGTABLE_STATE_BORROWED) > > > + ignored_bits |= BIT(1); > > > + else > > > + ignored_bits |= BIT(0); > > > + } > > > + > > > + return FIELD_PREP(KVM_PTE_LEAF_ATTR_IGNORED, ignored_bits); > > > +} > > > + > > > static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data, u64 addr, > > > u32 level, kvm_pte_t *ptep, > > > enum kvm_pgtable_walk_flags flag) > > > @@ -357,6 +380,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep) > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_AP, ap); > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_SH, sh); > > > attr |= KVM_PTE_LEAF_ATTR_LO_S1_AF; > > > + attr |= pte_ignored_bit_prot(prot); > > > *ptep = attr; > > > > > > return 0; > > > @@ -558,6 +582,7 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p > > > > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh); > > > attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF; > > > + attr |= pte_ignored_bit_prot(prot); > > > *ptep = attr; > > > > > > return 0; > > > > How about kvm_pgtable_stage2_relax_perms()? > > It should leave SW bits untouched, and it really felt like a path were > we want to change permissions and nothing else. What did you have in > mind? It isn't clear to me that it would not (cannot?) be used to change other bits, given that it takes an arbitrary 'prot' set. If there is such an intended restriction, we definitely should document it. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm 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=-15.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 9D368C07E95 for ; Tue, 20 Jul 2021 10:15:13 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 6A61D610C7 for ; Tue, 20 Jul 2021 10:15:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6A61D610C7 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=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Rs+/IjwmsX18PLE8tFBXKtA6lz1usxpumzk12Qv4Vnc=; b=2ysBYVTygjerYQ owmQKSlFz1ufOisPrF3rs/PcFkaQtTqpm4kA1n1522z9k3FWzieECXI22sR4N8R4YPADA1D7ecyeS WZjWd+FbNAyoMWaBsZF4xR45DHG1IlQ/XJiViFIY/JCoPq2lKBtNoxxi7rKxftYk/ovpA4JdKTy++ Y+PQBFMQOaRh6FxlwdsvTLbnO2YUNRLkZt0GqVyMvRJkTZKSmGBuFd19h08HcKN6z8d0mewR2xgfO QKdKYGJ3j6vPDJLKhQMjGttiyQhymAUOfbKsR2TI0yEJqCEM1Lz5z6hlH8ConfhF45QEIqXhVnSjn 1JqYXVjGMANQdxBSDlWw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m5mkr-00CUDH-Tn; Tue, 20 Jul 2021 10:13:38 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m5mkn-00CUCl-TU for linux-arm-kernel@lists.infradead.org; Tue, 20 Jul 2021 10:13:35 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (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 8B0D0610C7; Tue, 20 Jul 2021 10:13:33 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1m5mkl-00EU1u-Gx; Tue, 20 Jul 2021 11:13:31 +0100 Date: Tue, 20 Jul 2021 11:13:31 +0100 Message-ID: <8735s99ttg.wl-maz@kernel.org> From: Marc Zyngier To: Quentin Perret Cc: james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, tabba@google.com, dbrazdil@google.com, kernel-team@android.com Subject: Re: [PATCH 08/14] KVM: arm64: Add support for tagging shared pages in page-table In-Reply-To: References: <20210719104735.3681732-1-qperret@google.com> <20210719104735.3681732-9-qperret@google.com> <87fswajre1.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: qperret@google.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, tabba@google.com, dbrazdil@google.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210720_031334_059265_8A90A128 X-CRM114-Status: GOOD ( 43.31 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 On Mon, 19 Jul 2021 16:49:13 +0100, Quentin Perret wrote: > > On Monday 19 Jul 2021 at 15:43:34 (+0100), Marc Zyngier wrote: > > On Mon, 19 Jul 2021 11:47:29 +0100, > > Quentin Perret wrote: > > > > > > The hypervisor will soon be in charge of tracking ownership of all > > > memory pages in the system. The current page-tracking infrastructure at > > > EL2 only allows binary states: a page is either owned or not by an > > > entity. But a number of use-cases will require more complex states for > > > pages that are shared between two entities (host, hypervisor, or guests). > > > > > > In preparation for supporting these use-cases, introduce in the KVM > > > page-table library some infrastructure allowing to tag shared pages > > > using ignored bits (a.k.a. software bits) in PTEs. > > > > > > Signed-off-by: Quentin Perret > > > --- > > > arch/arm64/include/asm/kvm_pgtable.h | 5 +++++ > > > arch/arm64/kvm/hyp/pgtable.c | 25 +++++++++++++++++++++++++ > > > 2 files changed, 30 insertions(+) > > > > > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h > > > index dd72653314c7..f6d3d5c8910d 100644 > > > --- a/arch/arm64/include/asm/kvm_pgtable.h > > > +++ b/arch/arm64/include/asm/kvm_pgtable.h > > > @@ -81,6 +81,8 @@ enum kvm_pgtable_stage2_flags { > > > * @KVM_PGTABLE_PROT_W: Write permission. > > > * @KVM_PGTABLE_PROT_R: Read permission. > > > * @KVM_PGTABLE_PROT_DEVICE: Device attributes. > > > + * @KVM_PGTABLE_STATE_SHARED: Page shared with another entity. > > > + * @KVM_PGTABLE_STATE_BORROWED: Page borrowed from another entity. > > > */ > > > enum kvm_pgtable_prot { > > > KVM_PGTABLE_PROT_X = BIT(0), > > > @@ -88,6 +90,9 @@ enum kvm_pgtable_prot { > > > KVM_PGTABLE_PROT_R = BIT(2), > > > > > > KVM_PGTABLE_PROT_DEVICE = BIT(3), > > > + > > > + KVM_PGTABLE_STATE_SHARED = BIT(4), > > > + KVM_PGTABLE_STATE_BORROWED = BIT(5), > > > > I'd rather have some indirection here, as we have other potential > > users for the SW bits outside of pKVM (see the NV series, which uses > > some of these SW bits as the backend for TTL-based TLB invalidation). > > > > Can we instead only describe the SW bit states in this enum, and let > > the users map the semantic they require onto that state? See [1] for > > what I carry in the NV branch. > > Works for me -- I just wanted to make sure we don't have users in > different places that use the same bits without knowing, but no strong > opinions, so happy to change. > > > > }; > > > > > > #define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W) > > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c > > > index 5bdbe7a31551..51598b79dafc 100644 > > > --- a/arch/arm64/kvm/hyp/pgtable.c > > > +++ b/arch/arm64/kvm/hyp/pgtable.c > > > @@ -211,6 +211,29 @@ static kvm_pte_t kvm_init_invalid_leaf_owner(u8 owner_id) > > > return FIELD_PREP(KVM_INVALID_PTE_OWNER_MASK, owner_id); > > > } > > > > > > +static kvm_pte_t pte_ignored_bit_prot(enum kvm_pgtable_prot prot) > > > > Can we call these sw rather than ignored? > > Sure. > > > > +{ > > > + kvm_pte_t ignored_bits = 0; > > > + > > > + /* > > > + * Ignored bits 0 and 1 are reserved to track the memory ownership > > > + * state of each page: > > > + * 00: The page is owned solely by the page-table owner. > > > + * 01: The page is owned by the page-table owner, but is shared > > > + * with another entity. > > > + * 10: The page is shared with, but not owned by the page-table owner. > > > + * 11: Reserved for future use (lending). > > > + */ > > > + if (prot & KVM_PGTABLE_STATE_SHARED) { > > > + if (prot & KVM_PGTABLE_STATE_BORROWED) > > > + ignored_bits |= BIT(1); > > > + else > > > + ignored_bits |= BIT(0); > > > + } > > > + > > > + return FIELD_PREP(KVM_PTE_LEAF_ATTR_IGNORED, ignored_bits); > > > +} > > > + > > > static int kvm_pgtable_visitor_cb(struct kvm_pgtable_walk_data *data, u64 addr, > > > u32 level, kvm_pte_t *ptep, > > > enum kvm_pgtable_walk_flags flag) > > > @@ -357,6 +380,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep) > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_AP, ap); > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_SH, sh); > > > attr |= KVM_PTE_LEAF_ATTR_LO_S1_AF; > > > + attr |= pte_ignored_bit_prot(prot); > > > *ptep = attr; > > > > > > return 0; > > > @@ -558,6 +582,7 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p > > > > > > attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh); > > > attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF; > > > + attr |= pte_ignored_bit_prot(prot); > > > *ptep = attr; > > > > > > return 0; > > > > How about kvm_pgtable_stage2_relax_perms()? > > It should leave SW bits untouched, and it really felt like a path were > we want to change permissions and nothing else. What did you have in > mind? It isn't clear to me that it would not (cannot?) be used to change other bits, given that it takes an arbitrary 'prot' set. If there is such an intended restriction, we definitely should document it. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel