linux-kernel - Re: [PATCH v6 4/4] RISC-V: Add arch functions to support hibernation/suspend-to-disk

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] [day] [month] [year] [list]

Message-ID: <ad8fcf05-0e3f-4350-b9ad-533cee984580@spud>
Date:   Mon, 20 Mar 2023 12:56:18 +0000
From:   Conor Dooley <conor.dooley@...rochip.com>
To:     Sia Jee Heng <jeeheng.sia@...rfivetech.com>
CC:     <paul.walmsley@...ive.com>, <palmer@...belt.com>,
        <aou@...s.berkeley.edu>, <linux-riscv@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <leyfoon.tan@...rfivetech.com>,
        <mason.huo@...rfivetech.com>
Subject: Re: [PATCH v6 4/4] RISC-V: Add arch functions to support
 hibernation/suspend-to-disk

On Tue, Mar 14, 2023 at 01:03:16PM +0800, Sia Jee Heng wrote:
> Low level Arch functions were created to support hibernation.
> swsusp_arch_suspend() relies code from __cpu_suspend_enter() to write
> cpu state onto the stack, then calling swsusp_save() to save the memory
> image.
> 
> Arch specific hibernation header is implemented and is utilized by the
> arch_hibernation_header_restore() and arch_hibernation_header_save()
> functions. The arch specific hibernation header consists of satp, hartid,
> and the cpu_resume address. The kernel built version is also need to be
> saved into the hibernation image header to making sure only the same
> kernel is restore when resume.
> 
> swsusp_arch_resume() creates a temporary page table that covering only
> the linear map. It copies the restore code to a 'safe' page, then start
> to restore the memory image. Once completed, it restores the original
> kernel's page table. It then calls into __hibernate_cpu_resume()
> to restore the CPU context. Finally, it follows the normal hibernation
> path back to the hibernation core.
> 
> To enable hibernation/suspend to disk into RISCV, the below config
> need to be enabled:
> - CONFIG_ARCH_HIBERNATION_HEADER
> - CONFIG_ARCH_HIBERNATION_POSSIBLE
> 
> Signed-off-by: Sia Jee Heng <jeeheng.sia@...rfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@...rfivetech.com>
> Reviewed-by: Mason Huo <mason.huo@...rfivetech.com>

> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index c5e42cc37604..473c2a1a6884 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -707,6 +707,12 @@  menu "Power management options"
>  
>  source "kernel/power/Kconfig"
>  
> +config ARCH_HIBERNATION_POSSIBLE
> +	def_bool y
> +
> +config ARCH_HIBERNATION_HEADER
> +	def_bool HIBERNATION
> +
>  endmenu # "Power management options"
>  
>  menu "CPU Power Management"

Since these options are not user-selectable, how come these are
def_bools, rather than just selecting them in the main `config RISCV`
section? Is there some sort of dependency that goes wrong when
structured that way?

> diff --git a/arch/riscv/kernel/hibernate.c b/arch/riscv/kernel/hibernate.c
> new file mode 100644
> index 000000000000..f11be60b0668
> --- /dev/null
> +++ b/arch/riscv/kernel/hibernate.c

> +/**
> + * struct arch_hibernate_hdr_invariants - container to store kernel build version.
> + * @uts_version: to save the build number and date so that the we do not resume with

nit: s/so that the we/so that we/

I noted previously that the page table stuff is beyond me, so modulo
that, I'm happy with how this looks now.

Reviewed-by: Conor Dooley <conor.dooley@...rochip.com>

Thanks for adding hibernation support!
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)