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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 843CCC25B75 for ; Tue, 14 May 2024 18:00:41 +0000 (UTC) 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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=JgG7h0Dfk96OQ+ASFVTcIDSKfUvm3LkcX0EZ/aT9D9o=; b=GkXDjJuUPBaDnp PsnbgwGAFcOj3elOu/WyG+D0Im8TLmF9nc7xXQzu3tC/YPg8Fu1N82Qy8KMUolGCb/6y9iHiqRwSG ACmg6WpfYG22RxF+9i9w4uVp36ABhA4LWO7aHNRH+2nmIhQAgTn4UPSscl0yKtIoqOxJ6fa64X+5L 4MvE2M+8IF0c/15EkjQDgcMTvn0jEjlDrd4iExmcAN2ixNlORuSMzvkrdUqYtQIE3G9Im2xBgSLsk PIIgPnjTOhb9HoE8ts3l+d0B1n+ENem7AZfo7DlKEfFvUUA0RBnNZh9f7F4RIt8DUTzlxwZ2JzSGL ws3LtSYdpF1LR9jO6BwQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s6wRv-0000000Gl3I-31Ga; Tue, 14 May 2024 18:00:27 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s6wRs-0000000Gl1S-1Tkd for linux-arm-kernel@lists.infradead.org; Tue, 14 May 2024 18:00:25 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id A58C9611A8; Tue, 14 May 2024 18:00:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 235D4C2BD10; Tue, 14 May 2024 18:00:14 +0000 (UTC) Date: Tue, 14 May 2024 19:00:12 +0100 From: Catalin Marinas To: Steven Price Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, Suzuki K Poulose , Marc Zyngier , Will Deacon , James Morse , Oliver Upton , Zenghui Yu , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Joey Gouly , Alexandru Elisei , Christoffer Dall , Fuad Tabba , linux-coco@lists.linux.dev, Ganapatrao Kulkarni Subject: Re: [PATCH v2 09/14] arm64: Enable memory encrypt for Realms Message-ID: References: <20240412084213.1733764-1-steven.price@arm.com> <20240412084213.1733764-10-steven.price@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240412084213.1733764-10-steven.price@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240514_110024_548900_BA73A90A X-CRM114-Status: GOOD ( 24.74 ) 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 Fri, Apr 12, 2024 at 09:42:08AM +0100, Steven Price wrote: > static int change_page_range(pte_t *ptep, unsigned long addr, void *data) > @@ -41,6 +45,7 @@ static int change_page_range(pte_t *ptep, unsigned long addr, void *data) > pte = clear_pte_bit(pte, cdata->clear_mask); > pte = set_pte_bit(pte, cdata->set_mask); > > + /* TODO: Break before make for PROT_NS_SHARED updates */ > __set_pte(ptep, pte); > return 0; Oh, this TODO is problematic, not sure we can do it safely. There are some patches on the list to trap faults from other CPUs if they happen to access the page when broken but so far we pushed back as complex and at risk of getting the logic wrong. >From an architecture perspective, you are changing the output address and D8.16.1 requires a break-before-make sequence (FEAT_BBM doesn't help). So we either come up with a way to do BMM safely (stop_machine() maybe if it's not too expensive or some way to guarantee no accesses to this page while being changed) or we get the architecture clarified on the possible side-effects here ("unpredictable" doesn't help). > } > @@ -192,6 +197,43 @@ int set_direct_map_default_noflush(struct page *page) > PAGE_SIZE, change_page_range, &data); > } > > +static int __set_memory_encrypted(unsigned long addr, > + int numpages, > + bool encrypt) > +{ > + unsigned long set_prot = 0, clear_prot = 0; > + phys_addr_t start, end; > + > + if (!is_realm_world()) > + return 0; > + > + WARN_ON(!__is_lm_address(addr)); Just return from this function if it's not a linear map address. No point in corrupting other areas since __virt_to_phys() will get it wrong. > + start = __virt_to_phys(addr); > + end = start + numpages * PAGE_SIZE; > + > + if (encrypt) { > + clear_prot = PROT_NS_SHARED; > + set_memory_range_protected(start, end); > + } else { > + set_prot = PROT_NS_SHARED; > + set_memory_range_shared(start, end); > + } > + > + return __change_memory_common(addr, PAGE_SIZE * numpages, > + __pgprot(set_prot), > + __pgprot(clear_prot)); > +} Can someone summarise what the point of this protection bit is? The IPA memory is marked as protected/unprotected already via the RSI call and presumably the RMM disables/permits sharing with a non-secure hypervisor accordingly irrespective of which alias the realm guest has the linear mapping mapped to. What does it do with the top bit of the IPA? Is it that the RMM will prevent (via Stage 2) access if the IPA does not match the requested protection? IOW, it unmaps one or the other at Stage 2? Also, the linear map is not the only one that points to this IPA. What if this is a buffer mapped in user-space or remapped as non-cacheable (upgraded to cacheable via FWB) in the kernel, the code above does not (and cannot) change the user mappings. It needs some digging into dma_direct_alloc() as well, it uses a pgprot_decrypted() but that's not implemented by your patches. Not sure it helps, it looks like the remap path in this function does not have a dma_set_decrypted() call (or maybe I missed it). -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel