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]
Date:	Fri, 26 Sep 2014 13:14:27 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Chenhui Zhao <chenhui.zhao@...escale.com>
Cc:	linux-kernel@...r.kernel.org, kernel@...gutronix.de,
	linux-arm-kernel@...ts.infradead.org, leoli@...escale.com,
	Jason.Jin@...escale.com, Zhuoyu.Zhang@...escale.com
Subject: Re: [PATCH 3/3] arm: pm: add deep sleep support for LS1

On Fri, Sep 26, 2014 at 07:25:03PM +0800, Chenhui Zhao wrote:
> +static int ls1_start_deepsleep(unsigned long addr)
> +{
> +	ls1_do_deepsleep(addr);
> +
> +	return 0;
> +}
...
> +	cpu_suspend(SRAM_CODE_BASE_PHY, ls1_start_deepsleep);

What's the point of this function?  Why can't ls1_do_deepsleep() just
return zero?

> +/*
> + * r0: the physical entry address of SRAM code
> + *
> + */
> +ENTRY(ls1_do_deepsleep)
> +	mov	r13, r0
> +
> +	/* flush cache */
> +	bl	v7_flush_dcache_all

Please explain the purpose of this call via a comment in the code.

The generic code will have saved the CPU state, and will have called
flush_cache_louis() to flush the caches to the point of unification.

The only data which will have been loaded into the cache between that
point is the stack for the return from __cpu_suspend_save, and
speculative prefetches.

So, the only reason I can gather is that you need to flush data from
lower levels of the cache below the point of unification.

> +
> +	/* disable cache, C bit in SCTLR */
> +	mrc	p15, 0, r0, c1, c0, 0
> +	bic	r0, r0, #(1 << 2)
> +	mcr	p15, 0, r0, c1, c0, 0
> +	isb
> +
> +	/* disable coherency, SMP bit in ACTLR */
> +	mrc	p15, 0, r0, c1, c0, 1
> +	bic	r0, r0, #(1 << 6)
> +	mcr	p15, 0, r0, c1, c0, 1
> +	isb
> +	dsb
> +
> +	/* disable MMU, M bit in SCTLR */
> +	mrc	p15, 0, r0, c1, c0, 0
> +	bic	r0, r0, #1
> +	mcr	p15, 0, r0, c1, c0, 0
> +	isb
> +
> +	/* jump to sram code using physical address */
> +	bx	r13

This looks extremely fragile.  You are running in virtual space, and you
turn the MMU off.  You are reliant on the MMU still being on for the
following instructions to already be in the pipeline.  That's not a
very good assumption to make (we've made it in the past and it breaks
every so often when things change, eg when the code is no longer laid
out how we expect.)

You need to disable the MMU safely, which means using the identity map
page tables and executing code in the identity map region.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