Re: [PATCH] m68knommu: remove set_fs()

Hi Christoph,

On Mon, Jul 5, 2021 at 7:58 AM Christoph Hellwig <[email protected]> wrote:
> m68knommu already does not distinguish between kernel and user address
> spaces.  Stop defining set_fs() and thus uaccess_kernel() to avoid
> confusion.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Thanks for your patch!

> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -32,7 +32,7 @@ config M68K
>         select NO_DMA if !MMU && !COLDFIRE
>         select OLD_SIGACTION
>         select OLD_SIGSUSPEND3
> -       select SET_FS
> +       select SET_FS if MMU

Probably this should be

    select SET_FS if CPU_HAS_ADDRESS_SPACES

instead.  Classic m68k (except for good old 68000) has address spaces,
Coldfire has not.

Which brought my attention to a problem with support for embedded
68EC020 cores: MCPU32  does not select CPU_HAS_ADDRESS_SPACES, but
it also does not select GENERIC_CSUM.  The latter means it will use
arch/m68k/lib/checksum.c, which does contain moves instructions, thus
assuming separate address spaces.  Fortunately MCPU32 is no longer
used since commit a3595962d82495f5 ("m68knommu: remove obsolete 68360
support"), so it can be removed, too.

I guess we want to simplify

    config GENERIC_CSUM
        default y if !CPU_HAS_ADDRESS_SPACES

which would allow us to get rid of the ifndef CONFIG_GENERIC_CSUM in
arch/m68k/lib/Makefile, in favor of lib-$(CPU_HAS_ADDRESS_SPACES).

> --- a/arch/m68k/include/asm/segment.h
> +++ b/arch/m68k/include/asm/segment.h
> @@ -2,19 +2,20 @@
>  #ifndef _M68K_SEGMENT_H
>  #define _M68K_SEGMENT_H
>
> -/* define constants */
>  /* Address spaces (FC0-FC2) */
>  #define USER_DATA     (1)
> +#define USER_PROGRAM  (2)
> +#define SUPER_DATA    (5)
> +#define SUPER_PROGRAM (6)
> +#define CPU_SPACE     (7)
> +
> +#ifdef CONFIG_MMU
>  #ifndef __USER_DS
>  #define __USER_DS     (USER_DATA)
>  #endif
> -#define USER_PROGRAM  (2)
> -#define SUPER_DATA    (5)
>  #ifndef __KERNEL_DS
>  #define __KERNEL_DS   (SUPER_DATA)
>  #endif
> -#define SUPER_PROGRAM (6)
> -#define CPU_SPACE     (7)
>
>  #ifndef __ASSEMBLY__

Just below this is the existing handling for !CPU_HAS_ADDRESS_SPACES.
Probably the [sg]et_fs() definitions there should be made dummies,
reducing the need to add a rather large number of #ifdefs.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds