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=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL 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 896E9C4338F for ; Tue, 3 Aug 2021 10:55:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 71E0E60FC2 for ; Tue, 3 Aug 2021 10:55:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235440AbhHCKz0 (ORCPT ); Tue, 3 Aug 2021 06:55:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33474 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235456AbhHCKzU (ORCPT ); Tue, 3 Aug 2021 06:55:20 -0400 Received: from mail-oi1-x236.google.com (mail-oi1-x236.google.com [IPv6:2607:f8b0:4864:20::236]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E1C26C061798 for ; Tue, 3 Aug 2021 03:55:09 -0700 (PDT) Received: by mail-oi1-x236.google.com with SMTP id 21so27671283oin.8 for ; Tue, 03 Aug 2021 03:55:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+WSoxwt2C8CYjlH9xHjQ+oboKdUtA9Rzp9ra0MKS0sc=; b=wM20ZIpUC2FBSnnZrCMauN6hWl7/tiPmQyuM3X7RY2OiF4DcfWAA2K9WP/8KiiIEcb dQVeVU/X0ukr8213stUsNNy617xD/83+c4SlsE4XlD770TEq4lro5WA9FRV9o7wOs3Ef 8Uuki8kxrh6oIA2KUPMk9YsXcoYbI1HcYHjL4nGMntTWKirqZM+8LVJMxsFvaL5FQfr2 sckZkQOCMlJVPa/xj4T04ySWt7wqy1HKXBYhPOBrzbSy8+przMwH0dWrwWa1XkNRjD/2 LC7QS1RGoU0ZMwsBP+q19aKABLCS/wU/1xc+mOWMM5NinLyNOxClp0EdnNAX9TmlBpJ4 1dAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+WSoxwt2C8CYjlH9xHjQ+oboKdUtA9Rzp9ra0MKS0sc=; b=ZIUlsdMonhNpC1Aydd3z7TbvlMJ7GA/YCiyGWPnMi1Hu3x8PILhIkG36DRu45oTGKs Vk8RUE+24NL8gZ+jsWkLY+x2jaS6AQuz8OpxULp7OzspZi1cfRHZSMH2avNOKVmJ9va6 KzmfvNqHIxXH3DAVJnRXaMtvE0P1R3osX4PFX/fgH8pphlHRDUsG/CoHxtd7knqsiC3T KrgTKOikMi7EY3Z+6nTKztsi5m/sWEEX8MtzOe9sgQG6E6IBEXaIcqA4q8CeatNqte9g ZCFNwmpXtz2SJU6m9tm8GEAciv1qicBcvY6eGspdYIBZ+uhmow1m2gJuaaB2OxZwJjYT /byw== X-Gm-Message-State: AOAM532JhyYJHfJI/sf29P2uxkBYJdpUK4te6FxWBCIl7TgFtYCLMUtE 4wiGZbmOlsXErEsKWmHjdiOBmkQnkwRCotYSukkIXQ== X-Google-Smtp-Source: ABdhPJwYlFcjoKB1AsgH5I1o+peqrhsWTuZXg6FQYN4DxDCoFu6YtiZbcpIvRedwV2a/4OLXMpfMsVv2ddav3MvA0Os= X-Received: by 2002:a05:6808:1981:: with SMTP id bj1mr1110754oib.8.1627988109163; Tue, 03 Aug 2021 03:55:09 -0700 (PDT) MIME-Version: 1.0 References: <20210729132818.4091769-1-qperret@google.com> <20210729132818.4091769-18-qperret@google.com> In-Reply-To: From: Fuad Tabba Date: Tue, 3 Aug 2021 12:54:32 +0200 Message-ID: Subject: Re: [PATCH v3 17/21] KVM: arm64: Mark host bss and rodata section as shared To: Quentin Perret Cc: maz@kernel.org, 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, dbrazdil@google.com, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Quentin, > > > + ret = pkvm_create_mappings(__hyp_bss_end, __bss_stop, prot); > > > > nit: for clarity, I wonder if it might be good to create an alias of > > __hyp_bss_end as __bss_start or something. When it's been moved here, > > it sticks out a bit more and makes the reader wonder about the > > significance of __hyp_bss_end. > > I understand what you mean, but I'm not sure this aliasing is really > going to clarify things much. We have a comment in arm.c (see > init_hyp_mode()) to explain exactly why we're doing this, so maybe it > would be worth adding it here too. WDYT? Not sure to be honest. Comments are good, until they're stale, and replicating the comment increases the odds of that happening. No strong opinion either way. > > > + if (ret) > > > + return ret; > > > + > > > return 0; > > > } > > > > > > @@ -148,6 +159,57 @@ static void hpool_put_page(void *addr) > > > hyp_put_page(&hpool, addr); > > > } > > > > > > +static int finalize_host_mappings_walker(u64 addr, u64 end, u32 level, > > > + kvm_pte_t *ptep, > > > + enum kvm_pgtable_walk_flags flag, > > > + void * const arg) > > > +{ > > > + enum kvm_pgtable_prot prot; > > > + enum pkvm_page_state state; > > > + kvm_pte_t pte = *ptep; > > > + phys_addr_t phys; > > > + > > > + if (!kvm_pte_valid(pte)) > > > + return 0; > > > + > > > + if (level != (KVM_PGTABLE_MAX_LEVELS - 1)) > > > + return -EINVAL; > > > > I know that it's not in scope here, but I'm wondering whether we > > should be checking for KVM_PTE_TYPE_PAGE instead of the level. Maybe > > Well these would check different things no? > > > it would be good to have a helper somewhere for all these checks both > > for clarity and to ensure that nothing has gone wrong with the pte. > > The reason I need this check is just to make sure the call to > host_stage2_idmap_locked() further down is correct with a hardcoded > PAGE_SIZE size. The alternative would be to not be lazy and actually > compute the current granule size based on the level and use that, as > that would make this code robust to using block mappings at EL2 stage-1 > in the future. > > And I'll fix this up for v4. I get it now. Thanks! /fuad > Cheers, > Quentin > > > > + > > > + phys = kvm_pte_to_phys(pte); > > > + if (!addr_is_memory(phys)) > > > + return 0; > > > + > > > + /* > > > + * Adjust the host stage-2 mappings to match the ownership attributes > > > + * configured in the hypervisor stage-1. > > > + */ > > > + state = pkvm_getstate(kvm_pgtable_hyp_pte_prot(pte)); > > > + switch (state) { > > > + case PKVM_PAGE_OWNED: > > > + return host_stage2_set_owner_locked(phys, phys + PAGE_SIZE, pkvm_hyp_id); > > > + case PKVM_PAGE_SHARED_OWNED: > > > + prot = pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_BORROWED); > > > + break; > > > + case PKVM_PAGE_SHARED_BORROWED: > > > + prot = pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_OWNED); > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + return host_stage2_idmap_locked(phys, phys + PAGE_SIZE, prot); > > > +} > > > + > > > +static int finalize_host_mappings(void) > > > +{ > > > + struct kvm_pgtable_walker walker = { > > > + .cb = finalize_host_mappings_walker, > > > + .flags = KVM_PGTABLE_WALK_LEAF, > > > + }; > > > + > > > + return kvm_pgtable_walk(&pkvm_pgtable, 0, BIT(pkvm_pgtable.ia_bits), &walker); > > > +} > > > + > > > void __noreturn __pkvm_init_finalise(void) > > > { > > > struct kvm_host_data *host_data = this_cpu_ptr(&kvm_host_data); > > > @@ -167,6 +229,10 @@ void __noreturn __pkvm_init_finalise(void) > > > if (ret) > > > goto out; > > > > > > + ret = finalize_host_mappings(); > > > + if (ret) > > > + goto out; > > > + > > > pkvm_pgtable_mm_ops = (struct kvm_pgtable_mm_ops) { > > > .zalloc_page = hyp_zalloc_hyp_page, > > > .phys_to_virt = hyp_phys_to_virt, > > > -- > > > 2.32.0.432.gabb21c7263-goog > > > > > > > Thanks, > > /fuad 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.6 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 ED24EC4338F for ; Tue, 3 Aug 2021 10:55:13 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 6B31460FC2 for ; Tue, 3 Aug 2021 10:55:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6B31460FC2 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id CDE3040623; Tue, 3 Aug 2021 06:55:12 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@google.com 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 5MPhtKjh+F9W; Tue, 3 Aug 2021 06:55:11 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 9A17140870; Tue, 3 Aug 2021 06:55:11 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 1D91E4086D for ; Tue, 3 Aug 2021 06:55:11 -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 Y4JFRdvMe48E for ; Tue, 3 Aug 2021 06:55:09 -0400 (EDT) Received: from mail-oi1-f177.google.com (mail-oi1-f177.google.com [209.85.167.177]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id E3C8A40623 for ; Tue, 3 Aug 2021 06:55:09 -0400 (EDT) Received: by mail-oi1-f177.google.com with SMTP id y18so27693239oiv.3 for ; Tue, 03 Aug 2021 03:55:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+WSoxwt2C8CYjlH9xHjQ+oboKdUtA9Rzp9ra0MKS0sc=; b=wM20ZIpUC2FBSnnZrCMauN6hWl7/tiPmQyuM3X7RY2OiF4DcfWAA2K9WP/8KiiIEcb dQVeVU/X0ukr8213stUsNNy617xD/83+c4SlsE4XlD770TEq4lro5WA9FRV9o7wOs3Ef 8Uuki8kxrh6oIA2KUPMk9YsXcoYbI1HcYHjL4nGMntTWKirqZM+8LVJMxsFvaL5FQfr2 sckZkQOCMlJVPa/xj4T04ySWt7wqy1HKXBYhPOBrzbSy8+przMwH0dWrwWa1XkNRjD/2 LC7QS1RGoU0ZMwsBP+q19aKABLCS/wU/1xc+mOWMM5NinLyNOxClp0EdnNAX9TmlBpJ4 1dAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+WSoxwt2C8CYjlH9xHjQ+oboKdUtA9Rzp9ra0MKS0sc=; b=j7L7CW6RY+yF02iBBB3CJcHgY928kfc5byGa085J7ErcFT+tbAlFa6XK7UjbZ5oM8T U91Uu7o9HsPjxGggHxbn3oVwt2Sd97RZnPbANXOhr/PKxesQkDyAJXnjwqHdKZS2mZ66 zvrZhTjv9pNTsor+lRf917He4kyWKQjca9tgobZq33wp0vqwbXxNkBEj+XmGBfjmZlI4 T2uMuLvejvFQPkJxt4jJwVgxATxnJU9Hf19uvoXy8zYXL6LNP6TIRbcA/HFXtxOvRFi7 9HvDLrEtFdumIfcBG/JZdEWfndpJsI6idrvN6NTht4x1660GnmH0OESDEZAjY2VqdJX1 1Qfw== X-Gm-Message-State: AOAM532RensPwXRlJWA4XaEIJMxKmMRoeV1wJvCVdA2GyhdQY0RN0wYN MYbJiOfiFX29OBQwO7Pp8X+kXdZHmXZZhtP6tXUK3g== X-Google-Smtp-Source: ABdhPJwYlFcjoKB1AsgH5I1o+peqrhsWTuZXg6FQYN4DxDCoFu6YtiZbcpIvRedwV2a/4OLXMpfMsVv2ddav3MvA0Os= X-Received: by 2002:a05:6808:1981:: with SMTP id bj1mr1110754oib.8.1627988109163; Tue, 03 Aug 2021 03:55:09 -0700 (PDT) MIME-Version: 1.0 References: <20210729132818.4091769-1-qperret@google.com> <20210729132818.4091769-18-qperret@google.com> In-Reply-To: From: Fuad Tabba Date: Tue, 3 Aug 2021 12:54:32 +0200 Message-ID: Subject: Re: [PATCH v3 17/21] KVM: arm64: Mark host bss and rodata section as shared To: Quentin Perret Cc: kernel-team@android.com, qwandor@google.com, maz@kernel.org, linux-kernel@vger.kernel.org, catalin.marinas@arm.com, will@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 Hi Quentin, > > > + ret = pkvm_create_mappings(__hyp_bss_end, __bss_stop, prot); > > > > nit: for clarity, I wonder if it might be good to create an alias of > > __hyp_bss_end as __bss_start or something. When it's been moved here, > > it sticks out a bit more and makes the reader wonder about the > > significance of __hyp_bss_end. > > I understand what you mean, but I'm not sure this aliasing is really > going to clarify things much. We have a comment in arm.c (see > init_hyp_mode()) to explain exactly why we're doing this, so maybe it > would be worth adding it here too. WDYT? Not sure to be honest. Comments are good, until they're stale, and replicating the comment increases the odds of that happening. No strong opinion either way. > > > + if (ret) > > > + return ret; > > > + > > > return 0; > > > } > > > > > > @@ -148,6 +159,57 @@ static void hpool_put_page(void *addr) > > > hyp_put_page(&hpool, addr); > > > } > > > > > > +static int finalize_host_mappings_walker(u64 addr, u64 end, u32 level, > > > + kvm_pte_t *ptep, > > > + enum kvm_pgtable_walk_flags flag, > > > + void * const arg) > > > +{ > > > + enum kvm_pgtable_prot prot; > > > + enum pkvm_page_state state; > > > + kvm_pte_t pte = *ptep; > > > + phys_addr_t phys; > > > + > > > + if (!kvm_pte_valid(pte)) > > > + return 0; > > > + > > > + if (level != (KVM_PGTABLE_MAX_LEVELS - 1)) > > > + return -EINVAL; > > > > I know that it's not in scope here, but I'm wondering whether we > > should be checking for KVM_PTE_TYPE_PAGE instead of the level. Maybe > > Well these would check different things no? > > > it would be good to have a helper somewhere for all these checks both > > for clarity and to ensure that nothing has gone wrong with the pte. > > The reason I need this check is just to make sure the call to > host_stage2_idmap_locked() further down is correct with a hardcoded > PAGE_SIZE size. The alternative would be to not be lazy and actually > compute the current granule size based on the level and use that, as > that would make this code robust to using block mappings at EL2 stage-1 > in the future. > > And I'll fix this up for v4. I get it now. Thanks! /fuad > Cheers, > Quentin > > > > + > > > + phys = kvm_pte_to_phys(pte); > > > + if (!addr_is_memory(phys)) > > > + return 0; > > > + > > > + /* > > > + * Adjust the host stage-2 mappings to match the ownership attributes > > > + * configured in the hypervisor stage-1. > > > + */ > > > + state = pkvm_getstate(kvm_pgtable_hyp_pte_prot(pte)); > > > + switch (state) { > > > + case PKVM_PAGE_OWNED: > > > + return host_stage2_set_owner_locked(phys, phys + PAGE_SIZE, pkvm_hyp_id); > > > + case PKVM_PAGE_SHARED_OWNED: > > > + prot = pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_BORROWED); > > > + break; > > > + case PKVM_PAGE_SHARED_BORROWED: > > > + prot = pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_OWNED); > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + return host_stage2_idmap_locked(phys, phys + PAGE_SIZE, prot); > > > +} > > > + > > > +static int finalize_host_mappings(void) > > > +{ > > > + struct kvm_pgtable_walker walker = { > > > + .cb = finalize_host_mappings_walker, > > > + .flags = KVM_PGTABLE_WALK_LEAF, > > > + }; > > > + > > > + return kvm_pgtable_walk(&pkvm_pgtable, 0, BIT(pkvm_pgtable.ia_bits), &walker); > > > +} > > > + > > > void __noreturn __pkvm_init_finalise(void) > > > { > > > struct kvm_host_data *host_data = this_cpu_ptr(&kvm_host_data); > > > @@ -167,6 +229,10 @@ void __noreturn __pkvm_init_finalise(void) > > > if (ret) > > > goto out; > > > > > > + ret = finalize_host_mappings(); > > > + if (ret) > > > + goto out; > > > + > > > pkvm_pgtable_mm_ops = (struct kvm_pgtable_mm_ops) { > > > .zalloc_page = hyp_zalloc_hyp_page, > > > .phys_to_virt = hyp_phys_to_virt, > > > -- > > > 2.32.0.432.gabb21c7263-goog > > > > > > > Thanks, > > /fuad _______________________________________________ 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=-9.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, 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 972F3C4338F for ; Tue, 3 Aug 2021 10:57:04 +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 6267161037 for ; Tue, 3 Aug 2021 10:57:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6267161037 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=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:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=AasXbbn4v/4csbYwCnOQCWwac32t2IfGV4bY3A3rJRg=; b=t3VgYLBWXSgaRb Q+2+eLgI3axk4wbrS+REe/OR5em1w6jXDICMlmdwvkrMu5Q/Ch7ZyCffAUn6tHVIM4K8WSqQEbXsK ndgWSPcLm2QaSz09rgan1px0EQPmkfEaCQuw8eXS62ybZylkEo78az921Va80ntlsRoScByJmROVJ UhRE0J90etcCfGeZFqHt7UdH7LR13RY+RfR27cwiQOkRt9uRbdEkzXTDzHN+sUIhR22lVCExBQVsC DtyBKSgsdSe3GU0Ao23BG/ypcMNmAZ1+KU3CylN8z0jOIonm1hM061RVwbhkXAvU3jGPB089EKOb+ BpJjB8WhitrhWqoK0AUQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAs4p-002CAh-VR; Tue, 03 Aug 2021 10:55:16 +0000 Received: from mail-oi1-x22d.google.com ([2607:f8b0:4864:20::22d]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAs4k-002C90-IC for linux-arm-kernel@lists.infradead.org; Tue, 03 Aug 2021 10:55:12 +0000 Received: by mail-oi1-x22d.google.com with SMTP id u10so27660700oiw.4 for ; Tue, 03 Aug 2021 03:55:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+WSoxwt2C8CYjlH9xHjQ+oboKdUtA9Rzp9ra0MKS0sc=; b=wM20ZIpUC2FBSnnZrCMauN6hWl7/tiPmQyuM3X7RY2OiF4DcfWAA2K9WP/8KiiIEcb dQVeVU/X0ukr8213stUsNNy617xD/83+c4SlsE4XlD770TEq4lro5WA9FRV9o7wOs3Ef 8Uuki8kxrh6oIA2KUPMk9YsXcoYbI1HcYHjL4nGMntTWKirqZM+8LVJMxsFvaL5FQfr2 sckZkQOCMlJVPa/xj4T04ySWt7wqy1HKXBYhPOBrzbSy8+przMwH0dWrwWa1XkNRjD/2 LC7QS1RGoU0ZMwsBP+q19aKABLCS/wU/1xc+mOWMM5NinLyNOxClp0EdnNAX9TmlBpJ4 1dAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+WSoxwt2C8CYjlH9xHjQ+oboKdUtA9Rzp9ra0MKS0sc=; b=cCND789Dd9ycGFDt9GtZpps7r81y0oqQnLdJPPfb80Es/UhlSRg1J2nc8xzEAHULkA R/w6UbygVYq24QVHt4pUPmdFEHI5WXqrIYfbS59r2sus4V/q0DscJ3Wk0CZ5dx/pmb+H aUcBIda0lyqR5hKtJZ1EYHwM9NIsiZ6KHezPhN82tMXrBcMBE3+tMnM5GSUqhtGnjKSM moc8xbUz2eCoXFv1EaYsNsGWjRM0Je1vki87mdwLN1GisFlmXEhzHo1GHXgNWnTuqCij pS984Vwtek830rJP3xSPvV7pQsAjLMZdRxaLiBndyKN1h28S0X76XmEqhdPQMx8+7h2m Y+oQ== X-Gm-Message-State: AOAM5305mACqTyUxaWSTqTu8d/5tYYVDHw68F0TNCho/rbgDZchst4Et +Xc6bTxNwBOVTNo5njRlOw9WB5R5bY0TNBh9t76+tQ== X-Google-Smtp-Source: ABdhPJwYlFcjoKB1AsgH5I1o+peqrhsWTuZXg6FQYN4DxDCoFu6YtiZbcpIvRedwV2a/4OLXMpfMsVv2ddav3MvA0Os= X-Received: by 2002:a05:6808:1981:: with SMTP id bj1mr1110754oib.8.1627988109163; Tue, 03 Aug 2021 03:55:09 -0700 (PDT) MIME-Version: 1.0 References: <20210729132818.4091769-1-qperret@google.com> <20210729132818.4091769-18-qperret@google.com> In-Reply-To: From: Fuad Tabba Date: Tue, 3 Aug 2021 12:54:32 +0200 Message-ID: Subject: Re: [PATCH v3 17/21] KVM: arm64: Mark host bss and rodata section as shared To: Quentin Perret Cc: maz@kernel.org, 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, dbrazdil@google.com, kernel-team@android.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210803_035510_689204_357E586E X-CRM114-Status: GOOD ( 36.11 ) 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 Hi Quentin, > > > + ret = pkvm_create_mappings(__hyp_bss_end, __bss_stop, prot); > > > > nit: for clarity, I wonder if it might be good to create an alias of > > __hyp_bss_end as __bss_start or something. When it's been moved here, > > it sticks out a bit more and makes the reader wonder about the > > significance of __hyp_bss_end. > > I understand what you mean, but I'm not sure this aliasing is really > going to clarify things much. We have a comment in arm.c (see > init_hyp_mode()) to explain exactly why we're doing this, so maybe it > would be worth adding it here too. WDYT? Not sure to be honest. Comments are good, until they're stale, and replicating the comment increases the odds of that happening. No strong opinion either way. > > > + if (ret) > > > + return ret; > > > + > > > return 0; > > > } > > > > > > @@ -148,6 +159,57 @@ static void hpool_put_page(void *addr) > > > hyp_put_page(&hpool, addr); > > > } > > > > > > +static int finalize_host_mappings_walker(u64 addr, u64 end, u32 level, > > > + kvm_pte_t *ptep, > > > + enum kvm_pgtable_walk_flags flag, > > > + void * const arg) > > > +{ > > > + enum kvm_pgtable_prot prot; > > > + enum pkvm_page_state state; > > > + kvm_pte_t pte = *ptep; > > > + phys_addr_t phys; > > > + > > > + if (!kvm_pte_valid(pte)) > > > + return 0; > > > + > > > + if (level != (KVM_PGTABLE_MAX_LEVELS - 1)) > > > + return -EINVAL; > > > > I know that it's not in scope here, but I'm wondering whether we > > should be checking for KVM_PTE_TYPE_PAGE instead of the level. Maybe > > Well these would check different things no? > > > it would be good to have a helper somewhere for all these checks both > > for clarity and to ensure that nothing has gone wrong with the pte. > > The reason I need this check is just to make sure the call to > host_stage2_idmap_locked() further down is correct with a hardcoded > PAGE_SIZE size. The alternative would be to not be lazy and actually > compute the current granule size based on the level and use that, as > that would make this code robust to using block mappings at EL2 stage-1 > in the future. > > And I'll fix this up for v4. I get it now. Thanks! /fuad > Cheers, > Quentin > > > > + > > > + phys = kvm_pte_to_phys(pte); > > > + if (!addr_is_memory(phys)) > > > + return 0; > > > + > > > + /* > > > + * Adjust the host stage-2 mappings to match the ownership attributes > > > + * configured in the hypervisor stage-1. > > > + */ > > > + state = pkvm_getstate(kvm_pgtable_hyp_pte_prot(pte)); > > > + switch (state) { > > > + case PKVM_PAGE_OWNED: > > > + return host_stage2_set_owner_locked(phys, phys + PAGE_SIZE, pkvm_hyp_id); > > > + case PKVM_PAGE_SHARED_OWNED: > > > + prot = pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_BORROWED); > > > + break; > > > + case PKVM_PAGE_SHARED_BORROWED: > > > + prot = pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_OWNED); > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + return host_stage2_idmap_locked(phys, phys + PAGE_SIZE, prot); > > > +} > > > + > > > +static int finalize_host_mappings(void) > > > +{ > > > + struct kvm_pgtable_walker walker = { > > > + .cb = finalize_host_mappings_walker, > > > + .flags = KVM_PGTABLE_WALK_LEAF, > > > + }; > > > + > > > + return kvm_pgtable_walk(&pkvm_pgtable, 0, BIT(pkvm_pgtable.ia_bits), &walker); > > > +} > > > + > > > void __noreturn __pkvm_init_finalise(void) > > > { > > > struct kvm_host_data *host_data = this_cpu_ptr(&kvm_host_data); > > > @@ -167,6 +229,10 @@ void __noreturn __pkvm_init_finalise(void) > > > if (ret) > > > goto out; > > > > > > + ret = finalize_host_mappings(); > > > + if (ret) > > > + goto out; > > > + > > > pkvm_pgtable_mm_ops = (struct kvm_pgtable_mm_ops) { > > > .zalloc_page = hyp_zalloc_hyp_page, > > > .phys_to_virt = hyp_phys_to_virt, > > > -- > > > 2.32.0.432.gabb21c7263-goog > > > > > > > Thanks, > > /fuad _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel