[PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Do we also want

    select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE && !M68000)

???

 arch/m68k/Kconfig                  |    2 +
 arch/m68k/include/asm/Kbuild       |    2 +
 arch/m68k/include/asm/uaccess_mm.h |   11 +++--
 arch/m68k/lib/uaccess.c            |   74 ------------------------------------
 4 files changed, 11 insertions(+), 78 deletions(-)

diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index cac5b6b..1471201 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -7,6 +7,8 @@ config M68K
 	select GENERIC_IRQ_SHOW
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG if RMW_INSNS
 	select GENERIC_CPU_DEVICES
+	select GENERIC_STRNCPY_FROM_USER if MMU
+	select GENERIC_STRNLEN_USER if MMU
 	select FPU if MMU
 	select ARCH_USES_GETTIMEOFFSET if MMU && !COLDFIRE
 
diff --git a/arch/m68k/include/asm/Kbuild b/arch/m68k/include/asm/Kbuild
index 1a922fa..eafa253 100644
--- a/arch/m68k/include/asm/Kbuild
+++ b/arch/m68k/include/asm/Kbuild
@@ -1,2 +1,4 @@
 include include/asm-generic/Kbuild.asm
 header-y += cachectl.h
+
+generic-y += word-at-a-time.h
diff --git a/arch/m68k/include/asm/uaccess_mm.h b/arch/m68k/include/asm/uaccess_mm.h
index 9c80cd5..472c891 100644
--- a/arch/m68k/include/asm/uaccess_mm.h
+++ b/arch/m68k/include/asm/uaccess_mm.h
@@ -379,12 +379,15 @@ __constant_copy_to_user(void __user *to, const void *from, unsigned long n)
 #define copy_from_user(to, from, n)	__copy_from_user(to, from, n)
 #define copy_to_user(to, from, n)	__copy_to_user(to, from, n)
 
-long strncpy_from_user(char *dst, const char __user *src, long count);
-long strnlen_user(const char __user *src, long n);
+#define user_addr_max() \
+	(segment_eq(get_fs(), USER_DS) ? TASK_SIZE : ~0UL)
+
+extern long strncpy_from_user(char *dst, const char __user *src, long count);
+extern __must_check long strlen_user(const char __user *str);
+extern __must_check long strnlen_user(const char __user *str, long n);
+
 unsigned long __clear_user(void __user *to, unsigned long n);
 
 #define clear_user	__clear_user
 
-#define strlen_user(str) strnlen_user(str, 32767)
-
 #endif /* _M68K_UACCESS_H */
diff --git a/arch/m68k/lib/uaccess.c b/arch/m68k/lib/uaccess.c
index 5664386..5e97f2e 100644
--- a/arch/m68k/lib/uaccess.c
+++ b/arch/m68k/lib/uaccess.c
@@ -104,80 +104,6 @@ unsigned long __generic_copy_to_user(void __user *to, const void *from,
 EXPORT_SYMBOL(__generic_copy_to_user);
 
 /*
- * Copy a null terminated string from userspace.
- */
-long strncpy_from_user(char *dst, const char __user *src, long count)
-{
-	long res;
-	char c;
-
-	if (count <= 0)
-		return count;
-
-	asm volatile ("\n"
-		"1:	"MOVES".b	(%2)+,%4\n"
-		"	move.b	%4,(%1)+\n"
-		"	jeq	2f\n"
-		"	subq.l	#1,%3\n"
-		"	jne	1b\n"
-		"2:	sub.l	%3,%0\n"
-		"3:\n"
-		"	.section .fixup,\"ax\"\n"
-		"	.even\n"
-		"10:	move.l	%5,%0\n"
-		"	jra	3b\n"
-		"	.previous\n"
-		"\n"
-		"	.section __ex_table,\"a\"\n"
-		"	.align	4\n"
-		"	.long	1b,10b\n"
-		"	.previous"
-		: "=d" (res), "+a" (dst), "+a" (src), "+r" (count), "=&d" (c)
-		: "i" (-EFAULT), "0" (count));
-
-	return res;
-}
-EXPORT_SYMBOL(strncpy_from_user);
-
-/*
- * Return the size of a string (including the ending 0)
- *
- * Return 0 on exception, a value greater than N if too long
- */
-long strnlen_user(const char __user *src, long n)
-{
-	char c;
-	long res;
-
-	asm volatile ("\n"
-		"1:	subq.l	#1,%1\n"
-		"	jmi	3f\n"
-		"2:	"MOVES".b	(%0)+,%2\n"
-		"	tst.b	%2\n"
-		"	jne	1b\n"
-		"	jra	4f\n"
-		"\n"
-		"3:	addq.l	#1,%0\n"
-		"4:	sub.l	%4,%0\n"
-		"5:\n"
-		"	.section .fixup,\"ax\"\n"
-		"	.even\n"
-		"20:	sub.l	%0,%0\n"
-		"	jra	5b\n"
-		"	.previous\n"
-		"\n"
-		"	.section __ex_table,\"a\"\n"
-		"	.align	4\n"
-		"	.long	2b,20b\n"
-		"	.previous\n"
-		: "=&a" (res), "+d" (n), "=&d" (c)
-		: "0" (src), "r" (src));
-
-	return res;
-}
-EXPORT_SYMBOL(strnlen_user);
-
-/*
  * Zero Userspace
  */
 
-- 
1.7.0.4

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Greg Ungerer]

