Re: [PATCH, V3] parisc: Rewrite cache flush code for PA8800/PA8900

[Rolf Eike Beer <eike-kernel@sf-tec.de>]

[-- Attachment #1: Type: text/plain, Size: 2625 bytes --]

Am Montag, 16. Mai 2022, 17:14:47 CEST schrieb John David Anglin:
> Originally, I was convinced that we needed to use tmpalias flushes
> everwhere, for both user and kernel flushes. However, when I modified
> flush_kernel_dcache_page_addr, to use a tmpalias flush, my c8000
> would crash quite early when booting.
> 
> The PDC returns alias values of 0 for the icache and dcache. This
> indicates that either the alias boundary is greater than 16MB or
> equivalent aliasing doesn't work. I modified the tmpalias code to
> make it easy to try alternate boundaries. I tried boundaries up to
> 128MB but still kernel tmpalias flushes didn't work on c8000.
> 
> This led me to conclude that tmpalias flushes don't work on PA8800
> and PA8900 machines, and that we needed to flush directly using the
> virtual address of user and kernel pages. This is likely the major
> cause of instability on the c8000 and rp34xx machines.
> 
> Flushing user pages requires doing a temporary context switch as we
> have to flush pages that don't belong to the current context. Further,
> we have to deal with pages that aren't present. If a page isn't
> present, the flush instructions fault on every line.
> 
> Other code has been rearranged and simplified based on testing. For
> example, I introduced a flush_cache_dup_mm routine. flush_cache_mm
> and flush_cache_dup_mm differ in that flush_cache_mm calls
> purge_cache_pages and flush_cache_dup_mm calls flush_cache_pages.
> In some implementations, pdc is more efficient than fdc. Based on
> my testing, I don't believe there's any performance benefit on the
> c8000.

> diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c
> index f114e102aaf2..ca49765784fc 100644
> --- a/arch/parisc/mm/fault.c
> +++ b/arch/parisc/mm/fault.c
> @@ -22,6 +22,8 @@
> 
>  #include <asm/traps.h>
> 
> +/* #define DEBUG_NATLB 1 */
> +
>  /* Various important other fields */
>  #define bit22set(x)		(x & 0x00000200)
>  #define bits23_25set(x)		(x & 0x000001c0)
> @@ -450,10 +452,12 @@ handle_nadtlb_fault(struct pt_regs *regs)
>  		fallthrough;
>  	case 0x380:
>  		/* PDC and FIC instructions */
> +#ifdef DEBUG_NATLB
>  		if (printk_ratelimit()) {
> -			pr_warn("BUG: nullifying cache flush/purge 
instruction\n");
> +			pr_warn("WARNING: nullifying cache flush/
purge instruction\n");
>  			show_regs(regs);
>  		}
> +#endif
>  		if (insn & 0x20) {
>  			/* Base modification */
>  			breg = (insn >> 21) & 0x1f;

This surely deserves it's own commit as it has nothing to do with the actual 
change. I wonder if it is actually intended to go upstream or if this was just 
a local debug hack?

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]