From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [RFC PATCH 05/16] KVM: arm: Add arch init/uninit hooks Date: Fri, 06 Jul 2018 11:02:20 +0100 Message-ID: <877em88yxf.fsf@linaro.org> References: <1529593060-542-1-git-send-email-Dave.Martin@arm.com> <1529593060-542-6-git-send-email-Dave.Martin@arm.com> 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 778164A054 for ; Fri, 6 Jul 2018 05:49:51 -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 DY8x1m-7MbGQ for ; Fri, 6 Jul 2018 05:49:50 -0400 (EDT) Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 2A18649F8C for ; Fri, 6 Jul 2018 05:49:50 -0400 (EDT) Received: by mail-wm0-f66.google.com with SMTP id v16-v6so14320394wmv.5 for ; Fri, 06 Jul 2018 03:02:23 -0700 (PDT) In-reply-to: <1529593060-542-6-git-send-email-Dave.Martin@arm.com> 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: Dave Martin Cc: Okamoto Takayuki , Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu CkRhdmUgTWFydGluIDxEYXZlLk1hcnRpbkBhcm0uY29tPiB3cml0ZXM6Cgo+IEluIHByZXBhcmF0 aW9uIGZvciBhZGRpbmcgc3VwcG9ydCBmb3IgU1ZFIGluIGd1ZXN0cyBvbiBhcm02NCwgaG9va3MK PiBmb3IgYWxsb2NhdGluZyBhbmQgZnJlZWluZyBhZGRpdGlvbmFsIHBlci12Y3B1IG1lbW9yeSBh cmUgbmVlZGVkLgo+Cj4ga3ZtX2FyY2hfdmNwdV9zZXR1cCgpIGNvdWxkIGJlIHVzZWQgZm9yIGFs bG9jYXRpb24sIGJ1dCB0aGlzCj4gZnVuY3Rpb24gaXMgbm90IGNsZWFybHkgYmFsYW5jZWQgYnkg dW4gInVuc2V0dXAiIGZ1bmN0aW9uLCBtYWtpbmcKCklzbid0IHRoYXQgYSBkb3VibGUgbmVnYXRp dmUgdGhlcmU/IFN1cmVseSBpdCB3b3VsZCBiZSBiYWxhbmNlZCBieSBhCmt2bV9hcmNoX3ZjcHVf dW5zZXR1cCgpIG9yIHBvc3NpYmx5IGJldHRlciBuYW1lZCBmdW5jdGlvbi4KCj4gaXQgdW5jbGVh ciB3aGVyZSBtZW1vcnkgYWxsb2NhdGVkIGluIHRoaXMgZnVuY3Rpb24gc2hvdWxkIGJlIGZyZWVk Lgo+Cj4gVG8ga2VlcCB0aGluZ3Mgc2ltcGxlLCB0aGlzIHBhdGNoIGRlZmluZXMgYmFja2VuZCBo b29rcwo+IGt2bV9hcm1fYXJjaF92Y3B1X3ssdW59dW5pbnQoKSwgYW5kIHBsdW1icyB0aGVtIGlu IGFwcHJvcHJpYXRlbHkuCgpJcyB7LHVufSBhIG5vdGF0aW9uIGZvciBkcm9wcGluZyB1bj8gVGhp cyBtaWdodCBiZSB3aHkgSSdtIGNvbmZ1c2VkLiBJCndvdWxkIGhhdmUgd3JpdHRlbiBpdCBhcyBr dm1fYXJtX2FyY2hfdmNwdV9bdW5daW5pdCgpIG9yIGV2ZW4Ka3ZtX2FybV9hcmNoX3ZjcHVfW2lu aXR8dW5pbml0XS4KCj4gVGhlIGV4dXN0aW5nIGt2bV9hcmNoX3ZjcHVfaW5pdCgpIGZ1bmN0aW9u IG5vdyBjYWxscwoKL2V4aXN0aW5nLwoKPiBrdm1fYXJtX2FyY2hfdmNwdV9pbml0KCksIHdoaWxl IGFuIGV4cGxpY2l0IGt2bV9hcmNoX3ZjcHVfdW5pbml0KCkKPiBpcyBhZGRlZCB3aGljaCBjdXJy ZW50IGRvZXMgbm90aGluZyBleGNlcHQgdG8gY2FsbAo+IGt2bV9hcm1fYXJjaF92Y3B1X3VuaW5p dCgpLgoKT0sgSSdtIGEgbGl0dGxlIGNvbmZ1c2VkIGJ5IHRoaXMuIEl0IHNlZW1zIHRvIG1lIHRo YXQgS1ZNIGFscmVhZHkgaGFzCnRoZSBwcm92aXNpb24gZm9yIGFuIGluaXQvdW5pbml0LiBXaGF0 IGRvZXMgdGhlIGV4dHJhIGxldmVsIG9uCmluZGlyZWN0aW9uIGJ1eSB5b3UgdGhhdCBrZWVwaW5n IHRoZSBzdGF0aWMgaW5saW5lCmt2bV9hcm1fYXJjaF92Y3B1X3VuaW5pdCBpbiBhcm0va3ZtX2hv c3QuaCBhbmQgYSBjb25jcmV0ZSBpbXBsZW1lbnRhdGlvbgppbiBhcm02NC9rdm0vZ3Vlc3QuYyBk b2Vzbid0PwoKPgo+IFRoZSBiYWNrZW5kIGZ1bmN0aW9ucyBhcmUgY3VycmVudGx5IGRlZmluZWQg dG8gZG8gbm90aGluZy4KPgo+IE5vIGZ1bmN0aW9uYWwgY2hhbmdlLgo+Cj4gU2lnbmVkLW9mZi1i eTogRGF2ZSBNYXJ0aW4gPERhdmUuTWFydGluQGFybS5jb20+Cj4gLS0tCj4gIGFyY2gvYXJtL2lu Y2x1ZGUvYXNtL2t2bV9ob3N0LmggICB8ICA0ICsrKy0KPiAgYXJjaC9hcm02NC9pbmNsdWRlL2Fz bS9rdm1faG9zdC5oIHwgIDQgKysrLQo+ICB2aXJ0L2t2bS9hcm0vYXJtLmMgICAgICAgICAgICAg ICAgfCAxMyArKysrKysrKysrKystCj4gIDMgZmlsZXMgY2hhbmdlZCwgMTggaW5zZXJ0aW9ucygr KSwgMyBkZWxldGlvbnMoLSkKPgo+IGRpZmYgLS1naXQgYS9hcmNoL2FybS9pbmNsdWRlL2FzbS9r dm1faG9zdC5oIGIvYXJjaC9hcm0vaW5jbHVkZS9hc20va3ZtX2hvc3QuaAo+IGluZGV4IDFmMWZl NDEwLi45YjkwMmI4IDEwMDY0NAo+IC0tLSBhL2FyY2gvYXJtL2luY2x1ZGUvYXNtL2t2bV9ob3N0 LmgKPiArKysgYi9hcmNoL2FybS9pbmNsdWRlL2FzbS9rdm1faG9zdC5oCj4gQEAgLTI4NCwxMCAr Mjg0LDEyIEBAIHN0cnVjdCBrdm1fdmNwdSAqa3ZtX21waWRyX3RvX3ZjcHUoc3RydWN0IGt2bSAq a3ZtLCB1bnNpZ25lZCBsb25nIG1waWRyKTsKPiAgc3RhdGljIGlubGluZSBib29sIGt2bV9hcmNo X2NoZWNrX3N2ZV9oYXNfdmhlKHZvaWQpIHsgcmV0dXJuIHRydWU7IH0KPiAgc3RhdGljIGlubGlu ZSB2b2lkIGt2bV9hcmNoX2hhcmR3YXJlX3Vuc2V0dXAodm9pZCkge30KPiAgc3RhdGljIGlubGlu ZSB2b2lkIGt2bV9hcmNoX3N5bmNfZXZlbnRzKHN0cnVjdCBrdm0gKmt2bSkge30KPiAtc3RhdGlj IGlubGluZSB2b2lkIGt2bV9hcmNoX3ZjcHVfdW5pbml0KHN0cnVjdCBrdm1fdmNwdSAqdmNwdSkg e30KPiAgc3RhdGljIGlubGluZSB2b2lkIGt2bV9hcmNoX3NjaGVkX2luKHN0cnVjdCBrdm1fdmNw dSAqdmNwdSwgaW50IGNwdSkge30KPiAgc3RhdGljIGlubGluZSB2b2lkIGt2bV9hcmNoX3ZjcHVf YmxvY2tfZmluaXNoKHN0cnVjdCBrdm1fdmNwdSAqdmNwdSkge30KPgo+ICtzdGF0aWMgaW5saW5l IGludCBrdm1fYXJtX2FyY2hfdmNwdV9pbml0KHN0cnVjdCBrdm1fdmNwdSAqdmNwdSkgeyByZXR1 cm4gMDsgfQo+ICtzdGF0aWMgaW5saW5lIHZvaWQga3ZtX2FybV9hcmNoX3ZjcHVfdW5pbml0KHN0 cnVjdCBrdm1fdmNwdSAqdmNwdSkge30KPiArCj4gIHN0YXRpYyBpbmxpbmUgdm9pZCBrdm1fYXJt X2luaXRfZGVidWcodm9pZCkge30KPiAgc3RhdGljIGlubGluZSB2b2lkIGt2bV9hcm1fc2V0dXBf ZGVidWcoc3RydWN0IGt2bV92Y3B1ICp2Y3B1KSB7fQo+ICBzdGF0aWMgaW5saW5lIHZvaWQga3Zt X2FybV9jbGVhcl9kZWJ1ZyhzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpIHt9Cj4gZGlmZiAtLWdpdCBh L2FyY2gvYXJtNjQvaW5jbHVkZS9hc20va3ZtX2hvc3QuaCBiL2FyY2gvYXJtNjQvaW5jbHVkZS9h c20va3ZtX2hvc3QuaAo+IGluZGV4IDkyZDZlODguLjk2NzFkZGQgMTAwNjQ0Cj4gLS0tIGEvYXJj aC9hcm02NC9pbmNsdWRlL2FzbS9rdm1faG9zdC5oCj4gKysrIGIvYXJjaC9hcm02NC9pbmNsdWRl L2FzbS9rdm1faG9zdC5oCj4gQEAgLTQyNSwxMCArNDI1LDEyIEBAIHN0YXRpYyBpbmxpbmUgYm9v bCBrdm1fYXJjaF9jaGVja19zdmVfaGFzX3ZoZSh2b2lkKQo+Cj4gIHN0YXRpYyBpbmxpbmUgdm9p ZCBrdm1fYXJjaF9oYXJkd2FyZV91bnNldHVwKHZvaWQpIHt9Cj4gIHN0YXRpYyBpbmxpbmUgdm9p ZCBrdm1fYXJjaF9zeW5jX2V2ZW50cyhzdHJ1Y3Qga3ZtICprdm0pIHt9Cj4gLXN0YXRpYyBpbmxp bmUgdm9pZCBrdm1fYXJjaF92Y3B1X3VuaW5pdChzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpIHt9Cj4g IHN0YXRpYyBpbmxpbmUgdm9pZCBrdm1fYXJjaF9zY2hlZF9pbihzdHJ1Y3Qga3ZtX3ZjcHUgKnZj cHUsIGludCBjcHUpIHt9Cj4gIHN0YXRpYyBpbmxpbmUgdm9pZCBrdm1fYXJjaF92Y3B1X2Jsb2Nr X2ZpbmlzaChzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpIHt9Cj4KPiArc3RhdGljIGlubGluZSBpbnQg a3ZtX2FybV9hcmNoX3ZjcHVfaW5pdChzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpIHsgcmV0dXJuIDA7 IH0KPiArc3RhdGljIGlubGluZSB2b2lkIGt2bV9hcm1fYXJjaF92Y3B1X3VuaW5pdChzdHJ1Y3Qg a3ZtX3ZjcHUgKnZjcHUpIHt9Cj4gKwo+ICB2b2lkIGt2bV9hcm1faW5pdF9kZWJ1Zyh2b2lkKTsK PiAgdm9pZCBrdm1fYXJtX3NldHVwX2RlYnVnKHN0cnVjdCBrdm1fdmNwdSAqdmNwdSk7Cj4gIHZv aWQga3ZtX2FybV9jbGVhcl9kZWJ1ZyhzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpOwo+IGRpZmYgLS1n aXQgYS92aXJ0L2t2bS9hcm0vYXJtLmMgYi92aXJ0L2t2bS9hcm0vYXJtLmMKPiBpbmRleCAwNGU1 NTRjLi42NmYxNWNjIDEwMDY0NAo+IC0tLSBhL3ZpcnQva3ZtL2FybS9hcm0uYwo+ICsrKyBiL3Zp cnQva3ZtL2FybS9hcm0uYwo+IEBAIC0zNDUsNiArMzQ1LDggQEAgdm9pZCBrdm1fYXJjaF92Y3B1 X3VuYmxvY2tpbmcoc3RydWN0IGt2bV92Y3B1ICp2Y3B1KQo+Cj4gIGludCBrdm1fYXJjaF92Y3B1 X2luaXQoc3RydWN0IGt2bV92Y3B1ICp2Y3B1KQo+ICB7Cj4gKwlpbnQgcmV0Owo+ICsKPiAgCS8q IEZvcmNlIHVzZXJzIHRvIGNhbGwgS1ZNX0FSTV9WQ1BVX0lOSVQgKi8KPiAgCXZjcHUtPmFyY2gu dGFyZ2V0ID0gLTE7Cj4gIAliaXRtYXBfemVybyh2Y3B1LT5hcmNoLmZlYXR1cmVzLCBLVk1fVkNQ VV9NQVhfRkVBVFVSRVMpOwo+IEBAIC0zNTQsNyArMzU2LDE2IEBAIGludCBrdm1fYXJjaF92Y3B1 X2luaXQoc3RydWN0IGt2bV92Y3B1ICp2Y3B1KQo+Cj4gIAlrdm1fYXJtX3Jlc2V0X2RlYnVnX3B0 cih2Y3B1KTsKPgo+IC0JcmV0dXJuIGt2bV92Z2ljX3ZjcHVfaW5pdCh2Y3B1KTsKPiArCXJldCA9 IGt2bV92Z2ljX3ZjcHVfaW5pdCh2Y3B1KTsKPiArCWlmIChyZXQpCj4gKwkJcmV0dXJuIHJldDsK PiArCj4gKwlyZXR1cm4ga3ZtX2FybV9hcmNoX3ZjcHVfaW5pdCh2Y3B1KTsKPiArfQo+ICsKPiAr dm9pZCBrdm1fYXJjaF92Y3B1X3VuaW5pdChzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpCj4gK3sKPiAr CWt2bV9hcm1fYXJjaF92Y3B1X3VuaW5pdCh2Y3B1KTsKPiAgfQo+Cj4gIHZvaWQga3ZtX2FyY2hf dmNwdV9sb2FkKHN0cnVjdCBrdm1fdmNwdSAqdmNwdSwgaW50IGNwdSkKCgotLQpBbGV4IEJlbm7D qWUKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18Ka3ZtYXJt IG1haWxpbmcgbGlzdAprdm1hcm1AbGlzdHMuY3MuY29sdW1iaWEuZWR1Cmh0dHBzOi8vbGlzdHMu Y3MuY29sdW1iaWEuZWR1L21haWxtYW4vbGlzdGluZm8va3ZtYXJtCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Fri, 06 Jul 2018 11:02:20 +0100 Subject: [RFC PATCH 05/16] KVM: arm: Add arch init/uninit hooks In-Reply-To: <1529593060-542-6-git-send-email-Dave.Martin@arm.com> References: <1529593060-542-1-git-send-email-Dave.Martin@arm.com> <1529593060-542-6-git-send-email-Dave.Martin@arm.com> Message-ID: <877em88yxf.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dave Martin writes: > In preparation for adding support for SVE in guests on arm64, hooks > for allocating and freeing additional per-vcpu memory are needed. > > kvm_arch_vcpu_setup() could be used for allocation, but this > function is not clearly balanced by un "unsetup" function, making Isn't that a double negative there? Surely it would be balanced by a kvm_arch_vcpu_unsetup() or possibly better named function. > it unclear where memory allocated in this function should be freed. > > To keep things simple, this patch defines backend hooks > kvm_arm_arch_vcpu_{,un}unint(), and plumbs them in appropriately. Is {,un} a notation for dropping un? This might be why I'm confused. I would have written it as kvm_arm_arch_vcpu_[un]init() or even kvm_arm_arch_vcpu_[init|uninit]. > The exusting kvm_arch_vcpu_init() function now calls /existing/ > kvm_arm_arch_vcpu_init(), while an explicit kvm_arch_vcpu_uninit() > is added which current does nothing except to call > kvm_arm_arch_vcpu_uninit(). OK I'm a little confused by this. It seems to me that KVM already has the provision for an init/uninit. What does the extra level on indirection buy you that keeping the static inline kvm_arm_arch_vcpu_uninit in arm/kvm_host.h and a concrete implementation in arm64/kvm/guest.c doesn't? > > The backend functions are currently defined to do nothing. > > No functional change. > > Signed-off-by: Dave Martin > --- > arch/arm/include/asm/kvm_host.h | 4 +++- > arch/arm64/include/asm/kvm_host.h | 4 +++- > virt/kvm/arm/arm.c | 13 ++++++++++++- > 3 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h > index 1f1fe410..9b902b8 100644 > --- a/arch/arm/include/asm/kvm_host.h > +++ b/arch/arm/include/asm/kvm_host.h > @@ -284,10 +284,12 @@ struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr); > static inline bool kvm_arch_check_sve_has_vhe(void) { return true; } > static inline void kvm_arch_hardware_unsetup(void) {} > static inline void kvm_arch_sync_events(struct kvm *kvm) {} > -static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} > static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} > static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {} > > +static inline int kvm_arm_arch_vcpu_init(struct kvm_vcpu *vcpu) { return 0; } > +static inline void kvm_arm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} > + > static inline void kvm_arm_init_debug(void) {} > static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {} > static inline void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) {} > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 92d6e88..9671ddd 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -425,10 +425,12 @@ static inline bool kvm_arch_check_sve_has_vhe(void) > > static inline void kvm_arch_hardware_unsetup(void) {} > static inline void kvm_arch_sync_events(struct kvm *kvm) {} > -static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} > static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} > static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {} > > +static inline int kvm_arm_arch_vcpu_init(struct kvm_vcpu *vcpu) { return 0; } > +static inline void kvm_arm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} > + > void kvm_arm_init_debug(void); > void kvm_arm_setup_debug(struct kvm_vcpu *vcpu); > void kvm_arm_clear_debug(struct kvm_vcpu *vcpu); > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c > index 04e554c..66f15cc 100644 > --- a/virt/kvm/arm/arm.c > +++ b/virt/kvm/arm/arm.c > @@ -345,6 +345,8 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) > > int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > { > + int ret; > + > /* Force users to call KVM_ARM_VCPU_INIT */ > vcpu->arch.target = -1; > bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES); > @@ -354,7 +356,16 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > > kvm_arm_reset_debug_ptr(vcpu); > > - return kvm_vgic_vcpu_init(vcpu); > + ret = kvm_vgic_vcpu_init(vcpu); > + if (ret) > + return ret; > + > + return kvm_arm_arch_vcpu_init(vcpu); > +} > + > +void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) > +{ > + kvm_arm_arch_vcpu_uninit(vcpu); > } > > void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) -- Alex Benn?e