Instead of assuming that the system can safely do unaligned access to memory use packed structures to allow the compiler generate best code possible. A packed structure tells the compiler to not leave padding inside it and that the structure can be unaligned so any field can be unaligned having to generate proper access code based on architecture. For instance ARM7 can use unaligned access but not for 64 bit numbers (currently these accesses are emulated by Linux kernel with obvious performance consequences). This changes the current methods from: #ifdef WORDS_BIGENDIAN #define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(*((uint32_t *)(ptr)))) #define write_uint32(ptr, val) *(uint32_t *)(ptr) = SPICE_BYTESWAP32((uint32_t)val) #else #define read_uint32(ptr) (*((uint32_t *)(ptr))) #define write_uint32(ptr, val) (*((uint32_t *)(ptr))) = val #endif to: #include <spice/start-packed.h> typedef struct SPICE_ATTR_PACKED { uint32_t v; } uint32_unaligned_t; #include <spice/end-packed.h> #ifdef WORDS_BIGENDIAN #define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(((uint32_unaligned_t *)(ptr))->v)) #define write_uint32(ptr, val) ((uint32_unaligned_t *)(ptr))->v = SPICE_BYTESWAP32((uint32_t)val) #else #define read_uint32(ptr) (((uint32_unaligned_t *)(ptr))->v) #define write_uint32(ptr, val) (((uint32_unaligned_t *)(ptr))->v) = val #endif Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- common/marshaller.c | 57 ++++++++++++++++++++++++++++++++------------- python_modules/demarshal.py | 33 ++++++++++++++++++-------- 2 files changed, 64 insertions(+), 26 deletions(-) Changes since v1: - add same code to marshaller. diff --git a/common/marshaller.c b/common/marshaller.c index 2fccd52..adfb8cd 100644 --- a/common/marshaller.c +++ b/common/marshaller.c @@ -28,24 +28,49 @@ #include <unistd.h> #include <stdio.h> +#include <spice/start-packed.h> +typedef struct SPICE_ATTR_PACKED { + int16_t val; +} int16_unaligned_t; + +typedef struct SPICE_ATTR_PACKED { + uint16_t val; +} uint16_unaligned_t; + +typedef struct SPICE_ATTR_PACKED { + int32_t val; +} int32_unaligned_t; + +typedef struct SPICE_ATTR_PACKED { + uint32_t val; +} uint32_unaligned_t; + +typedef struct SPICE_ATTR_PACKED { + int64_t val; +} int64_unaligned_t; + +typedef struct SPICE_ATTR_PACKED { + uint64_t val; +} uint64_unaligned_t; +#include <spice/end-packed.h> + +#define write_int8(ptr,v) (*(int8_t *)(ptr) = v) +#define write_uint8(ptr,v) (*(uint8_t *)(ptr) = v) + #ifdef WORDS_BIGENDIAN -#define write_int8(ptr,v) (*((int8_t *)(ptr)) = v) -#define write_uint8(ptr,v) (*((uint8_t *)(ptr)) = v) -#define write_int16(ptr,v) (*((int16_t *)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v))) -#define write_uint16(ptr,v) (*((uint16_t *)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v))) -#define write_int32(ptr,v) (*((int32_t *)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v))) -#define write_uint32(ptr,v) (*((uint32_t *)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v))) -#define write_int64(ptr,v) (*((int64_t *)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v))) -#define write_uint64(ptr,v) (*((uint64_t *)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v))) +#define write_int16(ptr,v) (((uint16_unaligned_t *)(ptr))->val = SPICE_BYTESWAP16((uint16_t)(v))) +#define write_uint16(ptr,v) (((uint16_unaligned_t *)(ptr))->val = SPICE_BYTESWAP16((uint16_t)(v))) +#define write_int32(ptr,v) (((uint32_unaligned_t *)(ptr))->val = SPICE_BYTESWAP32((uint32_t)(v))) +#define write_uint32(ptr,v) (((uint32_unaligned_t *)(ptr))->val = SPICE_BYTESWAP32((uint32_t)(v))) +#define write_int64(ptr,v) (((uint64_unaligned_t *)(ptr))->val = SPICE_BYTESWAP64((uint64_t)(v))) +#define write_uint64(ptr,v) (((uint64_unaligned_t *)(ptr))->val = SPICE_BYTESWAP64((uint64_t)(v))) #else -#define write_int8(ptr,v) (*((int8_t *)(ptr)) = v) -#define write_uint8(ptr,v) (*((uint8_t *)(ptr)) = v) -#define write_int16(ptr,v) (*((int16_t *)(ptr)) = v) -#define write_uint16(ptr,v) (*((uint16_t *)(ptr)) = v) -#define write_int32(ptr,v) (*((int32_t *)(ptr)) = v) -#define write_uint32(ptr,v) (*((uint32_t *)(ptr)) = v) -#define write_int64(ptr,v) (*((int64_t *)(ptr)) = v) -#define write_uint64(ptr,v) (*((uint64_t *)(ptr)) = v) +#define write_int16(ptr,v) (((int16_unaligned_t *)(ptr))->val = v) +#define write_uint16(ptr,v) (((uint16_unaligned_t *)(ptr))->val = v) +#define write_int32(ptr,v) (((int32_unaligned_t *)(ptr))->val = v) +#define write_uint32(ptr,v) (((uint32_unaligned_t *)(ptr))->val = v) +#define write_int64(ptr,v) (((int64_unaligned_t *)(ptr))->val = v) +#define write_uint64(ptr,v) (((uint64_unaligned_t *)(ptr))->val = v) #endif typedef struct { diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py index 1ea131d..da87d44 100644 --- a/python_modules/demarshal.py +++ b/python_modules/demarshal.py @@ -40,24 +40,37 @@ def write_parser_helpers(writer): writer = writer.function_helper() + writer.writeln("#include <spice/start-packed.h>") + for size in [16, 32, 64]: + for sign in ["", "u"]: + type = "%sint%d" % (sign, size) + writer.begin_block("typedef struct SPICE_ATTR_PACKED") + writer.variable_def("%s_t" % type, "v") + writer.end_block(newline=False) + writer.writeln(" %s_unaligned_t;" % type) + writer.writeln("#include <spice/end-packed.h>") + writer.newline() + + for sign in ["", "u"]: + type = "%sint8" % sign + writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type) + writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) = val" % (type)) + writer.newline() + writer.writeln("#ifdef WORDS_BIGENDIAN") - for size in [8, 16, 32, 64]: + for size in [16, 32, 64]: for sign in ["", "u"]: utype = "uint%d" % (size) type = "%sint%d" % (sign, size) swap = "SPICE_BYTESWAP%d" % size - if size == 8: - writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type) - writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) = val" % (type)) - else: - writer.macro("read_%s" % type, "ptr", "((%s_t)%s(*((%s_t *)(ptr))))" % (type, swap, utype)) - writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) = %s((%s_t)val)" % (utype, swap, utype)) + writer.macro("read_%s" % type, "ptr", "((%s_t)%s(((%s_unaligned_t *)(ptr))->v))" % (type, swap, utype)) + writer.macro("write_%s" % type, "ptr, val", "((%s_unaligned_t *)(ptr))->v = %s((%s_t)val)" % (utype, swap, utype)) writer.writeln("#else") - for size in [8, 16, 32, 64]: + for size in [16, 32, 64]: for sign in ["", "u"]: type = "%sint%d" % (sign, size) - writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type) - writer.macro("write_%s" % type, "ptr, val", "(*((%s_t *)(ptr))) = val" % type) + writer.macro("read_%s" % type, "ptr", "(((%s_unaligned_t *)(ptr))->v)" % type) + writer.macro("write_%s" % type, "ptr, val", "(((%s_unaligned_t *)(ptr))->v) = val" % type) writer.writeln("#endif") for size in [8, 16, 32, 64]: -- 2.13.5 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel