Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT

From: Michael Ellerman <mpe@ellerman.id.au>
To: Rob Herring <robh@kernel.org>,
	Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>,
	Thiago Jung Bauermann <bauerman@linux.ibm.com>,
	"AKASHI, Takahiro" <takahiro.akashi@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Will Deacon <will@kernel.org>, Joe Perches <joe@perches.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	James Morse <james.morse@arm.com>,
	Sasha Levin <sashal@kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Frank Rowand <frowand.list@gmail.com>,
	vincenzo.frascino@arm.com, Mark Rutland <mark.rutland@arm.com>,
	dmitry.kasatkin@gmail.com, James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Pavel Tatashin <pasha.tatashin@soleen.com>,
	Allison Randal <allison@lohutok.net>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Bhupesh Sharma <bhsharma@redhat.com>,
	Matthias Brugger <mbrugger@suse.com>,
	Hsin-Yi Wang <hsinyi@chromium.org>,
	tao.li@vivo.com, Christophe Leroy <christophe.leroy@c-s.fr>,
	Prakhar Srivastava <prsriva@linux.microsoft.com>,
	balajib@linux.microsoft.com, linux-integrity@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	devicetree@vger.kernel.org,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
Date: Mon, 08 Feb 2021 15:12:59 +1100	[thread overview]
Message-ID: <87zh0fnqno.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <CAL_JsqK1Pb9nAeL84EP2U3MQgpBsm+E_0QXmzbigWXnS245WPQ@mail.gmail.com>

Rob Herring <robh@kernel.org> writes:
> On Thu, Feb 4, 2021 at 10:42 AM Lakshmi Ramasubramanian
> <nramas@linux.microsoft.com> wrote:
...
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index d0e459bb2f05..51d2d8eb6c1b 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/kexec.h>
>>  #include <linux/libfdt.h>
>>  #include <linux/module.h>
>> +#include <linux/of.h>
>>  #include <linux/of_fdt.h>
>>  #include <linux/slab.h>
>>  #include <linux/types.h>
>> @@ -32,7 +33,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>         unsigned int fdt_size;
>>         unsigned long kernel_load_addr;
>>         unsigned long initrd_load_addr = 0, fdt_load_addr;
>> -       void *fdt;
>> +       void *fdt = NULL;
>>         const void *slave_code;
>>         struct elfhdr ehdr;
>>         char *modified_cmdline = NULL;
>> @@ -103,18 +104,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>         }
>>
>>         fdt_size = fdt_totalsize(initial_boot_params) * 2;
>> -       fdt = kmalloc(fdt_size, GFP_KERNEL);
>> +       fdt = of_alloc_and_init_fdt(fdt_size);
>>         if (!fdt) {
>>                 pr_err("Not enough memory for the device tree.\n");
>>                 ret = -ENOMEM;
>>                 goto out;
>>         }
>> -       ret = fdt_open_into(initial_boot_params, fdt, fdt_size);
>> -       if (ret < 0) {
>> -               pr_err("Error setting up the new device tree.\n");
>> -               ret = -EINVAL;
>> -               goto out;
>> -       }
>>
>>         ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
>
> The first thing this function does is call setup_new_fdt() which first
> calls of_kexec_setup_new_fdt(). (Note, I really don't understand the
> PPC code split. It looks like there's a 32-bit and 64-bit split, but
> 32-bit looks broken to me. Nothing ever calls setup_new_fdt() except
> setup_new_fdt_ppc64()).

I think that's because 32-bit doesn't support kexec_file_load().

cheers