linux-kernel - Re: [PATCH] socfpga_a10: reset CPU1 in socfpga_cpu_kill()

Open Source and information security mailing list archives

Message-ID: <20170512143600.GB18818@leverpostej>
Date:   Fri, 12 May 2017 15:36:01 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     yanjiang.jin@...driver.com
Cc:     hiraku.toyooka.gu@...achi.com, dinguyen@...nel.org,
        dinguyen@...nsource.altera.com, linux@....linux.org.uk,
        jinyanjiang@...il.com, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] socfpga_a10: reset CPU1 in socfpga_cpu_kill()

On Wed, May 10, 2017 at 01:13:04AM -0400, yanjiang.jin@...driver.com wrote:
> From: Yanjiang Jin <yanjiang.jin@...driver.com>
> 
> Kexec's second kernel would hang if CPU1 isn't reset.
> 
> Signed-off-by: Yanjiang Jin <yanjiang.jin@...driver.com>
> ---
>  arch/arm/mach-socfpga/platsmp.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-socfpga/platsmp.c b/arch/arm/mach-socfpga/platsmp.c
> index 0ee7677..db3940e 100644
> --- a/arch/arm/mach-socfpga/platsmp.c
> +++ b/arch/arm/mach-socfpga/platsmp.c
> @@ -117,6 +117,16 @@ static int socfpga_cpu_kill(unsigned int cpu)
>  {
>  	return 1;
>  }
> +
> +static int socfpga_a10_cpu_kill(unsigned int cpu)
> +{
> +	/* This will put CPU #1 into reset. */
> +	if (socfpga_cpu1start_addr)
> +		writel(RSTMGR_MPUMODRST_CPU1, rst_manager_base_addr +
> +			SOCFPGA_A10_RSTMGR_MODMPURST);
> +
> +	return 1;
> +}
>  #endif

I agree that currently, socfpga_cpu_die is completely bogus, as the CPU is just
sat in WFI with the MMU, caches, etc enabled, and not actually off:

static void socfpga_cpu_die(unsigned int cpu)
{
        /* Do WFI. If we wake up early, go back into WFI */
        while (1)
                cpu_do_idle();
}

... so that goes wrong as soon as the kerenl text gets ovewritten.

However, AFAICT, this patch forcibly resets is without any teardown
having happened. That will surely result in data being lost from the
caches, for example.

So I think this is incomplete.

Thanks,
Mark.

>  
>  static const struct smp_operations socfpga_smp_ops __initconst = {
> @@ -133,7 +143,7 @@ static int socfpga_cpu_kill(unsigned int cpu)
>  	.smp_boot_secondary	= socfpga_a10_boot_secondary,
>  #ifdef CONFIG_HOTPLUG_CPU
>  	.cpu_die		= socfpga_cpu_die,
> -	.cpu_kill		= socfpga_cpu_kill,
> +	.cpu_kill		= socfpga_a10_cpu_kill,
>  #endif
>  };
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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