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 BA707C77B7E for ; Mon, 29 May 2023 08:33:47 +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-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=WAeE9mj/XoTavwJet9lwd5NDAWTLxdk/zSB4JMqlEdE=; b=C/DmaFLYKeMZPH KA5DmTthNXg2h+NUQt+0cpzPKyb3lAq44Kcd5T8qQYAb4pcJxNdPThtqt5YA+pEMgWuQc9lIZNZVK hAFBfU5J63uET5Jwit3ALzutKqpz+xB4O48qiovYmqaueglZA6tWOuwU+oNGaim6vSuL11Vn9tSVE 4bcV8k91ToGNLO8uTL6Nt+KRAQ9ebRwAy2mDYPcdysnNfwoWAX8N2xdaKn05l3AtzREevw8ezqVmv yCkiPiV6L0ckLpgQ5GrmbpSTlB25d2ez1CSwpuJKTWAUIZwAwyMAjjKg81tJoMu/60GpG4/4DwXed pSNF0u8o3Z+Zold6+pFg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q3YJc-009j0i-2u; Mon, 29 May 2023 08:33:20 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q3YJa-009j03-24 for linux-arm-kernel@lists.infradead.org; Mon, 29 May 2023 08:33:20 +0000 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 dfw.source.kernel.org (Postfix) with ESMTPS id C144F60F19; Mon, 29 May 2023 08:33:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 26EC8C433D2; Mon, 29 May 2023 08:33:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685349197; bh=WdkMlXgHUbbdzZIgt+sMi4JSkaJspGqcxm6p/N+X3FY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=l4Jedt8qBrjaq3ynBEod8SRHWFkTi9jv2m1cPkw5CdkwNAYKSTUzqkilic6DNbRXX SiQfH207c/yOZ2rEg3TGKhqRTH2qmDaEDV0GQ1vxchjeLKuH4B7Y01aCDYWeNWiDYA X6B8+VlqwPalwZr5h46oNufL+idNC5U9XLizIsSt9srJqUkvPdSl/5Sl939RdI9MyR vS+nNMVQBdERz/inxDEPuAENGkueS6mxKwh5VuPKJ0nAihCWEZ8xCuUXLkuz5P44+O xVz2Y9jYAHNycamiYIfkG5FqHEZ7lSHFBCv+XFACx6noCEExYi4qi77jqGWNs4vn8U F9VwEQx/eCo2g== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1q3YJW-0011nz-OT; Mon, 29 May 2023 09:33:14 +0100 Date: Mon, 29 May 2023 09:33:14 +0100 Message-ID: <86edmzczat.wl-maz@kernel.org> From: Marc Zyngier To: Akihiko Odaki Cc: James Morse , Suzuki K Poulose , Oliver Upton , Zenghui Yu , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] KVM: arm64: Populate fault info for watchpoint In-Reply-To: <20230529024847.8671-1-akihiko.odaki@daynix.com> References: <20230529024847.8671-1-akihiko.odaki@daynix.com> 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/28.2 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: akihiko.odaki@daynix.com, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230529_013318_752649_65248A90 X-CRM114-Status: GOOD ( 25.96 ) 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 29 May 2023 03:48:47 +0100, Akihiko Odaki wrote: > > When handling ESR_ELx_EC_WATCHPT_LOW, far_el2 member of struct > kvm_vcpu_fault_info will be copied to far member of struct > kvm_debug_exit_arch and exposed to the userspace. The userspace will > see stale values from older faults if the fault info does not get > populated. Nice catch! Some bike-shedding below... > > Fixes: 8fb2046180a0 ("KVM: arm64: Move early handlers to per-EC handlers") > Signed-off-by: Akihiko Odaki > --- > arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +- > arch/arm64/kvm/hyp/nvhe/switch.c | 6 ++++-- > arch/arm64/kvm/hyp/vhe/switch.c | 3 ++- > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 07d37ff88a3f..2eb0a1481ead 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -351,7 +351,7 @@ static bool kvm_hyp_handle_cp15_32(struct kvm_vcpu *vcpu, u64 *exit_code) > return false; > } > > -static bool kvm_hyp_handle_iabt_low(struct kvm_vcpu *vcpu, u64 *exit_code) > +static bool kvm_hyp_handle_generic_fault(struct kvm_vcpu *vcpu, u64 *exit_code) > { > if (!__populate_fault_info(vcpu)) > return true; > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index c2cb46ca4fb6..1b77866079dc 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -184,8 +184,9 @@ static const exit_handler_fn hyp_exit_handlers[] = { > [ESR_ELx_EC_SYS64] = kvm_hyp_handle_sysreg, > [ESR_ELx_EC_SVE] = kvm_hyp_handle_fpsimd, > [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd, > - [ESR_ELx_EC_IABT_LOW] = kvm_hyp_handle_iabt_low, > + [ESR_ELx_EC_IABT_LOW] = kvm_hyp_handle_generic_fault, > [ESR_ELx_EC_DABT_LOW] = kvm_hyp_handle_dabt_low, > + [ESR_ELx_EC_WATCHPT_LOW] = kvm_hyp_handle_generic_fault, > [ESR_ELx_EC_PAC] = kvm_hyp_handle_ptrauth, > }; It is slightly odd to merge instruction abort with watchpoint faults, as the later is solely concerned with data, and not instructions. I'd rather we don't completely unify the handlers, even if they have the same implementation. There will be various differences in handling with NV which will result in diverging implementations. So either duplicate the code (it wouldn't make a huge difference), or add kvm_hyp_handle_iabt_low() and kvm_hyp_handle_watchpt_low() as aliases of kvm_hyp_handle_generic_fault (which would probably be better renamed as kvm_hyp_handle_memory_fault). And then use that as the prologue to the dabt handler. Something like the patch below. Thanks, M. diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 0a344184f26c..928983726a62 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -414,17 +414,21 @@ static bool kvm_hyp_handle_cp15_32(struct kvm_vcpu *vcpu, u64 *exit_code) return false; } -static bool kvm_hyp_handle_iabt_low(struct kvm_vcpu *vcpu, u64 *exit_code) +static bool kvm_hyp_handle_memory_fault(struct kvm_vcpu *vcpu, u64 *exit_code) { if (!__populate_fault_info(vcpu)) return true; return false; } +static bool kvm_hyp_handle_iabt_low(struct kvm_vcpu *vcpu, u64 *exit_code) + __alias(kvm_hyp_handle_memory_fault); +static bool kvm_hyp_handle_watchpt_low(struct kvm_vcpu *vcpu, u64 *exit_code) + __alias(kvm_hyp_handle_memory_fault); static bool kvm_hyp_handle_dabt_low(struct kvm_vcpu *vcpu, u64 *exit_code) { - if (!__populate_fault_info(vcpu)) + if (kvm_hyp_handle_memory_fault(vcpu, exit_code)) return true; if (static_branch_unlikely(&vgic_v2_cpuif_trap)) { diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index 7730860552da..b357ba7c453d 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -186,6 +186,7 @@ static const exit_handler_fn hyp_exit_handlers[] = { [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd, [ESR_ELx_EC_IABT_LOW] = kvm_hyp_handle_iabt_low, [ESR_ELx_EC_DABT_LOW] = kvm_hyp_handle_dabt_low, + [ESR_ELx_EC_WATCHPT_LOW] = kvm_hyp_handle_watchpt_low, [ESR_ELx_EC_PAC] = kvm_hyp_handle_ptrauth, }; @@ -196,6 +197,7 @@ static const exit_handler_fn pvm_exit_handlers[] = { [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd, [ESR_ELx_EC_IABT_LOW] = kvm_hyp_handle_iabt_low, [ESR_ELx_EC_DABT_LOW] = kvm_hyp_handle_dabt_low, + [ESR_ELx_EC_WATCHPT_LOW] = kvm_hyp_handle_watchpt_low, [ESR_ELx_EC_PAC] = kvm_hyp_handle_ptrauth, }; diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index e03c3413cf49..46f0fce9b01f 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -311,6 +311,7 @@ static const exit_handler_fn hyp_exit_handlers[] = { [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd, [ESR_ELx_EC_IABT_LOW] = kvm_hyp_handle_iabt_low, [ESR_ELx_EC_DABT_LOW] = kvm_hyp_handle_dabt_low, + [ESR_ELx_EC_WATCHPT_LOW] = kvm_hyp_handle_watchpt_low, [ESR_ELx_EC_PAC] = kvm_hyp_handle_ptrauth, [ESR_ELx_EC_ERET] = kvm_hyp_handle_eret, }; -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel