Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210713184326.570923-1-maz@kernel.org>
In-Reply-To: <20210713184326.570923-1-maz@kernel.org>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 16 Jul 2021 14:10:14 +0200
Message-ID: <CAMj1kXEB6eRPOLAaz7gWgUrpn2R6fy6kJ=S8u_54kNfQCbEfqg@mail.gmail.com>
Subject: Re: [PATCH v2] firmware/efi: Tell memblock about EFI iomem reservations
To:     Marc Zyngier <maz@kernel.org>
Cc:     Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        wnliu@google.com, Moritz Fischer <mdf@kernel.org>,
        "# 3.4.x" <stable@vger.kernel.org>,
        James Morse <james.morse@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 13 Jul 2021 at 20:43, Marc Zyngier <maz@kernel.org> wrote:
>
> kexec_load_file() relies on the memblock infrastructure to avoid
> stamping over regions of memory that are essential to the survival
> of the system.
>
> However, nobody seems to agree how to flag these regions as reserved,
> and (for example) EFI only publishes its reservations in /proc/iomem
> for the benefit of the traditional, userspace based kexec tool.
>
> On arm64 platforms with GICv3, this can result in the payload being
> placed at the location of the LPI tables. Shock, horror!
>
> Let's augment the EFI reservation code with a memblock_reserve() call,
> protecting our dear tables from the secondary kernel invasion.
>
> Reported-by: Moritz Fischer <mdf@kernel.org>
> Tested-by: Moritz Fischer <mdf@kernel.org>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: James Morse <james.morse@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>

Thanks, I'll queue this as a fix.

> ---
>  drivers/firmware/efi/efi.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 4b7ee3fa9224..847f33ffc4ae 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -896,6 +896,7 @@ static int __init efi_memreserve_map_root(void)
>  static int efi_mem_reserve_iomem(phys_addr_t addr, u64 size)
>  {
>         struct resource *res, *parent;
> +       int ret;
>
>         res = kzalloc(sizeof(struct resource), GFP_ATOMIC);
>         if (!res)
> @@ -908,7 +909,17 @@ static int efi_mem_reserve_iomem(phys_addr_t addr, u64 size)
>
>         /* we expect a conflict with a 'System RAM' region */
>         parent = request_resource_conflict(&iomem_resource, res);
> -       return parent ? request_resource(parent, res) : 0;
> +       ret = parent ? request_resource(parent, res) : 0;
> +
> +       /*
> +        * Given that efi_mem_reserve_iomem() can be called at any
> +        * time, only call memblock_reserve() if the architecture
> +        * keeps the infrastructure around.
> +        */
> +       if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) && !ret)
> +               memblock_reserve(addr, size);
> +
> +       return ret;
>  }
>
>  int __ref efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
> --
> 2.30.2
>