Hi Geert,

On 30/05/12 07:33, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven<geert@linux-m68k.org>

Looks good:

Acked-by: Greg Ungerer <gerg@uclinux.org>


> ---
> Do we also want
>
>      select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE&&  !M68000)

Maybe yes.

Regards
Greg


>   arch/m68k/Kconfig                  |    2 +
>   arch/m68k/include/asm/Kbuild       |    2 +
>   arch/m68k/include/asm/uaccess_mm.h |   11 +++--
>   arch/m68k/lib/uaccess.c            |   74 ------------------------------------
>   4 files changed, 11 insertions(+), 78 deletions(-)
>
> diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> index cac5b6b..1471201 100644
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -7,6 +7,8 @@ config M68K
>   	select GENERIC_IRQ_SHOW
>   	select ARCH_HAVE_NMI_SAFE_CMPXCHG if RMW_INSNS
>   	select GENERIC_CPU_DEVICES
> +	select GENERIC_STRNCPY_FROM_USER if MMU
> +	select GENERIC_STRNLEN_USER if MMU
>   	select FPU if MMU
>   	select ARCH_USES_GETTIMEOFFSET if MMU&&  !COLDFIRE
>
> diff --git a/arch/m68k/include/asm/Kbuild b/arch/m68k/include/asm/Kbuild
> index 1a922fa..eafa253 100644
> --- a/arch/m68k/include/asm/Kbuild
> +++ b/arch/m68k/include/asm/Kbuild
> @@ -1,2 +1,4 @@
>   include include/asm-generic/Kbuild.asm
>   header-y += cachectl.h
> +
> +generic-y += word-at-a-time.h
> diff --git a/arch/m68k/include/asm/uaccess_mm.h b/arch/m68k/include/asm/uaccess_mm.h
> index 9c80cd5..472c891 100644
> --- a/arch/m68k/include/asm/uaccess_mm.h
> +++ b/arch/m68k/include/asm/uaccess_mm.h
> @@ -379,12 +379,15 @@ __constant_copy_to_user(void __user *to, const void *from, unsigned long n)
>   #define copy_from_user(to, from, n)	__copy_from_user(to, from, n)
>   #define copy_to_user(to, from, n)	__copy_to_user(to, from, n)
>
> -long strncpy_from_user(char *dst, const char __user *src, long count);
> -long strnlen_user(const char __user *src, long n);
> +#define user_addr_max() \
> +	(segment_eq(get_fs(), USER_DS) ? TASK_SIZE : ~0UL)
> +
> +extern long strncpy_from_user(char *dst, const char __user *src, long count);
> +extern __must_check long strlen_user(const char __user *str);
> +extern __must_check long strnlen_user(const char __user *str, long n);
> +
>   unsigned long __clear_user(void __user *to, unsigned long n);
>
>   #define clear_user	__clear_user
>
> -#define strlen_user(str) strnlen_user(str, 32767)
> -
>   #endif /* _M68K_UACCESS_H */
> diff --git a/arch/m68k/lib/uaccess.c b/arch/m68k/lib/uaccess.c
> index 5664386..5e97f2e 100644
> --- a/arch/m68k/lib/uaccess.c
> +++ b/arch/m68k/lib/uaccess.c
> @@ -104,80 +104,6 @@ unsigned long __generic_copy_to_user(void __user *to, const void *from,
>   EXPORT_SYMBOL(__generic_copy_to_user);
>
>   /*
> - * Copy a null terminated string from userspace.
> - */
> -long strncpy_from_user(char *dst, const char __user *src, long count)
> -{
> -	long res;
> -	char c;
> -
> -	if (count<= 0)
> -		return count;
> -
> -	asm volatile ("\n"
> -		"1:	"MOVES".b	(%2)+,%4\n"
> -		"	move.b	%4,(%1)+\n"
> -		"	jeq	2f\n"
> -		"	subq.l	#1,%3\n"
> -		"	jne	1b\n"
> -		"2:	sub.l	%3,%0\n"
> -		"3:\n"
> -		"	.section .fixup,\"ax\"\n"
> -		"	.even\n"
> -		"10:	move.l	%5,%0\n"
> -		"	jra	3b\n"
> -		"	.previous\n"
> -		"\n"
> -		"	.section __ex_table,\"a\"\n"
> -		"	.align	4\n"
> -		"	.long	1b,10b\n"
> -		"	.previous"
> -		: "=d" (res), "+a" (dst), "+a" (src), "+r" (count), "=&d" (c)
> -		: "i" (-EFAULT), "0" (count));
> -
> -	return res;
> -}
> -EXPORT_SYMBOL(strncpy_from_user);
> -
> -/*
> - * Return the size of a string (including the ending 0)
> - *
> - * Return 0 on exception, a value greater than N if too long
> - */
> -long strnlen_user(const char __user *src, long n)
> -{
> -	char c;
> -	long res;
> -
> -	asm volatile ("\n"
> -		"1:	subq.l	#1,%1\n"
> -		"	jmi	3f\n"
> -		"2:	"MOVES".b	(%0)+,%2\n"
> -		"	tst.b	%2\n"
> -		"	jne	1b\n"
> -		"	jra	4f\n"
> -		"\n"
> -		"3:	addq.l	#1,%0\n"
> -		"4:	sub.l	%4,%0\n"
> -		"5:\n"
> -		"	.section .fixup,\"ax\"\n"
> -		"	.even\n"
> -		"20:	sub.l	%0,%0\n"
> -		"	jra	5b\n"
> -		"	.previous\n"
> -		"\n"
> -		"	.section __ex_table,\"a\"\n"
> -		"	.align	4\n"
> -		"	.long	2b,20b\n"
> -		"	.previous\n"
> -		: "=&a" (res), "+d" (n), "=&d" (c)
> -		: "0" (src), "r" (src));
> -
> -	return res;
> -}
> -EXPORT_SYMBOL(strnlen_user);
> -
> -/*
>    * Zero Userspace
>    */
>


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Philippe De Muyter]

