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=-8.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,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 A1BE4C433E2 for ; Thu, 3 Sep 2020 16:38:24 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 67D1020578 for ; Thu, 3 Sep 2020 16:38:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="KvlNLj9a"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="BGe4KI1N" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 67D1020578 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=merlin.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:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ga8HbPa4tqs/yulDWosL6aWwMNWjytAUngiyZVgycFo=; b=KvlNLj9afUxUSkYW1thtxcdm0 E5/1+MR2njsBSAQQ1SliXJJVyeDzZT8Oryo9CjhrJP1CB+XDZt0Fs8hYQ1KqFyZzfG+WkS35BpQfa siz4bP2FHLA4xDLkwHqhBe3i7mVNEK5NWwLSZQe8fztXLxKAtGNfxAN0TgIzYaH+/LjUYSUIKBBiX JMMyqog2ThdcoaWmhK19+o2VaWefNxcbduXX7dh1VLxScqqe+i92hO2gxFxrhQcmH2/AysBandnSa Z4xY7y8zgl7fpHAlLTpUYYmJMl/BNv6i+SCaBFbzLhJU6tiu6tRwzwtGsPTtOXC2xgOdD75JtvRUp yI1v2kiXg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kDsEh-0000jZ-2O; Thu, 03 Sep 2020 16:37:19 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kDsEe-0000iy-IV for linux-arm-kernel@lists.infradead.org; Thu, 03 Sep 2020 16:37:17 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BC65720578; Thu, 3 Sep 2020 16:37:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599151035; bh=V2uzEbJjL0nt32Wj65d8TvDfKg2z8pr91NbW9R/dFhY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=BGe4KI1NK2vcwg2DJp6tqgfHl265Uk+t//FyKmVB16N9OJLdhncn3/M7zxZib0Vbb elC+6fAUiQ6VTGqa2PBy/n844v+qq9FQ52H5cwqFUciaezbxnT+VLeL+vA3EQZbZo3 V21DWyFEPPhMObCf466TlSNfDnXiiWsabI0spH6I= Date: Thu, 3 Sep 2020 17:37:11 +0100 From: Will Deacon To: Alexandru Elisei Subject: Re: [PATCH v3 08/21] KVM: arm64: Convert kvm_set_spte_hva() to generic page-table API Message-ID: <20200903163711.GA7791@willie-the-truck> References: <20200825093953.26493-1-will@kernel.org> <20200825093953.26493-9-will@kernel.org> <86bb295c-64d8-51a1-25f4-32a5a1bb2172@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <86bb295c-64d8-51a1-25f4-32a5a1bb2172@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200903_123716_727141_21882784 X-CRM114-Status: GOOD ( 22.91 ) 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: Marc Zyngier , kernel-team@android.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Catalin Marinas Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Sep 02, 2020 at 04:37:18PM +0100, Alexandru Elisei wrote: > On 8/25/20 10:39 AM, Will Deacon wrote: > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > > index 33146d3dc93a..704b471a48ce 100644 > > --- a/arch/arm64/kvm/mmu.c > > +++ b/arch/arm64/kvm/mmu.c > > @@ -1911,28 +1911,27 @@ int kvm_unmap_hva_range(struct kvm *kvm, > > > > static int kvm_set_spte_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *data) > > { > > - pte_t *pte = (pte_t *)data; > > + kvm_pfn_t *pfn = (kvm_pfn_t *)data; > > > > WARN_ON(size != PAGE_SIZE); > > + > > /* > > - * We can always call stage2_set_pte with KVM_S2PTE_FLAG_LOGGING_ACTIVE > > - * flag clear because MMU notifiers will have unmapped a huge PMD before > > - * calling ->change_pte() (which in turn calls kvm_set_spte_hva()) and > > - * therefore stage2_set_pte() never needs to clear out a huge PMD > > - * through this calling path. > > + * The MMU notifiers will have unmapped a huge PMD before calling > > + * ->change_pte() (which in turn calls kvm_set_spte_hva()) and > > + * therefore we never need to clear out a huge PMD through this > > + * calling path and a memcache is not required. > > */ > > - stage2_set_pte(&kvm->arch.mmu, NULL, gpa, pte, 0); > > + kvm_pgtable_stage2_map(kvm->arch.mmu.pgt, gpa, PAGE_SIZE, > > + __pfn_to_phys(*pfn), KVM_PGTABLE_PROT_R, NULL); > > I have to admit that I managed to confuse myself. > > According to the comment, this is called after unmapping a huge PMD. > __unmap_stage2_range() -> .. -> unmap_stage2_pmd() calls pmd_clear(), which means > the PMD entry is now 0. > > In __kvm_pgtable_visit(), kvm_pte_table() returns false, because the entry is > invalid, and so we call stage2_map_walk_leaf(). Here, stage2_map_walker_try_leaf() > will return false, because kvm_block_mapping_supported() returns false (PMD > granule is larger than PAGE_SIZE), and then we end up allocating a table from the > memcache. memcache which will NULL, and kvm_mmu_memory_cache_alloc() will > dereference the NULL pointer. > > I'm pretty sure there's something that I'm missing here, I would really appreciate > someone pointing out where I'm making a mistake. You're not missing anything, and this is actually a bug introduced by moving to the generic mmu cache code. My old implementation (which you can still see in the earlier patch) returns NULL if the cache is NULL, so I'll need to reintroduce that check here. This then mimics the current behaviour of ignoring map requests from the MMU if we need to allocate, and instead handling them lazily when we take the fault. Well spotted! Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel