From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v5 08/12] KVM: arm64: re-factor hyp.S debug register code Date: Thu, 04 Jun 2015 11:34:46 +0100 Message-ID: <87d21bbw3d.fsf@linaro.org> References: <1432891828-4816-1-git-send-email-alex.bennee@linaro.org> <1432891828-4816-9-git-send-email-alex.bennee@linaro.org> <20150604102308.GD7657@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 1E5AA52EDB for ; Thu, 4 Jun 2015 06:24:36 -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 LB9oN2qAmVxa for ; Thu, 4 Jun 2015 06:24:34 -0400 (EDT) Received: from socrates.bennee.com (static.88-198-71-155.clients.your-server.de [88.198.71.155]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id A2DAB52ED4 for ; Thu, 4 Jun 2015 06:24:34 -0400 (EDT) In-reply-to: <20150604102308.GD7657@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: Catalin Marinas , kvm@vger.kernel.org, marc.zyngier@arm.com, jan.kiszka@siemens.com, Will Deacon , open list , dahi@linux.vnet.ibm.com, zhichao.huang@linaro.org, r65777@freescale.com, pbonzini@redhat.com, bp@suse.de, Gleb Natapov , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu CkNocmlzdG9mZmVyIERhbGwgPGNocmlzdG9mZmVyLmRhbGxAbGluYXJvLm9yZz4gd3JpdGVzOgoK PiBPbiBGcmksIE1heSAyOSwgMjAxNSBhdCAxMDozMDoyNEFNICswMTAwLCBBbGV4IEJlbm7DqWUg d3JvdGU6Cj4+IFRoaXMgaXMgYSBwcmUtY3Vyc29yIHRvIHNoYXJpbmcgdGhlIGNvZGUgd2l0aCB0 aGUgZ3Vlc3QgZGVidWcgc3VwcG9ydC4KPj4gVGhpcyByZXBsYWNlcyB0aGUgYmlnIG1hY3JvIHRo YXQgZmlzaGVzIGRhdGEgb3V0IG9mIGEgZml4ZWQgbG9jYXRpb24KPj4gd2l0aCBhIG1vcmUgZ2Vu ZXJhbCBoZWxwZXIgbWFjcm8gdG8gcmVzdG9yZSBhIHNldCBvZiBkZWJ1ZyByZWdpc3RlcnMuIEl0 Cj4+IHVzZXMgbWFjcm8gc3Vic3RpdHV0aW9uIHNvIGl0IGNhbiBiZSByZS11c2VkIGZvciBkZWJ1 ZyBjb250cm9sIGFuZCB2YWx1ZQo+PiByZWdpc3RlcnMuIEl0IGRvZXMgaG93ZXZlciByZWx5IG9u IHRoZSBkZWJ1ZyByZWdpc3RlcnMgYmVpbmcgNjQgYml0Cj4+IGFsaWduZWQgKGFzIHRoZXkgaGFw cGVuIHRvIGJlIGluIHRoZSBoeXAgQUJJKS4KCk9vcHMgSSdkIGJldHRlciBmaXggdGhhdCBjb21t aXQgY29tbWVudC4KCj4+IAo+PiBTaWduZWQtb2ZmLWJ5OiBBbGV4IEJlbm7DqWUgPGFsZXguYmVu bmVlQGxpbmFyby5vcmc+Cj4+IAo+PiAtLS0KPj4gdjM6Cj4+ICAgLSByZXR1cm4gdG8gdGhlIHBh dGNoIHNlcmllcwo+PiAgIC0gYWRkIHNhdmUgYW5kIHJlc3RvcmUgdGFyZ2V0cwo+PiAgIC0gY2hh bmdlIHJlZ2lzdGVyIHVzZSBhbmQgZG9jdW1lbnQKPj4gdjQ6Cj4+ICAgLSBrZWVwIG9yaWdpbmFs IHNldHVwL3Jlc3RvcmUgbmFtZXMKPj4gICAtIGRvbid0IHVzZSBzcGxpdCB1MzIvdTY0IHN0cnVj dHVyZSB5ZXQKPj4gLS0tCj4+ICBhcmNoL2FybTY0L2t2bS9oeXAuUyB8IDUxOSArKysrKysrKysr KysrKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPj4gIDEgZmlsZSBjaGFu Z2VkLCAxNDAgaW5zZXJ0aW9ucygrKSwgMzc5IGRlbGV0aW9ucygtKQo+PiAKPj4gZGlmZiAtLWdp dCBhL2FyY2gvYXJtNjQva3ZtL2h5cC5TIGIvYXJjaC9hcm02NC9rdm0vaHlwLlMKPj4gaW5kZXgg NzRlNjNkOC4uOWM0ODk3ZCAxMDA2NDQKPj4gLS0tIGEvYXJjaC9hcm02NC9rdm0vaHlwLlMKPj4g KysrIGIvYXJjaC9hcm02NC9rdm0vaHlwLlMKPgo+Cj4gWy4uLl0KPgo+PiBAQCAtNDY1LDE5NSAr MzE4LDUyIEBACj4+ICAJbXNyCW1kc2NyX2VsMSwJeDI1Cj4+ICAuZW5kbQo+PiAgCj4+IC0ubWFj cm8gcmVzdG9yZV9kZWJ1Zwo+PiAtCS8vIHgyOiBiYXNlIGFkZHJlc3MgZm9yIGNwdSBjb250ZXh0 Cj4+IC0JLy8geDM6IHRtcCByZWdpc3Rlcgo+PiAtCj4+IC0JbXJzCXgyNiwgaWRfYWE2NGRmcjBf ZWwxCj4+IC0JdWJmeAl4MjQsIHgyNiwgIzEyLCAjNAkvLyBFeHRyYWN0IEJSUHMKPj4gLQl1YmZ4 CXgyNSwgeDI2LCAjMjAsICM0CS8vIEV4dHJhY3QgV1JQcwo+PiAtCW1vdgl3MjYsICMxNQo+PiAt CXN1Ygl3MjQsIHcyNiwgdzI0CQkvLyBIb3cgbWFueSBCUHMgdG8gc2tpcAo+PiAtCXN1Ygl3MjUs IHcyNiwgdzI1CQkvLyBIb3cgbWFueSBXUHMgdG8gc2tpcAo+PiAtCj4+IC0JYWRkCXgzLCB4Miwg I0NQVV9TWVNSRUdfT0ZGU0VUKERCR0JDUjBfRUwxKQo+PiArLm1hY3JvIHJlc3RvcmVfZGVidWcg dHlwZQo+PiArICAgICAgICAvLyB4NDogcG9pbnRlciB0byByZWdpc3RlciBzZXQKPj4gKyAgICAg ICAgLy8geDU6IG51bWJlciBvZiByZWdpc3RlcnMgdG8gc2tpcAo+Cj4gbml0OiB1c2UgdGFicyBp bnN0ZWFkIG9mIHNwYWNlcyBoZXJlLi4uCj4KPj4gKwkvLyB4Ni4ueDIyIHRyYXNoZWQKPj4gIAo+ Cj4gWy4uLl0KPgo+PiBAQCAtODg3LDEyICs1OTcsNjMgQEAgX19yZXN0b3JlX3N5c3JlZ3M6Cj4+ ICAJcmVzdG9yZV9zeXNyZWdzCj4+ICAJcmV0Cj4+ICAKPj4gKy8qIFNhdmUgZGVidWcgc3RhdGUg Ki8KPj4gIF9fc2F2ZV9kZWJ1ZzoKPj4gLQlzYXZlX2RlYnVnCj4+ICsJLy8geDA6IGJhc2UgYWRk cmVzcyBmb3IgdmNwdSBjb250ZXh0Cj4+ICsJLy8geDI6IHB0ciB0byBjdXJyZW50IENQVSBjb250 ZXh0Cj4+ICsJLy8geDQveDU6IHRyYXNoZWQKPgo+IEkgaGFkIGEgYnVuY2ggb2YgcXVlc3Rpb25z IGhlcmUgd2hpY2ggSSB0aGluayB5b3UgbWlzc2VkIGxhc3QgdGltZQo+IGFyb3VuZDoKPiAgMS4g d2h5IGRvIHdlIG5lZWQgdGhlIHZjcHUgY29udGV4dD8KCldlIGRvbid0LCBJJ2xsIGRyb3AgdGhh dAoKPiAgMi4gd2hhdCBkb2VzICdjdXJyZW50JyBtZWFuIGhlcmU/CgpFaXRoZXIgdGhlIGhvc3Qg b3IgdmNwdSBjb250ZXh0IGRlcGVuZGluZyB3aGljaCB3YXkgd2UgYXJlIGN1cnJlbnRseSBnb2lu Zy4KCj4gIDMuIHlvdSdyZSBhbHNvIHRyYXNoaW5nIGV2ZXJ5dGhpbmcgdGhhdCBzYXZlX2RlYnVn IHRyYXNoZXMsIHNvIHg0LzUsCj4gICAgIHg2LXgyMiB0cmFzaGVkIHdvdWxkIGJlIG1vcmUgY29y cmVjdAoKT0sKCj4KPj4gKwo+PiArCW1ycwl4MjYsIGlkX2FhNjRkZnIwX2VsMQo+PiArCXViZngJ eDI0LCB4MjYsICMxMiwgIzQJLy8gRXh0cmFjdCBCUlBzCj4+ICsJdWJmeAl4MjUsIHgyNiwgIzIw LCAjNAkvLyBFeHRyYWN0IFdSUHMKPj4gKwltb3YJdzI2LCAjMTUKPj4gKwlzdWIJdzI0LCB3MjYs IHcyNAkJLy8gSG93IG1hbnkgQlBzIHRvIHNraXAKPj4gKwlzdWIJdzI1LCB3MjYsIHcyNQkJLy8g SG93IG1hbnkgV1BzIHRvIHNraXAKPj4gKwo+PiArCW1vdiAgICAgeDUsIHgyNAo+PiArCWFkZAl4 NCwgeDIsICNDUFVfU1lTUkVHX09GRlNFVChEQkdCQ1IwX0VMMSkKPj4gKwlzYXZlX2RlYnVnIGRi Z2Jjcgo+PiArCWFkZAl4NCwgeDIsICNDUFVfU1lTUkVHX09GRlNFVChEQkdCVlIwX0VMMSkKPj4g KwlzYXZlX2RlYnVnIGRiZ2J2cgo+PiArCj4+ICsJbW92ICAgICB4NSwgeDI1Cj4+ICsJYWRkCXg0 LCB4MiwgI0NQVV9TWVNSRUdfT0ZGU0VUKERCR1dDUjBfRUwxKQo+PiArCXNhdmVfZGVidWcgZGJn d2NyCj4+ICsJYWRkCXg0LCB4MiwgI0NQVV9TWVNSRUdfT0ZGU0VUKERCR1dWUjBfRUwxKQo+PiAr CXNhdmVfZGVidWcgZGJnd3ZyCj4+ICsKPj4gKwltcnMJeDIxLCBtZGNjaW50X2VsMQo+PiArCXN0 cgl4MjEsIFt4MiwgI0NQVV9TWVNSRUdfT0ZGU0VUKE1EQ0NJTlRfRUwxKV0KPj4gIAlyZXQKPj4g IAo+PiArLyogUmVzdG9yZSBkZWJ1ZyBzdGF0ZSAqLwo+PiAgX19yZXN0b3JlX2RlYnVnOgo+PiAt CXJlc3RvcmVfZGVidWcKPj4gKwkvLyB4MDogYmFzZSBhZGRyZXNzIGZvciBjcHUgY29udGV4dAo+ PiArCS8vIHgyOiBwdHIgdG8gY3VycmVudCBDUFUgY29udGV4dAo+PiArCS8vIHg0L3g1OiB0cmFz aGVkCj4KPiBhbmQgeW91IG1pc3NlZCB0aGVzZSBjb21tZW50cyB0b28sIGJhc2ljYWxseSBzYW1l IHN0dWZmLCBidXQgd2h5IHdhcyBpdAo+ICdjcHUnIGhlcmUgYW5kIG5vdCAndmNwdScgbGlrZSBh Ym92ZT8KCkFnYWluIHdlIHVzZSB0aGUgZnVuY3Rpb25zIGJvdGggZm9yIHJlc3RvcmluZyBob3N0 IGFuZCB2Y3B1IGRlYnVnIGNvbnRleHQuCgo+Cj4gbm90ZSBhZ2FpbiwgdGhhdCB5b3UncmUgZXhw bGljaXRseSB0b3VjaGluZyB4MjQsIHh4MjUsIGFuZCB4MjYgaGVyZSBhcwo+IHdlbGwsIHNvIHNo b3VsZG4ndCB0aGV5IGJlIGxpc3RlZCBhcyB0cmFzaGVkPwo+Cj4+ICsKPj4gKwltcnMJeDI2LCBp ZF9hYTY0ZGZyMF9lbDEKPj4gKwl1YmZ4CXgyNCwgeDI2LCAjMTIsICM0CS8vIEV4dHJhY3QgQlJQ cwo+PiArCXViZngJeDI1LCB4MjYsICMyMCwgIzQJLy8gRXh0cmFjdCBXUlBzCj4+ICsJbW92CXcy NiwgIzE1Cj4+ICsJc3ViCXcyNCwgdzI2LCB3MjQJCS8vIEhvdyBtYW55IEJQcyB0byBza2lwCj4+ ICsJc3ViCXcyNSwgdzI2LCB3MjUJCS8vIEhvdyBtYW55IFdQcyB0byBza2lwCj4+ICsKPj4gKwlt b3YgICAgIHg1LCB4MjQKPj4gKwlhZGQJeDQsIHgyLCAjQ1BVX1NZU1JFR19PRkZTRVQoREJHQkNS MF9FTDEpCj4+ICsJcmVzdG9yZV9kZWJ1ZyBkYmdiY3IKPj4gKwlhZGQJeDQsIHgyLCAjQ1BVX1NZ U1JFR19PRkZTRVQoREJHQlZSMF9FTDEpCj4+ICsJcmVzdG9yZV9kZWJ1ZyBkYmdidnIKPj4gKwo+ PiArCW1vdiAgICAgeDUsIHgyNQo+PiArCWFkZAl4NCwgeDIsICNDUFVfU1lTUkVHX09GRlNFVChE QkdXQ1IwX0VMMSkKPj4gKwlyZXN0b3JlX2RlYnVnIGRiZ3djcgo+PiArCWFkZAl4NCwgeDIsICND UFVfU1lTUkVHX09GRlNFVChEQkdXVlIwX0VMMSkKPj4gKwlyZXN0b3JlX2RlYnVnIGRiZ3d2cgo+ PiArCj4+ICsJbGRyCXgyMSwgW3gyLCAjQ1BVX1NZU1JFR19PRkZTRVQoTURDQ0lOVF9FTDEpXQo+ PiArCW1zcgltZGNjaW50X2VsMSwgeDIxCj4+ICsKPj4gIAlyZXQKPj4gIAo+PiAgX19zYXZlX2Zw c2ltZDoKPgo+IFRoYW5rcywKPiAtQ2hyaXN0b2ZmZXIKCi0tIApBbGV4IEJlbm7DqWUKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18Ka3ZtYXJtIG1haWxpbmcg bGlzdAprdm1hcm1AbGlzdHMuY3MuY29sdW1iaWEuZWR1Cmh0dHBzOi8vbGlzdHMuY3MuY29sdW1i aWEuZWR1L21haWxtYW4vbGlzdGluZm8va3ZtYXJtCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Thu, 04 Jun 2015 11:34:46 +0100 Subject: [PATCH v5 08/12] KVM: arm64: re-factor hyp.S debug register code In-Reply-To: <20150604102308.GD7657@cbox> References: <1432891828-4816-1-git-send-email-alex.bennee@linaro.org> <1432891828-4816-9-git-send-email-alex.bennee@linaro.org> <20150604102308.GD7657@cbox> Message-ID: <87d21bbw3d.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Christoffer Dall writes: > On Fri, May 29, 2015 at 10:30:24AM +0100, Alex Benn?e wrote: >> This is a pre-cursor to sharing the code with the guest debug support. >> This replaces the big macro that fishes data out of a fixed location >> with a more general helper macro to restore a set of debug registers. It >> uses macro substitution so it can be re-used for debug control and value >> registers. It does however rely on the debug registers being 64 bit >> aligned (as they happen to be in the hyp ABI). Oops I'd better fix that commit comment. >> >> Signed-off-by: Alex Benn?e >> >> --- >> v3: >> - return to the patch series >> - add save and restore targets >> - change register use and document >> v4: >> - keep original setup/restore names >> - don't use split u32/u64 structure yet >> --- >> arch/arm64/kvm/hyp.S | 519 ++++++++++++++------------------------------------- >> 1 file changed, 140 insertions(+), 379 deletions(-) >> >> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S >> index 74e63d8..9c4897d 100644 >> --- a/arch/arm64/kvm/hyp.S >> +++ b/arch/arm64/kvm/hyp.S > > > [...] > >> @@ -465,195 +318,52 @@ >> msr mdscr_el1, x25 >> .endm >> >> -.macro restore_debug >> - // x2: base address for cpu context >> - // x3: tmp register >> - >> - mrs x26, id_aa64dfr0_el1 >> - ubfx x24, x26, #12, #4 // Extract BRPs >> - ubfx x25, x26, #20, #4 // Extract WRPs >> - mov w26, #15 >> - sub w24, w26, w24 // How many BPs to skip >> - sub w25, w26, w25 // How many WPs to skip >> - >> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1) >> +.macro restore_debug type >> + // x4: pointer to register set >> + // x5: number of registers to skip > > nit: use tabs instead of spaces here... > >> + // x6..x22 trashed >> > > [...] > >> @@ -887,12 +597,63 @@ __restore_sysregs: >> restore_sysregs >> ret >> >> +/* Save debug state */ >> __save_debug: >> - save_debug >> + // x0: base address for vcpu context >> + // x2: ptr to current CPU context >> + // x4/x5: trashed > > I had a bunch of questions here which I think you missed last time > around: > 1. why do we need the vcpu context? We don't, I'll drop that > 2. what does 'current' mean here? Either the host or vcpu context depending which way we are currently going. > 3. you're also trashing everything that save_debug trashes, so x4/5, > x6-x22 trashed would be more correct OK > >> + >> + mrs x26, id_aa64dfr0_el1 >> + ubfx x24, x26, #12, #4 // Extract BRPs >> + ubfx x25, x26, #20, #4 // Extract WRPs >> + mov w26, #15 >> + sub w24, w26, w24 // How many BPs to skip >> + sub w25, w26, w25 // How many WPs to skip >> + >> + mov x5, x24 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1) >> + save_debug dbgbcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1) >> + save_debug dbgbvr >> + >> + mov x5, x25 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1) >> + save_debug dbgwcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1) >> + save_debug dbgwvr >> + >> + mrs x21, mdccint_el1 >> + str x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)] >> ret >> >> +/* Restore debug state */ >> __restore_debug: >> - restore_debug >> + // x0: base address for cpu context >> + // x2: ptr to current CPU context >> + // x4/x5: trashed > > and you missed these comments too, basically same stuff, but why was it > 'cpu' here and not 'vcpu' like above? Again we use the functions both for restoring host and vcpu debug context. > > note again, that you're explicitly touching x24, xx25, and x26 here as > well, so shouldn't they be listed as trashed? > >> + >> + mrs x26, id_aa64dfr0_el1 >> + ubfx x24, x26, #12, #4 // Extract BRPs >> + ubfx x25, x26, #20, #4 // Extract WRPs >> + mov w26, #15 >> + sub w24, w26, w24 // How many BPs to skip >> + sub w25, w26, w25 // How many WPs to skip >> + >> + mov x5, x24 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1) >> + restore_debug dbgbcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1) >> + restore_debug dbgbvr >> + >> + mov x5, x25 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1) >> + restore_debug dbgwcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1) >> + restore_debug dbgwvr >> + >> + ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)] >> + msr mdccint_el1, x21 >> + >> ret >> >> __save_fpsimd: > > 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 S1752870AbbFDKet (ORCPT ); Thu, 4 Jun 2015 06:34:49 -0400 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:40846 "EHLO socrates.bennee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751175AbbFDKek (ORCPT ); Thu, 4 Jun 2015 06:34:40 -0400 References: <1432891828-4816-1-git-send-email-alex.bennee@linaro.org> <1432891828-4816-9-git-send-email-alex.bennee@linaro.org> <20150604102308.GD7657@cbox> From: Alex =?utf-8?Q?Benn=C3=A9e?= To: Christoffer Dall 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 , Catalin Marinas , Will Deacon , open list Subject: Re: [PATCH v5 08/12] KVM: arm64: re-factor hyp.S debug register code In-reply-to: <20150604102308.GD7657@cbox> Date: Thu, 04 Jun 2015 11:34:46 +0100 Message-ID: <87d21bbw3d.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: X-Mailing-List: linux-kernel@vger.kernel.org Christoffer Dall writes: > On Fri, May 29, 2015 at 10:30:24AM +0100, Alex Bennée wrote: >> This is a pre-cursor to sharing the code with the guest debug support. >> This replaces the big macro that fishes data out of a fixed location >> with a more general helper macro to restore a set of debug registers. It >> uses macro substitution so it can be re-used for debug control and value >> registers. It does however rely on the debug registers being 64 bit >> aligned (as they happen to be in the hyp ABI). Oops I'd better fix that commit comment. >> >> Signed-off-by: Alex Bennée >> >> --- >> v3: >> - return to the patch series >> - add save and restore targets >> - change register use and document >> v4: >> - keep original setup/restore names >> - don't use split u32/u64 structure yet >> --- >> arch/arm64/kvm/hyp.S | 519 ++++++++++++++------------------------------------- >> 1 file changed, 140 insertions(+), 379 deletions(-) >> >> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S >> index 74e63d8..9c4897d 100644 >> --- a/arch/arm64/kvm/hyp.S >> +++ b/arch/arm64/kvm/hyp.S > > > [...] > >> @@ -465,195 +318,52 @@ >> msr mdscr_el1, x25 >> .endm >> >> -.macro restore_debug >> - // x2: base address for cpu context >> - // x3: tmp register >> - >> - mrs x26, id_aa64dfr0_el1 >> - ubfx x24, x26, #12, #4 // Extract BRPs >> - ubfx x25, x26, #20, #4 // Extract WRPs >> - mov w26, #15 >> - sub w24, w26, w24 // How many BPs to skip >> - sub w25, w26, w25 // How many WPs to skip >> - >> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1) >> +.macro restore_debug type >> + // x4: pointer to register set >> + // x5: number of registers to skip > > nit: use tabs instead of spaces here... > >> + // x6..x22 trashed >> > > [...] > >> @@ -887,12 +597,63 @@ __restore_sysregs: >> restore_sysregs >> ret >> >> +/* Save debug state */ >> __save_debug: >> - save_debug >> + // x0: base address for vcpu context >> + // x2: ptr to current CPU context >> + // x4/x5: trashed > > I had a bunch of questions here which I think you missed last time > around: > 1. why do we need the vcpu context? We don't, I'll drop that > 2. what does 'current' mean here? Either the host or vcpu context depending which way we are currently going. > 3. you're also trashing everything that save_debug trashes, so x4/5, > x6-x22 trashed would be more correct OK > >> + >> + mrs x26, id_aa64dfr0_el1 >> + ubfx x24, x26, #12, #4 // Extract BRPs >> + ubfx x25, x26, #20, #4 // Extract WRPs >> + mov w26, #15 >> + sub w24, w26, w24 // How many BPs to skip >> + sub w25, w26, w25 // How many WPs to skip >> + >> + mov x5, x24 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1) >> + save_debug dbgbcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1) >> + save_debug dbgbvr >> + >> + mov x5, x25 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1) >> + save_debug dbgwcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1) >> + save_debug dbgwvr >> + >> + mrs x21, mdccint_el1 >> + str x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)] >> ret >> >> +/* Restore debug state */ >> __restore_debug: >> - restore_debug >> + // x0: base address for cpu context >> + // x2: ptr to current CPU context >> + // x4/x5: trashed > > and you missed these comments too, basically same stuff, but why was it > 'cpu' here and not 'vcpu' like above? Again we use the functions both for restoring host and vcpu debug context. > > note again, that you're explicitly touching x24, xx25, and x26 here as > well, so shouldn't they be listed as trashed? > >> + >> + mrs x26, id_aa64dfr0_el1 >> + ubfx x24, x26, #12, #4 // Extract BRPs >> + ubfx x25, x26, #20, #4 // Extract WRPs >> + mov w26, #15 >> + sub w24, w26, w24 // How many BPs to skip >> + sub w25, w26, w25 // How many WPs to skip >> + >> + mov x5, x24 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1) >> + restore_debug dbgbcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1) >> + restore_debug dbgbvr >> + >> + mov x5, x25 >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1) >> + restore_debug dbgwcr >> + add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1) >> + restore_debug dbgwvr >> + >> + ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)] >> + msr mdccint_el1, x21 >> + >> ret >> >> __save_fpsimd: > > Thanks, > -Christoffer -- Alex Bennée