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 X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 51174C433E0 for ; Wed, 27 May 2020 09:29:18 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 1DBFF20890 for ; Wed, 27 May 2020 09:29:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ITCS9RKW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1DBFF20890 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=YJMOcHWFS4iqvcZANKrrWc0hngtIdCJ+B9lI3BaDikw=; b=ITCS9RKWQZxqL3 f0noBAdM35g/zqfDljuiifFV8S4OVbf9t+KkiOh4zgNYRhW7wi1/+LRAP1sntAsm5eOsk6wiDjfF8 7OCD3UPceEximn2M25Uia2VWLnQfd7NdgFXT5lD3TGvPhmLuq5tE8xk0W2FeNqWWI8A3cuFzPaOFd 75QDSWxfg8qTBogcnGNPNYKTN4QxT3j51cfWi8oUsofRfQE0ovsW0+o0w8+8f5UJ9zDr5SFHstOOT ogxhMBzwCGR+x3fnIuhkCf9e08Fq7g53mpudbapAQ0QGZcAG1JaxP2SqC9dLehv9MClkDJSTyxszo Xe17YDCCCHOw0DQKjtbA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jdsNB-0007Ij-E4; Wed, 27 May 2020 09:29:17 +0000 Received: from szxga05-in.huawei.com ([45.249.212.191] helo=huawei.com) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jdsN6-0007G6-Su for linux-arm-kernel@lists.infradead.org; Wed, 27 May 2020 09:29:15 +0000 Received: from DGGEMS401-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id 0B8E0E4858C83D8465D0; Wed, 27 May 2020 17:28:58 +0800 (CST) Received: from [10.173.221.230] (10.173.221.230) by DGGEMS401-HUB.china.huawei.com (10.3.19.201) with Microsoft SMTP Server id 14.3.487.0; Wed, 27 May 2020 17:28:51 +0800 Subject: Re: [RFC PATCH 2/7] KVM: arm64: Set DBM bit of PTEs if hw DBM enabled To: Catalin Marinas References: <20200525112406.28224-1-zhukeqian1@huawei.com> <20200525112406.28224-3-zhukeqian1@huawei.com> <20200526114926.GD17051@gaia> From: zhukeqian Message-ID: <01147c57-b45e-0a40-da9a-4a0e56aac78d@huawei.com> Date: Wed, 27 May 2020 17:28:39 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20200526114926.GD17051@gaia> X-Originating-IP: [10.173.221.230] X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200527_022913_104639_0DA4F207 X-CRM114-Status: GOOD ( 17.16 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Morton , kvm@vger.kernel.org, Suzuki K Poulose , Marc Zyngier , linux-kernel@vger.kernel.org, Sean Christopherson , Peng Liang , Alexios Zavras , zhengxiang9@huawei.com, Mark Brown , James Morse , Julien Thierry , wanghaibin.wang@huawei.com, Thomas Gleixner , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Catalin, On 2020/5/26 19:49, Catalin Marinas wrote: > On Mon, May 25, 2020 at 07:24:01PM +0800, Keqian Zhu wrote: >> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h >> index 1305e28225fc..f9910ba2afd8 100644 >> --- a/arch/arm64/include/asm/pgtable-prot.h >> +++ b/arch/arm64/include/asm/pgtable-prot.h >> @@ -79,6 +79,7 @@ extern bool arm64_use_ng_mappings; >> }) >> >> #define PAGE_S2 __pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PAGE_S2_XN) >> +#define PAGE_S2_DBM __pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PAGE_S2_XN | PTE_DBM) > > You don't need a new page permission (see below). > >> #define PAGE_S2_DEVICE __pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN) >> >> #define PAGE_NONE __pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) | PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN) >> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c >> index e3b9ee268823..dc97988eb2e0 100644 >> --- a/virt/kvm/arm/mmu.c >> +++ b/virt/kvm/arm/mmu.c >> @@ -1426,6 +1426,10 @@ static void stage2_wp_ptes(pmd_t *pmd, phys_addr_t addr, phys_addr_t end) >> pte = pte_offset_kernel(pmd, addr); >> do { >> if (!pte_none(*pte)) { >> +#ifdef CONFIG_ARM64_HW_AFDBM >> + if (kvm_hw_dbm_enabled() && !kvm_s2pte_dbm(pte)) >> + kvm_set_s2pte_dbm(pte); >> +#endif >> if (!kvm_s2pte_readonly(pte)) >> kvm_set_s2pte_readonly(pte); >> } > > Setting the DBM bit is equivalent to marking the page writable. The > actual writable pte bit (S2AP[1] or HAP[2] as we call them in Linux for > legacy reasons) tells you whether the page has been dirtied but it is > still writable if you set DBM. Doing this in stage2_wp_ptes() > practically means that you no longer have read-only pages at S2. There > are several good reasons why you don't want to break this. For example, > the S2 pte may already be read-only for other reasons (CoW). > Thanks, your comments help to solve the first problem in cover letter. > I think you should only set the DBM bit if the pte was previously > writable. In addition, any permission change to the S2 pte must take > into account the DBM bit and clear it while transferring the dirty > status to the underlying page. I'm not deeply familiar with all these > callbacks into KVM but two such paths are kvm_unmap_hva_range() and the > kvm_mmu_notifier_change_pte(). Yes, I agree. > > >> @@ -1827,7 +1831,15 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> >> ret = stage2_set_pmd_huge(kvm, memcache, fault_ipa, &new_pmd); >> } else { >> - pte_t new_pte = kvm_pfn_pte(pfn, mem_type); >> + pte_t new_pte; >> + >> +#ifdef CONFIG_ARM64_HW_AFDBM >> + if (kvm_hw_dbm_enabled() && >> + pgprot_val(mem_type) == pgprot_val(PAGE_S2)) { >> + mem_type = PAGE_S2_DBM; >> + } >> +#endif >> + new_pte = kvm_pfn_pte(pfn, mem_type); >> >> if (writable) { >> new_pte = kvm_s2pte_mkwrite(new_pte); > > That's wrong here. Basically for any fault you get, you just turn the S2 > page writable. The point of DBM is that you don't get write faults at > all if you have a writable page. So, as I said above, only set the DBM > bit if you stored a writable S2 pte (kvm_s2pte_mkwrite()). Yeah, you are right. I will correct it in Patch v1. > Thanks, Keqian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel