On Thu, Jan 02, 2014 at 04:47:22PM +0100, Andrew Jones wrote: > On Sat, Dec 28, 2013 at 10:30:35PM -0800, Christoffer Dall wrote: > > > diff --git a/lib/libio.c b/lib/libio.c > > > new file mode 100644 > > > index 0000000000000..e450e984fefb1 > > > --- /dev/null > > > +++ b/lib/libio.c > > > @@ -0,0 +1,67 @@ > > > +#include "libcflat.h" > > > +#include "libio.h" > > > + > > > +void read_len(const volatile void *addr, void *buf, unsigned len) > > > +{ > > > + unsigned long val; > > > + > > > + switch (len) { > > > + case 1: > > > + val = readb(addr); > > > + break; > > > + case 2: > > > + val = readw(addr); > > > + break; > > > + case 4: > > > + val = readl(addr); > > > + break; > > > +#ifdef CONFIG_64BIT > > > + case 8: > > > + val = readq(addr); > > > + break; > > > +#endif > > > + default: > > > + { > > > + u8 *p = buf; > > > + unsigned i; > > > + > > > + for (i = 0; i < len; ++i) > > > + p[i] = readb(addr + i); > > > + return; > > > > are you taking proper care of endianness in the common case? > > Nope, and on second thought I'll change the default to be an error > instead. Something like this doesn't really belong in an io lib. > > > > > > + } > > > + } > > > + memcpy(buf, &val, len); > > > +} > > > + > > > +void write_len(volatile void *addr, const void *buf, unsigned len) > > > +{ > > > + unsigned long val; > > > + > > > + if (len <= sizeof(unsigned long)) > > > + memcpy(&val, buf, len); > > > > Does this work with big-endian for sizes smaller than sizeof(unsigned > > long)? > > Dunno. I didn't test it. Looking at Marc's code makes me suspect > is does not though... > > > > > Take a look at arch/arm/kvm/mmio.c and see how Marc uses a union to > > solve this issue. > > I'll rework this with some shameless theft from Marc. > > > > +/* > > > + * Adapted from the Linux kernel's include/asm-generic/io.h and > > > + * arch/arm/include/asm/io.h > > > + */ > > > +#include "libcflat.h" > > > + > > > +typedef u32 compat_ptr_t; > > > > How is this going to be used here? The compat ptr stuff in the kernel > > deals with user space pointer iirc, but I'm not an expert on the > > details. Does typedef'ing a pointer to a u32 in generic code work on a > > 64-bit platform? > > This is necessary to eliminate compiler warnings when casting 64-bit > pointers to u32s. > ah, ok, a comment to that effect would be nice for us neandertals still mostly stuck in the 32-bit worlds. > > > > > + > > > +static inline void *compat_ptr(compat_ptr_t ptr) > > > +{ > > > + return (void *)(unsigned long)ptr; > > > +} > > > + > > > +static inline compat_ptr_t ptr_to_compat(void *ptr) > > > +{ > > > + return (u32)(unsigned long)ptr; > > > +} > > > + > > > +#ifndef __raw_readb > > > +static inline u8 __raw_readb(const volatile void *addr) > > > +{ > > > + return *(const volatile u8 *)addr; > > > +} > > > +#endif > > > + > > > +#ifndef __raw_readw > > > +static inline u16 __raw_readw(const volatile void *addr) > > > +{ > > > + return *(const volatile u16 *)addr; > > > +} > > > +#endif > > > + > > > +#ifndef __raw_readl > > > +static inline u32 __raw_readl(const volatile void *addr) > > > +{ > > > + return *(const volatile u32 *)addr; > > > +} > > > +#endif > > > + > > > +#ifdef CONFIG_64BIT > > > +#ifndef __raw_readq > > > +static inline u64 __raw_readq(const volatile void *addr) > > > +{ > > > + return *(const volatile u64 *)addr; > > > +} > > > +#endif > > > +#endif > > > + > > > +#ifndef __raw_writeb > > > +static inline void __raw_writeb(u8 b, volatile void *addr) > > > +{ > > > + *(volatile u8 *)addr = b; > > > +} > > > +#endif > > > + > > > +#ifndef __raw_writew > > > +static inline void __raw_writew(u16 b, volatile void *addr) > > > +{ > > > + *(volatile u16 *)addr = b; > > > +} > > > +#endif > > > + > > > +#ifndef __raw_writel > > > +static inline void __raw_writel(u32 b, volatile void *addr) > > > +{ > > > + *(volatile u32 *)addr = b; > > > +} > > > +#endif > > > + > > > +#ifdef CONFIG_64BIT > > > +#ifndef __raw_writeq > > > +static inline void __raw_writeq(u64 b, volatile void *addr) > > > +{ > > > + *(volatile u64 *)addr = b; > > > +} > > > +#endif > > > +#endif > > > + > > > +#ifndef __bswap16 > > > +static inline u16 __bswap16(u16 x) > > > +{ > > > + return ((x >> 8) & 0xff) | ((x & 0xff) << 8); > > > +} > > > +#endif > > > + > > > +#ifndef __bswap32 > > > +static inline u32 __bswap32(u32 x) > > > +{ > > > + return ((x & 0xff000000) >> 24) | ((x & 0x00ff0000) >> 8) | > > > + ((x & 0x0000ff00) << 8) | ((x & 0x000000ff) << 24); > > > +} > > > +#endif > > > + > > > +#ifdef CONFIG_64BIT > > > +#ifndef __bswap64 > > > +static inline u64 __bswap64(u64 x) > > > +{ > > > + return ((x & 0x00000000000000ffULL) << 56) | > > > + ((x & 0x000000000000ff00ULL) << 40) | > > > + ((x & 0x0000000000ff0000ULL) << 24) | > > > + ((x & 0x00000000ff000000ULL) << 8) | > > > + ((x & 0x000000ff00000000ULL) >> 8) | > > > + ((x & 0x0000ff0000000000ULL) >> 24) | > > > + ((x & 0x00ff000000000000ULL) >> 40) | > > > + ((x & 0xff00000000000000ULL) >> 56); > > > +} > > > +#endif > > > +#endif > > > + > > > +#ifndef cpu_is_be > > > +#define cpu_is_be 0 > > > +#endif > > > + > > > +#define le16_to_cpu(x) \ > > > + ({ u16 __r = cpu_is_be ? __bswap16(x) : (x); __r; }) > > > +#define cpu_to_le16 le16_to_cpu > > > + > > > +#define le32_to_cpu(x) \ > > > + ({ u32 __r = cpu_is_be ? __bswap32(x) : (x); __r; }) > > > +#define cpu_to_le32 le32_to_cpu > > > + > > > +#ifdef CONFIG_64BIT > > > +#define le64_to_cpu \ > > > + ({ u64 __r = cpu_is_be ? __bswap64(x) : (x); __r; }) > > > +#define cpu_to_le64 le64_to_cpu > > > +#endif > > > + > > > +#define be16_to_cpu(x) \ > > > + ({ u16 __r = !cpu_is_be ? __bswap16(x) : (x); __r; }) > > > +#define cpu_to_be16 be16_to_cpu > > > + > > > +#define be32_to_cpu(x) \ > > > + ({ u32 __r = !cpu_is_be ? __bswap32(x) : (x); __r; }) > > > +#define cpu_to_be32 be32_to_cpu > > > + > > > +#ifdef CONFIG_64BIT > > > +#define be64_to_cpu \ > > > + ({ u64 __r = !cpu_is_be ? __bswap64(x) : (x); __r; }) > > > +#define cpu_to_be64 be64_to_cpu > > > +#endif > > > + > > > +#ifndef rmb > > > +#define rmb() do { } while (0) > > > +#endif > > > +#ifndef wmb > > > +#define wmb() do { } while (0) > > > +#endif > > > + > > > +#define readb(addr) \ > > > + ({ u8 __r = __raw_readb(addr); rmb(); __r; }) > > > +#define readw(addr) \ > > > + ({ u16 __r = le16_to_cpu(__raw_readw(addr)); rmb(); __r; }) > > > +#define readl(addr) \ > > > + ({ u32 __r = le32_to_cpu(__raw_readl(addr)); rmb(); __r; }) > > > +#ifdef CONFIG_64BIT > > > +#define readq(addr) \ > > > + ({ u64 __r = le64_to_cpu(__raw_readq(addr)); rmb(); __r; }) > > > +#endif > > > + > > > +#define writeb(b, addr) \ > > > + ({ wmb(); __raw_writeb(b, addr); }) > > > +#define writew(b, addr) \ > > > + ({ wmb(); __raw_writew(cpu_to_le16(b), addr); }) > > > +#define writel(b, addr) \ > > > + ({ wmb(); __raw_writel(cpu_to_le32(b), addr); }) > > > +#ifdef CONFIG_64BIT > > > +#define writeq(b, addr) \ > > > + ({ wmb(); __raw_writeq(cpu_to_le64(b), addr); }) > > > +#endif > > > + > > > +extern void read_len(const volatile void *addr, void *buf, unsigned len); > > > +extern void write_len(volatile void *addr, const void *buf, unsigned len); > > > + > > > +#endif > > > -- > > > 1.8.1.4 > > > > I didn't review all these defines and raw_ functions carefully as I > > assume the individual functions and defines are copied verbatim from the > > kernel? > > > > You should take a look, as they're not verbatim - although almost. I can't > recall where I might have diverged, I just know I didn't copy+paste :-) > Wonderful, I'll take a look :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html