linux-kernel - Re: [1/4] powerpc/cache: add cache flush operation for various e500

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1427969652830.68303@freescale.com>
Date:	Thu, 2 Apr 2015 10:14:11 +0000
From:	"chenhui.zhao@...escale.com" <chenhui.zhao@...escale.com>
To:	Scott Wood <scottwood@...escale.com>
CC:	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Jason.Jin@...escale.com" <Jason.Jin@...escale.com>
Subject: Re: [1/4] powerpc/cache: add cache flush operation for various e500


________________________________________
From: Wood Scott-B07421
Sent: Tuesday, March 31, 2015 9:10
To: Zhao Chenhui-B35336
Cc: linuxppc-dev@...ts.ozlabs.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Jin Zhengxiong-R64188
Subject: Re: [1/4] powerpc/cache: add cache flush operation for various e500

On Thu, Mar 26, 2015 at 06:18:12PM +0800, chenhui zhao wrote:
> Various e500 core have different cache architecture, so they
> need different cache flush operations. Therefore, add a callback
> function cpu_flush_caches to the struct cpu_spec. The cache flush
> operation for the specific kind of e500 is selected at init time.
> The callback function will flush all caches inside the current cpu.
>
> Signed-off-by: Chenhui Zhao <chenhui.zhao@...escale.com>
> ---
>  arch/powerpc/include/asm/cacheflush.h     |   2 -
>  arch/powerpc/include/asm/cputable.h       |  11 +++
>  arch/powerpc/kernel/asm-offsets.c         |   3 +
>  arch/powerpc/kernel/cpu_setup_fsl_booke.S | 114 +++++++++++++++++++++++++++++-
>  arch/powerpc/kernel/cputable.c            |   4 ++
>  arch/powerpc/kernel/head_fsl_booke.S      |  74 -------------------
>  arch/powerpc/platforms/85xx/smp.c         |   3 +-
>  7 files changed, 133 insertions(+), 78 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> index 30b35ff..729fde4 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -30,8 +30,6 @@ extern void flush_dcache_page(struct page *page);
>  #define flush_dcache_mmap_lock(mapping)              do { } while (0)
>  #define flush_dcache_mmap_unlock(mapping)    do { } while (0)
>
> -extern void __flush_disable_L1(void);
> -
>  extern void flush_icache_range(unsigned long, unsigned long);
>  extern void flush_icache_user_range(struct vm_area_struct *vma,
>                                   struct page *page, unsigned long addr,
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index 5cf5a6d..c776efe4 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -43,6 +43,13 @@ extern int machine_check_e500(struct pt_regs *regs);
>  extern int machine_check_e200(struct pt_regs *regs);
>  extern int machine_check_47x(struct pt_regs *regs);
>
> +#if defined(CONFIG_E500) || defined(CONFIG_PPC_E500MC)
> +extern void __flush_caches_e500v2(void);
> +extern void __flush_caches_e500mc(void);
> +extern void __flush_caches_e5500(void);
> +extern void __flush_caches_e6500(void);
> +#endif

Why the leading underscores?

[chenhui] Will get rid of them.

>  /* NOTE WELL: Update identify_cpu() if fields are added or removed! */
>  struct cpu_spec {
>       /* CPU is matched via (PVR & pvr_mask) == pvr_value */
> @@ -59,6 +66,10 @@ struct cpu_spec {
>       unsigned int    icache_bsize;
>       unsigned int    dcache_bsize;
>
> +#if defined(CONFIG_E500) || defined(CONFIG_PPC_E500MC)

CONFIG_PPC_E500MC implies CONFIG_E500.  Why do we need this ifdef?

[chenhui] Change to "#ifdef CONFIG_E500".

> +     /* flush caches inside the current cpu */
> +     void (*cpu_flush_caches)(void);
> +#endif

It seems you literally mean "in the cpu" -- If it's a threaded core, then
by "cpu" do you mean "thread" (like we usually do) and thus no caches get
flushed (ignore the fact that it's moot on e6500 -- this is an interface
and needs to be clear).

Also, no-oping L1 flush on e6500 is not compliant with the claim that
you're flushing the cache.  You're relying on an unstated assumption that
you'll invalidate that cache later instead.

If you want to make this "flush whatever needs to be flushed for
suspend/hotplug", call it that.

-Scott

[chenhui] OK. Then, call "cpu_down_flush".
--
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/