On Tue, May 29, 2012 at 11:33:36PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Do we also want
> 
>     select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE && !M68000)

Sorry, I did not follow what happened to unaligned accesses, but
CPU32 family (at least 68340) crashes on unaligned accesses.

Philippe

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Geert Uytterhoeven]

On Wed, May 30, 2012 at 12:22 PM, Philippe De Muyter <phdm@macqel.be> wrote:
> On Tue, May 29, 2012 at 11:33:36PM +0200, Geert Uytterhoeven wrote:
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> Do we also want
>>
>>     select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE && !M68000)
>
> Sorry, I did not follow what happened to unaligned accesses, but
> CPU32 family (at least 68340) crashes on unaligned accesses.

We don't seem to have CONFIG_M68340 in arch/m68k/Kconfig.cpu?
But Freescale's website confirms both 68340 and 68360 are CPU32.

arch/m68k/include/asm/unaligned.h assumes CPU32 (CONFIG_MCPU32)
can do unaligned accesses:

#if defined(CONFIG_COLDFIRE) || defined(CONFIG_M68000)
#include <linux/unaligned/be_struct.h>
#include <linux/unaligned/le_byteshift.h>
#include <linux/unaligned/generic.h>

#define get_unaligned   __get_unaligned_be
#define put_unaligned   __put_unaligned_be

