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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 217C2C433F5 for ; Sat, 15 Jan 2022 12:19:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230093AbiAOMTV (ORCPT ); Sat, 15 Jan 2022 07:19:21 -0500 Received: from ams.source.kernel.org ([145.40.68.75]:41614 "EHLO ams.source.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229452AbiAOMTV (ORCPT ); Sat, 15 Jan 2022 07:19:21 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id EA4C1B80176 for ; Sat, 15 Jan 2022 12:19:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8A778C36AE7; Sat, 15 Jan 2022 12:19:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1642249158; bh=6+qoAG36azZixXorCyoPCDPI6DnRVUfl3m7bOnDj5KU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=KLl9nh3MIKbcNLt6+VXOV7hxY5sBJWY9sjxvHuXJZWMQTNruEPm2oE2K8IaFkf4Ec H/gZYOZrMNriQf2QhctuVZmxnXPhzS30+wAFgdI2AIBkeI1LG2V7DfQ9LAUZjfw5bY xDDgxulwAykNCiuenXZWxKNPbEL3wS4RHX5xHvGFEUnv+XGOUHC6HC9l8CZYqXF1Ha 2o57Cek2Faz7BpmDYbLTD9LCE6jNnOXQh/qdfZkrTzne+mnHJhq9fSVH8NnGD5r6p7 QnnHRAs+xJlL8giP4ooqfGeq+CibXwYKsapO307pkqPU73JUIUqUpRGcbTKfH492ho nUJ+HXoy2k8bg== Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1n8i1c-000eTm-Hp; Sat, 15 Jan 2022 12:19:16 +0000 Date: Sat, 15 Jan 2022 12:19:16 +0000 Message-ID: <87lezh8a4b.wl-maz@kernel.org> From: Marc Zyngier To: Alexandru Elisei Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Andre Przywara , Christoffer Dall , Jintack Lim , Haibo Xu , Ganapatrao Kulkarni , James Morse , Suzuki K Poulose , kernel-team@android.com Subject: Re: [PATCH v5 11/69] KVM: arm64: nv: Add nested virt VCPU primitives for vEL2 VCPU state In-Reply-To: References: <20211129200150.351436-1-maz@kernel.org> <20211129200150.351436-12-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: alexandru.elisei@arm.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, andre.przywara@arm.com, christoffer.dall@arm.com, jintack@cs.columbia.edu, haibo.xu@linaro.org, gankulkarni@os.amperecomputing.com, james.morse@arm.com, suzuki.poulose@arm.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, 14 Jan 2022 17:42:04 +0000, Alexandru Elisei wrote: > > Hi Marc, > > Bunch of bikeshedding regarding names below, which can be safely ignored. > > On Mon, Nov 29, 2021 at 08:00:52PM +0000, Marc Zyngier wrote: > > From: Christoffer Dall > > > > When running a nested hypervisor we commonly have to figure out if > > the VCPU mode is running in the context of a guest hypervisor or guest > > guest, or just a normal guest. > > > > Add convenient primitives for this. > > > > Signed-off-by: Christoffer Dall > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/include/asm/kvm_emulate.h | 55 ++++++++++++++++++++++++++++ > > 1 file changed, 55 insertions(+) > > > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h > > index f4871e47b2d0..f4b079945d0f 100644 > > --- a/arch/arm64/include/asm/kvm_emulate.h > > +++ b/arch/arm64/include/asm/kvm_emulate.h > > @@ -176,6 +176,61 @@ static __always_inline void vcpu_set_reg(struct kvm_vcpu *vcpu, u8 reg_num, > > vcpu_gp_regs(vcpu)->regs[reg_num] = val; > > } > > > > +static inline bool vcpu_mode_el2_ctxt(const struct kvm_cpu_context *ctxt) > > "The Armv8-A architecture defines a set of Exception levels, EL0 to EL3" (ARM > DDI 0487G.a, page G1-5941). > > "AArch64 state does not support modes. Modes are a concept that is specific to > AArch32 state." (ARM DDI 0487G.a, page G1-5944). > > Wouldn't it be better to use the same terminology as the architecture? Probably. I'll see how invasive it is to repaint this. It still remains that the 'mode' term is used all over the shop (for example, PSR_MODE_*). > > +{ > > + unsigned long cpsr = ctxt->regs.pstate; > > CPSR is an AArch32 register. Why not name the variable pstate? Because we have *a ton* of existing CPSR references all over the shop (more than references to pstate, actually), owing to the AArch32 heritage of KVM/arm64. Yes, I can change this locally without any damage. But repainting the whole of KVM would be fairly pointless (same with hsr/esr, hfar/far_el2...). > > > + > > + switch (cpsr & (PSR_MODE32_BIT | PSR_MODE_MASK)) { > > + case PSR_MODE_EL2h: > > + case PSR_MODE_EL2t: > > + return true; > > + default: > > + return false; > > + } > > +} > > + > > +static inline bool vcpu_mode_el2(const struct kvm_vcpu *vcpu) > > +{ > > + return vcpu_mode_el2_ctxt(&vcpu->arch.ctxt); > > +} > > + > > +static inline bool __vcpu_el2_e2h_is_set(const struct kvm_cpu_context *ctxt) > > +{ > > + return ctxt_sys_reg(ctxt, HCR_EL2) & HCR_E2H; > > +} > > + > > +static inline bool vcpu_el2_e2h_is_set(const struct kvm_vcpu *vcpu) > > +{ > > + return __vcpu_el2_e2h_is_set(&vcpu->arch.ctxt); > > +} > > + > > +static inline bool __vcpu_el2_tge_is_set(const struct kvm_cpu_context *ctxt) > > +{ > > + return ctxt_sys_reg(ctxt, HCR_EL2) & HCR_TGE; > > +} > > + > > +static inline bool vcpu_el2_tge_is_set(const struct kvm_vcpu *vcpu) > > This is confusing. What does the exception level have to do with the > HCR_EL2.TGE bit being set? Wouldn't vcpu_hcr_tge_is_set() be better? Sure, why not. Again, I'll see how invasive such a repainting is across 70 patches. > > > +{ > > + return __vcpu_el2_tge_is_set(&vcpu->arch.ctxt); > > +} > > + > > +static inline bool __is_hyp_ctxt(const struct kvm_cpu_context *ctxt) > > +{ > > + /* > > + * We are in a hypervisor context if the vcpu mode is EL2 or > > + * E2H and TGE bits are set. The latter means we are in the user space > > + * of the VHE kernel. ARMv8.1 ARM describes this as 'InHost' > > So why not call the function vcpu_is_inhost() or something along > those lines to match the architecture? Because this is not the architectural 'InHost' primitive, which returns 'false' if HCR_EL2.E2H==0. The second term of the expression could be written in terms of an InHost primitive, but that's about it. > > Thanks, > Alex > > > + */ > > + return vcpu_mode_el2_ctxt(ctxt) || > > + (__vcpu_el2_e2h_is_set(ctxt) && __vcpu_el2_tge_is_set(ctxt)) || > > + WARN_ON(__vcpu_el2_tge_is_set(ctxt)); > > +} > > + > > +static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu) > > +{ > > + return __is_hyp_ctxt(&vcpu->arch.ctxt); > > +} > > + > > /* > > * The layout of SPSR for an AArch32 state is different when observed from an > > * AArch64 SPSR_ELx or an AArch32 SPSR_*. This function generates the AArch32 Thanks, M. -- Without deviation from the norm, progress is not possible.