[PATCH V3 2/3] Implement byte-by-byte memory access facilitators

[PATCH V3 2/3] Implement byte-by-byte memory access facilitators

[Dave Kleikamp]

Sparc64 requires type-aligned memory access. Since data buffers may not
be properly aligned, implement a safe copy from memory per data type.

Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
---
 defs.h |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/defs.h b/defs.h
index e3afc58..d9d4559 100644
--- a/defs.h
+++ b/defs.h
@@ -2187,6 +2187,45 @@ struct builtin_debug_table {
  *  Facilitators for pulling correctly-sized data out of a buffer at a
  *  known address. 
  */
+
+#ifdef NEED_ALIGNED_MEM_ACCESS
+
+#define DEF_LOADER(TYPE)			\
+static inline TYPE				\
+load_##TYPE (char *addr)			\
+{						\
+	TYPE ret;				\
+	size_t i = sizeof(TYPE);		\
+	while (i--)				\
+		((char *)&ret)[i] = addr[i];	\
+	return ret;				\
+}
+
+DEF_LOADER(int);
+DEF_LOADER(uint);
+DEF_LOADER(long);
+DEF_LOADER(ulong);
+DEF_LOADER(ulonglong);
+DEF_LOADER(ushort);
+DEF_LOADER(short);
+typedef void *pointer_t;
+DEF_LOADER(pointer_t);
+
+#define LOADER(TYPE) load_##TYPE
+
+#define INT(ADDR)       LOADER(int) ((char *)(ADDR))
+#define UINT(ADDR)      LOADER(uint) ((char *)(ADDR))
+#define LONG(ADDR)      LOADER(long) ((char *)(ADDR))
+#define ULONG(ADDR)     LOADER(ulong) ((char *)(ADDR))
+#define ULONGLONG(ADDR) LOADER(ulonglong) ((char *)(ADDR))
+#define ULONG_PTR(ADDR) ((ulong *) (LOADER(pointer_t) ((char *)(ADDR))))
+#define USHORT(ADDR)    LOADER(ushort) ((char *)(ADDR))
+#define SHORT(ADDR)     LOADER(short) ((char *)(ADDR))
+#define UCHAR(ADDR)     *((unsigned char *)((char *)(ADDR)))
+#define VOID_PTR(ADDR)  ((void *) (LOADER(pointer_t) ((char *)(ADDR))))
+
+#else
+
 #define INT(ADDR)       *((int *)((char *)(ADDR)))
 #define UINT(ADDR)      *((uint *)((char *)(ADDR)))
 #define LONG(ADDR)      *((long *)((char *)(ADDR)))
@@ -2198,6 +2237,8 @@ struct builtin_debug_table {
 #define UCHAR(ADDR)     *((unsigned char *)((char *)(ADDR)))
 #define VOID_PTR(ADDR)  *((void **)((char *)(ADDR)))
 
+#endif /* NEED_ALIGNED_MEM_ACCESS */
+
 struct node_table {
 	int node_id;
 	ulong pgdat;
-- 
1.7.1

Re: [PATCH V3 2/3] Implement byte-by-byte memory access facilitators

[Sam Ravnborg]

Hi Dave.

> Sparc64 requires type-aligned memory access. Since data buffers may not
> be properly aligned, implement a safe copy from memory per data type.
> 
> Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
> ---
>  defs.h |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index e3afc58..d9d4559 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2187,6 +2187,45 @@ struct builtin_debug_table {
>   *  Facilitators for pulling correctly-sized data out of a buffer at a
>   *  known address. 
>   */
> +
> +#ifdef NEED_ALIGNED_MEM_ACCESS
> +
> +#define DEF_LOADER(TYPE)			\
> +static inline TYPE				\
> +load_##TYPE (char *addr)			\
> +{						\
> +	TYPE ret;				\
> +	size_t i = sizeof(TYPE);		\
> +	while (i--)				\
> +		((char *)&ret)[i] = addr[i];	\
> +	return ret;				\
> +}
> +
> +DEF_LOADER(int);
> +DEF_LOADER(uint);
> +DEF_LOADER(long);
> +DEF_LOADER(ulong);
> +DEF_LOADER(ulonglong);
> +DEF_LOADER(ushort);
> +DEF_LOADER(short);
> +typedef void *pointer_t;
> +DEF_LOADER(pointer_t);
> +
> +#define LOADER(TYPE) load_##TYPE
A simpler model would be
#define LOADER(TYPE, addr)                         \
({                                                 \
	TYPE ret;                                  \
	memcpy(&ret, (void *)addr, sizeof(TYPE));  \
	ret;                                       \
})

I would expect the compiler to optimize this away for archs
that do not require aligned access.

Quick test:

#include <string.h>
#include <stdio.h>

#define LOADER(TYPE, addr)                       \
({                                               \
        TYPE ret;                                \
        memcpy(&ret, (void*)addr, sizeof(TYPE)); \
        ret;                                     \
})

#define INT(ADDR)       LOADER(int, ADDR)

void main(void)
{
	int i = 37;
	int j = -17;

        printf("i=%d j=%d\n", INT(&i), INT(&j));
}


	Sam

Re: [PATCH V3 2/3] Implement byte-by-byte memory access facilitators

[Dave Kleikamp]

On 04/25/2016 03:48 PM, Sam Ravnborg wrote:
> Hi Dave.
> 
>> Sparc64 requires type-aligned memory access. Since data buffers may not
>> be properly aligned, implement a safe copy from memory per data type.
>>
>> Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
>> ---
>>  defs.h |   41 +++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 41 insertions(+), 0 deletions(-)
>>
>> diff --git a/defs.h b/defs.h
>> index e3afc58..d9d4559 100644
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -2187,6 +2187,45 @@ struct builtin_debug_table {
>>   *  Facilitators for pulling correctly-sized data out of a buffer at a
>>   *  known address. 
>>   */
>> +
>> +#ifdef NEED_ALIGNED_MEM_ACCESS
>> +
>> +#define DEF_LOADER(TYPE)			\
>> +static inline TYPE				\
>> +load_##TYPE (char *addr)			\
>> +{						\
>> +	TYPE ret;				\
>> +	size_t i = sizeof(TYPE);		\
>> +	while (i--)				\
>> +		((char *)&ret)[i] = addr[i];	\
>> +	return ret;				\
>> +}
>> +
>> +DEF_LOADER(int);
>> +DEF_LOADER(uint);
>> +DEF_LOADER(long);
>> +DEF_LOADER(ulong);
>> +DEF_LOADER(ulonglong);
>> +DEF_LOADER(ushort);
>> +DEF_LOADER(short);
>> +typedef void *pointer_t;
>> +DEF_LOADER(pointer_t);
>> +
>> +#define LOADER(TYPE) load_##TYPE
> A simpler model would be
> #define LOADER(TYPE, addr)                         \
> ({                                                 \
> 	TYPE ret;                                  \
> 	memcpy(&ret, (void *)addr, sizeof(TYPE));  \
> 	ret;                                       \
> })

This does simplify things a bit.

> 
> I would expect the compiler to optimize this away for archs
> that do not require aligned access.

I hesitate to to replace the macros for every architecture with this.
I'll let Dave A. make that call, but I think it can improve the
NEED_ALIGNED_MEM_ACCESS case.

> 
> Quick test:
> 
> #include <string.h>
> #include <stdio.h>
> 
> #define LOADER(TYPE, addr)                       \
> ({                                               \
>         TYPE ret;                                \
>         memcpy(&ret, (void*)addr, sizeof(TYPE)); \
>         ret;                                     \
> })
> 
> #define INT(ADDR)       LOADER(int, ADDR)
> 
> void main(void)
> {
> 	int i = 37;
> 	int j = -17;
> 
>         printf("i=%d j=%d\n", INT(&i), INT(&j));
> }
> 
> 
> 	Sam
> 

Re: [PATCH V3 2/3] Implement byte-by-byte memory access facilitators

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Mon, 25 Apr 2016 22:48:45 +0200

> A simpler model would be
> #define LOADER(TYPE, addr)                         \
> ({                                                 \
> 	TYPE ret;                                  \
> 	memcpy(&ret, (void *)addr, sizeof(TYPE));  \
> 	ret;                                       \
> })
> 
> I would expect the compiler to optimize this away for archs
> that do not require aligned access.

The compiler is allowed to "look through" void pointer casts and
use the alignment guaranteed by the original types.

That is why memcpy()'s aren't used for unaligned access handling.

Take a look at include/linux/unaligned/packed_struct.h in the kernel
sources, which is in my opinion the most portable and efficient way to
deal with this problem.

That code does the right thing on both architectures where alignment
matters, and where it does not.