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 D4DB4C433FE for ; Mon, 21 Nov 2022 13:57:42 +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-Type: Content-Transfer-Encoding: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=dZrL34zPIZpJzivqFX/3nBKbd7bXnlyympM1k50s+PI=; b=Bix33IZgT0DTa6 Dp87y0J8fu0pEkBedR+uC8aFTzjn+8UfY5lEi9EvhWz45O18bWsXaDSbEEm9DDX3Klg0pn5xy038w tTpuHgtUiW4V2QrTL/5TO4Cg3CfiNJD+KfNzjuzQO/0CQW9S1YzL8/hU2cZKn2n+cx5oSDLJCCoU5 PjMop2e62KxR+khzaj9HKFbOufdTWq3ILFHmJWZTF4zVK1eYaKTEywgGxKDQViqVNOkT/cwP1iqKG 0CFWUVfbHE+ZjVVv//RFpPnV1B8OkfVZzSxz0MDfkpWBH3lqFJ9ShaTI5sJmShPNcFzHybU2c6+sR W5Gfo3egsYI2TltFqF1w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ox7Ht-00E9B1-J3; Mon, 21 Nov 2022 13:56:41 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ox7Hq-00E98k-1I for linux-arm-kernel@lists.infradead.org; Mon, 21 Nov 2022 13:56:40 +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 007C21FB; Mon, 21 Nov 2022 05:56:36 -0800 (PST) Received: from [10.57.71.118] (unknown [10.57.71.118]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 239AE3F73B; Mon, 21 Nov 2022 05:56:28 -0800 (PST) Message-ID: <5b69ff03-1694-bae6-3312-a63273be4073@arm.com> Date: Mon, 21 Nov 2022 13:56:22 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH] arm64/mm: Drop redundant BUG_ON(!pgtable_alloc) Content-Language: en-GB To: Mark Rutland , Anshuman Khandual Cc: Nathan Chancellor , linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , Andrew Morton , linux-kernel@vger.kernel.org, llvm@lists.linux.dev References: <20221118053102.500216-1-anshuman.khandual@arm.com> From: Robin Murphy In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221121_055638_217665_590241C6 X-CRM114-Status: GOOD ( 21.91 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2022-11-21 12:27, Mark Rutland wrote: > On Mon, Nov 21, 2022 at 11:00:42AM +0530, Anshuman Khandual wrote: >> Hello Nathan, >> >> Thanks for the report. >> >> On 11/20/22 21:46, Nathan Chancellor wrote: >>> Hi Anshuman, > >>> I just bisected a boot failure in our QEMU-based continuous integration >>> setup to this change as commit 9ed2b4616d4e ("arm64/mm: Drop redundant >>> BUG_ON(!pgtable_alloc)") in the arm64 tree. There is no output so the >>> panic clearly happens early at boot. If I move back to the previous >>> commit and add a WARN_ON() like so: >>> >>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >>> index d386033a074c..9280a92ff920 100644 >>> --- a/arch/arm64/mm/mmu.c >>> +++ b/arch/arm64/mm/mmu.c >>> @@ -383,6 +383,7 @@ static void __create_pgd_mapping_locked(pgd_t *pgdir, phys_addr_t phys, >>> phys &= PAGE_MASK; >>> addr = virt & PAGE_MASK; >>> end = PAGE_ALIGN(virt + size); >>> + WARN_ON(!pgtable_alloc); >>> >>> do { >>> next = pgd_addr_end(addr, end); >>> >>> I do see some stacktraces. I have attached the boot log from QEMU. >>> >>> If there is any additional information I can provide or patches I can >>> test, I am more than happy to do so. >> >> There are couple of instances, where __create_pgd_mapping() function gets called >> without a valid pgtable alloc function (NULL is passed on instead), as it is not >> expected to allocate page table pages, during the mapping process. The following >> change after this patch should solve the reported problem. >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> index 9ea8e9039992..a00563122fcb 100644 >> --- a/arch/arm64/mm/mmu.c >> +++ b/arch/arm64/mm/mmu.c >> @@ -42,6 +42,7 @@ >> #define NO_BLOCK_MAPPINGS BIT(0) >> #define NO_CONT_MAPPINGS BIT(1) >> #define NO_EXEC_MAPPINGS BIT(2) /* assumes FEAT_HPDS is not used */ >> +#define NO_ALLOC_MAPPINGS BIT(3) /* does not allocate page table pages */ >> >> int idmap_t0sz __ro_after_init; >> >> @@ -380,7 +381,7 @@ static void __create_pgd_mapping_locked(pgd_t *pgdir, phys_addr_t phys, >> phys &= PAGE_MASK; >> addr = virt & PAGE_MASK; >> end = PAGE_ALIGN(virt + size); >> - BUG_ON(!pgtable_alloc); >> + BUG_ON(!(flags & NO_ALLOC_MAPPINGS) && !pgtable_alloc); >> >> do { >> next = pgd_addr_end(addr, end); >> @@ -453,7 +454,7 @@ static void __init create_mapping_noalloc(phys_addr_t phys, unsigned long virt, >> return; >> } >> __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, >> - NO_CONT_MAPPINGS); >> + NO_CONT_MAPPINGS | NO_ALLOC_MAPPINGS); >> } >> >> void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys, >> @@ -481,7 +482,7 @@ static void update_mapping_prot(phys_addr_t phys, unsigned long virt, >> } >> >> __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, >> - NO_CONT_MAPPINGS); >> + NO_CONT_MAPPINGS | NO_ALLOC_MAPPINGS); >> >> /* flush the TLBs after updating live kernel mappings */ >> flush_tlb_kernel_range(virt, virt + size); > > This is now more complicated than what we had originally, and it doesn't catch > the case where the caller sets NO_ALLOC_MAPPINGS but the callee ends up needing > to perform an allocation, which the old code would have caught. Well, it's still "caught" as such - all that BUG_ON(!pgtable_alloc) does in these cases is encode the source location in the backtrace, vs. having to decode it (if necessary) from the LR in a backtrace from immediately dereferencing pgtable_alloc(). If that happens before the user has a console up then the difference is moot anyway. Cheers, Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel