linux-kernel - Re: [PATCH 09/56] microblaze

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:	Mon, 05 May 2008 12:09:03 +1000
From:	John Williams <john.williams@...alogix.com>
To:	monstr@...nam.cz
Cc:	linux-kernel@...r.kernel.org, arnd@...db.de,
	linux-arch@...r.kernel.org, stephen.neuendorffer@...inx.com,
	John.Linn@...inx.com, matthew@....cx, will.newton@...il.com,
	drepper@...hat.com, microblaze-uclinux@...e.uq.edu.au,
	grant.likely@...retlab.ca, Michal Simek <monstr@...str.eu>
Subject: Re: [PATCH 09/56] microblaze_v2: cache support


> +++ b/arch/microblaze/kernel/cpu/cache.c

> +void __flush_icache_all(void)
> +{
> +	unsigned int i;
> +	unsigned flags;
> +
> +	if (cpuinfo.use_icache) {
> +		local_irq_save(flags);
> +		__disable_icache();
> +
> +		/* Just loop through cache size and invalidate, no need to add
> +			CACHE_BASE address */
> +		for (i = 0; i < cpuinfo.icache_size;
> +			i += cpuinfo.icache_line)
> +				__invalidate_icache(i);
> +
> +		__enable_icache();
> +		local_irq_restore(flags);
> +	}
> +}

This is the slowest and greatest latency-inducing loop in the MicroBlaze
kernel.  The CPU specs says icache must be disabled while you clear it -
if you have a chance it's worth checking to see what happens if you
violate that rule.

This will be less of a problem once we update the kernel_from_bram
patches, but that may be a little while yet.


> +void __flush_icache_range(unsigned long start, unsigned long end)

ditto

> +void __flush_icache_page(struct vm_area_struct *vma, struct page *page)
> +{
> +	__flush_icache_all();
> +}
> +

This could be smarter - if our cache size is less than a page then there
is a win to be had.  But, it's my fault so I won't complain too loudly!

> +void __flush_icache_user_range(struct vm_area_struct *vma,
> +				struct page *page, unsigned long adr,
> +				int len)
> +{
> +	__flush_icache_all();
> +}

and again.

> +
> +void __flush_cache_sigtramp(unsigned long addr)
> +{
> +	__flush_icache_range(addr, addr + 8);
> +}

Perhaps a #define in asm-microblaze/signal.h to tell us the size of the
sigtramp, and also used by microblaze/kernel/signal.c?

> +
> +void __flush_dcache_all(void)
> +{
> +	unsigned int i;
> +	unsigned flags;
> +
> +	if (cpuinfo.use_dcache) {
> +		local_irq_save(flags);
> +		__disable_dcache();
> +
> +		/*
> +		 * Just loop through cache size and invalidate,
> +		 * no need to add CACHE_BASE address
> +		 */
> +		for (i = 0; i < cpuinfo.dcache_size;
> +			i += cpuinfo.dcache_line)
> +				__invalidate_dcache(i);
> +
> +		__enable_dcache();
> +		local_irq_restore(flags);
> +	}
> +}

again might be worth testing if it still works with dcache enabled
during the flush loop.


> --- /dev/null
> +++ b/include/asm-microblaze/cache.h

> +#ifndef L1_CACHE_BYTES
> +/* word-granular cache in microblaze */
> +#define L1_CACHE_BYTES		4
> +#endif

Is this actually used anywhere?  If so, it's wrong because cache line
sizes is configurable now.


> +/* FIXME - I don't think this is right */
> +#ifdef CONFIG_XILINX_UNCACHED_SHADOW
> +#define UNCACHED_SHADOW_MASK (CONFIG_XILINX_ERAM_SIZE)
> +#endif

If you are throwing out the alloc_consist()/free_consist()
implementation then the UNCACHED_SHADOW stuff can go too.


> +#define copy_to_user_page(vma, page, vaddr, dst, src, len) \
> +do { memcpy(dst, src, len); \
> +	flush_icache_user_range(vma, page, vaddr, len); \
> +} while (0)

Is the icache flush required?  Is copy_to_user_page() called from the
binfmt_flat loader?


--
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/