#else
/*
 * The m68k can do unaligned accesses itself.
 */
#include <linux/unaligned/access_ok.h>
#include <linux/unaligned/generic.h>

#define get_unaligned   __get_unaligned_be
#define put_unaligned   __put_unaligned_be

#endif

Is this wrong?

However, for CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS,
the question is not whether unaligned accesses are supported, but
whether they are more efficient than byte copies when copying larger blocks.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Philippe De Muyter]

On Wed, May 30, 2012 at 01:20:02PM +0200, Geert Uytterhoeven wrote:
> On Wed, May 30, 2012 at 12:22 PM, Philippe De Muyter <phdm@macqel.be> wrote:
> > On Tue, May 29, 2012 at 11:33:36PM +0200, Geert Uytterhoeven wrote:
> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> ---
> >> Do we also want
> >>
> >>     select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE && !M68000)
> >
> > Sorry, I did not follow what happened to unaligned accesses, but
> > CPU32 family (at least 68340) crashes on unaligned accesses.
> 
> We don't seem to have CONFIG_M68340 in arch/m68k/Kconfig.cpu?

I have a local port here (but based on an ancient linux kernel, 2.6.2 IIRC)

> But Freescale's website confirms both 68340 and 68360 are CPU32.
> 
> arch/m68k/include/asm/unaligned.h assumes CPU32 (CONFIG_MCPU32)
> can do unaligned accesses:

That's not true.  Accessing a 16- or 32-bit word at an odd address
with a 68340 generates an Address Error Exception.  I remember
discovering a bug in the ppp kernel code because of that.

> 
> #if defined(CONFIG_COLDFIRE) || defined(CONFIG_M68000)
> #include <linux/unaligned/be_struct.h>
> #include <linux/unaligned/le_byteshift.h>
> #include <linux/unaligned/generic.h>
> 
> #define get_unaligned   __get_unaligned_be
> #define put_unaligned   __put_unaligned_be
> 
> #else
> /*
>  * The m68k can do unaligned accesses itself.
>  */
> #include <linux/unaligned/access_ok.h>
> #include <linux/unaligned/generic.h>
> 
> #define get_unaligned   __get_unaligned_be
> #define put_unaligned   __put_unaligned_be
> 
> #endif
> 
> Is this wrong?

I can't tell from reading just the lines above, but I think one should add
"|| defined(CONFIG_MCPU32)" at the end of the if condition.

I also think that the Coldfire 5272 can do unaligned accesses, but I
cannot test that at the moment.

> 
> However, for CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS,
> the question is not whether unaligned accesses are supported, but
> whether they are more efficient than byte copies when copying larger blocks.

OK, thanks

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Andreas Schwab]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> On Wed, May 30, 2012 at 12:22 PM, Philippe De Muyter <phdm@macqel.be> wrote:
>> On Tue, May 29, 2012 at 11:33:36PM +0200, Geert Uytterhoeven wrote:
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>> Do we also want
>>>
>>>     select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE && !M68000)
>>
>> Sorry, I did not follow what happened to unaligned accesses, but
>> CPU32 family (at least 68340) crashes on unaligned accesses.
>
> We don't seem to have CONFIG_M68340 in arch/m68k/Kconfig.cpu?

But we have CONFIG_MCPU32.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Geert Uytterhoeven]

