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 5119DC4345F for ; Wed, 1 May 2024 14:56:53 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id: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=FeUTHsp6JbG7Ynun13KS2QwpZyzobqamBMQgU2r9P6Q=; b=TCX9U4GlqsmgNh 7nuVHf9kNPVrol653rsR99I1hhc+GiEjs2NN7pb04Mt0S1ZqZsMPDuEFkhwjTnSLH7GyyN6RmYTiW lodI2qf1wSS+ZrYGKiPbBywcl4yb8Jwrqzc7+VbmO/o/DbGQXxrQppv5fDzIHCK/X5YnVyOI+VNvc k9OduqaT/h+gMsHjB17eAqig1VhLS/6/ox9U12JDVUYP+zMvKtJeHBL7zMAe+PlOTIQLHHXoBturO qB9/zGA0gS9tmG3of7/d1mL7RzZnxnxJD42592xFWORFbNti9wJyaKdyNN70EP83mo5Io1pHWeaVb AcAQADQY9Ps0aYdESK5Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s2BNx-00000009riZ-1YoN; Wed, 01 May 2024 14:56:41 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s2BNt-00000009rhV-2rRG for linux-arm-kernel@lists.infradead.org; Wed, 01 May 2024 14:56:39 +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 20F062F4; Wed, 1 May 2024 07:57:03 -0700 (PDT) Received: from [10.57.82.68] (unknown [10.57.82.68]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B37333F793; Wed, 1 May 2024 07:56:34 -0700 (PDT) Message-ID: Date: Wed, 1 May 2024 15:56:33 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 17/43] arm64: RME: Allow VMM to set RIPAS Content-Language: en-GB To: Jean-Philippe Brucker , Steven Price Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, 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 References: <20240412084056.1733704-1-steven.price@arm.com> <20240412084309.1733783-1-steven.price@arm.com> <20240412084309.1733783-18-steven.price@arm.com> <20240501142712.GB484338@myrica> From: Suzuki K Poulose In-Reply-To: <20240501142712.GB484338@myrica> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240501_075637_894233_120A76B2 X-CRM114-Status: GOOD ( 23.70 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 01/05/2024 15:27, Jean-Philippe Brucker wrote: > On Fri, Apr 12, 2024 at 09:42:43AM +0100, Steven Price wrote: >> +static inline bool realm_is_addr_protected(struct realm *realm, >> + unsigned long addr) >> +{ >> + unsigned int ia_bits = realm->ia_bits; >> + >> + return !(addr & ~(BIT(ia_bits - 1) - 1)); > > Is it enough to return !(addr & BIT(realm->ia_bits - 1))? I thought about that too. But if we are dealing with an IPA that is > (BIT(realm->ia_bits)), we don't want to be treating that as a protected address. This could only happen if the Realm is buggy (or the VMM has tricked it). So the existing check looks safer. > >> +static void realm_unmap_range_shared(struct kvm *kvm, >> + int level, >> + unsigned long start, >> + unsigned long end) >> +{ >> + struct realm *realm = &kvm->arch.realm; >> + unsigned long rd = virt_to_phys(realm->rd); >> + ssize_t map_size = rme_rtt_level_mapsize(level); >> + unsigned long next_addr, addr; >> + unsigned long shared_bit = BIT(realm->ia_bits - 1); >> + >> + if (WARN_ON(level > RME_RTT_MAX_LEVEL)) >> + return; >> + >> + start |= shared_bit; >> + end |= shared_bit; >> + >> + for (addr = start; addr < end; addr = next_addr) { >> + unsigned long align_addr = ALIGN(addr, map_size); >> + int ret; >> + >> + next_addr = ALIGN(addr + 1, map_size); >> + >> + if (align_addr != addr || next_addr > end) { >> + /* Need to recurse deeper */ >> + if (addr < align_addr) >> + next_addr = align_addr; >> + realm_unmap_range_shared(kvm, level + 1, addr, >> + min(next_addr, end)); >> + continue; >> + } >> + >> + ret = rmi_rtt_unmap_unprotected(rd, addr, level, &next_addr); >> + switch (RMI_RETURN_STATUS(ret)) { >> + case RMI_SUCCESS: >> + break; >> + case RMI_ERROR_RTT: >> + if (next_addr == addr) { >> + next_addr = ALIGN(addr + 1, map_size); >> + realm_unmap_range_shared(kvm, level + 1, addr, >> + next_addr); >> + } >> + break; >> + default: >> + WARN_ON(1); > > In this case we also need to return, because RMM returns with next_addr == > 0, causing an infinite loop. At the moment a VMM can trigger this easily > by creating guest memfd before creating a RD, see below Thats a good point. I agree. > >> + } >> + } >> +} >> + >> +static void realm_unmap_range_private(struct kvm *kvm, >> + unsigned long start, >> + unsigned long end) >> +{ >> + struct realm *realm = &kvm->arch.realm; >> + ssize_t map_size = RME_PAGE_SIZE; >> + unsigned long next_addr, addr; >> + >> + for (addr = start; addr < end; addr = next_addr) { >> + int ret; >> + >> + next_addr = ALIGN(addr + 1, map_size); >> + >> + ret = realm_destroy_protected(realm, addr, &next_addr); >> + >> + if (WARN_ON(ret)) >> + break; >> + } >> +} >> + >> +static void realm_unmap_range(struct kvm *kvm, >> + unsigned long start, >> + unsigned long end, >> + bool unmap_private) >> +{ > > Should this check for a valid kvm->arch.realm.rd, or a valid realm state? > I'm not sure what the best place is but none of the RMM calls will succeed > if the RD is NULL, causing some WARNs. > > I can trigger this with set_memory_attributes() ioctls before creating a > RD for example. > True, this could be triggered by a buggy VMM in other ways, and we could easily gate it on the Realm state >= NEW. Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel