From patchwork Wed Jan 14 10:32:33 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 43097 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f71.google.com (mail-wg0-f71.google.com [74.125.82.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id A0518240B9 for ; Wed, 14 Jan 2015 10:34:17 +0000 (UTC) Received: by mail-wg0-f71.google.com with SMTP id k14sf4389502wgh.2 for ; Wed, 14 Jan 2015 02:34:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent :cc:precedence:list-id:list-unsubscribe:list-archive:list-post :list-help:list-subscribe:content-type:content-transfer-encoding :sender:errors-to:x-original-sender :x-original-authentication-results:mailing-list; bh=8xiHbTZhtmaH0y9sVPmnjcVv1zqwpUa81kO0nhscF/g=; b=FXDtWR441/aurw9+h7RKWWjlUlAlw0ARK8/lmlhB0EGyLAxdRFqA3J1YNHXS+E2sVR hSY9ODYwR0VySDJH2DCX/A+cU1B3a77VqdBWS1QJSTOdkd/924oGWvq6t5QIzP2NO/Hg 6PS1FhSnLkhUkvOQrjSX+VCktiNwmLgr/u842+qJ7sqzgHEUjLHhJ+zGFjtucLUYODIf G5D7eqYmmClkC9n4PUG5q1HoHdwcL0GEvHz5EWy+wSOBmFoi8tR3acAHBuCmB75c7a9G h0SbnOvJ+P+ELgYNfRQmfE3vpiX+hR2b22adcucSoTZUjI9ZuIe0xBO9wccYc1w6JDxO 6S/w== X-Gm-Message-State: ALoCoQnwT9shIkIVqWb9gkMv7ktoSmw2WRjiQW/zDbIlghiHpC7rSiZ8s4r0G6SYMEIWhgDgVR7B X-Received: by 10.152.44.225 with SMTP id h1mr402586lam.2.1421231656907; Wed, 14 Jan 2015 02:34:16 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.87.134 with SMTP id ay6ls98197lab.107.gmail; Wed, 14 Jan 2015 02:34:16 -0800 (PST) X-Received: by 10.152.7.206 with SMTP id l14mr3133448laa.1.1421231656764; Wed, 14 Jan 2015 02:34:16 -0800 (PST) Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com. [209.85.217.181]) by mx.google.com with ESMTPS id fb6si26772483lbc.117.2015.01.14.02.34.16 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 14 Jan 2015 02:34:16 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.181 as permitted sender) client-ip=209.85.217.181; Received: by mail-lb0-f181.google.com with SMTP id l4so7179229lbv.12 for ; Wed, 14 Jan 2015 02:34:16 -0800 (PST) X-Received: by 10.112.90.170 with SMTP id bx10mr3069314lbb.69.1421231656592; Wed, 14 Jan 2015 02:34:16 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.9.200 with SMTP id c8csp1649713lbb; Wed, 14 Jan 2015 02:34:15 -0800 (PST) X-Received: by 10.66.253.34 with SMTP id zx2mr4598221pac.96.1421231654641; Wed, 14 Jan 2015 02:34:14 -0800 (PST) Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id xg13si30365070pac.74.2015.01.14.02.34.13 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Jan 2015 02:34:14 -0800 (PST) Received-SPF: none (google.com: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org does not designate permitted sender hosts) client-ip=2001:1868:205::9; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YBLFz-0004AB-0A; Wed, 14 Jan 2015 10:32:59 +0000 Received: from mail-la0-f41.google.com ([209.85.215.41]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YBLFu-00043B-SK for linux-arm-kernel@lists.infradead.org; Wed, 14 Jan 2015 10:32:55 +0000 Received: by mail-la0-f41.google.com with SMTP id hv19so7336069lab.0 for ; Wed, 14 Jan 2015 02:32:32 -0800 (PST) X-Received: by 10.152.5.132 with SMTP id s4mr3083991las.39.1421231552014; Wed, 14 Jan 2015 02:32:32 -0800 (PST) Received: from localhost (188-178-240-98-static.dk.customer.tdc.net. [188.178.240.98]) by mx.google.com with ESMTPSA id j19sm5756652lbl.23.2015.01.14.02.32.30 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 14 Jan 2015 02:32:31 -0800 (PST) Date: Wed, 14 Jan 2015 11:32:33 +0100 From: Christoffer Dall To: Mario Smarduch Subject: Re: [PATCH RESEND v15 07/10] KVM: arm: page logging 2nd stage fault handling Message-ID: <20150114103233.GI26222@cbox> References: <1420863441-32592-1-git-send-email-m.smarduch@samsung.com> <20150111140003.GA2722@cbox> <54B3F5D7.4010501@samsung.com> <20150112174927.GM3868@cbox> <54B5A6E7.6090904@samsung.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <54B5A6E7.6090904@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150114_023255_102334_AD4952E8 X-CRM114-Status: GOOD ( 21.52 ) X-Spam-Score: -0.7 (/) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-0.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.215.41 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3) [209.85.215.41 listed in wl.mailspike.net] -0.0 RCVD_IN_MSPIKE_WL Mailspike good senders Cc: marc.zyngier@arm.com, pbonzini@redhat.com, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: christoffer.dall@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.181 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 On Tue, Jan 13, 2015 at 03:14:47PM -0800, Mario Smarduch wrote: [...] > >>> > >>> > >>> What I meant last time around concerning user_mem_abort was more > >>> something like this: > >>> > >>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > >>> index 1dc9778..38ea58e 100644 > >>> --- a/arch/arm/kvm/mmu.c > >>> +++ b/arch/arm/kvm/mmu.c > >>> @@ -935,7 +935,14 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > >>> return -EFAULT; > >>> } > >>> > >>> - if (is_vm_hugetlb_page(vma)) { > >>> + /* > >>> + * Writes to pages in a memslot with logging enabled are always logged > >>> + * on a singe page-by-page basis. > >>> + */ > >>> + if (memslot_is_logging(memslot) && write_fault) > >>> + force_pte = true; > >> > >> If it's a write you take the pte route and > >> dissolves huge page, if it's a read you reconstruct the > >> THP that seems to yield pretty bad results. > > > > ok, then remove the ' && write_fault' part of the clause. > Hi Christoffer, > couple comments/questions. > > setting force_pte here, disables huge pages for > non-writable regions. > hmmm, by a non-writable region you mean a read-only memslot? Can you set dirty page logging for such one? That doesn't make much sense to me. Note, that if you receive writable == false from gfn_to_pfn_prot() that doesn't mean that the page can never be written to, it just means that the current mapping of the page is not a writable one, you can call that same function again later with write_fault=true, and you either get a writable page back or you simply get an error. [...] > >>> if (kvm_is_device_pfn(pfn)) > >>> mem_type = PAGE_S2_DEVICE; > > If we're not setting the IOMAP flag do we have need > this, since we're forfeiting error checking later > in stage2_set_pte()? > we still need this, remember the error checking is about cache == NULL, not about the IOMAP flag. I think I address this in the new proposal below, but please check carefully. Take a look at this one: Thanks, -Christoffer diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 1dc9778..841e053 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -919,6 +919,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, pfn_t pfn; pgprot_t mem_type = PAGE_S2; bool fault_ipa_uncached; + unsigned long flags = 0; write_fault = kvm_is_write_fault(vcpu); if (fault_status == FSC_PERM && !write_fault) { @@ -976,8 +977,26 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, if (is_error_pfn(pfn)) return -EFAULT; - if (kvm_is_device_pfn(pfn)) + if (kvm_is_device_pfn(pfn)) { mem_type = PAGE_S2_DEVICE; + flags |= KVM_S2PTE_FLAG_IS_IOMAP; + } else if (memslot_is_logging(memslot)) { + /* + * Faults on pages in a memslot with logging enabled that can + * should not be mapped with huge pages (it introduces churn + * and performance degradation), so force a pte mapping. + */ + force_pte = true; + flags |= KVM_S2_FLAG_LOGGING_ACTIVE; + + /* + * Only actually map the page as writable if this was a write + * fault. + */ + if (!write_fault) + writable = false; + + } spin_lock(&kvm->mmu_lock); if (mmu_notifier_retry(kvm, mmu_seq)) @@ -1002,13 +1021,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, if (writable) { kvm_set_s2pte_writable(&new_pte); kvm_set_pfn_dirty(pfn); + mark_page_dirty(kvm, gfn); } coherent_cache_guest_page(vcpu, hva, PAGE_SIZE, fault_ipa_uncached); - ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, - pgprot_val(mem_type) == pgprot_val(PAGE_S2_DEVICE)); - } + ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, flags); + } out_unlock: spin_unlock(&kvm->mmu_lock);