From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 5735D166F21; Mon, 19 Aug 2024 14:10:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724076623; cv=none; b=GF4kwj/49EvC6wGtBFJA4Z036u4WzC+iybBkALBHsqysle1g1SLE3E0JaOTNyP64/64QZB6TfeYgc0NzOxgnId+Sm3kiiVlKfoFC7J+YZvqaUpWQNBeFYdiW0hgCLGQaOEITxs7TQzMzrvLR/J0/t+uGD9OKuszkhRIzIQu4ABk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724076623; c=relaxed/simple; bh=Jgcmn7cNSHC9O5Fypdkuo7mZ5zRoudMi01BnYlAJV1c=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=WbEnn4G0kv6UxeRg7f3RuY+zxjl5kvC0Uvw/fWvK75wfm9S7tYcgYRSkXCp0m+Y7RQEO55Dn1c95DV692w+uCZmpTF6SA1gyGS0gj9tCtCGTnFzYJExf92bkKWc1TeLFH1alunnsc4p0VeSVz710sssGs3tipjzlltCgasemUsg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 956AD339; Mon, 19 Aug 2024 07:10:46 -0700 (PDT) Received: from [10.57.85.21] (unknown [10.57.85.21]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E8A073F73B; Mon, 19 Aug 2024 07:10:16 -0700 (PDT) Message-ID: Date: Mon, 19 Aug 2024 15:10:14 +0100 Precedence: bulk X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 05/19] arm64: Detect if in a realm and set RIPAS RAM To: Suzuki K Poulose , kvm@vger.kernel.org, kvmarm@lists.linux.dev Cc: Catalin Marinas , 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 , Gavin Shan , Shanker Donthineni , Alper Gun References: <20240819131924.372366-1-steven.price@arm.com> <20240819131924.372366-6-steven.price@arm.com> From: Steven Price Content-Language: en-GB In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 19/08/2024 15:04, Suzuki K Poulose wrote: > Hi Steven > > On 19/08/2024 14:19, Steven Price wrote: >> From: Suzuki K Poulose >> >> Detect that the VM is a realm guest by the presence of the RSI >> interface. >> >> If in a realm then all memory needs to be marked as RIPAS RAM initially, >> the loader may or may not have done this for us. To be sure iterate over >> all RAM and mark it as such. Any failure is fatal as that implies the >> RAM regions passed to Linux are incorrect - which would mean failing >> later when attempting to access non-existent RAM. >> >> Signed-off-by: Suzuki K Poulose >> Co-developed-by: Steven Price >> Signed-off-by: Steven Price >> --- >> Changes since v4: >>   * Minor tidy ups. >> Changes since v3: >>   * Provide safe/unsafe versions for converting memory to protected, >>     using the safer version only for the early boot. >>   * Use the new psci_early_test_conduit() function to avoid calling an >>     SMC if EL3 is not present (or not configured to handle an SMC). >> Changes since v2: >>   * Use DECLARE_STATIC_KEY_FALSE rather than "extern struct >>     static_key_false". >>   * Rename set_memory_range() to rsi_set_memory_range(). >>   * Downgrade some BUG()s to WARN()s and handle the condition by >>     propagating up the stack. Comment the remaining case that ends in a >>     BUG() to explain why. >>   * Rely on the return from rsi_request_version() rather than checking >>     the version the RMM claims to support. >>   * Rename the generic sounding arm64_setup_memory() to >>     arm64_rsi_setup_memory() and move the call site to setup_arch(). >> --- >>   arch/arm64/include/asm/rsi.h | 65 ++++++++++++++++++++++++++++++ >>   arch/arm64/kernel/Makefile   |  3 +- >>   arch/arm64/kernel/rsi.c      | 78 ++++++++++++++++++++++++++++++++++++ >>   arch/arm64/kernel/setup.c    |  8 ++++ >>   4 files changed, 153 insertions(+), 1 deletion(-) >>   create mode 100644 arch/arm64/include/asm/rsi.h >>   create mode 100644 arch/arm64/kernel/rsi.c >> >> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h >> new file mode 100644 >> index 000000000000..2bc013badbc3 >> --- /dev/null >> +++ b/arch/arm64/include/asm/rsi.h >> @@ -0,0 +1,65 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (C) 2024 ARM Ltd. >> + */ >> + >> +#ifndef __ASM_RSI_H_ >> +#define __ASM_RSI_H_ >> + >> +#include >> +#include >> + >> +DECLARE_STATIC_KEY_FALSE(rsi_present); >> + >> +void __init arm64_rsi_init(void); >> +void __init arm64_rsi_setup_memory(void); >> +static inline bool is_realm_world(void) >> +{ >> +    return static_branch_unlikely(&rsi_present); >> +} >> + >> +static inline int rsi_set_memory_range(phys_addr_t start, phys_addr_t >> end, >> +                       enum ripas state, unsigned long flags) >> +{ >> +    unsigned long ret; >> +    phys_addr_t top; >> + >> +    while (start != end) { >> +        ret = rsi_set_addr_range_state(start, end, state, flags, &top); >> +        if (WARN_ON(ret || top < start || top > end)) >> +            return -EINVAL; >> +        start = top; >> +    } >> + >> +    return 0; >> +} >> + >> +/* >> + * Convert the specified range to RAM. Do not use this if you rely on >> the >> + * contents of a page that may already be in RAM state. >> + */ >> +static inline int rsi_set_memory_range_protected(phys_addr_t start, >> +                         phys_addr_t end) >> +{ >> +    return rsi_set_memory_range(start, end, RSI_RIPAS_RAM, >> +                    RSI_CHANGE_DESTROYED); >> +} >> + >> +/* >> + * Convert the specified range to RAM. Do not convert any pages that >> may have >> + * been DESTROYED, without our permission. >> + */ >> +static inline int rsi_set_memory_range_protected_safe(phys_addr_t start, >> +                              phys_addr_t end) >> +{ >> +    return rsi_set_memory_range(start, end, RSI_RIPAS_RAM, >> +                    RSI_NO_CHANGE_DESTROYED); >> +} >> + >> +static inline int rsi_set_memory_range_shared(phys_addr_t start, >> +                          phys_addr_t end) >> +{ >> +    return rsi_set_memory_range(start, end, RSI_RIPAS_EMPTY, >> +                    RSI_NO_CHANGE_DESTROYED); > > I think this should be RSI_CHANGE_DESTROYED, as we are transitioning a > page to "shared" (i.e, IPA state to EMPTY) and we do not expect the data > to be retained over the transition. Thus we do not care if the IPA was > in RIPAS_DESTROYED. Fair point - although something has gone wrong if the VMM has destroyed the memory we're calling this on. But it's not going to cause problems using RSI_CHANGE_DESTROYED and might be (slightly) more efficient. Thanks, Steve > Rest looks good to me. > > > Suzuki