linux-kernel - Re: [PATCH] of/kexec: save pa of initial_boot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqJVEjPt9tHNr0uAGHQwGnUbZDZoe7kURp3Qx0ce1jv+vw@mail.gmail.com>
Date: Mon, 7 Oct 2024 09:39:42 -0500
From: Rob Herring <robh@...nel.org>
To: Usama Arif <usamaarif642@...il.com>
Cc: mark.rutland@....com, will@...nel.org, leitao@...ian.org, 
	catalin.marinas@....com, saravanak@...gle.com, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, kexec@...ts.infradead.org
Subject: Re: [PATCH] of/kexec: save pa of initial_boot_params for arm64 and
 use it at kexec

On Mon, Oct 7, 2024 at 9:06 AM Usama Arif <usamaarif642@...il.com> wrote:
>
>
>
> On 04/10/2024 01:03, Rob Herring wrote:
> > On Thu, Oct 03, 2024 at 12:38:40PM +0100, Usama Arif wrote:
> >>  __pa() is only intended to be used for linear map addresses and using
> >> it for initial_boot_params which is in fixmap for arm64 will give an
> >> incorrect value. Hence stash the physical address when it is known at
> >> boot time and use it at kexec time instead of converting the virtual
> >> address using __pa().
> >>
> >> Reported-by: Breno Leitao <leitao@...ian.org>
> >> Suggested-by: Mark Rutland <mark.rutland@....com>
> >> Signed-off-by: Usama Arif <usamaarif642@...il.com>
> >> Fixes: ac10be5cdbfa ("arm64: Use common of_kexec_alloc_and_setup_fdt()")
> >> ---
> >>  arch/arm64/kernel/setup.c | 8 ++++++++
> >>  drivers/of/fdt.c          | 6 ++++++
> >>  drivers/of/kexec.c        | 8 ++++++--
> >>  include/linux/of_fdt.h    | 2 ++
> >>  4 files changed, 22 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> >> index b22d28ec8028..a4d96f5e2e05 100644
> >> --- a/arch/arm64/kernel/setup.c
> >> +++ b/arch/arm64/kernel/setup.c
> >> @@ -194,6 +194,14 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
> >>      /* Early fixups are done, map the FDT as read-only now */
> >>      fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
> >>
> >> +    /*
> >> +     * Save dt_phys address so that it can be used later for kexec. This
> >> +     * is done as __pa() is only intended to be used for linear map addresses
> >> +     * and using it for initial_boot_params which is in fixmap will give an
> >> +     * incorrect value.
> >> +     */
> >> +    set_initial_boot_params_pa(dt_phys);
> >
> > No new arch->dt functions please. If we need to save off the PA, then do
> > that when we set initial_boot_params.
> >
> > Rob
>
>
> initial_boot_params is set in early_init_dt_verify, called by early_init_dt_scan.
> This is done in setup_machine_fdt in arm64 where the PA is available,
> but in other functions in other architectures, where the PA is not available.

Doesn't __pa() work for all the other architectures? That's what your
patch indicates.

> So it makes it quite messy to set it in the same place as initial_boot_params.
> Its only needed for arm64 and making a change in all archs probably isnt a good idea?
>
> Any reason to not add a new function to make arch -> of/fdt call?

Yes. It is the opposite direction I have reworked the interfaces to.
We don't want each arch calling various early DT functions at random
times and order. That's fragile when the DT functions make assumptions
about when they are called or what's been initialized already.

Another option is to make arm64 copy the DT as some arches do.

Rob