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 02982D0D79C for ; Fri, 11 Oct 2024 14:26:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=WQxTMsBmZ4+AysClfA11TVIuqfEn8OwEgnRV72sWUuc=; b=UPoW+lKUH3+06eyyZdJskEZ5yo c0bXwFcnxNpm9gpovF4tddOIIghbDJtnXxnLMqRQx+ALGlSSAooDxBSgbqKquaudlc3aKZ67FdT8H SFIDjCe87RrQT0o2eHJ/hboYuK4EXmPuw/SH/Sea74EEHs9gnKnEDvFgt9eweoPWevYLxd8BUlhl4 cWFAIRBxm9hHk3e9F1u3kaSh2i0tJyNuBBjJHizoFy4ltJsW8ngTuB+THY235/GHfBGxwqIvwkfLY NSb5YtMlHiWcHyVNs5ITjw7pFSK251TlVHpon7rJFwh/cTqS4mepb0mk+FWW4DEloD7aC8z1LYzWf LS+GZ56w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1szGar-0000000GbJc-0UFK; Fri, 11 Oct 2024 14:26:13 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1szGPZ-0000000GZ1G-0NC8 for linux-arm-kernel@lists.infradead.org; Fri, 11 Oct 2024 14:14:35 +0000 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 33747DA7; Fri, 11 Oct 2024 07:15:02 -0700 (PDT) Received: from [10.1.31.21] (e122027.cambridge.arm.com [10.1.31.21]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B59763F5A1; Fri, 11 Oct 2024 07:14:28 -0700 (PDT) Message-ID: <5540892e-bb25-47fe-8bf4-ac4481498470@arm.com> Date: Fri, 11 Oct 2024 15:14:26 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 02/11] arm64: Detect if in a realm and set RIPAS RAM To: Gavin Shan , kvm@vger.kernel.org, kvmarm@lists.linux.dev Cc: Suzuki K Poulose , 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 , Shanker Donthineni , Alper Gun , "Aneesh Kumar K . V" References: <20241004144307.66199-1-steven.price@arm.com> <20241004144307.66199-3-steven.price@arm.com> <8a8ad27f-dc8f-4d44-bb35-67fd1133afbb@redhat.com> From: Steven Price Content-Language: en-GB In-Reply-To: <8a8ad27f-dc8f-4d44-bb35-67fd1133afbb@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241011_071433_289265_7F6F355D X-CRM114-Status: GOOD ( 31.63 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 08/10/2024 00:31, Gavin Shan wrote: > On 10/5/24 12:42 AM, Steven Price wrote: >> From: Suzuki K Poulose >> >> Detect that the VM is a realm guest by the presence of the RSI >> interface. This is done after PSCI has been initialised so that we can >> check the SMCCC conduit before making any RSI calls. >> >> 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 v5: >>   * Replace BUG_ON() with a panic() call that provides a message with the >>     memory range that couldn't be set to RIPAS_RAM. >>   * Move the call to arm64_rsi_init() later so that it is after PSCI, >>     this means we can use arm_smccc_1_1_get_conduit() to check if it is >>     safe to make RSI calls. >> 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 | 66 +++++++++++++++++++++++++++++++ >>   arch/arm64/kernel/Makefile   |  3 +- >>   arch/arm64/kernel/rsi.c      | 75 ++++++++++++++++++++++++++++++++++++ >>   arch/arm64/kernel/setup.c    |  3 ++ >>   4 files changed, 146 insertions(+), 1 deletion(-) >>   create mode 100644 arch/arm64/include/asm/rsi.h >>   create mode 100644 arch/arm64/kernel/rsi.c >> > > Two nitpicks below. > > Reviewed-by: Gavin Shan > >> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h >> new file mode 100644 >> index 000000000000..e4c01796c618 >> --- /dev/null >> +++ b/arch/arm64/include/asm/rsi.h >> @@ -0,0 +1,66 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (C) 2024 ARM Ltd. >> + */ >> + >> +#ifndef __ASM_RSI_H_ >> +#define __ASM_RSI_H_ >> + >> +#include >> +#include >> +#include >> + >> +DECLARE_STATIC_KEY_FALSE(rsi_present); >> + >> +void __init arm64_rsi_init(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; >> +} >> + > > The WARN_ON() is redundant when the caller is arm64_rsi_setup_memory(), > where > system panic is invoked on any errors. So we perhaps need to drop the > WARN_ON(). Actually this is error when I was preparing the series - the WARN_ON is then dropped in the next patch. Thanks for pointing it out! > [...] > >> + >> +static void __init arm64_rsi_setup_memory(void) >> +{ >> +    u64 i; >> +    phys_addr_t start, end; >> + >> +    /* >> +     * Iterate over the available memory ranges and convert the state to >> +     * protected memory. We should take extra care to ensure that we >> DO NOT >> +     * permit any "DESTROYED" pages to be converted to "RAM". >> +     * >> +     * panic() is used because if the attempt to switch the memory to >> +     * protected has failed here, then future accesses to the memory are >> +     * simply going to be reflected as a SEA (Synchronous External >> Abort) >> +     * which we can't handle.  Bailing out early prevents the guest >> limping >> +     * on and dying later. >> +     */ >> +    for_each_mem_range(i, &start, &end) { >> +        if (rsi_set_memory_range_protected_safe(start, end)) >> +            panic("Failed to set memory range to protected: %pa-%pa", >> +                  &start, &end); >> +    } >> +} >> + > > {} is needed since the panic statement spans multiple lines. Ack. Thanks, Steve