Re: RFC: remove set_fs for m68k

[Michael Schmitz <schmitzmic@xxxxxxxxx>]

Hi Christoph,

I run my tests on a 030, so that patch is unlikely to have any effect 
there. I'll give it a spin though (once my current tests with restoring 
USER_DATA everywhere but logging if get_fc() reads something else) are 
done.

I wonder whether copy_thread() ought to set SUPER_DATA when creating 
kernel threads though (see the comment there). With init_thread starting 
out with USER_DATA now, we'd have to explicitly set SUPER_DATA instead 
of just copying the current value there...

Leaves flush_icache_range() and flush_tlb_page() to worry about.

Cheers,

	Michael

Am 11.08.2021 um 21:12 schrieb Christoph Hellwig:
> Hi Michael,
>
> from looking at your patches I suspect the 040 traps might be able
> to be called witha different fc.  Can you check if the below patch,
> which is a cut down version of your changes makes all the issues
> go away?
>
> diff --git a/arch/m68k/include/asm/processor.h b/arch/m68k/include/asm/processor.h
> index debb4537eab5..a554e529fd26 100644
> --- a/arch/m68k/include/asm/processor.h
> +++ b/arch/m68k/include/asm/processor.h
> @@ -94,6 +94,13 @@ static inline void set_fc(unsigned long val)
>  			      "movec %0,%/dfc\n\t"
>  			      : /* no outputs */ : "r" (val) : "memory");
>  }
> +static inline unsigned long get_fc(void)
> +{
> +	unsigned long val;
> +	__asm__ ("movec %/dfc,%0":"=r" (val):);
> +	WARN_ON_ONCE(val != USER_DATA);
> +	return val;
> +}
>  #else
>  static inline void set_fc(unsigned long val)
>  {
> diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c
> index c5e0a4f93bd5..1b073299fa55 100644
> --- a/arch/m68k/kernel/traps.c
> +++ b/arch/m68k/kernel/traps.c
> @@ -181,6 +181,7 @@ static inline void access_error060 (struct frame *fp)
>  static inline unsigned long probe040(int iswrite, unsigned long addr, int wbs)
>  {
>  	unsigned long mmusr;
> +	unsigned long old_fc = get_fc();
>
>  	set_fc(wbs);
>
> @@ -191,7 +192,7 @@ static inline unsigned long probe040(int iswrite, unsigned long addr, int wbs)
>
>  	asm volatile (".chip 68040; movec %%mmusr,%0; .chip 68k" : "=r" (mmusr));
>
> -	set_fc(USER_DATA);
> +	set_fc(old_fc);
>
>  	return mmusr;
>  }
> @@ -200,6 +201,7 @@ static inline int do_040writeback1(unsigned short wbs, unsigned long wba,
>  				   unsigned long wbd)
>  {
>  	int res = 0;
> +	unsigned long old_fc = get_fc();
>
>  	set_fc(wbs);
>
> @@ -215,7 +217,7 @@ static inline int do_040writeback1(unsigned short wbs, unsigned long wba,
>  		break;
>  	}
>
> -	set_fc(USER_DATA);
> +	set_fc(old_fc);
>
>  	pr_debug("do_040writeback1, res=%d\n", res);