On Wed, May 30, 2012 at 4:04 PM, Philippe De Muyter <phdm@macqel.be> wrote:
> On Wed, May 30, 2012 at 01:20:02PM +0200, Geert Uytterhoeven wrote:
>> On Wed, May 30, 2012 at 12:22 PM, Philippe De Muyter <phdm@macqel.be> wrote:
>> > On Tue, May 29, 2012 at 11:33:36PM +0200, Geert Uytterhoeven wrote:
>> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> >> ---
>> >> Do we also want
>> >>
>> >>     select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE && !M68000)
>> >
>> > Sorry, I did not follow what happened to unaligned accesses, but
>> > CPU32 family (at least 68340) crashes on unaligned accesses.
>>
>> We don't seem to have CONFIG_M68340 in arch/m68k/Kconfig.cpu?
>
> I have a local port here (but based on an ancient linux kernel, 2.6.2 IIRC)
>
>> But Freescale's website confirms both 68340 and 68360 are CPU32.
>>
>> arch/m68k/include/asm/unaligned.h assumes CPU32 (CONFIG_MCPU32)
>> can do unaligned accesses:
>
> That's not true.  Accessing a 16- or 32-bit word at an odd address
> with a 68340 generates an Address Error Exception.  I remember
> discovering a bug in the ppp kernel code because of that.
>
>>
>> #if defined(CONFIG_COLDFIRE) || defined(CONFIG_M68000)
>> #include <linux/unaligned/be_struct.h>
>> #include <linux/unaligned/le_byteshift.h>
>> #include <linux/unaligned/generic.h>
>>
>> #define get_unaligned   __get_unaligned_be
>> #define put_unaligned   __put_unaligned_be
>>
>> #else
>> /*
>>  * The m68k can do unaligned accesses itself.
>>  */
>> #include <linux/unaligned/access_ok.h>
>> #include <linux/unaligned/generic.h>
>>
>> #define get_unaligned   __get_unaligned_be
>> #define put_unaligned   __put_unaligned_be
>>
>> #endif
>>
>> Is this wrong?
>
> I can't tell from reading just the lines above, but I think one should add
> "|| defined(CONFIG_MCPU32)" at the end of the if condition.

Greg?

If more CPUs cannot handle unaligned accesses, I propose to add
CONFIG_CPU_HAS_NO_UNALIGNED.

> I also think that the Coldfire 5272 can do unaligned accesses, but I
> cannot test that at the moment.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Greg Ungerer]

Hi Geert,

On 06/06/12 06:57, Geert Uytterhoeven wrote:
> On Wed, May 30, 2012 at 4:04 PM, Philippe De Muyter<phdm@macqel.be>  wrote:
>> On Wed, May 30, 2012 at 01:20:02PM +0200, Geert Uytterhoeven wrote:
>>> On Wed, May 30, 2012 at 12:22 PM, Philippe De Muyter<phdm@macqel.be>  wrote:
>>>> On Tue, May 29, 2012 at 11:33:36PM +0200, Geert Uytterhoeven wrote:
>>>>> Signed-off-by: Geert Uytterhoeven<geert@linux-m68k.org>
>>>>> ---
>>>>> Do we also want
>>>>>
>>>>> 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (!COLDFIRE&&  !M68000)
>>>>
>>>> Sorry, I did not follow what happened to unaligned accesses, but
>>>> CPU32 family (at least 68340) crashes on unaligned accesses.
>>>
>>> We don't seem to have CONFIG_M68340 in arch/m68k/Kconfig.cpu?
>>
>> I have a local port here (but based on an ancient linux kernel, 2.6.2 IIRC)
>>
>>> But Freescale's website confirms both 68340 and 68360 are CPU32.
>>>
>>> arch/m68k/include/asm/unaligned.h assumes CPU32 (CONFIG_MCPU32)
>>> can do unaligned accesses:
>>
>> That's not true. Accessing a 16- or 32-bit word at an odd address
>> with a 68340 generates an Address Error Exception. I remember
>> discovering a bug in the ppp kernel code because of that.
>>
>>>
>>> #if defined(CONFIG_COLDFIRE) || defined(CONFIG_M68000)
>>> #include<linux/unaligned/be_struct.h>
>>> #include<linux/unaligned/le_byteshift.h>
>>> #include<linux/unaligned/generic.h>
>>>
>>> #define get_unaligned	__get_unaligned_be
>>> #define put_unaligned	__put_unaligned_be
>>>
>>> #else
>>> /*
>>>  * The m68k can do unaligned accesses itself.
>>>  */
>>> #include<linux/unaligned/access_ok.h>
>>> #include<linux/unaligned/generic.h>
>>>
>>> #define get_unaligned	__get_unaligned_be
>>> #define put_unaligned	__put_unaligned_be
>>>
>>> #endif
>>>
>>> Is this wrong?
>>
>> I can't tell from reading just the lines above, but I think one should add
>> "|| defined(CONFIG_MCPU32)" at the end of the if condition.
>
> Greg?
>
> If more CPUs cannot handle unaligned accesses, I propose to add
> CONFIG_CPU_HAS_NO_UNALIGNED.

