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=-9.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 52FC4C48BCD for ; Wed, 9 Jun 2021 22:44:34 +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 1998861364 for ; Wed, 9 Jun 2021 22:44:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1998861364 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=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=W0e4g9VYmvKwKJH7o1iq3HbkCp3aGVcGmn73i8Di3dk=; b=sMt+Y5zxhtsR0C +VhMCosE3Aj2byQ3nyvsjPShq6qU89LiUZVogr15ciMV8B8AXJ6w/06hzTnUJF3xPhFOQV+pwIT3O K8LSXc54aLZ14vSHStOF+uBRm6SD0AjhtHSjfMbj8+hD1ryTLbN/ygZNZ9YaWkwOderz1U8p753XL EIYlXytwbJT36x9hwFnWfVDnzkeQRzC5sCEpxXMKHNt90m49FZOyz1Qvy/z9V4M0wA+9NRMQdmdTW XO1p+5UQNL3zLW69BZrYqV/aPLialiR5s1AcsuNM966tDovTzbqvv71lwOSSkQN3WYu4pB+sENNu5 GmDGnJRn4U57SgrRuD8A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lr6th-00G0az-P4; Wed, 09 Jun 2021 22:42:05 +0000 Received: from mail-pj1-f52.google.com ([209.85.216.52]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lr6tc-00G0Pw-V8; Wed, 09 Jun 2021 22:42:02 +0000 Received: by mail-pj1-f52.google.com with SMTP id o17-20020a17090a9f91b029015cef5b3c50so2530512pjp.4; Wed, 09 Jun 2021 15:39:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=y9rdhpixbvXKHcp1SoNHlksJfvedlhffce9YZeswBsw=; b=rBuzLQq8dSqUZv6/ROLuDpQ+P7TNIJUsMaw+rt/rsrNcVujQzzRtIftHcA91YqBMyD EUlDtjcgjc6OkdfCug6bDib+8nUHv/XJhkgcr2HmSo09FtzAQqNBvu0Hk6SbNuCU/fcV yjuGn2uzHmIsVt20jeQ4WpYETaHCTBlBRkDNzCG4t0XBjKRktsJOKEKfpJxAWMZq4qCp cfzcE5jSfkYrlWd5wzQkD8OAej9J52wiaXJhErGnrJhQGvzhKa0cYi0tAakatBNKrPVe 2rlbUmBraHFu0wAiyw2fNtc0GY55x7ctwljvYyHb40HyI2jQnE7zbql3DGiBwMqx8q66 sMjQ== X-Gm-Message-State: AOAM5302RnCmHsY1fLfDUViu3Lb8+eAERC1/qyMpgVLaoJlNZiW2l35f 5IjjtJKfr0D694mquPkBqeI= X-Google-Smtp-Source: ABdhPJyDXjv7HuKGn5UkuMCDJNjsFiwMiWtho+OwdB1IxhZNTTlHGnoFatthbWzFvqr05U2Tjzj8lQ== X-Received: by 2002:a17:90a:31c4:: with SMTP id j4mr13156394pjf.105.1623278398559; Wed, 09 Jun 2021 15:39:58 -0700 (PDT) Received: from localhost ([2601:647:5b00:1161:a4cc:eef9:fbc0:2781]) by smtp.gmail.com with ESMTPSA id r19sm470332pfh.152.2021.06.09.15.39.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Jun 2021 15:39:58 -0700 (PDT) Date: Wed, 9 Jun 2021 15:39:57 -0700 From: Moritz Fischer To: James Morse Cc: Marc Zyngier , kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Catalin Marinas , Will Deacon , Ard Biesheuvel , Mark Rutland , Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , Eric Biederman , Bhupesh SHARMA , AKASHI Takahiro , Dave Young , Andrew Morton , Moritz Fischer , kernel-team@android.com Subject: Re: [PATCH v2 0/5] arm64: Make kexec_file_load honor iomem reservations Message-ID: References: <20210531095720.77469-1-maz@kernel.org> <41ac1e4b-3779-4cb2-5b64-d638521e67c1@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <41ac1e4b-3779-4cb2-5b64-d638521e67c1@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210609_154201_030675_86BD176F X-CRM114-Status: GOOD ( 43.67 ) 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="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 Hi James, Marc On Fri, Jun 04, 2021 at 05:20:38PM +0100, James Morse wrote: > Hi Marc, > > On 31/05/2021 10:57, Marc Zyngier wrote: > > This series is a complete departure from the approach I initially sent > > almost a month ago[0]. Instead of trying to teach EFI, ACPI and other > > subsystem to use memblock, I've decided to stick with the iomem > > resource tree and use that exclusively for arm64. > > > This means that my current approach is (despite what I initially > > replied to both Dave and Catalin) to provide an arm64-specific > > implementation of arch_kexec_locate_mem_hole() which walks the > > resource tree and excludes ranges of RAM that have been registered for > > any odd purpose. This is exactly what the userspace implementation > > does, and I don't really see a good reason to diverge from it. > > Because in the ideal world we'd have only 'is it reserved' list to check against. > Memblock has been extended before. The resource-list is overly stringy, and I'm not sure > we can shove everything in the resource list. > > Kexec already has problems on arm64 with memory hotplug. Fixing this for regular kexec in > /proc/iomem was rejected, and memblock's memblock_is_hotpluggable() is broken because > free_low_memory_core_early() does this: > | memblock_clear_hotplug(0, -1) > > Once that has been unpicked its clear kexec_file_load() can use > memblock_is_hotpluggable(). (its on the todo list, well, jira) > > > I'd prefer to keep kexec using memblock because it _shouldn't_ change after boot. Having > an "I want to reserve this and make it persistent over kexec" call that can happen at any > time can't work if the kexec image has already been loaded. > Practically, once user-space has started, you can't have new things you want to reserve > over kexec. > > > I don't see how the ACPI tables can escape short of a firmware bug. Could someone with an > affected platform boot with efi=debug and post the EFI memory map and the 'ACPI: FOO > 0xphysicaladdress' stuff at the top of the boot log? > > > efi_mem_reserve_persistent() has one caller for the GIC ITS stuff. > > For the ITS, the reservations look like they are behind irqchip_init(), which is well > before the arch_initcall() that updates the resource tree from memblock. Your v1's first > patch should be sufficient. > > > > Again, this allows my Synquacer board to reliably use kexec_file_load > > with as little as 256M, something that would always fail before as it > > would overwrite most of the reserved tables. > > > > Although this series still targets 5.14, the initial patch is a > > -stable candidate, and disables non-kdump uses of kexec_file_load. I > > have limited it to 5.10, as earlier kernels will require a different, > > probably more invasive approach. > > > > Catalin, Ard: although this series has changed a bit compared to v1, > > I've kept your AB/RB tags. Should anything seem odd, please let me > > know and I'll drop them. > > > Thanks, > > James > > > [0] I'm pretty sure this is enough. (Not tested) > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > index 4b7ee3fa9224..3ed45153ce7f 100644 > --- a/drivers/firmware/efi/efi.c > +++ b/drivers/firmware/efi/efi.c > @@ -893,7 +893,7 @@ static int __init efi_memreserve_map_root(void) > return 0; > } > > -static int efi_mem_reserve_iomem(phys_addr_t addr, u64 size) > +static int __efi_mem_reserve_iomem(phys_addr_t addr, u64 size) > { > struct resource *res, *parent; > > @@ -911,6 +911,16 @@ static int efi_mem_reserve_iomem(phys_addr_t addr, u64 size) > return parent ? request_resource(parent, res) : 0; > } > > +static int efi_mem_reserve_iomem(phys_addr_t addr, u64 size) > +{ > + int err = __efi_mem_reserve_iomem(addr, size); > + > + if(IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) && !err) > + memblock_reserve(addr, size); > + > + return err; > +} > + > int __ref efi_mem_reserve_persistent(phys_addr_t addr, u64 size) > { > struct linux_efi_memreserve *rsv; Sorry for the long radio silence. Just got around to testing this. I can confirm that the above change James proposed does work on the platform that the issue was first observed on. Cheers, Moritz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel