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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F11F4C4338F for ; Sat, 21 Aug 2021 10:56:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D39246120E for ; Sat, 21 Aug 2021 10:56:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234228AbhHUK52 (ORCPT ); Sat, 21 Aug 2021 06:57:28 -0400 Received: from mail.kernel.org ([198.145.29.99]:38150 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229968AbhHUK51 (ORCPT ); Sat, 21 Aug 2021 06:57:27 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 92BB16113E; Sat, 21 Aug 2021 10:56:48 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=wait-a-minute.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 1mHOgA-006MEI-Gc; Sat, 21 Aug 2021 11:56:46 +0100 Date: Sat, 21 Aug 2021 11:56:46 +0100 Message-ID: <875yvzqd5d.wl-maz@kernel.org> From: Marc Zyngier To: Raghavendra Rao Ananta Cc: Alexandru Elisei , Suzuki K Poulose , Catalin Marinas , Will Deacon , Peter Shier , Ricardo Koller , Oliver Upton , Reiji Watanabe , Jing Zhang , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org Subject: Re: [PATCH] KVM: arm64: Ratelimit error log during guest debug exception In-Reply-To: References: <20210819223406.1132426-1-rananta@google.com> <87sfz4qx9r.wl-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: rananta@google.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, pshier@google.com, ricarkol@google.com, oupton@google.com, reijiw@google.com, jingzhangos@google.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, 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 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 21 Aug 2021 00:01:24 +0100, Raghavendra Rao Ananta wrote: > > [1 ] > On Fri, Aug 20, 2021 at 2:29 AM Marc Zyngier wrote: > > > > On Thu, 19 Aug 2021 23:34:06 +0100, > > Raghavendra Rao Ananta wrote: > > > > > > Potentially, the guests could trigger a debug exception that's > > > outside the exception class range. > > > > How? All the exception classes that lead to this functions are already > > handled in the switch/case statement. > > > I guess I didn't think this through. Landing into kvm_handle_guest_debug() > itself is not possible :) Exactly. > > My take on this is that this code isn't reachable, and that it could > > be better rewritten as: > > > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > > index 6f48336b1d86..ae7ec086827b 100644 > > --- a/arch/arm64/kvm/handle_exit.c > > +++ b/arch/arm64/kvm/handle_exit.c > > @@ -119,28 +119,14 @@ static int kvm_handle_guest_debug(struct kvm_vcpu > *vcpu) > > { > > struct kvm_run *run = vcpu->run; > > u32 esr = kvm_vcpu_get_esr(vcpu); > > - int ret = 0; > > > > run->exit_reason = KVM_EXIT_DEBUG; > > run->debug.arch.hsr = esr; > > > > - switch (ESR_ELx_EC(esr)) { > > - case ESR_ELx_EC_WATCHPT_LOW: > > + if (ESR_ELx_EC(esr) == ESR_ELx_EC_WATCHPT_LOW) > > run->debug.arch.far = vcpu->arch.fault.far_el2; > > - fallthrough; > > - case ESR_ELx_EC_SOFTSTP_LOW: > > - case ESR_ELx_EC_BREAKPT_LOW: > > - case ESR_ELx_EC_BKPT32: > > - case ESR_ELx_EC_BRK64: > > - break; > > - default: > > - kvm_err("%s: un-handled case esr: %#08x\n", > > - __func__, (unsigned int) esr); > > - ret = -1; > > - break; > > - } > > > > - return ret; > > + return 0; > > } > > > This looks better, but do you think we would be compromising on readability? I don't think so. The exit handler table is, on its own, pretty explicit about what we route to this handler, and the comment above the function clearly states that we exit to userspace for all the debug ECs. M. -- Without deviation from the norm, progress is not possible.