Yes, looks like that should have a "|| defined(CONFIG_CPU32)".
(According to the CPU32 reference manual words and long words must
be aligned on word boundaries.)

I think something like CONFIG_CPU_HAS_NO_UNALIGNED makes sense.


>> I also think that the Coldfire 5272 can do unaligned accesses, but I
>> cannot test that at the moment.

According to the MCF5272 User Manual, "it supports misaligned data
accesses ...". So it looks like it does.

Having a CONFIG_CPU_HAS_NO_UNALIGNED looks like a really good solution
then. We need to be able select it as required on individual CPU types.

Regards
Greg



------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Geert Uytterhoeven]

On Wed, May 30, 2012 at 4:04 PM, Philippe De Muyter <phdm@macqel.be> wrote:
>> > Sorry, I did not follow what happened to unaligned accesses, but
>> > CPU32 family (at least 68340) crashes on unaligned accesses.
>>
>> We don't seem to have CONFIG_M68340 in arch/m68k/Kconfig.cpu?
>
> I have a local port here (but based on an ancient linux kernel, 2.6.2 IIRC)

Just to be sure: basically include/asm-m68knommu/unaligned.h has always
been wrong (also in 2.6.2), so you had to fix this locally?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Philippe De Muyter]

Hi Geert,

On Wed, Jun 06, 2012 at 03:44:03PM +0200, Geert Uytterhoeven wrote:
> On Wed, May 30, 2012 at 4:04 PM, Philippe De Muyter <phdm@macqel.be> wrote:
> >> > Sorry, I did not follow what happened to unaligned accesses, but
> >> > CPU32 family (at least 68340) crashes on unaligned accesses.
> >>
> >> We don't seem to have CONFIG_M68340 in arch/m68k/Kconfig.cpu?
> >
> > I have a local port here (but based on an ancient linux kernel, 2.6.2 IIRC)
> 
> Just to be sure: basically include/asm-m68knommu/unaligned.h has always
> been wrong (also in 2.6.2), so you had to fix this locally?

I have just verified, and you are right :

	include/asm-m68knommu/unaligned.h was wrong in 2.6.2

But I did not have to fix it locally.

I remember the ppp driver kernel doing by error unaligned accesses
(of course without using unaligned macros) and crashing my kernel.
I fixed the error in the ppp driver and the fix went into the
mainline kernel.

I am not aware that unaligned macros were used in the parts of the
kernel I used on this board.  Just to be sure I made a quick list :

$ find -name \*.o | sed -e 's/\.o$/.c/' | xargs grep t_unaligned 2> /dev/null
./fs/partitions/msdos.c:#define SYS_IND(p) (get_unaligned(&p->sys_ind))
./fs/partitions/msdos.c:                   get_unaligned(&p->nr_sects); \
./fs/partitions/msdos.c:                   get_unaligned(&p->start_sect);      \
./drivers/net/slhc.c:   put_unaligned(ip_fast_csum(icp, ip->ihl),

I am 100% sure I did not use msdos partitions on that board.  I don't
know why msdos.c was compiled in.  For the slhc case, I surmise that it
was not used because its usage depends on some option we did not use.

Best regards

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Geert Uytterhoeven]

Hi Greg,

On Wed, Jun 6, 2012 at 8:31 AM, Greg Ungerer <gerg@snapgear.com> wrote:
> Yes, looks like that should have a "|| defined(CONFIG_CPU32)".
> (According to the CPU32 reference manual words and long words must
> be aligned on word boundaries.)
>
> I think something like CONFIG_CPU_HAS_NO_UNALIGNED makes sense.

