Message-ID: <8735epf7j5.fsf@email.froward.int.ebiederm.org>
Date:   Mon, 25 Jul 2022 12:04:30 -0500
From:   "Eric W. Biederman" <ebiederm@...ssion.com>
To:     Albert Huang <huangjie.albert@...edance.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Michael Roth <michael.roth@....com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Sean Christopherson <seanjc@...gle.com>,
        Joerg Roedel <jroedel@...e.de>,
        Mark Rutland <mark.rutland@....com>,
        Kees Cook <keescook@...omium.org>,
        linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
        linux-kbuild@...r.kernel.org
Subject: Re: [PATCH 0/4] faster kexec reboot

Albert Huang <huangjie.albert@...edance.com> writes:

> From: "huangjie.albert" <huangjie.albert@...edance.com>
>
> In many time-sensitive scenarios, we need a shorter time to restart 
> the kernel. However, in the current kexec fast restart code, there 
> are many places in the memory copy operation, verification operation 
> and decompression operation, which take more time than 500ms. Through 
> the following patch series. machine_kexec-->start_kernel only takes
> 15ms

Is this a tiny embedded device you are taking the timings of?

How are you handling driver shutdown and restart?  I would expect those
to be a larger piece of the puzzle than memory.

My desktop can do something like 128GiB/s.  Which would suggest that
copying 128MiB of kernel+initrd would take perhaps 10ms.  The SHA256
implementation may not be tuned so that could be part of the performance
issue.  The SHA256 hash has a reputation for having fast
implementations.  I chose SHA256 originally simply because it has more
bits so it makes the odds of detecting an error higher.


If all you care about is booting a kernel as fast as possible it make
make sense to have a large reserved region of memory like we have for
the kexec on panic kernel.  If that really makes sense I recommend
adding a second kernel command line option and a reserving second region
of reserved memory.  That makes telling if the are any conflicts simple.


I am having a hard time seeing how anyone else would want these options.
Losing megabytes of memory simply because you might reboot using kexec
seems like the wrong side of a trade-off.

The CONFIG_KEXEC_PURGATORY_SKIP_SIG option is very misnamed.  It is not
signature verification that is happening it is a hash verification.
There are not encrypted bits at play.  Instead there is a check to
ensure that the kernel has not been corrupted by in-flight DMA that some
driver forgot to shut down.

So you are building a version of kexec that if something goes wrong it
could very easily eat your data, or otherwise do some very bad things
that are absolutely non-trivial to debug.

That the decision to skip the sha256 hash that prevents corruption is
happening at compile time, instead of at run-time, will guarantee the
option is simply not available on any general purpose kernel
configuration.  Given how dangerous it is to skip the hash verification
it is probably not a bad thing overall, but it is most definitely
something that will make maintenance more difficult.


If done well I don't see why anyone would mind a uncompressed kernel
but I don't see what the advantage of what you are doing is over using
vmlinux is the build directory.  It isn't a bzImage but it is the
uncompressed kernel.

As I proof of concept I think what you are doing goes a way to showing
that things can be improved.  My overall sense is that improving things
the way you are proposing does not help the general case and simply adds
to the maintenance burden.

Eric

>
> How to measure time:
>
> c code:
> uint64_t current_cycles(void)
> {
>     uint32_t low, high;
>     asm volatile("rdtsc" : "=a"(low), "=d"(high));
>     return ((uint64_t)low) | ((uint64_t)high << 32);
> }
> assembly code:
>        pushq %rax
>        pushq %rdx
>        rdtsc
>        mov   %eax,%eax
>        shl   $0x20,%rdx
>        or    %rax,%rdx
>        movq  %rdx,0x840(%r14)
>        popq  %rdx
>        popq  %rax
> the timestamp may store in boot_params or kexec control page, so we can
> get the all timestamp after kernel boot up.
>
> huangjie.albert (4):
>   kexec: reuse crash kernel reserved memory for normal kexec
>   kexec: add CONFING_KEXEC_PURGATORY_SKIP_SIG
>   x86: Support the uncompressed kernel to speed up booting
>   x86: boot: avoid memory copy if kernel is uncompressed
>
>  arch/x86/Kconfig                   | 10 +++++++++
>  arch/x86/boot/compressed/Makefile  |  5 ++++-
>  arch/x86/boot/compressed/head_64.S |  8 +++++--
>  arch/x86/boot/compressed/misc.c    | 35 +++++++++++++++++++++++++-----
>  arch/x86/purgatory/purgatory.c     |  7 ++++++
>  include/linux/kexec.h              |  9 ++++----
>  include/uapi/linux/kexec.h         |  2 ++
>  kernel/kexec.c                     | 19 +++++++++++++++-
>  kernel/kexec_core.c                | 16 ++++++++------
>  kernel/kexec_file.c                | 20 +++++++++++++++--
>  scripts/Makefile.lib               |  5 +++++
>  11 files changed, 114 insertions(+), 22 deletions(-)

Powered by blists - more mailing lists