From patchwork Tue Oct 3 16:30:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 9983215 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 25FAE6029B for ; Tue, 3 Oct 2017 16:30:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 17BAF28613 for ; Tue, 3 Oct 2017 16:30:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0C6E6287B9; Tue, 3 Oct 2017 16:30:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id F389E28613 for ; Tue, 3 Oct 2017 16:30:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date: In-reply-to:Subject:To:From:References:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=fhjSrn7Tz1Rqo+QYxrZYVRYIV+rsrB0VgF/qe3UYOas=; b=pZVsWDiM00XOgs82bdgJGl1XOy mGjxq5ogLfci1otNrruRna37yVFOIv+LXgAKw9VklBz9vemdFRXg+OvNU6xGPCqtnS+mhqTSQ8Wdv PSX3Sygbs+ErHDKDTJTwDiZsLNG/jtDe+aK7YiW1CLVh/Kab/HSdsW0NJg+vQl3t1hzxdwyk/7ZNb cqQT72UkzhjPEANcUnCJxkxI0crxW6+gijl2N6dKuiSy/t8N3CKCtnHZKJ4qAVOP3KPfZCeAedVZD AiQPKcHkcNzglXZQBaVXofcvngw1Y1Hfs/F7ioWD4JlP8pypQWjUNGxMqN9c2gwQW8K2z1nsw/3hz kY1QVaCQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dzQ5f-0006IU-3H; Tue, 03 Oct 2017 16:30:39 +0000 Received: from mail-wm0-x22a.google.com ([2a00:1450:400c:c09::22a]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dzQ5b-0006FA-0n for linux-arm-kernel@lists.infradead.org; Tue, 03 Oct 2017 16:30:37 +0000 Received: by mail-wm0-x22a.google.com with SMTP id 196so6608434wma.1 for ; Tue, 03 Oct 2017 09:30:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=references:user-agent:from:to:cc:subject:in-reply-to:date :message-id:mime-version:content-transfer-encoding; bh=CXCNAbZfvhbxKaI+xJQAsq+K0bWLXRcJztPebhAiaUg=; b=Rs6ZKP5WNCx1awgSiXEJXTNPOVLnu7JolaXi57sbp9eHxs1lEZgQGdEuExGr3uW+5+ vNFn8+Qrd4Ure0ZqUdmEvGbeb5uJyyxMY6Pg7qZI1KVNpd4tktQ5F2H0yUQsaFJ/TgSB jINvxspYml7UucIHMS6i0OGRT/vb6RILbH1Vc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:references:user-agent:from:to:cc:subject :in-reply-to:date:message-id:mime-version:content-transfer-encoding; bh=CXCNAbZfvhbxKaI+xJQAsq+K0bWLXRcJztPebhAiaUg=; b=nklm5j+gwKTL8BHf5p2e6GsI3E4y18Qmq8XHD2ob7hm3swA9X2cRl5c6l30Z9brz7/ fTUB8g1CWxOpqjzgahUdBTBGH4uafTkd8XHyic2ksZYq2gEXp8Y/DctlBPGZXl5cjb5z 4JsjsfQ0vrblhbeX8+VYf9AYBTDSn8QRJKjDWRzTnhrGCWOglDud8DoNrefLRNGk9iwk TiUp8BtonuRKP1kdazANUF2TKJdLdqzkhxK4mC5rOWRaXW/4BB8wccAzpK2RgNTYHIHt 1w3kUpaHs+UB7dn4sefmiM7kYtkHBQSCLr7OeMi64g+sg4D4PZPcN9Wl0qlG89997Wop hnkA== X-Gm-Message-State: AHPjjUiUgpTdDQF4ke0w68nOc8arso78KRFI3nqlrElVOGVdlwIQGohi J4+B1aHN21M6umw2oxpWF07M0GrSLlY= X-Google-Smtp-Source: AOwi7QB3Vwela42YAWwafz2yvOjA6MrNwl6cAsC+v2JGbmoYqXfsHDWkxu8/Kg2SpK6obxBvXBlvGA== X-Received: by 10.28.24.7 with SMTP id 7mr15111629wmy.78.1507048212742; Tue, 03 Oct 2017 09:30:12 -0700 (PDT) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id r123sm3056579wmf.12.2017.10.03.09.30.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 03 Oct 2017 09:30:11 -0700 (PDT) Received: from zen (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTPS id 1D87A3E1337; Tue, 3 Oct 2017 17:30:11 +0100 (BST) References: <1504083688-48334-1-git-send-email-julien.thierry@arm.com> <1504083688-48334-4-git-send-email-julien.thierry@arm.com> <3c249a68-45e3-a3a5-7d05-4cfc2d97713b@arm.com> <3d7d2b36-da2f-04dc-611e-d7aab7666c29@arm.com> <9bc5abc2-ab03-3137-82bd-e8afa62624eb@arm.com> <861b4e4f-0fbe-cbc6-39ad-4660065449de@arm.com> <877ewcz3bv.fsf@linaro.org> <4d9fc0a2-bcf9-ca26-8646-037c2dcc6545@arm.com> User-agent: mu4e 0.9.19; emacs 26.0.60 From: Alex =?utf-8?Q?Benn=C3=A9e?= To: Julien Thierry Subject: Re: [PATCH 3/3] arm64: kvm: Fix single step for guest skipped instructions In-reply-to: <4d9fc0a2-bcf9-ca26-8646-037c2dcc6545@arm.com> Date: Tue, 03 Oct 2017 17:30:11 +0100 Message-ID: <873770yz1o.fsf@linaro.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171003_093035_228086_BEB88C02 X-CRM114-Status: GOOD ( 22.69 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Marc Zyngier , Catalin Marinas , Will Deacon , "linux-arm-kernel@lists.infradead.org" , Christoffer Dall Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Julien Thierry writes: > On 03/10/17 15:57, Alex Bennée wrote: >> >> Julien Thierry writes: >> >>> On 31/08/17 15:01, Christoffer Dall wrote: >>>>>>> >>>>>>> Does that sound like what you had in mind? Or does it seem better than >>>>>>> the >>>>>>> current patch? >>>>>>> >>>>>> I was thinking to change the skip_instruction function to return an >>>>>> int, and then call kvm_handle_debug_ss() from skip_instruction, which >>>>>> would update the kvm_run structure and exit here and then. >>>>>> >>>>> >>>>> Setting up the debug exception from within kvm_skip_instruction seem to >>>>> change a bit too much its semantic from arm to arm64. I would find this >>>>> easily confusing. >>>>> >>>>>> However, I'm now thinking that this doesn't really work either, >>>>>> because we could have to emulate a trapped MMIO instruction in user >>>>>> space, and then it's not clear how to exit with a debug exception at >>>>>> the same time. >> >> A debug exception at guest exit point is (IIRC) just having the >> appropriate status in the run->exit_reason (KVM_EXIT_DEBUG). If you need >> to exit for MMIO emulation (i.e. the instruction has not run yet) you >> shouldn't do that. Exit, emulate and return. We could handle the ioctl >> to clear SS in userspace but I guess that gets just as messy. >> >>>>>> >>>>>> So perhaps we should stick with your original approach. >>>>>> >>>>> >>>>> I had not realized that was possible. This makes things more complicated for >>>>> avoiding a back and forth with the guest for trapped exceptions. Out of >>>>> luck, having the debug flag does look like single stepping would work as >>>>> expected for userland MMIOs. This is my currently untested but otherwise simpler solution: From 46ea80d7dc9b98661fcd51c41090f8ad74a6690f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 3 Oct 2017 17:17:15 +0100 Subject: [PATCH] KVM: arm64: handle single-stepping trapped instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we are using guest debug to single-step the guest we need to ensure we exit after emulating the instruction. This only affects instructions emulated by the kernel. If we exit to userspace anyway we leave it to userspace to work out what to do. Signed-off-by: Alex Bennée --- arch/arm64/kvm/handle_exit.c | 51 ++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 14 deletions(-) -- 2.14.1 diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 7debb74843a0..b197ffb10e96 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -178,6 +178,42 @@ static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu) return arm_exit_handlers[hsr_ec]; } +/* + * When handling traps we need to ensure exit the guest if we + * successfully emulated the instruction while single-stepping. If we + * have to exit anyway for userspace emulation then it's up to + * userspace to handle the "while SSing case". + */ + +static int handle_trap_exceptions(struct kvm_vcpu *vcpu, struct kvm_run *run) +{ + int handled; + + /* + * See ARM ARM B1.14.1: "Hyp traps on instructions + * that fail their condition code check" + */ + if (!kvm_condition_valid(vcpu)) { + kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); + handled = 1; + } else { + exit_handle_fn exit_handler; + + exit_handler = kvm_get_exit_handler(vcpu); + handled = exit_handler(vcpu, run); + } + + if (handled && (vcpu->debug_flags & KVM_GUESTDBG_SINGLESTEP)) { + u32 hsr = kvm_vcpu_get_hsr(vcpu); + + handled = 0; + run->exit_reason = KVM_EXIT_DEBUG; + run->debug.arch.hsr = hsr; + } + + return handled; +} + /* * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on * proper exit to userspace. @@ -185,8 +221,6 @@ static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu) int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, int exception_index) { - exit_handle_fn exit_handler; - if (ARM_SERROR_PENDING(exception_index)) { u8 hsr_ec = ESR_ELx_EC(kvm_vcpu_get_hsr(vcpu)); @@ -214,18 +248,7 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, kvm_inject_vabt(vcpu); return 1; case ARM_EXCEPTION_TRAP: - /* - * See ARM ARM B1.14.1: "Hyp traps on instructions - * that fail their condition code check" - */ - if (!kvm_condition_valid(vcpu)) { - kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); - return 1; - } - - exit_handler = kvm_get_exit_handler(vcpu); - - return exit_handler(vcpu, run); + return handle_trap_exceptions(vcpu, run); case ARM_EXCEPTION_HYP_GONE: /* * EL2 has been reset to the hyp-stub. This happens when a guest