OK, doing that now...

Then I saw arch/m68k/lib/memcpy.c.

commit f230e80b423f6cb002015ab4771c06a53d5a2287
("m68k: fix memcpy to unmatched/unaligned source and dest on 68000")

| The original 68000 processors cannot copy 16bit or larger quantities from
| odd addresses. All newer members of the 68k family (including ColdFire)
| can do this.

So all Coldfires can do unaligned _reads_, but not unaligned _writes_
(exceptions below)?

>>> I also think that the Coldfire 5272 can do unaligned accesses, but I
>>> cannot test that at the moment.
>
>
> According to the MCF5272 User Manual, "it supports misaligned data
> accesses ...". So it looks like it does.
>
> Having a CONFIG_CPU_HAS_NO_UNALIGNED looks like a really good solution
> then. We need to be able select it as required on individual CPU types.

For now, I just make COLDFIRE select it, but we can move it to the individual
CPU types later.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Greg Ungerer]

Hi Geert,

On 06/07/2012 03:46 AM, Geert Uytterhoeven wrote:
> On Wed, Jun 6, 2012 at 8:31 AM, Greg Ungerer<gerg@snapgear.com>  wrote:
>> Yes, looks like that should have a "|| defined(CONFIG_CPU32)".
>> (According to the CPU32 reference manual words and long words must
>> be aligned on word boundaries.)
>>
>> I think something like CONFIG_CPU_HAS_NO_UNALIGNED makes sense.
>
> OK, doing that now...
>
> Then I saw arch/m68k/lib/memcpy.c.
>
> commit f230e80b423f6cb002015ab4771c06a53d5a2287
> ("m68k: fix memcpy to unmatched/unaligned source and dest on 68000")
>
> | The original 68000 processors cannot copy 16bit or larger quantities from
> | odd addresses. All newer members of the 68k family (including ColdFire)
> | can do this.
>
> So all Coldfires can do unaligned _reads_, but not unaligned _writes_
> (exceptions below)?

This strikes me as odd. Maybe this has been wrong all along. I need
to check further but in a little testing I did today I think it may
well be that all ColdFire support unaligned reads and writes.


>>>> I also think that the Coldfire 5272 can do unaligned accesses, but I
>>>> cannot test that at the moment.
>>
>>
>> According to the MCF5272 User Manual, "it supports misaligned data
>> accesses ...". So it looks like it does.
>>
>> Having a CONFIG_CPU_HAS_NO_UNALIGNED looks like a really good solution
>> then. We need to be able select it as required on individual CPU types.
>
> For now, I just make COLDFIRE select it, but we can move it to the individual
> CPU types later.

No matter what I find I still think this is the way to go.

Thanks
Greg


------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close,                            FAX:         +61 7 3891 3630
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Andreas Schwab]

Greg Ungerer <gerg@snapgear.com> writes:

> This strikes me as odd. Maybe this has been wrong all along. I need
> to check further but in a little testing I did today I think it may
> well be that all ColdFire support unaligned reads and writes.

The CFPRM doesn't indicate otherwise (the address error exception is
only documented to be raised for a word index addressing mode).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] m68k: Use generic strncpy_from_user(), strlen_user(), and strnlen_user()

[Greg Ungerer]

On 07/06/12 23:14, Andreas Schwab wrote:
> Greg Ungerer<gerg@snapgear.com>  writes:
>
>> This strikes me as odd. Maybe this has been wrong all along. I need
>> to check further but in a little testing I did today I think it may
>> well be that all ColdFire support unaligned reads and writes.
>
> The CFPRM doesn't indicate otherwise (the address error exception is
> only documented to be raised for a word index addressing mode).

Yeah, I can't find anything that says otherwise in any of the CPU
specific data sheets either. I suspect the internals of unaligned.h
have just been carried from the original non-mmu 68k support to
ColdFire.

Testing on the ColdFires I have at hand here shows unaligned reads
and writes working on all. And that includes testing with a modified
unaligned.h that allows unaligned accesses.

Geert, I'll put a patch together that changes this over. I'll make
that on top of your recent patch series.

Regards
Greg


------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com