From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S965859AbbDWO1A (ORCPT <rfc822;w@1wt.eu>);
	Thu, 23 Apr 2015 10:27:00 -0400
Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:58015
	"EHLO socrates.bennee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S964779AbbDWO05 (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 23 Apr 2015 10:26:57 -0400
References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-7-git-send-email-alex.bennee@linaro.org> <20150414082558.GS6186@cbox>
From: Alex =?utf-8?Q?Benn=C3=A9e?= <alex.bennee@linaro.org>
To: Christoffer Dall <christoffer.dall@linaro.org>
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com,
        peter.maydell@linaro.org, agraf@suse.de, drjones@redhat.com,
        pbonzini@redhat.com, zhichao.huang@linaro.org, jan.kiszka@siemens.com,
        dahi@linux.vnet.ibm.com, r65777@freescale.com, bp@suse.de,
        Gleb Natapov <gleb@kernel.org>, Jonathan Corbet <corbet@lwn.net>,
        Russell King <linux@arm.linux.org.uk>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        "open list\:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support
In-reply-to: <20150414082558.GS6186@cbox>
Date: Thu, 23 Apr 2015 15:26:53 +0100
Message-ID: <87y4li6hua.fsf@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
X-SA-Exim-Connect-IP: 127.0.0.1
X-SA-Exim-Mail-From: alex.bennee@linaro.org
X-SA-Exim-Scanned: No (on socrates.bennee.com); SAEximRunCond expanded to false
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Christoffer Dall <christoffer.dall@linaro.org> writes:

> On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Bennée wrote:
>> This adds support for SW breakpoints inserted by userspace.
>> 
>> We do this by trapping all BKPT exceptions in the
>> hypervisor (MDCR_EL2_TDE).
>
> you mean trapping all exceptions in the guest to the hypervisor?
>
>> The kvm_debug_exit_arch carries the address
>> of the exception.
>
> why?  can userspace not simply read out the PC using GET_ONE_REG?

Yes, I have re-worded and removed PC from the debug information.

<snip>
>>  
>> +	/* Trap breakpoints? */
>> +	if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
>> +		vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
>> +	else
>> +		vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
>
> so now you're trapping all debug exceptions, right?
>
> what happens if the guest is using the hardware to debug debug stuff and
> generates other kinds of debug exceptions, like a hardware breakpoint,
> will we not see an unhandled exception and the guest being forcefully
> killed?

Yes until the later patches which stop the guest using HW debug
registers while we are using them.

