From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [RFC PATCH v2 11/23] KVM: arm64: Support runtime sysreg filtering for KVM_GET_REG_LIST Date: Thu, 22 Nov 2018 11:27:53 +0000 Message-ID: <87bm6hic7a.fsf@linaro.org> References: <1538141967-15375-1-git-send-email-Dave.Martin@arm.com> <1538141967-15375-12-git-send-email-Dave.Martin@arm.com> <20181102081625.GS12057@e113682-lin.lund.arm.com> <20181115172711.GQ3505@e103592.cambridge.arm.com> <20181122105344.GF17441@e113682-lin.lund.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 218334A1DE for ; Thu, 22 Nov 2018 06:27:57 -0500 (EST) 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 KiOFNsKZzP1Z for ; Thu, 22 Nov 2018 06:27:56 -0500 (EST) Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id F24D540A6E for ; Thu, 22 Nov 2018 06:27:55 -0500 (EST) Received: by mail-wr1-f65.google.com with SMTP id t27so944176wra.6 for ; Thu, 22 Nov 2018 03:27:55 -0800 (PST) In-reply-to: <20181122105344.GF17441@e113682-lin.lund.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: Christoffer Dall Cc: Okamoto Takayuki , Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , kvmarm@lists.cs.columbia.edu, Dave Martin , linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu CkNocmlzdG9mZmVyIERhbGwgPGNocmlzdG9mZmVyLmRhbGxAYXJtLmNvbT4gd3JpdGVzOgoKPiBb QWRkaW5nIFBldGVyIGFuZCBBbGV4IGZvciB0aGVpciB2aWV3IG9uIHRoZSBRRU1VIHNpZGVdCj4K PiBPbiBUaHUsIE5vdiAxNSwgMjAxOCBhdCAwNToyNzoxMVBNICswMDAwLCBEYXZlIE1hcnRpbiB3 cm90ZToKPj4gT24gRnJpLCBOb3YgMDIsIDIwMTggYXQgMDk6MTY6MjVBTSArMDEwMCwgQ2hyaXN0 b2ZmZXIgRGFsbCB3cm90ZToKPj4gPiBPbiBGcmksIFNlcCAyOCwgMjAxOCBhdCAwMjozOToxNVBN ICswMTAwLCBEYXZlIE1hcnRpbiB3cm90ZToKPj4gPiA+IEtWTV9HRVRfUkVHX0xJU1Qgc2hvdWxk IG9ubHkgZW51bWVyYXRlIHJlZ2lzdGVycyB0aGF0IGFyZSBhY3R1YWxseQo+PiA+ID4gYWNjZXNz aWJsZSwgc28gaXQgaXMgbmVjZXNzYXJ5IHRvIGZpbHRlciBvdXQgYW55IHJlZ2lzdGVyIHRoYXQg aXMKPj4gPiA+IG5vdCBleHBvc2VkIHRvIHRoZSBndWVzdC4gIEZvciBmZWF0dXJlcyB0aGF0IGFy ZSBjb25maWd1cmVkIGF0Cj4+ID4gPiBydW50aW1lLCB0aGlzIHdpbGwgcmVxdWlyZSBhIGR5bmFt aWMgY2hlY2suCj4+ID4gPgo+PiA+ID4gRm9yIGV4YW1wbGUsIFpDUl9FTDEgYW5kIElEX0FBNjRa RlIwX0VMMSB3b3VsZCBuZWVkIHRvIGJlIGhpZGRlbgo+PiA+ID4gaWYgU1ZFIGlzIG5vdCBlbmFi bGVkIGZvciB0aGUgZ3Vlc3QuCj4+ID4KPj4gPiBUaGlzIGltcGxpZXMgdGhhdCB1c2Vyc3BhY2Ug Y2FuIG5ldmVyIGFjY2VzcyB0aGlzIGludGVyZmFjZSBmb3IgYSB2Y3B1Cj4+ID4gYmVmb3JlIGhh dmluZyBkZWNpZGVkIHdoZXRoZXIgc3VjaCBmZWF0dXJlcyBhcmUgZW5hYmxlZCBmb3IgdGhlIGd1 ZXN0IG9yCj4+ID4gbm90LCBzaW5jZSBvdGhlcndpc2UgdXNlcnNwYWNlIHdpbGwgc2VlIGRpZmZl cmVudCBzdGF0ZXMgZm9yIGEgVkNQVQo+PiA+IGRlcGVuZGluZyBvbiBzZXF1ZW5jaW5nIG9mIHRo ZSBBUEksIHdoaWNoIHNvdW5kcyBmcmFnaWxlIHRvIG1lLgo+PiA+Cj4+ID4gVGhhdCBzaG91bGQg cHJvYmFibHkgYmUgZG9jdW1lbnRlZCBzb21ld2hlcmUsIGFuZCBJIGhvcGUgdGhlCj4+ID4gZW5h YmxlL2Rpc2FibGUgQVBJIGZvciBTVkUgaW4gZ3Vlc3RzIGFscmVhZHkgdGFrZXMgdGhhdCBpbnRv IGFjY291bnQuCj4+ID4KPj4gPiBOb3Qgc3VyZSBpZiB0aGVyZSdzIGFuIGFjdGlvbiB0byB0YWtl IGhlcmUsIGJ1dCBpdCB3YXMgdGhlIGJlc3QgcGxhY2UgSQo+PiA+IGNvdWxkIHJhaXNlIHRoaXMg Y29uY2Vybi4KPj4KPj4gRmFpciBwb2ludC4gIEkgc3RydWdnbGVkIHRvIGNvbWUgdXAgd2l0aCBz b21ldGhpbmcgYmV0dGVyIHRoYXQgc29sdmVzCj4+IGFsbCBwcm9ibGVtcy4KPj4KPj4gTXkgZXhw ZWN0YXRpb24gaXMgdGhhdCBLVk1fQVJNX1NWRV9DT05GSUdfU0VUIGlzIGNvbnNpZGVyZWQgcGFy dCBvZgo+PiBjcmVhdGluZyB0aGUgdmNwdSwgc28gdGhhdCBpZiBpc3N1ZWQgYXQgYWxsIGZvciBh IHZjcHUsIGl0IGlzIGlzc3VlZAo+PiB2ZXJ5IHNvb24gYWZ0ZXIgS1ZNX1ZDUFVfSU5JVC4KPj4K Pj4gSSB0aGluayB0aGlzIHdvcmtlZCBPSyB3aXRoIHRoZSBjdXJyZW50IHN0cnVjdHVyZSBvZiBr dm10b29sIGFuZCBJCj4+IHNlZW0gdG8gcmVtZW1iZXIgZGlzY3Vzc2luZyB0aGlzIHdpdGggUGV0 ZXIgTWF5ZGVsbCByZSBxZW11IC0tIGJ1dAo+PiBpdCBzb3VuZHMgbGlrZSBJIHNob3VsZCBkb3Vi bGUtY2hlY2suCj4KPiBRRU1VIGRvZXMgc29tZSB0aGluZyBhcm91bmQgZW51bWVyYXRpbmcgYWxs IHRoZSBzeXN0ZW0gcmVnaXN0ZXJzIGV4cG9zZWQKPiBieSBLVk0gYW5kIHNhdmluZy9yZXN0b3Jp bmcgdGhlbSBhcyBwYXJ0IG9mIGl0cyBzdGFydHVwLCBidXQgSSBkb24ndAo+IHJlbWVtYmVyIHRo ZSBleGFjdCBzZXF1ZW5jZS4KClFFTVUgZG9lcyB0aGlzIGZvciBlYWNoIHZDUFUgYXMgcGFydCBv ZiBpdCdzIHN0YXJ0LXVwIHNlcXVlbmNlOgoKICBrdm1faW5pdF92Y3B1CiAgICBrdm1fZ2V0X2Nw dSAoLT4gS1ZNX0NSRUFURV9WQ1BVKQogICAgS1ZNX0dFVF9WQ1BVX01NQVBfU0laRQogICAga3Zt X2FyY2hfaW5pdF92Y3B1CiAgICAgIGt2bV9hcm1fdmNwdV9pbml0ICgtPiBLVk1fQVJNX1ZDUFVf SU5JVCkKICAgICAga3ZtX2dldF9vbmVfcmVnKEFSTV9DUFVfSURfTVBJRFIpCiAgICAgIGt2bV9h cm1faW5pdF9kZWJ1ZyAoY2hrIGZvciBLVk1fQ0FQIFNFVF9HVUVTVF9ERUJVRy9HVUVTVF9ERUJV R19IV19XUFMvQlBTKQogICAgICBrdm1fYXJtX2luaXRfc2Vycm9yX2luamVjdGlvbiAoY2hrIEtW TV9DQVBfQVJNX0lOSkVDVF9TRVJST1JfRVNSKQogICAgICBrdm1fYXJtX2luaXRfY3ByZWdfbGlz dCAoS1ZNX0dFVF9SRUdfTElTVCkKCkF0IHRoaXMgcG9pbnQgd2UgaGF2ZSB0aGUgcmVnaXN0ZXIg bGlzdCB3ZSBuZWVkIGZvcgprdm1fYXJjaF9nZXRfcmVnaXN0ZXJzIHdoaWNoIGlzIHdoYXQgd2Ug Y2FsbCBldmVyeSB0aW1lIHdlIHdhbnQgdG8Kc3luY2hyb25pc2Ugc3RhdGUuIFdlIG9ubHkgcmVh bGx5IGRvIHRoaXMgZm9yIGRlYnVnIGV2ZW50cywgY3Jhc2hlcyBhbmQKYXQgc29tZSBwb2ludCB3 aGVuIG1pZ3JhdGluZy4KCj4KPj4KPj4gRWl0aGVyIHdheSwgeW91J3JlIHJpZ2h0LCB0aGlzIG5l ZWRzIHRvIGJlIGNsZWFybHkgZG9jdW1lbnRlZC4KPj4KPj4KPj4gSWYgd2Ugd2FudCB0byBiZSBt b3JlIHJvYnVzdCwgbWF5YmUgd2Ugc2hvdWxkIGFkZCBhIGNhcGFiaWxpdHkgdG9vLAo+PiBzbyB0 aGF0IHVzZXJzcGFjZSB0aGF0IGVuYWJsZXMgdGhpcyBjYXBhYmlsaXR5IHByb21pc2VzIHRvIGNh bGwKPj4gS1ZNX0FSTV9TVkVfQ09ORklHX1NFVCBmb3IgZWFjaCB2Y3B1LCBhbmQgYWZmZWN0ZWQg aW9jdGxzIChLVk1fUlVOLAo+PiBLVk1fR0VUX1JFR19MSVNUIGV0Yy4pIGFyZSBmb3JiaWRkZW4g dW50aWwgdGhhdCBpcyBkb25lPwo+Pgo+PiBUaGF0IHNob3VsZCBoZWxwIGF2b2lkIGFjY2lkZW50 cy4KPj4KPj4gSSBjb3VsZCBhZGQgYSBzcGVjaWFsIG1lYW5pbmcgZm9yIGFuIGVtcHR5IGt2bV9z dmVfdmxzLCBzdWNoIHRoYXQKPj4gaXQgZG9lc24ndCBlbmFibGUgU1ZFIG9uIHRoZSBhZmZlY3Rl ZCB2Y3B1LiAgVGhhdCByZXRhaW5zIHRoZSBhYmlsaXR5Cj4+IHRvIGNyZWF0ZSBoZXRlcm9nZW5l b3VzIGd1ZXN0cyB3aGlsZSBzdGlsbCBmb2xsb3dpbmcgdGhlIGFib3ZlIGZsb3cuCj4+Cj4gSSB0 aGluayBtYWtpbmcgc3VyZSB0aGF0IHVzZXJzcGFjZSBjYW4gZXZlciBvbmx5IHNlZSB0aGUgc2Ft ZSBsaXN0IG9mCj4gYXZhaWxhYmxlIHN5c3RlbSByZWdpdGVycyBpcyBnb2luZyB0byBjYXVzZSB1 cyBsZXNzIHBhaW4gZ29pbmcgZm9yd2FyZC4KPgo+IElmIHRoZSBzZXBhcmF0ZSBpb2N0bCBhbmQg Y2FwYWJpbGl0eSBjaGVjayBpcyB0aGUgZWFzaWVzdCB3YXkgb2YgZG9pbmcKPiB0aGF0LCB0aGVu IEkgdGhpbmsgdGhhdCBzb3VuZHMgZ29vZC4gIChJIGhhZCB3aXNoZWQgd2UgY291bGQgaGF2ZSBq dXN0Cj4gYWRkZWQgc29tZSBkYXRhIHRvIEtWTV9DUkVBVEVfVkNQVSwgYnV0IHRoYXQgZG9lc24n dCBzZWVtIHRvIGJlIHRoZQo+IGNhc2UuKQo+Cj4KPiBUaGFua3MsCj4KPiAgICAgQ2hyaXN0b2Zm ZXIKCgotLQpBbGV4IEJlbm7DqWUKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18Ka3ZtYXJtIG1haWxpbmcgbGlzdAprdm1hcm1AbGlzdHMuY3MuY29sdW1iaWEu ZWR1Cmh0dHBzOi8vbGlzdHMuY3MuY29sdW1iaWEuZWR1L21haWxtYW4vbGlzdGluZm8va3ZtYXJt Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Thu, 22 Nov 2018 11:27:53 +0000 Subject: [RFC PATCH v2 11/23] KVM: arm64: Support runtime sysreg filtering for KVM_GET_REG_LIST In-Reply-To: <20181122105344.GF17441@e113682-lin.lund.arm.com> References: <1538141967-15375-1-git-send-email-Dave.Martin@arm.com> <1538141967-15375-12-git-send-email-Dave.Martin@arm.com> <20181102081625.GS12057@e113682-lin.lund.arm.com> <20181115172711.GQ3505@e103592.cambridge.arm.com> <20181122105344.GF17441@e113682-lin.lund.arm.com> Message-ID: <87bm6hic7a.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Christoffer Dall writes: > [Adding Peter and Alex for their view on the QEMU side] > > On Thu, Nov 15, 2018 at 05:27:11PM +0000, Dave Martin wrote: >> On Fri, Nov 02, 2018 at 09:16:25AM +0100, Christoffer Dall wrote: >> > On Fri, Sep 28, 2018 at 02:39:15PM +0100, Dave Martin wrote: >> > > KVM_GET_REG_LIST should only enumerate registers that are actually >> > > accessible, so it is necessary to filter out any register that is >> > > not exposed to the guest. For features that are configured at >> > > runtime, this will require a dynamic check. >> > > >> > > For example, ZCR_EL1 and ID_AA64ZFR0_EL1 would need to be hidden >> > > if SVE is not enabled for the guest. >> > >> > This implies that userspace can never access this interface for a vcpu >> > before having decided whether such features are enabled for the guest or >> > not, since otherwise userspace will see different states for a VCPU >> > depending on sequencing of the API, which sounds fragile to me. >> > >> > That should probably be documented somewhere, and I hope the >> > enable/disable API for SVE in guests already takes that into account. >> > >> > Not sure if there's an action to take here, but it was the best place I >> > could raise this concern. >> >> Fair point. I struggled to come up with something better that solves >> all problems. >> >> My expectation is that KVM_ARM_SVE_CONFIG_SET is considered part of >> creating the vcpu, so that if issued at all for a vcpu, it is issued >> very soon after KVM_VCPU_INIT. >> >> I think this worked OK with the current structure of kvmtool and I >> seem to remember discussing this with Peter Maydell re qemu -- but >> it sounds like I should double-check. > > QEMU does some thing around enumerating all the system registers exposed > by KVM and saving/restoring them as part of its startup, but I don't > remember the exact sequence. QEMU does this for each vCPU as part of it's start-up sequence: kvm_init_vcpu kvm_get_cpu (-> KVM_CREATE_VCPU) KVM_GET_VCPU_MMAP_SIZE kvm_arch_init_vcpu kvm_arm_vcpu_init (-> KVM_ARM_VCPU_INIT) kvm_get_one_reg(ARM_CPU_ID_MPIDR) kvm_arm_init_debug (chk for KVM_CAP SET_GUEST_DEBUG/GUEST_DEBUG_HW_WPS/BPS) kvm_arm_init_serror_injection (chk KVM_CAP_ARM_INJECT_SERROR_ESR) kvm_arm_init_cpreg_list (KVM_GET_REG_LIST) At this point we have the register list we need for kvm_arch_get_registers which is what we call every time we want to synchronise state. We only really do this for debug events, crashes and at some point when migrating. > >> >> Either way, you're right, this needs to be clearly documented. >> >> >> If we want to be more robust, maybe we should add a capability too, >> so that userspace that enables this capability promises to call >> KVM_ARM_SVE_CONFIG_SET for each vcpu, and affected ioctls (KVM_RUN, >> KVM_GET_REG_LIST etc.) are forbidden until that is done? >> >> That should help avoid accidents. >> >> I could add a special meaning for an empty kvm_sve_vls, such that >> it doesn't enable SVE on the affected vcpu. That retains the ability >> to create heterogeneous guests while still following the above flow. >> > I think making sure that userspace can ever only see the same list of > available system regiters is going to cause us less pain going forward. > > If the separate ioctl and capability check is the easiest way of doing > that, then I think that sounds good. (I had wished we could have just > added some data to KVM_CREATE_VCPU, but that doesn't seem to be the > case.) > > > Thanks, > > Christoffer -- Alex Benn?e