Received: by 2002:ab2:6203:0:b0:1f5:f2ab:c469 with SMTP id o3csp618351lqt; Fri, 19 Apr 2024 06:00:58 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCXIXDI72Jm6Rn1MqhD7E6hPvwuDjs7Petb/WPm83gZ5N7dB6qIQZLfetsimo7vLol9Skn9w65KJTq0cP0UiU8VAXA+/m0JftboJcdDldw== X-Google-Smtp-Source: AGHT+IFnmVXdAXGUSf98Wjx5xL//GOF3yeOTG6WtsRwLDk1caiHq/BDbl/DMg9IWh4hKnMEjD6zr X-Received: by 2002:a05:6a20:1005:b0:1a3:9f87:f389 with SMTP id gs5-20020a056a20100500b001a39f87f389mr2431128pzc.38.1713531657045; Fri, 19 Apr 2024 06:00:57 -0700 (PDT) Return-Path: Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id i134-20020a636d8c000000b005f7f2848626si1551792pgc.216.2024.04.19.06.00.56 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Apr 2024 06:00:57 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-151438-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; arc=fail (body hash mismatch); spf=pass (google.com: domain of linux-kernel+bounces-151438-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-151438-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 667D2283BF1 for ; Fri, 19 Apr 2024 13:00:52 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 9033212836D; Fri, 19 Apr 2024 13:00:37 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id B3BE385948; Fri, 19 Apr 2024 13:00:34 +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=1713531636; cv=none; b=H4bAiZxi5tdfRSq4Ps0ykoTRCsgdrUMi1kOaKeu62jR6HT2HhfZvH4hFI5JeP6QUYjTAGB1LnuxuSF5XQjrYthAfnAYfX1r++tZjFs1nWgZm2Ov4ffxbsM+tHrqR9MNYaSpNKk23J6NMExDlluQBQY3tRsflpF7pe2mi9rAvX1g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713531636; c=relaxed/simple; bh=aCwfyFsGqeYBypUL6ENw7wQt4VJhUg9101fZ8WXT9yI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=HLOSr0cHnyX8fdGzPWf/IzMfS9LLHtypHb9Hdh+tjT0/KYcsWloeDoce6QouC2AsNRBiqV1DWq7Th4MU8OMwr30tzTSzTs+BhiZvdhVul+whCgPi/HiNJoQRyHu+c6VIp4GDr7L+d/cPSWlX1PQafzq+UjQUnPLNcMfHucu4aJo= 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 41A292F; Fri, 19 Apr 2024 06:01:02 -0700 (PDT) Received: from [10.1.197.1] (ewhatever.cambridge.arm.com [10.1.197.1]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A1D633F64C; Fri, 19 Apr 2024 06:00:31 -0700 (PDT) Message-ID: <0606bda2-7f3f-4fe9-8d7d-70dc4720dcd1@arm.com> Date: Fri, 19 Apr 2024 14:00:30 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 18/43] arm64: RME: Handle realm enter/exit To: Steven Price , 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 References: <20240412084056.1733704-1-steven.price@arm.com> <20240412084309.1733783-1-steven.price@arm.com> <20240412084309.1733783-19-steven.price@arm.com> Content-Language: en-US From: Suzuki K Poulose In-Reply-To: <20240412084309.1733783-19-steven.price@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Steven On 12/04/2024 09:42, Steven Price wrote: > Entering a realm is done using a SMC call to the RMM. On exit the > exit-codes need to be handled slightly differently to the normal KVM > path so define our own functions for realm enter/exit and hook them > in if the guest is a realm guest. > > Signed-off-by: Steven Price Please find a comment about RIPAS change exit handling below. Rest looks good to me. > --- > arch/arm64/include/asm/kvm_rme.h | 3 + > arch/arm64/kvm/Makefile | 2 +- > arch/arm64/kvm/arm.c | 19 +++- > arch/arm64/kvm/rme-exit.c | 180 +++++++++++++++++++++++++++++++ > arch/arm64/kvm/rme.c | 11 ++ > 5 files changed, 209 insertions(+), 6 deletions(-) > create mode 100644 arch/arm64/kvm/rme-exit.c > .. > > /* Tell userspace about in-kernel device output levels */ > diff --git a/arch/arm64/kvm/rme-exit.c b/arch/arm64/kvm/rme-exit.c > new file mode 100644 > index 000000000000..5bf58c9b42b7 > --- /dev/null > +++ b/arch/arm64/kvm/rme-exit.c > @@ -0,0 +1,180 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 ARM Ltd. > + */ > + > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +typedef int (*exit_handler_fn)(struct kvm_vcpu *vcpu); > + > +static int rec_exit_reason_notimpl(struct kvm_vcpu *vcpu) > +{ > + struct realm_rec *rec = &vcpu->arch.rec; > + > + pr_err("[vcpu %d] Unhandled exit reason from realm (ESR: %#llx)\n", > + vcpu->vcpu_id, rec->run->exit.esr); > + return -ENXIO; > +} > + > +static int rec_exit_sync_dabt(struct kvm_vcpu *vcpu) > +{ > + return kvm_handle_guest_abort(vcpu); > +} > + > +static int rec_exit_sync_iabt(struct kvm_vcpu *vcpu) > +{ > + struct realm_rec *rec = &vcpu->arch.rec; > + > + pr_err("[vcpu %d] Unhandled instruction abort (ESR: %#llx).\n", > + vcpu->vcpu_id, rec->run->exit.esr); > + return -ENXIO; > +} > + > +static int rec_exit_sys_reg(struct kvm_vcpu *vcpu) > +{ > + struct realm_rec *rec = &vcpu->arch.rec; > + unsigned long esr = kvm_vcpu_get_esr(vcpu); > + int rt = kvm_vcpu_sys_get_rt(vcpu); > + bool is_write = !(esr & 1); > + int ret; > + > + if (is_write) > + vcpu_set_reg(vcpu, rt, rec->run->exit.gprs[0]); > + > + ret = kvm_handle_sys_reg(vcpu); > + > + if (ret >= 0 && !is_write) > + rec->run->entry.gprs[0] = vcpu_get_reg(vcpu, rt); > + > + return ret; > +} > + > +static exit_handler_fn rec_exit_handlers[] = { > + [0 ... ESR_ELx_EC_MAX] = rec_exit_reason_notimpl, > + [ESR_ELx_EC_SYS64] = rec_exit_sys_reg, > + [ESR_ELx_EC_DABT_LOW] = rec_exit_sync_dabt, > + [ESR_ELx_EC_IABT_LOW] = rec_exit_sync_iabt > +}; > + > +static int rec_exit_psci(struct kvm_vcpu *vcpu) > +{ > + struct realm_rec *rec = &vcpu->arch.rec; > + int i; > + int ret; > + > + for (i = 0; i < REC_RUN_GPRS; i++) > + vcpu_set_reg(vcpu, i, rec->run->exit.gprs[i]); > + > + ret = kvm_smccc_call_handler(vcpu); > + > + for (i = 0; i < REC_RUN_GPRS; i++) > + rec->run->entry.gprs[i] = vcpu_get_reg(vcpu, i); > + > + return ret; > +} > + > +static int rec_exit_ripas_change(struct kvm_vcpu *vcpu) > +{ > + struct kvm *kvm = vcpu->kvm; > + struct realm *realm = &kvm->arch.realm; > + struct realm_rec *rec = &vcpu->arch.rec; > + unsigned long base = rec->run->exit.ripas_base; > + unsigned long top = rec->run->exit.ripas_top; > + unsigned long ripas = rec->run->exit.ripas_value & 1; > + int ret = -EINVAL; > + > + if (realm_is_addr_protected(realm, base) && > + realm_is_addr_protected(realm, top - 1)) { > + kvm_mmu_topup_memory_cache(&vcpu->arch.mmu_page_cache, > + kvm_mmu_cache_min_pages(vcpu->arch.hw_mmu)); > + write_lock(&kvm->mmu_lock); > + ret = realm_set_ipa_state(vcpu, base, top, ripas); > + write_unlock(&kvm->mmu_lock); > + } There are couple of issues here : 1. We don't verify if the range is covered by a memslot, for ripas == RIPAS_RAM conversions and reject the request if not backed by memory. 2. The realm_set_ipa_state() might have partially completed the request before it ran out of pages in the cache. And as such we should only let the VMM know about what was completed, below. So, we need some feedback from realm_set_ipa_state() to indicate the completed range. Rest looks fine to me. Suzuki > + > + WARN(ret && ret != -ENOMEM, > + "Unable to satisfy SET_IPAS for %#lx - %#lx, ripas: %#lx\n", > + base, top, ripas); > + > + /* Exit to VMM to complete the change */ > + kvm_prepare_memory_fault_exit(vcpu, base, top - base, false, false, > + ripas == 1); > + > + return 0; > +} > + > +static void update_arch_timer_irq_lines(struct kvm_vcpu *vcpu) > +{ > + struct realm_rec *rec = &vcpu->arch.rec; > + > + __vcpu_sys_reg(vcpu, CNTV_CTL_EL0) = rec->run->exit.cntv_ctl; > + __vcpu_sys_reg(vcpu, CNTV_CVAL_EL0) = rec->run->exit.cntv_cval; > + __vcpu_sys_reg(vcpu, CNTP_CTL_EL0) = rec->run->exit.cntp_ctl; > + __vcpu_sys_reg(vcpu, CNTP_CVAL_EL0) = rec->run->exit.cntp_cval; > + > + kvm_realm_timers_update(vcpu); > +} > + > +/* > + * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on > + * proper exit to userspace. > + */ > +int handle_rme_exit(struct kvm_vcpu *vcpu, int rec_run_ret) > +{ > + struct realm_rec *rec = &vcpu->arch.rec; > + u8 esr_ec = ESR_ELx_EC(rec->run->exit.esr); > + unsigned long status, index; > + > + status = RMI_RETURN_STATUS(rec_run_ret); > + index = RMI_RETURN_INDEX(rec_run_ret); > + > + /* > + * If a PSCI_SYSTEM_OFF request raced with a vcpu executing, we might > + * see the following status code and index indicating an attempt to run > + * a REC when the RD state is SYSTEM_OFF. In this case, we just need to > + * return to user space which can deal with the system event or will try > + * to run the KVM VCPU again, at which point we will no longer attempt > + * to enter the Realm because we will have a sleep request pending on > + * the VCPU as a result of KVM's PSCI handling. > + */ > + if (status == RMI_ERROR_REALM && index == 1) { > + vcpu->run->exit_reason = KVM_EXIT_UNKNOWN; > + return 0; > + } > + > + if (rec_run_ret) > + return -ENXIO; > + > + vcpu->arch.fault.esr_el2 = rec->run->exit.esr; > + vcpu->arch.fault.far_el2 = rec->run->exit.far; > + vcpu->arch.fault.hpfar_el2 = rec->run->exit.hpfar; > + > + update_arch_timer_irq_lines(vcpu); > + > + /* Reset the emulation flags for the next run of the REC */ > + rec->run->entry.flags = 0; > + > + switch (rec->run->exit.exit_reason) { > + case RMI_EXIT_SYNC: > + return rec_exit_handlers[esr_ec](vcpu); > + case RMI_EXIT_IRQ: > + case RMI_EXIT_FIQ: > + return 1; > + case RMI_EXIT_PSCI: > + return rec_exit_psci(vcpu); > + case RMI_EXIT_RIPAS_CHANGE: > + return rec_exit_ripas_change(vcpu); > + } I guess, the RMI_EXIT_HOST_CALL could be handled here similar to the rec_exit_psci() ? Suzuki