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.
>
> > +
> > +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 :-)
drew
--
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