>
>> +
>>  }
>>  
>>  void kvm_arch_clear_debug(struct kvm_vcpu *vcpu)
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 524fa25..ed1bbb4 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -82,6 +82,37 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>  	return 1;
>>  }
>>  
>> +/**
>> + * kvm_handle_debug_exception - handle a debug exception instruction
>
> handle a software breadkpoint exception
>
>> + *
>> + * @vcpu:	the vcpu pointer
>> + * @run:	access to the kvm_run structure for results
>> + *
>> + * We route all debug exceptions through the same handler as we
>
> all debug exceptions?  software breakpoints and all?  then why the above
> shot text?
>
>> + * just need to report the PC and the HSR values to userspace.
>> + * Userspace may decide to re-inject the exception and deliver it to
>> + * the guest if it wasn't for the host to deal with.
>
> now I'm confused - does userspace setup the guest to receive an
> exception or does it tell KVM to emulate an exception for the guest or
> do we execute the breakpoint without trapping the debug exception?

I've made it all go through userspace as we may have to translate the
hypervisor visible exception code to what the guest was expecting to see.

>
>> + */
>> +static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> +{
>> +	u32 hsr = kvm_vcpu_get_hsr(vcpu);
>> +
>> +	run->exit_reason = KVM_EXIT_DEBUG;
>> +	run->debug.arch.hsr = hsr;
>> +
>> +	switch (hsr >> ESR_ELx_EC_SHIFT) {
>> +	case ESR_ELx_EC_BKPT32:
>> +	case ESR_ELx_EC_BRK64:
>> +		run->debug.arch.pc = *vcpu_pc(vcpu);
>> +		break;
>> +	default:
>> +		kvm_err("%s: un-handled case hsr: %#08x\n",
>> +			__func__, (unsigned int) hsr);
>
> this should never happen right?

At the moment it could, at the end of the patch series we should cover
all the cases so it would indicate a bug. I've made it return an error
code so it fails hard as suggested by David.

-- 
Alex Bennée

From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alex =?utf-8?Q?Benn=C3=A9e?= <alex.bennee@linaro.org>
Subject: Re: [PATCH v2 06/10] KVM: arm64: guest debug,
 add SW break point support
Date: Thu, 23 Apr 2015 15:26:53 +0100
Message-ID: <87y4li6hua.fsf@linaro.org>
References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org>
 <1427814488-28467-7-git-send-email-alex.bennee@linaro.org>
 <20150414082558.GS6186@cbox>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Cc: Russell King <linux@arm.linux.org.uk>, kvm@vger.kernel.org,
 Jonathan Corbet <corbet@lwn.net>, marc.zyngier@arm.com, jan.kiszka@siemens.com,
 "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
 Will Deacon <will.deacon@arm.com>, open list <linux-kernel@vger.kernel.org>,
 dahi@linux.vnet.ibm.com, Catalin Marinas <catalin.marinas@arm.com>,
 zhichao.huang@linaro.org, r65777@freescale.com, pbonzini@redhat.com,
 bp@suse.de, Gleb Natapov <gleb@kernel.org>, kvmarm@lists.cs.columbia.edu,
 linux-arm-kernel@lists.infradead.org
To: Christoffer Dall <christoffer.dall@linaro.org>
Return-path: <kvmarm-bounces@lists.cs.columbia.edu>
In-reply-to: <20150414082558.GS6186@cbox>
List-Unsubscribe: <https://lists.cs.columbia.edu/mailman/options/kvmarm>,
 <mailto:kvmarm-request@lists.cs.columbia.edu?subject=unsubscribe>
List-Archive: <https://lists.cs.columbia.edu/pipermail/kvmarm>
List-Post: <mailto:kvmarm@lists.cs.columbia.edu>
List-Help: <mailto:kvmarm-request@lists.cs.columbia.edu?subject=help>
List-Subscribe: <https://lists.cs.columbia.edu/mailman/listinfo/kvmarm>,
 <mailto:kvmarm-request@lists.cs.columbia.edu?subject=subscribe>
Errors-To: kvmarm-bounces@lists.cs.columbia.edu
Sender: kvmarm-bounces@lists.cs.columbia.edu
List-Id: kvm.vger.kernel.org

CkNocmlzdG9mZmVyIERhbGwgPGNocmlzdG9mZmVyLmRhbGxAbGluYXJvLm9yZz4gd3JpdGVzOgoK
PiBPbiBUdWUsIE1hciAzMSwgMjAxNSBhdCAwNDowODowNFBNICswMTAwLCBBbGV4IEJlbm7DqWUg
d3JvdGU6Cj4+IFRoaXMgYWRkcyBzdXBwb3J0IGZvciBTVyBicmVha3BvaW50cyBpbnNlcnRlZCBi
eSB1c2Vyc3BhY2UuCj4+IAo+PiBXZSBkbyB0aGlzIGJ5IHRyYXBwaW5nIGFsbCBCS1BUIGV4Y2Vw
dGlvbnMgaW4gdGhlCj4+IGh5cGVydmlzb3IgKE1EQ1JfRUwyX1RERSkuCj4KPiB5b3UgbWVhbiB0
cmFwcGluZyBhbGwgZXhjZXB0aW9ucyBpbiB0aGUgZ3Vlc3QgdG8gdGhlIGh5cGVydmlzb3I/Cj4K
Pj4gVGhlIGt2bV9kZWJ1Z19leGl0X2FyY2ggY2FycmllcyB0aGUgYWRkcmVzcwo+PiBvZiB0aGUg
ZXhjZXB0aW9uLgo+Cj4gd2h5PyAgY2FuIHVzZXJzcGFjZSBub3Qgc2ltcGx5IHJlYWQgb3V0IHRo
ZSBQQyB1c2luZyBHRVRfT05FX1JFRz8KClllcywgSSBoYXZlIHJlLXdvcmRlZCBhbmQgcmVtb3Zl
ZCBQQyBmcm9tIHRoZSBkZWJ1ZyBpbmZvcm1hdGlvbi4KCjxzbmlwPgo+PiAgCj4+ICsJLyogVHJh
cCBicmVha3BvaW50cz8gKi8KPj4gKwlpZiAodmNwdS0+Z3Vlc3RfZGVidWcgJiBLVk1fR1VFU1RE
QkdfVVNFX1NXX0JQKQo+PiArCQl2Y3B1LT5hcmNoLm1kY3JfZWwyIHw9IE1EQ1JfRUwyX1RERTsK
Pj4gKwllbHNlCj4+ICsJCXZjcHUtPmFyY2gubWRjcl9lbDIgJj0gfk1EQ1JfRUwyX1RERTsKPgo+
IHNvIG5vdyB5b3UncmUgdHJhcHBpbmcgYWxsIGRlYnVnIGV4Y2VwdGlvbnMsIHJpZ2h0Pwo+Cj4g
d2hhdCBoYXBwZW5zIGlmIHRoZSBndWVzdCBpcyB1c2luZyB0aGUgaGFyZHdhcmUgdG8gZGVidWcg
ZGVidWcgc3R1ZmYgYW5kCj4gZ2VuZXJhdGVzIG90aGVyIGtpbmRzIG9mIGRlYnVnIGV4Y2VwdGlv
bnMsIGxpa2UgYSBoYXJkd2FyZSBicmVha3BvaW50LAo+IHdpbGwgd2Ugbm90IHNlZSBhbiB1bmhh
bmRsZWQgZXhjZXB0aW9uIGFuZCB0aGUgZ3Vlc3QgYmVpbmcgZm9yY2VmdWxseQo+IGtpbGxlZD8K
ClllcyB1bnRpbCB0aGUgbGF0ZXIgcGF0Y2hlcyB3aGljaCBzdG9wIHRoZSBndWVzdCB1c2luZyBI
VyBkZWJ1ZwpyZWdpc3RlcnMgd2hpbGUgd2UgYXJlIHVzaW5nIHRoZW0uCgo+Cj4+ICsKPj4gIH0K
Pj4gIAo+PiAgdm9pZCBrdm1fYXJjaF9jbGVhcl9kZWJ1ZyhzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUp
Cj4+IGRpZmYgLS1naXQgYS9hcmNoL2FybTY0L2t2bS9oYW5kbGVfZXhpdC5jIGIvYXJjaC9hcm02
NC9rdm0vaGFuZGxlX2V4aXQuYwo+PiBpbmRleCA1MjRmYTI1Li5lZDFiYmI0IDEwMDY0NAo+PiAt
LS0gYS9hcmNoL2FybTY0L2t2bS9oYW5kbGVfZXhpdC5jCj4+ICsrKyBiL2FyY2gvYXJtNjQva3Zt
L2hhbmRsZV9leGl0LmMKPj4gQEAgLTgyLDYgKzgyLDM3IEBAIHN0YXRpYyBpbnQga3ZtX2hhbmRs
ZV93Zngoc3RydWN0IGt2bV92Y3B1ICp2Y3B1LCBzdHJ1Y3Qga3ZtX3J1biAqcnVuKQo+PiAgCXJl
dHVybiAxOwo+PiAgfQo+PiAgCj4+ICsvKioKPj4gKyAqIGt2bV9oYW5kbGVfZGVidWdfZXhjZXB0
aW9uIC0gaGFuZGxlIGEgZGVidWcgZXhjZXB0aW9uIGluc3RydWN0aW9uCj4KPiBoYW5kbGUgYSBz
b2Z0d2FyZSBicmVhZGtwb2ludCBleGNlcHRpb24KPgo+PiArICoKPj4gKyAqIEB2Y3B1Ogl0aGUg
dmNwdSBwb2ludGVyCj4+ICsgKiBAcnVuOglhY2Nlc3MgdG8gdGhlIGt2bV9ydW4gc3RydWN0dXJl
IGZvciByZXN1bHRzCj4+ICsgKgo+PiArICogV2Ugcm91dGUgYWxsIGRlYnVnIGV4Y2VwdGlvbnMg
dGhyb3VnaCB0aGUgc2FtZSBoYW5kbGVyIGFzIHdlCj4KPiBhbGwgZGVidWcgZXhjZXB0aW9ucz8g
IHNvZnR3YXJlIGJyZWFrcG9pbnRzIGFuZCBhbGw/ICB0aGVuIHdoeSB0aGUgYWJvdmUKPiBzaG90
IHRleHQ/Cj4KPj4gKyAqIGp1c3QgbmVlZCB0byByZXBvcnQgdGhlIFBDIGFuZCB0aGUgSFNSIHZh
bHVlcyB0byB1c2Vyc3BhY2UuCj4+ICsgKiBVc2Vyc3BhY2UgbWF5IGRlY2lkZSB0byByZS1pbmpl
Y3QgdGhlIGV4Y2VwdGlvbiBhbmQgZGVsaXZlciBpdCB0bwo+PiArICogdGhlIGd1ZXN0IGlmIGl0
IHdhc24ndCBmb3IgdGhlIGhvc3QgdG8gZGVhbCB3aXRoLgo+Cj4gbm93IEknbSBjb25mdXNlZCAt
IGRvZXMgdXNlcnNwYWNlIHNldHVwIHRoZSBndWVzdCB0byByZWNlaXZlIGFuCj4gZXhjZXB0aW9u
IG9yIGRvZXMgaXQgdGVsbCBLVk0gdG8gZW11bGF0ZSBhbiBleGNlcHRpb24gZm9yIHRoZSBndWVz
dCBvcgo+IGRvIHdlIGV4ZWN1dGUgdGhlIGJyZWFrcG9pbnQgd2l0aG91dCB0cmFwcGluZyB0aGUg
ZGVidWcgZXhjZXB0aW9uPwoKSSd2ZSBtYWRlIGl0IGFsbCBnbyB0aHJvdWdoIHVzZXJzcGFjZSBh
cyB3ZSBtYXkgaGF2ZSB0byB0cmFuc2xhdGUgdGhlCmh5cGVydmlzb3IgdmlzaWJsZSBleGNlcHRp
b24gY29kZSB0byB3aGF0IHRoZSBndWVzdCB3YXMgZXhwZWN0aW5nIHRvIHNlZS4KCj4KPj4gKyAq
Lwo+PiArc3RhdGljIGludCBrdm1faGFuZGxlX2d1ZXN0X2RlYnVnKHN0cnVjdCBrdm1fdmNwdSAq
dmNwdSwgc3RydWN0IGt2bV9ydW4gKnJ1bikKPj4gK3sKPj4gKwl1MzIgaHNyID0ga3ZtX3ZjcHVf
Z2V0X2hzcih2Y3B1KTsKPj4gKwo+PiArCXJ1bi0+ZXhpdF9yZWFzb24gPSBLVk1fRVhJVF9ERUJV
RzsKPj4gKwlydW4tPmRlYnVnLmFyY2guaHNyID0gaHNyOwo+PiArCj4+ICsJc3dpdGNoIChoc3Ig
Pj4gRVNSX0VMeF9FQ19TSElGVCkgewo+PiArCWNhc2UgRVNSX0VMeF9FQ19CS1BUMzI6Cj4+ICsJ
Y2FzZSBFU1JfRUx4X0VDX0JSSzY0Ogo+PiArCQlydW4tPmRlYnVnLmFyY2gucGMgPSAqdmNwdV9w
Yyh2Y3B1KTsKPj4gKwkJYnJlYWs7Cj4+ICsJZGVmYXVsdDoKPj4gKwkJa3ZtX2VycigiJXM6IHVu
LWhhbmRsZWQgY2FzZSBoc3I6ICUjMDh4XG4iLAo+PiArCQkJX19mdW5jX18sICh1bnNpZ25lZCBp
bnQpIGhzcik7Cj4KPiB0aGlzIHNob3VsZCBuZXZlciBoYXBwZW4gcmlnaHQ/CgpBdCB0aGUgbW9t
ZW50IGl0IGNvdWxkLCBhdCB0aGUgZW5kIG9mIHRoZSBwYXRjaCBzZXJpZXMgd2Ugc2hvdWxkIGNv
dmVyCmFsbCB0aGUgY2FzZXMgc28gaXQgd291bGQgaW5kaWNhdGUgYSBidWcuIEkndmUgbWFkZSBp
dCByZXR1cm4gYW4gZXJyb3IKY29kZSBzbyBpdCBmYWlscyBoYXJkIGFzIHN1Z2dlc3RlZCBieSBE
YXZpZC4KCi0tIApBbGV4IEJlbm7DqWUKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX18Ka3ZtYXJtIG1haWxpbmcgbGlzdAprdm1hcm1AbGlzdHMuY3MuY29sdW1i
aWEuZWR1Cmh0dHBzOi8vbGlzdHMuY3MuY29sdW1iaWEuZWR1L21haWxtYW4vbGlzdGluZm8va3Zt
YXJtCg==

From mboxrd@z Thu Jan  1 00:00:00 1970
From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=)
Date: Thu, 23 Apr 2015 15:26:53 +0100
Subject: [PATCH v2 06/10] KVM: arm64: guest debug,
 add SW break point support
In-Reply-To: <20150414082558.GS6186@cbox>
References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org>
 <1427814488-28467-7-git-send-email-alex.bennee@linaro.org>
 <20150414082558.GS6186@cbox>
Message-ID: <87y4li6hua.fsf@linaro.org>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org


Christoffer Dall <christoffer.dall@linaro.org> writes:

> On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Benn?e wrote:
>> This adds support for SW breakpoints inserted by userspace.
>> 
>> We do this by trapping all BKPT exceptions in the
>> hypervisor (MDCR_EL2_TDE).
>
> you mean trapping all exceptions in the guest to the hypervisor?
>
>> The kvm_debug_exit_arch carries the address
>> of the exception.
>
> why?  can userspace not simply read out the PC using GET_ONE_REG?

Yes, I have re-worded and removed PC from the debug information.

<snip>
>>  
>> +	/* Trap breakpoints? */
>> +	if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
>> +		vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
>> +	else
>> +		vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
>
> so now you're trapping all debug exceptions, right?
>
> what happens if the guest is using the hardware to debug debug stuff and
> generates other kinds of debug exceptions, like a hardware breakpoint,
> will we not see an unhandled exception and the guest being forcefully
> killed?

Yes until the later patches which stop the guest using HW debug
registers while we are using them.

>
>> +
>>  }
>>  
>>  void kvm_arch_clear_debug(struct kvm_vcpu *vcpu)
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 524fa25..ed1bbb4 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -82,6 +82,37 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>  	return 1;
>>  }
>>  
>> +/**
>> + * kvm_handle_debug_exception - handle a debug exception instruction
>
> handle a software breadkpoint exception
>
>> + *
>> + * @vcpu:	the vcpu pointer
>> + * @run:	access to the kvm_run structure for results
>> + *
>> + * We route all debug exceptions through the same handler as we
>
> all debug exceptions?  software breakpoints and all?  then why the above
> shot text?
>
>> + * just need to report the PC and the HSR values to userspace.
>> + * Userspace may decide to re-inject the exception and deliver it to
>> + * the guest if it wasn't for the host to deal with.
>
> now I'm confused - does userspace setup the guest to receive an
> exception or does it tell KVM to emulate an exception for the guest or
> do we execute the breakpoint without trapping the debug exception?

I've made it all go through userspace as we may have to translate the
hypervisor visible exception code to what the guest was expecting to see.

>
>> + */
>> +static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> +{
>> +	u32 hsr = kvm_vcpu_get_hsr(vcpu);
>> +
>> +	run->exit_reason = KVM_EXIT_DEBUG;
>> +	run->debug.arch.hsr = hsr;
>> +
>> +	switch (hsr >> ESR_ELx_EC_SHIFT) {
>> +	case ESR_ELx_EC_BKPT32:
>> +	case ESR_ELx_EC_BRK64:
>> +		run->debug.arch.pc = *vcpu_pc(vcpu);
>> +		break;
>> +	default:
>> +		kvm_err("%s: un-handled case hsr: %#08x\n",
>> +			__func__, (unsigned int) hsr);
>
> this should never happen right?

At the moment it could, at the end of the patch series we should cover
all the cases so it would indicate a bug. I've made it return an error
code so it fails hard as suggested by David.

-- 
Alex Benn?e