From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E9AC957CA7; Fri, 31 May 2024 16:04:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717171442; cv=none; b=B6gaa13HQmbt1v0U4qHhaS4djNuNMxfm+KdOaBZUu3W/8C2u/sBlu3r+P9WbLL/8VHeCfHut+0X3M/f04n5pBjeXUIfPg55W0SKrMJjlQqI52nUDabChEL6RR2pSt2oud49/uY3Y0OATAcTQITYx8QqzkpWBt0h79pmQGqtefWc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717171442; c=relaxed/simple; bh=IFUjXMQpAq5HwdlN0F4Ro/EwDdykkZC4cPHBidBAlvw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=OZfMmBduE8GkzDx1D6m1HCUZoZKKl3elO9Foeg5Tq0mVZeesZFus7Sq/zbisTsToXxkyIHa/PpdeAkAmAjcThSz/7zrxyjtnAtmzD3O2Cg6dJoFdUn8baaEC4wAYFGwtwE22/wpCcFSvBNx5msDyQcc3YTTr9K7FwPctA4XUxzs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6A6631424; Fri, 31 May 2024 09:04:24 -0700 (PDT) Received: from [10.1.27.19] (e122027.cambridge.arm.com [10.1.27.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7A9A33F792; Fri, 31 May 2024 09:03:56 -0700 (PDT) Message-ID: Date: Fri, 31 May 2024 17:03:54 +0100 Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 21/43] arm64: RME: Runtime faulting of memory To: Fuad Tabba Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, Catalin Marinas , Marc Zyngier , Will Deacon , James Morse , Oliver Upton , Suzuki K Poulose , Zenghui Yu , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Joey Gouly , Alexandru Elisei , Christoffer Dall , linux-coco@lists.linux.dev, Ganapatrao Kulkarni References: <20240412084056.1733704-1-steven.price@arm.com> <20240412084309.1733783-1-steven.price@arm.com> <20240412084309.1733783-22-steven.price@arm.com> From: Steven Price Content-Language: en-GB In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 25/04/2024 11:43, Fuad Tabba wrote: > Hi, Hi, Thanks for the review. Sorry I didn't respond earlier. > On Fri, Apr 12, 2024 at 9:44 AM Steven Price wrote: >> >> +static int private_memslot_fault(struct kvm_vcpu *vcpu, >> + phys_addr_t fault_ipa, >> + struct kvm_memory_slot *memslot) >> +{ >> + struct kvm *kvm = vcpu->kvm; >> + gpa_t gpa_stolen_mask = kvm_gpa_stolen_bits(kvm); >> + gfn_t gfn = (fault_ipa & ~gpa_stolen_mask) >> PAGE_SHIFT; >> + bool is_priv_gfn = !((fault_ipa & gpa_stolen_mask) == gpa_stolen_mask); >> + bool priv_exists = kvm_mem_is_private(kvm, gfn); >> + struct kvm_mmu_memory_cache *memcache = &vcpu->arch.mmu_page_cache; >> + int order; >> + kvm_pfn_t pfn; >> + int ret; >> + >> + if (priv_exists != is_priv_gfn) { >> + kvm_prepare_memory_fault_exit(vcpu, >> + fault_ipa & ~gpa_stolen_mask, >> + PAGE_SIZE, >> + kvm_is_write_fault(vcpu), >> + false, is_priv_gfn); >> + >> + return 0; >> + } >> + >> + if (!is_priv_gfn) { >> + /* Not a private mapping, handling normally */ >> + return -EAGAIN; >> + } >> + >> + if (kvm_gmem_get_pfn(kvm, memslot, gfn, &pfn, &order)) >> + return 1; /* Retry */ > > You don't need to pass a variable to hold the order if you don't need > it. You can pass NULL. Ah, good point - that simplifies things. > I am also confused about the return, why do you return 1 regardless of > the reason kvm_gmem_get_pfn() fails? Thinking about this, I don't think we actually expect kvm_gmem_get_pfn() to fail, so it's actually more appropriate to just pass return any error value. >> + ret = kvm_mmu_topup_memory_cache(memcache, >> + kvm_mmu_cache_min_pages(vcpu->arch.hw_mmu)); >> + if (ret) >> + return ret; > > If this fails you should release the page you got earlier (e.g., > kvm_release_pfn_clean()), or you could move it before > kvm_gmem_get_pfn(). Good point, however... >> + /* FIXME: Should be able to use bigger than PAGE_SIZE mappings */ >> + ret = realm_map_ipa(kvm, fault_ipa, pfn, PAGE_SIZE, KVM_PGTABLE_PROT_W, >> + memcache); >> + if (!ret) >> + return 1; /* Handled */ > > Should also release the page if it fails. Speaking of which, > where/when do you eventually release the page? ... I messed this up ;) It seems I'm managing to leak all guestmem pages. I'm not sure what I was thinking but I think I'd got it into my head guestmem wasn't reference counting the pages. I'll fix this up in the next version. Thanks, Steve From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 998BAC25B75 for ; Fri, 31 May 2024 16:04:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=RFwZH+bS0vbj13Xl+InE7U9EAa7oFjuzfELbDHMmKv8=; b=MuQq1T5rOaCBP7 BvMtCNPj7w6VXZwV8qOBxFaGjoY9A6xgjN4xed5rnjiyZK1gYN7FBaDJzztDZ/xNZkVq9GtC6ou+Q 7HEEF89L9Ludg7oXnqhXS0eNqf9XgsihXeGc6sTJ3mXuDyeAiChDqgcH3p3HqLAfBaZAhE25FaOTM t78BY0fajirUIg7KqgZ/K0mVS6kgeo/ReiaPMV3elJGJGnIpzD4e7cNQ15VrGsD6s2lfS2tPEuT94 LgXv0AEU4eoOEje4/+rpbE5Nu3iZO9eJYyN7Urm+bhr3jtPKtT/iwxJzr4g/YBpcorwCdx8vJRb+H awRO7iG9XXd3HgGi6WBA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sD4jg-0000000AlLQ-1AeR; Fri, 31 May 2024 16:04:08 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sD4jc-0000000AlKJ-3AcU for linux-arm-kernel@lists.infradead.org; Fri, 31 May 2024 16:04:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6A6631424; Fri, 31 May 2024 09:04:24 -0700 (PDT) Received: from [10.1.27.19] (e122027.cambridge.arm.com [10.1.27.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7A9A33F792; Fri, 31 May 2024 09:03:56 -0700 (PDT) Message-ID: Date: Fri, 31 May 2024 17:03:54 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 21/43] arm64: RME: Runtime faulting of memory To: Fuad Tabba Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, Catalin Marinas , Marc Zyngier , Will Deacon , James Morse , Oliver Upton , Suzuki K Poulose , Zenghui Yu , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Joey Gouly , Alexandru Elisei , Christoffer Dall , linux-coco@lists.linux.dev, Ganapatrao Kulkarni References: <20240412084056.1733704-1-steven.price@arm.com> <20240412084309.1733783-1-steven.price@arm.com> <20240412084309.1733783-22-steven.price@arm.com> From: Steven Price Content-Language: en-GB In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240531_090404_926109_000A9188 X-CRM114-Status: GOOD ( 22.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org T24gMjUvMDQvMjAyNCAxMTo0MywgRnVhZCBUYWJiYSB3cm90ZToKPiBIaSwKCkhpLAoKVGhhbmtz IGZvciB0aGUgcmV2aWV3LiBTb3JyeSBJIGRpZG4ndCByZXNwb25kIGVhcmxpZXIuCgo+IE9uIEZy aSwgQXByIDEyLCAyMDI0IGF0IDk6NDTigK9BTSBTdGV2ZW4gUHJpY2UgPHN0ZXZlbi5wcmljZUBh cm0uY29tPiB3cm90ZToKPj4KPHNuaXA+Cj4+ICtzdGF0aWMgaW50IHByaXZhdGVfbWVtc2xvdF9m YXVsdChzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUsCj4+ICsgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIHBoeXNfYWRkcl90IGZhdWx0X2lwYSwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgc3RydWN0IGt2bV9tZW1vcnlfc2xvdCAqbWVtc2xvdCkKPj4gK3sKPj4gKyAgICAg ICBzdHJ1Y3Qga3ZtICprdm0gPSB2Y3B1LT5rdm07Cj4+ICsgICAgICAgZ3BhX3QgZ3BhX3N0b2xl bl9tYXNrID0ga3ZtX2dwYV9zdG9sZW5fYml0cyhrdm0pOwo+PiArICAgICAgIGdmbl90IGdmbiA9 IChmYXVsdF9pcGEgJiB+Z3BhX3N0b2xlbl9tYXNrKSA+PiBQQUdFX1NISUZUOwo+PiArICAgICAg IGJvb2wgaXNfcHJpdl9nZm4gPSAhKChmYXVsdF9pcGEgJiBncGFfc3RvbGVuX21hc2spID09IGdw YV9zdG9sZW5fbWFzayk7Cj4+ICsgICAgICAgYm9vbCBwcml2X2V4aXN0cyA9IGt2bV9tZW1faXNf cHJpdmF0ZShrdm0sIGdmbik7Cj4+ICsgICAgICAgc3RydWN0IGt2bV9tbXVfbWVtb3J5X2NhY2hl ICptZW1jYWNoZSA9ICZ2Y3B1LT5hcmNoLm1tdV9wYWdlX2NhY2hlOwo+PiArICAgICAgIGludCBv cmRlcjsKPj4gKyAgICAgICBrdm1fcGZuX3QgcGZuOwo+PiArICAgICAgIGludCByZXQ7Cj4+ICsK Pj4gKyAgICAgICBpZiAocHJpdl9leGlzdHMgIT0gaXNfcHJpdl9nZm4pIHsKPj4gKyAgICAgICAg ICAgICAgIGt2bV9wcmVwYXJlX21lbW9yeV9mYXVsdF9leGl0KHZjcHUsCj4+ICsgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBmYXVsdF9pcGEgJiB+Z3BhX3N0b2xl bl9tYXNrLAo+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg UEFHRV9TSVpFLAo+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAga3ZtX2lzX3dyaXRlX2ZhdWx0KHZjcHUpLAo+PiArICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgZmFsc2UsIGlzX3ByaXZfZ2ZuKTsKPj4gKwo+PiArICAgICAg ICAgICAgICAgcmV0dXJuIDA7Cj4+ICsgICAgICAgfQo+PiArCj4+ICsgICAgICAgaWYgKCFpc19w cml2X2dmbikgewo+PiArICAgICAgICAgICAgICAgLyogTm90IGEgcHJpdmF0ZSBtYXBwaW5nLCBo YW5kbGluZyBub3JtYWxseSAqLwo+PiArICAgICAgICAgICAgICAgcmV0dXJuIC1FQUdBSU47Cj4+ ICsgICAgICAgfQo+PiArCj4+ICsgICAgICAgaWYgKGt2bV9nbWVtX2dldF9wZm4oa3ZtLCBtZW1z bG90LCBnZm4sICZwZm4sICZvcmRlcikpCj4+ICsgICAgICAgICAgICAgICByZXR1cm4gMTsgLyog UmV0cnkgKi8KPiAKPiBZb3UgZG9uJ3QgbmVlZCB0byBwYXNzIGEgdmFyaWFibGUgdG8gaG9sZCB0 aGUgb3JkZXIgaWYgeW91IGRvbid0IG5lZWQKPiBpdC4gWW91IGNhbiBwYXNzIE5VTEwuCgpBaCwg Z29vZCBwb2ludCAtIHRoYXQgc2ltcGxpZmllcyB0aGluZ3MuCgo+IEkgYW0gYWxzbyBjb25mdXNl ZCBhYm91dCB0aGUgcmV0dXJuLCB3aHkgZG8geW91IHJldHVybiAxIHJlZ2FyZGxlc3Mgb2YKPiB0 aGUgcmVhc29uIGt2bV9nbWVtX2dldF9wZm4oKSBmYWlscz8KClRoaW5raW5nIGFib3V0IHRoaXMs IEkgZG9uJ3QgdGhpbmsgd2UgYWN0dWFsbHkgZXhwZWN0IGt2bV9nbWVtX2dldF9wZm4oKQp0byBm YWlsLCBzbyBpdCdzIGFjdHVhbGx5IG1vcmUgYXBwcm9wcmlhdGUgdG8ganVzdCBwYXNzIHJldHVy biBhbnkgZXJyb3IKdmFsdWUuCgo+PiArICAgICAgIHJldCA9IGt2bV9tbXVfdG9wdXBfbWVtb3J5 X2NhY2hlKG1lbWNhY2hlLAo+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgIGt2bV9tbXVfY2FjaGVfbWluX3BhZ2VzKHZjcHUtPmFyY2guaHdfbW11KSk7Cj4+ICsgICAg ICAgaWYgKHJldCkKPj4gKyAgICAgICAgICAgICAgIHJldHVybiByZXQ7Cj4gCj4gSWYgdGhpcyBm YWlscyB5b3Ugc2hvdWxkIHJlbGVhc2UgdGhlIHBhZ2UgeW91IGdvdCBlYXJsaWVyIChlLmcuLAo+ IGt2bV9yZWxlYXNlX3Bmbl9jbGVhbigpKSwgb3IgeW91IGNvdWxkIG1vdmUgaXQgYmVmb3JlCj4g a3ZtX2dtZW1fZ2V0X3BmbigpLgoKR29vZCBwb2ludCwgaG93ZXZlci4uLgoKPj4gKyAgICAgICAv KiBGSVhNRTogU2hvdWxkIGJlIGFibGUgdG8gdXNlIGJpZ2dlciB0aGFuIFBBR0VfU0laRSBtYXBw aW5ncyAqLwo+PiArICAgICAgIHJldCA9IHJlYWxtX21hcF9pcGEoa3ZtLCBmYXVsdF9pcGEsIHBm biwgUEFHRV9TSVpFLCBLVk1fUEdUQUJMRV9QUk9UX1csCj4+ICsgICAgICAgICAgICAgICAgICAg ICAgICAgICAgbWVtY2FjaGUpOwo+PiArICAgICAgIGlmICghcmV0KQo+PiArICAgICAgICAgICAg ICAgcmV0dXJuIDE7IC8qIEhhbmRsZWQgKi8KPiAKPiBTaG91bGQgYWxzbyByZWxlYXNlIHRoZSBw YWdlIGlmIGl0IGZhaWxzLiBTcGVha2luZyBvZiB3aGljaCwKPiB3aGVyZS93aGVuIGRvIHlvdSBl dmVudHVhbGx5IHJlbGVhc2UgdGhlIHBhZ2U/CgouLi4gSSBtZXNzZWQgdGhpcyB1cCA7KSBJdCBz ZWVtcyBJJ20gbWFuYWdpbmcgdG8gbGVhayBhbGwgZ3Vlc3RtZW0KcGFnZXMuIEknbSBub3Qgc3Vy ZSB3aGF0IEkgd2FzIHRoaW5raW5nIGJ1dCBJIHRoaW5rIEknZCBnb3QgaXQgaW50byBteQpoZWFk IGd1ZXN0bWVtIHdhc24ndCByZWZlcmVuY2UgY291bnRpbmcgdGhlIHBhZ2VzLiBJJ2xsIGZpeCB0 aGlzIHVwIGluCnRoZSBuZXh0IHZlcnNpb24uCgpUaGFua3MsCgpTdGV2ZQoKCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFybS1rZXJuZWwgbWFp bGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlz dHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK