From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v1 1/2] KVM: arm64: handle single-stepping trapped instructions Date: Fri, 13 Oct 2017 10:15:09 +0100 Message-ID: <87r2u79zoi.fsf@linaro.org> References: <20171006113921.24880-1-alex.bennee@linaro.org> <20171006113921.24880-2-alex.bennee@linaro.org> <20171013082615.GC8927@cbox> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id E2B2549D24 for ; Fri, 13 Oct 2017 05:14:26 -0400 (EDT) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vsud9Xkp0ZYC for ; Fri, 13 Oct 2017 05:14:25 -0400 (EDT) Received: from mail-wm0-f51.google.com (mail-wm0-f51.google.com [74.125.82.51]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 99B8149D21 for ; Fri, 13 Oct 2017 05:14:25 -0400 (EDT) Received: by mail-wm0-f51.google.com with SMTP id q132so19932097wmd.2 for ; Fri, 13 Oct 2017 02:15:12 -0700 (PDT) In-reply-to: <20171013082615.GC8927@cbox> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Christoffer Dall Cc: kvm@vger.kernel.org, julien.thierry@arm.com, marc.zyngier@arm.com, Catalin Marinas , Will Deacon , open list , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu List-Id: kvmarm@lists.cs.columbia.edu CkNocmlzdG9mZmVyIERhbGwgPGNkYWxsQGxpbmFyby5vcmc+IHdyaXRlczoKCj4gT24gRnJpLCBP Y3QgMDYsIDIwMTcgYXQgMTI6Mzk6MjBQTSArMDEwMCwgQWxleCBCZW5uw6llIHdyb3RlOgo+PiBJ ZiB3ZSBhcmUgdXNpbmcgZ3Vlc3QgZGVidWcgdG8gc2luZ2xlLXN0ZXAgdGhlIGd1ZXN0IHdlIG5l ZWQgdG8gZW5zdXJlCj4+IHdlIGV4aXQgYWZ0ZXIgZW11bGF0aW5nIHRoZSBpbnN0cnVjdGlvbi4g VGhpcyBvbmx5IGFmZmVjdHMKPj4gaW5zdHJ1Y3Rpb25zIGNvbXBsZXRlbHkgZW11bGF0ZWQgYnkg dGhlIGtlcm5lbC4gRm9yIHVzZXJzcGFjZSBlbXVsYXRlZAo+PiBpbnN0cnVjdGlvbnMgd2UgbmVl ZCB0byBleGl0IGFuZCByZXR1cm4gdG8gY29tcGxldGUgdGhlIGVtdWxhdGlvbi4KPj4KPj4gV2Ug ZmFrZSBkZWJ1Zy5hcmNoLmhzciB0byBjb250YWluIEVTUl9FTHhfRUNfU09GVFNUUF9MT1cgc28g UUVNVSBrbm93cwo+PiBpdCB3YXMgYSBzaW5nbGUtc3RlcCBldmVudCAoYW5kIHdpdGhvdXQgYWx0 ZXJpbmcgdGhlIHVzZXJzcGFjZSBBQkkpLgo+Pgo+PiBTaWduZWQtb2ZmLWJ5OiBBbGV4IEJlbm7D qWUgPGFsZXguYmVubmVlQGxpbmFyby5vcmc+Cj4+IC0tLQo+PiAgYXJjaC9hcm02NC9rdm0vaGFu ZGxlX2V4aXQuYyB8IDQ4ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKystLS0tLS0tLS0t LS0tCj4+ICAxIGZpbGUgY2hhbmdlZCwgMzQgaW5zZXJ0aW9ucygrKSwgMTQgZGVsZXRpb25zKC0p Cj4+Cj4+IGRpZmYgLS1naXQgYS9hcmNoL2FybTY0L2t2bS9oYW5kbGVfZXhpdC5jIGIvYXJjaC9h cm02NC9rdm0vaGFuZGxlX2V4aXQuYwo+PiBpbmRleCA3ZGViYjc0ODQzYTAuLmM5MThkMjkxY2I1 OCAxMDA2NDQKPj4gLS0tIGEvYXJjaC9hcm02NC9rdm0vaGFuZGxlX2V4aXQuYwo+PiArKysgYi9h cmNoL2FybTY0L2t2bS9oYW5kbGVfZXhpdC5jCj4+IEBAIC0xNzgsNiArMTc4LDM5IEBAIHN0YXRp YyBleGl0X2hhbmRsZV9mbiBrdm1fZ2V0X2V4aXRfaGFuZGxlcihzdHJ1Y3Qga3ZtX3ZjcHUgKnZj cHUpCj4+ICAJcmV0dXJuIGFybV9leGl0X2hhbmRsZXJzW2hzcl9lY107Cj4+ICB9Cj4+Cj4+ICsv Kgo+PiArICogV2hlbiBoYW5kbGluZyB0cmFwcyB3ZSBuZWVkIHRvIGVuc3VyZSBleGl0IHRoZSBn dWVzdCBpZiB3ZQo+PiArICogY29tcGxldGVseSBlbXVsYXRlZCB0aGUgaW5zdHJ1Y3Rpb24gd2hp bGUgc2luZ2xlLXN0ZXBwaW5nLiBTdHVmZiB0bwo+PiArICogYmUgZW11bGF0ZWQgaW4gdXNlcnNw YWNlIG5lZWRzIHRvIGNvbXBsZXRlIHRoYXQgZmlyc3QuCj4+ICsgKi8KPgo+IEkgcmVhbGx5IGRv bid0IHVuZGVyc3RhbmQgdGhlIGZpcnN0IHNlbnRlbmNlIGhlcmUuICBXZSBhcmUgYWxyZWFkeSBv dXQKPiBvZiB0aGUgZ3Vlc3QsIHNvIGRvIHlvdSBtZWFuIGEgcmV0dXJuIHRvIHVzZXJzcGFjZT8K PiBJIHRoaW5rIHRoZSBzZWNvbmQgc2VudGVuY2UgY291bGQgYmUgbW9yZSBjbGVhciBhcyB3ZWxs LiAgSXMgJ3N0dWZmJyBub3QKPiBhY3R1YWxseSAnTU1JTyBlbXVsYXRpb24nIG9yICdlbXVsYXRp b24nIG1vcmUgYnJvYWRseT8KCllvdXIgcmlnaHQgLSBpdCdzIHNsb3BwaWx5IHdvcmRlZCBob3cg YWJvdXQ6CgogLyoKICAqIFdlIG1heSBiZSBzaW5nbGUtc3RlcHBpbmcgYW4gZW11bGF0ZWQgaW5z dHJ1Y3Rpb24uIElmIHRoZSBlbXVsYXRpb24KICAqIGhhcyBiZWVuIGNvbXBsZXRlZCBpbi1rZXJu ZWwgd2UgY2FuIHJldHVybiB0byB1c2Vyc3BhY2Ugd2l0aCBhCiAgKiBLVk1fRVhJVF9ERUJVRywg b3RoZXJ3aXNlIHRoZSB1c2Vyc3BhY2UgbmVlZHMgdG8gY29tcGxldGUgaXQncwogICogZW11bGF0 aW9uIGZpcnN0LgogICovCgpGb3IgeDg2IHRoZXJlIGlzIGFsc28gSU8gZW11bGF0aW9uIGJ1dCBp biBwcmluY2lwbGUgYW55dGhpbmcgdGhhdCBtaWdodApiZSBwYXNzZWQgb2ZmIHRvIHVzZXJzcGFj ZSB0byBiZSBjb21wbGV0ZWQgc2hvdWxkIGJlIGRvbmUgZmlyc3QuCgo+Cj4+ICsKPj4gK3N0YXRp YyBpbnQgaGFuZGxlX3RyYXBfZXhjZXB0aW9ucyhzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUsIHN0cnVj dCBrdm1fcnVuICpydW4pCj4+ICt7Cj4+ICsJaW50IGhhbmRsZWQ7Cj4+ICsKPj4gKwkvKgo+PiAr CSAqIFNlZSBBUk0gQVJNIEIxLjE0LjE6ICJIeXAgdHJhcHMgb24gaW5zdHJ1Y3Rpb25zCj4+ICsJ ICogdGhhdCBmYWlsIHRoZWlyIGNvbmRpdGlvbiBjb2RlIGNoZWNrIgo+PiArCSAqLwo+PiArCWlm ICgha3ZtX2NvbmRpdGlvbl92YWxpZCh2Y3B1KSkgewo+PiArCQlrdm1fc2tpcF9pbnN0cih2Y3B1 LCBrdm1fdmNwdV90cmFwX2lsX2lzMzJiaXQodmNwdSkpOwo+PiArCQloYW5kbGVkID0gMTsKPj4g Kwl9IGVsc2Ugewo+PiArCQlleGl0X2hhbmRsZV9mbiBleGl0X2hhbmRsZXI7Cj4+ICsKPj4gKwkJ ZXhpdF9oYW5kbGVyID0ga3ZtX2dldF9leGl0X2hhbmRsZXIodmNwdSk7Cj4+ICsJCWhhbmRsZWQg PSBleGl0X2hhbmRsZXIodmNwdSwgcnVuKTsKPj4gKwl9Cj4+ICsKPj4gKwlpZiAoaGFuZGxlZCAm JiAodmNwdS0+Z3Vlc3RfZGVidWcgJiBLVk1fR1VFU1REQkdfU0lOR0xFU1RFUCkpIHsKPgo+IERv bid0IHlvdSB3YW50IGlmIChoYW5kbGVkID09IDEpIG9yIGlmIChoYW5kbGVkID4gMCkgPwo+Cj4g SWYgdGhlcmUgd2FzIGFuIGVycm9yIEkgdGhpbmsgd2Ugd2FudCB0byBqdXN0IHJldHVybiB0aGF0 IHRvIHVzZXJzcGFjZQo+IGFuZCBub3Qgb3ZlcnJpZGUgaXQgYW5kIHByZXNlbnQgc2luZ2xlLXN0 ZXBwaW5nLgoKWWVzLCBJJ2xsIGZpeCBpdC4KCj4KPj4gKwkJaGFuZGxlZCA9IDA7Cj4+ICsJCXJ1 bi0+ZXhpdF9yZWFzb24gPSBLVk1fRVhJVF9ERUJVRzsKPj4gKwkJcnVuLT5kZWJ1Zy5hcmNoLmhz ciA9IEVTUl9FTHhfRUNfU09GVFNUUF9MT1cgPDwgRVNSX0VMeF9FQ19TSElGVDsKPj4gKwl9Cj4+ ICsKPj4gKwlyZXR1cm4gaGFuZGxlZDsKPj4gK30KPj4gKwo+PiAgLyoKPj4gICAqIFJldHVybiA+ IDAgdG8gcmV0dXJuIHRvIGd1ZXN0LCA8IDAgb24gZXJyb3IsIDAgKGFuZCBzZXQgZXhpdF9yZWFz b24pIG9uCj4+ICAgKiBwcm9wZXIgZXhpdCB0byB1c2Vyc3BhY2UuCj4+IEBAIC0xODUsOCArMjE4 LDYgQEAgc3RhdGljIGV4aXRfaGFuZGxlX2ZuIGt2bV9nZXRfZXhpdF9oYW5kbGVyKHN0cnVjdCBr dm1fdmNwdSAqdmNwdSkKPj4gIGludCBoYW5kbGVfZXhpdChzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUs IHN0cnVjdCBrdm1fcnVuICpydW4sCj4+ICAJCSAgICAgICBpbnQgZXhjZXB0aW9uX2luZGV4KQo+ PiAgewo+PiAtCWV4aXRfaGFuZGxlX2ZuIGV4aXRfaGFuZGxlcjsKPj4gLQo+PiAgCWlmIChBUk1f U0VSUk9SX1BFTkRJTkcoZXhjZXB0aW9uX2luZGV4KSkgewo+PiAgCQl1OCBoc3JfZWMgPSBFU1Jf RUx4X0VDKGt2bV92Y3B1X2dldF9oc3IodmNwdSkpOwo+Pgo+PiBAQCAtMjE0LDE4ICsyNDUsNyBA QCBpbnQgaGFuZGxlX2V4aXQoc3RydWN0IGt2bV92Y3B1ICp2Y3B1LCBzdHJ1Y3Qga3ZtX3J1biAq cnVuLAo+PiAgCQlrdm1faW5qZWN0X3ZhYnQodmNwdSk7Cj4+ICAJCXJldHVybiAxOwo+PiAgCWNh c2UgQVJNX0VYQ0VQVElPTl9UUkFQOgo+PiAtCQkvKgo+PiAtCQkgKiBTZWUgQVJNIEFSTSBCMS4x NC4xOiAiSHlwIHRyYXBzIG9uIGluc3RydWN0aW9ucwo+PiAtCQkgKiB0aGF0IGZhaWwgdGhlaXIg Y29uZGl0aW9uIGNvZGUgY2hlY2siCj4+IC0JCSAqLwo+PiAtCQlpZiAoIWt2bV9jb25kaXRpb25f dmFsaWQodmNwdSkpIHsKPj4gLQkJCWt2bV9za2lwX2luc3RyKHZjcHUsIGt2bV92Y3B1X3RyYXBf aWxfaXMzMmJpdCh2Y3B1KSk7Cj4+IC0JCQlyZXR1cm4gMTsKPj4gLQkJfQo+PiAtCj4+IC0JCWV4 aXRfaGFuZGxlciA9IGt2bV9nZXRfZXhpdF9oYW5kbGVyKHZjcHUpOwo+PiAtCj4+IC0JCXJldHVy biBleGl0X2hhbmRsZXIodmNwdSwgcnVuKTsKPj4gKwkJcmV0dXJuIGhhbmRsZV90cmFwX2V4Y2Vw dGlvbnModmNwdSwgcnVuKTsKPj4gIAljYXNlIEFSTV9FWENFUFRJT05fSFlQX0dPTkU6Cj4+ICAJ CS8qCj4+ICAJCSAqIEVMMiBoYXMgYmVlbiByZXNldCB0byB0aGUgaHlwLXN0dWIuIFRoaXMgaGFw cGVucyB3aGVuIGEgZ3Vlc3QKPj4gLS0KPj4gMi4xNC4xCj4+Cj4KPiBUaGFua3MsCj4gLUNocmlz dG9mZmVyCgoKLS0KQWxleCBCZW5uw6llCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fCmt2bWFybSBtYWlsaW5nIGxpc3QKa3ZtYXJtQGxpc3RzLmNzLmNvbHVt YmlhLmVkdQpodHRwczovL2xpc3RzLmNzLmNvbHVtYmlhLmVkdS9tYWlsbWFuL2xpc3RpbmZvL2t2 bWFybQo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Fri, 13 Oct 2017 10:15:09 +0100 Subject: [PATCH v1 1/2] KVM: arm64: handle single-stepping trapped instructions In-Reply-To: <20171013082615.GC8927@cbox> References: <20171006113921.24880-1-alex.bennee@linaro.org> <20171006113921.24880-2-alex.bennee@linaro.org> <20171013082615.GC8927@cbox> Message-ID: <87r2u79zoi.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Christoffer Dall writes: > On Fri, Oct 06, 2017 at 12:39:20PM +0100, Alex Benn?e wrote: >> 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 completely emulated by the kernel. For userspace emulated >> instructions we need to exit and return to complete the emulation. >> >> We fake debug.arch.hsr to contain ESR_ELx_EC_SOFTSTP_LOW so QEMU knows >> it was a single-step event (and without altering the userspace ABI). >> >> Signed-off-by: Alex Benn?e >> --- >> arch/arm64/kvm/handle_exit.c | 48 +++++++++++++++++++++++++++++++------------- >> 1 file changed, 34 insertions(+), 14 deletions(-) >> >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >> index 7debb74843a0..c918d291cb58 100644 >> --- a/arch/arm64/kvm/handle_exit.c >> +++ b/arch/arm64/kvm/handle_exit.c >> @@ -178,6 +178,39 @@ 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 >> + * completely emulated the instruction while single-stepping. Stuff to >> + * be emulated in userspace needs to complete that first. >> + */ > > I really don't understand the first sentence here. We are already out > of the guest, so do you mean a return to userspace? > I think the second sentence could be more clear as well. Is 'stuff' not > actually 'MMIO emulation' or 'emulation' more broadly? Your right - it's sloppily worded how about: /* * We may be single-stepping an emulated instruction. If the emulation * has been completed in-kernel we can return to userspace with a * KVM_EXIT_DEBUG, otherwise the userspace needs to complete it's * emulation first. */ For x86 there is also IO emulation but in principle anything that might be passed off to userspace to be completed should be done first. > >> + >> +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->guest_debug & KVM_GUESTDBG_SINGLESTEP)) { > > Don't you want if (handled == 1) or if (handled > 0) ? > > If there was an error I think we want to just return that to userspace > and not override it and present single-stepping. Yes, I'll fix it. > >> + handled = 0; >> + run->exit_reason = KVM_EXIT_DEBUG; >> + run->debug.arch.hsr = ESR_ELx_EC_SOFTSTP_LOW << ESR_ELx_EC_SHIFT; >> + } >> + >> + return handled; >> +} >> + >> /* >> * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on >> * proper exit to userspace. >> @@ -185,8 +218,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 +245,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 >> -- >> 2.14.1 >> > > Thanks, > -Christoffer -- Alex Benn?e From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758187AbdJMJPR (ORCPT ); Fri, 13 Oct 2017 05:15:17 -0400 Received: from mail-wm0-f51.google.com ([74.125.82.51]:51884 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757014AbdJMJPM (ORCPT ); Fri, 13 Oct 2017 05:15:12 -0400 X-Google-Smtp-Source: AOwi7QAc7kmwOkbNb3XBQAHPTpihfk6EbFCAZTJKqQUarC29uV3RH2BE3zZF6w5iEC3lD5906CHkeQ== References: <20171006113921.24880-1-alex.bennee@linaro.org> <20171006113921.24880-2-alex.bennee@linaro.org> <20171013082615.GC8927@cbox> User-agent: mu4e 0.9.19; emacs 26.0.60 From: Alex =?utf-8?Q?Benn=C3=A9e?= To: Christoffer Dall Cc: julien.thierry@arm.com, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org, marc.zyngier@arm.com, Catalin Marinas , Will Deacon , open list Subject: Re: [PATCH v1 1/2] KVM: arm64: handle single-stepping trapped instructions In-reply-to: <20171013082615.GC8927@cbox> Date: Fri, 13 Oct 2017 10:15:09 +0100 Message-ID: <87r2u79zoi.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v9D9FNZ8020086 Christoffer Dall writes: > On Fri, Oct 06, 2017 at 12:39:20PM +0100, Alex Bennée wrote: >> 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 completely emulated by the kernel. For userspace emulated >> instructions we need to exit and return to complete the emulation. >> >> We fake debug.arch.hsr to contain ESR_ELx_EC_SOFTSTP_LOW so QEMU knows >> it was a single-step event (and without altering the userspace ABI). >> >> Signed-off-by: Alex Bennée >> --- >> arch/arm64/kvm/handle_exit.c | 48 +++++++++++++++++++++++++++++++------------- >> 1 file changed, 34 insertions(+), 14 deletions(-) >> >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >> index 7debb74843a0..c918d291cb58 100644 >> --- a/arch/arm64/kvm/handle_exit.c >> +++ b/arch/arm64/kvm/handle_exit.c >> @@ -178,6 +178,39 @@ 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 >> + * completely emulated the instruction while single-stepping. Stuff to >> + * be emulated in userspace needs to complete that first. >> + */ > > I really don't understand the first sentence here. We are already out > of the guest, so do you mean a return to userspace? > I think the second sentence could be more clear as well. Is 'stuff' not > actually 'MMIO emulation' or 'emulation' more broadly? Your right - it's sloppily worded how about: /* * We may be single-stepping an emulated instruction. If the emulation * has been completed in-kernel we can return to userspace with a * KVM_EXIT_DEBUG, otherwise the userspace needs to complete it's * emulation first. */ For x86 there is also IO emulation but in principle anything that might be passed off to userspace to be completed should be done first. > >> + >> +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->guest_debug & KVM_GUESTDBG_SINGLESTEP)) { > > Don't you want if (handled == 1) or if (handled > 0) ? > > If there was an error I think we want to just return that to userspace > and not override it and present single-stepping. Yes, I'll fix it. > >> + handled = 0; >> + run->exit_reason = KVM_EXIT_DEBUG; >> + run->debug.arch.hsr = ESR_ELx_EC_SOFTSTP_LOW << ESR_ELx_EC_SHIFT; >> + } >> + >> + return handled; >> +} >> + >> /* >> * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on >> * proper exit to userspace. >> @@ -185,8 +218,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 +245,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 >> -- >> 2.14.1 >> > > Thanks, > -Christoffer -- Alex Bennée